From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 21DDAA0503; Thu, 28 Apr 2022 05:10:16 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B87A240691; Thu, 28 Apr 2022 05:10:15 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id A37F84068F for ; Thu, 28 Apr 2022 05:10:14 +0200 (CEST) Received: from kwepemi500012.china.huawei.com (unknown [172.30.72.57]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4Kpgb93JVYzhYkx; Thu, 28 Apr 2022 11:09:57 +0800 (CST) Received: from [10.67.103.128] (10.67.103.128) by kwepemi500012.china.huawei.com (7.221.188.12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Thu, 28 Apr 2022 11:10:11 +0800 Subject: Re: [PATCH 1/2] app/testpmd: fix stats get when display fwd stats To: "Singh, Aman Deep" , CC: , , References: <20220406084537.16799-1-humin29@huawei.com> <20220406084537.16799-2-humin29@huawei.com> <24445fab-6aa3-a8e0-0faa-e32b8ddb2eef@intel.com> From: "Min Hu (Connor)" Message-ID: Date: Thu, 28 Apr 2022 11:10:10 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 MIME-Version: 1.0 In-Reply-To: <24445fab-6aa3-a8e0-0faa-e32b8ddb2eef@intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.128] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To kwepemi500012.china.huawei.com (7.221.188.12) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi, Singh, 在 2022/4/27 20:15, Singh, Aman Deep 写道: > > > On 4/6/2022 2:15 PM, Min Hu (Connor) wrote: >> In function 'fwd_stats_display', if function 'rte_eth_stats_get' fails, >> 'stats' is uncertainty value. The display result will be abnormal. >> >> This patch check the return value of 'rte_eth_stats_get' to avoid >> display abnormal stats. >> >> Fixes: 53324971a14e ("app/testpmd: display/clear forwarding stats on >> demand") >> Cc: stable@dpdk.org >> >> Signed-off-by: Min Hu (Connor) >> --- >>   app/test-pmd/config.c  | 10 ++++++++-- >>   app/test-pmd/testpmd.c | 16 ++++++++++++++-- >>   2 files changed, 22 insertions(+), 4 deletions(-) >> >> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c >> index cc8e7aa138..bd689f9f86 100644 >> --- a/app/test-pmd/config.c >> +++ b/app/test-pmd/config.c >> @@ -249,14 +249,20 @@ nic_stats_display(portid_t port_id) >>                                   diff_ns; >>       uint64_t mpps_rx, mpps_tx, mbps_rx, mbps_tx; >>       struct rte_eth_stats stats; >> - >>       static const char *nic_stats_border = "########################"; >> +    int ret; >>       if (port_id_is_invalid(port_id, ENABLED_WARN)) { >>           print_valid_ports(); >>           return; >>       } >> -    rte_eth_stats_get(port_id, &stats); >> +    ret = rte_eth_stats_get(port_id, &stats); >> +    if (ret != 0) { >> +        fprintf(stderr, >> +            "%s: Error: failed to get stats (port %u): %d", >> +            __func__, port_id, ret); >> +        return; >> +    } >>       printf("\n  %s NIC statistics for port %-2d %s\n", >>              nic_stats_border, port_id, nic_stats_border); >> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c >> index fe2ce19f99..79bb23264b 100644 >> --- a/app/test-pmd/testpmd.c >> +++ b/app/test-pmd/testpmd.c >> @@ -1982,6 +1982,7 @@ fwd_stats_display(void) >>       struct rte_port *port; >>       streamid_t sm_id; >>       portid_t pt_id; >> +    int ret; >>       int i; >>       memset(ports_stats, 0, sizeof(ports_stats)); >> @@ -2013,7 +2014,13 @@ fwd_stats_display(void) >>           pt_id = fwd_ports_ids[i]; >>           port = &ports[pt_id]; >> -        rte_eth_stats_get(pt_id, &stats); >> +        ret = rte_eth_stats_get(pt_id, &stats); >> +        if (ret != 0) { >> +            fprintf(stderr, >> +                "%s: Error: failed to get stats (port %u): %d", >> +                __func__, pt_id, ret); >> +            continue; >> +        } >>           stats.ipackets -= port->stats.ipackets; >>           stats.opackets -= port->stats.opackets; >>           stats.ibytes -= port->stats.ibytes; >> @@ -2108,11 +2115,16 @@ fwd_stats_reset(void) >>   { >>       streamid_t sm_id; >>       portid_t pt_id; >> +    int ret; >>       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); >> +        ret = rte_eth_stats_get(pt_id, &ports[pt_id].stats); >> +        if (ret != 0) >> +            fprintf(stderr, >> +                "%s: Error: failed to clear stats (port %u):%d", >> +                __func__, pt_id, ret); > Should we clear "ports[pt_id].stats" in this condition. when done 'rte_eth_stats_get', if success, 'ports[pt_id].stats' will be cleared in this function. if failed, no need to clear 'ports[pt_id].stats' as the log said. >>       } >>       for (sm_id = 0; sm_id < cur_fwd_config.nb_fwd_streams; sm_id++) { >>           struct fwd_stream *fs = fwd_streams[sm_id]; > As such LGTM Thanks for your comment. > Acked-by: Aman Singh > .