Message ID | 1699627140-28003-2-git-send-email-haiyangz@microsoft.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | hv_netvsc: fix race of netvsc, VF register, and slave bit | expand |
On Fri, Nov 10, 2023 at 06:38:58AM -0800, 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. > > Cc: stable@vger.kernel.org > Fixes: e04e7a7bbd4b ("hv_netvsc: Fix a deadlock by getting rtnl lock earlier in netvsc_probe()") > Reported-by: Dexuan Cui <decui@microsoft.com> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> > Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com> Reviewed-by: Simon Horman <horms@kernel.org>
> From: LKML haiyangz <lkmlhyz@microsoft.com> On Behalf Of Haiyang Zhang > Sent: Friday, November 10, 2023 9:39 AM > [...] > > 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. > > Cc: stable@vger.kernel.org > Fixes: e04e7a7bbd4b ("hv_netvsc: Fix a deadlock by getting rtnl lock earlier in > netvsc_probe()") > Reported-by: Dexuan Cui <decui@microsoft.com> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> > Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com> Reviewed-by: Dexuan Cui <decui@microsoft.com>
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 3ba3c8fb28a5..5e528a76f5f5 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);