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 14:14:42 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: "Nambiar, Amritha" <amritha.nambiar@...el.com>
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 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.

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?

> 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?)