mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net] net: bcmgenet: cap the Tx queue threshold at the ring size
@ 2026-11-06 14:10 Nicolai Buchwitz
  2026-11-07  0:14 ` Florian Fainelli
  0 siblings, 1 reply; 3+ messages in thread
From: Nicolai Buchwitz @ 2026-11-06 14:10 UTC (permalink / raw)
  To: Doug Berger, Florian Fainelli, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Justin Chen, Broadcom internal kernel review list,
	Nikolay Aleksandrov, Jason Xing, netdev, linux-kernel,
	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>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: bcmgenet: cap the Tx queue threshold at the ring size
  2026-11-06 14:10 [PATCH net] net: bcmgenet: cap the Tx queue threshold at the ring size Nicolai Buchwitz
@ 2026-11-07  0:14 ` Florian Fainelli
  2026-11-07 11:53   ` Nicolai Buchwitz
  0 siblings, 1 reply; 3+ messages in thread
From: Florian Fainelli @ 2026-11-07  0:14 UTC (permalink / raw)
  To: Nicolai Buchwitz, Doug Berger, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Justin Chen, Broadcom internal kernel review list,
	Nikolay Aleksandrov, Jason Xing, netdev, linux-kernel



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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: bcmgenet: cap the Tx queue threshold at the ring size
  2026-11-07  0:14 ` Florian Fainelli
@ 2026-11-07 11:53   ` Nicolai Buchwitz
  0 siblings, 0 replies; 3+ messages in thread
From: Nicolai Buchwitz @ 2026-11-07 11:53 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Doug Berger, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Justin Chen,
	Broadcom internal kernel review list, Nikolay Aleksandrov,
	Jason Xing, netdev, linux-kernel

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-07 11:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-06 14:10 [PATCH net] net: bcmgenet: cap the Tx queue threshold at the ring size Nicolai Buchwitz
2026-11-07  0:14 ` Florian Fainelli
2026-11-07 11:53   ` Nicolai Buchwitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®