drivers/net/hyperv/netvsc_drv.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-)
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 earlier in netvsc_probe()")
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
v2:
Fix rtnl_unlock() in error handling as found by Wojciech Drewek.
---
drivers/net/hyperv/netvsc_drv.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 3ba3c8fb28a5..1d1491da303b 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 = rndis_filter_device_add(dev, device_info);
- if (IS_ERR(nvdev)) {
- ret = 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 wait
* 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 triggers
+ * VF NIC offering and registering. If VF NIC finished register_netdev()
+ * earlier it may cause name based config failure.
*/
rtnl_lock();
+ nvdev = rndis_filter_device_add(dev, device_info);
+ if (IS_ERR(nvdev)) {
+ ret = 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);
+
if (nvdev->num_chn > 1)
schedule_work(&nvdev->subchan_work);
@@ -2586,9 +2591,9 @@ static int netvsc_probe(struct hv_device *dev,
return 0;
register_failed:
- rtnl_unlock();
rndis_filter_device_remove(dev, nvdev);
rndis_failed:
+ rtnl_unlock();
netvsc_devinfo_put(device_info);
devinfo_failed:
free_percpu(net_device_ctx->vf_stats);
@@ -2788,11 +2793,14 @@ static int __init netvsc_drv_init(void)
}
netvsc_ring_bytes = ring_size * PAGE_SIZE;
+ register_netdevice_notifier(&netvsc_netdev_notifier);
+
ret = vmbus_driver_register(&netvsc_drv);
- if (ret)
+ if (ret) {
+ unregister_netdevice_notifier(&netvsc_netdev_notifier);
return ret;
+ }
- register_netdevice_notifier(&netvsc_netdev_notifier);
return 0;
}
--
2.25.1On Thu, 26 Oct 2023 14:22:34 -0700 Haiyang Zhang wrote: > And, move register_netdevice_notifier() earlier, so the call back > function is set before probing. Are you sure you need this? I thought the netdev notifier "replays" registration events (i.e. sends "fake" events for already present netdevs). If I'm wrong this should still be a separate patch from the rtnl reorder. -- pw-bot: cr
> -----Original Message----- > From: Jakub Kicinski <kuba@kernel.org> > Sent: Thursday, November 2, 2023 1:08 AM > To: Haiyang Zhang <haiyangz@microsoft.com> > Cc: linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; KY Srinivasan > <kys@microsoft.com>; wei.liu@kernel.org; Dexuan Cui > <decui@microsoft.com>; edumazet@google.com; pabeni@redhat.com; > davem@davemloft.net; linux-kernel@vger.kernel.org; stable@vger.kernel.org > Subject: Re: [PATCH net,v2] hv_netvsc: fix race of netvsc and VF > register_netdevice > > On Thu, 26 Oct 2023 14:22:34 -0700 Haiyang Zhang wrote: > > And, move register_netdevice_notifier() earlier, so the call back > > function is set before probing. > > Are you sure you need this? I thought the netdev notifier "replays" > registration events (i.e. sends "fake" events for already present > netdevs). > > If I'm wrong this should still be a separate patch from the rtnl > reorder. I tested, NETDEV_REGISTER is indeed replayed, but NETDEV_POST_INIT is not. And we will use NETDEV_POST_INIT soon. Also, we want to get notified by NETDEV_POST_INIT immediately from VF, before VF NIC shows up to upper layers. So, even if we make NETDEV_POST_INIT to be replayed, that may be too late. I will put the register_netdevice_notifier() change to a separate patch. Thanks, - Haiyang
On 26.10.2023 23:22, 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 earlier in netvsc_probe()")
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
>
> ---
> v2:
> Fix rtnl_unlock() in error handling as found by Wojciech Drewek.
> ---
> drivers/net/hyperv/netvsc_drv.c | 32 ++++++++++++++++++++------------
> 1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 3ba3c8fb28a5..1d1491da303b 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 = rndis_filter_device_add(dev, device_info);
> - if (IS_ERR(nvdev)) {
> - ret = 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 wait
> * 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 triggers
> + * VF NIC offering and registering. If VF NIC finished register_netdev()
> + * earlier it may cause name based config failure.
> */
> rtnl_lock();
>
> + nvdev = rndis_filter_device_add(dev, device_info);
> + if (IS_ERR(nvdev)) {
> + ret = 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);
> +
> if (nvdev->num_chn > 1)
> schedule_work(&nvdev->subchan_work);
>
> @@ -2586,9 +2591,9 @@ static int netvsc_probe(struct hv_device *dev,
> return 0;
>
> register_failed:
> - rtnl_unlock();
> rndis_filter_device_remove(dev, nvdev);
> rndis_failed:
> + rtnl_unlock();
> netvsc_devinfo_put(device_info);
> devinfo_failed:
> free_percpu(net_device_ctx->vf_stats);
> @@ -2788,11 +2793,14 @@ static int __init netvsc_drv_init(void)
> }
> netvsc_ring_bytes = ring_size * PAGE_SIZE;
>
> + register_netdevice_notifier(&netvsc_netdev_notifier);
> +
> ret = vmbus_driver_register(&netvsc_drv);
> - if (ret)
> + if (ret) {
> + unregister_netdevice_notifier(&netvsc_netdev_notifier);
> return ret;
> + }
>
> - register_netdevice_notifier(&netvsc_netdev_notifier);
> return 0;
> }
>
© 2016 - 2025 Red Hat, Inc.