netdev - Re: [PATCH iwl-next v3 2/9] ice: use cacheline groups for ice_rx

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 for Android: free password hash cracker in your pocket

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

Message-ID: <828b75f2-6717-4f30-a62a-4992b03ef74f@intel.com>
Date: Fri, 14 Nov 2025 16:36:53 +0100
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Jacob Keller <jacob.e.keller@...el.com>, Tony Nguyen
	<anthony.l.nguyen@...el.com>
CC: Aleksandr Loktionov <aleksandr.loktionov@...el.com>, Przemek Kitszel
	<przemyslaw.kitszel@...el.com>, Simon Horman <horms@...nel.org>,
	<intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH iwl-next v3 2/9] ice: use cacheline groups for ice_rx_ring
 structure

From: Jacob Keller <jacob.e.keller@...el.com>
Date: Fri, 7 Nov 2025 15:31:46 -0800

> The ice ring structure was reorganized back by commit 65124bbf980c ("ice:
> Reorganize tx_buf and ring structs"), and later split into a separate
> ice_rx_ring structure by commit e72bba21355d ("ice: split ice_ring onto
> Tx/Rx separate structs")
> 
> The ice_rx_ring structure has comments left over from this prior
> reorganization indicating which fields belong to which cachelines.
> Unfortunately, these comments are not all accurate. The intended layout is
> for x86_64 systems with a 64-byte cache.
> 
>  * Cacheline 1 spans from the start of the struct to the end of the rx_fqes
>    and xdp_buf union. The comments correctly match this.
> 
>  * Cacheline 2 spans from hdr_fqes to the end of hdr_truesize, but the
>    comment indicates it should end xdp and xsk union.
> 
>  * Cacheline 3 spans from the truesize field to the xsk_pool, but the
>    comment wants this to be from the pkt_ctx down to the rcu head field.
> 
>  * Cacheline 4 spans from the rx_hdr_len down to the flags field, but the
>    comment indicates that it starts back at the ice_channel structure
>    pointer.
> 
>  * Cacheline 5 is indicated to cover the xdp_rxq. Because this field is
>    aligned to 64 bytes, this is actually true. However, there is a large 45
>    byte gap at the end of cacheline 4.

Sorry for reviewing this so late, but these comments really are outdated
as hell and don't really reflect what we'd like to achieve.

I would like to work together with you on rearranging and packing both
structures in an optimal way, the same what I did quite a bit ago for idpf.

Maybe we could drop the series from the next-queue for now?

> 
> The use of comments to indicate cachelines is poor design. In practice,
> comments like this quickly become outdated as developers add or remove
> fields, or as other sub-structures change layout and size unexpectedly.
Thanks,
Olek