netdev - Re: [PATCH v2 net-next 2/4] ibmvnic: Fix hardcoded NUM_RX_STATS/NUM_TX

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: <20250704171400.GN41770@horms.kernel.org>
Date: Fri, 4 Jul 2025 18:14:00 +0100
From: Simon Horman <horms@...nel.org>
To: Mingming Cao <mmc@...ux.ibm.com>
Cc: netdev@...r.kernel.org, bjking1@...ux.ibm.com, haren@...ux.ibm.com,
	ricklind@...ux.ibm.com, davemarq@...ux.ibm.com
Subject: Re: [PATCH v2 net-next 2/4] ibmvnic: Fix hardcoded
 NUM_RX_STATS/NUM_TX_STATS with dynamic sizeof

On Wed, Jul 02, 2025 at 10:18:02AM -0700, Mingming Cao wrote:
> The previous hardcoded definitions of NUM_RX_STATS and
> NUM_TX_STATS were not updated when new fields were added
> to the ibmvnic_{rx,tx}_queue_stats structures. Specifically,
> commit 2ee73c54a615 ("ibmvnic: Add stat for tx direct vs tx
> batched") added a fourth TX stat, but NUM_TX_STATS remained 3,
> leading to a mismatch.
> 
> This patch replaces the static defines with dynamic sizeof-based
> calculations to ensure the stat arrays are correctly sized.
> This fixes incorrect indexing and prevents incomplete stat
> reporting in tools like ethtool.
> 
> Fixes: 2ee73c54a615 ("ibmvnic: Add stat for tx direct vs tx batched")

nit: no blank line between Fixes and other tags please

More importantly, the cited commit is present in net so this
fix patch sould also be targeted at net. IOW, please break this
patch out of this patchset and post it targeted at net, while
the remaining patches should be posted as a v3 for net-next.

> 
> Signed-off-by: Mingming Cao <mmc@...ux.ibm.com>
> Reviewed-by: Dave Marquardt <davemarq@...ux.ibm.com>
> Reviewed by: Haren Myneni <haren@...ux.ibm.com>
> ---
>  drivers/net/ethernet/ibm/ibmvnic.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
> index 9b1693d817..e574eed97c 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.h
> +++ b/drivers/net/ethernet/ibm/ibmvnic.h
> @@ -211,7 +211,6 @@ struct ibmvnic_statistics {
>  	u8 reserved[72];
>  } __packed __aligned(8);
>  
> -#define NUM_TX_STATS 3
>  struct ibmvnic_tx_queue_stats {
>  	atomic64_t batched_packets;
>  	atomic64_t direct_packets;
> @@ -219,13 +218,18 @@ struct ibmvnic_tx_queue_stats {
>  	atomic64_t dropped_packets;
>  };
>  
> -#define NUM_RX_STATS 3
> +#define NUM_TX_STATS \
> +	(sizeof(struct ibmvnic_tx_queue_stats) / sizeof(atomic64_t))
> +

I'd suggest changing this to use the old, u64, type instead of atomic64_t.
Then, once this patch has hit net-next, via net, post the remaining patches
of this patchset for net-next. And at that time, in what is currently the
first patch of this series, which changes the type of the members of struct
ibmvnic_tx_queue_stats, also update u64 to atomic64_t here.

>  struct ibmvnic_rx_queue_stats {
>  	atomic64_t packets;
>  	atomic64_t bytes;
>  	atomic64_t interrupts;
>  };
>  
> +#define NUM_RX_STATS \
> +	(sizeof(struct ibmvnic_rx_queue_stats) / sizeof(atomic64_t))
> +

Likewise here.

>  struct ibmvnic_acl_buffer {
>  	__be32 len;
>  	__be32 version;