On Fri, Sep 6, 2013 at 12:30 PM, Pravin Shelar <[email protected]> wrote: > On Fri, Sep 6, 2013 at 12:15 PM, Jesse Gross <[email protected]> wrote: >> On Fri, Sep 6, 2013 at 11:39 AM, Pravin B Shelar <[email protected]> wrote: >>> diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c >>> index db14f2f..3d4fec3 100644 >>> --- a/datapath/linux/compat/vxlan.c >>> +++ b/datapath/linux/compat/vxlan.c >>> @@ -124,7 +124,8 @@ static int vxlan_udp_encap_recv(struct sock *sk, struct >>> sk_buff *skb) >>> if (iptunnel_pull_header(skb, VXLAN_HLEN, htons(ETH_P_TEB))) >>> goto drop; >>> >>> - vs = vxlan_find_sock(sock_net(sk), inet_sport(sk)); >>> + smp_read_barrier_depends(); >>> + vs = (struct vxlan_sock *)sk->sk_user_data; >>> if (!vs) >>> goto drop; >> >> I think this is a correct backport of the upstream commit but this >> part worries me a little bit because it is missing the ACCESS_ONCE >> part of rcu_dereference. It's unlikely due to the code structure but >> in theory the compiler could decide to omit 'vs' completely causing >> refetches across the NULL check. > > How compiler can omit 'vs' when NULL check has direct dependency on it?
I mean rather than allocating some register or additional local storage on the stack, the compiler could treat 'vs' as an alias for sk->sk_user_data. Then any time that somebody accesses 'vs' it would fetch sk->sk_user_data instead, which could change. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
