[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/#review4867
-----------------------------------------------------------



/branches/1.8/configs/res_stun_monitor.conf.sample
<https://reviewboard.asterisk.org/r/1595/#comment9072>

    Not clear what the syntax is with IPv6 addresses. Are brackets needed? Always or only with a port? Examples would make it clear I think.



/branches/1.8/res/res_stun_monitor.c
<https://reviewboard.asterisk.org/r/1595/#comment9073>

    Why do we need to keep the server address in memory? (Keeping DNS resolution results in memory is an anti-pattern.)
    
    It seems to me that we can just call connect() on the STUN socket and forget about the server's address. When we create the socket, we resolve the host name, create the socket, connect() it, and discard the server address. When a STUN request fails or times out, we close() the socket and create a new one, starting with the DNS resolution.



/branches/1.8/res/res_stun_monitor.c
<https://reviewboard.asterisk.org/r/1595/#comment9074>

    Why do you need to determine if it's a host name or an address literal? Only netsock2 code should care about that. Why is it important for res_stun_monitor to know that?


- Simon


On Nov. 23, 2011, 5:42 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1595/
> -----------------------------------------------------------
> 
> (Updated Nov. 23, 2011, 5:42 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 346241 
>   /branches/1.8/include/asterisk/stun.h 346241 
>   /branches/1.8/main/stun.c 346241 
>   /branches/1.8/res/res_stun_monitor.c 346241 
> 
> Diff: https://reviewboard.asterisk.org/r/1595/diff
> 
> 
> Testing
> -------
> 
> The STUN request still goes out periodically.
> The STUN server DNS address is refreshed periodically if the new stunservermonitor option is enabled.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

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