[ovs-dev] [PATCH 1/3] datapath: backport: vxlan: avoid using stale vxlan socket.

[Joe Stringer]

On 29 October 2016 at 21:33, Pravin B Shelar <pshelar at ovn.org> wrote:
> Upstream commit:
>     commit c6fcc4fc5f8b592600c7409e769ab68da0fb1eca
>     Author: pravin shelar <pshelar at ovn.org>
>     Date:   Fri Oct 28 09:59:15 2016 -0700
>
>     vxlan: avoid using stale vxlan socket.
>
>     When vxlan device is closed vxlan socket is freed. This
>     operation can race with vxlan-xmit function which
>     dereferences vxlan socket. Following patch uses RCU
>     mechanism to avoid this situation.
>
>     Signed-off-by: Pravin B Shelar <pshelar at ovn.org>
>     Signed-off-by: David S. Miller <davem at davemloft.net>
>
> Signed-off-by: Pravin B Shelar <pshelar at ovn.org>

There's another dereference of vxlan->vn*_sock in vxlan_dev_dst_port()
in the compat code.. is this supposed to use rcu_dereference() now?
(Upstream too)

<snip>

> @@ -1434,9 +1449,10 @@ int ovs_vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
>         dport = info->key.tp_dst ? : vxlan->cfg.dst_port;
>
>         if (ip_tunnel_info_af(info) == AF_INET) {
> +               struct vxlan_sock *sock4 = rcu_dereference(vxlan->vn4_sock);
>                 struct rtable *rt;
>
> -               if (!vxlan->vn4_sock)
> +               if (!sock4)
>                         return -EINVAL;
>                 rt = vxlan_get_route(vxlan, skb, 0, info->key.tos,
>                                      info->key.u.ipv4.dst,
> @@ -1448,8 +1464,6 @@ int ovs_vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
>  #if IS_ENABLED(CONFIG_IPV6)
>                 struct dst_entry *ndst;
>
> -               if (!vxlan->vn6_sock)
> -                       return -EINVAL;

It's a bit odd that we hide this in vxlan6_get_route() for IPv6, but
we do it differently in the v4 case above. But that's just cosmetic.

>                 ndst = vxlan6_get_route(vxlan, skb, 0, info->key.tos,
>                                         info->key.label, &info->key.u.ipv6.dst,
>                                         &info->key.u.ipv6.src, NULL, info);

<snip>

Otherwise, LGTM thanks.