* [PATCH net-next v4 0/2] net: stmmac: Fix XSK crashes on stm32mp2
@ 2026-10-22 7:01 Kurt Kanzenbach
2026-10-22 7:01 ` [PATCH net-next v4 1/2] net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release() Kurt Kanzenbach
2026-10-22 7:01 ` [PATCH net-next v4 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools Kurt Kanzenbach
0 siblings, 2 replies; 5+ messages in thread
From: Kurt Kanzenbach @ 2026-10-22 7:01 UTC (permalink / raw)
To: Maxime Chevallier, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Eric Dumazet
Cc: Maxime Coquelin, Alexandre Torgue, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Stanislav Fomichev, Song Yoong Siang, Mohd Faizal Abdul Rahim,
Ong Boon Leong, Sebastian Andrzej Siewior, Maciej Fijalkowski,
Nicolai Buchwitz, netdev, linux-stm32, linux-arm-kernel, bpf,
Kurt Kanzenbach
Hi,
while looking at the TBS thingy [1], I've noticed kernel crashes. That
happens when opening or closing an AF_XDP/ZC socket while parallel Tx
traffic is going on.
So far, I see two issues:
- Patch #1: stmmac_xdp_release() seems to have the teardown order wrong
- Patch #2: stmmac_xdp_(en|dis)able_pool() does not stop the Tx queue
at all
The kernel crashes can be reproduced instantly on the stm32mp2 like this:
- Run iperf3
- Run application which opens an AF_XDP/ZC socket
[1] - https://lore.kernel.org/all/20260812-stm32mp2_txtime-v1-1-f9e2462cc85d@linutronix.de/
Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
Changes in v4:
- Rebase to net-next (Jakub)
- Add tags (Nicolai)
- Link to v3: https://patch.msgid.link/20261013-stmmac_xsk_crashes-v3-0-50fca7790af2@linutronix.de
Changes in v3:
- Remove txq_trans_cond_update() (Maciej)
- Add queue stop check for XDP TX etc. (Nicolai)
- Add tags (Nicolai and Maxime)
- Link to v2: https://patch.msgid.link/20261005-stmmac_xsk_crashes-v2-0-46c60cba6421@linutronix.de
Changes in v2:
- Remove RFC. No comments.
- Link to v1: https://patch.msgid.link/20260915-stmmac_xsk_crashes-v1-0-e14553fcf553@linutronix.de
---
Kurt Kanzenbach (2):
net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release()
net: stmmac: Stop Tx queue when (en|dis)abling XSK pools
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 +++++++---
drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c | 13 +++++++++++++
2 files changed, 20 insertions(+), 3 deletions(-)
---
base-commit: f71ff7d406523748bfb8236830c7eaa6a308aaa1
change-id: 20260914-stmmac_xsk_crashes-10a77f614ab5
Best regards,
--
Kurt Kanzenbach <kurt@linutronix.de>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net-next v4 1/2] net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release()
2026-10-22 7:01 [PATCH net-next v4 0/2] net: stmmac: Fix XSK crashes on stm32mp2 Kurt Kanzenbach
@ 2026-10-22 7:01 ` Kurt Kanzenbach
2026-10-26 7:24 ` netdev-bot+sashiko
2026-10-22 7:01 ` [PATCH net-next v4 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools Kurt Kanzenbach
1 sibling, 1 reply; 5+ messages in thread
From: Kurt Kanzenbach @ 2026-10-22 7:01 UTC (permalink / raw)
To: Maxime Chevallier, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Eric Dumazet
Cc: Maxime Coquelin, Alexandre Torgue, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Stanislav Fomichev, Song Yoong Siang, Mohd Faizal Abdul Rahim,
Ong Boon Leong, Sebastian Andrzej Siewior, Maciej Fijalkowski,
Nicolai Buchwitz, netdev, linux-stm32, linux-arm-kernel, bpf,
Kurt Kanzenbach
Attaching an XDP program while Tx traffic is running results in kernel
crashes in stmmac_xmit() -> dwmac4_set_addr().
Loading an XDP program tears down and reallocates all DMA resources via
stmmac_xdp_release() and stmmac_xdp_open(). stmmac_xdp_release() stops
the Tx queues before disabling NAPI:
stmmac_xdp_release:
netif_tx_disable
stmmac_disable_all_queues
...
free_dma_desc_resources
A Tx NAPI poll may still be in flight at that point. stmmac_tx_clean()
takes the Tx queue lock, reaps completed descriptors and wakes the queue
again when it observes it stopped with enough descriptors available.
Nothing stops the queue afterwards, so the Tx path resumes while
free_dma_desc_resources() releases the descriptor rings underneath it.
On non-coherent platforms dma_free_coherent() tears down the vmalloc
mapping of the descriptors, so the subsequent stmmac_xmit() faults on an
unmapped address instead of corrupting memory silently.
Disable NAPI first and stop the Tx queues afterwards, which is the order
already used by __stmmac_release().
The issue can be easily reproduced by:
1. Run iperf
2. Run application which opens an AF_XDP/ZC socket
Assisted-by: LLM
Fixes: 77711683a504 ("net: stmmac: ensure tx function is not running in stmmac_xdp_release()")
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Tested-by: Nicolai Buchwitz <nb@tipi-net.de> # stm32mp215
Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index fb53b1f17714..27c4a249e560 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7222,15 +7222,15 @@ void stmmac_xdp_release(struct net_device *dev)
struct stmmac_priv *priv = netdev_priv(dev);
u8 chan;
- /* Ensure tx function is not running */
- netif_tx_disable(dev);
-
/* Disable NAPI process */
stmmac_disable_all_queues(priv);
for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
+ /* Ensure tx function is not running */
+ netif_tx_disable(dev);
+
/* Free the IRQ lines */
stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net-next v4 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools
2026-10-22 7:01 [PATCH net-next v4 0/2] net: stmmac: Fix XSK crashes on stm32mp2 Kurt Kanzenbach
2026-10-22 7:01 ` [PATCH net-next v4 1/2] net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release() Kurt Kanzenbach
@ 2026-10-22 7:01 ` Kurt Kanzenbach
2026-10-26 7:24 ` netdev-bot+sashiko
1 sibling, 1 reply; 5+ messages in thread
From: Kurt Kanzenbach @ 2026-10-22 7:01 UTC (permalink / raw)
To: Maxime Chevallier, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Eric Dumazet
Cc: Maxime Coquelin, Alexandre Torgue, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Stanislav Fomichev, Song Yoong Siang, Mohd Faizal Abdul Rahim,
Ong Boon Leong, Sebastian Andrzej Siewior, Maciej Fijalkowski,
Nicolai Buchwitz, netdev, linux-stm32, linux-arm-kernel, bpf,
Kurt Kanzenbach
When enabling or disabling XSK pools in parallel to Tx traffic, kernel
crashes occur. For VLAN tagged frames that happens in stmmac_xmit() ->
dwmac4_set_vlan_tag() and for normal frames in stmmac_xmit() ->
dwmac4_set_addr(). Both of these functions access the Tx DMA descriptors.
The XDP pool (en|dis)ablement frees and reallocates the Tx DMA resources:
stmmac_disable_tx_queue:
__free_dma_tx_desc_resources
stmmac_enable_tx_queue:
__alloc_dma_tx_desc_resources
__init_dma_tx_desc_rings
NAPI is disabled during that allocation window, but the Tx queue is not
stopped. Therefore, add the stopping of the Tx queue during the enabling
and disabling of XSK pools.
The issue can be easily reproduced by:
1. Run iperf
2. Run application which opens an AF_XDP/ZC socket
Also add a dedicated queue check for the XDP Tx paths.
Fixes: 132c32ee5bc0 ("net: stmmac: Add TX via XDP zero-copy socket")
Suggested-by: Nicolai Buchwitz <nb@tipi-net.de> # XDP Tx checks
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Tested-by: Nicolai Buchwitz <nb@tipi-net.de> # stm32mp215
Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++++
drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c | 13 +++++++++++++
2 files changed, 17 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 27c4a249e560..2083520179a6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5270,6 +5270,10 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
if (unlikely(xdp_frame_has_frags(xdpf)))
num_frames += sinfo->nr_frags;
+ /* Ring may be torn down for an XSK pool switch */
+ if (netif_tx_queue_stopped(netdev_get_tx_queue(priv->dev, queue)))
+ return STMMAC_XDP_CONSUMED;
+
if (stmmac_tx_avail(priv, queue) < max(num_frames, txq_thr))
return STMMAC_XDP_CONSUMED;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
index 9af9e0cb366c..14445683a5c3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
@@ -6,6 +6,15 @@
#include "stmmac.h"
#include "stmmac_xdp.h"
+static void stmmac_xdp_stop_tx_queue(struct stmmac_priv *priv, u16 queue)
+{
+ struct netdev_queue *nq = netdev_get_tx_queue(priv->dev, queue);
+
+ __netif_tx_lock_bh(nq);
+ netif_tx_stop_queue(nq);
+ __netif_tx_unlock_bh(nq);
+}
+
static int stmmac_xdp_enable_pool(struct stmmac_priv *priv,
struct xsk_buff_pool *pool, u16 queue)
{
@@ -36,6 +45,7 @@ static int stmmac_xdp_enable_pool(struct stmmac_priv *priv,
if (need_update) {
napi_disable(&ch->rx_napi);
napi_disable(&ch->tx_napi);
+ stmmac_xdp_stop_tx_queue(priv, queue);
stmmac_disable_rx_queue(priv, queue);
stmmac_disable_tx_queue(priv, queue);
}
@@ -46,6 +56,7 @@ static int stmmac_xdp_enable_pool(struct stmmac_priv *priv,
stmmac_enable_rx_queue(priv, queue);
stmmac_enable_tx_queue(priv, queue);
napi_enable(&ch->rxtx_napi);
+ netif_tx_wake_queue(netdev_get_tx_queue(priv->dev, queue));
err = stmmac_xsk_wakeup(priv->dev, queue, XDP_WAKEUP_RX);
if (err)
@@ -73,6 +84,7 @@ static int stmmac_xdp_disable_pool(struct stmmac_priv *priv, u16 queue)
if (need_update) {
napi_disable(&ch->rxtx_napi);
+ stmmac_xdp_stop_tx_queue(priv, queue);
stmmac_disable_rx_queue(priv, queue);
stmmac_disable_tx_queue(priv, queue);
synchronize_rcu();
@@ -87,6 +99,7 @@ static int stmmac_xdp_disable_pool(struct stmmac_priv *priv, u16 queue)
stmmac_enable_tx_queue(priv, queue);
napi_enable(&ch->rx_napi);
napi_enable(&ch->tx_napi);
+ netif_tx_wake_queue(netdev_get_tx_queue(priv->dev, queue));
}
return 0;
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v4 1/2] net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release()
2026-10-22 7:01 ` [PATCH net-next v4 1/2] net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release() Kurt Kanzenbach
@ 2026-10-26 7:24 ` netdev-bot+sashiko
0 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-10-26 7:24 UTC (permalink / raw)
To: kurt
Cc: maxime.chevallier, andrew+netdev, davem, kuba, pabeni, edumazet,
mcoquelin.stm32, alexandre.torgue, ast, daniel, hawk,
john.fastabend, sdf, yoong.siang.song, faizal.abdul.rahim,
boon.leong.ong, bigeasy, maciej.fijalkowski, nb, netdev,
linux-stm32, linux-arm-kernel, bpf
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 2 (1) · Medium: 0 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] stmmac_xdp_release() now runs napi_disable (via
stmmac_disable_all_queues()), then hrtimer_cancel() on every…
- [High] The commit says the Tx queue must not be woken again after
netif_tx_disable() in stmmac_xdp_release(), and it fixes this only for…
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but
stmmac_xdp_set_prog() discards the return value of stmmac_xdp_open()…
--
net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release()
This patch reorders stmmac_xdp_release() so that NAPI is disabled and the
Tx coalescing timers are cancelled before netif_tx_disable(). The goal is
to stop stmmac_tx_clean() from waking the Tx queue after it was stopped,
which let stmmac_xmit() run on descriptor rings freed by
free_dma_desc_resources().
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index fb53b1f17714..27c4a249e560 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7222,15 +7222,15 @@ void stmmac_xdp_release(struct net_device *dev)
> struct stmmac_priv *priv = netdev_priv(dev);
> u8 chan;
>
> - /* Ensure tx function is not running */
> - netif_tx_disable(dev);
> -
> /* Disable NAPI process */
> stmmac_disable_all_queues(priv);
>
> for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
> hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
>
> + /* Ensure tx function is not running */
> + netif_tx_disable(dev);
> +
[Severity: High]
With this order, ndo_start_xmit is still live while the hrtimer_cancel()
loop runs. Can stmmac_xmit() or stmmac_tso_xmit() re-arm txtimer after it
has been cancelled?
Both of them end with stmmac_tx_timer_arm(), which does:
stmmac_tx_timer_arm() {
...
if (unlikely(!napi_is_scheduled(napi))) {
if (unlikely(!(hrtimer_active(&tx_q->txtimer))))
hrtimer_start(&tx_q->txtimer,
STMMAC_COAL_TIMER(tx_coal_timer),
HRTIMER_MODE_REL);
...
}
Nothing makes this check-then-start sequence atomic with respect to
napi_disable() and hrtimer_cancel(). One possible interleaving:
CPU1 (stmmac_xmit, holding txq lock)
napi_is_scheduled(napi) -> false
CPU0 (stmmac_xdp_release)
stmmac_disable_all_queues()
hrtimer_cancel(&tx_q->txtimer)
timer not armed, returns at once
hrtimer_start(&tx_q->txtimer, ...)
netif_tx_disable(dev)
only waits for CPU1 to drop txq lock
Nothing later in stmmac_xdp_release() cancels the timer. It stays queued
for tx_coal_timer, which is 5ms by default and up to 100ms via ethtool -C.
A hardirq on CPU1 widens the window. On PREEMPT_RT it can be arbitrarily
long.
stmmac_xdp_set_prog() then calls stmmac_xdp_open() right away, and that
function does:
stmmac_xdp_open() {
...
hrtimer_setup(&tx_q->txtimer, stmmac_tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
...
}
Would this re-initialize an hrtimer that is still linked in the per-CPU
timerqueue and corrupt the hrtimer base?
Before this patch, netif_tx_disable() ran first and serialized against
in-flight xmit callers. Only NAPI could arm the timer, and NAPI was
disabled before the cancel.
Would the order napi disable, then netif_tx_disable(), then the
hrtimer_cancel() loop avoid this?
> /* Free the IRQ lines */
> stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
>
[Severity: High]
The commit message says the problem is the Tx queue being woken again
after netif_tx_disable(), and this patch covers the stmmac_tx_clean()
waker. Is there still a waker in IRQ context?
netif_tx_disable() still runs before stmmac_free_irq() and
stmmac_stop_all_dma(). At that point the DMA interrupt handlers are still
installed and Tx DMA is still running.
stmmac_xdp_release() never sets STMMAC_DOWN, so the STMMAC_DOWN check in
stmmac_interrupt() and stmmac_msi_intr_tx() does not stop the handlers.
On a Tx fatal bus error (tx_hard_error), stmmac_dma_interrupt() or
stmmac_msi_intr_tx() calls stmmac_tx_err(), which ends with an
unconditional wake:
stmmac_tx_err() {
...
priv->xstats.tx_errors++;
netif_tx_wake_queue(netdev_get_tx_queue(priv->dev, chan));
}
The qdisc is still active in the XDP reconfigure path. In ndo_stop,
dev_deactivate_many() has already run, but not here. So the wake can
start stmmac_xmit() again:
stmmac_xdp_release()
netif_tx_disable(dev)
stmmac_dma_interrupt()
stmmac_tx_err()
netif_tx_wake_queue()
stmmac_free_irq()
stmmac_stop_all_dma()
free_dma_desc_resources()
stmmac_xmit()
writes into tx_q->dma_tx / dma_etx
free_irq() only waits for handlers that are already running. It does not
undo the wake, and nothing stops the queue again after it.
Could stmmac_xmit() then write descriptors into rings that
free_dma_desc_resources() has already freed?
Moving netif_tx_disable() after stmmac_free_irq() might close this path.
Another option is a teardown flag that stmmac_tx_err() and the ISRs
check. The last patch of the series ("net: stmmac: Stop Tx queue when
(en|dis)abling XSK pools") does not seem to cover this case.
[Severity: High]
This is a pre-existing issue and was not introduced by this patch. It is
on the same release/reopen path, though. stmmac_xdp_set_prog() ignores
the return value of stmmac_xdp_open():
drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c:stmmac_xdp_set_prog() {
...
if (if_running && need_update)
stmmac_xdp_open(dev);
...
}
stmmac_xdp_open() can fail in alloc_dma_desc_resources(),
init_dma_desc_rings() or stmmac_request_irq(). On failure it frees the
descriptor resources and returns before stmmac_enable_all_queues(),
netif_carrier_on() and netif_tx_start_all_queues(). Userspace is still
told the XDP update succeeded.
What happens on the next ifdown in that state? The call chain
stmmac_release()->__stmmac_release()->stmmac_disable_all_queues() calls
napi_disable() on NAPI instances that stmmac_xdp_release() has already
disabled. In napi_disable_locked():
val = READ_ONCE(n->state);
do {
while (val & (NAPIF_STATE_SCHED | NAPIF_STATE_NPSVC)) {
usleep_range(20, 200);
val = READ_ONCE(n->state);
}
Both bits are already set, so wouldn't this loop forever while holding
rtnl_lock and the netdev instance lock? Another XDP program change would
take the same path through stmmac_xdp_release().
Should stmmac_xdp_set_prog() propagate the error, and should the device
be left in a state that a later close can handle?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261022-stmmac_xsk_crashes-v4-0-15c0c750bb96%40linutronix.de
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v4 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools
2026-10-22 7:01 ` [PATCH net-next v4 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools Kurt Kanzenbach
@ 2026-10-26 7:24 ` netdev-bot+sashiko
0 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-10-26 7:24 UTC (permalink / raw)
To: kurt
Cc: maxime.chevallier, andrew+netdev, davem, kuba, pabeni, edumazet,
mcoquelin.stm32, alexandre.torgue, ast, daniel, hawk,
john.fastabend, sdf, yoong.siang.song, faizal.abdul.rahim,
boon.leong.ong, bigeasy, maciej.fijalkowski, nb, netdev,
linux-stm32, linux-arm-kernel, bpf
Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.
Critical: 0 · High: 1 (1) · Medium: 2 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] stmmac_xdp_enable_pool() (stmmac_xdp.c:59) and
stmmac_xdp_disable_pool() (stmmac_xdp.c:102) now call…
- [Medium] The patch only works if __QUEUE_STATE_DRV_XOFF stays set from
stmmac_xdp_stop_tx_queue() until the ring is rebuilt.
- [Medium] In stmmac_xdp_xmit_xdpf(), first_entry and entry are read from
tx_q->cur_tx when the locals are declared (stmmac_main.c:5263-5264).
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but if
__alloc_dma_tx_desc_resources() fails during an XSK pool switch, tx_q…
--
net: stmmac: Stop Tx queue when (en|dis)abling XSK pools
This patch stops the netdev Tx queue while an XSK pool is enabled or
disabled on a running interface. It does this because
stmmac_disable_tx_queue() and stmmac_enable_tx_queue() free and reallocate
the Tx DMA ring. The queue is woken once the ring is rebuilt, and
stmmac_xdp_xmit_xdpf() now drops XDP frames while the queue is stopped.
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 27c4a249e560..2083520179a6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5270,6 +5270,10 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
> if (unlikely(xdp_frame_has_frags(xdpf)))
> num_frames += sinfo->nr_frags;
>
> + /* Ring may be torn down for an XSK pool switch */
> + if (netif_tx_queue_stopped(netdev_get_tx_queue(priv->dev, queue)))
> + return STMMAC_XDP_CONSUMED;
> +
[Severity: Medium]
Is this check early enough? first_entry and entry have already been read
from tx_q->cur_tx at the top of stmmac_xdp_xmit_xdpf():
unsigned int first_entry = tx_q->cur_tx;
unsigned int entry = tx_q->cur_tx;
The pool switch path does not hold the Tx lock while it rebuilds the ring
or wakes the queue. stmmac_enable_tx_queue()->stmmac_reset_tx_queue() sets
cur_tx and dirty_tx to 0. After that, netif_tx_wake_queue() clears XOFF
without taking the lock.
Take an XDP sender (stmmac_xdp_xmit_back() or stmmac_xdp_xmit()) that:
1. takes __netif_tx_lock() while the queue is stopped
2. loads the old cur_tx == N
3. is delayed past the whole rebuild and wake, for example by being
preempted on PREEMPT_RT
4. then finds the queue running and passes stmmac_tx_avail()
Could that sender fill and OWN-mark descriptors at the stale index N on
the new ring and then set cur_tx = N + 1?
DMA restarts at descriptor 0, which nobody owns. That looks like it would
stall Tx. stmmac_tx_clean() would then walk entries 0..N-1 as if they had
completed.
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
> index 9af9e0cb366c..14445683a5c3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
> @@ -6,6 +6,15 @@
> #include "stmmac.h"
> #include "stmmac_xdp.h"
>
> +static void stmmac_xdp_stop_tx_queue(struct stmmac_priv *priv, u16 queue)
> +{
> + struct netdev_queue *nq = netdev_get_tx_queue(priv->dev, queue);
> +
> + __netif_tx_lock_bh(nq);
> + netif_tx_stop_queue(nq);
> + __netif_tx_unlock_bh(nq);
> +}
[Severity: Medium]
Can stmmac_tx_err() undo this stop while the pool switch is still running?
stmmac_tx_err() runs in hardirq context from stmmac_dma_interrupt() and
stmmac_msi_intr_tx() when the channel status is tx_hard_error. It takes no
lock and always finishes by waking the queue:
stmmac_tx_err() {
...
stmmac_stop_tx_dma(priv, chan);
dma_free_tx_skbufs(priv, &priv->dma_conf, chan);
stmmac_clear_tx_descriptors(priv, &priv->dma_conf, chan);
stmmac_reset_tx_queue(priv, chan);
stmmac_init_tx_chan(priv, priv->ioaddr, priv->plat->dma_cfg,
tx_q->dma_tx_phy, chan);
stmmac_start_tx_dma(priv, chan);
priv->xstats.tx_errors++;
netif_tx_wake_queue(netdev_get_tx_queue(priv->dev, chan));
}
The switch does not seem to keep it out. dwmac4_disable_dma_irq() clears
only RIE and TIE, so AIE, RPS, RBU and FBE stay enabled. There is also no
synchronize_irq().
Stopping RX DMA during the switch raises RPS, and with it AIS.
dwmac4_dma_interrupt() decodes AIS together with TPS or FBE as
tx_hard_error.
TPS is never acknowledged, because only the enabled bits get written back:
writel(intr_status & intr_en,
ioaddr + DMA_CHAN_STATUS(dwmac4_addrs, chan));
That means TPS can stay latched from an earlier Tx DMA stop.
Suppose tx_hard_error is decoded inside the stop-to-wake window. Wouldn't
stmmac_tx_err() then work on the freed or half-rebuilt ring and restart
DMA on freed coherent memory? It would also reopen the queue for
stmmac_xmit() and the XDP Tx paths.
The source alone does not show whether the hardware reports exactly
tx_hard_error in that window. For example, a latched RI bit would change
the status value.
[ ... ]
> @@ -46,6 +56,7 @@ static int stmmac_xdp_enable_pool(struct stmmac_priv *priv,
> stmmac_enable_rx_queue(priv, queue);
> stmmac_enable_tx_queue(priv, queue);
> napi_enable(&ch->rxtx_napi);
> + netif_tx_wake_queue(netdev_get_tx_queue(priv->dev, queue));
[Severity: High]
What happens here if stmmac_enable_tx_queue() failed? It returns void.
When an allocation fails, it only logs a message and returns:
stmmac_enable_tx_queue() {
...
ret = __alloc_dma_tx_desc_resources(priv, &priv->dma_conf, queue);
if (ret) {
netdev_err(priv->dev, "Failed to alloc TX desc.\n");
return;
}
...
}
The state left behind depends on which allocation in
__alloc_dma_tx_desc_resources() failed:
- tx_q->tx_skbuff_dma or tx_q->tx_skbuff is NULL, or
- dma_tx/dma_etx/dma_entx still point at the coherent ring that
__free_dma_tx_desc_resources() just freed. That function never clears
those pointers.
stmmac_reset_tx_queue() is skipped as well, so cur_tx and dirty_tx keep
their old values.
Once this unconditional wake runs, stmmac_xmit() can execute:
WARN_ON(tx_q->tx_skbuff[first_entry]);
It would then write tx_skbuff/tx_skbuff_dma entries, and descriptors via
stmmac_get_tx_desc(), into NULL or freed memory. The new
netif_tx_queue_stopped() check in stmmac_xdp_xmit_xdpf() is open again
too.
stmmac_xdp_disable_pool() gets the same unconditional wake.
The commit message says the crash comes from Tx touching the ring while it
is freed and reallocated. On this error path, that still seems reachable.
Could stmmac_enable_tx_queue() return the error, so the queue stays
stopped (or the switch is unwound) instead of being woken?
[ ... ]
> @@ -87,6 +99,7 @@ static int stmmac_xdp_disable_pool(struct stmmac_priv *priv, u16 queue)
> stmmac_enable_tx_queue(priv, queue);
> napi_enable(&ch->rx_napi);
> napi_enable(&ch->tx_napi);
> + netif_tx_wake_queue(netdev_get_tx_queue(priv->dev, queue));
> }
>
> return 0;
[Severity: High]
This is a pre-existing issue and was not introduced by this patch. After
stmmac_enable_tx_queue() fails, tx_q looks set up for a double free on the
next teardown.
__alloc_dma_tx_desc_resources() does no unwind. It returns -ENOMEM at the
first step that fails:
tx_q->tx_skbuff_dma = kzalloc_objs(*tx_q->tx_skbuff_dma,
dma_conf->dma_tx_size);
if (!tx_q->tx_skbuff_dma)
return -ENOMEM;
tx_q->tx_skbuff = kzalloc_objs(struct sk_buff *, dma_conf->dma_tx_size);
if (!tx_q->tx_skbuff)
return -ENOMEM;
...
addr = dma_alloc_coherent(priv->device, size,
&tx_q->dma_tx_phy, GFP_KERNEL);
if (!addr)
return -ENOMEM;
__free_dma_tx_desc_resources() never clears dma_tx/dma_etx/dma_entx,
tx_skbuff or tx_skbuff_dma. The next teardown therefore calls it again on
the stale pointers. That teardown can be stmmac_disable_tx_queue() during
another pool switch, or free_dma_tx_desc_resources() from stmmac_release().
Wouldn't that call dma_free_coherent() a second time on the ring that was
already freed?
If the tx_skbuff_dma allocation was the one that failed, two more things
happen:
- dma_free_tx_skbufs() dereferences the NULL tx_skbuff_dma in
stmmac_free_tx_buffer()
- kfree(tx_q->tx_skbuff) frees the old array a second time
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261022-stmmac_xsk_crashes-v4-0-15c0c750bb96%40linutronix.de
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-26 7:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-22 7:01 [PATCH net-next v4 0/2] net: stmmac: Fix XSK crashes on stm32mp2 Kurt Kanzenbach
2026-10-22 7:01 ` [PATCH net-next v4 1/2] net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release() Kurt Kanzenbach
2026-10-26 7:24 ` netdev-bot+sashiko
2026-10-22 7:01 ` [PATCH net-next v4 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools Kurt Kanzenbach
2026-10-26 7:24 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox