[asterisk-dev] [Code Review] Don't keep the STUN socket open between STUN monitor checks.

[Simon Perreault]

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


One last bit of advice: when you reopen a new socket for each STUN request, you will end up using different source ports. Some NATs using a pool of public IP addresses will assign a different external source IP to flows originating from different internal source ports, even if they originate from the same internal source address. With this patch, stun_res_monitor becomes unusable behind those NATs.

Overall, I dislike this patch. This is not how STUN was intended to be used. I described a number of problems, and I would not be surprised if more were discovered later.

- Simon


On Nov. 29, 2011, 7:17 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1595/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2011, 7:17 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Don't keep the STUN socket open between STUN monitor checks.
> 
> Keeping the STUN socket always open will cause the STUN monitor to fail if 
> the STUN server address or our own address changes.  
> 
> * Fix the reverse problem of the STUN server address changing after the 
> initial DNS resolution by using the dnsmgr to refresh the DNS resolution.  
> However, refreshing the DNS resolution may not be desireable for all 
> cases, as a result the stunservermonitor option is added to enable this.  
> 
> * Fix ast_stun_request() return value consistency.
> 
> 
> This addresses bug ASTERISK-18327.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18327
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/configs/res_stun_monitor.conf.sample 346465 
>   /branches/1.8/include/asterisk/stun.h 346465 
>   /branches/1.8/main/stun.c 346465 
>   /branches/1.8/res/res_stun_monitor.c 346465 
> 
> Diff: https://reviewboard.asterisk.org/r/1595/diff
> 
> 
> Testing
> -------
> 
> The STUN request still goes out periodically.
> The STUN server DNS address is refreshed if the STUN poll fails.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111130/c7f7b341/attachment.htm>