[ovs-dev] [PATCH 1/3] lib: Add ipv6 support for active and passive socket connections

[Arun Sharma]

Thanks Ben for review comments. Couple of queries inline.

On 18/12/13 4:57 AM, "Ben Pfaff" <blp at nicira.com> wrote:

>On Mon, Dec 09, 2013 at 04:21:21AM -0800, Arun Sharma wrote:
>> Allows all socket operations to use ipv6 network addresses,
>> except in-band control which  is not supported for ipv6
>> 
>> 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>
>
>Thank you for the patch!
>
>Clang reports these errors:
>
>    ../lib/stream-tcp.c:170:43: error: cast from 'const struct sockaddr
>*' to 'const struct sockaddr_in6 *' increases required alignment from 2
>to 4 [-Werror,-Wcast-align]
>            const struct sockaddr_in6 *sin6 = (const struct sockaddr_in6
>*) sa;
>                  
>^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    ../lib/stream-tcp.c:174:40: error: cast from 'const struct sockaddr
>*' to 'struct sockaddr_storage *' increases required alignment from 2 to
>4 [-Werror,-Wcast-align]
>        return new_tcp_stream(name, fd, 0, (struct sockaddr_storage *) sa,
>                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>GCC reports these errors:
>
>    ../lib/stream-tcp.c:74:51: error: using member 'sin6_addr' in
>incomplete struct sockaddr_in6
>    ../lib/stream-tcp.c:75:52: error: using member 'sin6_port' in
>incomplete struct sockaddr_in6
>    ../lib/stream-tcp.c:83:50: error: using member 'sin6_addr' in
>incomplete struct sockaddr_in6
>    ../lib/stream-tcp.c:84:51: error: using member 'sin6_port' in
>incomplete struct sockaddr_in6
>    ../lib/stream-tcp.c:171:38: error: using member 'sin6_addr' in
>incomplete struct sockaddr_in6
>    ../lib/stream-tcp.c:145:25: error: using member 'sin6_port' in
>incomplete struct sockaddr_in6
>    ../lib/stream-tcp.c:147:32: error: using member 'sin6_port' in
>incomplete struct sockaddr_in6
>    -> lib/vconn.o
>    ../lib/socket-util.c:857:22: error: using member 'sin6_port' in
>incomplete struct sockaddr_in6
>    ../lib/socket-util.c:1160:27: error: using member 'sin6_addr' in
>incomplete struct sockaddr_in6
>    ../lib/stream-ssl.c:282:56: error: using member 'sin6_addr' in
>incomplete struct sockaddr_in6
>    ../lib/stream-ssl.c:283:57: error: using member 'sin6_port' in
>incomplete struct sockaddr_in6
>    ../lib/stream-ssl.c:291:54: error: using member 'sin6_addr' in
>incomplete struct sockaddr_in6
>    ../lib/stream-ssl.c:292:55: error: using member 'sin6_port' in
>incomplete struct sockaddr_in6
>    ../lib/stream-ssl.c:832:25: error: using member 'sin6_port' in
>incomplete struct sockaddr_in6
>    ../lib/stream-ssl.c:835:17: error: using member 'sin6_addr' in
>incomplete struct sockaddr_in6
>    ../lib/stream-ssl.c:889:38: error: using member 'sin6_addr' in
>incomplete struct sockaddr_in6
>    ../lib/stream-ssl.c:890:22: error: using member 'sin6_port' in
>incomplete struct sockaddr_in6
>    ../lib/stream-ssl.c:892:28: error: using member 'sin6_port' in
>incomplete struct sockaddr_in6
>    ../ofproto/ofproto-dpif-sflow.c:285:52: error: using member
>'sin6_addr' in incomplete struct sockaddr_in6


  [ARUN:18-Dec] - Which gcc version should I test to reproduce above
errors?
  So far i have tested on below versions but not able to reproduce the
same.
  gcc (Wheezy Debian 4.7.2-5), gcc (Ubuntu 12.10/Linaro 4.7.2-2ubuntu1),
gcc (Ubuntu 12.04/Linaro 4.6.3-1ubuntu5).
  Also tried with -Werror flag.


>
>
>IPv6 addresses formatted with IP6_FMT are going to be ugly.  Please
>use inet_ntop() instead.
>
>The parentheses around the second !strncmp here are not necessary:
>> +    if (!strncmp(rc->target, "tcp:", 4) || (!strncmp(rc->target,
>>"ssl:", 4))) {
>
>It's not necessary to add vconn and stream and rconn functions for
>retrieving IPv6 addresses, because these functions ultimately have no
>users.  I've already applied a patch to the OVS tree to remove the
>corresponding functions for IPv4.  (Please rebase against the latest
>master.)
>
>The code in parse_active() seems to act badly if the input string does
>not contain at least one ':'.
>
>Because our syntax already uses colons, I think that we should
>probably require IPv6 addresses to be put into brackets,
>e.g. tcp:[1::2]:1234.  I think that this is a common convention.

 [ARUN:18-Dec] - Just confirming, after above rework, the commands will be
executed as below?

 # ovsdb-server --remote=ptcp:6632:[2001:db8::1] ŠŠ.
 # ovs-vswitchd tcp:[2001:db8::1]:6632 Š...
 # ovs-vsctl --db=tcp:[2001:db8::1]:6632 list bridge

 But in case of man pages, these brackets are understood as optional.
 Here is the example change I have in mind for man pages.

    tcp:ip:port
           Connect to the given TCP port on ip. If ip is an IPv6 address,
then use format tcp:[ip]:port .


 
>
>Thanks,
>
>Ben.