[ovs-dev] FW: [PATCH 3.2] lib: Add ipv6 support for active and passive socket connections

[Arun Sharma]

Hello Ben,

Thanks for reply. Please find inline comments.
I will send v4 patch.

Arun

>On 18/01/14 4:53 AM, "Ben Pfaff" <blp at nicira.com> wrote:
>
>>On Tue, Dec 31, 2013 at 01:08:26PM +0530, Arun Sharma wrote:
>>> Allows active/passive socket operations to use IPv6 network addresses,
>>> except in-band control which is not supported for IPv6.
>>> 
>>> tests: Added tests for socket connections over IPv6.
>>> Also added tests for NetFlow and sFlow using IPv6 sockets.
>>> manpages: Updated connection information for IPv6 socket communication.
>>> 
>>> Signed-off-by: Nandan Nivgune <nandan.nivgune at calsoftinc.com>
>>> Signed-off-by: Abhijit Bhopatkar <abhijit.bhopatkar at calsoftinc.com>
>>> Signed-off-by: Arun Sharma <arun.sharma at calsoftinc.com>
>>
>>include/sparse/netinet/in.h
>>---------------------------
>>
>>Please don't define s6_addr16 here, or use it elsewhere, because it is
>>not portable: POSIX does not specify it.
>
>Thanks. Removed s6_addr16. Basically above changes in
>include/sparse/netinet/in.h is done earlier to take care of warnings
>thrown during compile with sparse. Now using s6_addr and removed
>the define of s6_addr16.
>
>>
>>Please don't use __SOCKADDR_COMMON in the definition of sockaddr_in6,
>>because it is a glibc-ism that doesn't belong here.
>
>Thanks, Using "sa_family_t sin_family;"
>
>>
>>
>>lib/packets.h
>>-------------
>>
>>Please don't use or define IP6_*, because IPv6 addresses formatted
>>this way are often not in the format that people expect.  Use
>>inet_ntop() instead.
>
>Done, using inet_ntop()
>
>>
>>
>>lib/rconn.c
>>-----------
>>
>>What is the reason for the change here?
>
>My bad, I missed to remove those codes while removal of all
>vconn_get_remote_ip* related calls of vconn as per first level review
>comments in the v1 patch.  Ref:
>http://openvswitch.org/pipermail/dev/2013-December/035283.html
>Removed all changes from rconn.c as its not required. Thanks.
>
>>
>>
>>lib/socket-util.c
>>-----------------
>>
>>Why does this use getaddrinfo() instead of just inet_pton() for
>>parsing addresses?  At least, AI_NUMERICHOST must be used to avoid DNS
>>lookups.
>
>One of the reason we choose getaddrinfo() as it helps to get "struct
>sockaddr*" having port, family, *_addr already filled in and use it as it
>is. We could have used inet_pton(), but in this case, we need to maintain
>Socket family in inet_parse_active() or inet_parse_passive() as we need
>to 
>Pass as first argument to inet_pton(). This family variable we are
>avoiding using
>getaddrinfo(). Also we are avoiding to maintain "struct
>in*_addr" where address will be stored and then prepare "struct
>sockaddr_in*" structure.
>
>Yes, we must use flag AI_NUMERICHOST to avoid DNS lookup, we missed this.
>Also using AI_NUMERICSERV as it will avoid name resolution service. Thanks
>
>
>>
>>
>>
>>Thanks,
>>
>>Ben.