[ovs-dev] [PATCH 1/3] datapath: backport: vxlan: avoid using stale vxlan socket.
Pravin Shelar
pshelar at ovn.org
Mon Oct 31 21:24:18 UTC 2016
On Mon, Oct 31, 2016 at 2:05 PM, Joe Stringer <joe at ovn.org> wrote:
> 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)
>
right.
This function is not used even on upstream kernel. I will just remove it.
> <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.
>
I did not wanted to check current structure of the code too much.
>> 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.
Thanks. I will post new version soon.
More information about the dev
mailing list