netdev - Re: [PATCH iwl-next v4 3/6] ice: remove ice_q_stats struct and use struct

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8311f3ef-7ead-47ce-9a41-4280a0c9fdda@intel.com>
Date: Wed, 3 Dec 2025 14:14:37 -0800
From: Jacob Keller <jacob.e.keller@...el.com>
To: Simon Horman <horms@...nel.org>
CC: Aleksandr Loktionov <aleksandr.loktionov@...el.com>, Alexander Lobakin
	<aleksander.lobakin@...el.com>, Tony Nguyen <anthony.l.nguyen@...el.com>,
	Przemek Kitszel <przemyslaw.kitszel@...el.com>,
	<intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH iwl-next v4 3/6] ice: remove ice_q_stats struct and use
 struct_group



On 11/25/2025 2:16 AM, Simon Horman wrote:
> On Thu, Nov 20, 2025 at 12:20:43PM -0800, Jacob Keller wrote:
>> The ice_qp_reset_stats function resets the stats for all rings on a VSI. It
>> currently behaves differently for Tx and Rx rings. For Rx rings, it only
>> clears the rx_stats which do not include the pkt and byte counts. For Tx
>> rings and XDP rings, it clears only the pkt and byte counts.
>>
>> We could add extra memset calls to cover both the stats and relevant
>> tx/rx stats fields. Instead, lets convert stats into a struct_group which
>> contains both the pkts and bytes fields as well as the Tx or Rx stats, and
>> remove the ice_q_stats structure entirely.
>>
>> The only remaining user of ice_q_stats is the ice_q_stats_len function in
>> ice_ethtool.c, which just counts the number of fields. Replace this with a
>> simple multiplication by 2. I find this to be simpler to reason about than
>> relying on knowing the layout of the ice_q_stats structure.
>>
>> Now that the stats field of the ice_ring_stats covers all of the statistic
>> values, the ice_qp_reset_stats function will properly zero out all of the
>> fields.
>>
>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
>> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> 
> I agree this is both more consistent and cleaner.
> 
> I do feel there might be a yet cleaner way to handle things
> in place of multiplication by 2. But I can't think of such
> a way at this time.
> 

I agree as well. Potentially some structure layouts would allow us to
get the total amount of stats, or we could count the number of Tx and Rx
queues separately.. The driver has some effort to allow varying number
of Tx/Rx queues in some places, but then lacks proper support for that
in others. In particular, the maximum number of both is always the same,
hence the multiply by 2 here.

> Reviewed-by: Simon Horman <horms@...nel.org>



Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)