[asterisk-dev] [Code Review] Add a SIP nat=auto setting

[Terry Wilson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1698/#review5320
-----------------------------------------------------------



/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1698/#comment9871>

    This relies on the global nat=auto setting. See my next comment for a thorough explanation about why I would like to change this.



/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1698/#comment9870>

    This block forces nat=auto to be set for [general] for nat=auto to work for a peer. Better would be to remove the check for NAT_AUTO and toset a bitfield on the pvt for a detected nat, so that peers could check that and set the flags appropriately. Also, once something is marked as natted, it can never be unnatted because we don't clear the flags when the ast_sockaddr_cmp matches again.
    
    Also, the ast_sockaddr_stringify_addr/port and snprintf can be replaced with a single call to ast_sockaddr_stringify.



/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1698/#comment9869>

    It shouldn't be necessary to clear SIP_PAGE3_NAT_AUTO everywhere. There is an existing bug here where we basically don't set the default 'no' values by setting the mask flags for them, so if one sets nat=comedia in general, and nat=no in a peer, the peer still has symmetric RTP set. Setting the flags for the mask should take care for this, then the FORCE_RPORT flag is the only one that would need to be cleared manually since it defaults to yes.



/trunk/configs/sip.conf.sample
<https://reviewboard.asterisk.org/r/1698/#comment9867>

    Since there are no other nat= options that require being set in [general] to work, I'd prefer the change mentioned above.


- Terry


On Jan. 26, 2012, 8:07 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1698/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2012, 8:07 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> First, this patch as originally posted is from JIRA user pedro-garcia. It has been in JIRA for a long while, and has finally come up to be considered. There were some changes that I wanted to made to the original patch, so I tried contacting the author to get them to put the patch on reviewboard, but after a week I got no response. So, I'm putting the original patch up, then will immediately add a review with my changes. Many thanks to pedro-garcia for his contribution.
> 
> From the JIRA issue:
> 
> I have some devices in the following scenario:
> 
> Asterisk server with public IP address
> Mobile devices (clients):
> 
> When in internal network, no NAT between the client and the server
> When in "roaming" (i.e. a Hotel with WiFi), the client is behing a NAT
> When in 3G, operator transparent sip proxy so it looks as no NAT, but does not support symmetric RTP.
> Sometime, the device gets a public IP with no NAT at all.
> No NAT setting available in asterisk works for all these scenarios at the same time, and I can not request the user to activate different accounts depending on its location.
> 
> I have added a new NAT setting (nat=auto) to the current ones. When set, chan_sip auto detects from the Via header, the recv sockaddr, and the rport setting if the client is behind a NAT.
> 
> It also adds to cli interface results (sip show peer/s) info on this (so now you could see "N" for NAT and nothing for no NAT as before, "a" for auto detect no NAT, and "A" for autodetect NAT.
> 
> 
> This addresses bug ASTERISK-17860.
>     https://issues.asterisk.org/jira/browse/ASTERISK-17860
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 352610 
>   /trunk/channels/sip/include/sip.h 352610 
>   /trunk/configs/sip.conf.sample 352610 
> 
> Diff: https://reviewboard.asterisk.org/r/1698/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120127/2800d417/attachment-0001.htm>