[ovs-dev] [PATCH 05/15] tunneling: add IPv6 support to netdev_tunnel_config

[Thadeu Lima de Souza Cascardo]

On Tue, Nov 10, 2015 at 02:24:06PM -0800, Ben Pfaff wrote:
> On Thu, Oct 22, 2015 at 03:28:58PM -0200, Thadeu Lima de Souza Cascardo wrote:
> > From: Jiri Benc <jbenc at redhat.com>
> > 
> > Allow configuration of IPv6 tunnel endpoints.
> > 
> > [cascardo: removed support for netlink datapath configuration]
> > [cascardo: use IPv4 mapped IPv6 addresses]
> > [cascardo: use only flow, instead of flow and flow6]
> > 
> > Signed-off-by: Jiri Benc <jbenc at redhat.com>
> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at redhat.com>
> > Co-authored-by: Thadeu Lima de Souza Cascardo <cascardo at redhat.com>
> 
> I suggest folding this in, for style and to avoid a cast:
> 
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index cf393fa..2500996 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -428,24 +428,28 @@ static int
>  parse_tunnel_ip(const char *value, bool accept_mcast, bool *flow,
>                  struct in6_addr *ipv6, uint16_t *protocol)
>  {
> -    ovs_be32 ip;
>      if (!strcmp(value, "flow")) {
>          *flow = true;
>          *protocol = 0;
>          return 0;
>      }
>      if (addr_is_ipv6(value)) {
> -        if (lookup_ipv6(value, ipv6))
> -                return ENOENT;
> -        if (!accept_mcast && ipv6_addr_is_multicast(ipv6))
> -                return EINVAL;
> +        if (lookup_ipv6(value, ipv6)) {
> +            return ENOENT;
> +        }
> +        if (!accept_mcast && ipv6_addr_is_multicast(ipv6)){
> +            return EINVAL;
> +        }
>          *protocol = ETH_TYPE_IPV6;
>      } else {
> -        if (lookup_ip(value, (struct in_addr *)&ip))
> -                return ENOENT;
> -        if (!accept_mcast && ip_is_multicast(ip))
> -                return EINVAL;
> -        in6_addr_set_mapped_ipv4(ipv6, ip);
> +        struct in_addr ip;
> +        if (lookup_ip(value, &ip)) {
> +            return ENOENT;
> +        }
> +        if (!accept_mcast && ip_is_multicast(ip.s_addr)) {
> +            return EINVAL;
> +        }
> +        in6_addr_set_mapped_ipv4(ipv6, ip.s_addr);
>          *protocol = ETH_TYPE_IP;
>      }
>      return 0;
> 
> The code in set_tunnel_config() makes me think that smap_add_ipv6()
> itself should check whether the address is IPv4-mapped and just use an
> IPv4 address in that case.

Thanks for all the reviews, Ben. I appreciate it a lot.

I will fold that in, and take a look at set_tunnel_config.

Thanks.
Cascardo.