Commit c728a95c authored by Wei Fang's avatar Wei Fang Committed by Jakub Kicinski

net: enetc: block concurrent XDP transmissions during ring reconfiguration

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: c33bfaf9 ("net: enetc: set up XDP program under enetc_reconfigure()")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarWei Fang <wei.fang@nxp.com>
Reviewed-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
Link: https://patch.msgid.link/20241010092056.298128-3-wei.fang@nxp.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 412950d5
...@@ -902,6 +902,7 @@ static bool enetc_clean_tx_ring(struct enetc_bdr *tx_ring, int napi_budget) ...@@ -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) && if (unlikely(tx_frm_cnt && netif_carrier_ok(ndev) &&
__netif_subqueue_stopped(ndev, tx_ring->index) && __netif_subqueue_stopped(ndev, tx_ring->index) &&
!test_bit(ENETC_TX_DOWN, &priv->flags) &&
(enetc_bd_unused(tx_ring) >= ENETC_TXBDS_MAX_NEEDED))) { (enetc_bd_unused(tx_ring) >= ENETC_TXBDS_MAX_NEEDED))) {
netif_wake_subqueue(ndev, tx_ring->index); netif_wake_subqueue(ndev, tx_ring->index);
} }
...@@ -1377,6 +1378,9 @@ int enetc_xdp_xmit(struct net_device *ndev, int num_frames, ...@@ -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_bd_cnt, i, k;
int xdp_tx_frm_cnt = 0; int xdp_tx_frm_cnt = 0;
if (unlikely(test_bit(ENETC_TX_DOWN, &priv->flags)))
return -ENETDOWN;
enetc_lock_mdio(); enetc_lock_mdio();
tx_ring = priv->xdp_tx_ring[smp_processor_id()]; 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, ...@@ -1602,6 +1606,12 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring,
break; break;
case XDP_TX: case XDP_TX:
tx_ring = priv->xdp_tx_ring[rx_ring->index]; 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, xdp_tx_bd_cnt = enetc_rx_swbd_to_xdp_tx_swbd(xdp_tx_arr,
rx_ring, rx_ring,
orig_i, i); orig_i, i);
...@@ -2463,6 +2473,8 @@ void enetc_start(struct net_device *ndev) ...@@ -2463,6 +2473,8 @@ void enetc_start(struct net_device *ndev)
enetc_enable_bdrs(priv); enetc_enable_bdrs(priv);
netif_tx_start_all_queues(ndev); netif_tx_start_all_queues(ndev);
clear_bit(ENETC_TX_DOWN, &priv->flags);
} }
EXPORT_SYMBOL_GPL(enetc_start); EXPORT_SYMBOL_GPL(enetc_start);
...@@ -2520,6 +2532,8 @@ void enetc_stop(struct net_device *ndev) ...@@ -2520,6 +2532,8 @@ void enetc_stop(struct net_device *ndev)
struct enetc_ndev_priv *priv = netdev_priv(ndev); struct enetc_ndev_priv *priv = netdev_priv(ndev);
int i; int i;
set_bit(ENETC_TX_DOWN, &priv->flags);
netif_tx_stop_all_queues(ndev); netif_tx_stop_all_queues(ndev);
enetc_disable_bdrs(priv); enetc_disable_bdrs(priv);
......
...@@ -325,6 +325,7 @@ enum enetc_active_offloads { ...@@ -325,6 +325,7 @@ enum enetc_active_offloads {
enum enetc_flags_bit { enum enetc_flags_bit {
ENETC_TX_ONESTEP_TSTAMP_IN_PROGRESS = 0, ENETC_TX_ONESTEP_TSTAMP_IN_PROGRESS = 0,
ENETC_TX_DOWN,
}; };
/* interrupt coalescing modes */ /* interrupt coalescing modes */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment