> -----Original Message-----
> From: Shradha Gupta <[email protected]>
> Sent: Thursday, September 26, 2024 6:35 AM
> To: Haiyang Zhang <[email protected]>
> Cc: Joe Damato <[email protected]>; [email protected]; Shradha
> Gupta <[email protected]>; Erni Sri Satya Vennela
> <[email protected]>; KY Srinivasan <[email protected]>; Wei Liu
> <[email protected]>; Dexuan Cui <[email protected]>; David S. Miller
> <[email protected]>; Eric Dumazet <[email protected]>; Jakub Kicinski
> <[email protected]>; Paolo Abeni <[email protected]>; open list:Hyper-
> V/Azure CORE AND DRIVERS <[email protected]>; open list
> <[email protected]>
> 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 <[email protected]>
> > > Sent: Tuesday, September 24, 2024 7:49 PM
> > > To: [email protected]
> > > Cc: Joe Damato <[email protected]>; KY Srinivasan
> <[email protected]>;
> > > Haiyang Zhang <[email protected]>; Wei Liu <[email protected]>;
> > > Dexuan Cui <[email protected]>; David S. Miller
> <[email protected]>;
> > > Eric Dumazet <[email protected]>; Jakub Kicinski <[email protected]>;
> > > Paolo Abeni <[email protected]>; open list:Hyper-V/Azure CORE AND
> DRIVERS
> > > <[email protected]>; open list <linux-
> [email protected]>
> > > Subject: [RFC net-next 1/1] hv_netvsc: Link queues to NAPIs
> > >
> > > [You don't often get email from [email protected]. 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 <[email protected]>
> > > ---
> > > 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
> >
> >
> Hi Joe, Haiyang,
>
> I have verified the patch on a VM with netvsc interfaces and the seems
> to be working as expected
>
> CLI output after applying the patch:
>
> [{'id': 0, 'ifindex': 2, 'napi-id': 8193, 'type': 'rx'},
> {'id': 1, 'ifindex': 2, 'napi-id': 8194, 'type': 'rx'},
> {'id': 2, 'ifindex': 2, 'napi-id': 8195, 'type': 'rx'},
> {'id': 3, 'ifindex': 2, 'napi-id': 8196, 'type': 'rx'},
> {'id': 4, 'ifindex': 2, 'napi-id': 8197, 'type': 'rx'},
> {'id': 5, 'ifindex': 2, 'napi-id': 8198, 'type': 'rx'},
> {'id': 6, 'ifindex': 2, 'napi-id': 8199, 'type': 'rx'},
> {'id': 7, 'ifindex': 2, 'napi-id': 8200, 'type': 'rx'},
> {'id': 0, 'ifindex': 2, 'napi-id': 8193, 'type': 'tx'},
> {'id': 1, 'ifindex': 2, 'napi-id': 8194, 'type': 'tx'},
> {'id': 2, 'ifindex': 2, 'napi-id': 8195, 'type': 'tx'},
> {'id': 3, 'ifindex': 2, 'napi-id': 8196, 'type': 'tx'},
> {'id': 4, 'ifindex': 2, 'napi-id': 8197, 'type': 'tx'},
> {'id': 5, 'ifindex': 2, 'napi-id': 8198, 'type': 'tx'},
> {'id': 6, 'ifindex': 2, 'napi-id': 8199, 'type': 'tx'},
> {'id': 7, 'ifindex': 2, 'napi-id': 8200, 'type': 'tx'}]
>
> The code changes also look good.
>
> Tested-by: Shradha Gupta <[email protected]>
>
Thank you for the testing!
- Haiyang
