Re: [dpdk-dev] [PATCH] ethdev: fix statistics description

["Tahhan, Maryam" <maryam.tahhan@intel.com>]

> 
> Hi, John & Greg
> 
> Would you please give any opinion for this patch ?
> 
> I have looked through all PMDs and found not all statistics items can be
> supported by some NIC.
> For example,  rx_nombuf,  q_ipackets,  q_opackets,  q_ibytes and q_obytes
> are not supported by i40e.

Queue stats should be supported by i40e as we have access to struct i40e_queue_stats  this is a gap. Same for e1000.
For me (from a stats perspective), we should be able to report everything that ethtool can report for the different kernel network drivers (as we have the same base driver code in DPDK). In other words, the DPDK stats API should provide the same set of stats as a standard networking interface would to an external monitoring tool in case we want to perform some sort of analytics on it afterwards.
At a very minimum the top level stats should include: ipackets, opackets, ibytes,  obytes,  imissed,  ierrors,  oerrors. The queue stats in theory could be migrated to the xstats, it would require a lot of clean up in existing drivers which is why we didn't remove them when we did the original cleanup of the struct for the xstats API.


> But when the function rte_eth_stats_get(uint8_t port_id, struct
> rte_eth_stats *stats) is called for i40e PMD, Above un-supported statistics
> item in output stats are zero, this is not real value.

Agreed - should not output 0 for these. But should ensure where stats are possible to obtain, we support them in DPDK.

> So far, there is no way to know whether an item in struct rte_eth_stats is
> supported or not only from this structure definition.
> Maybe some structure member can be added to indicate each of statistics
> item valid or not.
> But this means ABI change.

Migrating the queue/nonstandard stats to the xstats API would fix this, the only issue is with the existing drivers that are unsupported fields with 0.

> 
> In following list, I list statistics support details of all PMDs.
> Hope it can be displayed in your screen.
> 
Thanks for this, it's very helpful. I'm currently collating a list of the missing stats for e1000, ixgbe and i40e from DPDK. So this is very helpful.

> Thanks
> /Wei
> 
> NIC           ipackets   opackets  ibytes  obytes  imissed  ierrors  oerrors
> rx_nombuf  q_ipackets   q_opacktes     q_ibytes    q_obytes  q_errors
> af_packet      y          y         y       y       n        n        y        n          y            y            y         y
> y
> bnx2x         y          y         y       y       y        y        y        y          n            n            n         n
> n
> bnxt          y          y         y       y       y        y        y        n          y            y            y         y
> y
> bonding       y          y         y       y       y        y        y        y          y            y            y         y
> y
> cxgbe         y          y         y       y       y        y        y        n          y            y            y         y
> y
> e1000(igb)     y          y         y       y       y        y        y        n          n            n            n         n
> n
> e1000(igbvf)   y          y         y       y       n        n        n        n          n            n            n
> n         n
> ena           y          y         y       y       y        y        y        y          n            n            n         n
> n
> enic          y          y         y       y       y        y        y        y          n            n            n         n
> n
> fm10k         y          y         y       y       n        n        n        n          y            y            y         y
> n
> i40e          y          y         y       y       y        y        y        n          n            n            n         n
> n
> i40evf        y          y         y       y       n        y        y        n          n            n            n         n
> n
> ixgbe         y          y         y       y       y        y        y        n          y            y            y         y
> y
> ixgbevf       y          y         y       y       n        n        n        n          n            n            n         n
> n
> mlx4          y          y         y       y       n        y        y        y          y            y            y         y
> y
> mlx5          y          y         y       y       n        y        y        y          y            y            y         y
> y
> mpipe         y          y         y       y       n        y        y        y          y            y            y         y
> y
> nfp           y          y         y       y       y        y        y        y          y            y            y         y
> n
> null          y          y         n       n       n        n        y        n          y            y            n         n
> y
> pcap          y          y         y       y       n        n        y        n          y            y            y         y
> y
> qede          y          y         y       y       y        y        y        y          n            n            n         n
> n
> ring          y          y         n       n       n        n        y        n          y            y            n         n
> y
> szedata2      y          y         y       y       n        n        y        n          y            y            y         y
> y
> thunderx      y          y         y       y       y        y        y        n          y            y            y         y
> n
> vhost         y          y         y       y       n        n        y        n          y            y            y         y
> n
> virtio         y          y         y       y       n        y        y        y          y            y            y         y
> n
> vmxnet3      y          y         y       y       n        y        y        y          y            y            y         y
> y
> xenvirt       y          y         n       n       n        n        n        n          n            n            n         n
> n
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Tuesday, October 4, 2016 5:35 PM
> > To: Dai, Wei <wei.dai@intel.com>; Mcnamara, John
> > <john.mcnamara@intel.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] ethdev: fix statistics description
> >
> > 2016-08-26 18:08, Wei Dai:
> > >  /**
> > >   * A structure used to retrieve statistics for an Ethernet port.
> > > + * Not all statistics fields in struct rte_eth_stats are supported
> > > + * by any type of network interface card (NIC). If any statistics
> > > + * field is not supported, its value is 0 .
> > >   */
> > >  struct rte_eth_stats {
> >
> > I'm missing the point of this patch.
> > Why do you think it is a fix?
> >
> > John, any opinion?
>