[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<SA1PR21MB133546A5D3B1E471E4021A00BF7C2@SA1PR21MB1335.namprd21.prod.outlook.com>
Date: Wed, 31 Jan 2024 17:40:18 +0000
From: Dexuan Cui <decui@...rosoft.com>
To: Haiyang Zhang <haiyangz@...rosoft.com>, Shradha Gupta
<shradhagupta@...ux.microsoft.com>
CC: KY Srinivasan <kys@...rosoft.com>, Wei Liu <wei.liu@...nel.org>, "David S.
Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Wojciech Drewek
<wojciech.drewek@...el.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>, Shradha Gupta <shradhagupta@...rosoft.com>,
"stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH] hv_netvsc:Register VF in netvsc_probe if
NET_DEVICE_REGISTER missed
> From: Haiyang Zhang <haiyangz@...rosoft.com>
> Sent: Wednesday, January 31, 2024 8:46 AM
> [...]
> > From: Shradha Gupta <shradhagupta@...ux.microsoft.com>
> > Sent: Wednesday, January 31, 2024 2:54 AM
> > > [...]
> > > > + netvsc_prepare_bonding(vf_netdev);
> > > > + netvsc_register_vf(vf_netdev, VF_REG_IN_PROBE);
> > > > + __netvsc_vf_setup(net, vf_netdev);
> > >
> > > add a "break;' ?
> > With MANA devices and multiport support there, the individual ports are
> > also net_devices.
> > Wouldn't this be needed for such scenario(where we have multiple mana
> > port net devices) to
> > register them all?
>
> Each device has separate probe() call, so only one VF will match in one
> netvsc_probe().
>
> netvsc_prepare_bonding() & netvsc_register_vf() have
> get_netvsc_byslot(vf_netdev), but __netvsc_vf_setup() doesn't have. So,
> in case of multi-Vfs, this code will run "this" netvsc NIC with multiple VFs by
> __netvsc_vf_setup() which isn't correct.
>
> You need to add the following lines before
> netvsc_prepare_bonding(vf_netdev)
> in netvsc_probe() to skip non-matching VFs:
>
> if (net != get_netvsc_byslot(vf_netdev))
> continue;
Haiyang is correct.
I think it's still good to add a "break;", e.g. my understanding is something
like the below (this is untested):
+static struct net_device *get_matching_netvsc_dev(net_device *event_ndev)
+{
+ /* Skip NetVSC interfaces */
+ if (event_ndev->netdev_ops == &device_ops)
+ return NULL;
+
+ /* Avoid non-Ethernet type devices */
+ if (event_ndev->type != ARPHRD_ETHER)
+ return NULL;
+
+ /* Avoid Vlan dev with same MAC registering as VF */
+ if (is_vlan_dev(event_ndev))
+ return NULL;
+
+ /* Avoid Bonding master dev with same MAC registering as VF */
+ if (netif_is_bond_master(event_ndev))
+ return NULL;
+
+ return get_netvsc_byslot(event_ndev);
+}
+ for_each_netdev(dev_net(net), vf_netdev) {
+ if (get_matching_netvsc_dev(event_dev) != net)
+ continue;
+
+ netvsc_prepare_bonding(vf_netdev);
+ netvsc_register_vf(vf_netdev, VF_REG_IN_PROBE);
+ __netvsc_vf_setup(net, vf_netdev);
+
+ break;
+ }
We can also use get_matching_netvsc_dev() in netvsc_netdev_event().
BTW, please add a space between "hv_netvsc:" and "Register" in the Subject.
Powered by blists - more mailing lists