When testing the XDP_REDIRECT function on the LS1028A platform, we
found a very reproducible issue that the Tx frames can no longer be
sent out even if XDP_REDIRECT is turned off. Specifically, if there
is a lot of traffic on Rx direction, when XDP_REDIRECT is turned on,
the console may display some warnings like "timeout for tx ring #6
clear", and all redirected frames will be dropped, the detailed log
is as follows.
root@ls1028ardb:~# ./xdp-bench redirect eno0 eno2
Redirecting from eno0 (ifindex 3; driver fsl_enetc) to eno2 (ifindex 4; driver fsl_enetc)
[203.849809] fsl_enetc 0000:00:00.2 eno2: timeout for tx ring #5 clear
[204.006051] fsl_enetc 0000:00:00.2 eno2: timeout for tx ring #6 clear
[204.161944] fsl_enetc 0000:00:00.2 eno2: timeout for tx ring #7 clear
eno0->eno2 1420505 rx/s 1420590 err,drop/s 0 xmit/s
xmit eno0->eno2 0 xmit/s 1420590 drop/s 0 drv_err/s 15.71 bulk-avg
eno0->eno2 1420484 rx/s 1420485 err,drop/s 0 xmit/s
xmit eno0->eno2 0 xmit/s 1420485 drop/s 0 drv_err/s 15.71 bulk-avg
By analyzing the XDP_REDIRECT implementation of enetc driver, the
driver will reconfigure Tx and Rx BD rings when a bpf program is
installed or uninstalled, but there is no mechanisms to block the
redirected frames when enetc driver reconfigures rings. Similarly,
XDP_TX verdicts on received frames can also lead to frames being
enqueued in the Tx rings. Because XDP ignores the state set by the
netif_tx_wake_queue() API, so introduce the ENETC_TX_DOWN flag to
suppress transmission of XDP frames.
Fixes: c33bfaf91c4c ("net: enetc: set up XDP program under enetc_reconfigure()")
Cc: stable@vger.kernel.org
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
v2 changes:
Remove a blank line from the end of enetc_disable_tx_bdrs().
v3 changes:
Block the XDP_TX frames when ENETC_TX_DOWN flag is set.
v4 changes:
1. Modify the title and rephrase the commit message.
2. Move the changes of operation order in enect_stop() and
enetc_stop() to a separated patch (patch 3).
---
drivers/net/ethernet/freescale/enetc/enetc.c | 14 ++++++++++++++
drivers/net/ethernet/freescale/enetc/enetc.h | 1 +
2 files changed, 15 insertions(+)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 56e59721ec7d..482c44ed9d46 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -902,6 +902,7 @@ static bool enetc_clean_tx_ring(struct enetc_bdr *tx_ring, int napi_budget)
if (unlikely(tx_frm_cnt && netif_carrier_ok(ndev) &&
__netif_subqueue_stopped(ndev, tx_ring->index) &&
+ !test_bit(ENETC_TX_DOWN, &priv->flags) &&
(enetc_bd_unused(tx_ring) >= ENETC_TXBDS_MAX_NEEDED))) {
netif_wake_subqueue(ndev, tx_ring->index);
}
@@ -1377,6 +1378,9 @@ int enetc_xdp_xmit(struct net_device *ndev, int num_frames,
int xdp_tx_bd_cnt, i, k;
int xdp_tx_frm_cnt = 0;
+ if (unlikely(test_bit(ENETC_TX_DOWN, &priv->flags)))
+ return -ENETDOWN;
+
enetc_lock_mdio();
tx_ring = priv->xdp_tx_ring[smp_processor_id()];
@@ -1602,6 +1606,12 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring,
break;
case XDP_TX:
tx_ring = priv->xdp_tx_ring[rx_ring->index];
+ if (unlikely(test_bit(ENETC_TX_DOWN, &priv->flags))) {
+ enetc_xdp_drop(rx_ring, orig_i, i);
+ tx_ring->stats.xdp_tx_drops++;
+ break;
+ }
+
xdp_tx_bd_cnt = enetc_rx_swbd_to_xdp_tx_swbd(xdp_tx_arr,
rx_ring,
orig_i, i);
@@ -2463,6 +2473,8 @@ void enetc_start(struct net_device *ndev)
enetc_enable_bdrs(priv);
netif_tx_start_all_queues(ndev);
+
+ clear_bit(ENETC_TX_DOWN, &priv->flags);
}
EXPORT_SYMBOL_GPL(enetc_start);
@@ -2520,6 +2532,8 @@ void enetc_stop(struct net_device *ndev)
struct enetc_ndev_priv *priv = netdev_priv(ndev);
int i;
+ set_bit(ENETC_TX_DOWN, &priv->flags);
+
netif_tx_stop_all_queues(ndev);
enetc_disable_bdrs(priv);
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
index 97524dfa234c..fb7d98d57783 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h
@@ -325,6 +325,7 @@ enum enetc_active_offloads {
enum enetc_flags_bit {
ENETC_TX_ONESTEP_TSTAMP_IN_PROGRESS = 0,
+ ENETC_TX_DOWN,
};
/* interrupt coalescing modes */
--
2.34.1
On Thu, Oct 10, 2024 at 05:20:54PM +0800, Wei Fang wrote: > When testing the XDP_REDIRECT function on the LS1028A platform, we > found a very reproducible issue that the Tx frames can no longer be > sent out even if XDP_REDIRECT is turned off. Specifically, if there > is a lot of traffic on Rx direction, when XDP_REDIRECT is turned on, > the console may display some warnings like "timeout for tx ring #6 > clear", and all redirected frames will be dropped, the detailed log > is as follows. > > root@ls1028ardb:~# ./xdp-bench redirect eno0 eno2 > Redirecting from eno0 (ifindex 3; driver fsl_enetc) to eno2 (ifindex 4; driver fsl_enetc) > [203.849809] fsl_enetc 0000:00:00.2 eno2: timeout for tx ring #5 clear > [204.006051] fsl_enetc 0000:00:00.2 eno2: timeout for tx ring #6 clear > [204.161944] fsl_enetc 0000:00:00.2 eno2: timeout for tx ring #7 clear > eno0->eno2 1420505 rx/s 1420590 err,drop/s 0 xmit/s > xmit eno0->eno2 0 xmit/s 1420590 drop/s 0 drv_err/s 15.71 bulk-avg > eno0->eno2 1420484 rx/s 1420485 err,drop/s 0 xmit/s > xmit eno0->eno2 0 xmit/s 1420485 drop/s 0 drv_err/s 15.71 bulk-avg > > By analyzing the XDP_REDIRECT implementation of enetc driver, the > driver will reconfigure Tx and Rx BD rings when a bpf program is > installed or uninstalled, but there is no mechanisms to block the > redirected frames when enetc driver reconfigures rings. Similarly, > XDP_TX verdicts on received frames can also lead to frames being > enqueued in the Tx rings. Because XDP ignores the state set by the > netif_tx_wake_queue() API, so introduce the ENETC_TX_DOWN flag to > suppress transmission of XDP frames. > > Fixes: c33bfaf91c4c ("net: enetc: set up XDP program under enetc_reconfigure()") > Cc: stable@vger.kernel.org > Signed-off-by: Wei Fang <wei.fang@nxp.com> > --- Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
© 2016 - 2024 Red Hat, Inc.