Re: [PATCH net v2 2/4] net: macb: drop in-flight Tx SKBs on close

[Jakub Kicinski]

On Tue, 28 Apr 2026 18:32:58 +0200 Théo Lebrun wrote:
>  	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> -		kfree(queue->tx_skb);
> -		queue->tx_skb = NULL;
> +		if (queue->tx_skb) {
> +			unsigned int dropped = 0, tail;
> +
> +			for (tail = queue->tx_tail; tail != queue->tx_head;
> +			     tail++) {
> +				if (macb_tx_skb(queue, tail)->skb)
> +					dropped++;
> +				macb_tx_unmap(bp, macb_tx_skb(queue, tail), 0,
> +					      SKB_DROP_REASON_NOT_SPECIFIED);
> +			}
> +
> +			queue->stats.tx_dropped += dropped;
> +			bp->dev->stats.tx_dropped += dropped;

I'm slightly baffled by the stats in this driver.

Incrementing of both device and queue stats is highly unusual.
The driver seems to already have the values for the per-queue drops
but currently never increments it (did I miss it?) It does for Rx
stats but not for Tx stats.

As sashiko correctly points out incrementing dev stats will lead
to races and lass of increments for multi-queue devices.

Since there are no increments for tx_dropped stat today - could you
please delete it from ethtool -S, migrate the only existing
dev->stats.tx_dropped++; to increment the per-queue stat and make 
macb_get_stats() collect the tx_dropped from all queues, instead
of relying on the device-level stat?

This should be patch 2 in this series, and then subsequent patches
don't have to do this double-counting dance.

I suppose you may want to migrate the byte and packet counters
while at it, and add a u64 sync...