Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/88116/?format=api
{ "id": 88116, "url": "https://patches.dpdk.org/api/patches/88116/?format=api", "web_url": "https://patches.dpdk.org/project/dpdk/patch/1614130139-42926-14-git-send-email-oulijun@huawei.com/", "project": { "id": 1, "url": "https://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<1614130139-42926-14-git-send-email-oulijun@huawei.com>", "list_archive_url": "https://inbox.dpdk.org/dev/1614130139-42926-14-git-send-email-oulijun@huawei.com", "date": "2021-02-24T01:28:59", "name": "[13/13] net/hns3: fix imprecise statistics", "commit_ref": null, "pull_url": null, "state": "changes-requested", "archived": true, "hash": "ed55b547e882e4c37a4f1b5e2b74f3e2a689a0ac", "submitter": { "id": 1675, "url": "https://patches.dpdk.org/api/people/1675/?format=api", "name": "Lijun Ou", "email": "oulijun@huawei.com" }, "delegate": { "id": 319, "url": "https://patches.dpdk.org/api/users/319/?format=api", "username": "fyigit", "first_name": "Ferruh", "last_name": "Yigit", "email": "ferruh.yigit@amd.com" }, "mbox": "https://patches.dpdk.org/project/dpdk/patch/1614130139-42926-14-git-send-email-oulijun@huawei.com/mbox/", "series": [ { "id": 15352, "url": "https://patches.dpdk.org/api/series/15352/?format=api", "web_url": "https://patches.dpdk.org/project/dpdk/list/?series=15352", "date": "2021-02-24T01:28:57", "name": "Features and bugfixes for hns3", "version": 1, "mbox": "https://patches.dpdk.org/series/15352/mbox/" } ], "comments": "https://patches.dpdk.org/api/patches/88116/comments/", "check": "success", "checks": "https://patches.dpdk.org/api/patches/88116/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@inbox.dpdk.org", "Delivered-To": "patchwork@inbox.dpdk.org", "Received": [ "from mails.dpdk.org (mails.dpdk.org [217.70.189.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id F41C6A034F;\n\tWed, 24 Feb 2021 02:29:00 +0100 (CET)", "from [217.70.189.124] (localhost [127.0.0.1])\n\tby mails.dpdk.org (Postfix) with ESMTP id 71EF1160738;\n\tWed, 24 Feb 2021 02:28:29 +0100 (CET)", "from szxga04-in.huawei.com (szxga04-in.huawei.com [45.249.212.190])\n by mails.dpdk.org (Postfix) with ESMTP id E76E240040\n for <dev@dpdk.org>; Wed, 24 Feb 2021 02:28:22 +0100 (CET)", "from DGGEMS414-HUB.china.huawei.com (unknown [172.30.72.58])\n by szxga04-in.huawei.com (SkyGuard) with ESMTP id 4DldYc0sqJz16C03;\n Wed, 24 Feb 2021 09:26:44 +0800 (CST)", "from localhost.localdomain (10.69.192.56) by\n DGGEMS414-HUB.china.huawei.com (10.3.19.214) with Microsoft SMTP Server id\n 14.3.498.0; Wed, 24 Feb 2021 09:28:17 +0800" ], "From": "Lijun Ou <oulijun@huawei.com>", "To": "<ferruh.yigit@intel.com>", "CC": "<dev@dpdk.org>, <linuxarm@openeuler.org>", "Date": "Wed, 24 Feb 2021 09:28:59 +0800", "Message-ID": "<1614130139-42926-14-git-send-email-oulijun@huawei.com>", "X-Mailer": "git-send-email 2.7.4", "In-Reply-To": "<1614130139-42926-1-git-send-email-oulijun@huawei.com>", "References": "<1614130139-42926-1-git-send-email-oulijun@huawei.com>", "MIME-Version": "1.0", "Content-Type": "text/plain", "X-Originating-IP": "[10.69.192.56]", "X-CFilter-Loop": "Reflected", "Subject": "[dpdk-dev] [PATCH 13/13] net/hns3: fix imprecise statistics", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n <mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n <mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "From: Chengchang Tang <tangchengchang@huawei.com>\n\nCurrently, the hns3 statistics may be inaccurate due to the\nfollowing two problems:\n\n1. Queue-level statistics are read from the firmware, and only\none Rx or Tx can be read at a time. This results in a large\ntime interval between reading multiple queues statistics in a\nstress scenario, such as 1280 queues used by a PF or 256 functions\nused at the same time. Especially when the 256 functions are used\nat the same time, the interval between every two firmware commands\nin a function can be huge, because the scheduling mechanism of the\nfirmware is similar to RR.\n\n2. The current statistics are read by type. The HW statistics are\nread first, and then the software statistics are read. Due to\npreceding reasons, HW reading may be time-consuming, which cause\na synchronization problem between SW and HW statistics of the same\nqueue.\n\nIn this patch, queue-level statistics are directly read from the bar\ninstead of the firmware, and all the statistics of a queue include HW\nand SW are read at a time to reduce inconsistency.\n\nFixes: 8839c5e202f3 (\"net/hns3: support device stats\")\nCc: stable@dpdk.org\n\nSigned-off-by: Chengchang Tang <tangchengchang@huawei.com>\nSigned-off-by: Lijun Ou <oulijun@huawei.com>\n---\n drivers/net/hns3/hns3_stats.c | 227 ++++++++++++++----------------------------\n 1 file changed, 76 insertions(+), 151 deletions(-)", "diff": "diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c\nindex 365a3eb..a0fd5bb 100644\n--- a/drivers/net/hns3/hns3_stats.c\n+++ b/drivers/net/hns3/hns3_stats.c\n@@ -367,7 +367,6 @@ static const struct hns3_xstats_name_offset hns3_imissed_stats_strings[] = {\n \t\t\t HNS3_NUM_RESET_XSTATS + HNS3_NUM_IMISSED_XSTATS)\n \n static void hns3_tqp_stats_clear(struct hns3_hw *hw);\n-static void hns3_tqp_basic_stats_clear(struct rte_eth_dev *dev);\n \n /*\n * Query all the MAC statistics data of Network ICL command ,opcode id: 0x0034.\n@@ -481,49 +480,6 @@ hns3_query_update_mac_stats(struct rte_eth_dev *dev)\n \treturn ret;\n }\n \n-/* Get tqp stats from register */\n-static int\n-hns3_update_tqp_stats(struct hns3_hw *hw)\n-{\n-\tstruct hns3_tqp_stats *stats = &hw->tqp_stats;\n-\tstruct hns3_cmd_desc desc;\n-\tuint64_t cnt;\n-\tuint16_t i;\n-\tint ret;\n-\n-\tfor (i = 0; i < hw->tqps_num; i++) {\n-\t\thns3_cmd_setup_basic_desc(&desc, HNS3_OPC_QUERY_RX_STATUS,\n-\t\t\t\t\t true);\n-\n-\t\tdesc.data[0] = rte_cpu_to_le_32((uint32_t)i);\n-\t\tret = hns3_cmd_send(hw, &desc, 1);\n-\t\tif (ret) {\n-\t\t\thns3_err(hw, \"Failed to query RX No.%u queue stat: %d\",\n-\t\t\t\t i, ret);\n-\t\t\treturn ret;\n-\t\t}\n-\t\tcnt = rte_le_to_cpu_32(desc.data[1]);\n-\t\tstats->rcb_rx_ring_pktnum_rcd += cnt;\n-\t\tstats->rcb_rx_ring_pktnum[i] += cnt;\n-\n-\t\thns3_cmd_setup_basic_desc(&desc, HNS3_OPC_QUERY_TX_STATUS,\n-\t\t\t\t\t true);\n-\n-\t\tdesc.data[0] = rte_cpu_to_le_32((uint32_t)i);\n-\t\tret = hns3_cmd_send(hw, &desc, 1);\n-\t\tif (ret) {\n-\t\t\thns3_err(hw, \"Failed to query TX No.%u queue stat: %d\",\n-\t\t\t\t i, ret);\n-\t\t\treturn ret;\n-\t\t}\n-\t\tcnt = rte_le_to_cpu_32(desc.data[1]);\n-\t\tstats->rcb_tx_ring_pktnum_rcd += cnt;\n-\t\tstats->rcb_tx_ring_pktnum[i] += cnt;\n-\t}\n-\n-\treturn 0;\n-}\n-\n static int\n hns3_update_rpu_drop_stats(struct hns3_hw *hw)\n {\n@@ -589,17 +545,11 @@ hns3_stats_get(struct rte_eth_dev *eth_dev, struct rte_eth_stats *rte_stats)\n \tstruct hns3_rx_missed_stats *imissed_stats = &hw->imissed_stats;\n \tstruct hns3_tqp_stats *stats = &hw->tqp_stats;\n \tstruct hns3_rx_queue *rxq;\n+\tstruct hns3_tx_queue *txq;\n \tuint64_t cnt;\n \tuint16_t i;\n \tint ret;\n \n-\t/* Update tqp stats by read register */\n-\tret = hns3_update_tqp_stats(hw);\n-\tif (ret) {\n-\t\thns3_err(hw, \"Update tqp stats fail : %d\", ret);\n-\t\treturn ret;\n-\t}\n-\n \tif (!hns->is_vf) {\n \t\t/* Update imissed stats */\n \t\tret = hns3_update_imissed_stats(hw, false);\n@@ -612,29 +562,41 @@ hns3_stats_get(struct rte_eth_dev *eth_dev, struct rte_eth_stats *rte_stats)\n \t\trte_stats->imissed = imissed_stats->rpu_rx_drop_cnt;\n \t}\n \n-\t/* Get the error stats and bytes of received packets */\n+\t/* Reads all the stats of a rxq in a loop to keep them synchronized */\n \tfor (i = 0; i < eth_dev->data->nb_rx_queues; i++) {\n \t\trxq = eth_dev->data->rx_queues[i];\n-\t\tif (rxq) {\n-\t\t\tcnt = rxq->err_stats.l2_errors +\n-\t\t\t\trxq->err_stats.pkt_len_errors;\n-\t\t\trte_stats->ierrors += cnt;\n+\t\tif (rxq == NULL)\n+\t\t\tcontinue;\n+\n+\t\tcnt = hns3_read_dev(rxq, HNS3_RING_RX_PKTNUM_RECORD_REG);\n+\t\t/*\n+\t\t * Read hardware and software in adjacent positions to minumize\n+\t\t * the timing variance.\n+\t\t */\n+\t\trte_stats->ierrors += rxq->err_stats.l2_errors +\n+\t\t\t\t\trxq->err_stats.pkt_len_errors;\n+\t\tstats->rcb_rx_ring_pktnum_rcd += cnt;\n+\t\tstats->rcb_rx_ring_pktnum[i] += cnt;\n \n #ifdef RTE_LIBRTE_HNS3_PMD_SOFT_COUNTERS\n-\t\t\trte_stats->ibytes += rxq->basic_stats.bytes;\n+\t\trte_stats->ibytes += rxq->basic_stats.bytes;\n #endif\n-\t\t}\n \t}\n \n-#ifdef RTE_LIBRTE_HNS3_PMD_SOFT_COUNTERS\n-\t/* Get the bytes of received packets */\n-\tstruct hns3_tx_queue *txq;\n+\t/* Reads all the stats of a txq in a loop to keep them synchronized */\n \tfor (i = 0; i < eth_dev->data->nb_tx_queues; i++) {\n \t\ttxq = eth_dev->data->tx_queues[i];\n-\t\tif (txq)\n-\t\t\trte_stats->obytes += txq->basic_stats.bytes;\n-\t}\n+\t\tif (txq == NULL)\n+\t\t\tcontinue;\n+\n+\t\tcnt = hns3_read_dev(txq, HNS3_RING_TX_PKTNUM_RECORD_REG);\n+\t\tstats->rcb_tx_ring_pktnum_rcd += cnt;\n+\t\tstats->rcb_tx_ring_pktnum[i] += cnt;\n+\n+#ifdef RTE_LIBRTE_HNS3_PMD_SOFT_COUNTERS\n+\t\trte_stats->obytes += txq->basic_stats.bytes;\n #endif\n+\t}\n \n \trte_stats->oerrors = 0;\n \t/*\n@@ -657,37 +619,11 @@ hns3_stats_reset(struct rte_eth_dev *eth_dev)\n {\n \tstruct hns3_adapter *hns = eth_dev->data->dev_private;\n \tstruct hns3_hw *hw = &hns->hw;\n-\tstruct hns3_cmd_desc desc_reset;\n \tstruct hns3_rx_queue *rxq;\n+\tstruct hns3_tx_queue *txq;\n \tuint16_t i;\n \tint ret;\n \n-\t/*\n-\t * Note: Reading hardware statistics of rx/tx queue packet number\n-\t * will clear them.\n-\t */\n-\tfor (i = 0; i < hw->tqps_num; i++) {\n-\t\thns3_cmd_setup_basic_desc(&desc_reset, HNS3_OPC_QUERY_RX_STATUS,\n-\t\t\t\t\t true);\n-\t\tdesc_reset.data[0] = rte_cpu_to_le_32((uint32_t)i);\n-\t\tret = hns3_cmd_send(hw, &desc_reset, 1);\n-\t\tif (ret) {\n-\t\t\thns3_err(hw, \"Failed to reset RX No.%u queue stat: %d\",\n-\t\t\t\t i, ret);\n-\t\t\treturn ret;\n-\t\t}\n-\n-\t\thns3_cmd_setup_basic_desc(&desc_reset, HNS3_OPC_QUERY_TX_STATUS,\n-\t\t\t\t\t true);\n-\t\tdesc_reset.data[0] = rte_cpu_to_le_32((uint32_t)i);\n-\t\tret = hns3_cmd_send(hw, &desc_reset, 1);\n-\t\tif (ret) {\n-\t\t\thns3_err(hw, \"Failed to reset TX No.%u queue stat: %d\",\n-\t\t\t\t i, ret);\n-\t\t\treturn ret;\n-\t\t}\n-\t}\n-\n \tif (!hns->is_vf) {\n \t\t/*\n \t\t * Note: Reading hardware statistics of imissed registers will\n@@ -701,25 +637,44 @@ hns3_stats_reset(struct rte_eth_dev *eth_dev)\n \t\t}\n \t}\n \n-\t/*\n-\t * Clear soft stats of rx error packet which will be dropped\n-\t * in driver.\n-\t */\n \tfor (i = 0; i < eth_dev->data->nb_rx_queues; i++) {\n \t\trxq = eth_dev->data->rx_queues[i];\n-\t\tif (rxq) {\n-\t\t\trxq->err_stats.pkt_len_errors = 0;\n-\t\t\trxq->err_stats.l2_errors = 0;\n-\t\t}\n+\t\tif (rxq == NULL)\n+\t\t\tcontinue;\n+\n+\t\trxq->err_stats.pkt_len_errors = 0;\n+\t\trxq->err_stats.l2_errors = 0;\n+\t}\n+\n+\t/* Clear all the stats of a rxq in a loop to keep them synchronized */\n+\tfor (i = 0; i < eth_dev->data->nb_rx_queues; i++) {\n+\t\trxq = eth_dev->data->rx_queues[i];\n+\t\tif (rxq == NULL)\n+\t\t\tcontinue;\n+\n+\t\tmemset(&rxq->basic_stats, 0,\n+\t\t\t\tsizeof(struct hns3_rx_basic_stats));\n+\n+\t\t/* This register is read-clear */\n+\t\t(void)hns3_read_dev(rxq, HNS3_RING_RX_PKTNUM_RECORD_REG);\n+\t\trxq->err_stats.pkt_len_errors = 0;\n+\t\trxq->err_stats.l2_errors = 0;\n+\t}\n+\n+\t/* Clear all the stats of a txq in a loop to keep them synchronized */\n+\tfor (i = 0; i < eth_dev->data->nb_tx_queues; i++) {\n+\t\ttxq = eth_dev->data->tx_queues[i];\n+\t\tif (txq == NULL)\n+\t\t\tcontinue;\n+\n+\t\tmemset(&txq->basic_stats, 0,\n+\t\t\t\tsizeof(struct hns3_tx_basic_stats));\n+\n+\t\t/* This register is read-clear */\n+\t\t(void)hns3_read_dev(txq, HNS3_RING_TX_PKTNUM_RECORD_REG);\n \t}\n \n-\t/*\n-\t * 'packets' in hns3_tx_basic_stats and hns3_rx_basic_stats come\n-\t * from hw->tqp_stats. And clearing tqp stats is like clearing\n-\t * their source.\n-\t */\n \thns3_tqp_stats_clear(hw);\n-\thns3_tqp_basic_stats_clear(eth_dev);\n \n \treturn 0;\n }\n@@ -885,6 +840,7 @@ hns3_rxq_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,\n \tstruct hns3_rx_basic_stats *rxq_stats;\n \tstruct hns3_rx_queue *rxq;\n \tuint16_t i, j;\n+\tuint32_t cnt;\n \tchar *val;\n \n \tfor (i = 0; i < dev->data->nb_rx_queues; i++) {\n@@ -892,9 +848,17 @@ hns3_rxq_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,\n \t\tif (rxq == NULL)\n \t\t\tcontinue;\n \n+\t\tcnt = hns3_read_dev(rxq, HNS3_RING_RX_PKTNUM_RECORD_REG);\n+\t\t/*\n+\t\t * Read hardware and software in adjacent positions to minimize\n+\t\t * the time difference.\n+\t\t */\n \t\trxq_stats = &rxq->basic_stats;\n \t\trxq_stats->errors = rxq->err_stats.l2_errors +\n \t\t\t\t\trxq->err_stats.pkt_len_errors;\n+\t\tstats->rcb_rx_ring_pktnum_rcd += cnt;\n+\t\tstats->rcb_rx_ring_pktnum[i] += cnt;\n+\n \t\t/*\n \t\t * If HW statistics are reset by stats_reset, but a lot of\n \t\t * residual packets exist in the hardware queue and these\n@@ -923,6 +887,7 @@ hns3_txq_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,\n \tstruct hns3_tx_basic_stats *txq_stats;\n \tstruct hns3_tx_queue *txq;\n \tuint16_t i, j;\n+\tuint32_t cnt;\n \tchar *val;\n \n \tfor (i = 0; i < dev->data->nb_tx_queues; i++) {\n@@ -930,6 +895,10 @@ hns3_txq_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,\n \t\tif (txq == NULL)\n \t\t\tcontinue;\n \n+\t\tcnt = hns3_read_dev(txq, HNS3_RING_TX_PKTNUM_RECORD_REG);\n+\t\tstats->rcb_tx_ring_pktnum_rcd += cnt;\n+\t\tstats->rcb_tx_ring_pktnum[i] += cnt;\n+\n \t\ttxq_stats = &txq->basic_stats;\n \t\ttxq_stats->packets = stats->rcb_tx_ring_pktnum[i];\n \n@@ -943,54 +912,12 @@ hns3_txq_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,\n \t}\n }\n \n-static int\n+static void\n hns3_tqp_basic_stats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,\n \t\t\t int *count)\n {\n-\tstruct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);\n-\tint ret;\n-\n-\t/* Update tqp stats by read register */\n-\tret = hns3_update_tqp_stats(hw);\n-\tif (ret) {\n-\t\thns3_err(hw, \"Update tqp stats fail, ret = %d.\", ret);\n-\t\treturn ret;\n-\t}\n-\n \thns3_rxq_basic_stats_get(dev, xstats, count);\n \thns3_txq_basic_stats_get(dev, xstats, count);\n-\n-\treturn 0;\n-}\n-\n-/*\n- * The function is only called by hns3_dev_xstats_reset to clear\n- * basic stats of per-queue. TQP stats are all cleared in hns3_stats_reset\n- * which is called before this function.\n- *\n- * @param dev\n- * Pointer to Ethernet device.\n- */\n-static void\n-hns3_tqp_basic_stats_clear(struct rte_eth_dev *dev)\n-{\n-\tstruct hns3_tx_queue *txq;\n-\tstruct hns3_rx_queue *rxq;\n-\tuint16_t i;\n-\n-\tfor (i = 0; i < dev->data->nb_rx_queues; i++) {\n-\t\trxq = dev->data->rx_queues[i];\n-\t\tif (rxq)\n-\t\t\tmemset(&rxq->basic_stats, 0,\n-\t\t\t sizeof(struct hns3_rx_basic_stats));\n-\t}\n-\n-\tfor (i = 0; i < dev->data->nb_tx_queues; i++) {\n-\t\ttxq = dev->data->tx_queues[i];\n-\t\tif (txq)\n-\t\t\tmemset(&txq->basic_stats, 0,\n-\t\t\t sizeof(struct hns3_tx_basic_stats));\n-\t}\n }\n \n /*\n@@ -1032,9 +959,7 @@ hns3_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,\n \n \tcount = 0;\n \n-\tret = hns3_tqp_basic_stats_get(dev, xstats, &count);\n-\tif (ret < 0)\n-\t\treturn ret;\n+\thns3_tqp_basic_stats_get(dev, xstats, &count);\n \n \tif (!hns->is_vf) {\n \t\t/* Update Mac stats */\n", "prefixes": [ "13/13" ] }