netdev - Re: [net-next PATCH v3 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <83edcba4-80df-4fc0-b482-52ed71763060@intel.com>
Date: Thu, 28 Sep 2023 16:12:39 -0700
From: "Nambiar, Amritha" <amritha.nambiar@...el.com>
To: Paolo Abeni <pabeni@...hat.com>, <netdev@...r.kernel.org>,
	<kuba@...nel.org>
CC: <sridhar.samudrala@...el.com>
Subject: Re: [net-next PATCH v3 02/10] net: Add queue and napi association

On 9/28/2023 3:47 AM, Paolo Abeni wrote:
> On Tue, 2023-09-19 at 15:27 -0700, Amritha Nambiar wrote:
>> Add the napi pointer in netdev queue for tracking the napi
>> instance for each queue. This achieves the queue<->napi mapping.
>>
>> Signed-off-by: Amritha Nambiar <amritha.nambiar@...el.com>
>> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@...el.com>
>> ---
>>   include/linux/netdevice.h     |    5 +++++
>>   include/net/netdev_rx_queue.h |    2 ++
>>   net/core/dev.c                |   34 ++++++++++++++++++++++++++++++++++
>>   3 files changed, 41 insertions(+)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index db3d8429d50d..69e363918e4b 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -657,6 +657,8 @@ struct netdev_queue {
>>   
>>   	unsigned long		state;
>>   
>> +	/* NAPI instance for the queue */
>> +	struct napi_struct      *napi;
> 
> This should be probably moved into the 'read-mostly' section, before
> the '_xmit_lock' field,
> 

Agree. Will fix.

>>   +/**
>> + * netif_napi_add_queue - Associate queue with the napi
>> + * @napi: NAPI context
>> + * @queue_index: Index of queue
>> + * @type: queue type as RX or TX
>> + *
>> + * Add queue with its corresponding napi context
>> + */
>> +int netif_napi_add_queue(struct napi_struct *napi, unsigned int queue_index,
>> +			 enum netdev_queue_type type)
> 
> Very minor nit and others may have different opinion, but I feel like
> this function name is misleading, since at this point both the rx and
> tx queue should already exist. Perhaps 'netif_napi_link_queue' ?
> 

Sounds right. Since, we are basically just setting the napi for the 
queue, and this function may probably be used by both 'queue-get' and 
'queue-set' commands in future, does 'netif_queue_set_napi' suit better ?

> Cheers,
> 
> Paolo
>