On 11/16/20 3:27 PM, Vladimir Oltean wrote: > On Mon, Nov 16, 2020 at 03:13:47PM -0800, Jakub Kicinski wrote: >> On Tue, 17 Nov 2020 01:00:53 +0200 Vladimir Oltean wrote: >>> On Mon, Nov 16, 2020 at 02:35:44PM -0800, Jakub Kicinski wrote: >>>> On Tue, 17 Nov 2020 00:21:46 +0200 Vladimir Oltean wrote: >>>>> On Mon, Nov 16, 2020 at 01:34:53PM -0800, Jakub Kicinski wrote: >>>>>> You must expose relevant statistics via the normal get_stats64 NDO >>>>>> before you start dumping free form stuff in ethtool -S. >>>>> >>>>> Completely agree on the point, Jakub, but to be honest we don't give him >>>>> that possibility within the DSA framework today, see .ndo_get_stats64 in >>>>> net/dsa/slave.c which returns the generic dev_get_tstats64 implementation, >>>>> and not something that hooks into the hardware counters, or into the >>>>> driver at all, for that matter. >>>> >>>> Simple matter of coding, right? I don't see a problem. >>>> >>>> Also I only mentioned .ndo_get_stats64, but now we also have stats in >>>> ethtool->get_pause_stats. >>> >>> Yes, sure we can do that. The pause stats and packet counter ops would >>> need to be exposed to the drivers by DSA first, though. Not sure if this >>> is something you expect Oleksij to do or if we could pick that up separately >>> afterwards. >> >> Well, I feel like unless we draw the line nobody will have >> the incentive to do the work. >> >> I don't mind if it's Oleksij or anyone else doing the plumbing work, >> but the task itself seems rather trivial. > > So then I'll let Oleksij show his availability. > >>>>> But it's good that you raise the point, I was thinking too that we >>>>> should do better in terms of keeping the software counters in sync with >>>>> the hardware. But what would be a good reference for keeping statistics >>>>> on an offloaded interface? Is it ok to just populate the netdev counters >>>>> based on the hardware statistics? >>>> >>>> IIRC the stats on the interface should be a sum of forwarded in software >>>> and in hardware. Which in practice means interface HW stats are okay, >>>> given eventually both forwarding types end up in the HW interface >>>> (/MAC block). >>> >>> A sum? Wouldn't that count the packets sent/received by the stack twice? >> >> Note that I said _forwarded_. Frames are either forwarded by the HW or >> SW (former never hit the CPU, while the latter do hit the CPU or >> originate from it). > > Ah, you were just thinking out loud, I really did not understand what > you meant by the separation between "forwarded in software" and > "forwarded in hardware". > Yes, the hardware typically only gives us MAC-level counters anyway. > Another way to look at it is that the number of packets forwarded in > hardware from a given port are equal to the total number of RX packets > on that MAC minus the packets seen by the CPU coming from that port. > So all in all, it's the MAC-level counters we should expose in > .ndo_get_stats64, I'm glad you agree. As for the error packets, I > suppose that would be a driver-specific aggregate. > > What about RMON/RFC2819 style etherStatsPkts65to127Octets? We have a > number of switches supporting that style of counters, including the one > that Oleksij is adding support for, apparently (but not all switches > though). I suppose your M.O. is that anything standardizable is welcome > to be standardized via rtnetlink? > > Andrew, Florian, any opinions here? > Settling on RMON/RFC2819 statistics would work for me, and hopefully is not too hard to get the various drivers converted to. With respect to Oleksij's patch, I would tend to accept it so we actually have more visibility into what standardized counters are available across switch drivers. -- Florian