netdev - Re: [net-next PATCH v8 02/10] net: Add queue and napi association

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]

Message-ID: <20231121142207.18ed9f6a@kernel.org>
Date: Tue, 21 Nov 2023 14:22:07 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: "Nambiar, Amritha" <amritha.nambiar@...el.com>
Cc: <netdev@...r.kernel.org>, <pabeni@...hat.com>,
 <sridhar.samudrala@...el.com>
Subject: Re: [net-next PATCH v8 02/10] net: Add queue and napi association

On Tue, 21 Nov 2023 13:26:27 -0800 Nambiar, Amritha wrote:
> So, currently, 'ethtool --show-channels' and 'ps -aef | grep napi' would 
> list all the queues and NAPIs if the device is DOWN. I think what you 
> are pointing at is:
> <ifdown and ./get-queues> should show something similar to <ethtool -L 
> eth0 combined 0 (0 is not valid... but almost to that effect) and 
> ./get-queues>.
> 
> But, 'ethtool -L' actually deletes the queues vs 'device DOWN' which 
> only disables or makes the queues inactive.
> 
> Maybe as a follow-up patch, would it be better to have an additional 
> parameter called 'state' for 'queues-get' and 'napi-get' that indicates 
> the queues or NAPIs as active/inactive. The queue/NAPI state would be 
> inherited from the device state. This way we can still list the 
> queues/NAPIs when the device is down, set/update parameter 
> configurations and then bring UP the device (in case where we stop 
> traffic and tune parameters).
> 
> Also, if in future, we have the interface to tune parameters per-queue 
> without full reset (of all queues or the device itself, as the hardware 
> supports this), the 'state' would report this for specific queue as 
> active/inactive. Maybe:
> 'queue-set' can set 'state = active' for a single queue '{"ifindex": 12, 
> "id": 0, "type": 0}' and start a queue.

To reiterate - the thing I find odd about the current situation is that
we hide the queues if they get disabled by lowering ethtool -L, but we
don't hide them when the entire interface is down. When the entire
interface is down there should be no queues, right?

Differently put - what logic that'd make sense to the user do we apply
when trying to decide if the queue is visible? < real_num_queues is
an implementation detail.

We can list all the queues, always, too. No preference. I just want to
make sure that the rules are clear and not very dependent on current
implementation and not different driver to driver.