On Wed, 2 Dec 2020 02:28:51 +0200 Vladimir Oltean wrote: > On Fri, Nov 27, 2020 at 10:36:42PM +0100, Andrew Lunn wrote: > > > Either way, can we conclude that ndo_get_stats64 is not a replacement > > > for ethtool -S, since the latter is blocking and, if implemented > > > correctly, > > > can return the counters at the time of the call (therefore making sure > > > that anything that happened before the syscall has been accounted into > > > the retrieved values), and the former isn't? > > > > ethtool -S is the best source of consistent, up to date statistics we > > have. It seems silly not to include everything the hardware offers > > there. > > To add to this, it would seem odd to me if we took the decision to not > expose MAC-level counters any longer in ethtool. Say the MAC has a counter > named rx_dropped. If we are only exposing this counter in ndo_get_stats64, > then we could hit the scenario where this counter keeps incrementing, > but it is the network stack who increments it, and not the MAC. > > dev_get_stats() currently does: > storage->rx_dropped += (unsigned > long)atomic_long_read(&dev->rx_dropped); > storage->tx_dropped += (unsigned > long)atomic_long_read(&dev->tx_dropped); > storage->rx_nohandler += (unsigned > long)atomic_long_read(&dev->rx_nohandler); > > thereby clobbering the MAC-provided counter. We would not know if it is > a MAC-level drop or not.
Fine granularity HW stats are fine, but the aggregate must be reported in standard stats first. The correct stat for MAC drops (AFAIU) is rx_missed. This should act as a generic "device had to drop valid packets" indication and ethtool -S should serve for manual debugging to find out which stage of pipeline / reason caused the drop.
