[asterisk-dev] [Code Review] 2660: Add IPv6 Support To chan_iax2

[Matt Jordan]

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


First, thanks for IPv6-ifying chan_iax2. It's been a long time coming, and I think *every* AstriDevCon we talk about it, try to make it a priority, and it never seems to quite get done. Or even started, really. So - thanks!

Olle mentioned on the asterisk-dev mailing list (http://lists.digium.com/pipermail/asterisk-dev/2013-July/061189.html) that this does suffer from a lack of support for the happy eyeballs RFC - see http://tools.ietf.org/html/rfc6555. I didn't see the branch he was referring to in his branch index (http://svn.asterisk.org/svn/asterisk/team/oej/00INDEX/branch-directory.txt) - but I could be missing it.

So, I'm not sure how much the lack of happy eyeball support impacts this work. I would hope that the management of the dual stack would occur at a lower layer than the chan_iax driver - having each of the channel drivers attempt to manage the IPv6/IPv4 fallback would get messy very quickly. At the same time, given that chan_iax tracks a peer based on its current address, there may be some ripple effects back up into chan_iax2. I'm not sure if this would have to change given Olle's patches.

Olle - what patches are you referring to? And can that work occur at a lower layer than the channel drivers?



/trunk/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/2660/#comment18372>

    While you're here, put a space between if and (!peer):
    
    if (!peer && realtime) {



/trunk/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/2660/#comment18373>

    I'm not sure if this matters, but this is a little bit different than the previous code.
    
    Given an address, ast_sockaddr_cmp_addr will compare the entire sin_addr/sin6_addr field between the two addresses; it will not check the sin_port/sin6_port.
    
    I'm not sure if the loss of the port comparison is important here or not. If not, feel free to close this issue - otherwise, a new "address/port" comparison function may be appropriate. 



/trunk/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/2660/#comment18374>

    Same address/port matching finding here.



/trunk/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/2660/#comment18376>

    Address/port comparison here as well



/trunk/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/2660/#comment18377>

    A few red blobs through here



/trunk/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/2660/#comment18378>

    Remove?



/trunk/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/2660/#comment18379>

    Check for the return of ast_parse_arg. Without PARSE_RANGE_DEFAULTS, a range error will not set the value to low/high and will return an error. If they provide an invalid port, we shouldn't set it on addr.



/trunk/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/2660/#comment18380>

    Remove



/trunk/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/2660/#comment18382>

    I'd throw some spaces between ADDR and PORT:
    
    PARSE_ADDR | PARSE_PORT_IGNORE


- Matt Jordan


On July 6, 2013, 1:54 a.m., Michael Young wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2660/
> -----------------------------------------------------------
> 
> (Updated July 6, 2013, 1:54 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22025
>     https://issues.asterisk.org/jira/browse/ASTERISK-22025
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch adds the ability to bind and communicate over IPv6 addresses using chan_iax2.
> 
> 
> Diffs
> -----
> 
>   /trunk/main/netsock.c 393737 
>   /trunk/main/acl.c 393737 
>   /trunk/include/asterisk/acl.h 393737 
>   /trunk/contrib/realtime/mysql/iaxfriends.sql 393737 
>   /trunk/channels/iax2/parser.c 393737 
>   /trunk/channels/iax2/include/parser.h 393737 
>   /trunk/channels/iax2/include/iax2.h 393737 
>   /trunk/channels/chan_iax2.c 393737 
>   /trunk/CHANGES 393737 
> 
> Diff: https://reviewboard.asterisk.org/r/2660/diff/
> 
> 
> Testing
> -------
> 
> Successfully made calls using iax2 over IPv6 between two dev machines.  SIP Softphone -> dev vm (iax2 ipv6) -> (iax2 ipv6) dev box -> SIP ITSP -> mobile phone
> 
> 
> Thanks,
> 
> Michael Young
> 
>

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