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@xxxxxxxxxx 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@xxxxxxxxxxx>