[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240926040617.GA18054@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
Date: Wed, 25 Sep 2024 21:06:17 -0700
From: Shradha Gupta <shradhagupta@...ux.microsoft.com>
To: Haiyang Zhang <haiyangz@...rosoft.com>
Cc: Joe Damato <jdamato@...tly.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Shradha Gupta <shradhagupta@...rosoft.com>,
Erni Sri Satya Vennela <ernis@...rosoft.com>,
KY Srinivasan <kys@...rosoft.com>, Wei Liu <wei.liu@...nel.org>,
Dexuan Cui <decui@...rosoft.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
"open list:Hyper-V/Azure CORE AND DRIVERS" <linux-hyperv@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [RFC net-next 1/1] hv_netvsc: Link queues to NAPIs
On Wed, Sep 25, 2024 at 07:39:03PM +0000, Haiyang Zhang wrote:
>
>
> > -----Original Message-----
> > From: Joe Damato <jdamato@...tly.com>
> > Sent: Tuesday, September 24, 2024 7:49 PM
> > To: netdev@...r.kernel.org
> > Cc: Joe Damato <jdamato@...tly.com>; KY Srinivasan <kys@...rosoft.com>;
> > Haiyang Zhang <haiyangz@...rosoft.com>; Wei Liu <wei.liu@...nel.org>;
> > Dexuan Cui <decui@...rosoft.com>; David S. Miller <davem@...emloft.net>;
> > Eric Dumazet <edumazet@...gle.com>; Jakub Kicinski <kuba@...nel.org>;
> > Paolo Abeni <pabeni@...hat.com>; open list:Hyper-V/Azure CORE AND DRIVERS
> > <linux-hyperv@...r.kernel.org>; open list <linux-kernel@...r.kernel.org>
> > Subject: [RFC net-next 1/1] hv_netvsc: Link queues to NAPIs
> >
> > [You don't often get email from jdamato@...tly.com. Learn why this is
> > important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Use netif_queue_set_napi to link queues to NAPI instances so that they
> > can be queried with netlink.
> >
> > Signed-off-by: Joe Damato <jdamato@...tly.com>
> > ---
> > drivers/net/hyperv/netvsc.c | 11 ++++++++++-
> > drivers/net/hyperv/rndis_filter.c | 9 +++++++--
> > 2 files changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> > index 2b6ec979a62f..ccaa4690dba0 100644
> > --- a/drivers/net/hyperv/netvsc.c
> > +++ b/drivers/net/hyperv/netvsc.c
> > @@ -712,8 +712,11 @@ void netvsc_device_remove(struct hv_device *device)
> > for (i = 0; i < net_device->num_chn; i++) {
> > /* See also vmbus_reset_channel_cb(). */
> > /* only disable enabled NAPI channel */
> > - if (i < ndev->real_num_rx_queues)
> > + if (i < ndev->real_num_rx_queues) {
> > + netif_queue_set_napi(ndev, i,
> > NETDEV_QUEUE_TYPE_TX, NULL);
> > + netif_queue_set_napi(ndev, i,
> > NETDEV_QUEUE_TYPE_RX, NULL);
> > napi_disable(&net_device->chan_table[i].napi);
> > + }
> >
> > netif_napi_del(&net_device->chan_table[i].napi);
> > }
> > @@ -1787,6 +1790,10 @@ struct netvsc_device *netvsc_device_add(struct
> > hv_device *device,
> > netdev_dbg(ndev, "hv_netvsc channel opened successfully\n");
> >
> > napi_enable(&net_device->chan_table[0].napi);
> > + netif_queue_set_napi(ndev, 0, NETDEV_QUEUE_TYPE_RX,
> > + &net_device->chan_table[0].napi);
> > + netif_queue_set_napi(ndev, 0, NETDEV_QUEUE_TYPE_TX,
> > + &net_device->chan_table[0].napi);
> >
> > /* Connect with the NetVsp */
> > ret = netvsc_connect_vsp(device, net_device, device_info);
> > @@ -1805,6 +1812,8 @@ struct netvsc_device *netvsc_device_add(struct
> > hv_device *device,
> >
> > close:
> > RCU_INIT_POINTER(net_device_ctx->nvdev, NULL);
> > + netif_queue_set_napi(ndev, 0, NETDEV_QUEUE_TYPE_TX, NULL);
> > + netif_queue_set_napi(ndev, 0, NETDEV_QUEUE_TYPE_RX, NULL);
> > napi_disable(&net_device->chan_table[0].napi);
> >
> > /* Now, we can close the channel safely */
> > diff --git a/drivers/net/hyperv/rndis_filter.c
> > b/drivers/net/hyperv/rndis_filter.c
> > index ecc2128ca9b7..c0ceeef4fcd8 100644
> > --- a/drivers/net/hyperv/rndis_filter.c
> > +++ b/drivers/net/hyperv/rndis_filter.c
> > @@ -1269,10 +1269,15 @@ static void netvsc_sc_open(struct vmbus_channel
> > *new_sc)
> > ret = vmbus_open(new_sc, netvsc_ring_bytes,
> > netvsc_ring_bytes, NULL, 0,
> > netvsc_channel_cb, nvchan);
> > - if (ret == 0)
> > + if (ret == 0) {
> > napi_enable(&nvchan->napi);
> > - else
> > + netif_queue_set_napi(ndev, chn_index,
> > NETDEV_QUEUE_TYPE_RX,
> > + &nvchan->napi);
> > + netif_queue_set_napi(ndev, chn_index,
> > NETDEV_QUEUE_TYPE_TX,
> > + &nvchan->napi);
> > + } else {
> > netdev_notice(ndev, "sub channel open failed: %d\n",
> > ret);
> > + }
> >
> > if (atomic_inc_return(&nvscdev->open_chn) == nvscdev->num_chn)
> > wake_up(&nvscdev->subchan_open);
> > --
>
> The code change looks fine to me.
> @Shradha Gupta or @Erni Sri Satya Vennela, Do you have time to test this?
>
> Thanks,
> - Haiyang
>
>
Sure, we will review and test and get back. Thanks
Powered by blists - more mailing lists