[PATCH v2 net-next 0/4] Fix/Improve queue stats and subcrq indirect handling

[PATCH v2 net-next 0/4] Fix/Improve queue stats and subcrq indirect handling

[Mingming Cao]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=true, Size: 2427 bytes --]

This patch series introduces fixes and improvements to the ibmvnic driver, 
focusing on accurate statistics reporting, CRQ scalability.

Patch 1: Convert per-queue RX/TX stats to atomic64_t to ensure thread-safe 
updates in concurrent environments. This establishes a safe and consistent 
foundation for subsequent statistics-related fixes.

Patch 2: Replace hardcoded NUM_RX_STATS and NUM_TX_STATS macros by dynamically
 deriving the counts using sizeof. This fixes a mismatch introduced in commit 
2ee73c54a615 (“ibmvnic: Add stat for tx direct vs tx batched”) and ensures 
that all stat fields are correctly reported via ethtool -S.

Patch 3: Fix inaccurate sar statistics by implementing ndo_get_stats64() and 
removing the outdated manual updates to netdev->stats.

Patch 4: Raise the default number of indirect sub-CRQ entries to 128 on POWER9 
and later systems, improving performance under high-throughput workloads. A 
module parameter is included as a fallback for compatibility on older systems; 
this is documented as a transitional mechanism, not intended for dynamic tuning.


--------------------------------------

Changes since v1:
 Link to v1: https://www.spinics.net/lists/netdev/msg1103893.html

Patch 1 (was Patch 2 in v1): Introduces atomic64_t stats early to establish 
a consistent foundation. Replaces atomic64_sub() with atomic64_sub_return() 
and adds a warning for underflow. This change now comes first to ensure correct
 ordering of dependency with stat size derivation.

Patch 2 (was Patch 1 in v1): Commit message rewritten to clearly highlight the 
mismatch bug caused by static stat counts and rebased on top of the atomic64 
conversion to avoid broken logic during intermediate states.

Patch 3: No functional changes

Patch 4: Commit message clarified to explain the use of 128 indirect entries 
on POWER9+ systems and the intent of the module parameter as a transitional 
fallback for legacy or constrained systems.

Mingming Cao (4):
  ibmvnic: Use atomic64_t for queue stats
  ibmvnic: Fix hardcoded NUM_RX_STATS/NUM_TX_STATS with dynamic sizeof
  ibmvnic: Use ndo_get_stats64 to fix inaccurate SAR reporting
  ibmvnic: Make max subcrq indirect entries tunable via module param

 drivers/net/ethernet/ibm/ibmvnic.c | 101 ++++++++++++++++++++---------
 drivers/net/ethernet/ibm/ibmvnic.h |  29 +++++----
 2 files changed, 89 insertions(+), 41 deletions(-)

-- 
2.39.3 (Apple Git-146)

[PATCH v2 net-next 1/4] ibmvnic: Use atomic64_t for queue stats

[Mingming Cao]

This patch improves the ibmvnic driver by changing the per-queue
packet and byte counters to atomic64_t types. This makes updates
thread-safe and easier to manage across multiple cores.

It also updates the ethtool statistics to safely read these new counters.

Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
Reviewed-by: Brian King <bjking1@linux.ibm.com>
Reviewed-by: Dave Marquardt <davemarq@linux.ibm.com>
Reviewed by: Rick Lindsley <ricklind@linux.ibm.com>
Reviewed by: Haren Myneni <haren@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 43 ++++++++++++++++++------------
 drivers/net/ethernet/ibm/ibmvnic.h | 14 +++++-----
 2 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 92647e137c..8b1c518124 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -69,6 +69,7 @@
 #include <linux/if_vlan.h>
 #include <linux/utsname.h>
 #include <linux/cpu.h>
+#include <linux/atomic.h>
 
 #include "ibmvnic.h"
 
@@ -2314,9 +2315,17 @@ static void ibmvnic_tx_scrq_clean_buffer(struct ibmvnic_adapter *adapter,
 		tx_buff = &tx_pool->tx_buff[index];
 		adapter->netdev->stats.tx_packets--;
 		adapter->netdev->stats.tx_bytes -= tx_buff->skb->len;
-		adapter->tx_stats_buffers[queue_num].batched_packets--;
-		adapter->tx_stats_buffers[queue_num].bytes -=
-						tx_buff->skb->len;
+		atomic64_dec(&adapter->tx_stats_buffers[queue_num].batched_packets);
+		if (atomic64_sub_return(tx_buff->skb->len,
+					&adapter->tx_stats_buffers[queue_num].bytes) < 0) {
+			atomic64_set(&adapter->tx_stats_buffers[queue_num].bytes, 0);
+			netdev_warn(adapter->netdev,
+				    "TX stats underflow on queue %u: bytes (%lld) < skb->len (%u),\n"
+				    "clamping to 0\n",
+				    queue_num,
+				    atomic64_read(&adapter->tx_stats_buffers[queue_num].bytes),
+				    tx_buff->skb->len);
+		}
 		dev_kfree_skb_any(tx_buff->skb);
 		tx_buff->skb = NULL;
 		adapter->netdev->stats.tx_dropped++;
@@ -2652,10 +2661,10 @@ out:
 	netdev->stats.tx_packets += tx_bpackets + tx_dpackets;
 	adapter->tx_send_failed += tx_send_failed;
 	adapter->tx_map_failed += tx_map_failed;
-	adapter->tx_stats_buffers[queue_num].batched_packets += tx_bpackets;
-	adapter->tx_stats_buffers[queue_num].direct_packets += tx_dpackets;
-	adapter->tx_stats_buffers[queue_num].bytes += tx_bytes;
-	adapter->tx_stats_buffers[queue_num].dropped_packets += tx_dropped;
+	atomic64_add(tx_bpackets, &adapter->tx_stats_buffers[queue_num].batched_packets);
+	atomic64_add(tx_dpackets, &adapter->tx_stats_buffers[queue_num].direct_packets);
+	atomic64_add(tx_bytes, &adapter->tx_stats_buffers[queue_num].bytes);
+	atomic64_add(tx_dropped, &adapter->tx_stats_buffers[queue_num].dropped_packets);
 
 	return ret;
 }
@@ -3569,8 +3578,8 @@ restart_poll:
 		napi_gro_receive(napi, skb); /* send it up */
 		netdev->stats.rx_packets++;
 		netdev->stats.rx_bytes += length;
-		adapter->rx_stats_buffers[scrq_num].packets++;
-		adapter->rx_stats_buffers[scrq_num].bytes += length;
+		atomic64_inc(&adapter->rx_stats_buffers[scrq_num].packets);
+		atomic64_add(length, &adapter->rx_stats_buffers[scrq_num].bytes);
 		frames_processed++;
 	}
 
@@ -3874,22 +3883,22 @@ static void ibmvnic_get_ethtool_stats(struct net_device *dev,
 				      (adapter, ibmvnic_stats[i].offset));
 
 	for (j = 0; j < adapter->req_tx_queues; j++) {
-		data[i] = adapter->tx_stats_buffers[j].batched_packets;
+		data[i] = atomic64_read(&adapter->tx_stats_buffers[j].batched_packets);
 		i++;
-		data[i] = adapter->tx_stats_buffers[j].direct_packets;
+		data[i] = atomic64_read(&adapter->tx_stats_buffers[j].direct_packets);
 		i++;
-		data[i] = adapter->tx_stats_buffers[j].bytes;
+		data[i] = atomic64_read(&adapter->tx_stats_buffers[j].bytes);
 		i++;
-		data[i] = adapter->tx_stats_buffers[j].dropped_packets;
+		data[i] = atomic64_read(&adapter->tx_stats_buffers[j].dropped_packets);
 		i++;
 	}
 
 	for (j = 0; j < adapter->req_rx_queues; j++) {
-		data[i] = adapter->rx_stats_buffers[j].packets;
+		data[i] = atomic64_read(&adapter->rx_stats_buffers[j].packets);
 		i++;
-		data[i] = adapter->rx_stats_buffers[j].bytes;
+		data[i] = atomic64_read(&adapter->rx_stats_buffers[j].bytes);
 		i++;
-		data[i] = adapter->rx_stats_buffers[j].interrupts;
+		data[i] = atomic64_read(&adapter->rx_stats_buffers[j].interrupts);
 		i++;
 	}
 }
@@ -4307,7 +4316,7 @@ static irqreturn_t ibmvnic_interrupt_rx(int irq, void *instance)
 	if (unlikely(adapter->state != VNIC_OPEN))
 		return IRQ_NONE;
 
-	adapter->rx_stats_buffers[scrq->scrq_num].interrupts++;
+	atomic64_inc(&adapter->rx_stats_buffers[scrq->scrq_num].interrupts);
 
 	if (napi_schedule_prep(&adapter->napi[scrq->scrq_num])) {
 		disable_scrq_irq(adapter, scrq);
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index a189038d88..9b1693d817 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -213,17 +213,17 @@ struct ibmvnic_statistics {
 
 #define NUM_TX_STATS 3
 struct ibmvnic_tx_queue_stats {
-	u64 batched_packets;
-	u64 direct_packets;
-	u64 bytes;
-	u64 dropped_packets;
+	atomic64_t batched_packets;
+	atomic64_t direct_packets;
+	atomic64_t bytes;
+	atomic64_t dropped_packets;
 };
 
 #define NUM_RX_STATS 3
 struct ibmvnic_rx_queue_stats {
-	u64 packets;
-	u64 bytes;
-	u64 interrupts;
+	atomic64_t packets;
+	atomic64_t bytes;
+	atomic64_t interrupts;
 };
 
 struct ibmvnic_acl_buffer {
-- 
2.39.3 (Apple Git-146)

[PATCH v2 net-next 2/4] ibmvnic: Fix hardcoded NUM_RX_STATS/NUM_TX_STATS with dynamic sizeof

[Mingming Cao]

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")

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))
+
 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))
+
 struct ibmvnic_acl_buffer {
 	__be32 len;
 	__be32 version;
-- 
2.39.3 (Apple Git-146)

[PATCH v2 net-next 3/4] ibmvnic: Use ndo_get_stats64 to fix inaccurate SAR reporting

[Mingming Cao]

VNIC testing on multi-core Power systems showed SAR stats drift
and packet rate inconsistencies under load.

Implements ndo_get_stats64 to provide safe aggregation of queue-level
atomic64 counters into rtnl_link_stats64 for use by tools like 'ip -s',
'ifconfig', and 'sar'. Switch to ndo_get_stats64 to align SAR reporting
with the standard kernel interface for retrieving netdev stats.

This removes redundant per-adapter stat updates, reduces overhead,
eliminates cacheline bouncing from hot path updates, and improves
the accuracy of reported packet rates.

Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
Reviewed by: Brian King <bjking1@linux.ibm.com>
Reviewed by: Dave Marquardt <davemarq@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 8b1c518124..9406ea06d9 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2313,8 +2313,6 @@ static void ibmvnic_tx_scrq_clean_buffer(struct ibmvnic_adapter *adapter,
 					  tx_pool->num_buffers - 1 :
 					  tx_pool->consumer_index - 1;
 		tx_buff = &tx_pool->tx_buff[index];
-		adapter->netdev->stats.tx_packets--;
-		adapter->netdev->stats.tx_bytes -= tx_buff->skb->len;
 		atomic64_dec(&adapter->tx_stats_buffers[queue_num].batched_packets);
 		if (atomic64_sub_return(tx_buff->skb->len,
 					&adapter->tx_stats_buffers[queue_num].bytes) < 0) {
@@ -2326,9 +2324,9 @@ static void ibmvnic_tx_scrq_clean_buffer(struct ibmvnic_adapter *adapter,
 				    atomic64_read(&adapter->tx_stats_buffers[queue_num].bytes),
 				    tx_buff->skb->len);
 		}
+		atomic64_inc(&adapter->tx_stats_buffers[queue_num].dropped_packets);
 		dev_kfree_skb_any(tx_buff->skb);
 		tx_buff->skb = NULL;
-		adapter->netdev->stats.tx_dropped++;
 	}
 
 	ind_bufp->index = 0;
@@ -2656,9 +2654,6 @@ tx_err:
 	}
 out:
 	rcu_read_unlock();
-	netdev->stats.tx_dropped += tx_dropped;
-	netdev->stats.tx_bytes += tx_bytes;
-	netdev->stats.tx_packets += tx_bpackets + tx_dpackets;
 	adapter->tx_send_failed += tx_send_failed;
 	adapter->tx_map_failed += tx_map_failed;
 	atomic64_add(tx_bpackets, &adapter->tx_stats_buffers[queue_num].batched_packets);
@@ -3461,6 +3456,25 @@ err:
 	return -ret;
 }
 
+static void ibmvnic_get_stats64(struct net_device *netdev,
+				struct rtnl_link_stats64 *stats)
+{
+	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+	int i;
+
+	for (i = 0; i < adapter->req_rx_queues; i++) {
+		stats->rx_packets += atomic64_read(&adapter->rx_stats_buffers[i].packets);
+		stats->rx_bytes   += atomic64_read(&adapter->rx_stats_buffers[i].bytes);
+	}
+
+	for (i = 0; i < adapter->req_tx_queues; i++) {
+		stats->tx_packets += atomic64_read(&adapter->tx_stats_buffers[i].batched_packets);
+		stats->tx_packets += atomic64_read(&adapter->tx_stats_buffers[i].direct_packets);
+		stats->tx_bytes   += atomic64_read(&adapter->tx_stats_buffers[i].bytes);
+		stats->tx_dropped += atomic64_read(&adapter->tx_stats_buffers[i].dropped_packets);
+	}
+}
+
 static void ibmvnic_tx_timeout(struct net_device *dev, unsigned int txqueue)
 {
 	struct ibmvnic_adapter *adapter = netdev_priv(dev);
@@ -3576,8 +3590,6 @@ restart_poll:
 
 		length = skb->len;
 		napi_gro_receive(napi, skb); /* send it up */
-		netdev->stats.rx_packets++;
-		netdev->stats.rx_bytes += length;
 		atomic64_inc(&adapter->rx_stats_buffers[scrq_num].packets);
 		atomic64_add(length, &adapter->rx_stats_buffers[scrq_num].bytes);
 		frames_processed++;
@@ -3687,6 +3699,7 @@ static const struct net_device_ops ibmvnic_netdev_ops = {
 	.ndo_set_rx_mode	= ibmvnic_set_multi,
 	.ndo_set_mac_address	= ibmvnic_set_mac,
 	.ndo_validate_addr	= eth_validate_addr,
+	.ndo_get_stats64	= ibmvnic_get_stats64,
 	.ndo_tx_timeout		= ibmvnic_tx_timeout,
 	.ndo_change_mtu		= ibmvnic_change_mtu,
 	.ndo_features_check     = ibmvnic_features_check,
-- 
2.39.3 (Apple Git-146)

[PATCH v2 net-next 4/4] ibmvnic: Make max subcrq indirect entries tunable via module param

[Mingming Cao]

This patch increases the default number of subcrq indirect entries from 16
to 128, a value supported on POWER9 and later systems. Increasing this limit
improves batching efficiency in hypervisor communication, enhancing throughput
under high-load conditions.

To maintain compatibility with older or constrained systems (e.g., some POWER8 platforms),
 a module parameter max_subcrq_indirect is introduced as a transitional mechanism.
This allows administrators to manually reduce the limit if needed.

The module parameter is not intended for dynamic runtime tuning, but rather
provides forward compatibility without requiring broader structural changes at this time.

Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
Reviewed by: Rick Lindsley <ricklind@linux.ibm.com>
Reviewed by: Dave Marquardt <davemarq@linux.ibm.com>
Reviewed by: Brian King <bjking1@linux.ibm.com>
Reviewed by: Haren Myneni <haren@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 29 ++++++++++++++++++++++++-----
 drivers/net/ethernet/ibm/ibmvnic.h |  7 +++++--
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 9406ea06d9..33cdcae6a7 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -154,6 +154,11 @@ static const struct ibmvnic_stat ibmvnic_stats[] = {
 	{"internal_mac_rx_errors", IBMVNIC_STAT_OFF(internal_mac_rx_errors)},
 };
 
+/* Module parameter for max_ind_descs */
+static unsigned int max_ind_descs = IBMVNIC_MAX_IND_DESCS_DEFAULT;
+module_param(max_ind_descs, uint, 0444);
+MODULE_PARM_DESC(max_ind_descs, "Max indirect subcrq descriptors (16 to 128, default 128)");
+
 static int send_crq_init_complete(struct ibmvnic_adapter *adapter)
 {
 	union ibmvnic_crq crq;
@@ -844,7 +849,7 @@ static void replenish_rx_pool(struct ibmvnic_adapter *adapter,
 		sub_crq->rx_add.len = cpu_to_be32(pool->buff_size << shift);
 
 		/* if send_subcrq_indirect queue is full, flush to VIOS */
-		if (ind_bufp->index == IBMVNIC_MAX_IND_DESCS ||
+		if (ind_bufp->index == max_ind_descs ||
 		    i == count - 1) {
 			lpar_rc =
 				send_subcrq_indirect(adapter, handle,
@@ -2599,7 +2604,7 @@ static netdev_tx_t ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
 	tx_crq.v1.n_crq_elem = num_entries;
 	tx_buff->num_entries = num_entries;
 	/* flush buffer if current entry can not fit */
-	if (num_entries + ind_bufp->index > IBMVNIC_MAX_IND_DESCS) {
+	if (num_entries + ind_bufp->index > max_ind_descs) {
 		lpar_rc = ibmvnic_tx_scrq_flush(adapter, tx_scrq, true);
 		if (lpar_rc != H_SUCCESS)
 			goto tx_flush_err;
@@ -2612,7 +2617,7 @@ static netdev_tx_t ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
 	ind_bufp->index += num_entries;
 	if (__netdev_tx_sent_queue(txq, skb->len,
 				   netdev_xmit_more() &&
-				   ind_bufp->index < IBMVNIC_MAX_IND_DESCS)) {
+				   ind_bufp->index < max_ind_descs)) {
 		lpar_rc = ibmvnic_tx_scrq_flush(adapter, tx_scrq, true);
 		if (lpar_rc != H_SUCCESS)
 			goto tx_err;
@@ -4015,7 +4020,7 @@ static void release_sub_crq_queue(struct ibmvnic_adapter *adapter,
 	}
 
 	dma_free_coherent(dev,
-			  IBMVNIC_IND_ARR_SZ,
+			  max_ind_descs * IBMVNIC_IND_DESC_SZ,
 			  scrq->ind_buf.indir_arr,
 			  scrq->ind_buf.indir_dma);
 
@@ -4072,7 +4077,7 @@ static struct ibmvnic_sub_crq_queue *init_sub_crq_queue(struct ibmvnic_adapter
 
 	scrq->ind_buf.indir_arr =
 		dma_alloc_coherent(dev,
-				   IBMVNIC_IND_ARR_SZ,
+				   max_ind_descs * IBMVNIC_IND_DESC_SZ,
 				   &scrq->ind_buf.indir_dma,
 				   GFP_KERNEL);
 
@@ -6734,6 +6739,20 @@ static int __init ibmvnic_module_init(void)
 {
 	int ret;
 
+	if (max_ind_descs < IBMVNIC_MAX_IND_DESC_MIN ||
+	    max_ind_descs > IBMVNIC_MAX_IND_DESC_MAX) {
+		pr_info("ibmvnic: max_ind_descs=%u, must be between %d and %d. default %u\n",
+			max_ind_descs,
+			IBMVNIC_MAX_IND_DESC_MIN,
+			IBMVNIC_MAX_IND_DESC_MAX,
+			IBMVNIC_MAX_IND_DESCS_DEFAULT);
+
+		pr_info("ibmvnic: resetting max_ind_descs to default\n");
+		max_ind_descs = IBMVNIC_MAX_IND_DESCS_DEFAULT;
+	}
+
+	pr_info("ibmvnic: max_ind_descs set to %u\n", max_ind_descs);
+
 	ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, "net/ibmvnic:online",
 				      ibmvnic_cpu_online,
 				      ibmvnic_cpu_down_prep);
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index e574eed97c..48c16e6f8a 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -29,8 +29,10 @@
 #define IBMVNIC_BUFFS_PER_POOL	100
 #define IBMVNIC_MAX_QUEUES	16
 #define IBMVNIC_MAX_QUEUE_SZ   4096
-#define IBMVNIC_MAX_IND_DESCS  16
-#define IBMVNIC_IND_ARR_SZ	(IBMVNIC_MAX_IND_DESCS * 32)
+#define IBMVNIC_IND_DESC_SZ	32
+#define IBMVNIC_MAX_IND_DESCS_DEFAULT	128
+#define IBMVNIC_MAX_IND_DESC_MAX	128
+#define IBMVNIC_MAX_IND_DESC_MIN	16
 
 #define IBMVNIC_TSO_BUF_SZ	65536
 #define IBMVNIC_TSO_BUFS	64
@@ -945,6 +947,7 @@ struct ibmvnic_adapter {
 	int replenish_task_cycles;
 	int tx_send_failed;
 	int tx_map_failed;
+	u32 max_ind_descs;
 
 	struct ibmvnic_tx_queue_stats *tx_stats_buffers;
 	struct ibmvnic_rx_queue_stats *rx_stats_buffers;
-- 
2.39.3 (Apple Git-146)

Re: [PATCH v2 net-next 4/4] ibmvnic: Make max subcrq indirect entries tunable via module param

[Simon Horman]

On Wed, Jul 02, 2025 at 10:18:04AM -0700, Mingming Cao wrote:
> This patch increases the default number of subcrq indirect entries from 16
> to 128, a value supported on POWER9 and later systems. Increasing this limit
> improves batching efficiency in hypervisor communication, enhancing throughput
> under high-load conditions.
> 
> To maintain compatibility with older or constrained systems (e.g., some POWER8 platforms),
>  a module parameter max_subcrq_indirect is introduced as a transitional mechanism.
> This allows administrators to manually reduce the limit if needed.
> 
> The module parameter is not intended for dynamic runtime tuning, but rather
> provides forward compatibility without requiring broader structural changes at this time.
> 
> Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
> Reviewed by: Rick Lindsley <ricklind@linux.ibm.com>
> Reviewed by: Dave Marquardt <davemarq@linux.ibm.com>
> Reviewed by: Brian King <bjking1@linux.ibm.com>
> Reviewed by: Haren Myneni <haren@linux.ibm.com>

Hi,

In his review of v1 of this patchset, Jakub said:

  Module parameters are strongly discouraged. Please provide more details
  about what this parameter does, I supposed it should be mapped to on of
  the ethtool -g options.

  https://lore.kernel.org/netdev/20250701183107.6f6411c1@kernel.org/

Perhaps I'm missing something, but I don't see this discussed or addressed.

-- 
pw-bot: changes-requested

Re: [PATCH v2 net-next 2/4] ibmvnic: Fix hardcoded NUM_RX_STATS/NUM_TX_STATS with dynamic sizeof

[Simon Horman]

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;

Re: [PATCH v2 net-next 3/4] ibmvnic: Use ndo_get_stats64 to fix inaccurate SAR reporting

[Simon Horman]

On Wed, Jul 02, 2025 at 10:18:03AM -0700, Mingming Cao wrote:
> VNIC testing on multi-core Power systems showed SAR stats drift
> and packet rate inconsistencies under load.
> 
> Implements ndo_get_stats64 to provide safe aggregation of queue-level
> atomic64 counters into rtnl_link_stats64 for use by tools like 'ip -s',
> 'ifconfig', and 'sar'. Switch to ndo_get_stats64 to align SAR reporting
> with the standard kernel interface for retrieving netdev stats.
> 
> This removes redundant per-adapter stat updates, reduces overhead,
> eliminates cacheline bouncing from hot path updates, and improves
> the accuracy of reported packet rates.
> 
> Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
> Reviewed by: Brian King <bjking1@linux.ibm.com>
> Reviewed by: Dave Marquardt <davemarq@linux.ibm.com>

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