[PATCH net-next v2 0/5] hv_netvsc: make XDP propagation act more like bonding

[PATCH net-next v2 0/5] hv_netvsc: make XDP propagation act more like bonding

[Jakub Kicinski]

Currently XDP programs propagated from uppers are invisible
to safety checks. I'm trying to fix that, but AI reviewer
threw up a bunch of complaints about hv_netvsc. See previous
series here:

https://lore.kernel.org/20260928223648.2739371-1-kuba@kernel.org

That series is now on hold, let's try to sanitize the hv_netvsc
behavior. AFAIU the main use case is to catch the VF as it appears,
and attach it to the netvsc SW interface. If we catch the VF
as soon as it appears we shouldn't have to worry about the VF
already having XDP attached. Let's do what bonding does and
refuse to attach if VF already has XDP, also refuse to attach
if nv_netvsc has XDP but the VF refuses the propagation.

All patches are from LLM reviews and LLM generated.

v1: https://lore.kernel.org/20261001014131.310771-1-kuba@kernel.org

Jakub Kicinski (5):
  hv_netvsc: fix the program refcount when the VF refuses XDP
  hv_netvsc: hold the VF's instance lock when installing XDP on it
  hv_netvsc: treat the VF's XDP program the way bonding treats a slave's
  hv_netvsc: move a new VF to netvsc's netns from a work item
  hv_netvsc: let the core take XDP off a netvsc device that is going
    away

 .../ethernet/microsoft/netvsc.rst             |  11 +-
 drivers/net/hyperv/hyperv_net.h               |   3 +
 drivers/net/hyperv/netvsc_bpf.c               |  70 ++++++++++-
 drivers/net/hyperv/netvsc_drv.c               | 116 ++++++++++++++----
 4 files changed, 169 insertions(+), 31 deletions(-)

-- 
2.56.0

[PATCH net-next v2 1/5] hv_netvsc: fix the program refcount when the VF refuses XDP

[Jakub Kicinski]

ndo_bpf() takes over the reference it is passed only on success, the
caller puts it back itself on failure. netvsc_xdp_set() takes that one
plus num_chn - 1 more for its channels, and the rollback after the VF
refuses the program clears the channels again, putting all num_chn of
them. dev_xdp_install() then puts the one it passed in a second time,
and the program can be freed while the fd which loaded it still points
at it.

The VF refuses a program netvsc has already committed to when the
program is single-buffer and the VF has header-data split enabled, when
the VF has a memory provider bound, or when the VF's driver has
conditions of its own.

The rollback clears the channels rather than putting back the program
they ran before, too. A refused replace or removal then leaves the
synthetic path running no program at all, while the core and the VF
keep the old one. Put the old program back instead.

Reported by Sashiko during core rework. Unverified and untested.

Cc: stable+noautosel@kernel.org # untested fix to unlikely driver error path
Fixes: 184367dce4f7 ("hv_netvsc: Fix XDP refcnt for synthetic and VF NICs")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
 - put the old program back on the channels rather than clearing them
   (Kameron)
 - reword the comment on the extra reference, it's the caller's, which
   isn't always the core
 - Tested-by tags from v1 not carried over, the rollback changed
v1: https://lore.kernel.org/20261001014131.310771-2-kuba@kernel.org
---
 drivers/net/hyperv/netvsc_bpf.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
index 1dd3755d9e6d..fcc60729384e 100644
--- a/drivers/net/hyperv/netvsc_bpf.c
+++ b/drivers/net/hyperv/netvsc_bpf.c
@@ -197,6 +197,7 @@ int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 	struct netvsc_device *nvdev = rtnl_dereference(ndevctx->nvdev);
 	struct net_device *vf_netdev = rtnl_dereference(ndevctx->vf_netdev);
 	struct netlink_ext_ack *extack = bpf->extack;
+	struct bpf_prog *old_prog;
 	int ret;
 
 	if (!nvdev || nvdev->destroy) {
@@ -205,10 +206,17 @@ int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 
 	switch (bpf->command) {
 	case XDP_SETUP_PROG:
+		/* Hold on to the old program, to put it back on the channels
+		 * if the VF refuses the change.
+		 */
+		old_prog = netvsc_xdp_get(nvdev);
+		if (old_prog)
+			bpf_prog_inc(old_prog);
+
 		ret = netvsc_xdp_set(dev, bpf->prog, extack, nvdev);
 
 		if (ret)
-			return ret;
+			goto put_old_prog;
 
 		ret = netvsc_vf_setxdp(vf_netdev, bpf->prog);
 
@@ -216,9 +224,22 @@ int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 			netdev_err(dev, "vf_setxdp failed:%d\n", ret);
 			NL_SET_ERR_MSG_MOD(extack, "vf_setxdp failed");
 
-			netvsc_xdp_set(dev, NULL, extack, nvdev);
+			/* The caller hands us its reference to bpf->prog only
+			 * on success, take one so that putting the num_chn
+			 * channel references below doesn't consume it.
+			 */
+			if (bpf->prog)
+				bpf_prog_inc(bpf->prog);
+			/* Can't fail, old_prog has passed the same checks
+			 * before. The channels take over our reference to it.
+			 */
+			netvsc_xdp_set(dev, old_prog, extack, nvdev);
+			return ret;
 		}
 
+put_old_prog:
+		if (old_prog)
+			bpf_prog_put(old_prog);
 		return ret;
 
 	default:
-- 
2.56.0

[PATCH net-next v2 2/5] hv_netvsc: hold the VF's instance lock when installing XDP on it

[Jakub Kicinski]

netvsc_vf_setxdp() has two kinds of callers. netvsc_register_vf() runs
with the VF's instance lock already held, either by the NETDEV_REGISTER
notifier or by netvsc_probe() taking it, which is why the propagation
moved to the caller-locked netif_xdp_propagate(). netvsc_bpf() runs with
just RTNL, so the VF's ndo_bpf() gets called unlocked. For a VF with an
ops lock that trips the lockdep assertion in
dev_get_min_mp_channel_count(), which netif_xdp_propagate() calls, and
races with binding a memory provider, which takes only the instance
lock.

Take the VF's lock in netvsc_bpf().

Reported by Sashiko during core rework, the race was not reproduced.

Cc: stable+noautosel@kernel.org # LLM report + LLM fix, untested
Fixes: 3ec523304976 ("hv_netvsc: fix potential deadlock in netvsc_vf_setxdp()")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Tested-by: Kameron Carr <kameroncarr@linux.microsoft.com>
Tested-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
---
v2:
 - commit msg: say that the race was not reproduced, rather than that
   the fix is untested, as functional testing covered it
v1: https://lore.kernel.org/20261001014131.310771-3-kuba@kernel.org
---
 drivers/net/hyperv/netvsc_bpf.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
index fcc60729384e..ec8edfaebf0b 100644
--- a/drivers/net/hyperv/netvsc_bpf.c
+++ b/drivers/net/hyperv/netvsc_bpf.c
@@ -14,6 +14,7 @@
 #include <linux/bpf.h>
 #include <linux/bpf_trace.h>
 #include <linux/kernel.h>
+#include <net/netdev_lock.h>
 #include <net/xdp.h>
 
 #include <linux/mutex.h>
@@ -162,6 +163,7 @@ int netvsc_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 	return 0;
 }
 
+/* Caller holds the VF's lock, see netdev_lock_ops() */
 int netvsc_vf_setxdp(struct net_device *vf_netdev, struct bpf_prog *prog)
 {
 	struct netdev_bpf xdp;
@@ -172,6 +174,8 @@ int netvsc_vf_setxdp(struct net_device *vf_netdev, struct bpf_prog *prog)
 	if (!vf_netdev)
 		return 0;
 
+	netdev_assert_locked_ops_compat(vf_netdev);
+
 	if (!vf_netdev->netdev_ops->ndo_bpf)
 		return 0;
 
@@ -218,7 +222,15 @@ int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 		if (ret)
 			goto put_old_prog;
 
-		ret = netvsc_vf_setxdp(vf_netdev, bpf->prog);
+		/* Unlike netvsc_register_vf() we don't get the VF's lock
+		 * handed to us here.
+		 */
+		ret = 0;
+		if (vf_netdev) {
+			netdev_lock_ops(vf_netdev);
+			ret = netvsc_vf_setxdp(vf_netdev, bpf->prog);
+			netdev_unlock_ops(vf_netdev);
+		}
 
 		if (ret) {
 			netdev_err(dev, "vf_setxdp failed:%d\n", ret);
-- 
2.56.0

[PATCH net-next v2 3/5] hv_netvsc: treat the VF's XDP program the way bonding treats a slave's

[Jakub Kicinski]

netvsc_register_vf() pushes netvsc's XDP state down to every VF it
takes over - its program, or NULL if it has none - once the VF has
joined, and ignores the result. A VF which refuses the program leaves
netvsc running XDP on its channels while traffic through the VF
bypasses it, with nothing in the logs to say so. The core won't install
a single-buffer program on a device with tcp-data-split enabled, nor
any program on one with a memory provider, and the VF's driver has
conditions of its own. A VF which can't run XDP at all isn't even
asked, and netvsc_bpf() reports success for it too. A VF which runs a
program of its own gets it replaced or removed behind the back of
whoever attached it, when it joins as well as when netvsc gets a
program later.

Nothing takes netvsc's program back when the VF stops being netvsc's
either. netvsc_unregister_vf() runs when the VF is unregistered, when
either device moves to another netns (netvsc drags the VF along when it
moves itself), when netvsc is unbound or hot-removed, and on rmmod
hv_netvsc, where unregistering the notifier replays NETDEV_UNREGISTER
before netvsc_remove() ever runs. In all but the first case the VF
stays, running netvsc's program with no upper left to own it. We will
soon track in the core if the program is installed from upper; a
leftover would then become the VF's own, and netvsc would refuse to
take the VF back until someone removed it. netvsc_unregister_vf() used
to propagate NULL to the VF, until commit 3ec523304976 ("hv_netvsc: fix
potential deadlock in netvsc_vf_setxdp()") dropped the call on the
grounds that the core cleans up through dev_xdp_uninstall(). That only
runs when the VF itself is unregistered, and doesn't know about
propagated programs at all, so even then the VF's driver keeps its
reference to netvsc's program; mana, for one, leaks it on every VF
removal while netvsc runs XDP.

Do what bonding does with its slaves instead. With no program on netvsc
there is nothing to push down, so leave the VF's program alone. With a
program on netvsc, don't take over a VF which runs one of its own, in
any mode, or which can't or won't run netvsc's, and say why in the log.
netvsc then keeps its traffic on the synthetic path, where the program
runs, and doesn't use the VF. Nothing retries taking the VF over until
it registers again, just as bonding doesn't retry an enslave it refused.
For a VF already in use fail attaching the program to netvsc instead, as
netvsc_bpf() already did when the VF refused it, but don't fail a
removal over the VF's own program, leave that program be. Install the
program before joining the VF, so that a refusal needs no unwinding and
nothing reaches netvsc through the VF without having been through the
program, and sync the VF's features ahead of it, as LRO may keep the VF
from taking XDP; a VF which refuses the program all the same is left
with netvsc's features. Take the program back when the VF leaves, under
the VF's lock, which none of the paths into netvsc_unregister_vf() hold,
once netvsc's rx handler is gone, and warn if the VF won't let go of it,
as bonding does, but leave a program of the VF's own be, as on removal.
Key that on the program on netvsc's channels, the way bonding keys its
release on bond->xdp_prog; a generic program on netvsc never reached the
VF. Let the VF go in netvsc_remove() before the channels are cleared.

The two halves only work together. The NULL pushed at registration was
all that cleared a program an earlier netvsc instance left on the VF,
and taking the program back without the refusal would strip a VF of its
own program after it was joined despite refusing netvsc's.

A VF netvsc uses now always runs netvsc's program when netvsc has one.
Update netvsc's documentation, which still said that a program on the
VF may get overwritten, and describe the VFs netvsc doesn't use.

Reported by Sashiko during core rework. Unverified and untested.

Cc: stable+noautosel@kernel.org # LLM report + LLM fix, untested
Fixes: 351e1581395f ("hv_netvsc: Add XDP support")
Fixes: 3ec523304976 ("hv_netvsc: fix potential deadlock in netvsc_vf_setxdp()")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
 - refuse a VF which can't run XDP while netvsc has a program, and fail
   attaching a program to netvsc while such a VF is in use (Sashiko)
 - fail attaching a program to netvsc while the VF in use runs one of
   its own, rather than leave that to the core series, but leave the
   VF's program be on removal, and when the VF leaves (Sashiko)
 - warn when the VF won't give netvsc's program back (Sashiko)
 - move the documentation update here from the core series, and don't
   claim that the refused VF's traffic stays on the synthetic path
   (Vennela)
 - commit msg: say that nothing retries a refused VF, and that it keeps
   netvsc's features (Sashiko)
 - Tested-by tags from v1 not carried over, the behavior changed
v1: https://lore.kernel.org/20261001014131.310771-4-kuba@kernel.org
---
 .../ethernet/microsoft/netvsc.rst             | 11 +++-
 drivers/net/hyperv/netvsc_bpf.c               | 15 ++++-
 drivers/net/hyperv/netvsc_drv.c               | 65 +++++++++++++++----
 3 files changed, 76 insertions(+), 15 deletions(-)

diff --git a/Documentation/networking/device_drivers/ethernet/microsoft/netvsc.rst b/Documentation/networking/device_drivers/ethernet/microsoft/netvsc.rst
index 0d464706a59a..7f1731a819f9 100644
--- a/Documentation/networking/device_drivers/ethernet/microsoft/netvsc.rst
+++ b/Documentation/networking/device_drivers/ethernet/microsoft/netvsc.rst
@@ -109,8 +109,15 @@ XDP support
 
   Setting / unsetting XDP program on synthetic NIC (netvsc) propagates to
   VF NIC automatically. Setting / unsetting XDP program on VF NIC directly
-  is not recommended, also not propagated to synthetic NIC, and may be
-  overwritten by setting of synthetic NIC.
+  is not recommended, and not propagated to synthetic NIC. If the
+  synthetic NIC has a program when a VF NIC shows up, a VF NIC which runs
+  a program of its own, or which can't or won't run the synthetic NIC's,
+  is not used until it is registered again. The synthetic NIC then keeps
+  its traffic on the synthetic data path; what the VF NIC receives if it
+  is brought up on its own is outside of the synthetic NIC's control.
+  For a VF NIC already in use which runs a program of its own, or can't
+  or won't run the synthetic NIC's, setting the program on the synthetic
+  NIC fails instead.
 
   XDP program cannot run with LRO (RSC) enabled, so you need to disable LRO
   before running XDP::
diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
index ec8edfaebf0b..143fc0c12841 100644
--- a/drivers/net/hyperv/netvsc_bpf.c
+++ b/drivers/net/hyperv/netvsc_bpf.c
@@ -176,8 +176,9 @@ int netvsc_vf_setxdp(struct net_device *vf_netdev, struct bpf_prog *prog)
 
 	netdev_assert_locked_ops_compat(vf_netdev);
 
+	/* as with bonding, a VF which can't run XDP can't carry ours */
 	if (!vf_netdev->netdev_ops->ndo_bpf)
-		return 0;
+		return prog ? -EOPNOTSUPP : 0;
 
 	memset(&xdp, 0, sizeof(xdp));
 
@@ -210,6 +211,18 @@ int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 
 	switch (bpf->command) {
 	case XDP_SETUP_PROG:
+		/* The VF's own program is not ours: refuse to install over
+		 * it, as bonding does, and leave it be on removal.
+		 */
+		if (vf_netdev && dev_xdp_prog_count(vf_netdev)) {
+			if (bpf->prog) {
+				NL_SET_ERR_MSG_MOD(extack,
+						   "VF has an XDP program of its own");
+				return -EOPNOTSUPP;
+			}
+			vf_netdev = NULL;
+		}
+
 		/* Hold on to the old program, to put it back on the channels
 		 * if the VF refuses the change.
 		 */
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 1d43c73fd73f..61109ff9927f 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2331,6 +2331,13 @@ static int netvsc_register_vf(struct net_device *vf_netdev, int context)
 	if (!netvsc_dev || rtnl_dereference(net_device_ctx->vf_netdev))
 		return NOTIFY_DONE;
 
+	prog = netvsc_xdp_get(netvsc_dev);
+	if (prog && dev_xdp_prog_count(vf_netdev)) {
+		netdev_warn(ndev, "not using VF %s, it has an XDP program attached\n",
+			    vf_netdev->name);
+		return NOTIFY_DONE;
+	}
+
 	/* if synthetic interface is a different namespace,
 	 * then move the VF to that namespace; join will be
 	 * done again in that context.
@@ -2349,10 +2356,31 @@ static int netvsc_register_vf(struct net_device *vf_netdev, int context)
 		return NOTIFY_DONE;
 	}
 
+	/* Install netvsc's program before joining, so that a VF which
+	 * refuses it never gets used. Syncing the features first turns
+	 * off LRO, which the VF may not run XDP with.
+	 */
+	if (prog) {
+		vf_netdev->wanted_features = ndev->features;
+		netdev_update_features(vf_netdev);
+
+		ret = netvsc_vf_setxdp(vf_netdev, prog);
+		if (ret) {
+			netdev_warn(ndev, "not using VF %s, it refused netvsc's XDP program: %d\n",
+				    vf_netdev->name, ret);
+			return NOTIFY_DONE;
+		}
+	}
+
 	netdev_info(ndev, "VF registering: %s\n", vf_netdev->name);
 
-	if (netvsc_vf_join(vf_netdev, ndev, context) != 0)
+	if (netvsc_vf_join(vf_netdev, ndev, context) != 0) {
+		ret = prog ? netvsc_vf_setxdp(vf_netdev, NULL) : 0;
+		if (ret)
+			netdev_warn(ndev, "failed to remove XDP program from VF %s: %d\n",
+				    vf_netdev->name, ret);
 		return NOTIFY_DONE;
+	}
 
 	dev_hold(vf_netdev);
 	rcu_assign_pointer(net_device_ctx->vf_netdev, vf_netdev);
@@ -2363,9 +2391,6 @@ static int netvsc_register_vf(struct net_device *vf_netdev, int context)
 	vf_netdev->wanted_features = ndev->features;
 	netdev_update_features(vf_netdev);
 
-	prog = netvsc_xdp_get(netvsc_dev);
-	netvsc_vf_setxdp(vf_netdev, prog);
-
 	return NOTIFY_OK;
 }
 
@@ -2441,6 +2466,8 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
 {
 	struct net_device *ndev;
 	struct net_device_context *net_device_ctx;
+	struct netvsc_device *nvdev;
+	int ret;
 
 	ndev = get_netvsc_byref(vf_netdev);
 	if (!ndev)
@@ -2453,6 +2480,20 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
 
 	reinit_completion(&net_device_ctx->vf_add);
 	netdev_rx_handler_unregister(vf_netdev);
+
+	/* Take our program back now that frames from the VF no longer
+	 * reach netvsc, but leave a program of the VF's own be.
+	 */
+	nvdev = rtnl_dereference(net_device_ctx->nvdev);
+	if (nvdev && netvsc_xdp_get(nvdev) && !dev_xdp_prog_count(vf_netdev)) {
+		netdev_lock_ops(vf_netdev);
+		ret = netvsc_vf_setxdp(vf_netdev, NULL);
+		netdev_unlock_ops(vf_netdev);
+		if (ret)
+			netdev_warn(ndev, "failed to remove XDP program from VF %s: %d\n",
+				    vf_netdev->name, ret);
+	}
+
 	netdev_upper_dev_unlink(vf_netdev, ndev);
 	RCU_INIT_POINTER(net_device_ctx->vf_netdev, NULL);
 	dev_put(vf_netdev);
@@ -2664,21 +2705,21 @@ static void netvsc_remove(struct hv_device *dev)
 	cancel_delayed_work_sync(&ndev_ctx->vfns_work);
 
 	nvdev = rtnl_dereference(ndev_ctx->nvdev);
-	if (nvdev) {
+	if (nvdev)
 		cancel_work_sync(&nvdev->subchan_work);
-		netvsc_xdp_set(net, NULL, NULL, nvdev);
-	}
+
+	vf_netdev = rtnl_dereference(ndev_ctx->vf_netdev);
+	if (vf_netdev)
+		netvsc_unregister_vf(vf_netdev);
 
 	/*
 	 * Call to the vsc driver to let it know that the device is being
 	 * removed. Also blocks mtu and channel changes.
 	 */
-	vf_netdev = rtnl_dereference(ndev_ctx->vf_netdev);
-	if (vf_netdev)
-		netvsc_unregister_vf(vf_netdev);
-
-	if (nvdev)
+	if (nvdev) {
+		netvsc_xdp_set(net, NULL, NULL, nvdev);
 		rndis_filter_device_remove(dev, nvdev);
+	}
 
 	unregister_netdevice(net);
 	list_del(&ndev_ctx->list);
-- 
2.56.0

[PATCH net-next v2 4/5] hv_netvsc: move a new VF to netvsc's netns from a work item

[Jakub Kicinski]

When the VF registers in a different netns than netvsc, for example
because it was hot-added into init_net while netvsc lives in a container
netns, netvsc_register_vf() moves it over with dev_change_net_namespace()
straight from the VF's NETDEV_REGISTER notifier. Since commit
4c975fd70002 ("net: hold instance lock during NETDEV_REGISTER/UP") that
notifier runs under the VF's instance lock, which the netns change takes
again. For a VF with an ops lock the registration then deadlocks with
RTNL held, taking the networking of the whole VM down with it. mana
always has an ops lock, as it selects NET_SHAPER, and so does mlx5 for
its queue management ops, so every VF hot-add or servicing event hangs a
VM running netvsc in a netns, as does moving the VF out by hand.

netvsc already leaves moving its VF to vfns_work when netvsc itself
changes netns. Hand the new VF to the same work, holding a reference
until it runs. The VF still joins netvsc when it registers again in
netvsc's netns. If netvsc gets to the VF's netns before the work runs,
for example because netvsc's netns went away, there is no move to
register the VF again, so have the work take the VF over itself. Close
the VF first, as the move would have, as one which is up already may
never send the event which switches the data path over. Suspend cancels
the work, keep the VF and queue the work again on resume, so that the
VF isn't lost. Userspace now sees the VF in the netns it registered in,
with its uevent sent there, until the work moves it.

Spotted by AI while reviewing the XDP propagation series. Untested.

Cc: stable+noautosel@kernel.org # LLM report + LLM fix, untested
Fixes: 4c975fd70002 ("net: hold instance lock during NETDEV_REGISTER/UP")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
 - close the VF the work takes over, one which was up already would
   never switch the data path over (Sashiko)
 - don't drop the VF on suspend, queue the work again on resume
   (Sashiko)
 - commit msg: mention that userspace sees the VF in its original netns
   first
 - Tested-by tags from v1 not carried over, the code changed
v1: https://lore.kernel.org/20261001014131.310771-5-kuba@kernel.org
---
 drivers/net/hyperv/hyperv_net.h |  3 ++
 drivers/net/hyperv/netvsc_drv.c | 51 +++++++++++++++++++++++++--------
 2 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 4841367fdab2..a7681404df2d 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -1064,6 +1064,9 @@ struct net_device_context {
 	struct netvsc_vf_pcpu_stats __percpu *vf_stats;
 	struct delayed_work vf_takeover;
 	struct delayed_work vfns_work;
+	/* VF for vfns_work to move here or take over, see netvsc_register_vf() */
+	struct net_device *vfns_dev;
+	netdevice_tracker vfns_dev_tracker;
 
 	/* 1: allocated, serial number is valid. 0: not allocated */
 	u32 vf_alloc;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 61109ff9927f..d534e574bcf2 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2311,6 +2311,12 @@ static int netvsc_prepare_bonding(struct net_device *vf_netdev)
 	return NOTIFY_DONE;
 }
 
+static void netvsc_vfns_dev_put(struct net_device_context *ndev_ctx)
+{
+	netdev_put(ndev_ctx->vfns_dev, &ndev_ctx->vfns_dev_tracker);
+	ndev_ctx->vfns_dev = NULL;
+}
+
 static int netvsc_register_vf(struct net_device *vf_netdev, int context)
 {
 	struct net_device_context *net_device_ctx;
@@ -2340,19 +2346,17 @@ static int netvsc_register_vf(struct net_device *vf_netdev, int context)
 
 	/* if synthetic interface is a different namespace,
 	 * then move the VF to that namespace; join will be
-	 * done again in that context.
+	 * done again in that context. The VF's lock is held
+	 * for its registration, so leave the move to vfns_work.
 	 */
 	if (!net_eq(dev_net(ndev), dev_net(vf_netdev))) {
-		ret = dev_change_net_namespace(vf_netdev,
-					       dev_net(ndev), "eth%d");
-		if (ret)
-			netdev_err(vf_netdev,
-				   "could not move to same namespace as %s: %d\n",
-				   ndev->name, ret);
-		else
-			netdev_info(vf_netdev,
-				    "VF moved to namespace with: %s\n",
-				    ndev->name);
+		if (net_device_ctx->vfns_dev != vf_netdev) {
+			netvsc_vfns_dev_put(net_device_ctx);
+			netdev_hold(vf_netdev, &net_device_ctx->vfns_dev_tracker,
+				    GFP_KERNEL);
+			net_device_ctx->vfns_dev = vf_netdev;
+		}
+		schedule_delayed_work(&net_device_ctx->vfns_work, 0);
 		return NOTIFY_DONE;
 	}
 
@@ -2703,6 +2707,7 @@ static void netvsc_remove(struct hv_device *dev)
 
 	rtnl_lock();
 	cancel_delayed_work_sync(&ndev_ctx->vfns_work);
+	netvsc_vfns_dev_put(ndev_ctx);
 
 	nvdev = rtnl_dereference(ndev_ctx->nvdev);
 	if (nvdev)
@@ -2745,6 +2750,7 @@ static int netvsc_suspend(struct hv_device *dev)
 	cancel_delayed_work_sync(&ndev_ctx->dwork);
 
 	rtnl_lock();
+	/* keep vfns_dev, netvsc_resume() queues the work again */
 	cancel_delayed_work_sync(&ndev_ctx->vfns_work);
 
 	nvdev = rtnl_dereference(ndev_ctx->nvdev);
@@ -2761,6 +2767,9 @@ static int netvsc_suspend(struct hv_device *dev)
 	}
 	ret = netvsc_detach(net, nvdev);
 out:
+	/* netvsc_resume() won't run if we fail */
+	if (ret)
+		schedule_delayed_work(&ndev_ctx->vfns_work, 0);
 	rtnl_unlock();
 
 	return ret;
@@ -2789,6 +2798,9 @@ static int netvsc_resume(struct hv_device *dev)
 	netvsc_devinfo_put(device_info);
 	net_device_ctx->saved_netvsc_dev_info = NULL;
 
+	/* pick up whatever netvsc_suspend() stopped vfns_work from doing */
+	schedule_delayed_work(&net_device_ctx->vfns_work, 0);
+
 	rtnl_unlock();
 
 	return ret;
@@ -2823,7 +2835,9 @@ static void netvsc_event_set_vf_ns(struct net_device *ndev)
 
 	vf_netdev = rtnl_dereference(ndev_ctx->vf_netdev);
 	if (!vf_netdev)
-		return;
+		vf_netdev = ndev_ctx->vfns_dev;
+	if (!vf_netdev || vf_netdev->reg_state != NETREG_REGISTERED)
+		goto out;
 
 	if (!net_eq(dev_net(ndev), dev_net(vf_netdev))) {
 		ret = dev_change_net_namespace(vf_netdev, dev_net(ndev),
@@ -2836,7 +2850,20 @@ static void netvsc_event_set_vf_ns(struct net_device *ndev)
 			netdev_info(vf_netdev,
 				    "Moved VF to namespace with: %s\n",
 				    ndev->name);
+	} else if (!rtnl_dereference(ndev_ctx->vf_netdev)) {
+		/* netvsc got to the VF's netns first, so no move will
+		 * register the VF again, take it over from here. Close
+		 * it first, as the move would have: a VF which is up
+		 * already may never send the event which switches the
+		 * data path over.
+		 */
+		netdev_lock_ops(vf_netdev);
+		netif_close(vf_netdev);
+		netvsc_register_vf(vf_netdev, VF_REG_IN_NOTIFIER);
+		netdev_unlock_ops(vf_netdev);
 	}
+out:
+	netvsc_vfns_dev_put(ndev_ctx);
 }
 
 void netvsc_vfns_work(struct work_struct *w)
-- 
2.56.0

[PATCH net-next v2 5/5] hv_netvsc: let the core take XDP off a netvsc device that is going away

[Jakub Kicinski]

netvsc_remove() lets the VF go, taking the XDP program back from it,
takes the program off the channels, then frees the channels and clears
nvdev, and only then unregisters the netdev. dev_xdp_uninstall() still
finds the program in netvsc's xdp_state[] at that point and asks
netvsc_bpf() to remove it, which fails with -ENODEV for the lack of
nvdev. That trips the WARN_ON() in dev_xdp_uninstall() on every unbind
or hot-remove of a netvsc device running native XDP, and
dev_xdp_install() returns before it drops the program from the XDP
dispatcher, which then holds on to it for good. The WARN came with
commit 7f0a838254bd ("bpf, xdp: Maintain info on attached XDP BPF
programs in net_device"): the core used to ask netvsc for its program
first, and netvsc reported none once nvdev was gone, which left the
program with the dispatcher all the same.

Accept the removal once there are no channels and no VF left to take the
program off. If suspend parked the program for netvsc_resume() to put
back, drop it from there, or resume would bring back a program the core
no longer knows about. This also lets the program be removed from a
netvsc device without a VF which lost its channels to a failed
reconfiguration, rather than fail. The core is about to record the
programs uppers propagate as well, which would bring the same to a
netvsc device enslaved to a bond running XDP.

Spotted by AI while reviewing the XDP propagation series. Untested.

Cc: stable+noautosel@kernel.org # LLM report + LLM fix, untested
Fixes: 7f0a838254bd ("bpf, xdp: Maintain info on attached XDP BPF programs in net_device")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
 - drop the program suspend parked for resume rather than refuse the
   removal, which still tripped the WARN with the config a failed
   suspend leaves behind (Sashiko)
 - commit msg: the dispatcher kept the program before 7f0a838254bd too,
   name the function which skips the dispatcher, and mention devices
   which lost their channels to a failed reconfiguration
 - Tested-by tags from v1 not carried over, the code changed
v1: https://lore.kernel.org/20261001014131.310771-6-kuba@kernel.org
---
 drivers/net/hyperv/netvsc_bpf.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
index 143fc0c12841..14bb520b44ed 100644
--- a/drivers/net/hyperv/netvsc_bpf.c
+++ b/drivers/net/hyperv/netvsc_bpf.c
@@ -206,7 +206,21 @@ int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 	int ret;
 
 	if (!nvdev || nvdev->destroy) {
-		return -ENODEV;
+		struct netvsc_device_info *saved;
+
+		if (bpf->command != XDP_SETUP_PROG || bpf->prog || vf_netdev)
+			return -ENODEV;
+
+		/* The channels and the VF are gone, and so is the program,
+		 * except for the reference netvsc_suspend() keeps for
+		 * netvsc_resume().
+		 */
+		saved = ndevctx->saved_netvsc_dev_info;
+		if (saved && saved->bprog) {
+			bpf_prog_put(saved->bprog);
+			saved->bprog = NULL;
+		}
+		return 0;
 	}
 
 	switch (bpf->command) {
-- 
2.56.0

Re: [PATCH net-next v2 2/5] hv_netvsc: hold the VF's instance lock when installing XDP on it

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The stable Cc trailer contradicts the rest of the commit
  "hv_netvsc: hold the VF's instance lock when installing XDP on it".

--

hv_netvsc: hold the VF's instance lock when installing XDP on it

netvsc_bpf() runs with only RTNL held, so the VF's ndo_bpf() is called
without the VF's instance lock. That trips the lockdep assertion in
dev_get_min_mp_channel_count() and can race with memory provider binding.
This patch takes the VF's ops lock around netvsc_vf_setxdp() in
netvsc_bpf(), and adds an assertion in netvsc_vf_setxdp().

> Reported by Sashiko during core rework, the race was not reproduced.
>
> Cc: stable+noautosel@kernel.org # LLM report + LLM fix, untested
> Fixes: 3ec523304976 ("hv_netvsc: fix potential deadlock in netvsc_vf_setxdp()")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> Tested-by: Kameron Carr <kameroncarr@linux.microsoft.com>
> Tested-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>

[Severity: Low]
This isn't a bug, but the stable trailer still says "untested". The same
commit carries two Tested-by: tags.

The v2 changelog says:

> v2:
>  - commit msg: say that the race was not reproduced, rather than that
>    the fix is untested, as functional testing covered it

The body was updated to say "the race was not reproduced". The comment on
the Cc: stable+noautosel trailer, which stable maintainers read when they
decide on the backport, was not changed.

Should the trailer be updated to match, for example to something like
"LLM report + LLM fix, race not reproduced"?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109194843.1353592-1-kuba%40kernel.org

Re: [PATCH net-next v2 3/5] hv_netvsc: treat the VF's XDP program the way bonding treats a slave's

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] netvsc_unregister_vf()
  (drivers/net/hyperv/netvsc_drv.c:2487-2494) takes netvsc's program…
- [Low] The commit message says that installing netvsc's program before
  joining means 'nothing reaches netvsc through the VF without having…

--

hv_netvsc: treat the VF's XDP program the way bonding treats a slave's

This patch makes netvsc refuse to take over a VF that has its own XDP
program, or that can't or won't run netvsc's program. It installs
netvsc's program on the VF before joining it. It also takes netvsc's
program back from the VF in netvsc_unregister_vf().

> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 1d43c73fd73f1..61109ff9927fb 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c

[ ... ]

> @@ -2349,10 +2356,31 @@ static int netvsc_register_vf(struct net_device *vf_netdev, int context)
>  		return NOTIFY_DONE;
>  	}
>  
> +	/* Install netvsc's program before joining, so that a VF which
> +	 * refuses it never gets used. Syncing the features first turns
> +	 * off LRO, which the VF may not run XDP with.
> +	 */
> +	if (prog) {
> +		vf_netdev->wanted_features = ndev->features;
> +		netdev_update_features(vf_netdev);
> +
> +		ret = netvsc_vf_setxdp(vf_netdev, prog);
> +		if (ret) {
> +			netdev_warn(ndev, "not using VF %s, it refused netvsc's XDP program: %d\n",
> +				    vf_netdev->name, ret);
> +			return NOTIFY_DONE;
> +		}
> +	}
> +
>  	netdev_info(ndev, "VF registering: %s\n", vf_netdev->name);
>  
> -	if (netvsc_vf_join(vf_netdev, ndev, context) != 0)
> +	if (netvsc_vf_join(vf_netdev, ndev, context) != 0) {

[Severity: Low]

The commit message says that installing the program before joining means
"nothing reaches netvsc through the VF without having been through the
program". Does this ordering fully guarantee that?

netvsc_vf_setxdp() is followed directly by netvsc_vf_join(), which calls
netdev_rx_handler_register(). That function publishes the handler without
any synchronization:

net/core/dev.c:netdev_rx_handler_register() {
    ...
	rcu_assign_pointer(dev->rx_handler_data, rx_handler_data);
	rcu_assign_pointer(dev->rx_handler, rx_handler);
    ...
}

Some VF drivers also publish the new program without waiting for NAPI.
mana is one example:

drivers/net/ethernet/microsoft/mana/mana_bpf.c:mana_chn_setxdp() {
    ...
	rcu_assign_pointer(apc->rxqs[i]->bpf_prog, prog);
    ...
}

Suppose a VF NAPI poll on another CPU has already read a NULL
rxq->bpf_prog. It could build an skb, hold it in GRO, and deliver it after
rx_handler becomes visible. netvsc_vf_handle_frame() would then pass that
frame to netvsc without it having run the XDP program.

The teardown side does wait, because netdev_rx_handler_unregister() calls
synchronize_net(). The setup side mostly matters for the netvsc_probe()
path (VF_REG_IN_PROBE), where the VF may already be up and polling. The
window is about one NAPI poll, and the ordering is still better than it
was before the patch.

Would a synchronize_net() between netvsc_vf_setxdp() and netvsc_vf_join()
make the claim hold? If not, should the commit message be softened?

[ ... ]

> @@ -2453,6 +2480,20 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
>  
>  	reinit_completion(&net_device_ctx->vf_add);
>  	netdev_rx_handler_unregister(vf_netdev);
> +
> +	/* Take our program back now that frames from the VF no longer
> +	 * reach netvsc, but leave a program of the VF's own be.
> +	 */
> +	nvdev = rtnl_dereference(net_device_ctx->nvdev);
> +	if (nvdev && netvsc_xdp_get(nvdev) && !dev_xdp_prog_count(vf_netdev)) {

[Severity: Medium]

This take-back depends on netvsc's channel program, not on whether a
program was actually pushed to the VF. Can the VF be left running
netvsc's program when the two get out of sync?

There seem to be two reachable states where the condition is false while
the VF stays joined and still runs the program from netvsc_vf_setxdp().

In the first state, nvdev exists but its channels are empty.
netvsc_detach() clears the channel program before steps that can still
fail:

drivers/net/hyperv/netvsc_drv.c:netvsc_detach() {
    ...
	netvsc_xdp_set(ndev, NULL, NULL, nvdev);

	/* If device was up (receiving) then shutdown */
	if (netif_running(ndev)) {
		netvsc_tx_disable(nvdev, ndev);

		ret = rndis_filter_close(nvdev);
		if (ret) {
			...
			return ret;
		}

		ret = netvsc_wait_until_empty(nvdev);
		if (ret) {
			...
			return ret;
		}
	}
    ...
}

If this fails, netvsc_change_mtu() goes to rollback_vf and
netvsc_set_channels() goes to out, and neither one re-attaches. nvdev is
still there, but netvsc_xdp_get(nvdev) returns NULL and the VF was never
touched.

In the second state, nvdev is NULL. If netvsc_attach() fails, either on
an rndis_filter_device_add() error or through the err1/err2 paths,
ndev_ctx->nvdev stays NULL. That happens when both attach attempts fail
in change_mtu or set_channels. It also happens when the single attempt
fails in netvsc_resume(), which drops the saved bprog as well.

In either state, any later path into netvsc_unregister_vf() skips the
take-back: a VF hot-remove, a netns move, netvsc_remove(), unbind, or
rmmod. The VF is then unlinked still running netvsc's program, and its
driver keeps the reference. Isn't that the same orphaned program and
mana leak that the commit message says this patch fixes?

When nvdev is NULL, netvsc_bpf() also returns -ENODEV for a removal while
the VF is joined. So the program can't be cleared through netvsc either.

The patch also removes this push after join in netvsc_register_vf():

	prog = netvsc_xdp_get(netvsc_dev);
	netvsc_vf_setxdp(vf_netdev, prog);

That unconditional push used to clear such a leftover when the VF
registered again with a netvsc that has no program. Now the stale program
survives re-registration. dev_xdp_prog_count() can't see it, because it
was propagated rather than installed on the VF through dev_xdp_install().

The commit message compares this to bonding keying its release on
bond->xdp_prog. The two behave differently: bond->xdp_prog lasts as long
as the bond has a program attached. netvsc's channel pointer is torn down
on every detach/attach and on a failed reconfiguration.

The later patches in the series ("move a new VF to netvsc's netns from a
work item" and "let the core take XDP off a netvsc device that is going
away") leave this condition unchanged.

Could the release be keyed on what was actually pushed? One way is a
pointer or flag in net_device_ctx that is set when
netvsc_vf_setxdp(vf, prog) succeeds. Another is to have the detach and
attach failure paths also pull the program off the VF.

> +		netdev_lock_ops(vf_netdev);
> +		ret = netvsc_vf_setxdp(vf_netdev, NULL);
> +		netdev_unlock_ops(vf_netdev);
> +		if (ret)
> +			netdev_warn(ndev, "failed to remove XDP program from VF %s: %d\n",
> +				    vf_netdev->name, ret);
> +	}
> +
>  	netdev_upper_dev_unlink(vf_netdev, ndev);
>  	RCU_INIT_POINTER(net_device_ctx->vf_netdev, NULL);
>  	dev_put(vf_netdev);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109194843.1353592-1-kuba%40kernel.org

Re: [PATCH net-next v2 4/5] hv_netvsc: move a new VF to netvsc's netns from a work item

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 (1) · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] netvsc_resume() now always requeues vfns_work, and it does so
  from the vmbus noirq resume phase (vmbus_pm:…
- [Low] netvsc_register_vf() takes a tracked reference on a VF registered
  in another netns (netdev_hold(vf_netdev, &vfns_dev_tracker) and stores…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but netvsc_probe()
  can free the net_device while its embedded vfns_work is still queued.

--

hv_netvsc: move a new VF to netvsc's netns from a work item

The patch stops calling dev_change_net_namespace() directly from the VF's
NETDEV_REGISTER notifier, which deadlocks for ops-locked VFs. Instead it
takes a reference on the VF in vfns_dev and has vfns_work move it, or take
it over if netvsc reached the VF's netns first. Suspend now keeps vfns_dev,
and resume queues the work again.

> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 61109ff9927fb..d534e574bcf22 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c

[ ... ]

> @@ -2340,19 +2346,17 @@ static int netvsc_register_vf(struct net_device *vf_netdev, int context)
>  
>  	/* if synthetic interface is a different namespace,
>  	 * then move the VF to that namespace; join will be
> -	 * done again in that context.
> +	 * done again in that context. The VF's lock is held
> +	 * for its registration, so leave the move to vfns_work.
>  	 */
>  	if (!net_eq(dev_net(ndev), dev_net(vf_netdev))) {
> -		ret = dev_change_net_namespace(vf_netdev,
> -					       dev_net(ndev), "eth%d");
> -		if (ret)
> -			netdev_err(vf_netdev,
> -				   "could not move to same namespace as %s: %d\n",
> -				   ndev->name, ret);
> -		else
> -			netdev_info(vf_netdev,
> -				    "VF moved to namespace with: %s\n",
> -				    ndev->name);
> +		if (net_device_ctx->vfns_dev != vf_netdev) {
> +			netvsc_vfns_dev_put(net_device_ctx);
> +			netdev_hold(vf_netdev, &net_device_ctx->vfns_dev_tracker,
> +				    GFP_KERNEL);
> +			net_device_ctx->vfns_dev = vf_netdev;
> +		}

[Severity: Low]
What drops this reference if the VF is unregistered while it is parked
in vfns_dev?

The NETDEV_UNREGISTER handler finds netvsc only through the master upper
device:

drivers/net/hyperv/netvsc_drv.c:netvsc_unregister_vf() {
    ...
	ndev = get_netvsc_byref(vf_netdev);
	if (!ndev)
		return NOTIFY_DONE;
    ...
}

A VF that is held only in vfns_dev has not joined netvsc, so this lookup
returns NULL. Only vfns_work, through netvsc_event_set_vf_ns() and
netvsc_vfns_dev_put(), or netvsc_remove() drops the reference.

Usually the work runs soon and drops it when it sees reg_state !=
NETREG_REGISTERED. But netvsc_suspend() cancels vfns_work and keeps
vfns_dev until netvsc_resume() queues the work again.

If the VF is unregistered in that window, will unregister_netdevice()
wait in netdev_wait_allrefs_any() until thaw_noirq/restore_noirq, with
"waiting for ethX to become free" messages? netvsc ignores the repeated
NETDEV_UNREGISTER that the wait loop sends.

The window looks narrow, since hv_pci and the VF driver have already
suspended by then. Should the unregister path still check vfns_dev and
drop the reference there?

> +		schedule_delayed_work(&net_device_ctx->vfns_work, 0);
>  		return NOTIFY_DONE;
>  	}

[ ... ]

> @@ -2703,6 +2707,7 @@ static void netvsc_remove(struct hv_device *dev)
>  
>  	rtnl_lock();
>  	cancel_delayed_work_sync(&ndev_ctx->vfns_work);
> +	netvsc_vfns_dev_put(ndev_ctx);
>  
>  	nvdev = rtnl_dereference(ndev_ctx->nvdev);
>  	if (nvdev)

[Severity: High]
This isn't a bug introduced by this patch, but netvsc_remove() cancels
vfns_work and the netvsc_probe() error path does not. Can netvsc_probe()
free the net_device while vfns_work is still queued?

register_netdevice(net) sends NETDEV_REGISTER for the netvsc device, and
netvsc_netdev_event() queues the work:

	if (event_dev->netdev_ops == &device_ops && event == NETDEV_REGISTER) {
		ndev_ctx = netdev_priv(event_dev);
		schedule_delayed_work(&ndev_ctx->vfns_work, 0);
		return NOTIFY_DONE;
	}

If a later NETDEV_REGISTER notifier rejects the device, register_netdevice()
calls unregister_netdevice_queue() and returns an error. For example,
mpls_dev_notify() returns an error when mpls_add_dev() fails with -ENOMEM.

netvsc_probe() then goes through register_failed -> rndis_failed -> ... ->
free_netdev(net), and it never calls
cancel_delayed_work_sync(&net_device_ctx->vfns_work).

Probe holds RTNL at this point, so rtnl_trylock() fails in
netvsc_vfns_work() and the work requeues itself with a 1 jiffy delay.

If the work is still pending when free_netdev() frees the netdev_priv
memory, wouldn't the timer and workqueue linkage, and the container_of()
in netvsc_vfns_work(), touch freed memory?

[ ... ]

> @@ -2789,6 +2798,9 @@ static int netvsc_resume(struct hv_device *dev)
>  	netvsc_devinfo_put(device_info);
>  	net_device_ctx->saved_netvsc_dev_info = NULL;
>  
> +	/* pick up whatever netvsc_suspend() stopped vfns_work from doing */
> +	schedule_delayed_work(&net_device_ctx->vfns_work, 0);
> +

[Severity: Medium]
Can this run the VF move or takeover before the VF's own driver has
resumed?

netvsc_resume() is called from the vmbus noirq phase:

drivers/hv/vmbus_drv.c:vmbus_pm {
	.freeze_noirq	= vmbus_suspend,
	.thaw_noirq	= vmbus_resume,
	.poweroff_noirq	= vmbus_suspend,
	.restore_noirq	= vmbus_resume,
}

vfns_work runs on system_percpu_wq, which isn't freezable, and it only
needs rtnl_trylock(). It can therefore run before the VF's PCI driver gets
its normal-phase .thaw/.restore callback. This happens when vfns_dev, or a
netvsc netns move, is still pending when netvsc_suspend() cancels the
work.

netvsc_event_set_vf_ns() then calls one of two paths:

- dev_change_net_namespace(), which closes the VF via ndo_stop
- netif_close() followed by netvsc_register_vf()

The second path calls ndo_bpf, ndo_set_features and netvsc_vf_join(), and
netvsc_vf_join() schedules vf_takeover and dev_open().

Take mana as an example. mana_gd_suspend() -> mana_remove(..., true) ->
mana_detach() saves port_st_save and clears port_is_up. The early
netif_close() then hits:

drivers/net/ethernet/microsoft/mana/mana_en.c:mana_close() {
    ...
	if (!apc->port_is_up)
		return 0;
    ...
}

The core then clears IFF_UP. On resume, mana_attach() brings the port
back up from port_st_save. The hardware port is then up while the netdev
is marked down.

A later dev_open() from vf_takeover would then either fail with -ENODEV
while the device is still detached, or call mana_open() ->
mana_alloc_queues() a second time on a port that is already up.

Before this patch, netvsc_resume() didn't touch the VF. Should this
requeue wait until the VF driver has resumed?

>  	rtnl_unlock();
>  
>  	return ret;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109194843.1353592-1-kuba%40kernel.org

Re: [PATCH net-next v2 5/5] hv_netvsc: let the core take XDP off a netvsc device that is going away

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In drivers/net/hyperv/netvsc_bpf.c, netvsc_bpf() now reports
  success for an XDP removal when nvdev is gone and vf_netdev is NULL.

--

hv_netvsc: let the core take XDP off a netvsc device that is going away

When nvdev is gone, netvsc_bpf() now lets an XDP removal succeed if
there is also no VF. If netvsc_suspend() parked a program reference for
netvsc_resume(), that reference is dropped. This avoids the WARN_ON() in
dev_xdp_uninstall() and stops the dispatcher from keeping the program on
unbind or hot-remove.

> diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
> index 143fc0c128411..14bb520b44ed5 100644
> --- a/drivers/net/hyperv/netvsc_bpf.c
> +++ b/drivers/net/hyperv/netvsc_bpf.c
> @@ -206,7 +206,21 @@ int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
>  	int ret;
>  
>  	if (!nvdev || nvdev->destroy) {
> -		return -ENODEV;
> +		struct netvsc_device_info *saved;
> +
> +		if (bpf->command != XDP_SETUP_PROG || bpf->prog || vf_netdev)
> +			return -ENODEV;
> +
> +		/* The channels and the VF are gone, and so is the program,
> +		 * except for the reference netvsc_suspend() keeps for
> +		 * netvsc_resume().
> +		 */

[Severity: Medium]
Is "and so is the program" true on the unbind path in the failed-suspend
and failed-reconfiguration cases that v2 targets?

netvsc_remove() unlinks the VF before it unregisters the netdev:

drivers/net/hyperv/netvsc_drv.c:netvsc_remove() {
    ...
	vf_netdev = rtnl_dereference(ndev_ctx->vf_netdev);
	if (vf_netdev)
		netvsc_unregister_vf(vf_netdev);
    ...
	unregister_netdevice(net);
    ...
}

So when dev_xdp_uninstall() calls netvsc_bpf(), vf_netdev is always NULL
and the new vf_netdev check never fires.

netvsc_unregister_vf() only takes the program back from the VF while it
is still on netvsc's channels:

drivers/net/hyperv/netvsc_drv.c:netvsc_unregister_vf() {
    ...
	nvdev = rtnl_dereference(net_device_ctx->nvdev);
	if (nvdev && netvsc_xdp_get(nvdev) && !dev_xdp_prog_count(vf_netdev)) {
		netdev_lock_ops(vf_netdev);
		ret = netvsc_vf_setxdp(vf_netdev, NULL);
    ...
	netdev_upper_dev_unlink(vf_netdev, ndev);
	RCU_INIT_POINTER(net_device_ctx->vf_netdev, NULL);
    ...
}

That condition is false in at least two cases.

First, netvsc_detach() clears the channels and can then fail. This can
happen when it is called from netvsc_suspend(), netvsc_set_channels(),
netvsc_change_mtu() or netvsc_set_ringparam():

drivers/net/hyperv/netvsc_drv.c:netvsc_detach() {
    ...
	netvsc_xdp_set(ndev, NULL, NULL, nvdev);

	/* If device was up (receiving) then shutdown */
	if (netif_running(ndev)) {
		netvsc_tx_disable(nvdev, ndev);

		ret = rndis_filter_close(nvdev);
		if (ret) {
			...
			return ret;
		}

		ret = netvsc_wait_until_empty(nvdev);
		if (ret) {
			...
			return ret;
		}
    ...
}

After that, nvdev is still there and the channels are empty. The VF
still runs the program that netvsc_vf_setxdp() installed.

Second, both netvsc_attach() calls in a reconfiguration can fail. Then
nvdev is NULL and the VF stays joined with the program still on it.

In either case, netvsc_unregister_vf() skips
netvsc_vf_setxdp(vf_netdev, NULL) but still clears vf_netdev. This new
branch then returns 0, and the core drops its reference, the dispatcher
entry and the xdp_state record.

Doesn't the former VF then keep running netvsc's program under the
reference netvsc_vf_setxdp() took?

netif_xdp_propagate() doesn't record propagated programs, so
dev_xdp_prog_count() on the VF is 0. That means neither device reports
the program, and "ip link set dev <vf> xdp off" never reaches the VF
driver.

The earlier patch in this series, "hv_netvsc: treat the VF's XDP program
the way bonding treats a slave's", also removed the NULL push at VF
registration. So a later netvsc instance with no program won't clear the
leftover either.

That looks like the same result that patch set out to prevent: a VF
running netvsc's program with no upper that owns it.

Would it work to put the program back on the channels (and free
saved_netvsc_dev_info) when netvsc_detach() or netvsc_suspend() fails?
Another option would be for netvsc_unregister_vf() to check netvsc's
recorded native program instead of the channel state.

> +		saved = ndevctx->saved_netvsc_dev_info;
> +		if (saved && saved->bprog) {
> +			bpf_prog_put(saved->bprog);
> +			saved->bprog = NULL;
> +		}
> +		return 0;
>  	}
>  
>  	switch (bpf->command) {

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109194843.1353592-1-kuba%40kernel.org

Re: [PATCH net-next v2 3/5] hv_netvsc: treat the VF's XDP program the way bonding treats a slave's

[Jakub Kicinski]

On Wed, 11 Nov 2026 10:48:50 +0000 netdev-bot+sashiko@kernel.org wrote:
> This take-back depends on netvsc's channel program, not on whether a
> program was actually pushed to the VF. Can the VF be left running
> netvsc's program when the two get out of sync?

Yes, the netvsc changes unblock core work which will let us distinguish
which progs were propagated and which attached directly. IOW fixing this
is sort of the point of the work..

Re: [PATCH net-next v2 4/5] hv_netvsc: move a new VF to netvsc's netns from a work item

[Jakub Kicinski]

On Wed, 11 Nov 2026 10:48:52 +0000 netdev-bot+sashiko@kernel.org wrote:
> > +	/* pick up whatever netvsc_suspend() stopped vfns_work from doing */
> > +	schedule_delayed_work(&net_device_ctx->vfns_work, 0);
> > +  
> 
> [Severity: Medium]
> Can this run the VF move or takeover before the VF's own driver has
> resumed?

The resume handling is a disaster, I can drop this patch completely,
if preferred, it's not really required for the XDP rework.

Re: [PATCH net-next v2 5/5] hv_netvsc: let the core take XDP off a netvsc device that is going away

[Jakub Kicinski]

On Wed, 11 Nov 2026 10:48:53 +0000 netdev-bot+sashiko@kernel.org wrote:
> > +		/* The channels and the VF are gone, and so is the program,
> > +		 * except for the reference netvsc_suspend() keeps for
> > +		 * netvsc_resume().
> > +		 */  
> 
> [Severity: Medium]
> Is "and so is the program" true on the unbind path in the failed-suspend
> and failed-reconfiguration cases that v2 targets?

The goal of this patch is to stop returning the false positive
errors when core clears XDP prog during netvsc shutdown.
This would trigger a WARN() in the core once core tracks the propagated
programs. The 100s of netvsc bugs are not my concern.