Re: [PATCH net-next v5 6/8] net: bcmgenet: pad transmit frames out of the packet ready window

[Nicolai Buchwitz]

On 14.10.2026 16:22, netdev-bot+sashiko@xxxxxxxxxx wrote:

> [...]

> [Severity: Medium]
> This is a pre-existing issue and was not introduced by this patch. The 
> new
> dropped increments here run under ring->lock, but bcmgenet_timeout()
> updates the same syncp after the lock has been dropped:
>
> drivers/net/ethernet/broadcom/genet/bcmgenet.c:bcmgenet_timeout() {
>     ...
> 	bcmgenet_tx_reclaim(dev, ring, true);
>
> 	/* Re-enable the TX interrupt for this ring */
> 	bcmgenet_intrl2_1_writel(priv, 1 << txqueue, INTRL2_CPU_MASK_CLEAR);
>
> 	txq_trans_cond_update(txq);
>
> 	BCMGENET_STATS64_INC((&ring->stats64), errors);
>     ...
> }
>
> bcmgenet_tx_reclaim() takes ring->lock and releases it before 
> returning,
> and the TX interrupt has just been unmasked. TX NAPI writes the same 
> syncp
> under ring->lock:
>
> bcmgenet_tx_poll()
>     spin_lock(&ring->lock);
>     __bcmgenet_tx_reclaim()
>         u64_stats_update_begin(&stats->syncp);
>         u64_stats_add(&stats->packets, pkts_compl);
>         u64_stats_add(&stats->bytes, bytes_compl);
>         u64_stats_update_end(&stats->syncp);
>
> dev_watchdog() freezes the tx queues, but it doesn't stop TX NAPI.
>
> On 32-bit SMP kernels, such as on BCM2711 or arm32 BCM7xxx, can these 
> two
> writers overlap on ring->stats64.syncp? Readers in 
> bcmgenet_get_stats64()
> and ethtool -S could then see torn counters. A lost sequence increment
> would also leave the seqcount odd, and readers would spin.
>
> Would moving the errors increment in bcmgenet_timeout() under 
> ring->lock
> avoid this?

Yes. It is pre-existing though, bcmgenet_timeout() increments errors 
after
bcmgenet_tx_reclaim() has dropped ring->lock and the Tx interrupt is
unmasked again by then. I can fix it in net.

> [...]