linux-kernel - Re: [PATCH net-next 07/16] idpf: link NAPIs to queues

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: <5c2dd43f-2890-44db-a5ab-68a86dd2d89a@intel.com>
Date: Wed, 12 Mar 2025 18:16:11 +0100
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Eric Dumazet <edumazet@...gle.com>
CC: <intel-wired-lan@...ts.osuosl.org>, Michal Kubiak
	<michal.kubiak@...el.com>, Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
	Tony Nguyen <anthony.l.nguyen@...el.com>, Przemek Kitszel
	<przemyslaw.kitszel@...el.com>, Andrew Lunn <andrew+netdev@...n.ch>, "David
 S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, "Paolo
 Abeni" <pabeni@...hat.com>, Alexei Starovoitov <ast@...nel.org>, "Daniel
 Borkmann" <daniel@...earbox.net>, Jesper Dangaard Brouer <hawk@...nel.org>,
	John Fastabend <john.fastabend@...il.com>, Simon Horman <horms@...nel.org>,
	<bpf@...r.kernel.org>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 07/16] idpf: link NAPIs to queues

From: Eric Dumazet <edumazet@...gle.com>
Date: Fri, 7 Mar 2025 11:28:36 +0100

> On Wed, Mar 5, 2025 at 5:22 PM Alexander Lobakin
> <aleksander.lobakin@...el.com> wrote:
>>
>> Add the missing linking of NAPIs to netdev queues when enabling
>> interrupt vectors in order to support NAPI configuration and
>> interfaces requiring get_rx_queue()->napi to be set (like XSk
>> busy polling).
>>
>> Signed-off-by: Alexander Lobakin <aleksander.lobakin@...el.com>
>> ---
>>  drivers/net/ethernet/intel/idpf/idpf_txrx.c | 30 +++++++++++++++++++++
>>  1 file changed, 30 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
>> index 2f221c0abad8..a3f6e8cff7a0 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
>> @@ -3560,8 +3560,11 @@ void idpf_vport_intr_rel(struct idpf_vport *vport)
>>  static void idpf_vport_intr_rel_irq(struct idpf_vport *vport)
>>  {
>>         struct idpf_adapter *adapter = vport->adapter;
>> +       bool unlock;
>>         int vector;
>>
>> +       unlock = rtnl_trylock();
> 
> This is probably not what you want here ?
> 
> If another thread is holding RTNL, then rtnl_ttrylock() will not add
> any protection.

Yep I know. trylock() is because this function can be called in two
scenarios:

1) .ndo_close(), when RTNL is already locked;
2) "soft reset" aka "stop the traffic, reallocate the queues, start the
   traffic", when RTNL is not taken.

The second one spits a WARN without the RTNL being locked. So this
trylock() will do nothing for the first scenario and will take the lock
for the second one.

If that is not correct, let me know, I'll do it a different way (maybe
it's better to unconditionally take the lock on the callsite for the
second case?).

Thanks,
Olek