[kamailio/kamailio] Dispatcher: added two new flags to mode parameter of ds_is_from_list function for more strictly matching (PR #3699)
<!-- Kamailio Pull Request Template -->
<!-- IMPORTANT: - for detailed contributing guidelines, read: https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md - pull requests must be done to master branch, unless they are backports of fixes from master branch to a stable branch - backports to stable branches must be done with 'git cherry-pick -x ...' - code is contributed under BSD for core and main components (tm, sl, auth, tls) - code is contributed GPLv2 or a compatible license for the other components - GPL code is contributed with OpenSSL licensing exception -->
#### Pre-Submission Checklist <!-- Go over all points below, and after creating the PR, tick all the checkboxes that apply --> <!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from above--> <!-- If you're unsure about any of these, don't hesitate to ask on sr-dev mailing list --> - [ ] Commit message has the format required by CONTRIBUTING guide - [ ] Commits are split per component (core, individual modules, libs, utils, ...) - [ ] Each component has a single commit (if not, squash them into one commit) - [ ] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [ ] Small bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [ ] PR should be backported to stable branches - [ ] Tested changes locally - [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description
This PR adds 2 new flags to the mode parameter of the ds_is_from_list function.
Some times the dispatcher configuration can look like this (route attribute is just for exampe, sockname is important here):
70 sip:192.168.1.1:5060;transport=udp 0 1 route=route1;sockname=udp_5060 71 sip:192.168.1.1:5060;transport=udp 0 1 route=route2;sockname=udp_5061 72 sip:192.168.1.1:5060;transport=udp 0 1 route=route3;sockname=udp_5062 73 sip:192.168.1.1:5060;transport=udp 0 1 route=route4;sockname=udp_5063
Here we have one host/port/protocol on the remote side and different ports that we are listening on, In stock dispatcher version ds_is_from_list(-1,0) will always match first available - 70, but this is not right for dst ports 5061-5063 (sockets in realality, because we can have different local ip on same port also). The new mode flag DS_MATCH_SOCKET (8) allow match for dispatcher socket also.
Second flag is DS_MATCH_STRICTEST (16), it allow to match more strictness target in "address/protocol/port/socket" key, for example:
70 sip:192.168.1.1;transport=udp 0 1 route=route1 71 sip:192.168.1.1:5061;transport=udp 0 1 route=route2 72 sip:192.168.1.1:5062;transport=udp 0 1 route=route3 73 sip:192.168.1.1;transport=udp 0 1 route=route4;socket=udp:192.168.10.10:5063
if packet come from from any port of sip:192.168.1.1;transport=udp except 5061-5062 ds_is_from list will return 70, if received port also is 5063 the result will be 73, packets from ports 5061-5062 will be matched at 71-72 respectively. In stock version of ds_is_from_list the result will always be 70.
Note, the DS_MATCH_STRICTEST method can be more compute intensive, because we can potentially traverse all the dispatcher tree.
The behavior of existing flags are not changed.
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/3699
-- Commit Summary --
* dispatcher: added two new flags to mode parameter of ds_is_from_list function for more strictly matching
-- File Changes --
M src/modules/dispatcher/dispatch.c (112) M src/modules/dispatcher/dispatch.h (2)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/3699.patch https://github.com/kamailio/kamailio/pull/3699.diff
Thanks for the PR!
I haven't done a full review, but first, you have to add documentation to the xml fine in the doc/ subfolder. It helps to understand how is supposed to be used and see if follows the code.
Also, the DS_MATCH_STRICTEST should be changed to try to reflect better what kind of match is done, I am sure one can think of another match mode more stricter, to include more than ip/port/proto/socket. Maybe DS_MATCH_FULLADDRSOCK.
I've not done any testing myself, but by my understanding the DS_MATCH_STRICTEST flag is basically a flag to indicate it should not return on the first match but scan through all. Then sort and return the one that is matching one most criterias?
Maybe DS_MATCH_FULLADDRSOCK.
OK, i will change the flag name, need some time to think about adequate name, writing docs hope can help with this.
I've not done any testing myself, but by my understanding the DS_MATCH_STRICTEST flag is basically a flag to indicate it should not return on the first match but scan through all. Then sort and return the one that is matching one most criterias?
Right, but not exactly, there is no sorting as separate entity, during the traverse of tree the gobal variable filled with strictess flags (which are the bitmask of sip uri components - addr/proto/port and local socket) and more strictess node is saved in process global variable, if full set of possible flags is found, the cycle is stopped (we found full match and no more iterations needed), when traverse pocess finished (resursive method in original code) and we are not found full match, the global vars will contains more strictness node (if found at least address match), from this global vars then making out the final result of ds_is_from_list.
@Den4t pushed 1 commit.
53ae14294f021ae615c0fae6ec9b48758d9a051e dispatcher: added two new flags to mode parameter of ds_is_from_list function for more strictly matching
I have changed the flag name and added docs to xml.
If I understand the code correctly, a destination record that matches only the socket is selected against one that matches address/port/proto? Is it like that and if yes, is it the expected result?
If I understand the code correctly, a destination record that matches only the socket is selected against one that matches address/port/proto? Is it like that and if yes, is it the expected result?
Yes, it is a separate flag - DS_MATCH_SOCKET (8), it work like ds_is_from_list(-1,0), but socket is take in account also (if specified in the configuration).
I meant for the case/flag DS_MATCH_FULLADDRSOCK: a destination record that matches **only** the socket is selected against one that matches address+port+proto.
I meant for the case/flag DS_MATCH_FULLADDRSOCK: a destination record that matches **only** the socket is selected against one that matches address+port+proto.
Using DS_MATCH_TRY_FULL ADDR SOCK, at least an ip must also be matched, and in this mode we do not interrupt the serach cycle, trying to find a more complete match, so the ip+socket combination will be valid, but not only the socket match. If i am right understand the question.
Somehow I got it that the destination that has the most matches is selected. As we have ip. port, protocol and socket, I thought that if a record matches three attributes (ip, port, protocol) is selected over another record that matches two attributes (ip, socket). But as I could understand from the condition:
``` if(ds_strictness < node_strictness) { ds_strictness = node_strictness; ds_strictest_node = node; ds_strictest_idx = j; } ```
because of the flag for socket matching, the record matching (ip, socket) is selected. Is this the desired behaviour. Somehow it has to be clear in the docs how is the destination record selected.
Hi! Sorry, I was sick last week...
because of the flag for socket matching, record matching (ip, socket) is selected. Is this the desired behavior.
Yes, this is the desired behavior, because we can only have a target with an ip and socket like this (it seems to be a valid configuration):
70 sip:192.168.1.1 0 1 route=route 1;sockname=udp_5060
Somehow, it should be clear in the documents how the destination record is selected.
I understood, I will supplement the documentation more fully on this occasion.
Your last comment does not clarify if a destination record matching (ip, socket) has to be selected over one that matches (ip, port, proto). Can you present the order of selection based on matching the values for ip, port, proto or socket?
Your last comment does not clarify if a destination record matching (ip, socket) has to be selected over one that matches (ip, port, proto). Can you present the order of selection based on matching the values for ip, port, proto or socket?
The strictness are defined by the flags combination:
#define DS_MATCHED_ADDR 1 #define DS_MATCHED_PORT (1 << 1) #define DS_MATCHED_PROTO (1 << 2) #define DS_MATCHED_SOCK (1 << 3)
in each iteration global variable ds_strictness are "OR-ed" with corresponding flag by criteria of matching, the winner is that which have the greatest value, for example if we have:
##ip, port, protocol 70 sip:192.168.1.1:5060;transport=udp 0 1 route=route1 #ip and socket 90 sip:192.168.1.1 0 1 route=route2;sockname=udp_5060
in first case we have flags: DS_MATCHED_ADDR | DS_MATCHED_PORT | DS_MATCHED_PROTO (7) in second: MATCHED_ADDR | DS_MATCHED_SOCK (9)
The winner will be second, in this case 90.
@Den4t @miconda Any more questions or things that needs to be discussed? Otherwise its probably a good idea to merge it in the next days before the window for 5.8.0 closes completely.
Hi !
Yes, i need to refine the docs, will try to do at this weekends.
I am merging it, then rename the option for FULLADDRSOCK trying to reflect better that socket alone has more priority than port+protocol.
Merged #3699 into master.
Thansk for merging @miconda !
I wanted to offer a version of the documentation for flag 16 to make it more clear:
If bit five is set, then try to find the most closest target from all dispatcher targets with the mandatory ip and combination of local socket, protocol, port. The weighted search result is combined from bit flags of matched components: ip - 1, port - 2, protocol - 4, local socket - 8, the target with the maximum value of flags combination will be selected.
Hi, @miconda !
About the docs changes above, do i need to make a new PR, this one is closed ?
Yes, you probably need to create a new PR for that, against git master. For 5.8 branch it will be cherry-picked.
-
Daniel-Constantin Mierla -
Dennis
-
Henning Westerholt
-
Morten Tryfoss