From: Jakub Kicinski <kuba@xxxxxxxxxx> Date: Tue, 30 Nov 2021 08:12:07 -0800 > On Tue, 30 Nov 2021 16:56:12 +0100 Alexander Lobakin wrote: > > 3. XDP and XSK ctrs separately or not. > > > > My PoV is that those are two quite different worlds. > > However, stats for actions on XSK really make a little sense since > > 99% of time we have xskmap redirect. So I think it'd be fine to just > > expand stats structure with xsk_{rx,tx}_{packets,bytes} and count > > the rest (actions, errors) together with XDP. > > > > > > Rest: > > - don't create a separate `ip` command and report under `-s`; > > - save some RTNL skb space by skipping zeroed counters. > > Let me ruin this point of clarity for you. I think that stats should > be skipped when they are not collected (see ETHTOOL_STAT_NOT_SET). > If messages get large user should use the GETSTATS call and avoid > the problem more effectively. Well, it was Dave's thought here: [0] > Another thought on this patch: with individual attributes you could save > some overhead by not sending 0 counters to userspace. e.g., define a > helper that does: I know about ETHTOOL_STAT_NOT_SET, but RTNL xstats doesn't use this, does it? GETSTATS is another thing, and I'll use it, thanks. > > > Also, regarding that I count all on the stack and then add to the > > storage once in a polling cycle -- most drivers don't do that and > > just increment the values in the storage directly, but this can be > > less performant for frequently updated stats (or it's just my > > embedded past). > > Re u64 vs u64_stats_t -- the latter is more universal and > > architecture-friendly, the former is used directly in most of the > > drivers primarily because those drivers and the corresponding HW > > are being run on 64-bit systems in the vast majority of cases, and > > Ethtools stats themselves are not so critical to guard them with > > anti-tearing. Anyways, local64_t is cheap on ARM64/x86_64 I guess? [0] https://lore.kernel.org/netdev/a4602b15-25b1-c388-73b4-1f97f6f0e555@xxxxxxxxx/ Al