On Fri, Sep 6, 2013 at 12:38 PM, Jesse Gross <[email protected]> wrote: > 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.
ok, I will send updated patch. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
