| Message ID | 1380000501-13969-1-git-send-email-pshelar@nicira.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 2013-09-23 at 22:28 -0700, Pravin B Shelar wrote: > Use of RCU api makes vxlan code easier to understand. It also > fixes bug due to missing ACCESS_ONCE() on sk_user_data dereference. > In rare case without ACCESS_ONCE() compiler might omit vs on > sk_user_data dereference. > Compiler can use vs as alias for sk->sk_user_data, resulting in > multiple sk_user_data dereference in rcu read context which > could change. > > CC: Jesse Gross <jesse@nicira.com> > Signed-off-by: Pravin B Shelar <pshelar@nicira.com> > --- > drivers/net/vxlan.c | 9 +++------ > include/net/sock.h | 2 ++ > 2 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c > index d1292fe..3519a71 100644 > --- a/drivers/net/vxlan.c > +++ b/drivers/net/vxlan.c > @@ -952,8 +952,7 @@ void vxlan_sock_release(struct vxlan_sock *vs) > > spin_lock(&vn->sock_lock); > hlist_del_rcu(&vs->hlist); > - smp_wmb(); > - vs->sock->sk->sk_user_data = NULL; > + rcu_assign_pointer(sk_user_data_rcu(vs->sock->sk), NULL); RCU_INIT_POINTER(sk_user_data_rcu(vs->sock->sk), NULL) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 23 Sep 2013 22:28:21 -0700 Pravin B Shelar <pshelar@nicira.com> wrote: > > +#define sk_user_data_rcu(sk) ((*((void __rcu **)&(sk)->sk_user_data))) I don't like the macro, is it just to keep sparse happy? The name also implies that rcu_dereference() is built in. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 24, 2013 at 9:20 AM, Stephen Hemminger <stephen@networkplumber.org> wrote: > On Mon, 23 Sep 2013 22:28:21 -0700 > Pravin B Shelar <pshelar@nicira.com> wrote: > >> >> +#define sk_user_data_rcu(sk) ((*((void __rcu **)&(sk)->sk_user_data))) > > I don't like the macro, is it just to keep sparse happy? yes, it is for sparse. > The name also implies that rcu_dereference() is built in. ok, I can right separate API for rcu-set and rcu-deref. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index d1292fe..3519a71 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -952,8 +952,7 @@ void vxlan_sock_release(struct vxlan_sock *vs) spin_lock(&vn->sock_lock); hlist_del_rcu(&vs->hlist); - smp_wmb(); - vs->sock->sk->sk_user_data = NULL; + rcu_assign_pointer(sk_user_data_rcu(vs->sock->sk), NULL); vxlan_notify_del_rx_port(sk); spin_unlock(&vn->sock_lock); @@ -1048,8 +1047,7 @@ static int vxlan_udp_encap_recv(struct sock *sk, struct sk_buff *skb) port = inet_sk(sk)->inet_sport; - smp_read_barrier_depends(); - vs = (struct vxlan_sock *)sk->sk_user_data; + vs = rcu_dereference(sk_user_data_rcu(sk)); if (!vs) goto drop; @@ -2302,8 +2300,7 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, __be16 port, atomic_set(&vs->refcnt, 1); vs->rcv = rcv; vs->data = data; - smp_wmb(); - vs->sock->sk->sk_user_data = vs; + rcu_assign_pointer(sk_user_data_rcu(vs->sock->sk), vs); spin_lock(&vn->sock_lock); hlist_add_head_rcu(&vs->hlist, vs_head(net, port)); diff --git a/include/net/sock.h b/include/net/sock.h index 6ba2e7b..ffd1356 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -409,6 +409,8 @@ struct sock { void (*sk_destruct)(struct sock *sk); }; +#define sk_user_data_rcu(sk) ((*((void __rcu **)&(sk)->sk_user_data))) + /* * SK_CAN_REUSE and SK_NO_REUSE on a socket mean that the socket is OK * or not whether his port will be reused by someone else. SK_FORCE_REUSE
Use of RCU api makes vxlan code easier to understand. It also fixes bug due to missing ACCESS_ONCE() on sk_user_data dereference. In rare case without ACCESS_ONCE() compiler might omit vs on sk_user_data dereference. Compiler can use vs as alias for sk->sk_user_data, resulting in multiple sk_user_data dereference in rcu read context which could change. CC: Jesse Gross <jesse@nicira.com> Signed-off-by: Pravin B Shelar <pshelar@nicira.com> --- drivers/net/vxlan.c | 9 +++------ include/net/sock.h | 2 ++ 2 files changed, 5 insertions(+), 6 deletions(-)