[PATCH net-next] net: bcmgenet: complete Tx NAPI after one reclaim pass

[PATCH net-next] net: bcmgenet: complete Tx NAPI after one reclaim pass

[Nicolai Buchwitz]

After reclaiming, the Tx poll asks to be polled again. That extra poll
finds nothing to do and does not reduce the interrupt rate.

Complete the NAPI and unmask the ring interrupt right after the reclaim.

On a CM4 this raises 60 byte pktgen Tx from about 187k to 224k pps and
saves about 5% of one core at TCP line rate.

Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 21668e41b696..f17ad20b4b9c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2020,25 +2020,20 @@ static int bcmgenet_tx_poll(struct napi_struct *napi, int budget)
 {
 	struct bcmgenet_tx_ring *ring =
 		container_of(napi, struct bcmgenet_tx_ring, napi);
-	unsigned int work_done = 0;
 	struct netdev_queue *txq;
 
 	spin_lock(&ring->lock);
-	work_done = __bcmgenet_tx_reclaim(ring->priv->dev, ring);
+	__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);
 		netif_tx_wake_queue(txq);
 	}
 	spin_unlock(&ring->lock);
 
-	if (work_done == 0) {
-		napi_complete(napi);
+	if (budget && napi_complete_done(napi, 0))
 		bcmgenet_tx_ring_int_enable(ring);
 
-		return 0;
-	}
-
-	return budget;
+	return 0;
 }
 
 static void bcmgenet_tx_reclaim_all(struct net_device *dev)
-- 
2.53.0

Re: [PATCH net-next] net: bcmgenet: complete Tx NAPI after one reclaim pass

[Eric Dumazet]

On Mon, Sep 28, 2026 at 2:15 PM Nicolai Buchwitz <nb@tipi-net.de> wrote:
>
> After reclaiming, the Tx poll asks to be polled again. That extra poll
> finds nothing to do and does not reduce the interrupt rate.
>
> Complete the NAPI and unmask the ring interrupt right after the reclaim.
>
> On a CM4 this raises 60 byte pktgen Tx from about 187k to 224k pps and
> saves about 5% of one core at TCP line rate.

Reviewed-by: Eric Dumazet <edumazet@google.com>

As a follow-up, you can get rid of ring->lock in the TX
fastpath altogether (it looks like a leftover from before TX reclaim
was moved to NAPI in commit 4092e6acf5cb ("net: bcmgenet: use NAPI for
Tx completion")).

bcmgenet_xmit() is already serialized by the netdev queue lock, and
bcmgenet_tx_poll() by NAPI. To make them lockless with respect to each
other:

1. Drop ring->free_bds and compute available space from
   (READ_ONCE(ring->prod_index) - READ_ONCE(ring->c_index)) & DMA_C_INDEX_MASK.
2. Use the lockless queue stop/wake helpers from <net/pkt_sched.h>
   (netif_txq_maybe_stop() and __netif_txq_completed_wake()).
3. Ensure proper memory barriers between populating tx_cb_ptr /
   writing TDMA_PROD_INDEX in bcmgenet_xmit() and reading
   TDMA_CONS_INDEX / freeing tx_cbs in __bcmgenet_tx_reclaim() (since
   bcmgenet uses readl_relaxed/writel_relaxed).
4. Avoid sharing ring->stats64.syncp between bcmgenet_add_tsb()
   (tx_dropped) and __bcmgenet_tx_reclaim(), and serialize
   bcmgenet_timeout() with napi_disable() + __netif_tx_lock_bh().

Thanks.

Re: [PATCH net-next] net: bcmgenet: complete Tx NAPI after one reclaim pass

[Eric Dumazet]

On Mon, Sep 28, 2026 at 3:14 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Mon, Sep 28, 2026 at 2:15 PM Nicolai Buchwitz <nb@tipi-net.de> wrote:
> >
> > After reclaiming, the Tx poll asks to be polled again. That extra poll
> > finds nothing to do and does not reduce the interrupt rate.
> >
> > Complete the NAPI and unmask the ring interrupt right after the reclaim.
> >
> > On a CM4 this raises 60 byte pktgen Tx from about 187k to 224k pps and
> > saves about 5% of one core at TCP line rate.
>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
>
> As a follow-up, you can get rid of ring->lock in the TX
> fastpath altogether (it looks like a leftover from before TX reclaim
> was moved to NAPI in commit 4092e6acf5cb ("net: bcmgenet: use NAPI for
> Tx completion")).

The commit sha1 is correct, but its true title was:
("net: bcmgenet: fix throughtput regression")).

(Even if this was a NAPI adoption for TX completions)

Re: [PATCH net-next] net: bcmgenet: complete Tx NAPI after one reclaim pass

[Nicolai Buchwitz]

On 28.9.2026 15:14, Eric Dumazet wrote:

> [...]

> Reviewed-by: Eric Dumazet <edumazet@google.com>
> 
> As a follow-up, you can get rid of ring->lock in the TX
> fastpath altogether (it looks like a leftover from before TX reclaim
> was moved to NAPI in commit 4092e6acf5cb ("net: bcmgenet: use NAPI for
> Tx completion")).

Thanks Eric, makes sense. I'll sent a follow up along with some some 
other findings.

> bcmgenet_xmit() is already serialized by the netdev queue lock, and
> bcmgenet_tx_poll() by NAPI. To make them lockless with respect to each
> other:
> 
> 1. Drop ring->free_bds and compute available space from
>    (READ_ONCE(ring->prod_index) - READ_ONCE(ring->c_index)) & 
> DMA_C_INDEX_MASK.
> 2. Use the lockless queue stop/wake helpers from <net/pkt_sched.h>
>    (netif_txq_maybe_stop() and __netif_txq_completed_wake()).
> 3. Ensure proper memory barriers between populating tx_cb_ptr /
>    writing TDMA_PROD_INDEX in bcmgenet_xmit() and reading
>    TDMA_CONS_INDEX / freeing tx_cbs in __bcmgenet_tx_reclaim() (since
>    bcmgenet uses readl_relaxed/writel_relaxed).
> 4. Avoid sharing ring->stats64.syncp between bcmgenet_add_tsb()
>    (tx_dropped) and __bcmgenet_tx_reclaim(), and serialize
>    bcmgenet_timeout() with napi_disable() + __netif_tx_lock_bh().

The timeout path needs rework anyway. It currently resets the ring
while TDMA is still running, which it definitely shouldn't.

> [...]

Regards,
Nicolai