netdev - Re: [PATCH net-next 1/3] netdev: add per-queue statistics

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 26 Feb 2024 14:19:28 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Stanislav Fomichev <sdf@...gle.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
 pabeni@...hat.com, amritha.nambiar@...el.com, danielj@...dia.com,
 mst@...hat.com, michael.chan@...adcom.com, vadim.fedorenko@...ux.dev
Subject: Re: [PATCH net-next 1/3] netdev: add per-queue statistics

On Mon, 26 Feb 2024 13:35:34 -0800 Stanislav Fomichev wrote:
> > +  -
> > +    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.

The change is because I switched from enum to flags.

I'm not 100% sure which one is going to cause fewer issues down
the line. It's a question of whether the next scope we add will 
be disjoint with or subdividing previous scopes.

I think only subdividing previous scopes makes sense. If we were 
to add "stats per NAPI" (bad example) or "per buffer pool" or IDK what
other thing -- we should expose that as a new netlink command, not mix 
it with the queues.

The expectation is that scopes will be extended with hw vs sw, or
per-CPU (e.g. page pool recycling). In which case we'll want flags,
so that we can combine them -- ask for HW stats for a queue or hw
stats for the entire netdev.

Perhaps I should rename stats -> queue-stats to make this more explicit?

The initial version I wrote could iterate both over NAPIs and
queues. This could be helpful to some drivers - but I realized that it
would lead to rather painful user experience (does the driver maintain
stats per NAPI or per queue?) and tricky implementation of the device
level sum (device stats = Sum(queue) or Sum(queue) + Sum(NAPI)??)