| Message ID | 20170909112132.13936-42-shreyansh.jain@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
On 9/9/2017 12:21 PM, Shreyansh Jain wrote: > From: Hemant Agrawal <hemant.agrawal@nxp.com> > > Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com> <...> > +static int > +dpaa_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats, > + unsigned int n) > +{ > + struct dpaa_if *dpaa_intf = dev->data->dev_private; > + unsigned int i = 0, num = RTE_DIM(dpaa_xstats_strings); > + uint64_t values[sizeof(struct dpaa_if_stats) / 8]; > + > + if (xstats == NULL) > + return 0; This is a little not clear from API definition, but I guess when xstats is NULL, it should return num of available stats, "num" for this case. I guess there are PMDs implements both, can you please double check? > + > + if (n < num) > + return num; > + > + fman_if_stats_get_all(dpaa_intf->fif, values, > + sizeof(struct dpaa_if_stats) / 8); > + > + for (i = 0; i < num; i++) { > + xstats[i].id = i; > + xstats[i].value = values[dpaa_xstats_strings[i].offset / 8]; > + } > + return i; > +} <...>
On Monday 18 September 2017 08:27 PM, Ferruh Yigit wrote: > On 9/9/2017 12:21 PM, Shreyansh Jain wrote: >> From: Hemant Agrawal <hemant.agrawal@nxp.com> >> >> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com> > > <...> > >> +static int >> +dpaa_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats, >> + unsigned int n) >> +{ >> + struct dpaa_if *dpaa_intf = dev->data->dev_private; >> + unsigned int i = 0, num = RTE_DIM(dpaa_xstats_strings); >> + uint64_t values[sizeof(struct dpaa_if_stats) / 8]; >> + >> + if (xstats == NULL) >> + return 0; > > This is a little not clear from API definition, but I guess when xstats > is NULL, it should return num of available stats, "num" for this case. I > guess there are PMDs implements both, can you please double check? Ok. I will check again. > >> + >> + if (n < num) >> + return num; >> + >> + fman_if_stats_get_all(dpaa_intf->fif, values, >> + sizeof(struct dpaa_if_stats) / 8); >> + >> + for (i = 0; i < num; i++) { >> + xstats[i].id = i; >> + xstats[i].value = values[dpaa_xstats_strings[i].offset / 8]; >> + } >> + return i; >> +} > > <...> >
On Thursday 21 September 2017 06:56 PM, Shreyansh Jain wrote: > On Monday 18 September 2017 08:27 PM, Ferruh Yigit wrote: >> On 9/9/2017 12:21 PM, Shreyansh Jain wrote: >>> From: Hemant Agrawal <hemant.agrawal@nxp.com> >>> >>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com> >> >> <...> >> >>> +static int >>> +dpaa_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat >>> *xstats, >>> + unsigned int n) >>> +{ >>> + struct dpaa_if *dpaa_intf = dev->data->dev_private; >>> + unsigned int i = 0, num = RTE_DIM(dpaa_xstats_strings); >>> + uint64_t values[sizeof(struct dpaa_if_stats) / 8]; >>> + >>> + if (xstats == NULL) >>> + return 0; >> >> This is a little not clear from API definition, but I guess when xstats >> is NULL, it should return num of available stats, "num" for this case. I >> guess there are PMDs implements both, can you please double check? > > Ok. I will check again. I checked a number of other ethev implementations. Some like i40e/e1000 also return 0 when xstats is NULL. Others, like bnx2x and qede don't handle this situation. All return "num" when passed argument is larger than number of elements in the table. Though, I think the logic that get_xstats should return its size (num) when passed with NULL, looks good to me. How does one standardize such semantics for existing APIs? (I can add this info to the API document that you created - but only once we know if others will agree to change) > >> >>> + >>> + if (n < num) >>> + return num; >>> + >>> + fman_if_stats_get_all(dpaa_intf->fif, values, >>> + sizeof(struct dpaa_if_stats) / 8); >>> + >>> + for (i = 0; i < num; i++) { >>> + xstats[i].id = i; >>> + xstats[i].value = values[dpaa_xstats_strings[i].offset / 8]; >>> + } >>> + return i; >>> +} >> >> <...> >> > >
On 9/27/2017 9:26 AM, Shreyansh Jain wrote: > On Thursday 21 September 2017 06:56 PM, Shreyansh Jain wrote: >> On Monday 18 September 2017 08:27 PM, Ferruh Yigit wrote: >>> On 9/9/2017 12:21 PM, Shreyansh Jain wrote: >>>> From: Hemant Agrawal <hemant.agrawal@nxp.com> >>>> >>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com> >>> >>> <...> >>> >>>> +static int >>>> +dpaa_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat >>>> *xstats, >>>> + unsigned int n) >>>> +{ >>>> + struct dpaa_if *dpaa_intf = dev->data->dev_private; >>>> + unsigned int i = 0, num = RTE_DIM(dpaa_xstats_strings); >>>> + uint64_t values[sizeof(struct dpaa_if_stats) / 8]; >>>> + >>>> + if (xstats == NULL) >>>> + return 0; >>> >>> This is a little not clear from API definition, but I guess when xstats >>> is NULL, it should return num of available stats, "num" for this case. I >>> guess there are PMDs implements both, can you please double check? >> >> Ok. I will check again. > > I checked a number of other ethev implementations. Some like i40e/e1000 > also return 0 when xstats is NULL. Others, like bnx2x and qede don't > handle this situation. > All return "num" when passed argument is larger than number of elements > in the table. > > Though, I think the logic that get_xstats should return its size (num) > when passed with NULL, looks good to me. > How does one standardize such semantics for existing APIs? Thanks for checking, I guess first we should clarify the API and the expected behavior [1] and later update required PMDs. So for now I think PMD is OK as it is. [1] I double checked the rte_eth_xstats_get(). It does: If xstats == NULL xcount = dev_ops->xstats_get(dev, NULL, 0); return count + xcount; Intention looks like to returning number of available stats, otherwise returning "count + 0" will be useless. So it looks like expectation from eth_xstats_get_t for that case is returning xstats size, but this not clear and not documented in API comment. > > (I can add this info to the API document that you created - but only > once we know if others will agree to change) > >> >>> >>>> + >>>> + if (n < num) >>>> + return num; >>>> + >>>> + fman_if_stats_get_all(dpaa_intf->fif, values, >>>> + sizeof(struct dpaa_if_stats) / 8); >>>> + >>>> + for (i = 0; i < num; i++) { >>>> + xstats[i].id = i; >>>> + xstats[i].value = values[dpaa_xstats_strings[i].offset / 8]; >>>> + } >>>> + return i; >>>> +} >>> >>> <...> >>> >> >> >
Hi Ferruh, > -----Original Message----- > From: Ferruh Yigit [mailto:ferruh.yigit@intel.com] > Sent: Thursday, September 28, 2017 5:07 AM > To: Shreyansh Jain <shreyansh.jain@nxp.com> > Cc: dev@dpdk.org; Hemant Agrawal <hemant.agrawal@nxp.com> > Subject: Re: [PATCH v4 41/41] net/dpaa: support for extended statistics > > On 9/27/2017 9:26 AM, Shreyansh Jain wrote: > > On Thursday 21 September 2017 06:56 PM, Shreyansh Jain wrote: > >> On Monday 18 September 2017 08:27 PM, Ferruh Yigit wrote: > >>> On 9/9/2017 12:21 PM, Shreyansh Jain wrote: > >>>> From: Hemant Agrawal <hemant.agrawal@nxp.com> > >>>> > >>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com> > >>> > >>> <...> > >>> > >>>> +static int > >>>> +dpaa_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat > >>>> *xstats, > >>>> + unsigned int n) > >>>> +{ > >>>> + struct dpaa_if *dpaa_intf = dev->data->dev_private; > >>>> + unsigned int i = 0, num = RTE_DIM(dpaa_xstats_strings); > >>>> + uint64_t values[sizeof(struct dpaa_if_stats) / 8]; > >>>> + > >>>> + if (xstats == NULL) > >>>> + return 0; > >>> > >>> This is a little not clear from API definition, but I guess when xstats > >>> is NULL, it should return num of available stats, "num" for this case. I > >>> guess there are PMDs implements both, can you please double check? > >> > >> Ok. I will check again. > > > > I checked a number of other ethev implementations. Some like i40e/e1000 > > also return 0 when xstats is NULL. Others, like bnx2x and qede don't > > handle this situation. > > All return "num" when passed argument is larger than number of elements > > in the table. > > > > Though, I think the logic that get_xstats should return its size (num) > > when passed with NULL, looks good to me. > > How does one standardize such semantics for existing APIs? > > Thanks for checking, I guess first we should clarify the API and the > expected behavior [1] and later update required PMDs. > > So for now I think PMD is OK as it is. > > > [1] > I double checked the rte_eth_xstats_get(). It does: > > If xstats == NULL > xcount = dev_ops->xstats_get(dev, NULL, 0); > return count + xcount; > > Intention looks like to returning number of available stats, otherwise > returning "count + 0" will be useless. Makes sense. I missed this and kept looking for implementations. I will at least fix dpaa code. > > So it looks like expectation from eth_xstats_get_t for that case is > returning xstats size, but this not clear and not documented in API comment. > > > > > (I can add this info to the API document that you created - but only > > once we know if others will agree to change) Probably this info should be in Doxygen APIs [2]. [2] http://dpdk.org/doc/api/rte__ethdev_8h.html#adad5c65f659487db1fefba7d7d902973 > > > >> > >>> > >>>> + > >>>> + if (n < num) > >>>> + return num; > >>>> + > >>>> + fman_if_stats_get_all(dpaa_intf->fif, values, > >>>> + sizeof(struct dpaa_if_stats) / 8); > >>>> + > >>>> + for (i = 0; i < num; i++) { > >>>> + xstats[i].id = i; > >>>> + xstats[i].value = values[dpaa_xstats_strings[i].offset / 8]; > >>>> + } > >>>> + return i; > >>>> +} > >>> > >>> <...> > >>> > >> > >> > >
> -----Original Message----- > From: Shreyansh Jain > Sent: Thursday, September 28, 2017 7:59 AM > To: 'Ferruh Yigit' <ferruh.yigit@intel.com> > Cc: dev@dpdk.org; Hemant Agrawal <hemant.agrawal@nxp.com> > Subject: RE: [PATCH v4 41/41] net/dpaa: support for extended statistics > > Hi Ferruh, > > > -----Original Message----- > > From: Ferruh Yigit [mailto:ferruh.yigit@intel.com] > > Sent: Thursday, September 28, 2017 5:07 AM > > To: Shreyansh Jain <shreyansh.jain@nxp.com> > > Cc: dev@dpdk.org; Hemant Agrawal <hemant.agrawal@nxp.com> > > Subject: Re: [PATCH v4 41/41] net/dpaa: support for extended statistics > > > > On 9/27/2017 9:26 AM, Shreyansh Jain wrote: > > > On Thursday 21 September 2017 06:56 PM, Shreyansh Jain wrote: > > >> On Monday 18 September 2017 08:27 PM, Ferruh Yigit wrote: > > >>> On 9/9/2017 12:21 PM, Shreyansh Jain wrote: > > >>>> From: Hemant Agrawal <hemant.agrawal@nxp.com> > > >>>> > > >>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com> > > >>> > > >>> <...> > > >>> > > >>>> +static int > > >>>> +dpaa_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat > > >>>> *xstats, > > >>>> + unsigned int n) > > >>>> +{ > > >>>> + struct dpaa_if *dpaa_intf = dev->data->dev_private; > > >>>> + unsigned int i = 0, num = RTE_DIM(dpaa_xstats_strings); > > >>>> + uint64_t values[sizeof(struct dpaa_if_stats) / 8]; > > >>>> + > > >>>> + if (xstats == NULL) > > >>>> + return 0; > > >>> > > >>> This is a little not clear from API definition, but I guess when xstats > > >>> is NULL, it should return num of available stats, "num" for this case. > I > > >>> guess there are PMDs implements both, can you please double check? > > >> > > >> Ok. I will check again. > > > > > > I checked a number of other ethev implementations. Some like i40e/e1000 > > > also return 0 when xstats is NULL. Others, like bnx2x and qede don't > > > handle this situation. > > > All return "num" when passed argument is larger than number of elements > > > in the table. > > > > > > Though, I think the logic that get_xstats should return its size (num) > > > when passed with NULL, looks good to me. > > > How does one standardize such semantics for existing APIs? > > > > Thanks for checking, I guess first we should clarify the API and the > > expected behavior [1] and later update required PMDs. > > > > So for now I think PMD is OK as it is. > > > > > > [1] > > I double checked the rte_eth_xstats_get(). It does: > > > > If xstats == NULL > > xcount = dev_ops->xstats_get(dev, NULL, 0); > > return count + xcount; > > > > Intention looks like to returning number of available stats, otherwise > > returning "count + 0" will be useless. > > Makes sense. I missed this and kept looking for implementations. > I will at least fix dpaa code. On a second though: there might be another issue. Application calls rte_eth_xstats_get_names and finds that 'N' xstats exist. Thereafter, in a call to rte_eth_xstats_get, xstats==NULL but n=N, so the API would return: if (n < count + xcount || xstats == NULL) return count + xcount; 'count' is size of generic stats. If drivers->xstats_get were to return xcount='N', the application would think that it has got a positive response. See the doxygen page [3] - it states: -- Returns: * A positive value lower or equal to size: success. The return value is the number of entries filled in the stats table -- There might be a case where the generic stats are exactly equal to xstats size and application would attempt to access the array. I am not even sure why the xstats_get is returning (count + xcount) when the API definition doesn't say that generic+xstat is returned. Am I missing something? [3] http://dpdk.org/doc/api/rte__ethdev_8h.html#adad5c65f659487db1fefba7d7d902973 > > > > > So it looks like expectation from eth_xstats_get_t for that case is > > returning xstats size, but this not clear and not documented in API > comment. > > > > > > > > (I can add this info to the API document that you created - but only > > > once we know if others will agree to change) > > Probably this info should be in Doxygen APIs [2]. > > [2] > http://dpdk.org/doc/api/rte__ethdev_8h.html#adad5c65f659487db1fefba7d7d902973 >
On 9/28/2017 3:52 AM, Shreyansh Jain wrote: >> -----Original Message----- >> From: Shreyansh Jain >> Sent: Thursday, September 28, 2017 7:59 AM >> To: 'Ferruh Yigit' <ferruh.yigit@intel.com> >> Cc: dev@dpdk.org; Hemant Agrawal <hemant.agrawal@nxp.com> >> Subject: RE: [PATCH v4 41/41] net/dpaa: support for extended statistics >> >> Hi Ferruh, >> >>> -----Original Message----- >>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com] >>> Sent: Thursday, September 28, 2017 5:07 AM >>> To: Shreyansh Jain <shreyansh.jain@nxp.com> >>> Cc: dev@dpdk.org; Hemant Agrawal <hemant.agrawal@nxp.com> >>> Subject: Re: [PATCH v4 41/41] net/dpaa: support for extended statistics >>> >>> On 9/27/2017 9:26 AM, Shreyansh Jain wrote: >>>> On Thursday 21 September 2017 06:56 PM, Shreyansh Jain wrote: >>>>> On Monday 18 September 2017 08:27 PM, Ferruh Yigit wrote: >>>>>> On 9/9/2017 12:21 PM, Shreyansh Jain wrote: >>>>>>> From: Hemant Agrawal <hemant.agrawal@nxp.com> >>>>>>> >>>>>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com> >>>>>> >>>>>> <...> >>>>>> >>>>>>> +static int >>>>>>> +dpaa_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat >>>>>>> *xstats, >>>>>>> + unsigned int n) >>>>>>> +{ >>>>>>> + struct dpaa_if *dpaa_intf = dev->data->dev_private; >>>>>>> + unsigned int i = 0, num = RTE_DIM(dpaa_xstats_strings); >>>>>>> + uint64_t values[sizeof(struct dpaa_if_stats) / 8]; >>>>>>> + >>>>>>> + if (xstats == NULL) >>>>>>> + return 0; >>>>>> >>>>>> This is a little not clear from API definition, but I guess when xstats >>>>>> is NULL, it should return num of available stats, "num" for this case. >> I >>>>>> guess there are PMDs implements both, can you please double check? >>>>> >>>>> Ok. I will check again. >>>> >>>> I checked a number of other ethev implementations. Some like i40e/e1000 >>>> also return 0 when xstats is NULL. Others, like bnx2x and qede don't >>>> handle this situation. >>>> All return "num" when passed argument is larger than number of elements >>>> in the table. >>>> >>>> Though, I think the logic that get_xstats should return its size (num) >>>> when passed with NULL, looks good to me. >>>> How does one standardize such semantics for existing APIs? >>> >>> Thanks for checking, I guess first we should clarify the API and the >>> expected behavior [1] and later update required PMDs. >>> >>> So for now I think PMD is OK as it is. >>> >>> >>> [1] >>> I double checked the rte_eth_xstats_get(). It does: >>> >>> If xstats == NULL >>> xcount = dev_ops->xstats_get(dev, NULL, 0); >>> return count + xcount; >>> >>> Intention looks like to returning number of available stats, otherwise >>> returning "count + 0" will be useless. >> >> Makes sense. I missed this and kept looking for implementations. >> I will at least fix dpaa code. > > On a second though: there might be another issue. > Application calls rte_eth_xstats_get_names and finds that 'N' xstats exist. > Thereafter, in a call to rte_eth_xstats_get, xstats==NULL but n=N, so the API would return: > > if (n < count + xcount || xstats == NULL) > return count + xcount; > > 'count' is size of generic stats. If drivers->xstats_get were to return xcount='N', the application would think that it has got a positive response. > See the doxygen page [3] - it states: > > -- > Returns: > * A positive value lower or equal to size: success. > The return value is the number of entries filled in the > stats table > -- > > There might be a case where the generic stats are exactly equal to xstats size and application would attempt to access the array. > > I am not even sure why the xstats_get is returning (count + xcount) when the API definition doesn't say that generic+xstat is returned. > Am I missing something? Even for rte_eth_xstats_get_names(), returned N is generic + xstat. dev_ops->xstats_get() manages xstat only, rte_eth_xstats_xxx() on top of them manages generic + xstat, this seems how it is designed. for rte_eth_xstats_get(), I guess there is an assumption that when app provides xstats == NULL, n also should be 0. Perhaps this should be implemented into API. > > [3] http://dpdk.org/doc/api/rte__ethdev_8h.html#adad5c65f659487db1fefba7d7d902973 > >> >>> >>> So it looks like expectation from eth_xstats_get_t for that case is >>> returning xstats size, but this not clear and not documented in API >> comment. >>> >>>> >>>> (I can add this info to the API document that you created - but only >>>> once we know if others will agree to change) >> >> Probably this info should be in Doxygen APIs [2]. >> >> [2] >> http://dpdk.org/doc/api/rte__ethdev_8h.html#adad5c65f659487db1fefba7d7d902973 >> >
diff --git a/doc/guides/nics/features/dpaa.ini b/doc/guides/nics/features/dpaa.ini index 09b9bd9..24cfd85 100644 --- a/doc/guides/nics/features/dpaa.ini +++ b/doc/guides/nics/features/dpaa.ini @@ -18,6 +18,7 @@ L3 checksum offload = Y L4 checksum offload = Y Packet type parsing = Y Basic stats = Y +Extended stats = Y FW version = Y ARMv8 = Y Usage doc = Y diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c index 22f56a4..5b5ec9c 100644 --- a/drivers/net/dpaa/dpaa_ethdev.c +++ b/drivers/net/dpaa/dpaa_ethdev.c @@ -75,6 +75,40 @@ /* Keep track of whether QMAN and BMAN have been globally initialized */ static int is_global_init; +struct rte_dpaa_xstats_name_off { + char name[RTE_ETH_XSTATS_NAME_SIZE]; + uint32_t offset; +}; + +static const struct rte_dpaa_xstats_name_off dpaa_xstats_strings[] = { + {"rx_align_err", + offsetof(struct dpaa_if_stats, raln)}, + {"rx_valid_pause", + offsetof(struct dpaa_if_stats, rxpf)}, + {"rx_fcs_err", + offsetof(struct dpaa_if_stats, rfcs)}, + {"rx_vlan_frame", + offsetof(struct dpaa_if_stats, rvlan)}, + {"rx_frame_err", + offsetof(struct dpaa_if_stats, rerr)}, + {"rx_drop_err", + offsetof(struct dpaa_if_stats, rdrp)}, + {"rx_undersized", + offsetof(struct dpaa_if_stats, rund)}, + {"rx_oversize_err", + offsetof(struct dpaa_if_stats, rovr)}, + {"rx_fragment_pkt", + offsetof(struct dpaa_if_stats, rfrg)}, + {"tx_valid_pause", + offsetof(struct dpaa_if_stats, txpf)}, + {"tx_fcs_err", + offsetof(struct dpaa_if_stats, terr)}, + {"tx_vlan_frame", + offsetof(struct dpaa_if_stats, tvlan)}, + {"rx_undersized", + offsetof(struct dpaa_if_stats, tund)}, +}; + static int dpaa_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) { @@ -268,6 +302,110 @@ static void dpaa_eth_stats_reset(struct rte_eth_dev *dev) fman_if_stats_reset(dpaa_intf->fif); } +static int +dpaa_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats, + unsigned int n) +{ + struct dpaa_if *dpaa_intf = dev->data->dev_private; + unsigned int i = 0, num = RTE_DIM(dpaa_xstats_strings); + uint64_t values[sizeof(struct dpaa_if_stats) / 8]; + + if (xstats == NULL) + return 0; + + if (n < num) + return num; + + fman_if_stats_get_all(dpaa_intf->fif, values, + sizeof(struct dpaa_if_stats) / 8); + + for (i = 0; i < num; i++) { + xstats[i].id = i; + xstats[i].value = values[dpaa_xstats_strings[i].offset / 8]; + } + return i; +} + +static int +dpaa_xstats_get_names(__rte_unused struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names, + __rte_unused unsigned int limit) +{ + unsigned int i, stat_cnt = RTE_DIM(dpaa_xstats_strings); + + if (xstats_names != NULL) + for (i = 0; i < stat_cnt; i++) + snprintf(xstats_names[i].name, + sizeof(xstats_names[i].name), + "%s", + dpaa_xstats_strings[i].name); + + return stat_cnt; +} + +static int +dpaa_xstats_get_by_id(struct rte_eth_dev *dev, const uint64_t *ids, + uint64_t *values, unsigned int n) +{ + unsigned int i, stat_cnt = RTE_DIM(dpaa_xstats_strings); + uint64_t values_copy[sizeof(struct dpaa_if_stats) / 8]; + + if (!ids) { + struct dpaa_if *dpaa_intf = dev->data->dev_private; + + if (n < stat_cnt) + return stat_cnt; + + if (!values) + return 0; + + fman_if_stats_get_all(dpaa_intf->fif, values_copy, + sizeof(struct dpaa_if_stats)); + + for (i = 0; i < stat_cnt; i++) + values[i] = + values_copy[dpaa_xstats_strings[i].offset / 8]; + + return stat_cnt; + } + + dpaa_xstats_get_by_id(dev, NULL, values_copy, stat_cnt); + + for (i = 0; i < n; i++) { + if (ids[i] >= stat_cnt) { + DPAA_PMD_ERR("id value isn't valid"); + return -1; + } + values[i] = values_copy[ids[i]]; + } + return n; +} + +static int +dpaa_xstats_get_names_by_id( + struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names, + const uint64_t *ids, + unsigned int limit) +{ + unsigned int i, stat_cnt = RTE_DIM(dpaa_xstats_strings); + struct rte_eth_xstat_name xstats_names_copy[stat_cnt]; + + if (!ids) + return dpaa_xstats_get_names(dev, xstats_names, limit); + + dpaa_xstats_get_names(dev, xstats_names_copy, limit); + + for (i = 0; i < limit; i++) { + if (ids[i] >= stat_cnt) { + DPAA_PMD_ERR("id value isn't valid"); + return -1; + } + strcpy(xstats_names[i].name, xstats_names_copy[ids[i]].name); + } + return limit; +} + static void dpaa_eth_promiscuous_enable(struct rte_eth_dev *dev) { struct dpaa_if *dpaa_intf = dev->data->dev_private; @@ -535,6 +673,11 @@ static struct eth_dev_ops dpaa_devops = { .link_update = dpaa_eth_link_update, .stats_get = dpaa_eth_stats_get, + .xstats_get = dpaa_dev_xstats_get, + .xstats_get_by_id = dpaa_xstats_get_by_id, + .xstats_get_names_by_id = dpaa_xstats_get_names_by_id, + .xstats_get_names = dpaa_xstats_get_names, + .xstats_reset = dpaa_eth_stats_reset, .stats_reset = dpaa_eth_stats_reset, .promiscuous_enable = dpaa_eth_promiscuous_enable, .promiscuous_disable = dpaa_eth_promiscuous_disable, diff --git a/drivers/net/dpaa/dpaa_ethdev.h b/drivers/net/dpaa/dpaa_ethdev.h index e1e062e..3f06d63 100644 --- a/drivers/net/dpaa/dpaa_ethdev.h +++ b/drivers/net/dpaa/dpaa_ethdev.h @@ -134,4 +134,44 @@ struct dpaa_if { struct rte_eth_fc_conf *fc_conf; }; +struct dpaa_if_stats { + /* Rx Statistics Counter */ + uint64_t reoct; /**<Rx Eth Octets Counter */ + uint64_t roct; /**<Rx Octet Counters */ + uint64_t raln; /**<Rx Alignment Error Counter */ + uint64_t rxpf; /**<Rx valid Pause Frame */ + uint64_t rfrm; /**<Rx Frame counter */ + uint64_t rfcs; /**<Rx frame check seq error */ + uint64_t rvlan; /**<Rx Vlan Frame Counter */ + uint64_t rerr; /**<Rx Frame error */ + uint64_t ruca; /**<Rx Unicast */ + uint64_t rmca; /**<Rx Multicast */ + uint64_t rbca; /**<Rx Broadcast */ + uint64_t rdrp; /**<Rx Dropped Packet */ + uint64_t rpkt; /**<Rx packet */ + uint64_t rund; /**<Rx undersized packets */ + uint32_t res_x[14]; + uint64_t rovr; /**<Rx oversized but good */ + uint64_t rjbr; /**<Rx oversized with bad csum */ + uint64_t rfrg; /**<Rx fragment Packet */ + uint64_t rcnp; /**<Rx control packets (0x8808 */ + uint64_t rdrntp; /**<Rx dropped due to FIFO overflow */ + uint32_t res01d0[12]; + /* Tx Statistics Counter */ + uint64_t teoct; /**<Tx eth octets */ + uint64_t toct; /**<Tx Octets */ + uint32_t res0210[2]; + uint64_t txpf; /**<Tx valid pause frame */ + uint64_t tfrm; /**<Tx frame counter */ + uint64_t tfcs; /**<Tx FCS error */ + uint64_t tvlan; /**<Tx Vlan Frame */ + uint64_t terr; /**<Tx frame error */ + uint64_t tuca; /**<Tx Unicast */ + uint64_t tmca; /**<Tx Multicast */ + uint64_t tbca; /**<Tx Broadcast */ + uint32_t res0258[2]; + uint64_t tpkt; /**<Tx Packet */ + uint64_t tund; /**<Tx Undersized */ +}; + #endif