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

Ship it!


Impressive! You addressed all my comments! :) Thanks a lot.

Just one minor thing, but that shouldn't prevent you from shipping.


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

    stun_addr doesn't seem to be used after this call. So what is the purpose? There's already an ast_get_ip() call up above that is invoked every time the socket is refreshed, including I suppose on the first run, so I don't see what is this call's purpose.


- Simon


On Nov. 30, 2011, 6:35 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1595/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2011, 6:35 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Re-resolve the STUN address if a STUN request poll fails.
> 
> The STUN socket must remain open between polls or the external address
> seen by the STUN server is likely to change.  However, if the STUN request
> poll fails then the STUN server address needs to be re-resolved and the
> STUN socket needs to be closed and reopened.
> 
> * Re-resolve the STUN server address and create a new socket if the STUN
> request poll fails.
> 
> * Fix ast_stun_request() return value consistency.
> 
> * Fix ast_stun_request() to check the received packet for expected message
> type and transaction ID.
> 
> * Fix ast_stun_request() to read packets until timeout or an associated
> response packet is found.  The stun_purge_socket() hack is no longer
> required.
> 
> 
> This addresses bug ASTERISK-18327.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18327
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/configs/res_stun_monitor.conf.sample 346655 
>   /branches/1.8/include/asterisk/stun.h 346655 
>   /branches/1.8/main/stun.c 346655 
>   /branches/1.8/res/res_stun_monitor.c 346655 
> 
> 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 request poll fails.
> * The received packets are read until there are none left or an associated packet is found.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

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