netdev - Re: [RFC v3 3/3] vxlan: handle underlay VRF changes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e10919dc-8001-af17-f7c1-f9adc420c78a@cumulusnetworks.com>
Date:   Tue, 20 Nov 2018 08:04:42 -0700
From:   David Ahern <dsa@...ulusnetworks.com>
To:     Alexis Bauvin <abauvin@...leway.com>, roopa@...ulusnetworks.com
Cc:     netdev@...r.kernel.org, akherbouche@...leway.com
Subject: Re: [RFC v3 3/3] vxlan: handle underlay VRF changes

On 11/20/18 7:23 AM, Alexis Bauvin wrote:
> When underlay VRF changes, either because the lower device itself changed,
> or its VRF changed, this patch releases the current socket of the VXLAN
> device and recreates another one in the right VRF. This allows for
> on-the-fly change of the underlay VRF of a VXLAN device.
> 
> Signed-off-by: Alexis Bauvin <abauvin@...leway.com>
> Reviewed-by: Amine Kherbouche <akherbouche@...leway.com>
> Tested-by: Amine Kherbouche <akherbouche@...leway.com>
> ---
>  drivers/net/vxlan.c | 94 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 94 insertions(+)
> 
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index a3de08122269..1e6ccad6df6a 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -208,6 +208,18 @@ static inline struct vxlan_rdst *first_remote_rtnl(struct vxlan_fdb *fdb)
>  	return list_first_entry(&fdb->remotes, struct vxlan_rdst, list);
>  }
>  
> +static int vxlan_is_in_l3mdev_chain(struct net_device *chain,
> +				    struct net_device *dev)
> +{
> +	if (!chain)
> +		return 0;
> +
> +	if (chain->ifindex == dev->ifindex)
> +		return 1;
> +	return vxlan_is_in_l3mdev_chain(netdev_master_upper_dev_get(chain),
> +					dev);
> +}

This should return bool and true/false.

Also, why l3mdev in the name? None of the checks look at whether it is
an l3mdev master.

And again here, someone more familiar with the vxlan code should review it.

> +
>  /* Find VXLAN socket based on network namespace, address family and UDP port
>   * and enabled unshareable flags.
>   */
> @@ -3720,6 +3732,33 @@ struct net_device *vxlan_dev_create(struct net *net, const char *name,
>  }
>  EXPORT_SYMBOL_GPL(vxlan_dev_create);
>  
> +static int vxlan_reopen(struct vxlan_net *vn, struct vxlan_dev *vxlan)
> +{
> +	int ret = 0;
> +
> +	if (vxlan_addr_multicast(&vxlan->default_dst.remote_ip) &&
> +	    !vxlan_group_used(vn, vxlan))
> +		ret = vxlan_igmp_leave(vxlan);
> +	vxlan_sock_release(vxlan);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = vxlan_sock_add(vxlan);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (vxlan_addr_multicast(&vxlan->default_dst.remote_ip)) {
> +		ret = vxlan_igmp_join(vxlan);
> +		if (ret == -EADDRINUSE)
> +			ret = 0;
> +		if (ret)
> +			vxlan_sock_release(vxlan);
> +	}
> +
> +	return ret;
> +}
> +
>  static void vxlan_handle_lowerdev_unregister(struct vxlan_net *vn,
>  					     struct net_device *dev)
>  {
> @@ -3742,6 +3781,55 @@ static void vxlan_handle_lowerdev_unregister(struct vxlan_net *vn,
>  	unregister_netdevice_many(&list_kill);
>  }
>  
> +static void vxlan_handle_change_upper(struct vxlan_net *vn,
> +				      struct net_device *dev)
> +{
> +	struct vxlan_dev *vxlan, *next;
> +
> +	list_for_each_entry_safe(vxlan, next, &vn->vxlan_list, next) {
> +		struct net_device *lower;
> +		int err;
> +
> +		lower = __dev_get_by_index(vxlan->net,
> +					   vxlan->cfg.remote_ifindex);
> +		if (!vxlan_is_in_l3mdev_chain(lower, dev))
> +			continue;
> +
> +		err = vxlan_reopen(vn, vxlan);
> +		if (err < 0)
> +			netdev_err(vxlan->dev, "Failed to reopen socket: %d\n",
> +				   err);
> +	}
> +}
> +
> +static void vxlan_handle_change(struct vxlan_net *vn, struct net_device *dev)
> +{
> +	struct vxlan_dev *vxlan = netdev_priv(dev);
> +	struct vxlan_sock *sock;
> +	int l3mdev_index;
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> +	bool metadata = vxlan->cfg.flags & VXLAN_F_COLLECT_METADATA;
> +	bool ipv6 = vxlan->cfg.flags & VXLAN_F_IPV6 || metadata;
> +
> +	sock = ipv6 ? rcu_dereference(vxlan->vn6_sock)
> +		    : rcu_dereference(vxlan->vn4_sock);
> +#else
> +	sock = rcu_dereference(vxlan->vn4_sock);
> +#endif
> +
> +	l3mdev_index =
> +		l3mdev_master_upper_ifindex_by_index(vxlan->net,
> +						     vxlan->cfg.remote_ifindex);
> +	if (sock->sock->sk->sk_bound_dev_if != l3mdev_index) {
> +		int ret = vxlan_reopen(vn, vxlan);
> +
> +		if (ret < 0)
> +			netdev_err(vxlan->dev, "Failed to reopen socket: %d\n",
> +				   ret);
> +	}
> +}
> +
>  static int vxlan_netdevice_event(struct notifier_block *unused,
>  				 unsigned long event, void *ptr)
>  {
> @@ -3756,6 +3844,12 @@ static int vxlan_netdevice_event(struct notifier_block *unused,
>  	} else if (event == NETDEV_UDP_TUNNEL_PUSH_INFO ||
>  		   event == NETDEV_UDP_TUNNEL_DROP_INFO) {
>  		vxlan_offload_rx_ports(dev, event == NETDEV_UDP_TUNNEL_PUSH_INFO);
> +	} else if (event == NETDEV_CHANGEUPPER) {
> +		vxlan_handle_change_upper(vn, dev);
> +	} else if (event == NETDEV_CHANGE) {
> +		if (dev->rtnl_link_ops &&
> +		    !strcmp(dev->rtnl_link_ops->kind, vxlan_link_ops.kind))
> +			vxlan_handle_change(vn, dev);
>  	}
>  
>  	return NOTIFY_DONE;
>