netdev - Re: [PATCH net-next v8 3/6] net: napi: add CPU affinity to napi

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: <7dd230fb-ca58-4407-98df-03ddb2ac68a9@intel.com>
Date: Thu, 13 Feb 2025 08:45:21 -0700
From: Ahmed Zaki <ahmed.zaki@...el.com>
To: Paolo Abeni <pabeni@...hat.com>, <netdev@...r.kernel.org>
CC: <intel-wired-lan@...ts.osuosl.org>, <andrew+netdev@...n.ch>,
	<edumazet@...gle.com>, <kuba@...nel.org>, <horms@...nel.org>,
	<davem@...emloft.net>, <michael.chan@...adcom.com>, <tariqt@...dia.com>,
	<anthony.l.nguyen@...el.com>, <przemyslaw.kitszel@...el.com>,
	<jdamato@...tly.com>, <shayd@...dia.com>, <akpm@...ux-foundation.org>,
	<shayagr@...zon.com>, <kalesh-anakkur.purayil@...adcom.com>,
	<pavan.chebbi@...adcom.com>
Subject: Re: [PATCH net-next v8 3/6] net: napi: add CPU affinity to
 napi_config



On 2025-02-13 5:26 a.m., Paolo Abeni wrote:
> On 2/11/25 10:06 PM, Ahmed Zaki wrote:
>> @@ -394,10 +395,8 @@ struct napi_struct {
>>   	struct list_head	dev_list;
>>   	struct hlist_node	napi_hash_node;
>>   	int			irq;
>> -#ifdef CONFIG_RFS_ACCEL
>>   	struct irq_affinity_notify notify;
>>   	int			napi_rmap_idx;
>> -#endif
> 
> I'm sorry for the late doubt, but it's not clear to me why you need to
> add the #ifdef in the previous patch ?!?

It was there to make the code consistent, since the rmap and the 
notifier were only needed for ARFS.

It can be removed, although I am not sure if there would be any warnings 
since on !CONFIG_ARFS_ACCEL the fields would never be used.

> 
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 209296cef3cd..d2c942bbd5e6 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -6871,28 +6871,39 @@ void netif_queue_set_napi(struct net_device *dev, unsigned int queue_index,
>>   }
>>   EXPORT_SYMBOL(netif_queue_set_napi);
>>   
>> -#ifdef CONFIG_RFS_ACCEL
>>   static void
>> -netif_irq_cpu_rmap_notify(struct irq_affinity_notify *notify,
>> -			  const cpumask_t *mask)
>> +netif_napi_irq_notify(struct irq_affinity_notify *notify,
>> +		      const cpumask_t *mask)
>>   {
>>   	struct napi_struct *napi =
>>   		container_of(notify, struct napi_struct, notify);
>> +#ifdef CONFIG_RFS_ACCEL
>>   	struct cpu_rmap *rmap = napi->dev->rx_cpu_rmap;
>>   	int err;
>> +#endif
>>   
>> -	err = cpu_rmap_update(rmap, napi->napi_rmap_idx, mask);
>> -	if (err)
>> -		netdev_warn(napi->dev, "RMAP update failed (%d)\n",
>> -			    err);
>> +	if (napi->config && napi->dev->irq_affinity_auto)
>> +		cpumask_copy(&napi->config->affinity_mask, mask);
>> +
>> +#ifdef CONFIG_RFS_ACCEL
>> +	if (napi->dev->rx_cpu_rmap_auto) {
>> +		err = cpu_rmap_update(rmap, napi->napi_rmap_idx, mask);
>> +		if (err)
>> +			netdev_warn(napi->dev, "RMAP update failed (%d)\n",
>> +				    err);
>> +	}
>> +#endif
> 
> Minor nit: if you provide a netif_rx_cpu_rmap() helper returning
> dev->rx_cpu_rmap or NULL for !CONFIG_RFS_ACCEL build, you can avoid the
> above 2 ifdefs and possibly more below.
> 

Thanks, I will add this if there is a new version.


>> @@ -6915,7 +6926,6 @@ static int napi_irq_cpu_rmap_add(struct napi_struct *napi, int irq)
>>   	if (rc)
>>   		goto err_set;
>>   
>> -	set_bit(NAPI_STATE_HAS_NOTIFIER, &napi->state);
> 
> Minor nit: I think it would be better if the previous patch would add
> directly this line in netif_napi_set_irq_locked() (avoding the removal
> here).
> 

yes, it just made more sense for that patch.