[V3] app/testpmd: fix forward stats after clear stats command

Message ID 20200428115045.31391-1-huwei013@chinasoftinc.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series [V3] app/testpmd: fix forward stats after clear stats command |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/iol-intel-Performance fail Performance Testing issues
ci/iol-nxp-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/Intel-compilation fail Compilation issues

Commit Message

Wei Hu (Xavier) April 28, 2020, 11:50 a.m. UTC
  From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>

Currently, when running start/clear stats&xstats/stop command many times
based on testpmd application, there are incorrect forward Rx/Tx-packets
stats as below:
---------------------- Forward statistics for port 0  --------------
RX-packets: 18446744073709544808 RX-dropped: 0                <snip>
TX-packets: 18446744073709536616 TX-dropped: 0                <snip>
--------------------------------------------------------------------

The root cause as below:
1. The struct rte_port of testpmd.h has a member variable
   "struct rte_eth_stats stats" to store the last port statistics.
2. When runnig start command, it execute cmd_start_parsed ->
   start_packet_forwarding -> fwd_stats_reset, which call rte_eth_stats_get
   API function to save current port statistics.
3. When running stop command, it execute fwd_stats_display, which call
   rte_eth_stats_get to get current port statistics, and then minus last
   port statistics.
4. If we run clear stats or xstats after start command, then run stop,
   it may display above incorrect stats because the current Rx/Tx-packets
   is lower than the last saved RX/TX-packets(uint64_t overflow).

This patch fixes it by clearing last port statistics when executing
"clear stats/xstats" command.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
v2 -> v3:
	Replace memset operation with calling rte_eth_stats_get API
v1 -> v2:
	Update the title and the documentation
	(doc/guides/testpmd_app_ug/testpmd_funcs.rst) based on
	Ferruh Yigit's comment as below:
	http://patches.dpdk.org/patch/69252/
---
 app/test-pmd/config.c                       | 26 ++++++++++++++++++++-
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  2 +-
 2 files changed, 26 insertions(+), 2 deletions(-)
  

Comments

Ferruh Yigit May 5, 2020, 3:43 p.m. UTC | #1
On 4/28/2020 12:50 PM, Wei Hu (Xavier) wrote:
> From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>
> 
> Currently, when running start/clear stats&xstats/stop command many times
> based on testpmd application, there are incorrect forward Rx/Tx-packets
> stats as below:
> ---------------------- Forward statistics for port 0  --------------
> RX-packets: 18446744073709544808 RX-dropped: 0                <snip>
> TX-packets: 18446744073709536616 TX-dropped: 0                <snip>
> --------------------------------------------------------------------
> 
> The root cause as below:
> 1. The struct rte_port of testpmd.h has a member variable
>    "struct rte_eth_stats stats" to store the last port statistics.
> 2. When runnig start command, it execute cmd_start_parsed ->
>    start_packet_forwarding -> fwd_stats_reset, which call rte_eth_stats_get
>    API function to save current port statistics.
> 3. When running stop command, it execute fwd_stats_display, which call
>    rte_eth_stats_get to get current port statistics, and then minus last
>    port statistics.
> 4. If we run clear stats or xstats after start command, then run stop,
>    it may display above incorrect stats because the current Rx/Tx-packets
>    is lower than the last saved RX/TX-packets(uint64_t overflow).
> 
> This patch fixes it by clearing last port statistics when executing
> "clear stats/xstats" command.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/master, thanks.
  

Patch

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 72f25d152..712092fdb 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -234,11 +234,26 @@  nic_stats_display(portid_t port_id)
 void
 nic_stats_clear(portid_t port_id)
 {
+	int ret;
+
 	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
 		print_valid_ports();
 		return;
 	}
-	rte_eth_stats_reset(port_id);
+
+	ret = rte_eth_stats_reset(port_id);
+	if (ret != 0) {
+		printf("%s: Error: failed to reset stats (port %u): %s",
+		       __func__, port_id, strerror(ret));
+		return;
+	}
+
+	ret = rte_eth_stats_get(port_id, &ports[port_id].stats);
+	if (ret != 0) {
+		printf("%s: Error: failed to get stats (port %u): %s",
+		       __func__, port_id, strerror(ret));
+		return;
+	}
 	printf("\n  NIC statistics for port %d cleared\n", port_id);
 }
 
@@ -314,10 +329,19 @@  nic_xstats_clear(portid_t port_id)
 		print_valid_ports();
 		return;
 	}
+
 	ret = rte_eth_xstats_reset(port_id);
 	if (ret != 0) {
 		printf("%s: Error: failed to reset xstats (port %u): %s",
 		       __func__, port_id, strerror(ret));
+		return;
+	}
+
+	ret = rte_eth_stats_get(port_id, &ports[port_id].stats);
+	if (ret != 0) {
+		printf("%s: Error: failed to get stats (port %u): %s",
+		       __func__, port_id, strerror(ret));
+		return;
 	}
 }
 
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index a360ecccf..ca83a2ab5 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -237,7 +237,7 @@  Display the RSS hash functions and RSS hash key of a port::
 clear port
 ~~~~~~~~~~
 
-Clear the port statistics for a given port or for all ports::
+Clear the port statistics and forward engine statistics for a given port or for all ports::
 
    testpmd> clear port (info|stats|xstats|fdir|stat_qmap) (port_id|all)