From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= Subject: Re: [PATCH bpf-next v3 07/15] bpf: introduce new bpf AF_XDP map type BPF_MAP_TYPE_XSKMAP Date: Mon, 8 Oct 2018 18:52:22 +0200 Message-ID: References: <20180502110136.3738-1-bjorn.topel@gmail.com> <20180502110136.3738-8-bjorn.topel@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: "Karlsson, Magnus" , "Duyck, Alexander H" , Alexander Duyck , John Fastabend , Alexei Starovoitov , Jesper Dangaard Brouer , Willem de Bruijn , Daniel Borkmann , "Michael S. Tsirkin" , Netdev , =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= , michael.lundkvist@ericsson.com, "Brandeburg, Jesse" , "Singhai, Anjali" , "Zhang, Qi Z" To: Eric Dumazet Return-path: Received: from mail-qk1-f196.google.com ([209.85.222.196]:40025 "EHLO mail-qk1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726291AbeJIAFU (ORCPT ); Mon, 8 Oct 2018 20:05:20 -0400 Received: by mail-qk1-f196.google.com with SMTP id a13-v6so7829969qkc.7 for ; Mon, 08 Oct 2018 09:52:42 -0700 (PDT) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Den m=C3=A5n 8 okt. 2018 kl 18:05 skrev Bj=C3=B6rn T=C3=B6pel : > > Den m=C3=A5n 8 okt. 2018 kl 17:31 skrev Eric Dumazet : > > [...] > > So it is illegal to call synchronize_net(), since it is a reschedule po= int. > > > > Thanks for finding and pointing this out, Eric! > > I'll have look and get back with a patch. > Eric, something in the lines of the patch below? Or is it considered bad practice to use call_rcu in this context (prone to DoSing the kernel)? Thanks for spending time on the xskmap code. Very much appreciated! >>From 491f7bd87705f72c45e59242fc6c3b1db9d3b56d Mon Sep 17 00:00:00 2001 From: =3D?UTF-8?q?Bj=3DC3=3DB6rn=3D20T=3DC3=3DB6pel?=3D Date: Mon, 8 Oct 2018 18:34:11 +0200 Subject: [PATCH] xsk: do not call synchronize_net() under RCU read lock MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit XSKMAP update and delete functions called synchronize_net(), which can sleep. It is not allowed to sleep during an RCU read section. Fixes: fbfc504a24f5 ("bpf: introduce new bpf AF_XDP map type BPF_MAP_TYPE_XSKMAP") Reported-by: Eric Dumazet Signed-off-by: Bj=C3=B6rn T=C3=B6pel --- include/net/xdp_sock.h | 1 + kernel/bpf/xskmap.c | 21 +++++++++++---------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h index 13acb9803a6d..5b430141a3f6 100644 --- a/include/net/xdp_sock.h +++ b/include/net/xdp_sock.h @@ -68,6 +68,7 @@ struct xdp_sock { */ spinlock_t tx_completion_lock; u64 rx_dropped; + struct rcu_head rcu; }; struct xdp_buff; diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c index 9f8463afda9c..51e8e2785612 100644 --- a/kernel/bpf/xskmap.c +++ b/kernel/bpf/xskmap.c @@ -157,6 +157,13 @@ static void *xsk_map_lookup_elem(struct bpf_map *map, void *key) return NULL; } +static void __xsk_map_remove_async(struct rcu_head *rcu) +{ + struct xdp_sock *xs =3D container_of(rcu, struct xdp_sock, rcu); + + sock_put((struct sock *)xs); +} + static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value= , u64 map_flags) { @@ -192,11 +199,8 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value, sock_hold(sock->sk); old_xs =3D xchg(&m->xsk_map[i], xs); - if (old_xs) { - /* Make sure we've flushed everything. */ - synchronize_net(); - sock_put((struct sock *)old_xs); - } + if (old_xs) + call_rcu(&old_xs->rcu, __xsk_map_remove_async); sockfd_put(sock); return 0; @@ -212,11 +216,8 @@ static int xsk_map_delete_elem(struct bpf_map *map, void *key) return -EINVAL; old_xs =3D xchg(&m->xsk_map[k], NULL); - if (old_xs) { - /* Make sure we've flushed everything. */ - synchronize_net(); - sock_put((struct sock *)old_xs); - } + if (old_xs) + call_rcu(&old_xs->rcu, __xsk_map_remove_async); return 0; } --=20 2.17.1