From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id C7749A00C5; Sun, 26 Apr 2020 11:22:24 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3CF401BF72; Sun, 26 Apr 2020 11:22:24 +0200 (CEST) Received: from m12-18.163.com (m12-18.163.com [220.181.12.18]) by dpdk.org (Postfix) with ESMTP id 369581BF70 for ; Sun, 26 Apr 2020 11:22:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=Subject:From:Message-ID:Date:MIME-Version; bh=hnzjy P/8ol5dPReW9mMi3WcaIwFcSy/1u4trnTd3XRY=; b=jEz4q3tc5hgJkkH8UWa3C zfeWTZbhSj9NTYnW0tbkIekqTS8tVIfB/zlUO5jNR49IBiyioWUK2IcIHBb7s8TV xrWTdTQP2x06/VaF03lACVi854GWq7mqm+NWncamv+rkLsvVkzAsuW/EOWLbQ7zS QTX1u1krkL8ic1cB9pz1dg= Received: from [192.168.1.199] (unknown [139.159.243.11]) by smtp14 (Coremail) with SMTP id EsCowABXfgLIUqVemMEkAA--.538S2; Sun, 26 Apr 2020 17:22:17 +0800 (CST) To: Ferruh Yigit , dev@dpdk.org References: <20200424110750.42456-1-huwei013@chinasoftinc.com> <1d83b9c8-a722-48db-18e5-b01a4bb382bb@intel.com> From: "Wei Hu (Xavier)" Message-ID: Date: Sun, 26 Apr 2020 17:22:16 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <1d83b9c8-a722-48db-18e5-b01a4bb382bb@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-CM-TRANSID: EsCowABXfgLIUqVemMEkAA--.538S2 X-Coremail-Antispam: 1Uf129KBjvJXoWxAr1UWF1UJry3tw1DtFW5KFg_yoWrAr4kpF s3Ga12kFW8ZF17Xr17Ja4j9r4UKrs5JrWUJrWfJa4xC3Z0yryrZr18KrWkZr97Gry8AryF va1DGFZrAFZ8KFDanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07jNDGOUUUUU= X-Originating-IP: [139.159.243.11] X-CM-SenderInfo: 50dyxv5ubk34lhl6il2tof0z/xtbBzwkSo1aD7AG3dQAAsN Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix Rx/Tx stats after clear stats command X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi, Ferruh Yigit On 2020/4/25 0:12, Ferruh Yigit wrote: > On 4/24/2020 12:07 PM, Wei Hu (Xavier) wrote: >> From: Chengwen Feng >> >> Currently, when running start/clear stats&xstats/stop command many times >> based on testpmd application, there are incorrect RX/TX-packets stats as >> below: >> ---------------------- Forward statistics for port 0 -------------- >> RX-packets: 18446744073709544808 RX-dropped: 0 ...ignore >> TX-packets: 18446744073709536616 TX-dropped: 0 ...ignore >> -------------------------------------------------------------------- >> >> 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). > > Looks like valid issue. > > Can you please update the title to mention this fixes the forward stats (to > prevent the misunderstanding that issue is in the port stats). > > Also can you please update the documentation > (doc/guides/testpmd_app_ug/testpmd_funcs.rst), "clear port" command to say this > will also affect the forward stats output (show fwd)? > >> >> 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 >> Signed-off-by: Wei Hu (Xavier) >> --- >> app/test-pmd/config.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c >> index 72f25d152..0d2375607 100644 >> --- a/app/test-pmd/config.c >> +++ b/app/test-pmd/config.c >> @@ -234,10 +234,16 @@ nic_stats_display(portid_t port_id) >> void >> nic_stats_clear(portid_t port_id) >> { >> + struct rte_port *port; >> + >> if (port_id_is_invalid(port_id, ENABLED_WARN)) { >> print_valid_ports(); >> return; >> } >> + >> + port = &ports[port_id]; >> + /* clear last port statistics because eth stats reset */ >> + memset(&port->stats, 0, sizeof(port->stats)); > > "clear fwd stats" command does same thing in "fwd_stats_reset()" as: > rte_eth_stats_get(pt_id, &ports[pt_id].stats); > > I suggest doing same here for consistency, but it should be after > 'rte_eth_stats_reset()' in that case. > I will modify it as follows, is it consistent with your comment? Thanks. void fwd_stats_reset(void) { streamid_t sm_id; portid_t pt_id; int i; for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) { pt_id = fwd_ports_ids[i]; - rte_eth_stats_get(pt_id, &ports[pt_id].stats); + rte_eth_stats_reset(port_id); + meset(&ports[pt_id].stats, 0, sizeof(ports[pt_id].stats)); } for (sm_id = 0; sm_id < cur_fwd_config.nb_fwd_streams; sm_id++) { struct fwd_stream *fs = fwd_streams[sm_id]; } } Regards Xavier >> rte_eth_stats_reset(port_id); >> printf("\n NIC statistics for port %d cleared\n", port_id); >> } >> @@ -308,12 +314,17 @@ nic_xstats_display(portid_t port_id) >> void >> nic_xstats_clear(portid_t port_id) >> { >> + struct rte_port *port; >> int ret; >> >> if (port_id_is_invalid(port_id, ENABLED_WARN)) { >> print_valid_ports(); >> return; >> } >> + >> + port = &ports[port_id]; >> + /* clear last port statistics because eth xstats(include stats) reset */ >> + memset(&port->stats, 0, sizeof(port->stats)); >> ret = rte_eth_xstats_reset(port_id); >> if (ret != 0) { >> printf("%s: Error: failed to reset xstats (port %u): %s", >>