Re: [PATCH net-next v2 2/3] net: dsa: add Arrow SpeedChips XRS700x driver

Hi Jakub,

On Fri, Nov 27, 2020 at 10:36:42PM +0100, Andrew Lunn wrote:

quoted

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.