From: Martin KaFai Lau <kafai@fb.com>
To: "Toke Høiland-Jørgensen" <toke@redhat.com>
Cc: <bpf@vger.kernel.org>, <netdev@vger.kernel.org>,
Hangbin Liu <liuhangbin@gmail.com>,
Jesper Dangaard Brouer <brouer@redhat.com>,
Magnus Karlsson <magnus.karlsson@gmail.com>,
"Paul E . McKenney" <paulmck@kernel.org>
Subject: Re: [PATCH bpf-next 02/17] bpf: allow RCU-protected lookups to happen from bh context
Date: Thu, 10 Jun 2021 12:33:26 -0700 [thread overview]
Message-ID: <20210610193326.p6x3t2c26jitsjob@kafai-mbp> (raw)
In-Reply-To: <20210609103326.278782-3-toke@redhat.com>
On Wed, Jun 09, 2021 at 12:33:11PM +0200, Toke Høiland-Jørgensen wrote:
> XDP programs are called from a NAPI poll context, which means the RCU
> reference liveness is ensured by local_bh_disable(). Add
> rcu_read_lock_bh_held() as a condition to the RCU checks for map lookups so
> lockdep understands that the dereferences are safe from inside *either* an
> rcu_read_lock() section *or* a local_bh_disable() section. This is done in
> preparation for removing the redundant rcu_read_lock()s from the drivers.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---
> kernel/bpf/hashtab.c | 21 ++++++++++++++-------
> kernel/bpf/helpers.c | 6 +++---
> kernel/bpf/lpm_trie.c | 6 ++++--
> 3 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 6f6681b07364..72c58cc516a3 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -596,7 +596,8 @@ static void *__htab_map_lookup_elem(struct bpf_map *map, void *key)
> struct htab_elem *l;
> u32 hash, key_size;
>
> - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held());
> + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() &&
> + !rcu_read_lock_bh_held());
>
> key_size = map->key_size;
>
> @@ -989,7 +990,8 @@ static int htab_map_update_elem(struct bpf_map *map, void *key, void *value,
> /* unknown flags */
> return -EINVAL;
>
> - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held());
> + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() &&
> + !rcu_read_lock_bh_held());
>
> key_size = map->key_size;
>
> @@ -1082,7 +1084,8 @@ static int htab_lru_map_update_elem(struct bpf_map *map, void *key, void *value,
> /* unknown flags */
> return -EINVAL;
>
> - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held());
> + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() &&
> + !rcu_read_lock_bh_held());
>
> key_size = map->key_size;
>
> @@ -1148,7 +1151,8 @@ static int __htab_percpu_map_update_elem(struct bpf_map *map, void *key,
> /* unknown flags */
> return -EINVAL;
>
> - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held());
> + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() &&
> + !rcu_read_lock_bh_held());
>
> key_size = map->key_size;
>
> @@ -1202,7 +1206,8 @@ static int __htab_lru_percpu_map_update_elem(struct bpf_map *map, void *key,
> /* unknown flags */
> return -EINVAL;
>
> - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held());
> + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() &&
> + !rcu_read_lock_bh_held());
>
> key_size = map->key_size;
>
> @@ -1276,7 +1281,8 @@ static int htab_map_delete_elem(struct bpf_map *map, void *key)
> u32 hash, key_size;
> int ret;
>
> - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held());
> + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() &&
> + !rcu_read_lock_bh_held());
>
> key_size = map->key_size;
>
> @@ -1311,7 +1317,8 @@ static int htab_lru_map_delete_elem(struct bpf_map *map, void *key)
> u32 hash, key_size;
> int ret;
>
> - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held());
> + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() &&
> + !rcu_read_lock_bh_held());
>
> key_size = map->key_size;
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 544773970dbc..e880f6bb6f28 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -28,7 +28,7 @@
> */
> BPF_CALL_2(bpf_map_lookup_elem, struct bpf_map *, map, void *, key)
> {
> - WARN_ON_ONCE(!rcu_read_lock_held());
> + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_bh_held());
There is a discrepancy in rcu_read_lock_trace_held() here but
I think the patch_map_ops_generic step in the verifier has skipped
these helper calls. It is unrelated and can be addressed later
until it is needed.
Acked-by: Martin KaFai Lau <kafai@fb.com>
> return (unsigned long) map->ops->map_lookup_elem(map, key);
> }
>
> @@ -44,7 +44,7 @@ const struct bpf_func_proto bpf_map_lookup_elem_proto = {
> BPF_CALL_4(bpf_map_update_elem, struct bpf_map *, map, void *, key,
> void *, value, u64, flags)
> {
> - WARN_ON_ONCE(!rcu_read_lock_held());
> + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_bh_held());
> return map->ops->map_update_elem(map, key, value, flags);
> }
>
> @@ -61,7 +61,7 @@ const struct bpf_func_proto bpf_map_update_elem_proto = {
>
> BPF_CALL_2(bpf_map_delete_elem, struct bpf_map *, map, void *, key)
> {
> - WARN_ON_ONCE(!rcu_read_lock_held());
> + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_bh_held());
> return map->ops->map_delete_elem(map, key);
> }
next prev parent reply other threads:[~2021-06-10 19:33 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-09 10:33 [PATCH bpf-next 00/17] Clean up and document RCU-based object protection for XDP_REDIRECT Toke Høiland-Jørgensen
2021-06-09 10:33 ` [PATCH bpf-next 01/17] rcu: Create an unrcu_pointer() to remove __rcu from a pointer Toke Høiland-Jørgensen
2021-06-09 10:33 ` [PATCH bpf-next 02/17] bpf: allow RCU-protected lookups to happen from bh context Toke Høiland-Jørgensen
2021-06-10 18:38 ` Alexei Starovoitov
2021-06-10 21:24 ` Daniel Borkmann
2021-06-10 22:27 ` Toke Høiland-Jørgensen
2021-06-10 19:33 ` Martin KaFai Lau [this message]
2021-06-09 10:33 ` [PATCH bpf-next 03/17] dev: add rcu_read_lock_bh_held() as a valid check when getting a RCU dev ref Toke Høiland-Jørgensen
2021-06-10 19:37 ` Martin KaFai Lau
2021-06-10 23:05 ` Toke Høiland-Jørgensen
2021-06-09 10:33 ` [PATCH bpf-next 04/17] xdp: add proper __rcu annotations to redirect map entries Toke Høiland-Jørgensen
2021-06-10 21:09 ` Martin KaFai Lau
2021-06-10 23:19 ` Toke Høiland-Jørgensen
2021-06-10 23:32 ` Martin KaFai Lau
2021-06-10 23:41 ` Toke Høiland-Jørgensen
2021-06-09 10:33 ` [PATCH bpf-next 05/17] ena: remove rcu_read_lock() around XDP program invocation Toke Høiland-Jørgensen
2021-06-09 13:57 ` Paul E. McKenney
2021-06-09 10:33 ` [PATCH bpf-next 06/17] bnxt: " Toke Høiland-Jørgensen
2021-06-09 13:58 ` Paul E. McKenney
2021-06-10 8:47 ` Toke Høiland-Jørgensen
2021-06-09 10:33 ` [PATCH bpf-next 07/17] thunderx: " Toke Høiland-Jørgensen
2021-06-09 10:33 ` Toke Høiland-Jørgensen
2021-06-09 10:33 ` [PATCH bpf-next 08/17] freescale: " Toke Høiland-Jørgensen
2021-06-09 10:33 ` [PATCH bpf-next 09/17] net: intel: " Toke Høiland-Jørgensen
2021-06-09 10:33 ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2021-06-09 10:33 ` [PATCH bpf-next 10/17] marvell: " Toke Høiland-Jørgensen
2021-06-09 10:33 ` [PATCH bpf-next 11/17] mlx4: " Toke Høiland-Jørgensen
2021-06-10 7:10 ` Tariq Toukan
2021-06-09 10:33 ` [PATCH bpf-next 12/17] nfp: " Toke Høiland-Jørgensen
2021-06-11 16:30 ` Simon Horman
2021-06-09 10:33 ` [PATCH bpf-next 13/17] qede: " Toke Høiland-Jørgensen
2021-06-09 10:33 ` [PATCH bpf-next 14/17] sfc: " Toke Høiland-Jørgensen
2021-06-09 12:15 ` Edward Cree
2021-06-09 10:33 ` [PATCH bpf-next 15/17] netsec: " Toke Høiland-Jørgensen
2021-06-10 5:30 ` Ilias Apalodimas
2021-06-09 10:33 ` [PATCH bpf-next 16/17] stmmac: " Toke Høiland-Jørgensen
2021-06-09 10:33 ` [PATCH bpf-next 17/17] net: ti: " Toke Høiland-Jørgensen
2021-06-09 17:04 ` Grygorii Strashko
2021-06-10 0:18 ` [PATCH bpf-next 00/17] Clean up and document RCU-based object protection for XDP_REDIRECT Yonghong Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210610193326.p6x3t2c26jitsjob@kafai-mbp \
--to=kafai@fb.com \
--cc=bpf@vger.kernel.org \
--cc=brouer@redhat.com \
--cc=liuhangbin@gmail.com \
--cc=magnus.karlsson@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=paulmck@kernel.org \
--cc=toke@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.