linux-kernel - Re: [RFC net-next 1/1] idpf: Don't hard code napi

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: <6a440baa-fd9b-4d00-a15e-1cdbfce52168@intel.com>
Date: Mon, 30 Sep 2024 14:33:45 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Joe Damato <jdamato@...tly.com>
CC: <netdev@...r.kernel.org>, Tony Nguyen <anthony.l.nguyen@...el.com>,
	Przemek Kitszel <przemyslaw.kitszel@...el.com>, "David S. Miller"
	<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
	<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, "moderated list:INTEL
 ETHERNET DRIVERS" <intel-wired-lan@...ts.osuosl.org>, open list
	<linux-kernel@...r.kernel.org>
Subject: Re: [RFC net-next 1/1] idpf: Don't hard code napi_struct size

From: Joe Damato <jdamato@...tly.com>
Date: Wed, 25 Sep 2024 18:00:17 +0000

> The sizeof(struct napi_struct) can change. Don't hardcode the size to
> 400 bytes and instead use "sizeof(struct napi_struct)".

Just change this hardcode here when you submit your series.
I use sizeof() here only for structures which size can change depending
on .config, like ones containing spinlocks etc.
If you just add one field, it's easy to adjust the size here.

Otherwise, these assertions lose their sense. They're used to make sure
the structures are of *certain* *known* size, hardcoded inside them.
sizeof()s mean nothing, they don't give you the idea of how the
cachelines are organized after some code change.

> 
> While fixing this, also move other calculations into compile time
> defines.
> 
> Signed-off-by: Joe Damato <jdamato@...tly.com>
> ---
>  drivers/net/ethernet/intel/idpf/idpf_txrx.h | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> index f0537826f840..d5e904ddcb6e 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> @@ -437,9 +437,13 @@ struct idpf_q_vector {
>  	cpumask_var_t affinity_mask;
>  	__cacheline_group_end_aligned(cold);
>  };
> -libeth_cacheline_set_assert(struct idpf_q_vector, 112,
> -			    424 + 2 * sizeof(struct dim),
> -			    8 + sizeof(cpumask_var_t));
> +
> +#define IDPF_Q_VECTOR_RO_SZ (112)
> +#define IDPF_Q_VECTOR_RW_SZ (sizeof(struct napi_struct) + 24 + \
> +			     2 * sizeof(struct dim))
> +#define IDPF_Q_VECTOR_COLD_SZ (8 + sizeof(cpumask_var_t))
> +libeth_cacheline_set_assert(struct idpf_q_vector, IDPF_Q_VECTOR_RO_SZ,
> +			    IDPF_Q_VECTOR_RW_SZ, IDPF_Q_VECTOR_COLD_SZ);
>  
>  struct idpf_rx_queue_stats {
>  	u64_stats_t packets;

Thanks,
Olek