[ovs-dev] [PATCH 8/8] net: ip6_gre: fix tunnel metadata device sharing.

Gregory Rose gvrose8192 at gmail.com
Tue May 29 21:01:27 UTC 2018


On 5/29/2018 5:55 AM, William Tu wrote:
> commit b80d0b93b991e551a32157e0d9d38fc5bc9348a7
> Author: William Tu <u9012063 at gmail.com>
> Date:   Fri May 18 19:22:28 2018 -0700
>
>      net: ip6_gre: fix tunnel metadata device sharing.
>
>      Currently ip6gre and ip6erspan share single metadata mode device,
>      using 'collect_md_tun'.  Thus, when doing:
>        ip link add dev ip6gre11 type ip6gretap external
>        ip link add dev ip6erspan12 type ip6erspan external
>        RTNETLINK answers: File exists
>      simply fails due to the 2nd tries to create the same collect_md_tun.
>
>      The patch fixes it by adding a separate collect md tunnel device
>      for the ip6erspan, 'collect_md_tun_erspan'.  As a result, a couple
>      of places need to refactor/split up in order to distinguish ip6gre
>      and ip6erspan.
>
>      First, move the collect_md check at ip6gre_tunnel_{unlink,link} and
>      create separate function {ip6gre,ip6ersapn}_tunnel_{link_md,unlink_md}.
>      Then before link/unlink, make sure the link_md/unlink_md is called.
>      Finally, a separate ndo_uninit is created for ip6erspan.  Tested it
>      using the samples/bpf/test_tunnel_bpf.sh.
>
>      Fixes: ef7baf5e083c ("ip6_gre: add ip6 erspan collect_md mode")
>      Signed-off-by: William Tu <u9012063 at gmail.com>
>      Signed-off-by: David S. Miller <davem at davemloft.net>
>
> Cc: Greg Rose <gvrose8192 at gmail.com>
> Signed-off-by: William Tu <u9012063 at gmail.com>
> ---
>   datapath/linux/compat/ip6_gre.c | 116 +++++++++++++++++++++++++++++-----------
>   1 file changed, 86 insertions(+), 30 deletions(-)
>
> diff --git a/datapath/linux/compat/ip6_gre.c b/datapath/linux/compat/ip6_gre.c
> index 3a82c3d0fc6b..24e850a22266 100644
> --- a/datapath/linux/compat/ip6_gre.c
> +++ b/datapath/linux/compat/ip6_gre.c
> @@ -69,6 +69,7 @@ struct ip6gre_net {
>   	struct ip6_tnl __rcu *tunnels[4][IP6_GRE_HASH_SIZE];
>   
>   	struct ip6_tnl __rcu *collect_md_tun;
> +	struct ip6_tnl __rcu *collect_md_tun_erspan;
>   	struct net_device *fb_tunnel_dev;
>   };
>   
> @@ -245,7 +246,12 @@ static struct ip6_tnl *ip6gre_tunnel_lookup(struct net_device *dev,
>   	if (cand)
>   		return cand;
>   
> -	t = rcu_dereference(ign->collect_md_tun);
> +	if (gre_proto == htons(ETH_P_ERSPAN) ||
> +	    gre_proto == htons(ETH_P_ERSPAN2))
> +		t = rcu_dereference(ign->collect_md_tun_erspan);
> +	else
> +		t = rcu_dereference(ign->collect_md_tun);
> +
>   	if (t && t->dev->flags & IFF_UP)
>   		return t;
>   
> @@ -274,6 +280,31 @@ static struct ip6_tnl __rcu **__ip6gre_bucket(struct ip6gre_net *ign,
>   	return &ign->tunnels[prio][h];
>   }
>   
> +static void ip6gre_tunnel_link_md(struct ip6gre_net *ign, struct ip6_tnl *t)
> +{
> +       if (t->parms.collect_md)
> +               rcu_assign_pointer(ign->collect_md_tun, t);
> +}
> +
> +static void ip6erspan_tunnel_link_md(struct ip6gre_net *ign, struct ip6_tnl *t)
> +{
> +       if (t->parms.collect_md)
> +               rcu_assign_pointer(ign->collect_md_tun_erspan, t);
> +}
> +
> +static void ip6gre_tunnel_unlink_md(struct ip6gre_net *ign, struct ip6_tnl *t)
> +{
> +       if (t->parms.collect_md)
> +               rcu_assign_pointer(ign->collect_md_tun, NULL);
> +}
> +
> +static void ip6erspan_tunnel_unlink_md(struct ip6gre_net *ign,
> +                                      struct ip6_tnl *t)
> +{
> +       if (t->parms.collect_md)
> +               rcu_assign_pointer(ign->collect_md_tun_erspan, NULL);
> +}
> +
>   static inline struct ip6_tnl __rcu **ip6gre_bucket(struct ip6gre_net *ign,
>   		const struct ip6_tnl *t)
>   {
> @@ -284,9 +315,6 @@ static void ip6gre_tunnel_link(struct ip6gre_net *ign, struct ip6_tnl *t)
>   {
>   	struct ip6_tnl __rcu **tp = ip6gre_bucket(ign, t);
>   
> -	if (t->parms.collect_md)
> -		rcu_assign_pointer(ign->collect_md_tun, t);
> -
>   	rcu_assign_pointer(t->next, rtnl_dereference(*tp));
>   	rcu_assign_pointer(*tp, t);
>   }
> @@ -296,9 +324,6 @@ static void ip6gre_tunnel_unlink(struct ip6gre_net *ign, struct ip6_tnl *t)
>   	struct ip6_tnl __rcu **tp;
>   	struct ip6_tnl *iter;
>   
> -	if (t->parms.collect_md)
> -		rcu_assign_pointer(ign->collect_md_tun, NULL);
> -
>   	for (tp = ip6gre_bucket(ign, t);
>   	     (iter = rtnl_dereference(*tp)) != NULL;
>   	     tp = &iter->next) {
> @@ -385,11 +410,23 @@ failed_free:
>   	return NULL;
>   }
>   
> +static void ip6erspan_tunnel_uninit(struct net_device *dev)
> +{
> +	struct ip6_tnl *t = netdev_priv(dev);
> +	struct ip6gre_net *ign = net_generic(t->net, ip6gre_net_id);
> +
> +	ip6erspan_tunnel_unlink_md(ign, t);
> +	ip6gre_tunnel_unlink(ign, t);
> +	dst_cache_reset(&t->dst_cache);
> +	dev_put(dev);
> +}
> +
>   static void ip6gre_tunnel_uninit(struct net_device *dev)
>   {
>   	struct ip6_tnl *t = netdev_priv(dev);
>   	struct ip6gre_net *ign = net_generic(t->net, ip6gre_net_id);
>   
> +	ip6gre_tunnel_unlink_md(ign, t);
>   	ip6gre_tunnel_unlink(ign, t);
>   	dst_cache_reset(&t->dst_cache);
>   	dev_put(dev);
> @@ -2032,7 +2069,7 @@ static int ip6erspan_tap_init(struct net_device *dev)
>   
>   static const struct net_device_ops ip6erspan_netdev_ops = {
>   	.ndo_init =		ip6erspan_tap_init,
> -	.ndo_uninit =		ip6gre_tunnel_uninit,
> +	.ndo_uninit =		ip6erspan_tunnel_uninit,
>   	.ndo_start_xmit =	ip6erspan_tunnel_xmit,
>   	.ndo_set_mac_address =	eth_mac_addr,
>   	.ndo_validate_addr =	eth_validate_addr,
> @@ -2107,8 +2144,6 @@ static int rpl_ip6gre_newlink_common(struct net *src_net, struct net_device *dev
>   #endif
>   {
>   	struct ip6_tnl *nt;
> -	struct net *net = dev_net(dev);
> -	struct ip6gre_net *ign = net_generic(net, ip6gre_net_id);
>   	struct ip_tunnel_encap ipencap;
>   	int err;
>   
> @@ -2121,16 +2156,6 @@ static int rpl_ip6gre_newlink_common(struct net *src_net, struct net_device *dev
>   			return err;
>   	}
>   
> -	ip6gre_netlink_parms(data, &nt->parms);
> -
> -	if (nt->parms.collect_md) {
> -		if (rtnl_dereference(ign->collect_md_tun))
> -			return -EEXIST;
> -	} else {
> -		if (ip6gre_tunnel_find(net, &nt->parms, dev->type))
> -			return -EEXIST;
> -	}
> -
>   	if (dev->type == ARPHRD_ETHER && !tb[IFLA_ADDRESS])
>   		eth_hw_addr_random(dev);
>   
> @@ -2160,17 +2185,30 @@ static int rpl_ip6gre_newlink(struct net *src_net, struct net_device *dev,
>   			  struct nlattr *tb[], struct nlattr *data[])
>   #endif
>   {
> +	struct ip6_tnl *nt = netdev_priv(dev);
> +	struct net *net = dev_net(dev);
> +	struct ip6gre_net *ign;
> +	int err;
> +
> +	ip6gre_netlink_parms(data, &nt->parms);
> +	ign = net_generic(net, ip6gre_net_id);
> +
> +	if (nt->parms.collect_md) {
> +		if (rtnl_dereference(ign->collect_md_tun))
> +			return -EEXIST;
> +	} else {
> +		if (ip6gre_tunnel_find(net, &nt->parms, dev->type))
> +			return -EEXIST;
> +	}
>   
>   #ifdef HAVE_IP6GRE_EXTACK
> -	int err = ip6gre_newlink_common(src_net, dev, tb, data, extack);
> +	err = ip6gre_newlink_common(src_net, dev, tb, data, extack);
>   #else
> -	int err = ip6gre_newlink_common(src_net, dev, tb, data);
> +	err = ip6gre_newlink_common(src_net, dev, tb, data);
>   #endif
> -	struct ip6_tnl *nt = netdev_priv(dev);
> -	struct net *net = dev_net(dev);
> -
>   	if (!err) {
>   		ip6gre_tnl_link_config(nt, !tb[IFLA_MTU]);
> +		ip6gre_tunnel_link_md(ign, nt);
>   		ip6gre_tunnel_link(net_generic(net, ip6gre_net_id), nt);
>   	}
>   	return err;
> @@ -2240,8 +2278,10 @@ static int rpl_ip6gre_changelink(struct net_device *dev, struct nlattr *tb[],
>   	if (IS_ERR(t))
>   		return PTR_ERR(t);
>   
> +	ip6gre_tunnel_unlink_md(ign, t);
>   	ip6gre_tunnel_unlink(ign, t);
>   	ip6gre_tnl_change(t, &p, !tb[IFLA_MTU]);
> +	ip6gre_tunnel_link_md(ign, t);
>   	ip6gre_tunnel_link(ign, t);
>   	return 0;
>   }
> @@ -2405,16 +2445,30 @@ static int rpl_ip6erspan_newlink(struct net *src_net, struct net_device *dev,
>   				 struct nlattr *tb[], struct nlattr *data[])
>   #endif
>   {
> -#ifdef HAVE_IP6GRE_EXTACK
> -	int err = ip6gre_newlink_common(src_net, dev, tb, data, extack);
> -#else
> -	int err = ip6gre_newlink_common(src_net, dev, tb, data);
> -#endif
>   	struct ip6_tnl *nt = netdev_priv(dev);
>   	struct net *net = dev_net(dev);
> +	struct ip6gre_net *ign;
> +	int err;
>   
> +	ip6gre_netlink_parms(data, &nt->parms);
> +	ign = net_generic(net, ip6gre_net_id);
> +
> +	if (nt->parms.collect_md) {
> +		if (rtnl_dereference(ign->collect_md_tun_erspan))
> +			return -EEXIST;
> +	} else {
> +		if (ip6gre_tunnel_find(net, &nt->parms, dev->type))
> +			return -EEXIST;
> +	}
> +
> +#ifdef HAVE_IP6GRE_EXTACK
> +	err = ip6gre_newlink_common(src_net, dev, tb, data, extack);
> +#else
> +	err = ip6gre_newlink_common(src_net, dev, tb, data);
> +#endif
>   	if (!err) {
>   		ip6erspan_tnl_link_config(nt, !tb[IFLA_MTU]);
> +		ip6erspan_tunnel_link_md(ign, nt);
>   		ip6gre_tunnel_link(net_generic(net, ip6gre_net_id), nt);
>   	}
>   	return err;
> @@ -2455,8 +2509,10 @@ static int rpl_ip6erspan_changelink(struct net_device *dev, struct nlattr *tb[],
>   	if (IS_ERR(t))
>   		return PTR_ERR(t);
>   
> +	ip6gre_tunnel_unlink_md(ign, t);
>   	ip6gre_tunnel_unlink(ign, t);
>   	ip6erspan_tnl_change(t, &p, !tb[IFLA_MTU]);
> +	ip6erspan_tunnel_link_md(ign, t);
>   	ip6gre_tunnel_link(ign, t);
>   	return 0;
>   }

The patch seems fine but I still have problems with ip6gre and ip6erspan 
tunnels co-existing.

Thanks,

- Greg



More information about the dev mailing list