Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rtpengine module added receive-from option to flags to managing rtpengine by a kamailio node behind dispatcher #3230

Merged
merged 7 commits into from Sep 7, 2022

Conversation

toharish
Copy link
Contributor

@toharish toharish commented Sep 1, 2022

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

UE behind NAT ------>Dispatcher Kamailio-----> Location & Media management Kamailio ------> 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..

Pre-Submission Checklist

  • 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:

  • PR should be backported to stable branches
  • Tested changes locally
  • Related to issue #XXXX (replace XXXX with an open issue number)

Description

Add receive-from option to flags
receive-from=1.2.3.4
required for manageing rtpengine by a kamailio node behind a dispatcher
kamailio node
bugfix in line ip_af == AF_INET6
@miconda
Copy link
Member

miconda commented Sep 6, 2022

@toharish: you have to format the commit message as per contributing guidelines:

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?

@toharish
Copy link
Contributor Author

toharish commented Sep 6, 2022

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.

@rfuchs
Copy link
Member

rfuchs commented Sep 6, 2022

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.

@toharish
Copy link
Contributor Author

toharish commented Sep 7, 2022

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;
	}

Bugfix initialize  ng_flags.received-from = bencode_list(bencbuf);
@toharish
Copy link
Contributor Author

toharish commented Sep 7, 2022

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 2554

@@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo here (dash instead of underscore). Maybe at least try to compile it before submitting?

bencode_dictionary_add(ng_flags.dict, "received-from", ng_flags.received_from);
}
else {
item = bencode_list(bencbuf);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a suggestion, but you could reuse ng_flags.received_from here instead of creating a new list item. But up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fixed bug  
ng_flags.received-from = bencode_list(bencbuf); 
to 
ng_flags.received_from = bencode_list(bencbuf);

removed the double declaration  bencode_list(bencbuf) for received_from  when "received-from " is not given in flags by user.
Added received-from=IP in document
ADD received-from option in Document
Copy link
Member

@rfuchs rfuchs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now. @miconda I can squash and merge this if you want.

@miconda
Copy link
Member

miconda commented Sep 7, 2022

@rfuchs: ok, go ahead!

And thanks for looking into it!

@rfuchs rfuchs merged commit 7450104 into kamailio:master Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants