The MANA driver's probe registers netdevice via the following call chain:
mana_probe() register_netdev() register_netdevice()
register_netdevice() calls notifier callback for netvsc driver, holding the netdev mutex via netdev_lock_ops().
Further this netvsc notifier callback end up attempting to acquire the same lock again in dev_xdp_propagate() leading to deadlock.
netvsc_netdev_event() netvsc_vf_setxdp() dev_xdp_propagate()
This deadlock was not observed so far because net_shaper_ops was never set, and thus the lock was effectively a no-op in this case. Fix this by using netif_xdp_propagate() instead of dev_xdp_propagate() to avoid recursive locking in this path.
Also, clean up the unregistration path by removing the unnecessary call to netvsc_vf_setxdp(), since unregister_netdevice_many_notify() already performs this cleanup via dev_xdp_uninstall().
Fixes: 97246d6d21c2 ("net: hold netdev instance lock during ndo_bpf") Cc: stable@vger.kernel.org Signed-off-by: Saurabh Sengar ssengar@linux.microsoft.com Tested-by: Erni Sri Satya Vennela ernis@linux.microsoft.com Reviewed-by: Haiyang Zhang haiyangz@microsoft.com --- [V2] - Modified commit message
drivers/net/hyperv/netvsc_bpf.c | 2 +- drivers/net/hyperv/netvsc_drv.c | 2 -- net/core/dev.c | 1 + 3 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c index e01c5997a551..1dd3755d9e6d 100644 --- a/drivers/net/hyperv/netvsc_bpf.c +++ b/drivers/net/hyperv/netvsc_bpf.c @@ -183,7 +183,7 @@ int netvsc_vf_setxdp(struct net_device *vf_netdev, struct bpf_prog *prog) xdp.command = XDP_SETUP_PROG; xdp.prog = prog;
- ret = dev_xdp_propagate(vf_netdev, &xdp); + ret = netif_xdp_propagate(vf_netdev, &xdp);
if (ret && prog) bpf_prog_put(prog); diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index d8b169ac0343..ee3aaf9c10e6 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -2462,8 +2462,6 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
- netvsc_vf_setxdp(vf_netdev, NULL); - reinit_completion(&net_device_ctx->vf_add); netdev_rx_handler_unregister(vf_netdev); netdev_upper_dev_unlink(vf_netdev, ndev); diff --git a/net/core/dev.c b/net/core/dev.c index fccf2167b235..8c6c9d7fba26 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -9953,6 +9953,7 @@ int netif_xdp_propagate(struct net_device *dev, struct netdev_bpf *bpf)
return dev->netdev_ops->ndo_bpf(dev, bpf); } +EXPORT_SYMBOL_GPL(netif_xdp_propagate);
u32 dev_xdp_prog_id(struct net_device *dev, enum bpf_xdp_mode mode) {
On 2025-05-21 at 10:25:03, Saurabh Sengar (ssengar@linux.microsoft.com) wrote:
The MANA driver's probe registers netdevice via the following call chain:
mana_probe() register_netdev() register_netdevice()
register_netdevice() calls notifier callback for netvsc driver, holding the netdev mutex via netdev_lock_ops().
Further this netvsc notifier callback end up attempting to acquire the same lock again in dev_xdp_propagate() leading to deadlock.
netvsc_netdev_event() netvsc_vf_setxdp() dev_xdp_propagate()
This deadlock was not observed so far because net_shaper_ops was never set, and thus the lock was effectively a no-op in this case. Fix this by using netif_xdp_propagate() instead of dev_xdp_propagate() to avoid recursive locking in this path.
Also, clean up the unregistration path by removing the unnecessary call to netvsc_vf_setxdp(), since unregister_netdevice_many_notify() already performs this cleanup via dev_xdp_uninstall().
Fixes: 97246d6d21c2 ("net: hold netdev instance lock during ndo_bpf") Cc: stable@vger.kernel.org Signed-off-by: Saurabh Sengar ssengar@linux.microsoft.com Tested-by: Erni Sri Satya Vennela ernis@linux.microsoft.com Reviewed-by: Haiyang Zhang haiyangz@microsoft.com
Reviewed-by: Subbaraya Sundeep sbhatta@marvell.com
Thanks, Sundeep
[V2]
- Modified commit message
drivers/net/hyperv/netvsc_bpf.c | 2 +- drivers/net/hyperv/netvsc_drv.c | 2 -- net/core/dev.c | 1 + 3 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c index e01c5997a551..1dd3755d9e6d 100644 --- a/drivers/net/hyperv/netvsc_bpf.c +++ b/drivers/net/hyperv/netvsc_bpf.c @@ -183,7 +183,7 @@ int netvsc_vf_setxdp(struct net_device *vf_netdev, struct bpf_prog *prog) xdp.command = XDP_SETUP_PROG; xdp.prog = prog;
- ret = dev_xdp_propagate(vf_netdev, &xdp);
- ret = netif_xdp_propagate(vf_netdev, &xdp);
if (ret && prog) bpf_prog_put(prog); diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index d8b169ac0343..ee3aaf9c10e6 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -2462,8 +2462,6 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev) netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
- netvsc_vf_setxdp(vf_netdev, NULL);
- reinit_completion(&net_device_ctx->vf_add); netdev_rx_handler_unregister(vf_netdev); netdev_upper_dev_unlink(vf_netdev, ndev);
diff --git a/net/core/dev.c b/net/core/dev.c index fccf2167b235..8c6c9d7fba26 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -9953,6 +9953,7 @@ int netif_xdp_propagate(struct net_device *dev, struct netdev_bpf *bpf) return dev->netdev_ops->ndo_bpf(dev, bpf); } +EXPORT_SYMBOL_GPL(netif_xdp_propagate); u32 dev_xdp_prog_id(struct net_device *dev, enum bpf_xdp_mode mode) { -- 2.43.0
On Wed, 21 May 2025 03:25:03 -0700 Saurabh Sengar wrote:
The MANA driver's probe registers netdevice via the following call chain:
mana_probe() register_netdev() register_netdevice()
register_netdevice() calls notifier callback for netvsc driver, holding the netdev mutex via netdev_lock_ops().
Further this netvsc notifier callback end up attempting to acquire the same lock again in dev_xdp_propagate() leading to deadlock.
netvsc_netdev_event() netvsc_vf_setxdp() dev_xdp_propagate()
This deadlock was not observed so far because net_shaper_ops was never set,
The lock is on the VF, I think you meant to say that no device you use in Azure is ops locked?
There's also the call to netvsc_register_vf() on probe path, please fix or explain why it doesn't need locking in the commit message.
On Thu, May 22, 2025 at 03:13:46PM -0700, Jakub Kicinski wrote:
On Wed, 21 May 2025 03:25:03 -0700 Saurabh Sengar wrote:
The MANA driver's probe registers netdevice via the following call chain:
mana_probe() register_netdev() register_netdevice()
register_netdevice() calls notifier callback for netvsc driver, holding the netdev mutex via netdev_lock_ops().
Further this netvsc notifier callback end up attempting to acquire the same lock again in dev_xdp_propagate() leading to deadlock.
netvsc_netdev_event() netvsc_vf_setxdp() dev_xdp_propagate()
This deadlock was not observed so far because net_shaper_ops was never set,
The lock is on the VF, I think you meant to say that no device you use in Azure is ops locked?
There's also the call to netvsc_register_vf() on probe path, please fix or explain why it doesn't need locking in the commit message.
This patch specifically addresses the netvsc_register_vf() path only. I omitted the mention of netvsc_register_vf() in the commit message to keep the function path shorter. The full stack trace is provided below:
[ 92.542180] dev_xdp_propagate+0x2c/0x1b0 [ 92.542185] netvsc_vf_setxdp+0x10d/0x180 [hv_netvsc] [ 92.542192] netvsc_register_vf.part.0+0x179/0x200 [hv_netvsc] [ 92.542196] netvsc_netdev_event+0x267/0x340 [hv_netvsc] [ 92.542200] notifier_call_chain+0x5f/0xc0 [ 92.542203] raw_notifier_call_chain+0x16/0x20 [ 92.542205] call_netdevice_notifiers_info+0x52/0xa0 [ 92.542209] register_netdevice+0x7c8/0xaa0 [ 92.542211] register_netdev+0x1f/0x40 [ 92.542214] mana_probe+0x6e2/0x8e0 [mana] [ 92.542220] mana_gd_probe+0x187/0x220 [mana]
If you prefer I can update the stack trace in commit meesage From:
netvsc_netdev_event() netvsc_vf_setxdp() dev_xdp_propagate()
To:
netvsc_netdev_event() netvsc_register_vf() netvsc_vf_setxdp() dev_xdp_propagate()
- Saurabh
-- pw-bot: cr
On Thu, May 22, 2025 at 03:13:46PM -0700, Jakub Kicinski wrote:
On Wed, 21 May 2025 03:25:03 -0700 Saurabh Sengar wrote:
The MANA driver's probe registers netdevice via the following call chain:
mana_probe() register_netdev() register_netdevice()
register_netdevice() calls notifier callback for netvsc driver, holding the netdev mutex via netdev_lock_ops().
Further this netvsc notifier callback end up attempting to acquire the same lock again in dev_xdp_propagate() leading to deadlock.
netvsc_netdev_event() netvsc_vf_setxdp() dev_xdp_propagate()
This deadlock was not observed so far because net_shaper_ops was never set,
The lock is on the VF, I think you meant to say that no device you use in Azure is ops locked?
That's right.
There's also the call to netvsc_register_vf() on probe path, please fix or explain why it doesn't need locking in the commit message.
On rethinking I realize you were referring to the netvsc_probe() path not mana_probe(). Since this lock is effectively a no-op, it doesn't really matter whether it's there or not.
However, I think we can revisit this when we add ops for any of the VFs.
- Saurabh
linux-stable-mirror@lists.linaro.org
-
Jakub Kicinski -
Saurabh Sengar -
Saurabh Singh Sengar
-
Subbaraya Sundeep