[PATCH net] net: stmmac: raise TX completion interrupt at the end of an xmit burst

[PATCH net] net: stmmac: raise TX completion interrupt at the end of an xmit burst

[Johan Alvarado]

The TX mitigation logic only sets the Interrupt on Completion bit once
every tx_coal_frames descriptors (STMMAC_TX_FRAMES = 25), with the
tx_coal_timer hrtimer (STMMAC_COAL_TX_TIMER = 5000 us) as the only
fallback. TX skbs are freed exclusively from the TX completion path,
so any flow that keeps fewer than 25 frames in flight has all of its
skbs held for up to 5 ms after transmission.

Paced flows never queue enough frames to reach the frame threshold:
TCP Small Queues caps the amount of unfreed data at roughly two pacing
intervals worth, which at moderate pacing rates is only a couple of
packets. Every small burst then stalls until the coalesce timer fires,
and throughput collapses to approximately tsq_limit / tx_coal_timer
regardless of link capacity.

This is easily reproducible with BBR, which paces its output and thus
keeps only a few frames in flight at a time. On a YT6801
(dwmac-motorcomm) equipped Orange Pi 5 Pro, a BBR upload over a ~23 ms
RTT path is capped at 5.24 Mbit/s, while CUBIC reaches 207 Mbit/s on
the same path. BBR measures the stalled send rate as the path
bandwidth and locks its estimate near the floor, so the connection
never recovers. Lowering the coalesce settings with ethtool -C
(tx-usecs 100 tx-frames 1) lifts the same transfer to 447 Mbit/s,
confirming the mechanism.

Fix this by setting the IC bit on the last descriptor of every xmit
burst, i.e. whenever netdev_xmit_more() reports that no further frames
are pending in the current dequeue batch. Frame-based coalescing still
applies within a burst, bulk traffic keeps batching through qdisc bulk
dequeue and NAPI polling, and the coalesce timer becomes a pure
fallback instead of the primary completion mechanism for lightly
queued flows.

tx-frames 0 keeps its meaning of timer-based mitigation only.

Fixes: da2024510031 ("net: stmmac: Tune-up default coalesce settings")
Signed-off-by: Johan Alvarado <contact@c127.dev>
---
Notes for reviewers (not for the changelog):

Tested on an Orange Pi 5 Pro (RK3588, Motorcomm YT6801 PCIe GbE via
dwmac-motorcomm), iperf3 upload to a public server over a ~23 ms RTT
path, coalesce settings left at their shipped values (tx-usecs 5000,
tx-frames 25):

  before, BBR:    5.24 Mbit/s (cwnd pinned, bw estimate ~6 Mbit/s)
  before, CUBIC:  207 Mbit/s
  after,  BBR:    447 Mbit/s

Interrupt overhead stays sane: ~3.3k NIC IRQs/s total at 447 Mbit/s
(~38 kpps), i.e. roughly 12 packets per interrupt, since qdisc bulk
dequeue plus NAPI polling still coalesce within bursts.

The 5000 us STMMAC_COAL_TX_TIMER value postdates the tagged commit
(it was 1000 us back then); the stall mechanism is the same, only the
throughput ceiling differs, hence the Fixes tag on the frame-count
change.

The XSK/XDP TX paths keep their frame-count-only IC logic: there is
no skb/TSQ backpressure on those paths, and netdev_xmit_more() is not
meaningful outside ndo_start_xmit.

The same completion starvation was reported by Pavel Machek in 2016
(UDP burst pauses, back then a 40 ms low-res timer):
https://lore.kernel.org/netdev/20161123105125.GA26394@amd/
His patch disabling TX coalescing entirely was rejected in favour of
"a real solution":
https://lore.kernel.org/netdev/20161205122711.GA30774@amd/
The subsequent hrtimer conversion fixed the timer resolution but kept
the timer as the only completion mechanism for lightly queued flows;
this patch adds the missing burst-end interrupt.

 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 2a0d7eff88d3..ddf4ac03538d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4626,6 +4626,8 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
 		set_ic = true;
 	else if (!priv->tx_coal_frames[queue])
 		set_ic = false;
+	else if (!netdev_xmit_more())
+		set_ic = true;
 	else if (tx_packets > priv->tx_coal_frames[queue])
 		set_ic = true;
 	else if ((tx_q->tx_count_frames %
@@ -4910,6 +4912,8 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 		set_ic = true;
 	else if (!priv->tx_coal_frames[queue])
 		set_ic = false;
+	else if (!netdev_xmit_more())
+		set_ic = true;
 	else if (tx_packets > priv->tx_coal_frames[queue])
 		set_ic = true;
 	else if ((tx_q->tx_count_frames %
-- 
2.55.0

Re: [PATCH net] net: stmmac: raise TX completion interrupt at the end of an xmit burst

[Maciej Fijalkowski]

On Mon, Jul 06, 2026 at 05:32:45AM +0000, Johan Alvarado wrote:
> The TX mitigation logic only sets the Interrupt on Completion bit once
> every tx_coal_frames descriptors (STMMAC_TX_FRAMES = 25), with the
> tx_coal_timer hrtimer (STMMAC_COAL_TX_TIMER = 5000 us) as the only
> fallback. TX skbs are freed exclusively from the TX completion path,
> so any flow that keeps fewer than 25 frames in flight has all of its
> skbs held for up to 5 ms after transmission.
> 
> Paced flows never queue enough frames to reach the frame threshold:
> TCP Small Queues caps the amount of unfreed data at roughly two pacing
> intervals worth, which at moderate pacing rates is only a couple of
> packets. Every small burst then stalls until the coalesce timer fires,
> and throughput collapses to approximately tsq_limit / tx_coal_timer
> regardless of link capacity.
> 
> This is easily reproducible with BBR, which paces its output and thus
> keeps only a few frames in flight at a time. On a YT6801
> (dwmac-motorcomm) equipped Orange Pi 5 Pro, a BBR upload over a ~23 ms
> RTT path is capped at 5.24 Mbit/s, while CUBIC reaches 207 Mbit/s on
> the same path. BBR measures the stalled send rate as the path
> bandwidth and locks its estimate near the floor, so the connection
> never recovers. Lowering the coalesce settings with ethtool -C
> (tx-usecs 100 tx-frames 1) lifts the same transfer to 447 Mbit/s,
> confirming the mechanism.
> 
> Fix this by setting the IC bit on the last descriptor of every xmit
> burst, i.e. whenever netdev_xmit_more() reports that no further frames
> are pending in the current dequeue batch. Frame-based coalescing still
> applies within a burst, bulk traffic keeps batching through qdisc bulk
> dequeue and NAPI polling, and the coalesce timer becomes a pure
> fallback instead of the primary completion mechanism for lightly
> queued flows.
> 
> tx-frames 0 keeps its meaning of timer-based mitigation only.
> 
> Fixes: da2024510031 ("net: stmmac: Tune-up default coalesce settings")
> Signed-off-by: Johan Alvarado <contact@c127.dev>
> ---
> Notes for reviewers (not for the changelog):
> 
> Tested on an Orange Pi 5 Pro (RK3588, Motorcomm YT6801 PCIe GbE via
> dwmac-motorcomm), iperf3 upload to a public server over a ~23 ms RTT
> path, coalesce settings left at their shipped values (tx-usecs 5000,
> tx-frames 25):
> 
>   before, BBR:    5.24 Mbit/s (cwnd pinned, bw estimate ~6 Mbit/s)
>   before, CUBIC:  207 Mbit/s
>   after,  BBR:    447 Mbit/s
> 
> Interrupt overhead stays sane: ~3.3k NIC IRQs/s total at 447 Mbit/s
> (~38 kpps), i.e. roughly 12 packets per interrupt, since qdisc bulk
> dequeue plus NAPI polling still coalesce within bursts.
> 
> The 5000 us STMMAC_COAL_TX_TIMER value postdates the tagged commit
> (it was 1000 us back then); the stall mechanism is the same, only the
> throughput ceiling differs, hence the Fixes tag on the frame-count
> change.
> 
> The XSK/XDP TX paths keep their frame-count-only IC logic: there is
> no skb/TSQ backpressure on those paths, and netdev_xmit_more() is not
> meaningful outside ndo_start_xmit.
> 
> The same completion starvation was reported by Pavel Machek in 2016
> (UDP burst pauses, back then a 40 ms low-res timer):
> https://lore.kernel.org/netdev/20161123105125.GA26394@amd/
> His patch disabling TX coalescing entirely was rejected in favour of
> "a real solution":
> https://lore.kernel.org/netdev/20161205122711.GA30774@amd/

Very messy thread. To reiterate - does tx coalescing do any good in this
driver?

I did some digging and seems there was a rework of coalescing in 2018 and
then some more polishing happened in 2023 (net: stmmac: improve TX timer
arm logic).

> The subsequent hrtimer conversion fixed the timer resolution but kept
> the timer as the only completion mechanism for lightly queued flows;

Wouldn't your change imply that tx coalescing could be dropped altogether?
I do agree that each single batch of tx descs should be signalled with ic
bit at the end.


> this patch adds the missing burst-end interrupt.
> 
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 2a0d7eff88d3..ddf4ac03538d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4626,6 +4626,8 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
>  		set_ic = true;
>  	else if (!priv->tx_coal_frames[queue])
>  		set_ic = false;
> +	else if (!netdev_xmit_more())
> +		set_ic = true;
>  	else if (tx_packets > priv->tx_coal_frames[queue])
>  		set_ic = true;
>  	else if ((tx_q->tx_count_frames %
> @@ -4910,6 +4912,8 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>  		set_ic = true;
>  	else if (!priv->tx_coal_frames[queue])
>  		set_ic = false;
> +	else if (!netdev_xmit_more())
> +		set_ic = true;
>  	else if (tx_packets > priv->tx_coal_frames[queue])
>  		set_ic = true;
>  	else if ((tx_q->tx_count_frames %
> -- 
> 2.55.0
> 
> 

Re: [PATCH net] net: stmmac: raise TX completion interrupt at the end of an xmit burst

[Johan Alvarado]

On Mon, Jul 06, 2026 at 01:04:44PM +0200, Maciej Fijalkowski wrote:
> Very messy thread. To reiterate - does tx coalescing do any good in this
> driver?

It does, but today only for traffic that queues deeply enough. Within an
xmit_more batch the frame counter reduces the IC density to one descriptor
per tx_coal_frames, and that keeps working with this patch applied: in the
447 Mbit/s test from the changelog the NIC raised ~3.3k IRQs/s for ~40 kpps
on the wire, i.e. roughly 12 packets per interrupt, because qdisc bulk
dequeue, TSO and the intra-batch frame counter still coalesce. What was
broken is only the case where a flow never accumulates tx_coal_frames
in-flight frames; there the timer was the sole completion mechanism.

> I did some digging and seems there was a rework of coalescing in 2018 and
> then some more polishing happened in 2023 (net: stmmac: improve TX timer
> arm logic).

The 2023 change only avoids re-arming the timer while NAPI is already
scheduled, which helps the loaded case. For a lightly queued flow NAPI is
idle, so the timer still gets armed and the completion still waits out the
full tx_coal_timer. That is the path this patch addresses.

> Wouldn't your change imply that tx coalescing could be dropped altogether?
> I do agree that each single batch of tx descs should be signalled with ic
> bit at the end.

I don't think it can be dropped, for one correctness reason and two
practical ones.

The correctness one: the frame counter enforces an upper bound of
tx_coal_frames descriptors on the distance between IC bits in the TX ring.
If coalescing were dropped and the IC bit only set on batch tails, that
distance becomes unbounded, and a batch larger than the ring can fill it
without a single IC descriptor: every frame queued had xmit_more set, and
the tail frame that would have carried the IC bit is never queued because
stmmac_xmit stops the queue first. The hardware drains the entire ring
without raising a completion interrupt, and only the coalesce timer can
restart the queue - reintroducing the same timer-bound stall this patch
removes, now on the bulk path and once per ring drain. With the bound in
place a full ring always contains at least dma_tx_size / tx_coal_frames
completion points; even the minimum 64-descriptor ring (DMA_MIN_TX_SIZE)
still holds two.

The practical ones: the frame counter is what keeps IC density low inside
large batches (one IC per 25 descriptors rather than a single one at the
tail with nothing in between), and tx-frames / tx-usecs are user-visible
ethtool knobs on this driver that users can rely on.

Dropping the frame/timer machinery would also be a rework rather than a
fix. If there is appetite for that I would rather keep this patch minimal
for net and discuss a rework separately for net-next.

Best regards,
Johan