<snip>
> >
> >> +
> >> +  total_missed_rx = 0;
> >> +  total_qbrc = 0;
> >> +  total_qprc = 0;
> >> +  total_qprdc = 0;
> >> +  rxnfgpc = 0;
> >> +  txdgpc = 0;
> >> +  count = 0;
> >> +
> >> +  ixgbe_read_stats_registers(hw, hw_stats, &total_missed_rx,
> &total_qbrc,
> >> +                                                     &total_qprc,
> &rxnfgpc, &txdgpc, &total_qprdc);
> >> +
> >> +  if (!xstats)
> >> +          return 0;
> >
> > this cannot happen except if n == 0.
> > This condition is already tested above, and "count" should be returned.
> >
> >> +
> >> +  /* Error stats */
> >> +  for (i = 0; i < RTE_NB_XSTATS; i++) {
> >> +          snprintf(xstats[count].name, sizeof(xstats[count].name),
> >> +                          "%s", rte_ixgbe_stats_strings[i].name);
> >> +          xstats[count++].value = *(uint64_t *)(((char *)hw_stats) +
> >> +
>       rte_ixgbe_stats_strings[i].offset);
> >> +  }
> >> +
> >> +  return count;
> >> +}
> >
> > Shouldn't it be xstats[i] instead of xstats[count] ?
> >
> > Does it work when using "show port in test-pmd"?
> 
> ok I missed the 'count = 0' above.
> So why using count instead of i ?
> 
> Also, I think it would be clearer to use the constant IXGBE_NB_XSTATS instead
> of count at the beginning of the function.
> 
> 

Hi Olivier

Actually, count should not be 0, it should be n, which is the passed in index 
from rte_eth_xstats_get()...

Because we fill out the generic stats first in rte_eth_xstats_get() then we 
call ixgbe_dev_xstats_get to fill out the rest.

I will fix this up
> >
> >> +
> >> +static void
> >> +ixgbe_dev_xstats_reset(struct rte_eth_dev *dev) {
> >> +  struct ixgbe_hw_stats *stats =
> >> +                  IXGBE_DEV_PRIVATE_TO_STATS(dev->data-
> >dev_private);
> >> +
> >> +  /* HW registers are cleared on read */
> >> +  ixgbe_dev_xstats_get(dev, NULL, RTE_NB_XSTATS);
> >> +
> >> +  /* Reset software totals */
> >> +  memset(stats, 0, sizeof(*stats));
> >> +}
> >> +
> >>  static void
> >>  ixgbevf_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats
> >> *stats)  {
> >>

Reply via email to