[dpdk-dev] Which counters are set by rte_eth_stats_get

[dpdk-dev] Which counters are set by rte_eth_stats_get

[Tom Barbette]

Hi ethdev maintainers,


Support of drivers for the fields in rte_eth_stats is a bit random, and never mentioned in the doc. A quick survey showed me :


ipackets : implemented by all drivers
ibytes : all except null, ring
ierror : all except af_packet, ark, avf, axgbe, fm10k, kni, null, pcap, ring, szedata2, vhost
imissed : *only* af_packet, avp, axgbe, fm10k, kni, liquidio, mlx4, mlx5, null, pcap, ring, szedata2, tap, vhost, virtio
rx_nombuf : *only* bnx2x,bnxt,bonding,ena,enic,failsafe,mlx4,mlx5,netvsc,nfp,qede,szedata2,tap,virtio

With no way to know if we can rely on the value or not, as a DPDK user pov. The only way to know if we can rely on a given counter is to grep the driver code. Except if I missed something?

Also the doc of rte_eth_stats_get only mention io packets, bytes and errors. Not the other fields, and the way it is written let the reader think it is always supported if the function does not return 0.

I can update the doc to reflect the state of things. But maybe we could make that function return a bitmask which tells which counter has been set. But that would break the ABI... We could also have the bitmask set through a passed pointer, so it does not break code checking the return value is 0. Or maybe have the bitmask elsewhere, like for the offloads? Which fields are supported is probably a constant. So that may make more sense.

If you give me directions, I can propose a patch.

Cheers,
Tom

Re: [dpdk-dev] Which counters are set by rte_eth_stats_get

[Thomas Monjalon]

09/11/2018 09:28, Tom Barbette:
> Hi ethdev maintainers,
> 
> 
> Support of drivers for the fields in rte_eth_stats is a bit random, and never mentioned in the doc. A quick survey showed me :
> 
> 
> ipackets : implemented by all drivers
> ibytes : all except null, ring
> ierror : all except af_packet, ark, avf, axgbe, fm10k, kni, null, pcap, ring, szedata2, vhost
> imissed : *only* af_packet, avp, axgbe, fm10k, kni, liquidio, mlx4, mlx5, null, pcap, ring, szedata2, tap, vhost, virtio
> rx_nombuf : *only* bnx2x,bnxt,bonding,ena,enic,failsafe,mlx4,mlx5,netvsc,nfp,qede,szedata2,tap,virtio
> 
> With no way to know if we can rely on the value or not, as a DPDK user pov. The only way to know if we can rely on a given counter is to grep the driver code. Except if I missed something?
> 
> Also the doc of rte_eth_stats_get only mention io packets, bytes and errors. Not the other fields, and the way it is written let the reader think it is always supported if the function does not return 0.
> 
> I can update the doc to reflect the state of things. But maybe we could make that function return a bitmask which tells which counter has been set. But that would break the ABI... We could also have the bitmask set through a passed pointer, so it does not break code checking the return value is 0. Or maybe have the bitmask elsewhere, like for the offloads? Which fields are supported is probably a constant. So that may make more sense.

I think having capabilities, as for offload, is reasonnable.
The other option would be to push for implementing all basic stats
in all drivers, and consider an unimplemented stat as a bug.

Re: [dpdk-dev] Which counters are set by rte_eth_stats_get

[Tom Barbette]

> The other option would be to push for implementing all basic stats
> in all drivers, and consider an unimplemented stat as a bug.
It may be impossible in some cases. I think imissed is not even reported by hardware for mlx5 cards. Only an aggregate for all queues. 

I'll add a "uint64_t stats_capa" at the end of dev_info, unless you prefer something else.

Tom

Re: [dpdk-dev] Which counters are set by rte_eth_stats_get

[Stephen Hemminger]

On Fri, 09 Nov 2018 09:38:46 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> 09/11/2018 09:28, Tom Barbette:
> > Hi ethdev maintainers,
> > 
> > 
> > Support of drivers for the fields in rte_eth_stats is a bit random, and never mentioned in the doc. A quick survey showed me :
> > 
> > 
> > ipackets : implemented by all drivers
> > ibytes : all except null, ring
> > ierror : all except af_packet, ark, avf, axgbe, fm10k, kni, null, pcap, ring, szedata2, vhost
> > imissed : *only* af_packet, avp, axgbe, fm10k, kni, liquidio, mlx4, mlx5, null, pcap, ring, szedata2, tap, vhost, virtio
> > rx_nombuf : *only* bnx2x,bnxt,bonding,ena,enic,failsafe,mlx4,mlx5,netvsc,nfp,qede,szedata2,tap,virtio
> > 
> > With no way to know if we can rely on the value or not, as a DPDK user pov. The only way to know if we can rely on a given counter is to grep the driver code. Except if I missed something?
> > 
> > Also the doc of rte_eth_stats_get only mention io packets, bytes and errors. Not the other fields, and the way it is written let the reader think it is always supported if the function does not return 0.
> > 
> > I can update the doc to reflect the state of things. But maybe we could make that function return a bitmask which tells which counter has been set. But that would break the ABI... We could also have the bitmask set through a passed pointer, so it does not break code checking the return value is 0. Or maybe have the bitmask elsewhere, like for the offloads? Which fields are supported is probably a constant. So that may make more sense.  
> 
> I think having capabilities, as for offload, is reasonnable.
> The other option would be to push for implementing all basic stats
> in all drivers, and consider an unimplemented stat as a bug.
> 
> 
> 

More capabablities makes it harder for applications. 
For the examples you give, some of these are just *bugs* in the drivers. Like the ibytes field.
Let's fix the bugs rather than expect application to workaround them.


For others, if the driver has no places it allocates mbufs or drops packets in the driver I see no
reason that the driver needs to do anything with those fields.

Re: [dpdk-dev] Which counters are set by rte_eth_stats_get

[Tom Barbette]

> For others, if the driver has no places it allocates mbufs or drops packets in the driver I see no
> reason that the driver needs to do anything with those fields.

I understand. I'll fix the driver I use then. There may still be some counters that should increase but are not reported by hardware though.

Tom