[PATCH net] net: bcmgenet: cap the Tx queue threshold at the ring size

[PATCH net] net: bcmgenet: cap the Tx queue threshold at the ring size

[Nicolai Buchwitz]

A Tx queue stops when MAX_SKB_FRAGS + 1 or fewer descriptors are free and
wakes once there are more. Queues 1 to 4 hold 32 descriptors, so with
CONFIG_MAX_SKB_FRAGS at 31 they would need 33 to wake. As a result the
first packet stops the queue and it stays stopped:

  eth0: NETDEV WATCHDOG: CPU: 0: transmit queue 3 timed out 2004 ms
  eth0: NETDEV WATCHDOG: CPU: 1: transmit queue 4 timed out 2004 ms

On a Raspberry Pi CM4 only queue 0 carried traffic afterwards. The other
four counted an error on nearly every packet.

Fix this and cap the threshold at what the ring can free.

Fixes: 3948b05950fd ("net: introduce a config option to tweak MAX_SKB_FRAGS")
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
Found this while fixing the clashiko findings on v6 of my MTU series. After
those fixes this bug would be easier to hit. At the moment it needs a
larger CONFIG_MAX_SKB_FRAGS than the default.

Everything was tested on a Raspberry Pi CM4 (BCM2711, genet v5).
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 6 ++++--
 drivers/net/ethernet/broadcom/genet/bcmgenet.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 4d2be2a21ed1b..186d17781b890 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2030,7 +2030,7 @@ static int bcmgenet_tx_poll(struct napi_struct *napi, int budget)
 
 	spin_lock(&ring->lock);
 	__bcmgenet_tx_reclaim(ring->priv->dev, ring);
-	if (ring->free_bds > (MAX_SKB_FRAGS + 1)) {
+	if (ring->free_bds > ring->stop_thld) {
 		txq = netdev_get_tx_queue(ring->priv->dev, ring->index);
 		netif_tx_wake_queue(txq);
 	}
@@ -2235,7 +2235,7 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	netdev_tx_sent_queue(txq, GENET_CB(skb)->bytes_sent);
 
-	if (ring->free_bds <= (MAX_SKB_FRAGS + 1))
+	if (ring->free_bds <= ring->stop_thld)
 		netif_tx_stop_queue(txq);
 
 	if (!netdev_xmit_more() || netif_xmit_stopped(txq))
@@ -2742,6 +2742,8 @@ static void bcmgenet_init_tx_ring(struct bcmgenet_priv *priv,
 	ring->index = index;
 	ring->cbs = priv->tx_cbs + start_ptr;
 	ring->size = size;
+	/* A ring smaller than that would never wake */
+	ring->stop_thld = min_t(unsigned int, MAX_SKB_FRAGS + 1, size - 1);
 	ring->clean_ptr = start_ptr;
 	ring->c_index = 0;
 	ring->free_bds = size;
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index e48ce39d3e7c8..6ab3f73475550 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -546,6 +546,7 @@ struct bcmgenet_tx_ring {
 	unsigned int	index;		/* ring index */
 	struct enet_cb	*cbs;		/* tx ring buffer control block*/
 	unsigned int	size;		/* size of each tx ring */
+	unsigned int	stop_thld;	/* queue stops at this many free bds or fewer */
 	unsigned int    clean_ptr;      /* Tx ring clean pointer */
 	unsigned int	c_index;	/* last consumer index of each ring*/
 	unsigned int	free_bds;	/* # of free bds for each ring */

---
base-commit: 19a227903e65c7a9c179156485660a4e017575fe
change-id: 20261106-nb-genet-txq-thld-net-e0864f235d22

Best regards,
--  
Nicolai Buchwitz <nb@tipi-net.de>

Re: [PATCH net] net: bcmgenet: cap the Tx queue threshold at the ring size

[Florian Fainelli]

On 11/6/2026 6:10 AM, Nicolai Buchwitz wrote:
> A Tx queue stops when MAX_SKB_FRAGS + 1 or fewer descriptors are free and
> wakes once there are more. Queues 1 to 4 hold 32 descriptors, so with
> CONFIG_MAX_SKB_FRAGS at 31 they would need 33 to wake. As a result the
> first packet stops the queue and it stays stopped:
> 
>    eth0: NETDEV WATCHDOG: CPU: 0: transmit queue 3 timed out 2004 ms
>    eth0: NETDEV WATCHDOG: CPU: 1: transmit queue 4 timed out 2004 ms
> 
> On a Raspberry Pi CM4 only queue 0 carried traffic afterwards. The other
> four counted an error on nearly every packet.
> 
> Fix this and cap the threshold at what the ring can free.
> 
> Fixes: 3948b05950fd ("net: introduce a config option to tweak MAX_SKB_FRAGS")
> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>

Eventually we will have to decide what to do with those other queues :D
-- 
Florian

Re: [PATCH net] net: bcmgenet: cap the Tx queue threshold at the ring size

[Nicolai Buchwitz]

Hi Florian

On 7.11.2026 01:14, Florian Fainelli wrote:

> [...]

> Eventually we will have to decide what to do with those other queues :D

Probably sooner than later. Sashiko claims that this patch does not 
fully
fix hte CONFIG_MAX_SKB_FRAGS=31 case. The check in bcmgenet_xmit()

         if (ring->free_bds <= (nr_frags + 1))

wants 33 free descriptors for a 31 frag skb and those rings hold 32. 
Arguably
the bug is way older than this patch and you need a non-default config 
to hit.

So IMHO the options are:
Either linearize in bcmgenet_xmit() when the frame does not fit, or we 
drop
the queues as I proposed in June [1]?

With XDP in mind the second one seems more attractive, as it needs its 
own 64
descriptor ring. With the current split the main queue would end up with 
64
descriptors, against 192 if the queues are collapsed.

[1] 
https://lore.kernel.org/netdev/20260612205915.3156127-1-nb@tipi-net.de/

Regards
Nicolai

Re: [PATCH net] net: bcmgenet: cap the Tx queue threshold at the ring size

[netdev-bot+sashiko]

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

Critical: 0 · High: 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] This patch only partly fixes the Tx stall it targets.

--

net: bcmgenet: cap the Tx queue threshold at the ring size

This patch adds a per-ring stop_thld field, capped at size - 1. The field
replaces MAX_SKB_FRAGS + 1 as the threshold for stopping and waking Tx
queues in bcmgenet_xmit() and bcmgenet_tx_poll(). The goal is to stop the
32-descriptor priority rings from staying stopped forever when
CONFIG_MAX_SKB_FRAGS is 31 or larger.

> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 4d2be2a21ed1b..186d17781b890 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c

[ ... ]

> @@ -2742,6 +2742,8 @@ static void bcmgenet_init_tx_ring(struct bcmgenet_priv *priv,
>  	ring->index = index;
>  	ring->cbs = priv->tx_cbs + start_ptr;
>  	ring->size = size;
> +	/* A ring smaller than that would never wake */
> +	ring->stop_thld = min_t(unsigned int, MAX_SKB_FRAGS + 1, size - 1);

[Severity: High]

Does this fully fix the stall on queues 1 to 4 when CONFIG_MAX_SKB_FRAGS
is 31 or more? The wake threshold is now capped, but the per-skb admission
check at the top of bcmgenet_xmit() has not changed:

bcmgenet_xmit() {
    ...
	if (ring->free_bds <= (nr_frags + 1)) {
		if (!netif_tx_queue_stopped(txq))
			netif_tx_stop_queue(txq);
		ret = NETDEV_TX_BUSY;
		goto out;
	}
    ...
}

free_bds can never be larger than ring->size (32). So an skb with
nr_frags >= 31 gets rejected even when the ring is completely empty.

In that case nothing is in flight, so no Tx-done interrupt arrives and
bcmgenet_tx_poll() never wakes the queue. The watchdog is the only wake
source left:

dev_watchdog()
  bcmgenet_timeout()
    bcmgenet_tx_reclaim(dev, ring, true);
    BCMGENET_STATS64_INC((&ring->stats64), errors);
    netif_tx_wake_queue(txq);

The qdisc then retries the same requeued skb and gets NETDEV_TX_BUSY
again, so the queue stops again.

Wouldn't that leave the queue, and every flow hashed to it, stuck for
good? The "NETDEV WATCHDOG: transmit queue N timed out" message and the
error count from the commit message would then come back every
watchdog_timeo.

The core does not keep such skbs away from the driver. The driver
advertises NETIF_F_SG, and it has no ndo_features_check and no
skb_linearize() fallback. Highly fragmented skbs can come from
MSG_ZEROCOPY, from splice, or from forwarded traffic.

Should bcmgenet_xmit() also handle skbs where nr_frags + 1 >= ring->size?
It could linearize or drop them, or clear NETIF_F_SG for them in
ndo_features_check.

>  	ring->clean_ptr = start_ptr;
>  	ring->c_index = 0;
>  	ring->free_bds = size;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106-nb-genet-txq-thld-net-v1-1-53bbac647b71%40tipi-net.de

Re: [PATCH net] net: bcmgenet: cap the Tx queue threshold at the ring size

[Nicolai Buchwitz]

On Mon, 2026-11-09 at 17:12 +0000, netdev-bot+sashiko@kernel.org wrote:
>  [...]


> [Severity: High]
> 
> Does this fully fix the stall on queues 1 to 4 when
> CONFIG_MAX_SKB_FRAGS
> is 31 or more? The wake threshold is now capped, but the per-skb
> admission
> check at the top of bcmgenet_xmit() has not changed:
> 
> bcmgenet_xmit() {
>     ...
> 	if (ring->free_bds <= (nr_frags + 1)) {
> 		if (!netif_tx_queue_stopped(txq))
> 			netif_tx_stop_queue(txq);
> 		ret = NETDEV_TX_BUSY;
> 		goto out;
> 	}
>     ...
> }
> 
> free_bds can never be larger than ring->size (32). So an skb with
> nr_frags >= 31 gets rejected even when the ring is completely empty.
> 
> In that case nothing is in flight, so no Tx-done interrupt arrives
> and
> bcmgenet_tx_poll() never wakes the queue. The watchdog is the only
> wake
> source left:
> 
> dev_watchdog()
>   bcmgenet_timeout()
>     bcmgenet_tx_reclaim(dev, ring, true);
>     BCMGENET_STATS64_INC((&ring->stats64), errors);
>     netif_tx_wake_queue(txq);
> 
> The qdisc then retries the same requeued skb and gets NETDEV_TX_BUSY
> again, so the queue stops again.
> 
> Wouldn't that leave the queue, and every flow hashed to it, stuck for
> good? The "NETDEV WATCHDOG: transmit queue N timed out" message and
> the
> error count from the commit message would then come back every
> watchdog_timeo.
> 
> The core does not keep such skbs away from the driver. The driver
> advertises NETIF_F_SG, and it has no ndo_features_check and no
> skb_linearize() fallback. Highly fragmented skbs can come from
> MSG_ZEROCOPY, from splice, or from forwarded traffic.
> 
> Should bcmgenet_xmit() also handle skbs where nr_frags + 1 >= ring-
> >size?
> It could linearize or drop them, or clear NETIF_F_SG for them in
> ndo_features_check.
> 
> >  	ring->clean_ptr = start_ptr;
> >  	ring->c_index = 0;
> >  	ring->free_bds = size;
> 
> [ ... ]

Another angle, also pre-existing, but lets fix this properly with
another patch in v2. Long term goal should be to get rid of the
undersized queues (see my response to Florian)

--
pw-bot: cr

-- 
Nicolai Buchwitz <nb@tipi-net.de>