From 0939e0537896e421e391fa4b1a0b052907808e0d Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Thu, 24 Jun 2021 18:05:57 +0200 Subject: ena: Remove rcu_read_lock() around XDP program invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ena 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. Signed-off-by: Toke Høiland-Jørgensen Signed-off-by: Daniel Borkmann Cc: Saeed Bishara Cc: Guy Tzalik Link: https://lore.kernel.org/bpf/20210624160609.292325-8-toke@redhat.com --- drivers/net/ethernet/amazon/ena/ena_netdev.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c index 3bb0e66b2c7e..44ef6b88f715 100644 --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c @@ -382,7 +382,6 @@ static int ena_xdp_execute(struct ena_ring *rx_ring, struct xdp_buff *xdp) struct xdp_frame *xdpf; u64 *xdp_stat; - rcu_read_lock(); xdp_prog = READ_ONCE(rx_ring->xdp_bpf_prog); if (!xdp_prog) @@ -439,8 +438,6 @@ static int ena_xdp_execute(struct ena_ring *rx_ring, struct xdp_buff *xdp) ena_increase_stat(xdp_stat, 1, &rx_ring->syncp); out: - rcu_read_unlock(); - return verdict; } -- cgit v1.2.3 From 158c1399fc45c5178a3f2b8b68ff2faa2e36a52d Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Thu, 24 Jun 2021 18:05:58 +0200 Subject: bnxt: Remove rcu_read_lock() around XDP program invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bnxt 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. Signed-off-by: Toke Høiland-Jørgensen Signed-off-by: Daniel Borkmann Cc: Michael Chan Link: https://lore.kernel.org/bpf/20210624160609.292325-9-toke@redhat.com --- drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c index ec9564e584e0..bee6e091a997 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c @@ -138,9 +138,7 @@ bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr, u16 cons, xdp_prepare_buff(&xdp, *data_ptr - offset, offset, *len, false); orig_data = xdp.data; - rcu_read_lock(); act = bpf_prog_run_xdp(xdp_prog, &xdp); - rcu_read_unlock(); tx_avail = bnxt_tx_avail(bp, txr); /* If the tx ring is not full, we must not update the rx producer yet -- cgit v1.2.3 From 36baafe347a85a9d85f61aac0a9b53c53635829e Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Thu, 24 Jun 2021 18:05:59 +0200 Subject: thunderx: Remove rcu_read_lock() around XDP program invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: Toke Høiland-Jørgensen Signed-off-by: Daniel Borkmann Cc: Sunil Goutham Cc: linux-arm-kernel@lists.infradead.org Link: https://lore.kernel.org/bpf/20210624160609.292325-10-toke@redhat.com --- drivers/net/ethernet/cavium/thunder/nicvf_main.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c index c33b4e837515..e2b290135fd9 100644 --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c @@ -555,9 +555,7 @@ 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(); action = bpf_prog_run_xdp(prog, &xdp); - rcu_read_unlock(); len = xdp.data_end - xdp.data; /* Check if XDP program has changed headers */ -- cgit v1.2.3 From 547aabcac3251c40e4cd09d79dba70f7eab8cca2 Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Thu, 24 Jun 2021 18:06:00 +0200 Subject: freescale: Remove rcu_read_lock() around XDP program invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dpaa and dpaa2 drivers have 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. Signed-off-by: Toke Høiland-Jørgensen Signed-off-by: Daniel Borkmann Reviewed-by: Camelia Groza Cc: Ioana Radulescu Cc: Madalin Bucur Cc: Ioana Ciornei Link: https://lore.kernel.org/bpf/20210624160609.292325-11-toke@redhat.com --- drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 8 +------- drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 3 --- 2 files changed, 1 insertion(+), 10 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c index 177c020bf34a..e6826561cf11 100644 --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c @@ -2558,13 +2558,9 @@ static u32 dpaa_run_xdp(struct dpaa_priv *priv, struct qm_fd *fd, void *vaddr, u32 xdp_act; int err; - rcu_read_lock(); - xdp_prog = READ_ONCE(priv->xdp_prog); - if (!xdp_prog) { - rcu_read_unlock(); + if (!xdp_prog) return XDP_PASS; - } xdp_init_buff(&xdp, DPAA_BP_RAW_SIZE - DPAA_TX_PRIV_DATA_SIZE, &dpaa_fq->xdp_rxq); @@ -2638,8 +2634,6 @@ static u32 dpaa_run_xdp(struct dpaa_priv *priv, struct qm_fd *fd, void *vaddr, break; } - rcu_read_unlock(); - return xdp_act; } diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c index 8433aa730c42..973352393bd4 100644 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c @@ -352,8 +352,6 @@ static u32 dpaa2_eth_run_xdp(struct dpaa2_eth_priv *priv, u32 xdp_act = XDP_PASS; int err, offset; - rcu_read_lock(); - xdp_prog = READ_ONCE(ch->xdp.prog); if (!xdp_prog) goto out; @@ -414,7 +412,6 @@ static u32 dpaa2_eth_run_xdp(struct dpaa2_eth_priv *priv, ch->xdp.res |= xdp_act; out: - rcu_read_unlock(); return xdp_act; } -- cgit v1.2.3 From 49589b23d5a92dff4a7cb705608dff7dd13ef709 Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Thu, 24 Jun 2021 18:06:01 +0200 Subject: intel: Remove rcu_read_lock() around XDP program invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Intel drivers all have 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. Signed-off-by: Toke Høiland-Jørgensen Signed-off-by: Daniel Borkmann Tested-by: Jesper Dangaard Brouer # i40e Cc: Jesse Brandeburg Cc: Tony Nguyen Cc: intel-wired-lan@lists.osuosl.org Link: https://lore.kernel.org/bpf/20210624160609.292325-12-toke@redhat.com --- drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 -- drivers/net/ethernet/intel/i40e/i40e_xsk.c | 3 --- drivers/net/ethernet/intel/ice/ice_txrx.c | 6 +----- drivers/net/ethernet/intel/ice/ice_xsk.c | 3 --- drivers/net/ethernet/intel/igb/igb_main.c | 2 -- drivers/net/ethernet/intel/igc/igc_main.c | 7 ++----- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 -- drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 3 --- drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 -- 9 files changed, 3 insertions(+), 27 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index b883ab809df3..38eb8151ee9a 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c @@ -2298,7 +2298,6 @@ static int i40e_run_xdp(struct i40e_ring *rx_ring, struct xdp_buff *xdp) struct bpf_prog *xdp_prog; u32 act; - rcu_read_lock(); xdp_prog = READ_ONCE(rx_ring->xdp_prog); if (!xdp_prog) @@ -2334,7 +2333,6 @@ out_failure: break; } xdp_out: - rcu_read_unlock(); return result; } diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c index 68f177a86403..e7e778ca074c 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c @@ -153,7 +153,6 @@ static int i40e_run_xdp_zc(struct i40e_ring *rx_ring, struct xdp_buff *xdp) struct bpf_prog *xdp_prog; u32 act; - rcu_read_lock(); /* NB! xdp_prog will always be !NULL, due to the fact that * this path is enabled by setting an XDP program. */ @@ -164,7 +163,6 @@ static int i40e_run_xdp_zc(struct i40e_ring *rx_ring, struct xdp_buff *xdp) err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog); if (err) goto out_failure; - rcu_read_unlock(); return I40E_XDP_REDIR; } @@ -188,7 +186,6 @@ out_failure: result = I40E_XDP_CONSUMED; break; } - rcu_read_unlock(); return result; } diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c index 917eba7fdd0c..dd791ca34fab 100644 --- a/drivers/net/ethernet/intel/ice/ice_txrx.c +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c @@ -1135,15 +1135,11 @@ int ice_clean_rx_irq(struct ice_ring *rx_ring, int budget) xdp.frame_sz = ice_rx_frame_truesize(rx_ring, size); #endif - rcu_read_lock(); xdp_prog = READ_ONCE(rx_ring->xdp_prog); - if (!xdp_prog) { - rcu_read_unlock(); + if (!xdp_prog) goto construct_skb; - } xdp_res = ice_run_xdp(rx_ring, &xdp, xdp_prog); - rcu_read_unlock(); if (!xdp_res) goto construct_skb; if (xdp_res & (ICE_XDP_TX | ICE_XDP_REDIR)) { diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c index 239b9bf10794..8a093368f631 100644 --- a/drivers/net/ethernet/intel/ice/ice_xsk.c +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c @@ -466,7 +466,6 @@ ice_run_xdp_zc(struct ice_ring *rx_ring, struct xdp_buff *xdp) struct ice_ring *xdp_ring; u32 act; - rcu_read_lock(); /* ZC patch is enabled only when XDP program is set, * so here it can not be NULL */ @@ -478,7 +477,6 @@ ice_run_xdp_zc(struct ice_ring *rx_ring, struct xdp_buff *xdp) err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog); if (err) goto out_failure; - rcu_read_unlock(); return ICE_XDP_REDIR; } @@ -503,7 +501,6 @@ out_failure: break; } - rcu_read_unlock(); return result; } diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 5db303d64d14..7e6435dc7e80 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -8381,7 +8381,6 @@ static struct sk_buff *igb_run_xdp(struct igb_adapter *adapter, struct bpf_prog *xdp_prog; u32 act; - rcu_read_lock(); xdp_prog = READ_ONCE(rx_ring->xdp_prog); if (!xdp_prog) @@ -8416,7 +8415,6 @@ out_failure: break; } xdp_out: - rcu_read_unlock(); return ERR_PTR(-result); } diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c index 3f6b6d4543a8..95323095094d 100644 --- a/drivers/net/ethernet/intel/igc/igc_main.c +++ b/drivers/net/ethernet/intel/igc/igc_main.c @@ -2240,18 +2240,15 @@ static struct sk_buff *igc_xdp_run_prog(struct igc_adapter *adapter, struct bpf_prog *prog; int res; - rcu_read_lock(); - prog = READ_ONCE(adapter->xdp_prog); if (!prog) { res = IGC_XDP_PASS; - goto unlock; + goto out; } res = __igc_xdp_run_prog(adapter, prog, xdp); -unlock: - rcu_read_unlock(); +out: return ERR_PTR(-res); } diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 2ac5b82676f3..ffff69efd78a 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -2199,7 +2199,6 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter, struct xdp_frame *xdpf; u32 act; - rcu_read_lock(); xdp_prog = READ_ONCE(rx_ring->xdp_prog); if (!xdp_prog) @@ -2237,7 +2236,6 @@ out_failure: break; } xdp_out: - rcu_read_unlock(); return ERR_PTR(-result); } diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c index f72d2978263b..96dd1a4f956a 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c @@ -100,7 +100,6 @@ static int ixgbe_run_xdp_zc(struct ixgbe_adapter *adapter, struct xdp_frame *xdpf; u32 act; - rcu_read_lock(); xdp_prog = READ_ONCE(rx_ring->xdp_prog); act = bpf_prog_run_xdp(xdp_prog, xdp); @@ -108,7 +107,6 @@ static int ixgbe_run_xdp_zc(struct ixgbe_adapter *adapter, err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog); if (err) goto out_failure; - rcu_read_unlock(); return IXGBE_XDP_REDIR; } @@ -134,7 +132,6 @@ out_failure: result = IXGBE_XDP_CONSUMED; break; } - rcu_read_unlock(); return result; } diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c index dc56931fc1dc..c714e1ecd308 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c @@ -1054,7 +1054,6 @@ static struct sk_buff *ixgbevf_run_xdp(struct ixgbevf_adapter *adapter, struct bpf_prog *xdp_prog; u32 act; - rcu_read_lock(); xdp_prog = READ_ONCE(rx_ring->xdp_prog); if (!xdp_prog) @@ -1082,7 +1081,6 @@ out_failure: break; } xdp_out: - rcu_read_unlock(); return ERR_PTR(-result); } -- cgit v1.2.3 From 959ad7ec066d9a61557ad6aedf77ea9b54c82df0 Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Thu, 24 Jun 2021 18:06:02 +0200 Subject: marvell: Remove rcu_read_lock() around XDP program invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mvneta and mvpp2 drivers have 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. Signed-off-by: Toke Høiland-Jørgensen Signed-off-by: Daniel Borkmann Cc: Thomas Petazzoni Cc: Russell King Cc: Marcin Wojtas Link: https://lore.kernel.org/bpf/20210624160609.292325-13-toke@redhat.com --- drivers/net/ethernet/marvell/mvneta.c | 2 -- drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 4 ---- 2 files changed, 6 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c index c15ce06427d0..ada4e26a5492 100644 --- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -2373,7 +2373,6 @@ static int mvneta_rx_swbm(struct napi_struct *napi, /* Get number of received packets */ rx_todo = mvneta_rxq_busy_desc_num_get(pp, rxq); - rcu_read_lock(); xdp_prog = READ_ONCE(pp->xdp_prog); /* Fairness NAPI loop */ @@ -2451,7 +2450,6 @@ next: xdp_buf.data_hard_start = NULL; sinfo.nr_frags = 0; } - rcu_read_unlock(); if (xdp_buf.data_hard_start) mvneta_xdp_put_buff(pp, rxq, &xdp_buf, &sinfo, -1); diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index 9bca8c8f9f8d..c31677527a02 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -3881,8 +3881,6 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi, int rx_done = 0; u32 xdp_ret = 0; - rcu_read_lock(); - xdp_prog = READ_ONCE(port->xdp_prog); /* Get number of received packets and clamp the to-do */ @@ -4028,8 +4026,6 @@ err_drop_frame: mvpp2_bm_pool_put(port, pool, dma_addr, phys_addr); } - rcu_read_unlock(); - if (xdp_ret & MVPP2_XDP_REDIR) xdp_do_flush_map(); -- cgit v1.2.3 From c4411b371c104e65efb531ebd4d8892c568e3a29 Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Thu, 24 Jun 2021 18:06:03 +0200 Subject: mlx4: Remove rcu_read_lock() around XDP program invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mlx4 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. Also switch the RCU dereferences in the driver loop itself to the _bh variants. Signed-off-by: Toke Høiland-Jørgensen Signed-off-by: Daniel Borkmann Reviewed-by: Tariq Toukan Link: https://lore.kernel.org/bpf/20210624160609.292325-14-toke@redhat.com --- drivers/net/ethernet/mellanox/mlx4/en_rx.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c index cea62b8f554c..442991d91c15 100644 --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c @@ -679,9 +679,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud ring = priv->rx_ring[cq_ring]; - /* Protect accesses to: ring->xdp_prog, priv->mac_hash list */ - rcu_read_lock(); - xdp_prog = rcu_dereference(ring->xdp_prog); + xdp_prog = rcu_dereference_bh(ring->xdp_prog); xdp_init_buff(&xdp, priv->frag_info[0].frag_stride, &ring->xdp_rxq); doorbell_pending = false; @@ -744,7 +742,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud /* Drop the packet, since HW loopback-ed it */ mac_hash = ethh->h_source[MLX4_EN_MAC_HASH_IDX]; bucket = &priv->mac_hash[mac_hash]; - hlist_for_each_entry_rcu(entry, bucket, hlist) { + hlist_for_each_entry_rcu_bh(entry, bucket, hlist) { if (ether_addr_equal_64bits(entry->mac, ethh->h_source)) goto next; @@ -899,8 +897,6 @@ next: break; } - rcu_read_unlock(); - if (likely(polled)) { if (doorbell_pending) { priv->tx_cq[TX_XDP][cq_ring]->xdp_busy = true; -- cgit v1.2.3 From d5789621b658369b21bd13446bab8102cf75df65 Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Thu, 24 Jun 2021 18:06:04 +0200 Subject: nfp: Remove rcu_read_lock() around XDP program invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: Toke Høiland-Jørgensen Signed-off-by: Daniel Borkmann Reviewed-by: Simon Horman Cc: oss-drivers@netronome.com Link: https://lore.kernel.org/bpf/20210624160609.292325-15-toke@redhat.com --- drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c index eeb30680b4dc..5dfa4799c34f 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, @@ -2036,7 +2035,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; } -- cgit v1.2.3 From 4415db6ca85ae57830a83290388f2b9dfa5f237f Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Thu, 24 Jun 2021 18:06:05 +0200 Subject: qede: Remove rcu_read_lock() around XDP program invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The qede 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. Signed-off-by: Toke Høiland-Jørgensen Signed-off-by: Daniel Borkmann Cc: Ariel Elior Cc: gr-everest-linux-l2@marvell.com Link: https://lore.kernel.org/bpf/20210624160609.292325-16-toke@redhat.com --- drivers/net/ethernet/qlogic/qede/qede_fp.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c index 8e150dd4f899..065e9004598e 100644 --- a/drivers/net/ethernet/qlogic/qede/qede_fp.c +++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c @@ -1089,13 +1089,7 @@ static bool qede_rx_xdp(struct qede_dev *edev, xdp_prepare_buff(&xdp, page_address(bd->data), *data_offset, *len, false); - /* Queues always have a full reset currently, so for the time - * being until there's atomic program replace just mark read - * side for map helpers. - */ - rcu_read_lock(); act = bpf_prog_run_xdp(prog, &xdp); - rcu_read_unlock(); /* Recalculate, as XDP might have changed the headers */ *data_offset = xdp.data - xdp.data_hard_start; -- cgit v1.2.3 From 4eb14e3fc6197b7205069ed4e2b31eafa11a0697 Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Thu, 24 Jun 2021 18:06:06 +0200 Subject: sfc: Remove rcu_read_lock() around XDP program invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sfc 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. Signed-off-by: Toke Høiland-Jørgensen Signed-off-by: Daniel Borkmann Acked-by: Edward Cree Cc: Martin Habets Link: https://lore.kernel.org/bpf/20210624160609.292325-17-toke@redhat.com --- drivers/net/ethernet/sfc/rx.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c index 17b8119c48e5..606750938b89 100644 --- a/drivers/net/ethernet/sfc/rx.c +++ b/drivers/net/ethernet/sfc/rx.c @@ -260,18 +260,14 @@ static bool efx_do_xdp(struct efx_nic *efx, struct efx_channel *channel, s16 offset; int err; - rcu_read_lock(); - xdp_prog = rcu_dereference(efx->xdp_prog); - if (!xdp_prog) { - rcu_read_unlock(); + xdp_prog = rcu_dereference_bh(efx->xdp_prog); + if (!xdp_prog) return true; - } rx_queue = efx_channel_get_rx_queue(channel); if (unlikely(channel->rx_pkt_n_frags > 1)) { /* We can't do XDP on fragmented packets - drop. */ - rcu_read_unlock(); efx_free_rx_buffers(rx_queue, rx_buf, channel->rx_pkt_n_frags); if (net_ratelimit()) @@ -296,7 +292,6 @@ static bool efx_do_xdp(struct efx_nic *efx, struct efx_channel *channel, rx_buf->len, false); xdp_act = bpf_prog_run_xdp(xdp_prog, &xdp); - rcu_read_unlock(); offset = (u8 *)xdp.data - *ehp; -- cgit v1.2.3 From 7b6ee873ff20c22af355661b241defa7f6ed7582 Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Thu, 24 Jun 2021 18:06:07 +0200 Subject: netsec: Remove rcu_read_lock() around XDP program invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The netsec driver has a rcu_read_lock()/rcu_read_unlock() pair around the full RX loop, covering everything up to and including xdp_do_flush(). This is actually the correct behaviour, but because it all happens in a single NAPI poll cycle (and thus under local_bh_disable()), it is also technically redundant. 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 the rcu_read_lock() around anymore, so let's just remove it. Signed-off-by: Toke Høiland-Jørgensen Signed-off-by: Daniel Borkmann Acked-by: Ilias Apalodimas Cc: Jassi Brar Link: https://lore.kernel.org/bpf/20210624160609.292325-18-toke@redhat.com --- drivers/net/ethernet/socionext/netsec.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c index dfc85cc68173..20d148c019d8 100644 --- a/drivers/net/ethernet/socionext/netsec.c +++ b/drivers/net/ethernet/socionext/netsec.c @@ -958,7 +958,6 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget) xdp_init_buff(&xdp, PAGE_SIZE, &dring->xdp_rxq); - rcu_read_lock(); xdp_prog = READ_ONCE(priv->xdp_prog); dma_dir = page_pool_get_dma_dir(dring->page_pool); @@ -1069,8 +1068,6 @@ next: } netsec_finalize_xdp_rx(priv, xdp_act, xdp_xmit); - rcu_read_unlock(); - return done; } -- cgit v1.2.3 From 2f1e432d339c5fed435adf521cae392755721050 Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Thu, 24 Jun 2021 18:06:08 +0200 Subject: stmmac: Remove rcu_read_lock() around XDP program invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stmmac 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. Signed-off-by: Toke Høiland-Jørgensen Signed-off-by: Daniel Borkmann Cc: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Link: https://lore.kernel.org/bpf/20210624160609.292325-19-toke@redhat.com --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 16820873b01d..219535ab2c0c 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -4651,7 +4651,6 @@ static int stmmac_xdp_xmit_back(struct stmmac_priv *priv, return res; } -/* This function assumes rcu_read_lock() is held by the caller. */ static int __stmmac_xdp_run_prog(struct stmmac_priv *priv, struct bpf_prog *prog, struct xdp_buff *xdp) @@ -4693,17 +4692,14 @@ static struct sk_buff *stmmac_xdp_run_prog(struct stmmac_priv *priv, struct bpf_prog *prog; int res; - rcu_read_lock(); - prog = READ_ONCE(priv->xdp_prog); if (!prog) { res = STMMAC_XDP_PASS; - goto unlock; + goto out; } res = __stmmac_xdp_run_prog(priv, prog, xdp); -unlock: - rcu_read_unlock(); +out: return ERR_PTR(-res); } @@ -4973,10 +4969,8 @@ read_again: buf->xdp->data_end = buf->xdp->data + buf1_len; xsk_buff_dma_sync_for_cpu(buf->xdp, rx_q->xsk_pool); - rcu_read_lock(); prog = READ_ONCE(priv->xdp_prog); res = __stmmac_xdp_run_prog(priv, prog, buf->xdp); - rcu_read_unlock(); switch (res) { case STMMAC_XDP_PASS: -- cgit v1.2.3 From 0cc84b9a6003fa7f6ef5d19e7c8532a01cd41776 Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Thu, 24 Jun 2021 18:06:09 +0200 Subject: ti: Remove rcu_read_lock() around XDP program invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cpsw 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. Signed-off-by: Toke Høiland-Jørgensen Signed-off-by: Daniel Borkmann Tested-by: Grygorii Strashko Reviewed-by: Grygorii Strashko Cc: linux-omap@vger.kernel.org Link: https://lore.kernel.org/bpf/20210624160609.292325-20-toke@redhat.com --- drivers/net/ethernet/ti/cpsw_priv.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/ethernet/ti/cpsw_priv.c b/drivers/net/ethernet/ti/cpsw_priv.c index 5862f0a4a975..ecc2a6b7e28f 100644 --- a/drivers/net/ethernet/ti/cpsw_priv.c +++ b/drivers/net/ethernet/ti/cpsw_priv.c @@ -1328,13 +1328,9 @@ int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp, struct bpf_prog *prog; u32 act; - rcu_read_lock(); - prog = READ_ONCE(priv->xdp_prog); - if (!prog) { - ret = CPSW_XDP_PASS; - goto out; - } + if (!prog) + return CPSW_XDP_PASS; act = bpf_prog_run_xdp(prog, xdp); /* XDP prog might have changed packet data and boundaries */ @@ -1378,10 +1374,8 @@ int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp, ndev->stats.rx_bytes += *len; ndev->stats.rx_packets++; out: - rcu_read_unlock(); return ret; drop: - rcu_read_unlock(); page_pool_recycle_direct(cpsw->page_pool[ch], page); return ret; } -- cgit v1.2.3