On Mon, Oct 31, 2016 at 2:05 PM, Joe Stringer wrote:
> On 29 October 2016 at 21:33, Pravin B Shelar wrote:
>> Upstream commit:
>> commit c6fcc4fc5f8b592600c7409e769ab68da0fb1eca
>> Author: pravin shelar
>> 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
>> Signed-off-by: David S. Miller
>>
>> Signed-off-by: Pravin B Shelar
>
> 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.
>
>
>> @@ -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);
>
>
>
> Otherwise, LGTM thanks.
Thanks. I will post new version soon.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
