Received-SPF: pass (google.com: domain of linux-kernel+bounces-46916-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Dexuan Cui <decui@microsoft.com>
To: Haiyang Zhang <haiyangz@microsoft.com>, Shradha Gupta
	<shradhagupta@linux.microsoft.com>
CC: KY Srinivasan <kys@microsoft.com>, Wei Liu <wei.liu@kernel.org>, "David S.
 Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub
 Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Wojciech Drewek
	<wojciech.drewek@intel.com>, "linux-hyperv@vger.kernel.org"
	<linux-hyperv@vger.kernel.org>, "netdev@vger.kernel.org"
	<netdev@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, Shradha Gupta <shradhagupta@microsoft.com>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: RE: [PATCH] hv_netvsc:Register VF in netvsc_probe if
 NET_DEVICE_REGISTER missed
Thread-Topic: [PATCH] hv_netvsc:Register VF in netvsc_probe if
 NET_DEVICE_REGISTER missed
Thread-Index: AQHaU0yZPmSUZITQDkq6vegFrHdKtbDyyvGAgADDxwCAAI9LcIAACpLg
Date: Wed, 31 Jan 2024 17:40:18 +0000
Message-ID:
 <SA1PR21MB133546A5D3B1E471E4021A00BF7C2@SA1PR21MB1335.namprd21.prod.outlook.com>
References:
 <1706599135-12651-1-git-send-email-shradhagupta@linux.microsoft.com>
 <SA1PR21MB1335C5554F769454AAEDE1C8BF7D2@SA1PR21MB1335.namprd21.prod.outlook.com>
 <20240131075404.GA18190@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
 <DS1PEPF00012A5FD2F8DBDCA1A0A58C6AC6CA7C2@DS1PEPF00012A5F.namprd21.prod.outlook.com>
In-Reply-To:
 <DS1PEPF00012A5FD2F8DBDCA1A0A58C6AC6CA7C2@DS1PEPF00012A5F.namprd21.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
msip_labels:
 MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ActionId=94a4fd25-9eef-4088-8818-4ef9b583d8ec;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=true;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Standard;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Name=Internal;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2024-01-31T16:26:55Z;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47;
authentication-results: dkim=none (message not signed)
 header.d=none;dmarc=none action=none header.from=microsoft.com;
x-ms-publictraffictype: Email
x-ms-traffictypediagnostic: SA1PR21MB1335:EE_|CY5PR21MB3770:EE_
x-ms-office365-filtering-correlation-id: b54e467b-1ba7-47f1-50e3-08dc2283b136
x-ms-exchange-senderadcheck: 1
x-ms-exchange-antispam-relay: 0
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info:
 rqo3ZVTg99ZZ6f+UJ3y6EjFWrAI0rJa0PVfQSx81RcRLLzANLUiHL8X7QOkNwLV3gW9psTALIecm4pxAzdSyODDb0X3In66CIHb4KoqXT8qUrMFnwupLGDNvizpTieqUkeyTDVLa0dC08u6/ba7Q2ffRnqbiQIRZujShEVw6krO32o2TDtxSmdQp15jmeMCN4tUVEBfFwubXiuhVuaXmDuYrbjRgTFLj9X5co6Wg7MVSPTTelRw5qmpmbPr8g4Jz4b2lvuD0yT7/6Lj+jeiK8wTWQq2alfjQ7vexP8DTO3+yvsVcpTfcZp93oZ33N7jtK0H58oC3fU2bGKhdEIWNVSVViLId3LHhWfqYDyiCjR2PsfjfSD4XHRCVsrvhDGb5K2wVcPI7o8k9uxET4m0d4F0Ui5gxXd0rlTGcSDJ/Ih7FV/QpL67ZyPazgJe4P36hdGHWlhGwF9sjlvx9cDpYZbw1UaZ740CKZyoaKGpQ3XYo2vntmP9fOB2WpsStMN4mgDtabe7khfi9wIOkPbCU8l/IxhF1dHVey33iJ2y/LZgm9WJi6B4JuGPPB48nIjtjbXXRNEfvXie0X5Rpo7Wklk8M0YOeDWCJM7V/X616RtsqRR28zNv1URSMsYdQvFpG
x-forefront-antispam-report:
 CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:SA1PR21MB1335.namprd21.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(136003)(39860400002)(346002)(376002)(396003)(366004)(230922051799003)(1800799012)(186009)(451199024)(64100799003)(26005)(38100700002)(122000001)(82950400001)(82960400001)(7696005)(6506007)(4326008)(8936002)(8676002)(66946007)(66556008)(66446008)(110136005)(54906003)(5660300002)(7416002)(316002)(66476007)(2906002)(64756008)(52536014)(41300700001)(76116006)(71200400001)(86362001)(9686003)(8990500004)(10290500003)(478600001)(38070700009)(33656002)(55016003);DIR:OUT;SFP:1102;
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?VAPdCy+GMtJps8eVWO6RqlD8RqHxmyPQfaAyIb1on/Krd9+JA04vBD9Z63O+?=
 =?us-ascii?Q?FODtmQUGIWUM3LR5IWjxdM7LWYVr5AgCEqRTIvdZCF0UC5dNOvZkPxxaunUZ?=
 =?us-ascii?Q?VTb7926CWoUZBrlqiEFtxQ/NeOLSEnF2gkoWKH6KjlUC/MQh5RzbG9zKNqqP?=
 =?us-ascii?Q?ZLN0RxLcv5UPxICYDAvhZMiWewg2jE3eZQ3aoNzexT+yLojWi++UYKrhi0Xk?=
 =?us-ascii?Q?2xlIVYEyA9K8fpIde9aneIr4iIEAJdTbX2G10nxBUjiD1crQHpytxOXq8ucA?=
 =?us-ascii?Q?9z0XfWt64bk6llyEextF8jiF2VS55yktFw7jnNIs+micyhELIQZgEqDL9Cyn?=
 =?us-ascii?Q?zN1z7LUiNlnH2kBLr0AnY9hHdaxEUa1ce/AiD+ADWUsgZiP45LyUoc+4hHrH?=
 =?us-ascii?Q?TiRxsyQKk/WAXoZLmqSVMNWAOYm0asuoybI1wI6DlL0kZqC3kpBQC0f54GNB?=
 =?us-ascii?Q?TCDpJ6Ih1FV3raflIzhXtxkixeMugKj4gjHtRSMWziYwEmQUD/mlrY+mPs+C?=
 =?us-ascii?Q?kEMrO+z85/E5fH93jXWEZhFl7Rd4GBctzqmq1ZIhxOWLi6PVcdsAMVs+sMin?=
 =?us-ascii?Q?1oXmm+u/dxAfAPHJkP99Qny3di/KsFCMDh0OeP5WFHavlmmS/8Aa+gW3T15h?=
 =?us-ascii?Q?jfVFlQr7dY3ZVXzT+nMYmLOU+1RgDzQ3pFuOpPUxeWGKMmCL7jrOmaXN5PYM?=
 =?us-ascii?Q?sH+Qs5uB9fV1nrGxbVKoj/kajZyo35XrVpmbPBmcKP5VwofKI8BzoMVWfAQ3?=
 =?us-ascii?Q?hOQ5/RXAmT950zvNpA2g4y2StBHL6FlKS03c9yNG07JNeYAKQgmkDJJSodkl?=
 =?us-ascii?Q?aYLKt51eZgmmVMgfZr87aQnTER0rwfUB5CRmsM/LuZ9fsrhjRyLV2bcLue7I?=
 =?us-ascii?Q?zGEs2Rtls6Xk4qZrN4Lt6VmyiWxZJwbHuZ11IFCWtUuMeD4WxEiTc11KZDx6?=
 =?us-ascii?Q?BfQ9GnMAr+RnA62oheZRHOEOm/mfKlRORUnOczPaS9VX4qjH1jFDAiwacThN?=
 =?us-ascii?Q?xUar3LVDiZaKIKsI2bv1f90cD5pBIjYDnlM/EeplZxYFr/ZFBH+gBQ4855kB?=
 =?us-ascii?Q?1/LSu64AAUyJtmBNaypvUG9i/P9Hx8TOj0qbQbr6Ewovid+orsypefbkjj6O?=
 =?us-ascii?Q?qszdSa77cB9XP66CFGlFEPzPGvqCwec8Qcsmky6riGic4yN1rUXEZQUFGwZC?=
 =?us-ascii?Q?ebeyscIp5VviQ26KiZUQ6WcjrifNEv2i5ooWn8gL0IaZbrhczA6mq4YPLzY/?=
 =?us-ascii?Q?diDsH4QVtSeMU5om577AVuuEETL9G2w3812z4WiUApEFAa2fIwaczEDXnyQA?=
 =?us-ascii?Q?ikfOX4yhJW6TBYJNO9jCd2HwzDcaCP7qHsFw9htNhEnMso2kLX66Q8vILmno?=
 =?us-ascii?Q?pTIoPiff4qpqNxLg+dJVyW8Bazl/c0aNaGnaS/IzV4fMfgsv8jspIy2iRlTw?=
 =?us-ascii?Q?zrub0dx+8NYR5t+bd3RMicu/MXJWrjvL6NQKKThXsOG58i0L2K7tJGynJVxM?=
 =?us-ascii?Q?BDCh5W+J8FpQHrXYGOz/TXaA98A4R9Bn9G3T6xr89NmN54/KhbRwQHDILSbt?=
 =?us-ascii?Q?hmhO+bdqaP3SK3pinbalS9hFnnRm9+ZQIjf0pk+U?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: microsoft.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SA1PR21MB1335.namprd21.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b54e467b-1ba7-47f1-50e3-08dc2283b136
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Jan 2024 17:40:18.8362
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: BlbvIRQn2uCTNCkMK4p2E56dOdtwVpH1LtEJ8SBNee7oyrWzgwfU5gzZEFJSa+/pIxbEVV7qRKLnhtnDt5sKGQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR21MB3770

> From: Haiyang Zhang <haiyangz@microsoft.com>
> Sent: Wednesday, January 31, 2024 8:46 AM
>  [...]
> > From: Shradha Gupta <shradhagupta@linux.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?
>=20
> Each device has separate probe() call, so only one VF will match in one
> netvsc_probe().
>=20
> 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.
>=20
> You need to add the following lines before
> netvsc_prepare_bonding(vf_netdev)
> in netvsc_probe() to skip non-matching VFs:
>=20
> if (net !=3D get_netvsc_byslot(vf_netdev))
> 	continue;

Haiyang is correct.
I think it's still good to add a "break;", e.g. my understanding is somethi=
ng
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 =3D=3D &device_ops)
+               return NULL;
+
+       /* Avoid non-Ethernet type devices */
+       if (event_ndev->type !=3D 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) !=3D 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.