Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
From:   Haiyang Zhang <haiyangz@microsoft.com>
To:     Wojciech Drewek <wojciech.drewek@intel.com>,
        "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>
CC:     KY Srinivasan <kys@microsoft.com>,
        "wei.liu@kernel.org" <wei.liu@kernel.org>,
        Dexuan Cui <decui@microsoft.com>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: RE: [PATCH net] hv_netvsc: fix race of netvsc and VF
 register_netdevice
Thread-Topic: [PATCH net] hv_netvsc: fix race of netvsc and VF
 register_netdevice
Thread-Index: AQHaB4iZ4eB3Yz1VyUyB1g8GKVf8w7Bb5KsAgABD5tA=
Date:   Thu, 26 Oct 2023 14:52:07 +0000
Message-ID: <BL1PR21MB3113F0E65A58AEB1E617C2C4CADDA@BL1PR21MB3113.namprd21.prod.outlook.com>
References: <1698268592-20373-1-git-send-email-haiyangz@microsoft.com>
 <99f1f2d1-8fb0-4e56-9820-86254ce8bd02@intel.com>
In-Reply-To: <99f1f2d1-8fb0-4e56-9820-86254ce8bd02@intel.com>
Accept-Language: en-US
Content-Language: en-US
msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ActionId=84bd0587-5e81-4dbe-9d62-5f08fd7461a5;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=2023-10-26T14:50:52Z;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: BL1PR21MB3113:EE_|LV2PR21MB3061:EE_
x-ms-office365-filtering-correlation-id: 70613d90-e637-44b9-2089-08dbd6332079
x-ms-exchange-senderadcheck: 1
x-ms-exchange-antispam-relay: 0
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: 0LQdmeb7acOjrzLW7JF7gqAKQM2fuLbourRs5CAvp0g+7ndG9zBYnIvsE+Am3faLhU5YUOXUWYWaCR6W6cOlMvD7/6gXs2ck64Hk5qhRx0wC2vlH6SzsdrrHbcNBKrlV42QrMRfdYZVrM4BNDmB5EDTiMKq0EmRqMBYiQNaBlbIL8Lw1sGs5QSpJvBDDDpBtE+j2zcjC6CmydSHjwN/BfFHfhV/xfSs77m9fjqq08i/bHPSz1JOKE3Ub34lSoa/7ACvv+8Qc3F5BKqdmqXhWF2gSor8wQTlqXp1Ln/z9wYzqfUWqcUdVpINDO1KQeRLr4IUvA75vgv0ScgW1JN9lF5XWEm3b6fK9Utmr3slWYRyajEnidPBMnNWaCf2V7B5tzE2o6m48a9OnkJ/xXMwj8llWaC+ivlOo54ukbQWR+T9lcOR0vBJVPbW137i1wHKWHU3ejc67Fg0oDSkzXcH8lIGf5rQobIdTK0W32TORQJODQ/AKcJ+J2CtyUpoHkRnT1zZotEfkWjTA6woe93FHF20VOEyz5q8H55o85A+CWwYa8U/fmj0o9RMoGvbXzC3eBKoOl7XW9Ky9u2jPqmHT3KxEJ+icnquRRKE65yL7eDNpoj1JzswYKF1uJBrq+KUYzm8IBKwcbVs9K5n7cLAa3pX4Owk4BW0YHaDvQVvsIiI=
x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BL1PR21MB3113.namprd21.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(396003)(346002)(366004)(376002)(39860400002)(136003)(230922051799003)(64100799003)(451199024)(186009)(1800799009)(2906002)(7416002)(5660300002)(55016003)(38100700002)(52536014)(316002)(66899024)(26005)(8936002)(82960400001)(82950400001)(41300700001)(4326008)(38070700009)(8676002)(83380400001)(66446008)(66476007)(76116006)(9686003)(64756008)(8990500004)(54906003)(66946007)(110136005)(10290500003)(7696005)(71200400001)(6506007)(53546011)(478600001)(66556008)(122000001)(86362001)(966005)(33656002);DIR:OUT;SFP:1102;
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?L+RSukdKoR7YBcF6+eljsLy+giwC9g5QH5ZktxlBlJGXdw2r4oWbPxY4m+kP?=
 =?us-ascii?Q?wgdWuzArp07zfXK/E6ggBGjNGkEFotY9Apr5VJLgXXiFCPmIgamhLMPYWmZX?=
 =?us-ascii?Q?lTWSA+cjBqMyyDhmbqbr4LGwP8MFhj7e8Q73QWHW3U53nUbxvcyvS0jfCY4x?=
 =?us-ascii?Q?3IOoeI5Q7zqJuw0Rl5utFCWECdlZFAFa4jv1GCw4T7qU4F/xBDVHmQtYSoan?=
 =?us-ascii?Q?46rEMGtjU7ijFOO3TBW49C//I1c4qsSXIXUYem2L9Har594aYk9kHU1Faj4z?=
 =?us-ascii?Q?Fp9hT2yRUI6WZFJzODMo3XlWvsRYLXl6R6LULw0U5mtzDjgBJJy1JJUnBDD5?=
 =?us-ascii?Q?RNpiVnjGMKKcyEU0P+uGDcZmQKWcVY/OYamf5YOSibaW4Gsm3xS5pPOvUhWF?=
 =?us-ascii?Q?yV41jijvRrRToO/vtmA6LoHGsHesRG/3BuqpLtve9JEnqpslhp80zvm+m4ui?=
 =?us-ascii?Q?Hk1fZjubJGJ8yoFZQi1z+W5FBHvMzeEnbcFWPOLhMYosULnEZSE81L2m6vkg?=
 =?us-ascii?Q?SrvKkOCrdrAoYW8RzM9GfsaJskVUA95NjicrXDGBVl6gYcW9RUOnSlT4ZvUS?=
 =?us-ascii?Q?ZqjrnnZ2HqoNZ8LocgQKmWub6Ka7/EcsEdVY9dfIebimu+F2QbwlttT3obfl?=
 =?us-ascii?Q?KMtwduvHZOAU9e4AXyhHuedDmV4H7uR53rl7TPyVERizWoToXKZUETN/0hML?=
 =?us-ascii?Q?ikP8rvNqu9ou5vMCBf/G+yxE26jevPRrc6ho1ORksIdDmi1rsqnvknm35SAS?=
 =?us-ascii?Q?wTnvDMSjDnFAueqgLRm76BJOvR07fl0GBeZeOfNxMgEn847UN+m5UGP3UhDg?=
 =?us-ascii?Q?SAT8hOXcaY1FrK9gzFlw2bG6RovbZwpNV7sbVwwrwrqv4jfI8My4/h3qyYMA?=
 =?us-ascii?Q?likfnM4RM0n6TEhv1jZM0K7xnRYXgBMoeQ4+owGVI2zvwhL/4trNWFJhkeRj?=
 =?us-ascii?Q?vo5A+UOR/6hNaxjZPbOPplFR9pvfBsYzCSR06l5GKbSsk4Krr+f6yVmuANjT?=
 =?us-ascii?Q?fvt3nvz5w4IUKxRrr1LTzIPy+obPj8GXTeNVKYBdRpHTlB2IAzjj/y0Y7kBw?=
 =?us-ascii?Q?W6phbOiArPpLTutNne0gs3vwGU8BQZNO4HiY84S3aJXhqIJK2lbnDMX1UUjZ?=
 =?us-ascii?Q?U+B6t9ytiasR5k5ZZXBQvZ4ygVyMPFgHV5hxwHob6n68zgLlsWKXv5UX+p74?=
 =?us-ascii?Q?pCfs1gwaVV2a7tjJx0k91+JGPlMXCB60p9SOewckKCSuOLTKIxkUgbPveuJC?=
 =?us-ascii?Q?KcS2943EoKFXi6jwnXR3fHK6knG37oKz6S/k+lpms8rF8XbbMqU/GahY2FdL?=
 =?us-ascii?Q?XTr0feX6okfIOyA97IHBNjVNwx7Lf4BKGGxlJM9po0Mh+ehEJau29PjvBdIg?=
 =?us-ascii?Q?wdkYssPtoaPiHY4/msPPAMW4b9wkYrymNTesQiMW1LRXiZ3pdQrrO+yYB1Mj?=
 =?us-ascii?Q?Dz4v/xhnVXPpzOtp93DPzDD4Uana97arnk6Jr+XEt1FZrQi/Vy24pne/Onbc?=
 =?us-ascii?Q?6zj3WPLLwO0qYk/fhf3PFOfG74Dy8txUvcpAFVzx3uQaybGZ4EuDMsCq56kv?=
 =?us-ascii?Q?0wo1pw62H8ML+PG/XwfVnEbV7S4P0y7w+SOrYH07?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: microsoft.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BL1PR21MB3113.namprd21.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 70613d90-e637-44b9-2089-08dbd6332079
X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Oct 2023 14:52:07.9284
 (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: +xPttTAlQhCxGr4AwYhKTed6npyE76LZJIqGQXdFBO1A+FXfHO1I+JqGSMtMFO38ugVQYVR8mEArWuklvq6j+A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV2PR21MB3061
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Thu, 26 Oct 2023 07:52:29 -0700 (PDT)


> -----Original Message-----
> From: Wojciech Drewek <wojciech.drewek@intel.com>
> Sent: Thursday, October 26, 2023 6:48 AM
> To: Haiyang Zhang <haiyangz@microsoft.com>; linux-hyperv@vger.kernel.org;
> netdev@vger.kernel.org
> Cc: KY Srinivasan <kys@microsoft.com>; wei.liu@kernel.org; Dexuan Cui
> <decui@microsoft.com>; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; davem@davemloft.net; linux-kernel@vger.kernel.org;
> stable@vger.kernel.org
> Subject: Re: [PATCH net] hv_netvsc: fix race of netvsc and VF
> register_netdevice
>=20
> [You don't often get email from wojciech.drewek@intel.com. Learn why this=
 is
> important at https://aka.ms/LearnAboutSenderIdentification ]
>=20
> On 25.10.2023 23:16, Haiyang Zhang wrote:
> > The rtnl lock also needs to be held before rndis_filter_device_add()
> > which advertises nvsp_2_vsc_capability / sriov bit, and triggers
> > VF NIC offering and registering. If VF NIC finished register_netdev()
> > earlier it may cause name based config failure.
> >
> > To fix this issue, move the call to rtnl_lock() before
> > rndis_filter_device_add(), so VF will be registered later than netvsc
> > / synthetic NIC, and gets a name numbered (ethX) after netvsc.
> >
> > And, move register_netdevice_notifier() earlier, so the call back
> > function is set before probing.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: e04e7a7bbd4b ("hv_netvsc: Fix a deadlock by getting rtnl lock ea=
rlier
> in netvsc_probe()")
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > ---
> >  drivers/net/hyperv/netvsc_drv.c | 30 +++++++++++++++++++-----------
> >  1 file changed, 19 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/hyperv/netvsc_drv.c
> b/drivers/net/hyperv/netvsc_drv.c
> > index 3ba3c8fb28a5..feca1391f756 100644
> > --- a/drivers/net/hyperv/netvsc_drv.c
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> > @@ -2531,15 +2531,6 @@ static int netvsc_probe(struct hv_device *dev,
> >               goto devinfo_failed;
> >       }
> >
> > -     nvdev =3D rndis_filter_device_add(dev, device_info);
> > -     if (IS_ERR(nvdev)) {
> > -             ret =3D PTR_ERR(nvdev);
> > -             netdev_err(net, "unable to add netvsc device (ret %d)\n",=
 ret);
> > -             goto rndis_failed;
> > -     }
> > -
> > -     eth_hw_addr_set(net, device_info->mac_adr);
> > -
> >       /* We must get rtnl lock before scheduling nvdev->subchan_work,
> >        * otherwise netvsc_subchan_work() can get rtnl lock first and wa=
it
> >        * all subchannels to show up, but that may not happen because
> > @@ -2547,9 +2538,23 @@ static int netvsc_probe(struct hv_device *dev,
> >        * -> ... -> device_add() -> ... -> __device_attach() can't get
> >        * the device lock, so all the subchannels can't be processed --
> >        * finally netvsc_subchan_work() hangs forever.
> > +      *
> > +      * The rtnl lock also needs to be held before rndis_filter_device=
_add()
> > +      * which advertises nvsp_2_vsc_capability / sriov bit, and trigge=
rs
> > +      * VF NIC offering and registering. If VF NIC finished register_n=
etdev()
> > +      * earlier it may cause name based config failure.
> >        */
> >       rtnl_lock();
> >
> > +     nvdev =3D rndis_filter_device_add(dev, device_info);
> > +     if (IS_ERR(nvdev)) {
> > +             ret =3D PTR_ERR(nvdev);
> > +             netdev_err(net, "unable to add netvsc device (ret %d)\n",=
 ret);
> > +             goto rndis_failed;
>=20
> In case of error rtnl won't be unlocked.

Good catch! Will correct this.=20

Thanks,
- Haiyang