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

[PATCH net-next 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. I do not
have access to Hyper-V. I did 6 cycles of reviews and back
and forth with the LLM over these, so I think they should
be okay-ish. Let's be clear tho, that I do not care one bit
about this driver and it's brokenness is blocking core work.

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

 drivers/net/hyperv/hyperv_net.h |  3 +
 drivers/net/hyperv/netvsc_bpf.c | 27 ++++++++-
 drivers/net/hyperv/netvsc_drv.c | 98 +++++++++++++++++++++++++--------
 3 files changed, 103 insertions(+), 25 deletions(-)

-- 
2.55.0

[PATCH net-next 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.

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>
---
 drivers/net/hyperv/netvsc_bpf.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
index 1dd3755d9e6d..731bb7721fe2 100644
--- a/drivers/net/hyperv/netvsc_bpf.c
+++ b/drivers/net/hyperv/netvsc_bpf.c
@@ -216,6 +216,13 @@ 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");
 
+			/* Since we haven't completed the installation
+			 * of bpf->prog the reference core implicitly
+			 * transfers to us on success isn't ours.
+			 * Take a reference to balance the accounting.
+			 */
+			if (bpf->prog)
+				bpf_prog_inc(bpf->prog);
 			netvsc_xdp_set(dev, NULL, extack, nvdev);
 		}
 
-- 
2.55.0

[PATCH net-next 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. Unverified and untested.

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>
---
 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 731bb7721fe2..951c19ce15eb 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;
 
@@ -210,7 +214,15 @@ int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 		if (ret)
 			return ret;
 
-		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.55.0

[PATCH net-next 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 runs a program of its own gets it
replaced or removed behind the back of whoever attached it, and the
core is about to refuse propagating over a device's own program anyway.

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 refuses netvsc's; traffic then stays on the synthetic
path, where the program runs, and the log says why. 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 keeps LRO off.
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. 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.

netvsc_bpf() already fails attaching a program the VF refuses, so a VF
netvsc uses now always runs netvsc's program when netvsc has one.

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>
---
 drivers/net/hyperv/netvsc_drv.c | 57 ++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 12 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 1d43c73fd73f..17c86e749357 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,29 @@ 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) {
+		if (prog)
+			netvsc_vf_setxdp(vf_netdev, NULL);
 		return NOTIFY_DONE;
+	}
 
 	dev_hold(vf_netdev);
 	rcu_assign_pointer(net_device_ctx->vf_netdev, vf_netdev);
@@ -2363,9 +2389,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 +2464,7 @@ 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;
 
 	ndev = get_netvsc_byref(vf_netdev);
 	if (!ndev)
@@ -2453,6 +2477,15 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
 
 	reinit_completion(&net_device_ctx->vf_add);
 	netdev_rx_handler_unregister(vf_netdev);
+
+	/* Only once frames from the VF no longer reach netvsc */
+	nvdev = rtnl_dereference(net_device_ctx->nvdev);
+	if (nvdev && netvsc_xdp_get(nvdev)) {
+		netdev_lock_ops(vf_netdev);
+		netvsc_vf_setxdp(vf_netdev, NULL);
+		netdev_unlock_ops(vf_netdev);
+	}
+
 	netdev_upper_dev_unlink(vf_netdev, ndev);
 	RCU_INIT_POINTER(net_device_ctx->vf_netdev, NULL);
 	dev_put(vf_netdev);
@@ -2664,21 +2697,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.55.0

[PATCH net-next 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.

Spotted by AI while reviewing the XDP propagation series.
Not verified, and 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>
---
 drivers/net/hyperv/hyperv_net.h |  3 +++
 drivers/net/hyperv/netvsc_drv.c | 41 +++++++++++++++++++++++----------
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 4841367fdab2..5c771fe8d766 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 to our netns, 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 17c86e749357..10c05809c065 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;
 	}
 
@@ -2695,6 +2699,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)
@@ -2738,6 +2743,7 @@ static int netvsc_suspend(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 == NULL) {
@@ -2815,7 +2821,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),
@@ -2828,7 +2836,16 @@ 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
+		 */
+		netdev_lock_ops(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.55.0

[PATCH net-next 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 returns before
the program is dropped from the XDP dispatcher, which then holds on to
it for good. The core used to ask netvsc for its program first, and
netvsc reported none once nvdev was gone, until commit 7f0a838254bd
("bpf, xdp: Maintain info on attached XDP BPF programs in net_device").

Accept the removal once there are no channels and no VF left to take the
program off, unless suspend parked the program for resume to put back,
as the core would then lose track of it. 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>
---
 drivers/net/hyperv/netvsc_bpf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
index 951c19ce15eb..7fe38574f66f 100644
--- a/drivers/net/hyperv/netvsc_bpf.c
+++ b/drivers/net/hyperv/netvsc_bpf.c
@@ -204,6 +204,12 @@ int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 	int ret;
 
 	if (!nvdev || nvdev->destroy) {
+		/* The channels and the VF are gone, and so is the program,
+		 * unless suspend parked it for netvsc_resume() to put back.
+		 */
+		if (bpf->command == XDP_SETUP_PROG && !bpf->prog && !vf_netdev &&
+		    !ndevctx->saved_netvsc_dev_info)
+			return 0;
 		return -ENODEV;
 	}
 
-- 
2.55.0

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

[Haiyang Zhang]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, September 30, 2026 9:41 PM
> To: davem@davemloft.net
> Cc: netdev@vger.kernel.org; edumazet@google.com; pabeni@redhat.com;
> andrew+netdev@lunn.ch; horms@kernel.org; Haiyang Zhang
> <haiyangz@microsoft.com>; wei.liu@kernel.org; Dexuan Cui
> <DECUI@microsoft.com>; longli@microsoft.com; linux-hyperv@vger.kernel.org;
> hawk@kernel.org; andriin@fb.com; Jakub Kicinski <kuba@kernel.org>
> Subject: [EXTERNAL] [PATCH net-next 0/5] hv_netvsc: make XDP propagation
> act more like bonding
>
> 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.ker/
> nel.org%2F20260928223648.2739371-1-
> kuba%40kernel.org&data=05%7C02%7Chaiyangz%40microsoft.com%7C004597675fc941
> cc7a8808df1f5d2050%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6392641570
> 12414027%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwM
> CIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=Sd7
> 6Tk%2Bqcp1GeOUDM2YWyo3l4WzWW7U2vRHsXjx9SGk%3D&reserved=0
>
> 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. I do not
> have access to Hyper-V. I did 6 cycles of reviews and back
> and forth with the LLM over these, so I think they should
> be okay-ish. Let's be clear tho, that I do not care one bit
> about this driver and it's brokenness is blocking core work.
>
> 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
>
>  drivers/net/hyperv/hyperv_net.h |  3 +
>  drivers/net/hyperv/netvsc_bpf.c | 27 ++++++++-
>  drivers/net/hyperv/netvsc_drv.c | 98 +++++++++++++++++++++++++--------
>  3 files changed, 103 insertions(+), 25 deletions(-)

Thank you for making this patch set, looks good to me.
I will find a teammate to test it.

- Haiyang

Re: [PATCH net-next 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 6 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 4 · 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).

- [High] netvsc_vf_setxdp() returns 0 without installing anything when
  the VF has no ndo_bpf (drivers/net/hyperv/netvsc_bpf.c:180-181, `if…
- [Medium] netvsc_unregister_vf() only takes the program back from the VF
  when `nvdev && netvsc_xdp_get(nvdev)`, that is, when netvsc's channels…
- [Medium] Two calls to netvsc_vf_setxdp(vf_netdev, NULL) drop their
  result without logging: the new take-back in netvsc_unregister_vf()…
- [Medium] netvsc_register_vf() now refuses a VF with its own program
  (dev_xdp_prog_count(vf_netdev) != 0), but only at join time and only…
- [Medium] Once netvsc refuses a VF at registration, nothing retries the
  takeover.
- [Low] Before installing the program, netvsc_register_vf() now sets
  `vf_netdev->wanted_features = ndev->features;…

Pre-existing issues:
- [High] At this commit, netvsc_register_vf() still calls
  dev_change_net_namespace(vf_netdev, dev_net(ndev), "eth%d") directly…

--

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

When netvsc has an XDP program, netvsc_register_vf() now refuses a VF
that has its own program or that refuses netvsc's. It also installs
netvsc's program before joining the VF. netvsc_unregister_vf() now takes
netvsc's program back from the VF when the VF leaves, and
netvsc_remove() releases the VF before clearing the channel program.

> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 1d43c73fd73f1..17c86e7493577 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;
> +	}

[Severity: Medium]
A VF with its own program is only refused here, at join time, and only
when netvsc already has a program. Does netvsc_bpf() need the same
check? It still propagates onto a joined VF without looking at the VF's
own program:

drivers/net/hyperv/netvsc_bpf.c:netvsc_bpf() {
    ...
		if (vf_netdev) {
			netdev_lock_ops(vf_netdev);
			ret = netvsc_vf_setxdp(vf_netdev, bpf->prog);
			netdev_unlock_ops(vf_netdev);
		}
    ...
}

A joined VF can get its own program while netvsc has none.
dev_xdp_attach() on the VF only refuses when an upper has a program,
and nothing checks the lowers when a program is attached to netvsc.
Here is one sequence:

  netvsc has no program, VF joins
  admin attaches P to the VF (allowed, the upper has no program)
  admin attaches Q to netvsc
    netvsc_bpf()->netvsc_vf_setxdp()->netif_xdp_propagate()
      VF driver now runs Q, VF xdp_state still records P
  VF leaves
    netvsc_unregister_vf()->netvsc_vf_setxdp(vf_netdev, NULL)
      VF driver runs nothing, VF xdp_state still reports P

After that, the next netvsc_register_vf() refuses the VF because of a
program that is not running.

Bonding repeats the check for every slave in bond_xdp_set():

		if (dev_xdp_prog_count(slave_dev) > 0) {
			SLAVE_NL_ERR(dev, slave_dev, extack,
				     "Slave has XDP program loaded, please unload before enslaving");

The commit message names this as one of the problems being fixed: "A VF
which runs a program of its own gets it replaced or removed behind the
back of whoever attached it". The core change it mentions ("the core is
about to refuse propagating over a device's own program") is not part
of this series.

> +
>  	/* if synthetic interface is a different namespace,
>  	 * then move the VF to that namespace; join will be
>  	 * done again in that context.

[Severity: High]
This is a pre-existing issue and was not introduced by this patch. The
netns branch here still calls dev_change_net_namespace() directly from
the VF's NETDEV_REGISTER notifier:

	if (!net_eq(dev_net(ndev), dev_net(vf_netdev))) {
		ret = dev_change_net_namespace(vf_netdev,
					       dev_net(ndev), "eth%d");

Since commit 4c975fd70002, register_netdevice() holds the VF's instance
lock across NETDEV_REGISTER. __dev_change_net_namespace() then calls
netdev_lock_ops() on the same VF again.

Can this self-deadlock with RTNL held for ops-locked VFs such as mana or
mlx5? That would happen when netvsc is in a non-init netns and the VF
is hot-added into init_net.

The later patch in this series, "hv_netvsc: move a new VF to netvsc's
netns from a work item", replaces the direct call with a hand-off to
vfns_work, so this is resolved by the end of the series.

[ ... ]

> @@ -2349,10 +2356,29 @@ 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);

[Severity: Low]
This replaces the VF's entire wanted feature set, not only LRO, before
netvsc knows whether it will use the VF. If the VF then refuses the
program, or netvsc_vf_join() fails below, netvsc returns NOTIFY_DONE
and the original wanted_features are not restored.

Is it intended that offloads the user or driver enabled on the VF, and
that netvsc doesn't carry, stay off on a device netvsc doesn't own?
The commit message only mentions LRO: "a VF which refuses the program
all the same keeps LRO off". Before this patch, the full sync only ran
after a successful join.

Would clearing only NETIF_F_LRO here avoid the side effect?

> +
> +		ret = netvsc_vf_setxdp(vf_netdev, prog);

[Severity: High]
Can netvsc_vf_setxdp() return 0 here without installing anything? It
returns early when the VF has no ndo_bpf, before netif_xdp_propagate()
gets the chance to return -EOPNOTSUPP:

drivers/net/hyperv/netvsc_bpf.c:netvsc_vf_setxdp() {
    ...
	if (!vf_netdev->netdev_ops->ndo_bpf)
		return 0;
    ...
}

Suppose netvsc has a native program and the matched VF has no ndo_bpf
(iavf, for example). The refusal branch is then skipped and the VF is
joined. netvsc_vf_handle_frame() hands the VF's frames to netvsc, and
they never go through netvsc's program, which only runs on the synthetic
channels. Nothing is logged.

netvsc_bpf() has the same early return, so attaching a program to
netvsc while such a VF is joined also appears to succeed.

That seems to contradict two statements in the commit message: "nothing
reaches netvsc through the VF without having been through the program"
and "a VF netvsc uses now always runs netvsc's program when netvsc has
one".

Bonding, which this patch follows, refuses such a slave in
bond_enslave():

	if (!slave_dev->netdev_ops->ndo_bpf ||
	    !slave_dev->netdev_ops->ndo_xdp_xmit) {
		if (bond->xdp_prog) {
			SLAVE_NL_ERR(bond_dev, slave_dev, extack,
				     "Slave does not support XDP");

> +		if (ret) {
> +			netdev_warn(ndev, "not using VF %s, it refused netvsc's XDP program: %d\n",
> +				    vf_netdev->name, ret);
> +			return NOTIFY_DONE;
> +		}
> +	}

[Severity: Medium]
Once a VF is refused here, or by the dev_xdp_prog_count() check above,
what brings it back later?

netvsc_register_vf() is only called from netvsc_probe() and from the
VF's NETDEV_REGISTER notifier. The later patch "hv_netvsc: move a new VF
to netvsc's netns from a work item" adds a netns-move path.

netvsc_vf_changed() finds netvsc through get_netvsc_byref(), which needs
the master upper link that a refused VF never gets:

drivers/net/hyperv/netvsc_drv.c:get_netvsc_byref() {
    ...
	dev = netdev_master_upper_dev_get(vf_netdev);
	if (!dev || dev->netdev_ops != &device_ops)
		return NULL;
    ...
}

netvsc_bpf() only touches vf_netdev when a VF is already joined.

So detaching netvsc's program, or removing the VF's own program, does
not retry the takeover. The accelerated datapath stays off until the VF
registers again, for example on a host remove/re-add or a manual netns
round-trip.

The handling is also asymmetric. If the VF is already joined, attaching
a program it refuses fails the attach and the VF is kept. If the program
was there first, the VF is lost until it re-registers.

The commit message only says "traffic then stays on the synthetic path,
where the program runs". Bonding refuses a user-initiated enslave that
can be retried, but netvsc's takeover is automatic.

Should something retry the takeover when the program goes away, or
should the commit message at least mention this?

> +
>  	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) {
> +		if (prog)
> +			netvsc_vf_setxdp(vf_netdev, NULL);
>  		return NOTIFY_DONE;
> +	}

[ ... ]

> @@ -2453,6 +2477,15 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
>  
>  	reinit_completion(&net_device_ctx->vf_add);
>  	netdev_rx_handler_unregister(vf_netdev);
> +
> +	/* Only once frames from the VF no longer reach netvsc */
> +	nvdev = rtnl_dereference(net_device_ctx->nvdev);
> +	if (nvdev && netvsc_xdp_get(nvdev)) {

[Severity: Medium]
The take-back depends on netvsc's channels carrying a program right
now. Can the channel program be cleared while nvdev stays installed and
the VF still holds netvsc's propagated program, along with the reference
netvsc_vf_setxdp() took for it?

One case is a failed replace in netvsc_bpf(). P1 runs on netvsc and the
VF. Attaching P2 calls netvsc_xdp_set(P2), and then the VF refuses P2.
For example, mana_xdp_set() restores apc->bpf_prog = old_prog when
mana_pre_alloc_rxbufs() fails, and netif_xdp_propagate() returns -EBUSY
with tcp-data-split or a memory provider. The rollback then leaves the
channels empty:

drivers/net/hyperv/netvsc_bpf.c:netvsc_bpf() {
    ...
			if (bpf->prog)
				bpf_prog_inc(bpf->prog);
			netvsc_xdp_set(dev, NULL, extack, nvdev);
    ...
}

The VF keeps P1, and because the install failed, dev_xdp_attach() keeps
P1 in netvsc's xdp_state.

Another case is netvsc_detach(). It calls netvsc_xdp_set(ndev, NULL,
NULL, nvdev) first, and can then return early when rndis_filter_close()
or netvsc_wait_until_empty() fails, with nvdev still installed.
netvsc_set_channels(), for one, just bails out:

	ret = netvsc_detach(net, nvdev);
	if (ret)
		goto out;

In both states, the VF can later leave: VF unregister, netns move,
netvsc unbind or hot-remove, or the rmmod notifier replay. This check is
then false.

Does the VF keep running P1 with no upper, and does its driver's
reference to P1 leak?

The patch also removes the unconditional push at registration. The
commit message says that push "was all that cleared a program an earlier
netvsc instance left on the VF". A VF in this state that rejoins a
netvsc with no program, for example after reloading hv_netvsc, would
keep the stale program. dev_xdp_prog_count() cannot see it.

The nvdev == NULL paths, such as both netvsc_attach() attempts failing
or suspend, don't reach this check. get_netvsc_byref() already returns
NULL when nvdev is NULL, and that behaviour predates this patch.

> +		netdev_lock_ops(vf_netdev);
> +		netvsc_vf_setxdp(vf_netdev, NULL);

[Severity: Medium]
The return value of netvsc_vf_setxdp(vf_netdev, NULL) is dropped here.
The VF is then unlinked and released as though the program had been
removed. Can the removal fail?

netif_xdp_propagate() returns -EBUSY whenever a memory provider is
bound, even for a NULL program:

	if (dev_get_min_mp_channel_count(dev)) {
		NL_SET_ERR_MSG(bpf->extack, "unable to propagate XDP to device using memory provider");
		return -EBUSY;
	}

A memory provider can be bound to a joined VF that runs netvsc's
program. __net_mp_open_rxq() only checks dev_xdp_prog_count(dev), and
that count doesn't include propagated programs.

mana_xdp_set(NULL) on an up port can also fail in mana_pre_alloc_rxbufs(),
mana_detach() or mana_attach(). Each of those paths restores
apc->bpf_prog = old_prog and returns before bpf_prog_put(old_prog).

After such a failure, the VF keeps netvsc's program and the reference,
with no upper left to clean them up, and nothing is logged. The rollback
after a failed netvsc_vf_join() in netvsc_register_vf() also ignores the
result.

Bonding at least warns in __bond_release_one():

		if (dev_xdp_propagate(slave_dev, &xdp))
			slave_warn(bond_dev, slave_dev, "failed to unload XDP program\n");

Should these two calls at least log a failure?

> +		netdev_unlock_ops(vf_netdev);
> +	}
> +
>  	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/20261001014131.310771-1-kuba%40kernel.org

Re: [PATCH net-next 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: 2 · 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] The new takeover branch in netvsc_event_set_vf_ns()
  (drivers/net/hyperv/netvsc_drv.c:2839-2846) can enslave a VF without…
- [Medium] netvsc_suspend() now drops a pending VF netns move or
  takeover, and nothing restores it.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but netvsc_probe()
  can free a net_device whose vfns_work is still pending, which is a…

--

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

The patch stops netvsc_register_vf() from calling
dev_change_net_namespace() from inside the VF's NETDEV_REGISTER notifier.
That call deadlocks on the instance lock of ops-locked VFs. The patch
instead takes a reference on the VF in ndev_ctx->vfns_dev and leaves the
move to vfns_work. If netvsc is already in the VF's netns when the work
runs, the work takes the VF over itself.

> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 17c86e7493577..10c05809c0659 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c

[ ... ]

> @@ -2695,6 +2699,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 is a pre-existing issue and was not introduced by this patch. Does the
register_failed path in netvsc_probe() also need to cancel vfns_work, the
way netvsc_remove() does here?

register_netdevice() runs the NETDEV_REGISTER chain. netvsc's own handler
queues the work before anyone knows whether registration will succeed:

netvsc_netdev_event() {
	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;
	}
	...
}

A later notifier in the chain can fail, for example addrconf_notify()
when ipv6_add_dev() fails to allocate. register_netdevice() then unwinds
and returns an error, and netvsc_probe() runs:

register_failed:
	rndis_filter_device_remove(dev, nvdev);
rndis_failed:
	rtnl_unlock();
	...
	hv_set_drvdata(dev, NULL);
	free_netdev(net);

Probe holds RTNL the whole time. netvsc_vfns_work() therefore fails
rtnl_trylock() and keeps re-arming itself with
schedule_delayed_work(&ndev_ctx->vfns_work, 1).

Can that timer or work then run on the freed net_device priv area, where
netvsc_vfns_work() reads ndev_ctx->device_ctx?

The new vfns_dev reference cannot leak on this path. ndev_ctx is only
added to netvsc_dev_list after registration succeeds, so
get_netvsc_byslot() never finds it.

> @@ -2738,6 +2743,7 @@ static int netvsc_suspend(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 == NULL) {

[Severity: Medium]
Could this lose a pending VF move for good?

Before this patch the move ran synchronously in the VF's NETDEV_REGISTER
notifier. Now netvsc_register_vf() only records the VF in
ndev_ctx->vfns_dev and queues vfns_work. If netvsc_suspend() takes RTNL
before netvsc_vfns_work() wins rtnl_trylock(), this code throws away both
the request and the VF reference.

netvsc_resume() only resets data_path_is_vf and calls netvsc_attach().
Nothing queues vfns_work again.

The suspend error paths don't restore the request either. Those are the
nvdev == NULL check and a NULL return from the GFP_ATOMIC
netvsc_devinfo_get(). Resume is not called after a failed suspend.

A VF that survives suspend sends no new NETDEV_REGISTER. netvsc_vf_changed()
notes that mlx5 keeps the interface and sends only CHANGE.
NETDEV_UP/CHANGE can't recover the VF, because get_netvsc_byref() needs a
master link that was never set up.

Would the VF then stay in the other netns and never be joined? The last
patch in the series does not change this code.

> @@ -2815,7 +2821,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),
> @@ -2828,7 +2836,16 @@ 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
> +		 */
> +		netdev_lock_ops(vf_netdev);
> +		netvsc_register_vf(vf_netdev, VF_REG_IN_NOTIFIER);
> +		netdev_unlock_ops(vf_netdev);
>  	}
> +out:
> +	netvsc_vfns_dev_put(ndev_ctx);
>  }

[Severity: Medium]
Does this takeover path ever switch the data path to the VF if netvsc and
the VF are both already up?

netvsc_register_vf() joins the VF and schedules vf_takeover. That work
runs __netvsc_vf_setup():

__netvsc_vf_setup() {
	...
	dev_change_flags(vf_netdev, ndev->flags | IFF_SLAVE, NULL);
	...
	if (netif_running(ndev)) {
		ret = dev_open(vf_netdev, NULL);
	...
}

If both devices already have IFF_UP set, dev_change_flags() does not
change IFF_UP. __dev_notify_flags() then emits neither NETDEV_UP nor
NETDEV_CHANGE, and dev_open() on a running device emits nothing.

netvsc_switch_datapath() is only called from netvsc_vf_changed(), which
only runs on the VF's UP/DOWN/CHANGE/GOING_DOWN events.

Would data_path_is_vf then stay false? That would keep netvsc_start_xmit()
and host RX on the synthetic path until some later event on the VF.

One sequence that gets here:

- The VF registers in netns A while netvsc is in netns B.
- netvsc is moved to A.
- Both devices are brought up in A before vfns_work wins rtnl_trylock().

The normal re-register path doesn't have this gap.
dev_change_net_namespace() closes the VF, so the later open always produces
NETDEV_UP. The commit message treats the takeover as equivalent to that
path, which doesn't seem to hold in this case.

The later patch in the series ("hv_netvsc: let the core take XDP off a
netvsc device that is going away") only touches netvsc_bpf.c, so this gap
is still there at the end of the series.

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

Re: [PATCH net-next 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 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · 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] The new branch in netvsc_bpf()
  (drivers/net/hyperv/netvsc_bpf.c:206-213) returns success for…
- [Medium] The fix treats a non-NULL ndev_ctx->saved_netvsc_dev_info as
  meaning 'suspend parked the program and netvsc_resume() will put it…

--

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

With this patch, netvsc_bpf() returns success for an XDP program removal
when nvdev is gone, no VF is linked and suspend has not saved its config.
The aim is to stop dev_xdp_uninstall() from hitting its WARN_ON() on
unbind or hot-remove of a netvsc device running native XDP. It also lets
the program be dropped from the XDP dispatcher.

> diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
> index 951c19ce15ebc..7fe38574f66f6 100644
> --- a/drivers/net/hyperv/netvsc_bpf.c
> +++ b/drivers/net/hyperv/netvsc_bpf.c
> @@ -204,6 +204,12 @@ int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
>  	int ret;
>  
>  	if (!nvdev || nvdev->destroy) {
> +		/* The channels and the VF are gone, and so is the program,
> +		 * unless suspend parked it for netvsc_resume() to put back.
> +		 */
> +		if (bpf->command == XDP_SETUP_PROG && !bpf->prog && !vf_netdev &&
> +		    !ndevctx->saved_netvsc_dev_info)
> +			return 0;

[Severity: Medium]
Does !vf_netdev here guarantee that the program is gone from the VF?

netvsc_unregister_vf() takes netvsc's program back from the VF only while
nvdev exists and the channels still have a program. It clears vf_netdev
in every case:

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

	netdev_upper_dev_unlink(vf_netdev, ndev);
	RCU_INIT_POINTER(net_device_ctx->vf_netdev, NULL);
    ...
}

nvdev can be NULL while the VF is still linked and still runs netvsc's
program. netvsc_detach() clears XDP only on the channels:

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

Both attaches can then fail in netvsc_set_channels().
netvsc_change_mtu() and netvsc_set_ringparam() follow the same pattern:

drivers/net/hyperv/netvsc_drv.c:netvsc_set_channels() {
    ...
	ret = netvsc_attach(net, device_info);
	if (ret) {
		device_info->num_chn = orig;
		if (netvsc_attach(net, device_info))
			netdev_err(net, "restoring channel setting failed\n");
	}
    ...
}

After that, nvdev stays NULL. The VF still holds the program, along with
the reference that netvsc_vf_setxdp() took through
netif_xdp_propagate().

A later unbind, hot-remove, rmmod or netns move would then do this:

netvsc_unregister_vf()
    nvdev == NULL, so netvsc_vf_setxdp(vf_netdev, NULL) is skipped
    vf_netdev = NULL
dev_xdp_uninstall()->dev_xdp_install(NULL)->netvsc_bpf()
    nvdev == NULL, vf_netdev == NULL, saved_netvsc_dev_info == NULL
    return 0

At that point the core treats the program as gone. The former VF keeps
running netvsc's XDP program on its traffic, and nothing records the
program in the VF's xdp_state.

Before this patch, this state at least triggered the WARN in
dev_xdp_uninstall(). The gating in netvsc_unregister_vf() comes from the
earlier commit in the series, "hv_netvsc: treat the VF's XDP program the
way bonding treats a slave's". The new guard and comment assume that
gating covers every case.

Could the take-back in netvsc_unregister_vf() check what netvsc has
recorded, for example dev_xdp_prog_count(ndev), instead of the channel
state? Alternatively, should the guard here stop treating !vf_netdev as
proof that the program was removed?

[Severity: Medium]
Can saved_netvsc_dev_info be non-NULL here even though netvsc_resume()
will never run to put the program back?

netvsc_suspend() saves the config, which takes a bprog reference through
netvsc_devinfo_get(), and only then calls netvsc_detach():

drivers/net/hyperv/netvsc_drv.c:netvsc_suspend() {
    ...
	ndev_ctx->saved_netvsc_dev_info = netvsc_devinfo_get(nvdev);
	...
	ret = netvsc_detach(net, nvdev);
    ...
}

netvsc_detach() clears the channel programs first and can fail after
that:

drivers/net/hyperv/netvsc_drv.c:netvsc_detach() {
    ...
	netvsc_xdp_set(ndev, NULL, NULL, nvdev);
	...
		ret = rndis_filter_close(nvdev);
		if (ret) {
			...
			return ret;
		}

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

The error goes back through vmbus_suspend(), the noirq callback. Because
the callback failed, device_suspend_noirq() does not set
is_noirq_suspended. device_resume_noirq() then skips the device, so
netvsc_resume() never runs. netvsc_resume() is the only code that puts
and clears saved_netvsc_dev_info, and netvsc_remove() does not touch it.

A later unbind or hot-remove with native XDP still attached would then
go like this:

netvsc_remove()
    netvsc_xdp_set(net, NULL, NULL, nvdev)
    rndis_filter_device_remove()    /* nvdev becomes NULL */
    unregister_netdevice()
        dev_xdp_uninstall()
            dev_xdp_install(NULL)->netvsc_bpf()
                saved_netvsc_dev_info != NULL
                return -ENODEV
            WARN_ON(...)

On this path the WARN_ON() in dev_xdp_uninstall() that this patch is
meant to remove still fires. bpf_prog_change_xdp() is also skipped, so
the XDP dispatcher keeps its reference to the program.

The commit message gives the reason for the exclusion as:

  "unless suspend parked the program for resume to put back, as the core
  would then lose track of it"

dev_xdp_uninstall() drops xdp_state and the core's reference no matter
what the driver returns. Does that reason still hold on this path?

The leaked devinfo and its bprog reference predate this patch, but the
fix now depends on that stale pointer. Should saved_netvsc_dev_info be
put and cleared on the netvsc_suspend() error path, and in
netvsc_remove() before unregister_netdevice()?

[ ... ]

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

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

[Kameron Carr]

On 9/30/2026 6:41 PM, Jakub Kicinski wrote:
> 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.
> 
> 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>
> ---
>  drivers/net/hyperv/netvsc_bpf.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
> index 1dd3755d9e6d..731bb7721fe2 100644
> --- a/drivers/net/hyperv/netvsc_bpf.c
> +++ b/drivers/net/hyperv/netvsc_bpf.c
> @@ -216,6 +216,13 @@ 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");
>  
> +			/* Since we haven't completed the installation
> +			 * of bpf->prog the reference core implicitly
> +			 * transfers to us on success isn't ours.
> +			 * Take a reference to balance the accounting.
> +			 */
> +			if (bpf->prog)
> +				bpf_prog_inc(bpf->prog);
>  			netvsc_xdp_set(dev, NULL, extack, nvdev);
>  		}
>  

The refcount part looks right to me.

Not new, but this rollback clears the channels rather than restoring
them. If the VF refuses a replace (P1 -> P2), or refuses the NULL on
detach (memory provider bound, mana failing to re-create its queues),
the core keeps P1 in xdp_state and the VF keeps running P1, but the
synthetic path is left with no program at all.

Could we save a reference to the old program before netvsc_xdp_set() and
restore it here instead of setting NULL? In patch 3/5
netvsc_unregister_vf() decides the take-back based on the channels, so
this matters there too.

-- 
Regards,
Kameron

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

[Kameron Carr]

On 10/2/2026 12:38 PM, Haiyang Zhang wrote:
> 
> Thank you for making this patch set, looks good to me.
> I will find a teammate to test it.
> 
> - Haiyang

I ran LISA functional tests (T4) in Azure, including XDP related tests
[1], on the patch set based on v7.3-rc6.

I did not see any regressions in my testing, but my testing did not
cover:

  * Hibernation scenarios
  * Kdump tests (not super relevant but worth a note)
  * Performance (XDP or otherwise)
  * Scenarios trying to repro issues found in the patches

If you are worried about the performance impacts of this patchset, I can
do a comparison of performance with and without the patches.

Looking at the Sashiko comments, many of them look valid. It is at least
worth taking another look before merging. I am not providing a review
sign off, only the results of my regression testing.

[1] https://github.com/microsoft/lisa/tree/main/lisa/microsoft/testsuites/xdp


Tested-by: Kameron Carr <kameroncarr@linux.microsoft.com>

-- 
Regards,
Kameron

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

[Jakub Kicinski]

On Tue, 6 Oct 2026 13:59:05 -0700 Kameron Carr wrote:
> If you are worried about the performance impacts of this patchset, I can
> do a comparison of performance with and without the patches.

Thanks for testing! No concern about perf, changes should be purely
ctrl path.

> Looking at the Sashiko comments, many of them look valid. It is at least
> worth taking another look before merging. I am not providing a review
> sign off, only the results of my regression testing.

Ack, looked like follow up material when I glanced. I need to double
check them, but haven't had time to do any real work last few days :(

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

[Erni Sri Satya Vennela]

On Tue, Oct 06, 2026 at 01:59:05PM -0700, Kameron Carr wrote:
> On 10/2/2026 12:38 PM, Haiyang Zhang wrote:
> > 
> > Thank you for making this patch set, looks good to me.
> > I will find a teammate to test it.
> > 
> > - Haiyang
> 
> I ran LISA functional tests (T4) in Azure, including XDP related tests
> [1], on the patch set based on v7.3-rc6.
> 
> I did not see any regressions in my testing, but my testing did not
> cover:
> 
>   * Hibernation scenarios
>   * Kdump tests (not super relevant but worth a note)
>   * Performance (XDP or otherwise)
>   * Scenarios trying to repro issues found in the patches
> 
Hi Kameron, Jakub,

I tested all five patches applied together on net-next base
8b4e7209c842, with additional targeted lifecycle/error-path
coverage on Azure MANA and mlx5 VFs.

Passing coverage includes:

* VF removal/rejoin: without reattaching the parent program, three
  cycles preserved its program ID. Exact-five synthetic XDP drops
  passed while the VF was absent, and propagation plus exact-five
  live VF XDP drops passed after each rejoin.

* Initial attachment refusal: a program intended for netvsc survived
  five VF-induced attachment failures, then attached and propagated
  successfully after restoring the VF MTU. It was reclaimed after
  detach and pin removal.

* Concurrent replacement/hotplug: while cycling VF availability,
  68 parent-program replacements between PASS and DROP succeeded
  without command errors (38 with the VF present, 30 absent).
  Final PASS/DROP packet checks and program reclamation passed.

* Deferred namespace rejoin: the parent retained the same XDP program;
  the worker moved the VF into its namespace, rejoined it and restored
  propagation, with packet-processing checks.

* Synthetic-device removal: the old program was reclaimed, the
  surviving VF accepted an independent program, and rebinding netvsc
  restored association and connectivity.

* Actual Azure hibernation/resume on mlx5 passed with the VF present
  and with it absent. The parent program and counter-map state survived
  without reattachment; post-resume packet checks passed, including
  propagation to the returning VF in the VF-present case.

* Incompatible arriving VF on mlx5: a VF-only MTU of 9000 caused
  propagation refusal. Netvsc kept the VF unjoined and its existing
  XDP program active on the synthetic path. Restoring the VF MTU and
  re-registering it allowed join, propagation and VF packet processing;
  final reclamation and cleanup passed.

* Join-failure rollback on mlx5: a bridge-induced -EBUSY after
  successful propagation detached XDP from the unjoined VF while
  preserving the parent's program and synthetic-path processing.
  Removing the conflict allowed rejoin, propagation and VF packet
  processing.

I am continuing investigation of synthetic fallback with an
independently programmed MANA VF and rejected-replacement restoration.

Regards,
Vennela

Tested-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>