patch 'net/hns3: fix MAC and queues HW statistics overflow' has been queued to stable release 21.11.2

[Kevin Traynor <ktraynor@redhat.com>]

Hi,

FYI, your patch has been queued to stable release 21.11.2

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 05/30/22. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Queued patches are on a temporary branch at:
https://github.com/kevintraynor/dpdk-stable

This queued commit can be viewed at:
https://github.com/kevintraynor/dpdk-stable/commit/827f72e8ce2c02527469ab958e9be08b7b12355b

Thanks.

Kevin

---
From 827f72e8ce2c02527469ab958e9be08b7b12355b Mon Sep 17 00:00:00 2001
From: Huisong Li <lihuisong@huawei.com>
Date: Thu, 5 May 2022 20:27:02 +0800
Subject: [PATCH] net/hns3: fix MAC and queues HW statistics overflow

[ upstream commit a65342d9d5d25cf0a9d44388325e2251daa83eba ]

The MAC and queues statistics are 32-bit registers in hardware. If
hardware statistics are not obtained for a long time, these statistics
will be overflow.
So PF and VF driver have to periodically obtain and save these
statistics. Since the periodical task and the stats API are in different
threads, we introduce a statistics lock to protect the statistics.

Fixes: 8839c5e202f3 ("net/hns3: support device stats")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c    |   6 +-
 drivers/net/hns3/hns3_ethdev.h    |   6 ++
 drivers/net/hns3/hns3_ethdev_vf.c |   6 +-
 drivers/net/hns3/hns3_stats.c     | 144 +++++++++++++++++++++---------
 drivers/net/hns3/hns3_stats.h     |   1 +
 5 files changed, 116 insertions(+), 47 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index a33ab1a656..80a5aed426 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -4392,8 +4392,10 @@ hns3_service_handler(void *param)
 	struct hns3_hw *hw = &hns->hw;
 
-	if (!hns3_is_reset_pending(hns))
+	if (!hns3_is_reset_pending(hns)) {
 		hns3_update_linkstatus_and_event(hw, true);
-	else
+		hns3_update_hw_stats(hw);
+	} else {
 		hns3_warn(hw, "Cancel the query when reset is pending");
+	}
 
 	rte_eal_alarm_set(HNS3_SERVICE_INTERVAL, hns3_service_handler, eth_dev);
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index 0e12d054e4..952434c864 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -506,4 +506,10 @@ struct hns3_hw {
 	struct hns3_rx_missed_stats imissed_stats;
 	uint64_t oerror_stats;
+	/*
+	 * The lock is used to protect statistics update in stats APIs and
+	 * periodic task.
+	 */
+	rte_spinlock_t stats_lock;
+
 	uint32_t fw_version;
 	uint16_t pf_vf_if_version;  /* version of communication interface */
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index c5252035a0..7ad37b08d2 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -1386,8 +1386,10 @@ hns3vf_service_handler(void *param)
 	 * pending, and if so, abandon the query.
 	 */
-	if (!hns3vf_is_reset_pending(hns))
+	if (!hns3vf_is_reset_pending(hns)) {
 		hns3vf_request_link_info(hw);
-	else
+		hns3_update_hw_stats(hw);
+	} else {
 		hns3_warn(hw, "Cancel the query when reset is pending");
+	}
 
 	rte_eal_alarm_set(HNS3VF_SERVICE_INTERVAL, hns3vf_service_handler,
diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c
index e4a5dcf2f8..2799ff4432 100644
--- a/drivers/net/hns3/hns3_stats.c
+++ b/drivers/net/hns3/hns3_stats.c
@@ -585,4 +585,26 @@ hns3_update_oerror_stats(struct hns3_hw *hw, bool is_clear)
 }
 
+static void
+hns3_rcb_rx_ring_stats_get(struct hns3_rx_queue *rxq,
+			   struct hns3_tqp_stats *stats)
+{
+	uint32_t cnt;
+
+	cnt = hns3_read_dev(rxq, HNS3_RING_RX_PKTNUM_RECORD_REG);
+	stats->rcb_rx_ring_pktnum_rcd += cnt;
+	stats->rcb_rx_ring_pktnum[rxq->queue_id] += cnt;
+}
+
+static void
+hns3_rcb_tx_ring_stats_get(struct hns3_tx_queue *txq,
+			   struct hns3_tqp_stats *stats)
+{
+	uint32_t cnt;
+
+	cnt = hns3_read_dev(txq, HNS3_RING_TX_PKTNUM_RECORD_REG);
+	stats->rcb_tx_ring_pktnum_rcd += cnt;
+	stats->rcb_tx_ring_pktnum[txq->queue_id] += cnt;
+}
+
 /*
  * Query tqp tx queue statistics ,opcode id: 0x0B03.
@@ -605,5 +627,4 @@ hns3_stats_get(struct rte_eth_dev *eth_dev, struct rte_eth_stats *rte_stats)
 	struct hns3_rx_queue *rxq;
 	struct hns3_tx_queue *txq;
-	uint64_t cnt;
 	uint16_t i;
 	int ret;
@@ -612,7 +633,6 @@ hns3_stats_get(struct rte_eth_dev *eth_dev, struct rte_eth_stats *rte_stats)
 	ret = hns3_update_imissed_stats(hw, false);
 	if (ret) {
-		hns3_err(hw, "update imissed stats failed, ret = %d",
-			 ret);
-		return ret;
+		hns3_err(hw, "update imissed stats failed, ret = %d", ret);
+		goto out;
 	}
 	rte_stats->imissed = imissed_stats->rpu_rx_drop_cnt +
@@ -625,13 +645,10 @@ hns3_stats_get(struct rte_eth_dev *eth_dev, struct rte_eth_stats *rte_stats)
 			continue;
 
-		cnt = hns3_read_dev(rxq, HNS3_RING_RX_PKTNUM_RECORD_REG);
-		/*
-		 * Read hardware and software in adjacent positions to minimize
-		 * the timing variance.
-		 */
+		rte_spinlock_lock(&hw->stats_lock);
+		hns3_rcb_rx_ring_stats_get(rxq, stats);
+		rte_spinlock_unlock(&hw->stats_lock);
+
 		rte_stats->ierrors += rxq->err_stats.l2_errors +
 				      rxq->err_stats.pkt_len_errors;
-		stats->rcb_rx_ring_pktnum_rcd += cnt;
-		stats->rcb_rx_ring_pktnum[i] += cnt;
 		rte_stats->ibytes += rxq->basic_stats.bytes;
 	}
@@ -643,7 +660,7 @@ hns3_stats_get(struct rte_eth_dev *eth_dev, struct rte_eth_stats *rte_stats)
 			continue;
 
-		cnt = hns3_read_dev(txq, HNS3_RING_TX_PKTNUM_RECORD_REG);
-		stats->rcb_tx_ring_pktnum_rcd += cnt;
-		stats->rcb_tx_ring_pktnum[i] += cnt;
+		rte_spinlock_lock(&hw->stats_lock);
+		hns3_rcb_tx_ring_stats_get(txq, stats);
+		rte_spinlock_unlock(&hw->stats_lock);
 		rte_stats->obytes += txq->basic_stats.bytes;
 	}
@@ -651,7 +668,6 @@ hns3_stats_get(struct rte_eth_dev *eth_dev, struct rte_eth_stats *rte_stats)
 	ret = hns3_update_oerror_stats(hw, false);
 	if (ret) {
-		hns3_err(hw, "update oerror stats failed, ret = %d",
-			 ret);
-		return ret;
+		hns3_err(hw, "update oerror stats failed, ret = %d", ret);
+		goto out;
 	}
 	rte_stats->oerrors = hw->oerror_stats;
@@ -668,6 +684,6 @@ hns3_stats_get(struct rte_eth_dev *eth_dev, struct rte_eth_stats *rte_stats)
 		rte_stats->oerrors;
 	rte_stats->rx_nombuf = eth_dev->data->rx_mbuf_alloc_failed;
-
-	return 0;
+out:
+	return ret;
 }
 
@@ -689,5 +705,5 @@ hns3_stats_reset(struct rte_eth_dev *eth_dev)
 	if (ret) {
 		hns3_err(hw, "clear imissed stats failed, ret = %d", ret);
-		return ret;
+		goto out;
 	}
 
@@ -698,7 +714,6 @@ hns3_stats_reset(struct rte_eth_dev *eth_dev)
 	ret = hns3_update_oerror_stats(hw, true);
 	if (ret) {
-		hns3_err(hw, "clear oerror stats failed, ret = %d",
-			 ret);
-		return ret;
+		hns3_err(hw, "clear oerror stats failed, ret = %d", ret);
+		goto out;
 	}
 
@@ -718,4 +733,5 @@ hns3_stats_reset(struct rte_eth_dev *eth_dev)
 			continue;
 
+		rte_spinlock_lock(&hw->stats_lock);
 		memset(&rxq->basic_stats, 0,
 				sizeof(struct hns3_rx_basic_stats));
@@ -725,4 +741,5 @@ hns3_stats_reset(struct rte_eth_dev *eth_dev)
 		rxq->err_stats.pkt_len_errors = 0;
 		rxq->err_stats.l2_errors = 0;
+		rte_spinlock_unlock(&hw->stats_lock);
 	}
 
@@ -733,4 +750,5 @@ hns3_stats_reset(struct rte_eth_dev *eth_dev)
 			continue;
 
+		rte_spinlock_lock(&hw->stats_lock);
 		memset(&txq->basic_stats, 0,
 				sizeof(struct hns3_tx_basic_stats));
@@ -738,9 +756,12 @@ hns3_stats_reset(struct rte_eth_dev *eth_dev)
 		/* This register is read-clear */
 		(void)hns3_read_dev(txq, HNS3_RING_TX_PKTNUM_RECORD_REG);
+		rte_spinlock_unlock(&hw->stats_lock);
 	}
 
+	rte_spinlock_lock(&hw->stats_lock);
 	hns3_tqp_stats_clear(hw);
-
-	return 0;
+	rte_spinlock_unlock(&hw->stats_lock);
+out:
+	return ret;
 }
 
@@ -909,5 +930,4 @@ hns3_rxq_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	struct hns3_rx_queue *rxq;
 	uint16_t i, j;
-	uint32_t cnt;
 	char *val;
 
@@ -917,14 +937,8 @@ hns3_rxq_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 			continue;
 
-		cnt = hns3_read_dev(rxq, HNS3_RING_RX_PKTNUM_RECORD_REG);
-		/*
-		 * Read hardware and software in adjacent positions to minimize
-		 * the time difference.
-		 */
+		hns3_rcb_rx_ring_stats_get(rxq, stats);
 		rxq_stats = &rxq->basic_stats;
 		rxq_stats->errors = rxq->err_stats.l2_errors +
 					rxq->err_stats.pkt_len_errors;
-		stats->rcb_rx_ring_pktnum_rcd += cnt;
-		stats->rcb_rx_ring_pktnum[i] += cnt;
 
 		/*
@@ -956,5 +970,4 @@ hns3_txq_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	struct hns3_tx_queue *txq;
 	uint16_t i, j;
-	uint32_t cnt;
 	char *val;
 
@@ -964,7 +977,5 @@ hns3_txq_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 			continue;
 
-		cnt = hns3_read_dev(txq, HNS3_RING_TX_PKTNUM_RECORD_REG);
-		stats->rcb_tx_ring_pktnum_rcd += cnt;
-		stats->rcb_tx_ring_pktnum[i] += cnt;
+		hns3_rcb_tx_ring_stats_get(txq, stats);
 
 		txq_stats = &txq->basic_stats;
@@ -1051,4 +1062,5 @@ hns3_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	count = 0;
 
+	rte_spinlock_lock(&hw->stats_lock);
 	hns3_tqp_basic_stats_get(dev, xstats, &count);
 
@@ -1058,4 +1070,5 @@ hns3_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		if (ret < 0) {
 			hns3_err(hw, "Update Mac stats fail : %d", ret);
+			rte_spinlock_unlock(&hw->stats_lock);
 			return ret;
 		}
@@ -1069,9 +1082,9 @@ hns3_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		}
 	}
+	rte_spinlock_unlock(&hw->stats_lock);
 
 	ret = hns3_update_imissed_stats(hw, false);
 	if (ret) {
-		hns3_err(hw, "update imissed stats failed, ret = %d",
-			 ret);
+		hns3_err(hw, "update imissed stats failed, ret = %d", ret);
 		return ret;
 	}
@@ -1102,6 +1115,8 @@ hns3_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	}
 
+	rte_spinlock_lock(&hw->stats_lock);
 	hns3_tqp_dfx_stats_get(dev, xstats, &count);
 	hns3_queue_stats_get(dev, xstats, &count);
+	rte_spinlock_unlock(&hw->stats_lock);
 
 	return count;
@@ -1454,4 +1469,5 @@ hns3_dev_xstats_reset(struct rte_eth_dev *dev)
 {
 	struct hns3_adapter *hns = dev->data->dev_private;
+	struct hns3_hw *hw = &hns->hw;
 	int ret;
 
@@ -1461,4 +1477,5 @@ hns3_dev_xstats_reset(struct rte_eth_dev *dev)
 		return ret;
 
+	rte_spinlock_lock(&hw->stats_lock);
 	hns3_tqp_dfx_stats_clear(dev);
 
@@ -1467,12 +1484,13 @@ hns3_dev_xstats_reset(struct rte_eth_dev *dev)
 
 	if (hns->is_vf)
-		return 0;
+		goto out;
 
 	/* HW registers are cleared on read */
 	ret = hns3_mac_stats_reset(dev);
-	if (ret)
-		return ret;
 
-	return 0;
+out:
+	rte_spinlock_unlock(&hw->stats_lock);
+
+	return ret;
 }
 
@@ -1528,4 +1546,5 @@ hns3_stats_init(struct hns3_hw *hw)
 	int ret;
 
+	rte_spinlock_init(&hw->stats_lock);
 	/* Hardware statistics of imissed registers cleared. */
 	ret = hns3_update_imissed_stats(hw, true);
@@ -1543,2 +1562,41 @@ hns3_stats_uninit(struct hns3_hw *hw)
 	hns3_tqp_stats_uninit(hw);
 }
+
+static void
+hns3_update_queues_stats(struct hns3_hw *hw)
+{
+	struct rte_eth_dev_data *data = hw->data;
+	struct hns3_rx_queue *rxq;
+	struct hns3_tx_queue *txq;
+	uint16_t i;
+
+	for (i = 0; i < data->nb_rx_queues; i++) {
+		rxq = data->rx_queues[i];
+		if (rxq != NULL)
+			hns3_rcb_rx_ring_stats_get(rxq, &hw->tqp_stats);
+	}
+
+	for (i = 0; i < data->nb_tx_queues; i++) {
+		txq = data->tx_queues[i];
+		if (txq != NULL)
+			hns3_rcb_tx_ring_stats_get(txq, &hw->tqp_stats);
+	}
+}
+
+/*
+ * Some hardware statistics registers are not 64-bit. If hardware statistics are
+ * not obtained for a long time, these statistics may be reversed. This function
+ * is used to update these hardware statistics in periodic task.
+ */
+void
+hns3_update_hw_stats(struct hns3_hw *hw)
+{
+	struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
+
+	rte_spinlock_lock(&hw->stats_lock);
+	if (!hns->is_vf)
+		hns3_update_mac_stats(hw);
+
+	hns3_update_queues_stats(hw);
+	rte_spinlock_unlock(&hw->stats_lock);
+}
diff --git a/drivers/net/hns3/hns3_stats.h b/drivers/net/hns3/hns3_stats.h
index e89dc97632..b5cd6188b4 100644
--- a/drivers/net/hns3/hns3_stats.h
+++ b/drivers/net/hns3/hns3_stats.h
@@ -165,4 +165,5 @@ int hns3_stats_init(struct hns3_hw *hw);
 void hns3_stats_uninit(struct hns3_hw *hw);
 int hns3_query_mac_stats_reg_num(struct hns3_hw *hw);
+void hns3_update_hw_stats(struct hns3_hw *hw);
 
 #endif /* _HNS3_STATS_H_ */
-- 
2.34.3

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2022-05-25 17:26:59.074623676 +0100
+++ 0020-net-hns3-fix-MAC-and-queues-HW-statistics-overflow.patch	2022-05-25 17:26:58.569828334 +0100
@@ -1 +1 @@
-From a65342d9d5d25cf0a9d44388325e2251daa83eba Mon Sep 17 00:00:00 2001
+From 827f72e8ce2c02527469ab958e9be08b7b12355b Mon Sep 17 00:00:00 2001
@@ -5,0 +6,2 @@
+[ upstream commit a65342d9d5d25cf0a9d44388325e2251daa83eba ]
+
@@ -14 +15,0 @@
-Cc: stable@dpdk.org
@@ -27 +28 @@
-index 5aed7046d8..1d9b19d83e 100644
+index a33ab1a656..80a5aed426 100644
@@ -30 +31 @@
-@@ -4365,8 +4365,10 @@ hns3_service_handler(void *param)
+@@ -4392,8 +4392,10 @@ hns3_service_handler(void *param)
@@ -44 +45 @@
-index 75a89c3d1c..bb6ddd97ba 100644
+index 0e12d054e4..952434c864 100644
@@ -47 +48 @@
-@@ -504,4 +504,10 @@ struct hns3_hw {
+@@ -506,4 +506,10 @@ struct hns3_hw {
@@ -59 +60 @@
-index 9e9fdc4144..f641e0dc36 100644
+index c5252035a0..7ad37b08d2 100644
@@ -62 +63 @@
-@@ -1338,8 +1338,10 @@ hns3vf_service_handler(void *param)
+@@ -1386,8 +1386,10 @@ hns3vf_service_handler(void *param)