[sr-dev] dispatcher: limit a number of dst to failover to
Daniel-Constantin Mierla
miconda at gmail.com
Tue Aug 26 12:45:28 CEST 2014
Thanks for the patch. One note about -- use tab based indentation for
patches to existing modules, to keep the code looking coherent. I can
apply the patch and reset the formatting this time, but have in mind for
the future.
Otherwise, patch looks ok.
Cheers,
Daniel
On 25/08/14 15:32, Alekzander Spiridonov wrote:
> Patch for flag parsing attached.
>
>
> 2014-08-22 18:48 GMT+04:00 Alekzander Spiridonov <alekz at li.ru
> <mailto:alekz at li.ru>>:
>
> Ok, so at the moment src supports the following options:
> if (len>1 && (str1[1]=='p' || str1[1]=='P'))
> state |= DS_PROBING_DST;
> if(str1[0]=='i' || str1[0]=='I')
> state |= DS_INACTIVE_DST;
> else if(str1[0]=='t' || str1[0]=='T')
> state |= DS_TRYING_DST;
> else if(str1[0]=='d' || str1[0]=='D')
> state = DS_DISABLED_DST;
> else if(str1[0]=='p' || str1[0]=='P')
> state = DS_INACTIVE_DST|DS_PROBING_DST;
>
> I do believe that flag still should be single letter, so I'd
> suggest removing the first "if" since we still have the last "else
> if". What do you think about that?
>
> Also I'd add "A" option to this list.
>
>
> 2014-08-22 18:26 GMT+04:00 Daniel-Constantin Mierla
> <miconda at gmail.com <mailto:miconda at gmail.com>>:
>
>
> On 22/08/14 13:32, Alekzander Spiridonov wrote:
>> One more thing I've figured out:
>> In dispatcher module guide there is a way to mark dst as
>> Active, though in src w_ds_mark_dst1 has no such option.
>>
>> On the other hand, w_ds_mark_dst1 has an option to mark dst
>> as "admin disabled" that is not documented in the guide.
>>
>> Could you clarify what is the expected behavior so I could
>> send a fix for that.
>
> The purpose of that function is to be able to set a
> destination in one of supported states:
>
> - completely disabled (admin disabled) -- I guess is 'd' --
> not used and no chance to activate it automatically upon a
> successful probing request (actually no probing will be done)
> - inactive state - is not used currently, but if in probing
> mode, it can be activated automatically if a probe request is
> answered
> - active state - used currently
>
> The probing mode that comes as a flag, is related to whether
> to send periodically (probe) requests to check if targets are
> online.
>
> Trying wasn't coded by me (iirc :-) ), but I think the purpose
> was to still keep the target in active mode until a certain
> number of failures occur and then make it inactive (or disabled).
>
> If you find discrepancies in the code, let us know -- of
> course, patches are more than welcome!
>
> Cheers,
> Daniel
>
>
>>
>>
>> 2014-08-22 13:25 GMT+04:00 Alekzander Spiridonov <alekz at li.ru
>> <mailto:alekz at li.ru>>:
>>
>> Hi Daniel,
>>
>> 1. Yep, had the same concern, but somehow missed the
>> app_lua. Fixed as you've described.
>>
>> 2. Done.
>>
>> New patch attached.
>>
>>
>> 2014-08-21 20:19 GMT+04:00 Daniel-Constantin Mierla
>> <miconda at gmail.com <mailto:miconda at gmail.com>>:
>>
>> Hello,
>>
>> thanks for sending the patch! Two remarks for now:
>>
>> 1) you changed the inter-module API prototype for
>> ds_select_dst_f, respectively:
>>
>> typedef int (*ds_select_dst_f)(struct sip_msg *msg,
>> int set,
>> - int alg, int mode);
>> + int alg, unsigned int limit, int mode);
>>
>> I think the reason for that function is an export to
>> app_lua (and maybe other embedded languages). With
>> this change, things get broken there. I would do:
>>
>> - rename the function ds_select_dst() to
>> ds_select_dst_limit()
>> - add ds_select_dst() as a wrapper function calling
>> the new ds_select_dst_limit()
>>
>> The API stay unchanged, and eventually one can add a
>> new member for ds_select_dst_limit() when needed
>>
>> Otherwise, the change of ds_select_dst() prototype
>> should involve reviewing other modules and seeing if
>> people are happy with this change, because it breaks
>> existing embedded scripts.
>>
>> 2) You need to document the new functions exported to
>> kamailio config, in
>> modules/dispatcher/doc/dispatcher_admin.xml
>>
>> Once the patch is overall ok, I can commit it to the
>> repository.
>>
>> Cheers,
>> Daniel
>>
>>
>> On 21/08/14 17:40, Alekzander Spiridonov wrote:
>>> Hi list,
>>>
>>> I'd like to propose a patch that allows user to
>>> limit the number of items that appears in dst_avp
>>> after ds_select_dst or ds_select_domain call.
>>>
>>> There are some use cases when one could have couple
>>> of dst sets and would like to failover from the
>>> first to another without checking all items but have
>>> just 3 or 4 of them dead in a row.
>>>
>>> What do you think about that?
>>>
>>> --
>>> Best regards,
>>> Alekzander Spiridonov
>>>
>>>
>>>
>>> _______________________________________________
>>> sr-dev mailing list
>>> sr-dev at lists.sip-router.org <mailto:sr-dev at lists.sip-router.org>
>>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>>
>> --
>> Daniel-Constantin Mierla
>> http://twitter.com/#!/miconda <http://twitter.com/#%21/miconda> -http://www.linkedin.com/in/miconda
>> Next Kamailio Advanced Trainings 2014 -http://www.asipto.com
>> Sep 22-25, Berlin, Germany ::: Oct 15-17, San Francisco, USA
>>
>>
>> _______________________________________________
>> sr-dev mailing list
>> sr-dev at lists.sip-router.org
>> <mailto:sr-dev at lists.sip-router.org>
>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>>
>>
>>
>>
>> --
>> Best regards,
>> Alekzander Spiridonov
>>
>>
>>
>>
>> --
>> Best regards,
>> Alekzander Spiridonov
>>
>
> --
> Daniel-Constantin Mierla
> http://twitter.com/#!/miconda <http://twitter.com/#%21/miconda> -http://www.linkedin.com/in/miconda
> Next Kamailio Advanced Trainings 2014 -http://www.asipto.com
> Sep 22-25, Berlin, Germany ::: Oct 15-17, San Francisco, USA
>
>
>
>
> --
> Best regards,
> Alekzander Spiridonov
>
>
>
>
> --
> Best regards,
> Alekzander Spiridonov
>
--
Daniel-Constantin Mierla
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Next Kamailio Advanced Trainings 2014 - http://www.asipto.com
Sep 22-25, Berlin, Germany ::: Oct 15-17, San Francisco, USA
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20140826/08d2642d/attachment-0001.html>
More information about the sr-dev
mailing list