[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20240827045521.GA17487@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
Date: Mon, 26 Aug 2024 21:55:21 -0700
From: Erni Sri Satya Vennela <ernis@...ux.microsoft.com>
To: Michael Kelley <mhklinux@...look.com>
Cc: "kys@...rosoft.com" <kys@...rosoft.com>,
"haiyangz@...rosoft.com" <haiyangz@...rosoft.com>,
"wei.liu@...nel.org" <wei.liu@...nel.org>,
"decui@...rosoft.com" <decui@...rosoft.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>,
"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>,
"ernis@...rosoft.com" <ernis@...rosoft.com>
Subject: Re: [PATCH v3] net: netvsc: Update default VMBus channels
On Fri, Aug 23, 2024 at 04:37:17PM +0000, Michael Kelley wrote:
> From: Erni Sri Satya Vennela <ernis@...ux.microsoft.com> Sent: Thursday, August 22, 2024 8:06 AM
> >
> > 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 64 vCPUs, the channels
> > default to VRSS_CHANNEL_DEFAULT. For greater than 64 vCPUs,
> > set the channels to number of physical cores / 2 returned by
> > netif_get_num_default_rss_queues() as a way to optimize CPU
> > resource utilization and scale for high-end processors with
> > many cores. Due to hyper-threading, the number of
> > physical cores = vCPUs/2.
>
> But note that a given physical processor may or may not support
> hyper-threading. For example, the physical processor used for
> ARM64 VMs in Azure does not have hyper-threading. And even
> if the physical processor supports hyper-threading, the VM might
> not see hyper-threading as enabled. Many Azure GPU-based VM
> sizes see only full cores, with no hyper-threading. It's also possible
> to boot Linux with hyper-threading disabled even if the VM sees
> hyper-threaded cores (the "nosmt" or "smt=1" kernel boot option).
>
> Your code below probably isn't affected when hyper-threading
> isn't present. But in the interest of accuracy, the discussion here
> in the commit message should qualify the use of "vCPU/4" as
> the number of channels. It might be "vCPU/2" when
> hyper-threading isn't present or is disabled, and for vCPU
> counts between 16 and 64, you'll get more than 16 channels.
>
> > Maximum number of channels are by default set to 64.
> >
> > Based on this change the channel creation would change as follows:
> >
> > -------------------------------------------------------------
> > | No. of vCPU | dev_info->num_chn | channels created |
> > -------------------------------------------------------------
> > | 0-16 | 16 | vCPU |
>
> Nit: Presumably we won't ever have 0 vCPUs. :-)
>
> > | >16 & <=64 | 16 | 16 |
> > | >64 & <=256 | vCPU/4 | vCPU/4 |
> > | >256 | vCPU/4 | 64 |
> > -------------------------------------------------------------
> >
> > Performance tests showed significant improvement in throughput:
> > - 0.54% for 16 vCPUs
> > - 0.83% for 32 vCPUs
> > - 0.86% for 48 vCPUs
> > - 9.72% for 64 vCPUs
> > - 13.57% for 96 vCPUs
> >
> > Signed-off-by: Erni Sri Satya Vennela <ernis@...ux.microsoft.com>
> > Reviewed-by: Haiyang Zhang <haiyangz@...rosoft.com>
> > Reviewed-by: Shradha Gupta <shradhagupta@...ux.microsoft.com>
> > ---
> > Changes in v3:
> > * Use netif_get_num_default_rss_queues() to set channels
> > * Change terminology for channels in commit message
> > ---
> > Changes in v2:
> > * Set dev_info->num_chn based on vCPU count.
> > ---
> > drivers/net/hyperv/hyperv_net.h | 2 +-
> > drivers/net/hyperv/netvsc_drv.c | 3 ++-
> > 2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
> > index 810977952f95..e690b95b1bbb 100644
> > --- a/drivers/net/hyperv/hyperv_net.h
> > +++ b/drivers/net/hyperv/hyperv_net.h
> > @@ -882,7 +882,7 @@ struct nvsp_message {
> >
> > #define VRSS_SEND_TAB_SIZE 16 /* must be power of 2 */
> > #define VRSS_CHANNEL_MAX 64
> > -#define VRSS_CHANNEL_DEFAULT 8
> > +#define VRSS_CHANNEL_DEFAULT 16
> >
> > #define RNDIS_MAX_PKT_DEFAULT 8
> > #define RNDIS_PKT_ALIGN_DEFAULT 8
> > diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> > index 44142245343d..a6482afe4217 100644
> > --- a/drivers/net/hyperv/netvsc_drv.c
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> > @@ -987,7 +987,8 @@ struct netvsc_device_info *netvsc_devinfo_get(struct netvsc_device *nvdev)
> > dev_info->bprog = prog;
> > }
> > } else {
> > - dev_info->num_chn = VRSS_CHANNEL_DEFAULT;
> > + dev_info->num_chn = max(VRSS_CHANNEL_DEFAULT,
> > + netif_get_num_default_rss_queues());
> > dev_info->send_sections = NETVSC_DEFAULT_TX;
> > dev_info->send_section_size = NETVSC_SEND_SECTION_SIZE;
> > dev_info->recv_sections = NETVSC_DEFAULT_RX;
> > --
> > 2.34.1
> >
Thank you for the knowledge you shared. I’ll update the patch
with your suggestions in the next version.
Powered by blists - more mailing lists