netdev - Re: [net-next/RFC PATCH v1 1/4] net: Introduce new napi fields for rx/tx 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]

Date: Wed, 12 Jul 2023 16:11:55 -0700
From: "Nambiar, Amritha" <amritha.nambiar@...el.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: <netdev@...r.kernel.org>, <davem@...emloft.net>,
	<sridhar.samudrala@...el.com>
Subject: Re: [net-next/RFC PATCH v1 1/4] net: Introduce new napi fields for
 rx/tx queues

On 7/12/2023 2:14 PM, Jakub Kicinski wrote:
> On Wed, 12 Jul 2023 13:09:35 -0700 Nambiar, Amritha wrote:
>> On 6/2/2023 11:06 PM, Jakub Kicinski wrote:
>>> On Thu, 01 Jun 2023 10:42:25 -0700 Amritha Nambiar wrote:
>>>> Introduce new napi fields 'napi_rxq_list' and 'napi_txq_list'
>>>> for rx and tx queue set associated with the napi and
>>>> initialize them. Handle their removal as well.
>>>>
>>>> This enables a mapping of each napi instance with the
>>>> queue/queue-set on the corresponding irq line.
>>>
>>> Wouldn't it be easier to store the NAPI instance pointer in the queue?
>>> That way we don't have to allocate memory.
>>>    
>>
>> Could you please elaborate on this so I have more clarity ?
> 
> First off, let's acknowledge the fact you're asking me for
> clarifications ~40 days after I sent the feedback.
> 
Sorry about that, my vacation to be blamed.

> Pause for self-reflection.
> 
> Okay.
> 
>> Are you suggesting that there's a way to avoid maintaining the list
>> of queues in the napi struct?
> 
> Yes, why not add the napi pointer to struct netdev_queue and
> netdev_rx_queue, specifically?
> 

Yes, this would achieve the queue<->napi binding for each queue. But 
when there are multiple queues for a NAPI, I would need to walk a list 
of queues for the NAPI.

>> The idea was for netdev-genl to extract information out of
>> netdev->napi_list->napi. For tracking queues, we build a linked list
>> of queues for the napi and store it in the napi_struct. This would
>> also enable updating the napi<->queue[s] association (later with the
>> 'set' command), i.e. remove the queue[s] from the existing napi
>> instance that it is currently associated with and map with the new
>> napi instance, by simply deleting from one list and adding to the new
>> list.
> 
> Right, my point is that each queue can only be serviced by a single
> NAPI at a time, so we have a 1:N relationship. It's easier to store
> the state on the side that's the N, rather than 1.
> 
> You can add list_head to the queue structs, if you prefer to be able
> to walk queues of a NAPI more efficiently (that said the head for
> the list is in "control path only section" of napi_struct so...
> I think you don't?)

The napi pointer in the queue structs would give the napi<->queue 
mapping, I still need to walk the queues of a NAPI (when there are 
multiple queues for the NAPI), example:
'napi-id': 600, 'rx-queues': [7,6,5], 'tx-queues': [7,6,5]

in which case I would have a list of netdev queue structs within the 
napi_struct (instead of the list of queue indices that I currently have) 
to avoid memory allocation.

Does this sound right?