[kamailio/kamailio] rtpengine module added receive-from option to flags to managing rtpengine by a kamailio node behind dispatcher (PR #3230)
Add receive-from option to flags receive-from=1.2.3.4 required for managing rtpengine by a kamailio node behind a dispatcher kamailio node
<!-- Kamailio Pull Request Template --> UE behind NAT --------------->Dispatcher Kamailio--------> Location & Media managementKamailio ---------------> C5 SoftSwitch | | | V | RTPENGINE | |---------------------------->Location & Media management Kamailio ---------------> C5 SoftSwitch | V RTPENGINE
In this scenario when the Kamailio behind the Dispatcher will be getting the $Ri as dispatcher IP and by default that will be passed to RTP Engine for NAT negotiation which will fail. In this pull merge requst, RTP engine module a new flag is added receive-from=IP which is supposed to be the received IP from the First node through P-Accessnetwork-Info header or the contact recived tag etc.. <!-- 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 <!-- Describe your changes in detail -->
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/3230
-- Commit Summary --
* Add receive-from option
-- File Changes --
M src/modules/rtpengine/rtpengine.c (47)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/3230.patch https://github.com/kamailio/kamailio/pull/3230.diff
@toharish pushed 1 commit.
e097bd16e5723cc14411c9b73f95f4ac915e3286 Update rtpengine.c
@toharish: you have to format the commit message as per contributing guidelines:
- https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md#com...
This PR can be merged manually, but for future, use the appropriate format to make it easy to merge.
@rfuchs: any comment from your side on the patch?
but for future, use the appropriate format to make it easy to merge.
Thank you in the future, I will take care. I was not aware of this as it is my first merge request.
Hi,
From looking at the code, I'm not sure this is correct? In case of `received-from=` given in the list of flags, I see `ng_flags->received_from` being referenced and treated as a list (items added to it) before it's set to anything. I would expect to see something like `ng_flags->received_from = bencode_list()` somewhere.
Hi,
From looking at the code, I'm not sure this is correct? In case of `received-from=` given in the list of flags, I see `ng_flags->received_from` being referenced and treated as a list (items added to it) before it's set to anything. I would expect to see something like `ng_flags->received_from = bencode_list()` somewhere.
ng_flags-> recive_flags is set in the funtion static int parse_flags(struct ng_flags_parse *ng_flags, struct sip_msg *msg, enum rtpe_operation *op, const char *flags_str)
in line number 2279 as
if (str_key_val_prefix(&key, "received-from", &val, &s)) { ip_af = get_ip_type(s.s); if (ip_af == AF_INET) { s1.s="IP4"; s1.len=3; bencode_list_add_str(ng_flags->received_from, &s1); bencode_list_add_str(ng_flags->received_from, &s); }else if (ip_af == AF_INET6) { s1.s="IP6"; s1.len=3; bencode_list_add_str(ng_flags->received_from, &s1); bencode_list_add_str(ng_flags->received_from, &s); } goto next; }
@toharish pushed 1 commit.
a981bfbd3439c621fd20328f6dc588f6da466024 Update rtpengine.c
Hi,
From looking at the code, I'm not sure this is correct? In case of `received-from=` given in the list of flags, I see `ng_flags->received_from` being referenced and treated as a list (items added to it) before it's set to anything. I would expect to see something like `ng_flags->received_from = bencode_list()` somewhere.
Thank you I have fixed it by adding ng_flags.received-from = bencode_list(bencbuf); at line number 2562
@toharish pushed 1 commit.
70b29981c068e4814424015259f8b865953a5ecc Update rtpengine.c
@rfuchs requested changes on this pull request.
@@ -2529,6 +2551,7 @@ static bencode_item_t *rtpp_function_call(bencode_buffer_t *bencbuf, struct sip_
body.s = NULL; ng_flags.flags = bencode_list(bencbuf); + ng_flags.received-from = bencode_list(bencbuf);
Typo here (dash instead of underscore). Maybe at least try to compile it before submitting?
@@ -2649,13 +2672,18 @@ static bencode_item_t *rtpp_function_call(bencode_buffer_t *bencbuf, struct sip_
bencode_dictionary_add_str(ng_flags.dict, "via-branch", &viabranch); }
- item = bencode_list(bencbuf); - bencode_dictionary_add(ng_flags.dict, "received-from", item); - bencode_list_add_string(item, (msg->rcv.src_ip.af == AF_INET) ? "IP4" : ( - (msg->rcv.src_ip.af == AF_INET6) ? "IP6" : - "?" - ) ); - bencode_list_add_string(item, ip_addr2a(&msg->rcv.src_ip)); + if (ng_flags.received_from && ng_flags.received_from->child) { + bencode_dictionary_add(ng_flags.dict, "received-from", ng_flags.received_from); + } + else { + item = bencode_list(bencbuf);
Just a suggestion, but you could reuse `ng_flags.received_from` here instead of creating a new list item. But up to you.
@toharish pushed 1 commit.
937dd8d537482a1f822acf249d3074f51e959ead Update rtpengine.c
@toharish pushed 1 commit.
0f37e7ab881956ee6e57a77d4813c20380bdd4fe Update received-from=IP
@toharish pushed 1 commit.
68948b42518ca52cae58c413a617ff30b4da865b Update received-from
@toharish commented on this pull request.
@@ -2649,13 +2672,18 @@ static bencode_item_t *rtpp_function_call(bencode_buffer_t *bencbuf, struct sip_
bencode_dictionary_add_str(ng_flags.dict, "via-branch", &viabranch); }
- item = bencode_list(bencbuf); - bencode_dictionary_add(ng_flags.dict, "received-from", item); - bencode_list_add_string(item, (msg->rcv.src_ip.af == AF_INET) ? "IP4" : ( - (msg->rcv.src_ip.af == AF_INET6) ? "IP6" : - "?" - ) ); - bencode_list_add_string(item, ip_addr2a(&msg->rcv.src_ip)); + if (ng_flags.received_from && ng_flags.received_from->child) { + bencode_dictionary_add(ng_flags.dict, "received-from", ng_flags.received_from); + } + else { + item = bencode_list(bencbuf);
Reuse of ng_flags.received_from done fixed ng_flags.received-from = bencode_list(bencbuf); to ng_flags.received_from = bencode_list(bencbuf); complied and verified. Thank you for your support.
@rfuchs approved this pull request.
LGTM now. @miconda I can squash and merge this if you want.
@rfuchs: ok, go ahead!
And thanks for looking into it!
Merged #3230 into master.
-
Daniel-Constantin Mierla -
Richard Fuchs
-
toharish