Re: [PATCH net-next 1/3] netdev: add per-queue statistics
From: Stanislav Fomichev <hidden>
Date: 2024-02-26 21:35:36
On 02/26, Jakub Kicinski wrote:
quoted hunk ↗ jump to hunk
The ethtool-nl family does a good job exposing various protocol related and IEEE/IETF statistics which used to get dumped under ethtool -S, with creative names. Queue stats don't have a netlink API, yet, and remain a lion's share of ethtool -S output for new drivers. Not only is that bad because the names differ driver to driver but it's also bug-prone. Intuitively drivers try to report only the stats for active queues, but querying ethtool stats involves multiple system calls, and the number of stats is read separately from the stats themselves. Worse still when user space asks for values of the stats, it doesn't inform the kernel how big the buffer is. If number of stats increases in the meantime kernel will overflow user buffer. Add a netlink API for dumping queue stats. Queue information is exposed via the netdev-genl family, so add the stats there. Support per-queue and sum-for-device dumps. Latter will be useful when subsequent patches add more interesting common stats than just bytes and packets. The API does not currently distinguish between HW and SW stats. The expectation is that the source of the stats will either not matter much (good packets) or be obvious (skb alloc errors). Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- Documentation/netlink/specs/netdev.yaml | 84 +++++++++ Documentation/networking/statistics.rst | 17 +- include/linux/netdevice.h | 3 + include/net/netdev_queues.h | 54 ++++++ include/uapi/linux/netdev.h | 19 +++ net/core/netdev-genl-gen.c | 12 ++ net/core/netdev-genl-gen.h | 2 + net/core/netdev-genl.c | 217 ++++++++++++++++++++++++ tools/include/uapi/linux/netdev.h | 19 +++ 9 files changed, 426 insertions(+), 1 deletion(-)diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml index 3addac970680..2570cc371fc8 100644 --- a/Documentation/netlink/specs/netdev.yaml +++ b/Documentation/netlink/specs/netdev.yaml@@ -74,6 +74,10 @@ name: netdev name: queue-type type: enum entries: [ rx, tx ] + - + name: stats-scope + type: flags + entries: [ queue ]
IIUC, in order to get netdev-scoped stats in v1 (vs rfc) is to not set stats-scope, right? Any reason we dropped the explicit netdev entry? It seems more robust with a separate entry and removes the ambiguity about which stats we're querying.