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: <d696c18b-c129-41c1-8a8a-f9273da1f215@intel.com>
Date: Tue, 21 Nov 2023 16:08:07 -0800
From: "Nambiar, Amritha" <amritha.nambiar@...el.com>
To: Jakub Kicinski <kuba@...nel.org>
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 11/21/2023 2:22 PM, Jakub Kicinski wrote:
> 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?
> 

"When the entire interface is down there should be no queues" - 
currently, 'ethtool --show-channels' reports all the available queues 
when interface is DOWN (for all drivers, as drivers don't set 
real_num_queues to 0). Is this incorrect?

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

I think currently, the queue dump results when the device is down aligns 
for both APIs (netdev-genl queue-get and ethtool show-channels) for all 
the drivers. If we decide to NOT show queues/NAPIs (with netdev-genl) 
when the device is down, the user would see conflicting results, the 
dump results with netdev-genl APIs would be different from what 'ethtool 
--show-channels' and 'ps -aef | grep napi' reports.