All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Mingming Cao <mmc@linux.ibm.com>
Cc: netdev@vger.kernel.org, bjking1@linux.ibm.com,
	haren@linux.ibm.com, ricklind@linux.ibm.com,
	davemarq@linux.ibm.com
Subject: Re: [PATCH v2 net-next 2/4] ibmvnic: Fix hardcoded NUM_RX_STATS/NUM_TX_STATS with dynamic sizeof
Date: Fri, 4 Jul 2025 18:14:00 +0100	[thread overview]
Message-ID: <20250704171400.GN41770@horms.kernel.org> (raw)
In-Reply-To: <20250702171804.86422-3-mmc@linux.ibm.com>

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@linux.ibm.com>
> Reviewed-by: Dave Marquardt <davemarq@linux.ibm.com>
> Reviewed by: Haren Myneni <haren@linux.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;

  reply	other threads:[~2025-07-04 17:14 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-02 17:18 [PATCH v2 net-next 0/4] Fix/Improve queue stats and subcrq indirect handling Mingming Cao
2025-07-02 17:18 ` [PATCH v2 net-next 1/4] ibmvnic: Use atomic64_t for queue stats Mingming Cao
2025-07-02 17:18 ` [PATCH v2 net-next 2/4] ibmvnic: Fix hardcoded NUM_RX_STATS/NUM_TX_STATS with dynamic sizeof Mingming Cao
2025-07-04 17:14   ` Simon Horman [this message]
2025-07-02 17:18 ` [PATCH v2 net-next 3/4] ibmvnic: Use ndo_get_stats64 to fix inaccurate SAR reporting Mingming Cao
2025-07-04 17:19   ` Simon Horman
2025-07-02 17:18 ` [PATCH v2 net-next 4/4] ibmvnic: Make max subcrq indirect entries tunable via module param Mingming Cao
2025-07-04 17:05   ` Simon Horman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250704171400.GN41770@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=bjking1@linux.ibm.com \
    --cc=davemarq@linux.ibm.com \
    --cc=haren@linux.ibm.com \
    --cc=mmc@linux.ibm.com \
    --cc=netdev@vger.kernel.org \
    --cc=ricklind@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.