* [dpdk-dev] [PATCH] app/testpmd: fix Rx/Tx stats after clear stats command
@ 2020-04-24 11:07 Wei Hu (Xavier)
2020-04-24 16:12 ` Ferruh Yigit
0 siblings, 1 reply; 7+ messages in thread
From: Wei Hu (Xavier) @ 2020-04-24 11:07 UTC (permalink / raw)
To: dev
From: Chengwen Feng <fengchengwen@huawei.com>
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).
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>
---
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));
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",
--
2.23.0
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [dpdk-dev] [PATCH] app/testpmd: fix Rx/Tx stats after clear stats command
2020-04-24 11:07 [dpdk-dev] [PATCH] app/testpmd: fix Rx/Tx stats after clear stats command Wei Hu (Xavier)
@ 2020-04-24 16:12 ` Ferruh Yigit
2020-04-26 1:36 ` Wei Hu (Xavier)
2020-04-26 9:22 ` Wei Hu (Xavier)
0 siblings, 2 replies; 7+ messages in thread
From: Ferruh Yigit @ 2020-04-24 16:12 UTC (permalink / raw)
To: Wei Hu (Xavier), dev
On 4/24/2020 12:07 PM, Wei Hu (Xavier) wrote:
> From: Chengwen Feng <fengchengwen@huawei.com>
>
> 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 <fengchengwen@huawei.com>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> ---
> 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.
> 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",
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [dpdk-dev] [PATCH] app/testpmd: fix Rx/Tx stats after clear stats command
2020-04-24 16:12 ` Ferruh Yigit
@ 2020-04-26 1:36 ` Wei Hu (Xavier)
2020-04-26 1:58 ` Ferruh Yigit
2020-04-26 9:22 ` Wei Hu (Xavier)
1 sibling, 1 reply; 7+ messages in thread
From: Wei Hu (Xavier) @ 2020-04-26 1:36 UTC (permalink / raw)
To: Ferruh Yigit, Wei Hu (Xavier), 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 <fengchengwen@huawei.com>
>>
>> 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)?
>
OK, Thanks for your comments.
I will send V2.
Regards
Xavier
>>
>> 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>
>> ---
>> 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.
>
>> 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",
>>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [dpdk-dev] [PATCH] app/testpmd: fix Rx/Tx stats after clear stats command
2020-04-26 1:36 ` Wei Hu (Xavier)
@ 2020-04-26 1:58 ` Ferruh Yigit
0 siblings, 0 replies; 7+ messages in thread
From: Ferruh Yigit @ 2020-04-26 1:58 UTC (permalink / raw)
To: Wei Hu (Xavier), dev
On 4/26/2020 2:36 AM, Wei Hu (Xavier) wrote:
> 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 <fengchengwen@huawei.com>
>>>
>>> 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)?
>>
> OK, Thanks for your comments.
> I will send V2.
>
> Regards
> Xavier
>>>
>>> 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>
>>> ---
>>> 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.
What do you think about above comment?
>>
>>> 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",
>>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] app/testpmd: fix Rx/Tx stats after clear stats command
2020-04-24 16:12 ` Ferruh Yigit
2020-04-26 1:36 ` Wei Hu (Xavier)
@ 2020-04-26 9:22 ` Wei Hu (Xavier)
2020-04-27 14:00 ` Ferruh Yigit
1 sibling, 1 reply; 7+ messages in thread
From: Wei Hu (Xavier) @ 2020-04-26 9:22 UTC (permalink / raw)
To: Ferruh Yigit, 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 <fengchengwen@huawei.com>
>>
>> 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 <fengchengwen@huawei.com>
>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> ---
>> 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];
<snip>
}
}
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",
>>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [dpdk-dev] [PATCH] app/testpmd: fix Rx/Tx stats after clear stats command
2020-04-26 9:22 ` Wei Hu (Xavier)
@ 2020-04-27 14:00 ` Ferruh Yigit
2020-04-28 8:42 ` Wei Hu (Xavier)
0 siblings, 1 reply; 7+ messages in thread
From: Ferruh Yigit @ 2020-04-27 14:00 UTC (permalink / raw)
To: Wei Hu (Xavier), dev
On 4/26/2020 10:22 AM, Wei Hu (Xavier) wrote:
> 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 <fengchengwen@huawei.com>
>>>
>>> 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 <fengchengwen@huawei.com>
>>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>>> ---
>>> 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));
No, original code is better. It resets the baseline for forward stats, if you do
your suggested change it zeros the port stats too which may have
unexpected/unwanted side affect.
For consistency I mean the new code you are adding to follow the similar
approach as existing one (not other-way around :), like:
nic_xstats_clear(portid_t port_id)
ret = rte_eth_xstats_reset(port_id);
rte_eth_stats_get(port_id, &ports[port_id].stats);
but if you prefer to go with 'memset' I guess that is OK too, eventually they
both give same result although I prefer above one.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [dpdk-dev] [PATCH] app/testpmd: fix Rx/Tx stats after clear stats command
2020-04-27 14:00 ` Ferruh Yigit
@ 2020-04-28 8:42 ` Wei Hu (Xavier)
0 siblings, 0 replies; 7+ messages in thread
From: Wei Hu (Xavier) @ 2020-04-28 8:42 UTC (permalink / raw)
To: Ferruh Yigit, dev
Hi, Ferrh Yigit
On 2020/4/27 22:00, Ferruh Yigit wrote:
> On 4/26/2020 10:22 AM, Wei Hu (Xavier) wrote:
>> 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 <fengchengwen@huawei.com>
>>>>
>>>> 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 <fengchengwen@huawei.com>
>>>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>>>> ---
>>>> 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));
>
> No, original code is better. It resets the baseline for forward stats, if you do
> your suggested change it zeros the port stats too which may have
> unexpected/unwanted side affect.
>
> For consistency I mean the new code you are adding to follow the similar
> approach as existing one (not other-way around :), like:
>
> nic_xstats_clear(portid_t port_id)
> ret = rte_eth_xstats_reset(port_id);
> rte_eth_stats_get(port_id, &ports[port_id].stats);
>
> but if you prefer to go with 'memset' I guess that is OK too, eventually they
> both give same result although I prefer above one.
>
Ok, got it.
Thanks.
Regards
Xavier
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-04-28 8:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-24 11:07 [dpdk-dev] [PATCH] app/testpmd: fix Rx/Tx stats after clear stats command Wei Hu (Xavier)
2020-04-24 16:12 ` Ferruh Yigit
2020-04-26 1:36 ` Wei Hu (Xavier)
2020-04-26 1:58 ` Ferruh Yigit
2020-04-26 9:22 ` Wei Hu (Xavier)
2020-04-27 14:00 ` Ferruh Yigit
2020-04-28 8:42 ` Wei Hu (Xavier)
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).