From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <dev-bounces@dpdk.org> Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3FA35A034C; Thu, 28 Apr 2022 15:38:56 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D7D4E42819; Thu, 28 Apr 2022 15:38:55 +0200 (CEST) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id D201F40E50 for <dev@dpdk.org>; Thu, 28 Apr 2022 15:38:53 +0200 (CEST) Received: from dggpeml500024.china.huawei.com (unknown [172.30.72.56]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4KpxTn2sdmzGp2K; Thu, 28 Apr 2022 21:36:13 +0800 (CST) Received: from [127.0.0.1] (10.67.100.224) by dggpeml500024.china.huawei.com (7.185.36.10) 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 21:38:51 +0800 Subject: Re: [PATCH 1/3] ethdev: fix telemetry xstats return null with some PMDs To: =?UTF-8?Q?Morten_Br=c3=b8rup?= <mb@smartsharesystems.com>, Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>, Stephen Hemminger <stephen@networkplumber.org> CC: <thomas@monjalon.net>, <ferruh.yigit@xilinx.com>, <ndabilpuram@marvell.com>, <kirankumark@marvell.com>, <skori@marvell.com>, <skoteshwar@marvell.com>, <dev@dpdk.org> References: <20220416010747.40714-1-fengchengwen@huawei.com> <20220416010747.40714-2-fengchengwen@huawei.com> <20220415183831.2473346b@hermes.local> <c3ebf4d9-aa2a-1dd5-4377-e06c7d56e7d1@oktetlabs.ru> <f0cb3543-bd57-8978-e9c5-fa985540185f@huawei.com> <98CBD80474FA8B44BF855DF32C47DC35D86FF7@smartserver.smartshare.dk> From: fengchengwen <fengchengwen@huawei.com> Message-ID: <4aeb8f6a-0537-2f14-240c-bddc74977b7e@huawei.com> Date: Thu, 28 Apr 2022 21:38:50 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D86FF7@smartserver.smartshare.dk> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.100.224] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) To dggpeml500024.china.huawei.com (7.185.36.10) X-CFilter-Loop: Reflected 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>, <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>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org On 2022/4/25 18:16, Morten Brørup wrote: >> From: fengchengwen [mailto:fengchengwen@huawei.com] >> Sent: Sunday, 24 April 2022 05.45 >> >> The root cause is: when the xstats is NULL and n less than required >> number of >> elements is zero, the return value of rte_eth_xstats_get is ambiguous >> from >> rte_ethdev.h's declaration. >> >> But the implementation of rte_eth_xstats_get return required number of >> elements >> when xstats is NULL and n less than required number of elements. >> >> There are two modification schemes: >> a) the value of xstats cannot be NULL, and the value of n must be >> greater than >> or equal to the required number, otherwise, an error code is >> returned. >> b) define the behavior as the same as rte_eth_xstats_get_names, which >> means return >> required number of elelement when xstats is NULL and n less than >> required number >> of elements. >> >> I prefer the scheme a because rte_eth_xstats_get and >> rte_eth_xstats_get_name are >> symbiotic, and it's not necessary to both implement the same logic. >> >> Also for scheme a, there is no need to modify the PMD implementation. >> >> What about your opinions ? > > This is an excellent proposal. > > And if the documentation for rte_eth_xstats_get() refers to rte_eth_xstats_get_names() for getting the number of elements, it is perfect. Hi Morten, I did a deepin review and found that many application use rte_eth_xstats_get(port_id, NULL, 0), and also many PMD already support return required number of entries when xstats is NULL. So in the v2, I use the modified hns3/ipn3ke/mvpp2/axgbe PMD method and explicit return value when xstats is NULL of rte_eth_xstats-get(). Thanks. > >> >> On 2022/4/21 14:49, Andrew Rybchenko wrote: >>> On 4/16/22 04:38, Stephen Hemminger wrote: >>>> On Sat, 16 Apr 2022 09:07:45 +0800 >>>> Chengwen Feng <fengchengwen@huawei.com> wrote: >>>> >>>>> Currently the telemetry xstats uses rte_eth_xstats_get() to >> retrieve >>>>> the number of elements. But the value to be returned when the >> parameter >>>>> 'xstats' is NULL is not specified, some PMDs (eg. >> hns3/ipn3ke/mvpp2/ >>>>> axgbe) return zero while others return the required number of >> elements. >>>> >>>> Lets fix the PMD's this impacts other code as well. >>> >>> +1 >>> >>> . >> >