Re: [PATCH v2 net-next 00/26] net: introduce and use generic XDP stats

From: Jakub Kicinski
Date: Tue Nov 30 2021 - 11:12:17 EST


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.

> 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?