[PATCH net-next v2 08/13] net: mana: keep per-queue statistics in the port context

[Long Li <longli@microsoft.com>]

struct mana_rxq and struct mana_txq embed their statistics by value, and
mana_get_stats64()/mana_get_ethtool_stats() read them through
apc->rxqs[q] and apc->tx_qp[q]. The queues are destroyed and recreated
by every reconfiguration - ethtool channel count, ring size and private
flags, MTU changes and XDP attach - so the interface counters are reset
each time and rx_bytes/tx_bytes can be observed going backwards:

  rx_bytes before: 4475831638
  rx_bytes after:   526629152

Move the statistics into arrays owned by the port context, sized to
max_queues, allocated in mana_probe_port() and freed in mana_remove().
The queues keep pointers into those arrays, so a queue set can be freed
without losing the counters it accumulated.

mana_get_stats64() now walks max_queues rather than num_queues, so
counters accumulated on queues that a later reconfiguration removed are
still reported and the totals stay monotonic. The ethtool per-queue
statistics keep iterating num_queues, since mana_get_sset_count() sizes
the string table the same way.

Note this is not specific to the queue-set swap: the detach/attach path
reset the counters in exactly the same way. It matters more now only
because reconfiguration is hitless and therefore likely to be done on a
live link.

Signed-off-by: Long Li <longli@microsoft.com>
---
 .../net/ethernet/microsoft/mana/mana_bpf.c    |   4 +-
 drivers/net/ethernet/microsoft/mana/mana_en.c | 112 +++++++++++++++---
 .../ethernet/microsoft/mana/mana_ethtool.c    |   4 +-
 include/net/mana/mana.h                       |  27 ++++-
 4 files changed, 122 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/mana_bpf.c b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
index 1a6a490a84f2e5b4c9895645ff6f40beb55f497c..ff953cbfda0ea8b27d18367fd8047d2183f3591b 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_bpf.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
@@ -75,7 +75,7 @@ int mana_xdp_xmit(struct net_device *ndev, int n, struct xdp_frame **frames,
 		count++;
 	}
 
-	tx_stats = &apc->tx_qp[q_idx]->txq.stats;
+	tx_stats = apc->tx_qp[q_idx]->txq.stats;
 
 	u64_stats_update_begin(&tx_stats->syncp);
 	tx_stats->xdp_xmit += count;
@@ -102,7 +102,7 @@ u32 mana_run_xdp(struct net_device *ndev, struct mana_rxq *rxq,
 
 	act = bpf_prog_run_xdp(prog, xdp);
 
-	rx_stats = &rxq->stats;
+	rx_stats = rxq->stats;
 
 	switch (act) {
 	case XDP_PASS:
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 89d5215db160572f633e32711f251a6b1d82b276..60ea12e7e866385688c9095759e7c03f28c7771f 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -386,7 +386,7 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	txq = &apc->tx_qp[txq_idx]->txq;
 	gdma_sq = txq->gdma_sq;
 	cq = &apc->tx_qp[txq_idx]->tx_cq;
-	tx_stats = &txq->stats;
+	tx_stats = txq->stats;
 
 	BUILD_BUG_ON(MAX_TX_WQE_SGL_ENTRIES != MANA_MAX_TX_WQE_SGL_ENTRIES);
 	if (MAX_SKB_FRAGS + 2 > MAX_TX_WQE_SGL_ENTRIES &&
@@ -565,7 +565,7 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	/* Populated the packet and bytes counters based on post GSO packet
 	 * calculations
 	 */
-	tx_stats = &txq->stats;
+	tx_stats = txq->stats;
 	u64_stats_update_begin(&tx_stats->syncp);
 	tx_stats->packets += num_gso_seg;
 	tx_stats->bytes += len + ((num_gso_seg - 1) * gso_hs);
@@ -611,9 +611,9 @@ static void mana_get_stats64(struct net_device *ndev,
 			     struct rtnl_link_stats64 *st)
 {
 	struct mana_port_context *apc = netdev_priv(ndev);
-	unsigned int num_queues = apc->num_queues;
 	struct mana_stats_rx *rx_stats;
 	struct mana_stats_tx *tx_stats;
+	unsigned int num_queues;
 	unsigned int start;
 	u64 packets, bytes;
 	int q;
@@ -621,6 +621,12 @@ static void mana_get_stats64(struct net_device *ndev,
 	if (!apc->port_is_up)
 		return;
 
+	/* Walk every slot, not just the queues currently open: counters
+	 * accumulated on queues that a later reconfiguration removed must
+	 * still be reported, or the interface totals would go backwards.
+	 */
+	num_queues = apc->max_queues;
+
 	netdev_stats_to_stats64(st, &ndev->stats);
 
 	if (apc->ac->hwc_timeout_occurred)
@@ -629,7 +635,7 @@ static void mana_get_stats64(struct net_device *ndev,
 	st->rx_missed_errors = apc->ac->hc_stats.hc_rx_discards_no_wqe;
 
 	for (q = 0; q < num_queues; q++) {
-		rx_stats = &apc->rxqs[q]->stats;
+		rx_stats = &apc->rxq_stats[q];
 
 		do {
 			start = u64_stats_fetch_begin(&rx_stats->syncp);
@@ -642,7 +648,7 @@ static void mana_get_stats64(struct net_device *ndev,
 	}
 
 	for (q = 0; q < num_queues; q++) {
-		tx_stats = &apc->tx_qp[q]->txq.stats;
+		tx_stats = &apc->txq_stats[q];
 
 		do {
 			start = u64_stats_fetch_begin(&tx_stats->syncp);
@@ -1069,6 +1075,67 @@ static void mana_cleanup_port_context(struct mana_port_context *apc)
 	apc->rxqs = NULL;
 }
 
+/* Allocate the port-owned per-queue counters.
+ *
+ * Per-queue statistics are owned by the port context, not by the queues, so
+ * that a queue-set replacement (ethtool channel/ring/priv-flag change, MTU
+ * change, XDP attach) does not reset the interface counters. Sized to
+ * max_queues and allocated once.
+ *
+ * A swap adds no writer to a TX slot: mana_start_xmit() and mana_xdp_xmit()
+ * reach a txq only through apc->tx_qp[], and mana_publish_qset() swaps that
+ * array behind netif_tx_disable() and synchronize_net(), so a retiring txq
+ * can no longer be reached once the new set is installed. Those two callers
+ * can still race each other on one slot, but that is the existing lockless
+ * ndo_xdp_xmit() arrangement and not a consequence of replacing a queue set.
+ *
+ * The RX slots do overlap briefly. A retiring rxq keeps its NAPI until
+ * mana_free_qset() destroys it, so between mana_config_rss() steering onto
+ * the new RQs and that teardown, both generations at one index can update the
+ * same slot. MANA is 64-bit only, so u64_stats_sync carries no seqcount and
+ * there is nothing to corrupt; at worst a couple of increments are lost.
+ * Exposing counters that survive a rebuild is worth that.
+ *
+ * Closing that window is deliberately not worth it. Serialising the writers
+ * would put a lock in the per-packet receive path to protect a counter, and
+ * giving each queue set its own slots to merge later would make
+ * ndo_get_stats64() report a dip for the duration of the swap - the totals
+ * going backwards is the very thing this is meant to prevent, and a worse
+ * artefact than the occasional lost increment.
+ */
+static int mana_alloc_queue_stats(struct mana_port_context *apc)
+{
+	unsigned int i;
+
+	apc->rxq_stats = kcalloc(apc->max_queues, sizeof(*apc->rxq_stats),
+				 GFP_KERNEL);
+	if (!apc->rxq_stats)
+		return -ENOMEM;
+
+	apc->txq_stats = kcalloc(apc->max_queues, sizeof(*apc->txq_stats),
+				 GFP_KERNEL);
+	if (!apc->txq_stats) {
+		kfree(apc->rxq_stats);
+		apc->rxq_stats = NULL;
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < apc->max_queues; i++) {
+		u64_stats_init(&apc->rxq_stats[i].syncp);
+		u64_stats_init(&apc->txq_stats[i].syncp);
+	}
+
+	return 0;
+}
+
+static void mana_free_queue_stats(struct mana_port_context *apc)
+{
+	kfree(apc->rxq_stats);
+	apc->rxq_stats = NULL;
+	kfree(apc->txq_stats);
+	apc->txq_stats = NULL;
+}
+
 static void mana_cleanup_indir_table(struct mana_port_context *apc)
 {
 	apc->indir_table_sz = 0;
@@ -2127,7 +2194,7 @@ static void mana_rx_skb(void *buf_va, bool from_pool,
 			struct mana_rxcomp_oob *cqe, struct mana_rxq *rxq,
 			u32 pkt_len, u32 pkt_hash)
 {
-	struct mana_stats_rx *rx_stats = &rxq->stats;
+	struct mana_stats_rx *rx_stats = rxq->stats;
 	struct net_device *ndev = rxq->ndev;
 	u16 rxq_idx = rxq->rxq_idx;
 	struct napi_struct *napi;
@@ -2441,13 +2508,13 @@ static void mana_process_rx_cqe(struct mana_rxq *rxq, struct mana_cq *cq,
 	 * Coalesced CQEs have at least 2 packets, so index is pkt_i - 2.
 	 */
 	if (pkt_i > 1) {
-		u64_stats_update_begin(&rxq->stats.syncp);
-		rxq->stats.coalesced_cqe[pkt_i - 2]++;
-		u64_stats_update_end(&rxq->stats.syncp);
+		u64_stats_update_begin(&rxq->stats->syncp);
+		rxq->stats->coalesced_cqe[pkt_i - 2]++;
+		u64_stats_update_end(&rxq->stats->syncp);
 	} else if (!pkt_i && !pktlen) {
-		u64_stats_update_begin(&rxq->stats.syncp);
-		rxq->stats.pkt_len0_err++;
-		u64_stats_update_end(&rxq->stats.syncp);
+		u64_stats_update_begin(&rxq->stats->syncp);
+		rxq->stats->pkt_len0_err++;
+		u64_stats_update_end(&rxq->stats->syncp);
 		netdev_err_once(ndev,
 				"RX pkt len=0, rq=%u, cq=%u, rxobj=0x%llx\n",
 				rxq->gdma_id, cq->gdma_id, rxq->rxobj);
@@ -2579,8 +2646,8 @@ static void mana_update_rx_dim(struct mana_cq *cq)
 	if (!smp_load_acquire(&apc->rx_dim_enabled))
 		return;
 
-	dim_update_sample(READ_ONCE(cq->dim_event_ctr), rxq->stats.packets,
-			  rxq->stats.bytes, &dim_sample);
+	dim_update_sample(READ_ONCE(cq->dim_event_ctr), rxq->stats->packets,
+			  rxq->stats->bytes, &dim_sample);
 	net_dim(&cq->dim, &dim_sample);
 }
 
@@ -2797,7 +2864,7 @@ static int mana_create_txq(struct mana_port_context *apc,
 		/* Create SQ */
 		txq = &apc->tx_qp[i]->txq;
 
-		u64_stats_init(&txq->stats.syncp);
+		txq->stats = &apc->txq_stats[i];
 		txq->ndev = net;
 		txq->net_txq = netdev_get_tx_queue(net, i);
 		txq->reset_gen = READ_ONCE(apc->ac->reset_gen);
@@ -3112,6 +3179,8 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
 		return ERR_PTR(-ENOMEM);
 
 	rxq->ndev = ndev;
+	/* Wire up the port-owned statistics before the queue can be polled. */
+	rxq->stats = &apc->rxq_stats[rxq_idx];
 	rxq->num_rx_buf = apc->rx_queue_size;
 	rxq->rxq_idx = rxq_idx;
 	rxq->rxobj = INVALID_MANA_HANDLE;
@@ -3262,7 +3331,6 @@ static int mana_add_rx_queues(struct mana_port_context *apc,
 			goto out;
 		}
 
-		u64_stats_init(&rxq->stats.syncp);
 
 		apc->rxqs[i] = rxq;
 
@@ -4712,6 +4780,10 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
 		apc->tx_dim_enabled = MANA_ADAPTIVE_TX_DEF;
 	}
 
+	err = mana_alloc_queue_stats(apc);
+	if (err)
+		goto free_net;
+
 	mutex_init(&apc->vport_mutex);
 	apc->vport_use_count = 0;
 
@@ -4734,7 +4806,7 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
 
 	err = mana_init_port(ndev);
 	if (err)
-		goto free_net;
+		goto free_stats;
 
 	err = mana_rss_table_alloc(apc);
 	if (err)
@@ -4771,6 +4843,11 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
 	mana_cleanup_indir_table(apc);
 reset_apc:
 	mana_cleanup_port_context(apc);
+free_stats:
+	/* The counter arrays are separate allocations, so free_netdev() does
+	 * not release them with the port context.
+	 */
+	mana_free_queue_stats(apc);
 free_net:
 	*ndev_storage = NULL;
 	netdev_err(ndev, "Failed to probe vPort %d: %d\n", port_idx, err);
@@ -5111,6 +5188,7 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
 
 		unregister_netdevice(ndev);
 		mana_cleanup_indir_table(apc);
+		mana_free_queue_stats(apc);
 
 		/* Clear the slot before the netdev goes away. A later port
 		 * whose teardown has to reset the function walks ac->ports[]
diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
index 2cadd0f0d74e358c1b670dd6980603e0358918eb..4c8799a2be86ca1f7e52cb23f6fb71afdf38acb5 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
@@ -271,7 +271,7 @@ static void mana_get_ethtool_stats(struct net_device *ndev,
 		data[i++] = *(u64 *)(phy_stats + mana_phy_stats[q].offset);
 
 	for (q = 0; q < num_queues; q++) {
-		rx_stats = &apc->rxqs[q]->stats;
+		rx_stats = &apc->rxq_stats[q];
 
 		do {
 			start = u64_stats_fetch_begin(&rx_stats->syncp);
@@ -296,7 +296,7 @@ static void mana_get_ethtool_stats(struct net_device *ndev,
 	}
 
 	for (q = 0; q < num_queues; q++) {
-		tx_stats = &apc->tx_qp[q]->txq.stats;
+		tx_stats = &apc->txq_stats[q];
 
 		do {
 			start = u64_stats_fetch_begin(&tx_stats->syncp);
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index bc9808f826f83df26c7af98daa181d91b7d2661d..32035605c70c66299b9f9707924ee267fa63cd9a 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -102,7 +102,11 @@ struct mana_stats_rx {
 	u64 pkt_len0_err;
 	u64 coalesced_cqe[MANA_CQE_COAL_PKTS_8 - 1];
 	struct u64_stats_sync syncp;
-};
+	/* Kept in a per-port array indexed by queue, so give each entry its
+	 * own cache line. Without it two queues polled on different CPUs
+	 * would share one, and every packet would bounce it between them.
+	 */
+} ____cacheline_aligned_in_smp;
 
 struct mana_stats_tx {
 	u64 packets;
@@ -117,7 +121,8 @@ struct mana_stats_tx {
 	u64 csum_partial;
 	u64 mana_map_err;
 	struct u64_stats_sync syncp;
-};
+	/* Per-queue array entry, same cache line reasoning as the RX side. */
+} ____cacheline_aligned_in_smp;
 
 struct mana_txq {
 	struct gdma_queue *gdma_sq;
@@ -153,7 +158,10 @@ struct mana_txq {
 	 */
 	bool retiring;
 
-	struct mana_stats_tx stats;
+	/* Points into apc->txq_stats[]. Owned by the port context, not by
+	 * this queue, so counters survive queue-set replacement.
+	 */
+	struct mana_stats_tx *stats;
 };
 
 /* skb data and frags dma mappings */
@@ -415,7 +423,10 @@ struct mana_rxq {
 
 	u32 buf_index;
 
-	struct mana_stats_rx stats;
+	/* Points into apc->rxq_stats[]. Owned by the port context, not by
+	 * this queue, so counters survive queue-set replacement.
+	 */
+	struct mana_stats_rx *stats;
 
 	struct bpf_prog __rcu *bpf_prog;
 	struct xdp_rxq_info xdp_rxq;
@@ -618,6 +629,14 @@ struct mana_port_context {
 	unsigned int max_queues;
 	unsigned int num_queues;
 
+	/* Per-queue statistics, max_queues entries each. Allocated once at
+	 * probe and freed at remove, never on queue teardown, so counters
+	 * are not reset by an ethtool/MTU/XDP reconfiguration. The queues
+	 * point into these arrays.
+	 */
+	struct mana_stats_rx *rxq_stats;
+	struct mana_stats_tx *txq_stats;
+
 	unsigned int rx_queue_size;
 	unsigned int tx_queue_size;
 
-- 
2.43.0