Re: [Intel-wired-lan] [PATCH iwl-next 1/2] i40e: implement basic per-queue stats

[Paolo Abeni <pabeni@redhat.com>]

On 4/8/26 2:07 PM, Loktionov, Aleksandr wrote:
>> -----Original Message-----
>> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
>> Of Paolo Abeni
>> Sent: Wednesday, April 8, 2026 1:44 PM
>> To: intel-wired-lan@lists.osuosl.org
>> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
>> Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
>> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
>> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
>> Alexei Starovoitov <ast@kernel.org>; Daniel Borkmann
>> <daniel@iogearbox.net>; Jesper Dangaard Brouer <hawk@kernel.org>; John
>> Fastabend <john.fastabend@gmail.com>; Stanislav Fomichev
>> <sdf@fomichev.me>; netdev@vger.kernel.org
>> Subject: [Intel-wired-lan] [PATCH iwl-next 1/2] i40e: implement basic
>> per-queue stats
>>
>> Only expose the counters currently available (bytes, packets); add
>> account for base stats to deal with ring clear.
>>
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>> ---
>>  drivers/net/ethernet/intel/i40e/i40e.h      |   7 ++
>>  drivers/net/ethernet/intel/i40e/i40e_main.c | 133
>> ++++++++++++++++++++
>>  2 files changed, 140 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h
>> b/drivers/net/ethernet/intel/i40e/i40e.h
>> index dcb50c2e1aa2..fe642c464e9c 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>> @@ -836,16 +836,23 @@ struct i40e_vsi {
>>  	struct i40e_eth_stats eth_stats;
>>  	struct i40e_eth_stats eth_stats_offsets;
>>  	u64 tx_restart;
> 
> ...
> 
>> +static void i40e_zero_tx_ring_stats(struct netdev_queue_stats_tx *tx)
>> {
>> +	tx->bytes = 0;
>> +	tx->packets = 0;
>> +	tx->stop = 0;
>> +	tx->wake = 0;
>> +	tx->hw_drops = 0;
>> +}
>> +
>> +static void i40e_add_tx_ring_stats(struct i40e_ring *tx_ring,
>> +				   struct netdev_queue_stats_tx *tx) {
>> +	u64 bytes, packets;
>> +	unsigned int start;
>> +
>> +	do {
>> +		start = u64_stats_fetch_begin(&tx_ring->syncp);
>> +		bytes = tx_ring->stats.bytes;
>> +		packets = tx_ring->stats.packets;
>> +	} while (u64_stats_fetch_retry(&tx_ring->syncp, start));
>> +
>> +	tx->bytes += bytes;
>> +	tx->packets += packets;
>> +
>> +	tx->stop += tx_ring->tx_stats.tx_stopped;
>> +	tx->wake += tx_ring->tx_stats.restart_queue;
>> +	tx->hw_drops += tx_ring->tx_stats.tx_busy; }
> Why the reads are outside the seqlock region? 
> On 32-bit kernels, unprotected u64 reads can tear IMHO

Currently there is no seqlock on the write side; to keep the series
small I preferred avoid fixing the pre-existing issue. In any case I
think moving stop, wake, hw_drops (and others) under seqlock protection
is an orthogonal change.

/P