All of lore.kernel.org
 help / color / mirror / Atom feed
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>,
	"Simon Horman" <simon.horman@netronome.com>,
	oss-drivers@netronome.com
Subject: [PATCH bpf-next v2 11/16] nfp: remove rcu_read_lock() around XDP program invocation
Date: Tue, 15 Jun 2021 16:54:50 +0200	[thread overview]
Message-ID: <20210615145455.564037-12-toke@redhat.com> (raw)
In-Reply-To: <20210615145455.564037-1-toke@redhat.com>

The nfp 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.

While this is not actually an issue for the nfp driver because it doesn't
support XDP_REDIRECT (and thus doesn't call xdp_do_flush()), the
rcu_read_lock() is still unneeded. And 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: Simon Horman <simon.horman@netronome.com>
Cc: oss-drivers@netronome.com
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index eeb30680b4dc..a3d59abed6ae 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -1819,7 +1819,6 @@ static int nfp_net_rx(struct nfp_net_rx_ring *rx_ring, int budget)
 	struct xdp_buff xdp;
 	int idx;
 
-	rcu_read_lock();
 	xdp_prog = READ_ONCE(dp->xdp_prog);
 	true_bufsz = xdp_prog ? PAGE_SIZE : dp->fl_bufsz;
 	xdp_init_buff(&xdp, PAGE_SIZE - NFP_NET_RX_BUF_HEADROOM,
@@ -1919,6 +1918,10 @@ static int nfp_net_rx(struct nfp_net_rx_ring *rx_ring, int budget)
 					 pkt_off - NFP_NET_RX_BUF_HEADROOM,
 					 pkt_len, true);
 
+			/* This code is invoked within a single NAPI poll cycle
+			 * and thus under local_bh_disable(), which provides the
+			 * needed RCU protection.
+			 */
 			act = bpf_prog_run_xdp(xdp_prog, &xdp);
 
 			pkt_len = xdp.data_end - xdp.data;
@@ -2036,7 +2039,6 @@ static int nfp_net_rx(struct nfp_net_rx_ring *rx_ring, int budget)
 			if (!nfp_net_xdp_complete(tx_ring))
 				pkts_polled = budget;
 	}
-	rcu_read_unlock();
 
 	return pkts_polled;
 }
-- 
2.31.1


  parent reply	other threads:[~2021-06-15 15:04 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-15 14:54 [PATCH bpf-next v2 00/16] Clean up and document RCU-based object protection for XDP_REDIRECT Toke Høiland-Jørgensen
2021-06-15 14:54 ` [PATCH bpf-next v2 01/16] rcu: Create an unrcu_pointer() to remove __rcu from a pointer Toke Høiland-Jørgensen
2021-06-15 14:54 ` [PATCH bpf-next v2 02/16] bpf: allow RCU-protected lookups to happen from bh context Toke Høiland-Jørgensen
2021-06-15 14:54 ` [PATCH bpf-next v2 03/16] xdp: add proper __rcu annotations to redirect map entries Toke Høiland-Jørgensen
2021-06-17 19:41   ` Martin KaFai Lau
2021-06-17 21:13     ` Toke Høiland-Jørgensen
2021-06-15 14:54 ` [PATCH bpf-next v2 04/16] ena: remove rcu_read_lock() around XDP program invocation Toke Høiland-Jørgensen
2021-06-15 14:54 ` [PATCH bpf-next v2 05/16] bnxt: " Toke Høiland-Jørgensen
2021-06-15 14:54 ` [PATCH bpf-next v2 06/16] thunderx: " Toke Høiland-Jørgensen
2021-06-15 14:54   ` Toke Høiland-Jørgensen
2021-06-15 14:54 ` [PATCH bpf-next v2 07/16] freescale: " Toke Høiland-Jørgensen
2021-06-16 14:58   ` Camelia Alexandra Groza
2021-06-15 14:54 ` [PATCH bpf-next v2 08/16] net: intel: " Toke Høiland-Jørgensen
2021-06-15 14:54   ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2021-06-15 14:54 ` [PATCH bpf-next v2 09/16] marvell: " Toke Høiland-Jørgensen
2021-06-15 14:54 ` [PATCH bpf-next v2 10/16] mlx4: " Toke Høiland-Jørgensen
2021-06-15 14:54 ` Toke Høiland-Jørgensen [this message]
2021-06-15 14:54 ` [PATCH bpf-next v2 12/16] qede: " Toke Høiland-Jørgensen
2021-06-15 14:54 ` [PATCH bpf-next v2 13/16] sfc: " Toke Høiland-Jørgensen
2021-06-15 14:54 ` [PATCH bpf-next v2 14/16] netsec: " Toke Høiland-Jørgensen
2021-06-15 14:54 ` [PATCH bpf-next v2 15/16] stmmac: " Toke Høiland-Jørgensen
2021-06-15 14:54 ` [PATCH bpf-next v2 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=20210615145455.564037-12-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=liuhangbin@gmail.com \
    --cc=magnus.karlsson@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=oss-drivers@netronome.com \
    --cc=paulmck@kernel.org \
    --cc=simon.horman@netronome.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.