[PATCH net] net: stmmac: xgmac: do not treat TBU as a TX event

[PATCH net] net: stmmac: xgmac: do not treat TBU as a TX event

[Pengfei Zhang]

Commit 8d07a7930434 ("net: stmmac: xgmac: Do not enable TBU interrupt")
removed TBUE from the default DMA interrupt mask, but did not remove the
TBU check from dwxgmac2_dma_interrupt().

TBU is set whenever a TX channel runs out of descriptors. Since TBUE is
not enabled and only enabled bits are cleared, the TBU status bit is
never cleared. As a result, TBU stays set on every idle TX channel,
regardless of whether there is any TX traffic.

With a shared interrupt, stmmac_dma_interrupt() checks all channels,
so every interrupt from any source schedules TX NAPI on every idle TX
channel.

Only use TI to trigger TX processing.

Fixes: 8d07a7930434 ("net: stmmac: xgmac: Do not enable TBU interrupt")
Signed-off-by: Pengfei Zhang <zhangfeionline@gmail.com>
---
Tested on a board with 8 RX/TX DMA channels behind one shared
interrupt. A peer sends 10k UDP packets/s to the board and the board
sends nothing back, so there should be almost no TX work. Every idle
channel reads DMA_CH_STATUS 0x4 (TBU only).

Per second, averaged over 10 s:

                       before    after
  hw interrupts          9071     8580
  tx_normal_irq_n       72572        1
  tx_clean             ~73000        9

- hw interrupts: the shared interrupt line, from /proc/interrupts.
  Unchanged, the RX watchdog sets the rate.
- tx_normal_irq_n (ethtool -S): how often dwxgmac2_dma_interrupt()
  returned handle_tx. Before the fix every interrupt reports it for
  all 8 channels.
- tx_clean (ethtool -S): stmmac_tx_clean() runs, i.e. TX NAPI polls.
  Each spurious handle_tx schedules one. What is left after the fix
  is real TX and the TX coalesce timer. The "before" value is from a
  separate run with the same setup.

No packet loss before or after.

 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index ff83858ebc..0a6e82a155 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -346,7 +346,7 @@ static int dwxgmac2_dma_interrupt(struct stmmac_priv *priv,
 		u64_stats_update_end(&stats->syncp);
 		ret |= handle_rx;
 	}
-	if (likely(intr_status & (XGMAC_TI | XGMAC_TBU))) {
+	if (likely(intr_status & XGMAC_TI)) {
 		u64_stats_update_begin(&stats->syncp);
 		u64_stats_inc(&stats->tx_normal_irq_n[chan]);
 		u64_stats_update_end(&stats->syncp);
-- 
2.54.0

Re: [PATCH net] net: stmmac: xgmac: do not treat TBU as a TX event

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] net: stmmac: xgmac: do not treat TBU as a TX event

[Pengfei Zhang]

+Cc Jose Abreu, the author of the commit in Fixes:. I used an outdated
address for him in the original posting, sorry.

Re: [PATCH net] net: stmmac: xgmac: do not treat TBU as a TX event

[Pengfei Zhang]

It has been tested on the hardware device (not publicly available),
a DWC XGMAC with 8 RX/TX DMA channels behind one shared interrupt.
The numbers below the --- in the patch are from that device.

Re: [PATCH net] net: stmmac: xgmac: do not treat TBU as a TX event

[Maxime Chevallier]

Hi,

On 10/13/26 04:47, Pengfei Zhang wrote:
> Commit 8d07a7930434 ("net: stmmac: xgmac: Do not enable TBU interrupt")
> removed TBUE from the default DMA interrupt mask, but did not remove the
> TBU check from dwxgmac2_dma_interrupt().
> 
> TBU is set whenever a TX channel runs out of descriptors. Since TBUE is
> not enabled and only enabled bits are cleared, the TBU status bit is
> never cleared. As a result, TBU stays set on every idle TX channel,
> regardless of whether there is any TX traffic.
> 
> With a shared interrupt, stmmac_dma_interrupt() checks all channels,
> so every interrupt from any source schedules TX NAPI on every idle TX
> channel.
> 
> Only use TI to trigger TX processing.
> 
> Fixes: 8d07a7930434 ("net: stmmac: xgmac: Do not enable TBU interrupt")
> Signed-off-by: Pengfei Zhang <zhangfeionline@gmail.com>

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

> ---
> Tested on a board with 8 RX/TX DMA channels behind one shared
> interrupt. A peer sends 10k UDP packets/s to the board and the board
> sends nothing back, so there should be almost no TX work. Every idle
> channel reads DMA_CH_STATUS 0x4 (TBU only).
> 
> Per second, averaged over 10 s:
> 
>                        before    after
>   hw interrupts          9071     8580
>   tx_normal_irq_n       72572        1
>   tx_clean             ~73000        9
> 
> - hw interrupts: the shared interrupt line, from /proc/interrupts.
>   Unchanged, the RX watchdog sets the rate.
> - tx_normal_irq_n (ethtool -S): how often dwxgmac2_dma_interrupt()
>   returned handle_tx. Before the fix every interrupt reports it for
>   all 8 channels.
> - tx_clean (ethtool -S): stmmac_tx_clean() runs, i.e. TX NAPI polls.
>   Each spurious handle_tx schedules one. What is left after the fix
>   is real TX and the TX coalesce timer. The "before" value is from a
>   separate run with the same setup.
> 
> No packet loss before or after.

If you need to respin, that information should be in the commit log
directly.

Thanks,

Maxime

Re: [PATCH net] net: stmmac: xgmac: do not treat TBU as a TX event

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 (1) · 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).

- [Medium] Removing XGMAC_TBU from the TX condition in
  dwxgmac2_dma_interrupt() also removes an incidental TX-reclaim trigger.

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but the
  dwmac4/GMAC5 core has the same defect that this patch fixes for XGMAC.

--

net: stmmac: xgmac: do not treat TBU as a TX event

This patch removes XGMAC_TBU from the TX condition in
dwxgmac2_dma_interrupt(), so only TI schedules TX processing. TBUE is
not enabled and only enabled bits are acknowledged, so TBU was never
cleared. With a shared interrupt, that meant every interrupt scheduled
TX NAPI on every idle TX channel.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> index ff83858ebc1f3..0a6e82a155c0e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> @@ -346,7 +346,7 @@ static int dwxgmac2_dma_interrupt(struct stmmac_priv *priv,
>  		u64_stats_update_end(&stats->syncp);
>  		ret |= handle_rx;
>  	}
> -	if (likely(intr_status & (XGMAC_TI | XGMAC_TBU))) {
> +	if (likely(intr_status & XGMAC_TI)) {

[Severity: Medium]
Can the XDP_TX and ndo_xdp_xmit path stall permanently on XGMAC in
shared-IRQ mode when the TX coalesce timer is disabled?

__stmmac_set_coalesce() accepts tx-usecs=0 as long as tx-frames is
nonzero. stmmac_tx_timer_arm() then returns early:

    if (!tx_coal_timer)
        return;

stmmac_xdp_xmit_xdpf() sets IOC only on a tx_coal_frames boundary. It
has no !netdev_xmit_more() fallback like stmmac_xmit(). It also stops
taking new frames at 3/4 ring occupancy:

	if (stmmac_tx_avail(priv, queue) < STMMAC_TX_THRESH(priv))
		return STMMAC_XDP_CONSUMED;
	...
	if (tx_q->tx_count_frames % priv->tx_coal_frames[queue] == 0)
		set_ic = true;
	else
		set_ic = false;

Take a 64 entry TX ring (stmmac_set_ringparam() accepts
DMA_MIN_TX_SIZE) with tx-frames=64 and tx-usecs=0. Starting from an
empty ring, only 48 XDP_TX frames get in. None of them reaches the IOC
boundary, so no TI is raised.

Before this patch, the sticky TBU bit made every later shared
interrupt return handle_tx and run stmmac_tx_clean(). That includes
the RX interrupts that drive XDP_TX.

stmmac_napi_poll_rx() does not call stmmac_tx_clean(). After this
change, would dirty_tx never advance on that queue? If so, every
further XDP_TX or redirect frame would be dropped with
STMMAC_XDP_CONSUMED. Only skb TX on the same queue or a reconfiguration
would recover it.

More generally, with tx-usecs=0, trailing XDP frames below tx-frames
would no longer be reclaimed on the next interrupt. The same gap
already exists in XGMAC per-channel MSI mode. This patch exposes it in
shared-IRQ mode as well.

The commit message says that what remains is "real TX and the TX
coalesce timer". Does that also hold when the timer is disabled?

[Severity: Medium]
This isn't a bug introduced by this patch, but does
dwmac4_dma_interrupt() in dwmac4_lib.c have the same sticky TBU
problem?

drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c:dwmac4_dma_interrupt() {
    ...
	if (unlikely(intr_status & DMA_CHAN_STATUS_TBU))
		ret |= handle_tx;
	...
	writel(intr_status & intr_en,
	       ioaddr + DMA_CHAN_STATUS(dwmac4_addrs, chan));
    ...
}

DMA_CHAN_INTR_NORMAL and DMA_CHAN_INTR_NORMAL_4_10 in dwmac4_dma.h
only contain NIE, RIE and TIE, and no TBUE bit is defined. So TBU is
never part of intr_en. Once a TX channel goes idle, its TBU status bit
is never cleared.

In shared-IRQ mode, stmmac_dma_interrupt() calls
stmmac_napi_check(priv, chan, DMA_DIR_RXTX) for every channel, and the
status is not masked for that direction. This looks like it would
schedule TX NAPI and stmmac_tx_clean() on every idle TX channel for
every interrupt. That is the same behaviour measured for XGMAC in the
commit message.

Since commit 1975df880b959e, TBU is no longer counted in
tx_normal_irq_n on this core. That would hide the effect from the
statistic used to diagnose the XGMAC case.

Should the dwmac4 implementation behind the same dispatcher get the
same change?

>  		u64_stats_update_begin(&stats->syncp);
>  		u64_stats_inc(&stats->tx_normal_irq_n[chan]);
>  		u64_stats_update_end(&stats->syncp);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261013024747.2688984-1-zhangfeionline%40gmail.com

Re: [PATCH net] net: stmmac: xgmac: do not treat TBU as a TX event

[Pengfei Zhang]

> [Severity: Medium]
> Can the XDP_TX and ndo_xdp_xmit path stall permanently on XGMAC in
> shared-IRQ mode when the TX coalesce timer is disabled?

This is not caused by this patch. It needs a very specific setup: XDP
TX traffic, tx-usecs set to 0, and tx-frames too large to be reached
below STMMAC_TX_THRESH. With that setup the same stall can already
happen on XGMAC with per-channel MSI today, because
stmmac_xdp_xmit_xdpf() only sets IC on a tx_coal_frames boundary and
has no fallback when the TX timer is disabled.

I will look at fixing that in the XDP TX path with a separate patch,
but it is independent of this one.

> [Severity: Medium]
> This isn't a bug introduced by this patch, but does
> dwmac4_dma_interrupt() in dwmac4_lib.c have the same sticky TBU
> problem?

Yes, it looks like it does, especially since commit 6ccf12ae111e
("net: stmmac: use interrupt mode INTM=1 for multi-MSI") moved the TBU
check out of the NIS block. I only have XGMAC hardware at the moment,
so I kept this patch to XGMAC. I will send a separate patch for dwmac4
once I can test it on dwmac4 hardware.