> From: fengchengwen [mailto:fengcheng...@huawei.com] > Sent: Thursday, 28 April 2022 15.39 > > On 2022/4/25 18:16, Morten Brørup wrote: > >> From: fengchengwen [mailto:fengcheng...@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().
Instead of modifying the drivers, consider copying this code from rte_eth_xstats_get_names() to rte_eth_xstats_get(): cnt_expected_entries = eth_dev_get_xstats_count(port_id); if (xstats_names == NULL || cnt_expected_entries < 0 || (int)size < cnt_expected_entries) return cnt_expected_entries; Just a suggestion. Either solution is fine with me. > > 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 <fengcheng...@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 > >>> > >>> . > >> > > >