DPDK patches and discussions
 help / color / mirror / Atom feed
From: Lijun Ou <oulijun@huawei.com>
To: <ferruh.yigit@intel.com>
Cc: <dev@dpdk.org>, <linuxarm@openeuler.org>
Subject: [dpdk-dev] [PATCH V3 14/14] net/hns3: fix imprecise statistics
Date: Thu, 4 Mar 2021 15:44:54 +0800	[thread overview]
Message-ID: <1614843894-43845-15-git-send-email-oulijun@huawei.com> (raw)
In-Reply-To: <1614843894-43845-1-git-send-email-oulijun@huawei.com>

From: Chengchang Tang <tangchengchang@huawei.com>

Currently, the hns3 statistics may be inaccurate due to the
following two problems:

1. Queue-level statistics are read from the firmware, and only
one Rx or Tx can be read at a time. This results in a large
time interval between reading multiple queues statistics in a
stress scenario, such as 1280 queues used by a PF or 256 functions
used at the same time. Especially when the 256 functions are used
at the same time, the interval between every two firmware commands
in a function can be huge, because the scheduling mechanism of the
firmware is similar to RR.

2. The current statistics are read by type. The HW statistics are
read first, and then the software statistics are read. Due to
preceding reasons, HW reading may be time-consuming, which cause
a synchronization problem between SW and HW statistics of the same
queue.

In this patch, queue-level statistics are directly read from the bar
instead of the firmware, and all the statistics of a queue include HW
and SW are read at a time to reduce inconsistency.

Fixes: 8839c5e202f3 ("net/hns3: support device stats")
Cc: stable@dpdk.org

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_stats.c | 221 ++++++++++++++----------------------------
 1 file changed, 72 insertions(+), 149 deletions(-)

diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c
index 87035e3..941c75f 100644
--- a/drivers/net/hns3/hns3_stats.c
+++ b/drivers/net/hns3/hns3_stats.c
@@ -367,7 +367,6 @@ static const struct hns3_xstats_name_offset hns3_imissed_stats_strings[] = {
 			    HNS3_NUM_RESET_XSTATS + HNS3_NUM_IMISSED_XSTATS)
 
 static void hns3_tqp_stats_clear(struct hns3_hw *hw);
-static void hns3_tqp_basic_stats_clear(struct rte_eth_dev *dev);
 
 /*
  * Query all the MAC statistics data of Network ICL command ,opcode id: 0x0034.
@@ -481,49 +480,6 @@ hns3_query_update_mac_stats(struct rte_eth_dev *dev)
 	return ret;
 }
 
-/* Get tqp stats from register */
-static int
-hns3_update_tqp_stats(struct hns3_hw *hw)
-{
-	struct hns3_tqp_stats *stats = &hw->tqp_stats;
-	struct hns3_cmd_desc desc;
-	uint64_t cnt;
-	uint16_t i;
-	int ret;
-
-	for (i = 0; i < hw->tqps_num; i++) {
-		hns3_cmd_setup_basic_desc(&desc, HNS3_OPC_QUERY_RX_STATUS,
-					  true);
-
-		desc.data[0] = rte_cpu_to_le_32((uint32_t)i);
-		ret = hns3_cmd_send(hw, &desc, 1);
-		if (ret) {
-			hns3_err(hw, "Failed to query RX No.%u queue stat: %d",
-				 i, ret);
-			return ret;
-		}
-		cnt = rte_le_to_cpu_32(desc.data[1]);
-		stats->rcb_rx_ring_pktnum_rcd += cnt;
-		stats->rcb_rx_ring_pktnum[i] += cnt;
-
-		hns3_cmd_setup_basic_desc(&desc, HNS3_OPC_QUERY_TX_STATUS,
-					  true);
-
-		desc.data[0] = rte_cpu_to_le_32((uint32_t)i);
-		ret = hns3_cmd_send(hw, &desc, 1);
-		if (ret) {
-			hns3_err(hw, "Failed to query TX No.%u queue stat: %d",
-				 i, ret);
-			return ret;
-		}
-		cnt = rte_le_to_cpu_32(desc.data[1]);
-		stats->rcb_tx_ring_pktnum_rcd += cnt;
-		stats->rcb_tx_ring_pktnum[i] += cnt;
-	}
-
-	return 0;
-}
-
 static int
 hns3_update_rpu_drop_stats(struct hns3_hw *hw)
 {
@@ -589,17 +545,11 @@ hns3_stats_get(struct rte_eth_dev *eth_dev, struct rte_eth_stats *rte_stats)
 	struct hns3_rx_missed_stats *imissed_stats = &hw->imissed_stats;
 	struct hns3_tqp_stats *stats = &hw->tqp_stats;
 	struct hns3_rx_queue *rxq;
+	struct hns3_tx_queue *txq;
 	uint64_t cnt;
 	uint16_t i;
 	int ret;
 
-	/* Update tqp stats by read register */
-	ret = hns3_update_tqp_stats(hw);
-	if (ret) {
-		hns3_err(hw, "Update tqp stats fail : %d", ret);
-		return ret;
-	}
-
 	if (!hns->is_vf) {
 		/* Update imissed stats */
 		ret = hns3_update_imissed_stats(hw, false);
@@ -612,24 +562,34 @@ hns3_stats_get(struct rte_eth_dev *eth_dev, struct rte_eth_stats *rte_stats)
 		rte_stats->imissed = imissed_stats->rpu_rx_drop_cnt;
 	}
 
-	/* Get the error stats and bytes of received packets */
+	/* Reads all the stats of a rxq in a loop to keep them synchronized */
 	for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
 		rxq = eth_dev->data->rx_queues[i];
-		if (rxq) {
-			cnt = rxq->err_stats.l2_errors +
-				rxq->err_stats.pkt_len_errors;
-			rte_stats->ierrors += cnt;
+		if (rxq == NULL)
+			continue;
 
-			rte_stats->ibytes += rxq->basic_stats.bytes;
-		}
+		cnt = hns3_read_dev(rxq, HNS3_RING_RX_PKTNUM_RECORD_REG);
+		/*
+		 * Read hardware and software in adjacent positions to minumize
+		 * the timing variance.
+		 */
+		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;
 	}
 
-	/* Get the bytes of received packets */
-	struct hns3_tx_queue *txq;
+	/* Reads all the stats of a txq in a loop to keep them synchronized */
 	for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
 		txq = eth_dev->data->tx_queues[i];
-		if (txq)
-			rte_stats->obytes += txq->basic_stats.bytes;
+		if (txq == NULL)
+			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_stats->obytes += txq->basic_stats.bytes;
 	}
 
 	rte_stats->oerrors = 0;
@@ -653,37 +613,11 @@ hns3_stats_reset(struct rte_eth_dev *eth_dev)
 {
 	struct hns3_adapter *hns = eth_dev->data->dev_private;
 	struct hns3_hw *hw = &hns->hw;
-	struct hns3_cmd_desc desc_reset;
 	struct hns3_rx_queue *rxq;
+	struct hns3_tx_queue *txq;
 	uint16_t i;
 	int ret;
 
-	/*
-	 * Note: Reading hardware statistics of rx/tx queue packet number
-	 * will clear them.
-	 */
-	for (i = 0; i < hw->tqps_num; i++) {
-		hns3_cmd_setup_basic_desc(&desc_reset, HNS3_OPC_QUERY_RX_STATUS,
-					  true);
-		desc_reset.data[0] = rte_cpu_to_le_32((uint32_t)i);
-		ret = hns3_cmd_send(hw, &desc_reset, 1);
-		if (ret) {
-			hns3_err(hw, "Failed to reset RX No.%u queue stat: %d",
-				 i, ret);
-			return ret;
-		}
-
-		hns3_cmd_setup_basic_desc(&desc_reset, HNS3_OPC_QUERY_TX_STATUS,
-					  true);
-		desc_reset.data[0] = rte_cpu_to_le_32((uint32_t)i);
-		ret = hns3_cmd_send(hw, &desc_reset, 1);
-		if (ret) {
-			hns3_err(hw, "Failed to reset TX No.%u queue stat: %d",
-				 i, ret);
-			return ret;
-		}
-	}
-
 	if (!hns->is_vf) {
 		/*
 		 * Note: Reading hardware statistics of imissed registers will
@@ -697,25 +631,44 @@ hns3_stats_reset(struct rte_eth_dev *eth_dev)
 		}
 	}
 
-	/*
-	 * Clear soft stats of rx error packet which will be dropped
-	 * in driver.
-	 */
 	for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
 		rxq = eth_dev->data->rx_queues[i];
-		if (rxq) {
-			rxq->err_stats.pkt_len_errors = 0;
-			rxq->err_stats.l2_errors = 0;
-		}
+		if (rxq == NULL)
+			continue;
+
+		rxq->err_stats.pkt_len_errors = 0;
+		rxq->err_stats.l2_errors = 0;
+	}
+
+	/* Clear all the stats of a rxq in a loop to keep them synchronized */
+	for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
+		rxq = eth_dev->data->rx_queues[i];
+		if (rxq == NULL)
+			continue;
+
+		memset(&rxq->basic_stats, 0,
+				sizeof(struct hns3_rx_basic_stats));
+
+		/* This register is read-clear */
+		(void)hns3_read_dev(rxq, HNS3_RING_RX_PKTNUM_RECORD_REG);
+		rxq->err_stats.pkt_len_errors = 0;
+		rxq->err_stats.l2_errors = 0;
+	}
+
+	/* Clear all the stats of a txq in a loop to keep them synchronized */
+	for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
+		txq = eth_dev->data->tx_queues[i];
+		if (txq == NULL)
+			continue;
+
+		memset(&txq->basic_stats, 0,
+				sizeof(struct hns3_tx_basic_stats));
+
+		/* This register is read-clear */
+		(void)hns3_read_dev(txq, HNS3_RING_TX_PKTNUM_RECORD_REG);
 	}
 
-	/*
-	 * 'packets' in hns3_tx_basic_stats and hns3_rx_basic_stats come
-	 * from hw->tqp_stats. And clearing tqp stats is like clearing
-	 * their source.
-	 */
 	hns3_tqp_stats_clear(hw);
-	hns3_tqp_basic_stats_clear(eth_dev);
 
 	return 0;
 }
@@ -881,6 +834,7 @@ hns3_rxq_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	struct hns3_rx_basic_stats *rxq_stats;
 	struct hns3_rx_queue *rxq;
 	uint16_t i, j;
+	uint32_t cnt;
 	char *val;
 
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
@@ -888,9 +842,17 @@ hns3_rxq_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		if (rxq == NULL)
 			continue;
 
+		cnt = hns3_read_dev(rxq, HNS3_RING_RX_PKTNUM_RECORD_REG);
+		/*
+		 * Read hardware and software in adjacent positions to minimize
+		 * the time difference.
+		 */
 		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;
+
 		/*
 		 * If HW statistics are reset by stats_reset, but a lot of
 		 * residual packets exist in the hardware queue and these
@@ -919,6 +881,7 @@ hns3_txq_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	struct hns3_tx_basic_stats *txq_stats;
 	struct hns3_tx_queue *txq;
 	uint16_t i, j;
+	uint32_t cnt;
 	char *val;
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
@@ -926,6 +889,10 @@ hns3_txq_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		if (txq == NULL)
 			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;
+
 		txq_stats = &txq->basic_stats;
 		txq_stats->packets = stats->rcb_tx_ring_pktnum[i];
 
@@ -939,54 +906,12 @@ hns3_txq_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	}
 }
 
-static int
+static void
 hns3_tqp_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 			 int *count)
 {
-	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	int ret;
-
-	/* Update tqp stats by read register */
-	ret = hns3_update_tqp_stats(hw);
-	if (ret) {
-		hns3_err(hw, "Update tqp stats fail, ret = %d.", ret);
-		return ret;
-	}
-
 	hns3_rxq_basic_stats_get(dev, xstats, count);
 	hns3_txq_basic_stats_get(dev, xstats, count);
-
-	return 0;
-}
-
-/*
- * The function is only called by hns3_dev_xstats_reset to clear
- * basic stats of per-queue. TQP stats are all cleared in hns3_stats_reset
- * which is called before this function.
- *
- * @param dev
- *   Pointer to Ethernet device.
- */
-static void
-hns3_tqp_basic_stats_clear(struct rte_eth_dev *dev)
-{
-	struct hns3_tx_queue *txq;
-	struct hns3_rx_queue *rxq;
-	uint16_t i;
-
-	for (i = 0; i < dev->data->nb_rx_queues; i++) {
-		rxq = dev->data->rx_queues[i];
-		if (rxq)
-			memset(&rxq->basic_stats, 0,
-			       sizeof(struct hns3_rx_basic_stats));
-	}
-
-	for (i = 0; i < dev->data->nb_tx_queues; i++) {
-		txq = dev->data->tx_queues[i];
-		if (txq)
-			memset(&txq->basic_stats, 0,
-			       sizeof(struct hns3_tx_basic_stats));
-	}
 }
 
 /*
@@ -1028,9 +953,7 @@ hns3_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 
 	count = 0;
 
-	ret = hns3_tqp_basic_stats_get(dev, xstats, &count);
-	if (ret < 0)
-		return ret;
+	hns3_tqp_basic_stats_get(dev, xstats, &count);
 
 	if (!hns->is_vf) {
 		/* Update Mac stats */
-- 
2.7.4


  parent reply	other threads:[~2021-03-04  7:44 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-24  1:28 [dpdk-dev] [PATCH 00/13] Features and bugfixes for hns3 Lijun Ou
2021-02-24  1:28 ` [dpdk-dev] [PATCH 01/13] net/hns3: support module EEPROM dump Lijun Ou
2021-02-24  1:28 ` [dpdk-dev] [PATCH 02/13] net/hns3: add more registers to dump Lijun Ou
2021-02-24  1:28 ` [dpdk-dev] [PATCH 03/13] net/hns3: implement cleanup for Tx done Lijun Ou
2021-02-24  1:28 ` [dpdk-dev] [PATCH 04/13] net/hns3: add Rx and Tx bytes stats Lijun Ou
2021-02-26 15:25   ` Ferruh Yigit
2021-02-24  1:28 ` [dpdk-dev] [PATCH 05/13] net/hns3: add imissed packet stats Lijun Ou
2021-02-24  1:28 ` [dpdk-dev] [PATCH 06/13] net/hns3: encapsulate a port shaping interface Lijun Ou
2021-02-24  1:28 ` [dpdk-dev] [PATCH 07/13] net/hns3: support PF on electrical net device Lijun Ou
2021-02-26 15:25   ` Ferruh Yigit
2021-03-01 14:17     ` oulijun
2021-03-01 14:44       ` Ferruh Yigit
2021-02-24  1:28 ` [dpdk-dev] [PATCH 08/13] net/hns3: support RXD advanced layout Lijun Ou
2021-02-24  1:28 ` [dpdk-dev] [PATCH 09/13] net/hns3: fix maximum frame size update after buffer alloc Lijun Ou
2021-02-26 15:25   ` Ferruh Yigit
2021-02-27  3:56     ` oulijun
2021-03-03 13:27   ` Ferruh Yigit
2021-02-24  1:28 ` [dpdk-dev] [PATCH 10/13] net/hns3: remove unused parameter from func declaration Lijun Ou
2021-02-24  1:28 ` [dpdk-dev] [PATCH 11/13] net/hns3: fix memory leakage for mbuf Lijun Ou
2021-02-24  1:28 ` [dpdk-dev] [PATCH 12/13] net/hns3: add process for MAC interrupt Lijun Ou
2021-02-26 15:26   ` Ferruh Yigit
2021-02-27  9:24     ` oulijun
2021-02-24  1:28 ` [dpdk-dev] [PATCH 13/13] net/hns3: fix imprecise statistics Lijun Ou
2021-03-02 13:58 ` [dpdk-dev] [PATCH V2 00/14] Features and bugfixes for hns3 Lijun Ou
2021-03-02 13:58   ` [dpdk-dev] [PATCH V2 01/14] net/hns3: support module EEPROM dump Lijun Ou
2021-03-03 13:26     ` Ferruh Yigit
2021-03-03 13:38       ` oulijun
2021-03-02 13:58   ` [dpdk-dev] [PATCH V2 02/14] net/hns3: add more registers to dump Lijun Ou
2021-03-02 13:58   ` [dpdk-dev] [PATCH V2 03/14] net/hns3: implement cleanup for Tx done Lijun Ou
2021-03-03 13:27     ` Ferruh Yigit
2021-03-02 13:58   ` [dpdk-dev] [PATCH V2 04/14] net/hns3: add Rx and Tx bytes stats Lijun Ou
2021-03-03 13:28     ` Ferruh Yigit
2021-03-03 14:08       ` [dpdk-dev] [Linuxarm] " oulijun
2021-03-03 14:24         ` Ferruh Yigit
2021-03-04  1:36           ` oulijun
2021-03-02 13:58   ` [dpdk-dev] [PATCH V2 05/14] net/hns3: add imissed packet stats Lijun Ou
2021-03-02 13:58   ` [dpdk-dev] [PATCH V2 06/14] net/hns3: encapsulate a port shaping interface Lijun Ou
2021-03-02 13:58   ` [dpdk-dev] [PATCH V2 07/14] net/hns3: fix device capabilities for copper media type Lijun Ou
2021-03-03 13:27     ` Ferruh Yigit
2021-03-03 13:51       ` oulijun
2021-03-03 13:58         ` Ferruh Yigit
2021-03-02 13:58   ` [dpdk-dev] [PATCH V2 08/14] net/hns3: support PF device with copper phys Lijun Ou
2021-03-02 13:58   ` [dpdk-dev] [PATCH V2 09/14] net/hns3: support RXD advanced layout Lijun Ou
2021-03-02 13:58   ` [dpdk-dev] [PATCH V2 10/14] net/hns3: fix maximum frame size update after buffer alloc Lijun Ou
2021-03-02 13:58   ` [dpdk-dev] [PATCH V2 11/14] net/hns3: remove unused parameter from func declaration Lijun Ou
2021-03-02 13:58   ` [dpdk-dev] [PATCH V2 12/14] net/hns3: fix memory leakage for mbuf Lijun Ou
2021-03-02 13:58   ` [dpdk-dev] [PATCH V2 13/14] net/hns3: add process for MAC interrupt Lijun Ou
2021-03-02 13:58   ` [dpdk-dev] [PATCH V2 14/14] net/hns3: fix imprecise statistics Lijun Ou
2021-03-04  7:44   ` [dpdk-dev] [PATCH V3 00/14] Features and bugfixes for hns3 Lijun Ou
2021-03-04  7:44     ` [dpdk-dev] [PATCH V3 01/14] net/hns3: support module EEPROM dump Lijun Ou
2021-03-04  7:44     ` [dpdk-dev] [PATCH V3 02/14] net/hns3: add more registers to dump Lijun Ou
2021-03-04  7:44     ` [dpdk-dev] [PATCH V3 03/14] net/hns3: implement Tx mbuf free on demand Lijun Ou
2021-03-04  7:44     ` [dpdk-dev] [PATCH V3 04/14] net/hns3: add Rx and Tx bytes stats Lijun Ou
2021-03-04  7:44     ` [dpdk-dev] [PATCH V3 05/14] net/hns3: add imissed packet stats Lijun Ou
2021-03-04  7:44     ` [dpdk-dev] [PATCH V3 06/14] net/hns3: encapsulate a port shaping interface Lijun Ou
2021-03-04  7:44     ` [dpdk-dev] [PATCH V3 07/14] net/hns3: fix device capabilities for copper media type Lijun Ou
2021-03-04  7:44     ` [dpdk-dev] [PATCH V3 08/14] net/hns3: support PF device with copper phys Lijun Ou
2021-03-04  7:44     ` [dpdk-dev] [PATCH V3 09/14] net/hns3: support RXD advanced layout Lijun Ou
2021-03-04  7:44     ` [dpdk-dev] [PATCH V3 10/14] net/hns3: fix HW buffer size on MTU update Lijun Ou
2021-03-04  7:44     ` [dpdk-dev] [PATCH V3 11/14] net/hns3: remove unused parameter from func declaration Lijun Ou
2021-03-04  7:44     ` [dpdk-dev] [PATCH V3 12/14] net/hns3: fix memory leakage for mbuf Lijun Ou
2021-03-04  7:44     ` [dpdk-dev] [PATCH V3 13/14] net/hns3: add process for MAC interrupt Lijun Ou
2021-03-04  7:44     ` Lijun Ou [this message]
2021-03-04 14:10     ` [dpdk-dev] [PATCH V3 00/14] Features and bugfixes for hns3 Ferruh Yigit

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=1614843894-43845-15-git-send-email-oulijun@huawei.com \
    --to=oulijun@huawei.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=linuxarm@openeuler.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).