Re: [PATCH net v2 4/4] net: bcmgenet: relax the xmit ring full case

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

On 1.4.2026 02:38, Justin Chen wrote:
> We have a queue size of 32. If a packet is multiple fragments we can
> run through this queue really quickly. Currently we stop the xmit
> queue at SKB_FRAG_SIZE which by default can take up half the queue.
> Instead lets just look at the incoming packet and freeze the queue
> if the incoming packet has more fragments than free_bds. This will
> relieve some of the queue timeouts we have been seeing.
> 
> Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
> Signed-off-by: Justin Chen <justin.chen@broadcom.com>
> ---
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
> b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 54f71b1e85fc..a1aa1278842e 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -2018,10 +2018,10 @@ static int bcmgenet_tx_poll(struct napi_struct 
> *napi, int budget)
> 
>  	spin_lock(&ring->lock);
>  	work_done = __bcmgenet_tx_reclaim(ring->priv->dev, ring);
> -	if (ring->free_bds > (MAX_SKB_FRAGS + 1)) {
> -		txq = netdev_get_tx_queue(ring->priv->dev, ring->index);
> +	txq = netdev_get_tx_queue(ring->priv->dev, ring->index);
> +	if (netif_tx_queue_stopped(txq))
>  		netif_tx_wake_queue(txq);
> -	}
> +
>  	spin_unlock(&ring->lock);
> 
>  	if (work_done == 0) {
> @@ -2224,9 +2224,6 @@ 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))
> -		netif_tx_stop_queue(txq);
> -

With this removed, the only queue stop is the NETDEV_TX_BUSY path at
entry. That means every ring-full hits BUSY and requeues rather than
stopping proactively on the previous transmit.

Would it work to keep the proactive stop but base it on the actual
packet size instead of MAX_SKB_FRAGS?

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

That avoids reserving half the ring for the worst case while still
preventing BUSY from becoming the normal path.

>  	if (!netdev_xmit_more() || netif_xmit_stopped(txq))
>  		/* Packets are ready, update producer index */
>  		bcmgenet_tdma_ring_writel(priv, ring->index,

This wakes unconditionally even if only 1 BD was reclaimed. Combined
with the above, it can create a tight stop/wake/BUSY thrash when the
ring is near-full.

Thanks

Nicolai