netdev - RE: [PATCH v2] net: netvsc: Update default VMBus channels

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:
 <CH2PPF910B3338DB4798D086CB50600C2FBCA802@CH2PPF910B3338D.namprd21.prod.outlook.com>
Date: Thu, 15 Aug 2024 19:23:50 +0000
From: Haiyang Zhang <haiyangz@...rosoft.com>
To: Jakub Kicinski <kuba@...nel.org>, Erni Sri Satya Vennela
	<ernis@...ux.microsoft.com>, Erni Sri Satya Vennela <ernis@...rosoft.com>
CC: KY Srinivasan <kys@...rosoft.com>, "wei.liu@...nel.org"
	<wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "linux-hyperv@...r.kernel.org"
	<linux-hyperv@...r.kernel.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2] net: netvsc: Update default VMBus channels



> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Thursday, August 15, 2024 12:09 PM
> To: Erni Sri Satya Vennela <ernis@...ux.microsoft.com>
> Cc: KY Srinivasan <kys@...rosoft.com>; Haiyang Zhang
> <haiyangz@...rosoft.com>; wei.liu@...nel.org; Dexuan Cui
> <decui@...rosoft.com>; davem@...emloft.net; edumazet@...gle.com;
> pabeni@...hat.com; linux-hyperv@...r.kernel.org; netdev@...r.kernel.org;
> linux-kernel@...r.kernel.org; Erni Sri Satya Vennela <ernis@...rosoft.com>
> Subject: Re: [PATCH v2] net: netvsc: Update default VMBus channels
> 
> On Wed, 14 Aug 2024 09:59:13 -0700 Erni Sri Satya Vennela wrote:
> > Change VMBus channels macro (VRSS_CHANNEL_DEFAULT) in
> > Linux netvsc from 8 to 16 to align with Azure Windows VM
> > and improve networking throughput.
> >
> > For VMs having less than 16 vCPUS, the channels depend
> > on number of vCPUs. Between 16 to 32 vCPUs, the channels
> > default to VRSS_CHANNEL_DEFAULT. For greater than 32 vCPUs,
> > set the channels to number of physical cores / 2 as a way
> > to optimize CPU resource utilization and scale for high-end
> > processors with many cores.
> > Maximum number of channels are by default set to 64.
> >
> > Based on this change the subchannel creation would change as follows:
> >
> > -------------------------------------------------------------
> > |No. of vCPU	|dev_info->num_chn	|subchannel created |
> > -------------------------------------------------------------
> > |  0-16		|	16		|	vCPU	    |
> > | >16 & <=32	|	16		|	16          |
> > | >32 & <=128	|	vCPU/2		|	vCPU/2      |
> > | >128		|	vCPU/2		|	64          |
> > -------------------------------------------------------------
> >
> > Performance tests showed significant improvement in throughput:
> > - 0.54% for 16 vCPUs
> > - 0.83% for 32 vCPUs
> > - 1.76% for 48 vCPUs
> > - 10.35% for 64 vCPUs
> > - 13.47% for 96 vCPUs
> 
> Is there anything that needs clarifying in my feedback on v1?
> 
> https://lore.kernel.org/all/20240807201857.445f9f95@kernel.org/
> 
> Ignoring maintainer feedback is known to result in angry outbursts.

Your suggestion on netif_get_num_default_rss_queues() is not ignored.
We discussed internally on the formula we used for the num_chn, and
chose a similar formula for higher number of vCPUs as in 
netif_get_num_default_rss_queues().
For lower number of vCPUs, we use the same default as Windows guests,
because we don't want any potential regression.

So, the end result is a step function:
> > -------------------------------------------------------------
> > |No. of vCPU	|dev_info->num_chn	|subchannel created |
> > -------------------------------------------------------------
> > |  0-16		|	16			|	vCPU	      |
> > | >16 & <=32	|	16			|	16          |
> > | >32 & <=128	|	vCPU/2		|	vCPU/2      |
> > | >128		|	vCPU/2		|	64          |
> > -------------------------------------------------------------

@Erni Sri Satya Vennela
Next time, please reply to maintainer's emails to LKML, regarding
how you think of the suggestions.

Also, netif_get_num_default_rss_queues() uses #phys cores, which
is different from num_online_cpus().
You can try like below, in addition to your comparison test, see
if it's better than the patch v2.

	dev_info->num_chn = netif_get_num_default_rss_queues();
	if (dev_info->num_chn < VRSS_CHANNEL_DEFAULT)
		dev_info->num_chn = VRSS_CHANNEL_DEFAULT;

Thanks,
- Haiyang