From: "Toke Høiland-Jørgensen" <toke@redhat.com> To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: "Martin KaFai Lau" <kafai@fb.com>, "Hangbin Liu" <liuhangbin@gmail.com>, "Jesper Dangaard Brouer" <brouer@redhat.com>, "Magnus Karlsson" <magnus.karlsson@gmail.com>, "Paul E . McKenney" <paulmck@kernel.org>, "Jakub Kicinski" <kuba@kernel.org>, "Toke Høiland-Jørgensen" <toke@redhat.com>, "Sunil Goutham" <sgoutham@marvell.com>, linux-arm-kernel@lists.infradead.org Subject: [PATCH bpf-next v3 06/16] thunderx: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:38 +0200 [thread overview] Message-ID: <20210617212748.32456-7-toke@redhat.com> (raw) In-Reply-To: <20210617212748.32456-1-toke@redhat.com> The thunderx driver has rcu_read_lock()/rcu_read_unlock() pairs around XDP program invocations. However, the actual lifetime of the objects referred by the XDP program invocation is longer, all the way through to the call to xdp_do_flush(), making the scope of the rcu_read_lock() too small. This turns out to be harmless because it all happens in a single NAPI poll cycle (and thus under local_bh_disable()), but it makes the rcu_read_lock() misleading. Rather than extend the scope of the rcu_read_lock(), just get rid of it entirely. With the addition of RCU annotations to the XDP_REDIRECT map types that take bh execution into account, lockdep even understands this to be safe, so there's really no reason to keep it around. Cc: Sunil Goutham <sgoutham@marvell.com> Cc: linux-arm-kernel@lists.infradead.org Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> --- drivers/net/ethernet/cavium/thunder/nicvf_main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c index c33b4e837515..1d752815c69a 100644 --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c @@ -555,9 +555,10 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog, xdp_prepare_buff(&xdp, hard_start, data - hard_start, len, false); orig_data = xdp.data; - rcu_read_lock(); + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ action = bpf_prog_run_xdp(prog, &xdp); - rcu_read_unlock(); len = xdp.data_end - xdp.data; /* Check if XDP program has changed headers */ -- 2.32.0
WARNING: multiple messages have this Message-ID (diff)
From: "Toke Høiland-Jørgensen" <toke@redhat.com> To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: "Martin KaFai Lau" <kafai@fb.com>, "Hangbin Liu" <liuhangbin@gmail.com>, "Jesper Dangaard Brouer" <brouer@redhat.com>, "Magnus Karlsson" <magnus.karlsson@gmail.com>, "Paul E . McKenney" <paulmck@kernel.org>, "Jakub Kicinski" <kuba@kernel.org>, "Toke Høiland-Jørgensen" <toke@redhat.com>, "Sunil Goutham" <sgoutham@marvell.com>, linux-arm-kernel@lists.infradead.org Subject: [PATCH bpf-next v3 06/16] thunderx: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:38 +0200 [thread overview] Message-ID: <20210617212748.32456-7-toke@redhat.com> (raw) In-Reply-To: <20210617212748.32456-1-toke@redhat.com> The thunderx driver has rcu_read_lock()/rcu_read_unlock() pairs around XDP program invocations. However, the actual lifetime of the objects referred by the XDP program invocation is longer, all the way through to the call to xdp_do_flush(), making the scope of the rcu_read_lock() too small. This turns out to be harmless because it all happens in a single NAPI poll cycle (and thus under local_bh_disable()), but it makes the rcu_read_lock() misleading. Rather than extend the scope of the rcu_read_lock(), just get rid of it entirely. With the addition of RCU annotations to the XDP_REDIRECT map types that take bh execution into account, lockdep even understands this to be safe, so there's really no reason to keep it around. Cc: Sunil Goutham <sgoutham@marvell.com> Cc: linux-arm-kernel@lists.infradead.org Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> --- drivers/net/ethernet/cavium/thunder/nicvf_main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c index c33b4e837515..1d752815c69a 100644 --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c @@ -555,9 +555,10 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog, xdp_prepare_buff(&xdp, hard_start, data - hard_start, len, false); orig_data = xdp.data; - rcu_read_lock(); + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ action = bpf_prog_run_xdp(prog, &xdp); - rcu_read_unlock(); len = xdp.data_end - xdp.data; /* Check if XDP program has changed headers */ -- 2.32.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-06-17 21:37 UTC|newest] Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-06-17 21:27 [PATCH bpf-next v3 00/16] Clean up and document RCU-based object protection for XDP_REDIRECT Toke Høiland-Jørgensen 2021-06-17 21:27 ` [PATCH bpf-next v3 01/16] rcu: Create an unrcu_pointer() to remove __rcu from a pointer Toke Høiland-Jørgensen 2021-06-17 21:27 ` [PATCH bpf-next v3 02/16] bpf: allow RCU-protected lookups to happen from bh context Toke Høiland-Jørgensen 2021-06-17 21:27 ` [PATCH bpf-next v3 03/16] xdp: add proper __rcu annotations to redirect map entries Toke Høiland-Jørgensen 2021-06-18 4:55 ` Martin KaFai Lau 2021-06-18 23:27 ` Daniel Borkmann 2021-06-21 21:39 ` Toke Høiland-Jørgensen 2021-06-21 22:15 ` Daniel Borkmann 2021-06-21 22:35 ` Toke Høiland-Jørgensen 2021-06-22 8:50 ` Daniel Borkmann 2021-06-22 13:55 ` Toke Høiland-Jørgensen 2021-06-22 20:26 ` Paul E. McKenney 2021-06-22 21:48 ` Toke Høiland-Jørgensen 2021-06-22 23:19 ` Paul E. McKenney 2021-06-23 10:55 ` Toke Høiland-Jørgensen 2021-06-17 21:27 ` [PATCH bpf-next v3 04/16] ena: remove rcu_read_lock() around XDP program invocation Toke Høiland-Jørgensen 2021-06-17 21:27 ` [PATCH bpf-next v3 05/16] bnxt: " Toke Høiland-Jørgensen 2021-06-17 21:27 ` Toke Høiland-Jørgensen [this message] 2021-06-17 21:27 ` [PATCH bpf-next v3 06/16] thunderx: " Toke Høiland-Jørgensen 2021-06-17 21:27 ` [PATCH bpf-next v3 07/16] freescale: " Toke Høiland-Jørgensen 2021-06-17 21:27 ` [PATCH bpf-next v3 08/16] net: intel: " Toke Høiland-Jørgensen 2021-06-17 21:27 ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?= 2021-06-17 21:27 ` [PATCH bpf-next v3 09/16] marvell: " Toke Høiland-Jørgensen 2021-06-17 21:27 ` [PATCH bpf-next v3 10/16] mlx4: " Toke Høiland-Jørgensen 2021-06-17 21:27 ` [PATCH bpf-next v3 11/16] nfp: " Toke Høiland-Jørgensen 2021-06-17 21:27 ` [PATCH bpf-next v3 12/16] qede: " Toke Høiland-Jørgensen 2021-06-17 21:27 ` [PATCH bpf-next v3 13/16] sfc: " Toke Høiland-Jørgensen 2021-06-17 21:27 ` [PATCH bpf-next v3 14/16] netsec: " Toke Høiland-Jørgensen 2021-06-17 21:27 ` [PATCH bpf-next v3 15/16] stmmac: " Toke Høiland-Jørgensen 2021-06-18 9:47 ` Wong Vee Khee 2021-06-17 21:27 ` [PATCH bpf-next v3 16/16] net: ti: " Toke Høiland-Jørgensen
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=20210617212748.32456-7-toke@redhat.com \ --to=toke@redhat.com \ --cc=bpf@vger.kernel.org \ --cc=brouer@redhat.com \ --cc=kafai@fb.com \ --cc=kuba@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=liuhangbin@gmail.com \ --cc=magnus.karlsson@gmail.com \ --cc=netdev@vger.kernel.org \ --cc=paulmck@kernel.org \ --cc=sgoutham@marvell.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: linkBe 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.