[PATCH] net/netvsc: switch data path to synthetic on device stop

[PATCH] net/netvsc: switch data path to synthetic on device stop

[Long Li]

When DPDK stops a netvsc device (e.g. on testpmd quit), the data path
was left pointing to the VF/MANA device. If the kernel netvsc driver
subsequently reloads the MANA device and opens it, incoming traffic
arrives on the MANA device immediately, before the queues are fully
initialized. This causes bogus RX completion events to appear on the
TX completion queue, triggering a kernel WARNING in mana_poll_tx_cq().

Fix this by switching the data path back to synthetic (via
NVS_DATAPATH_SYNTHETIC) in hn_vf_stop() before stopping the VF device.
This tells the host to route traffic through the synthetic path, so
that when the MANA driver recreates its queues, no unexpected traffic
arrives until netvsc explicitly switches back to VF.

Also update hn_vf_start() to switch the data path back to VF after the
VF device is started, enabling correct stop/start cycling.

Both functions now use write locks instead of read locks since they
modify vf_vsc_switched state.

Fixes: dc7680e8597c ("net/netvsc: support integrated VF")
Cc: stable@dpdk.org

Signed-off-by: Long Li <longli@microsoft.com>
---
 drivers/net/netvsc/hn_vf.c | 52 ++++++++++++++++++++++++++++++++------
 1 file changed, 44 insertions(+), 8 deletions(-)

diff --git a/drivers/net/netvsc/hn_vf.c b/drivers/net/netvsc/hn_vf.c
index 99e8086afa..965468c58f 100644
--- a/drivers/net/netvsc/hn_vf.c
+++ b/drivers/net/netvsc/hn_vf.c
@@ -314,9 +314,15 @@ int hn_vf_add_unlocked(struct rte_eth_dev *dev, struct hn_data *hv)
 	}
 
 switch_data_path:
-	ret = hn_nvs_set_datapath(hv, NVS_DATAPATH_VF);
-	if (ret == 0)
-		hv->vf_ctx.vf_vsc_switched = true;
+	/* Only switch data path to VF if the device is started.
+	 * Otherwise defer to hn_vf_start() to avoid routing traffic
+	 * to the VF before queues are set up.
+	 */
+	if (dev->data->dev_started) {
+		ret = hn_nvs_set_datapath(hv, NVS_DATAPATH_VF);
+		if (ret == 0)
+			hv->vf_ctx.vf_vsc_switched = true;
+	}
 
 exit:
 	return ret;
@@ -521,11 +527,29 @@ int hn_vf_start(struct rte_eth_dev *dev)
 	struct rte_eth_dev *vf_dev;
 	int ret = 0;
 
-	rte_rwlock_read_lock(&hv->vf_lock);
+	rte_rwlock_write_lock(&hv->vf_lock);
 	vf_dev = hn_get_vf_dev(hv);
-	if (vf_dev)
+	if (vf_dev) {
 		ret = rte_eth_dev_start(vf_dev->data->port_id);
-	rte_rwlock_read_unlock(&hv->vf_lock);
+		if (ret == 0) {
+			/* Re-switch data path to VF if VSP has reported
+			 * VF is present and we haven't switched yet
+			 * (e.g. after a stop/start cycle).
+			 */
+			if (hv->vf_ctx.vf_vsp_reported &&
+			    !hv->vf_ctx.vf_vsc_switched) {
+				ret = hn_nvs_set_datapath(hv,
+							  NVS_DATAPATH_VF);
+				if (ret)
+					PMD_DRV_LOG(ERR,
+						    "Failed to switch to VF: %d",
+						    ret);
+				else
+					hv->vf_ctx.vf_vsc_switched = true;
+			}
+		}
+	}
+	rte_rwlock_write_unlock(&hv->vf_lock);
 	return ret;
 }
 
@@ -535,15 +559,27 @@ int hn_vf_stop(struct rte_eth_dev *dev)
 	struct rte_eth_dev *vf_dev;
 	int ret = 0;
 
-	rte_rwlock_read_lock(&hv->vf_lock);
+	rte_rwlock_write_lock(&hv->vf_lock);
 	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev) {
+		/* Switch data path back to synthetic before stopping VF,
+		 * so the host stops routing traffic to the VF device.
+		 */
+		if (hv->vf_ctx.vf_vsc_switched) {
+			ret = hn_nvs_set_datapath(hv, NVS_DATAPATH_SYNTHETIC);
+			if (ret)
+				PMD_DRV_LOG(ERR,
+					    "Failed to switch to synthetic: %d",
+					    ret);
+			hv->vf_ctx.vf_vsc_switched = false;
+		}
+
 		ret = rte_eth_dev_stop(vf_dev->data->port_id);
 		if (ret != 0)
 			PMD_DRV_LOG(ERR, "Failed to stop device on port %u",
 				    vf_dev->data->port_id);
 	}
-	rte_rwlock_read_unlock(&hv->vf_lock);
+	rte_rwlock_write_unlock(&hv->vf_lock);
 
 	return ret;
 }
-- 
2.43.0

Re: [PATCH] net/netvsc: switch data path to synthetic on device stop

[Stephen Hemminger]

On Fri, 20 Mar 2026 17:43:37 -0700
Long Li <longli@microsoft.com> wrote:

> When DPDK stops a netvsc device (e.g. on testpmd quit), the data path
> was left pointing to the VF/MANA device. If the kernel netvsc driver
> subsequently reloads the MANA device and opens it, incoming traffic
> arrives on the MANA device immediately, before the queues are fully
> initialized. This causes bogus RX completion events to appear on the
> TX completion queue, triggering a kernel WARNING in mana_poll_tx_cq().
> 
> Fix this by switching the data path back to synthetic (via
> NVS_DATAPATH_SYNTHETIC) in hn_vf_stop() before stopping the VF device.
> This tells the host to route traffic through the synthetic path, so
> that when the MANA driver recreates its queues, no unexpected traffic
> arrives until netvsc explicitly switches back to VF.
> 
> Also update hn_vf_start() to switch the data path back to VF after the
> VF device is started, enabling correct stop/start cycling.
> 
> Both functions now use write locks instead of read locks since they
> modify vf_vsc_switched state.
> 
> Fixes: dc7680e8597c ("net/netvsc: support integrated VF")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Long Li <longli@microsoft.com>

Looks good to me you might want to address the error condition
spotted by AI review.

---

**Patch: net/netvsc: switch data path to synthetic on device stop**

This patch addresses a real race condition where stopping a netvsc device leaves the data path pointing to the VF/MANA device, causing kernel warnings when the MANA driver later reinitializes. The fix is logically sound — switch to synthetic before stopping the VF, and re-switch to VF on start.

**Error: `hn_vf_stop()` — `vf_vsc_switched` cleared even when `hn_nvs_set_datapath()` fails**

In `hn_vf_stop()`, if `hn_nvs_set_datapath(hv, NVS_DATAPATH_SYNTHETIC)` fails, `vf_vsc_switched` is unconditionally set to `false`. This means the driver believes it switched to synthetic when the host may still be routing traffic through the VF. On a subsequent `hn_vf_start()`, the `!hv->vf_ctx.vf_vsc_switched` check will pass and the driver will try to re-switch to VF — but since the host never left VF mode, this is a no-op at best or confusing at worst. More importantly, if stop is being called on the path to teardown, the flag is now wrong.

I note that `hn_vf_remove_unlocked()` has the same pattern (clears the flag regardless, with the comment "Clear switched flag regardless — VF is being removed"), so this may be intentional for netvsc since on the remove path you want to forget the state. But on the stop path the device is still present and will be restarted — propagating the error and leaving `vf_vsc_switched = true` might be more correct so that `hn_vf_start()` retries the switch. Worth confirming this is intentional.

**Warning: `hn_vf_start()` — error from `hn_nvs_set_datapath()` returned but VF device left started**

In `hn_vf_start()`, if `rte_eth_dev_start()` succeeds but the subsequent `hn_nvs_set_datapath(hv, NVS_DATAPATH_VF)` fails, the function logs the error and returns the failure code. However, the VF device is left in the started state. The caller sees a failure from `hn_vf_start()`, but the VF is running with no traffic routed to it. This is a resource consistency issue — if the datapath switch fails, should the VF be stopped again to maintain consistent state?

**Warning: `hn_vf_add_unlocked()` — change defers datapath switch but still returns 0 on the deferred path**

The patch modifies `hn_vf_add_unlocked()` to skip the datapath switch when `!dev->data->dev_started`. This is correct, but note that in the original code the function would return the result of `hn_nvs_set_datapath()` — if that failed, it returned an error. Now on the deferred path, `ret` retains whatever value it had from the attach/configure path (could be 0 from a successful attach), so the caller gets success even though the datapath switch was not attempted. This is fine for the hot-add-before-start case, just noting the behavior change.

**Info: Lock upgrade from read to write is correct**

Both `hn_vf_start()` and `hn_vf_stop()` correctly switch from `rte_rwlock_read_lock` to `rte_rwlock_write_lock` since they now modify `vf_vsc_switched`. This matches the locking pattern used by `hn_vf_close()` and `hn_nvs_handle_vfassoc()`.

RE: [EXTERNAL] Re: [PATCH] net/netvsc: switch data path to synthetic on device stop

[Long Li]

> Long Li <longli@microsoft.com> wrote:
> 
> > When DPDK stops a netvsc device (e.g. on testpmd quit), the data path
> > was left pointing to the VF/MANA device. If the kernel netvsc driver
> > subsequently reloads the MANA device and opens it, incoming traffic
> > arrives on the MANA device immediately, before the queues are fully
> > initialized. This causes bogus RX completion events to appear on the
> > TX completion queue, triggering a kernel WARNING in mana_poll_tx_cq().
> >
> > Fix this by switching the data path back to synthetic (via
> > NVS_DATAPATH_SYNTHETIC) in hn_vf_stop() before stopping the VF device.
> > This tells the host to route traffic through the synthetic path, so
> > that when the MANA driver recreates its queues, no unexpected traffic
> > arrives until netvsc explicitly switches back to VF.
> >
> > Also update hn_vf_start() to switch the data path back to VF after the
> > VF device is started, enabling correct stop/start cycling.
> >
> > Both functions now use write locks instead of read locks since they
> > modify vf_vsc_switched state.
> >
> > Fixes: dc7680e8597c ("net/netvsc: support integrated VF")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Long Li <longli@microsoft.com>
> 
> Looks good to me you might want to address the error condition spotted by AI
> review.
> 
> ---
> 
> **Patch: net/netvsc: switch data path to synthetic on device stop**
> 
> This patch addresses a real race condition where stopping a netvsc device leaves
> the data path pointing to the VF/MANA device, causing kernel warnings when the
> MANA driver later reinitializes. The fix is logically sound — switch to synthetic
> before stopping the VF, and re-switch to VF on start.
> 
> **Error: `hn_vf_stop()` — `vf_vsc_switched` cleared even when
> `hn_nvs_set_datapath()` fails**
> 
> In `hn_vf_stop()`, if `hn_nvs_set_datapath(hv, NVS_DATAPATH_SYNTHETIC)` fails,
> `vf_vsc_switched` is unconditionally set to `false`. This means the driver believes
> it switched to synthetic when the host may still be routing traffic through the VF.
> On a subsequent `hn_vf_start()`, the `!hv->vf_ctx.vf_vsc_switched` check will
> pass and the driver will try to re-switch to VF — but since the host never left VF
> mode, this is a no-op at best or confusing at worst. More importantly, if stop is
> being called on the path to teardown, the flag is now wrong.
> 
> I note that `hn_vf_remove_unlocked()` has the same pattern (clears the flag
> regardless, with the comment "Clear switched flag regardless — VF is being
> removed"), so this may be intentional for netvsc since on the remove path you
> want to forget the state. But on the stop path the device is still present and will
> be restarted — propagating the error and leaving `vf_vsc_switched = true` might
> be more correct so that `hn_vf_start()` retries the switch. Worth confirming this is
> intentional.
> 
> **Warning: `hn_vf_start()` — error from `hn_nvs_set_datapath()` returned but VF
> device left started**
> 
> In `hn_vf_start()`, if `rte_eth_dev_start()` succeeds but the subsequent
> `hn_nvs_set_datapath(hv, NVS_DATAPATH_VF)` fails, the function logs the error
> and returns the failure code. However, the VF device is left in the started state.
> The caller sees a failure from `hn_vf_start()`, but the VF is running with no traffic
> routed to it. This is a resource consistency issue — if the datapath switch fails,
> should the VF be stopped again to maintain consistent state?

I'm sending v2 to fix this.

> 
> **Warning: `hn_vf_add_unlocked()` — change defers datapath switch but still
> returns 0 on the deferred path**
> 
> The patch modifies `hn_vf_add_unlocked()` to skip the datapath switch when
> `!dev->data->dev_started`. This is correct, but note that in the original code the
> function would return the result of `hn_nvs_set_datapath()` — if that failed, it
> returned an error. Now on the deferred path, `ret` retains whatever value it had
> from the attach/configure path (could be 0 from a successful attach), so the caller
> gets success even though the datapath switch was not attempted. This is fine for
> the hot-add-before-start case, just noting the behavior change.
> 

This warning can be ignored.

Thanks,
Long

> **Info: Lock upgrade from read to write is correct**
> 
> Both `hn_vf_start()` and `hn_vf_stop()` correctly switch from
> `rte_rwlock_read_lock` to `rte_rwlock_write_lock` since they now modify
> `vf_vsc_switched`. This matches the locking pattern used by `hn_vf_close()` and
> `hn_nvs_handle_vfassoc()`.

[PATCH v2] net/netvsc: switch data path to synthetic on device stop

[Long Li]

When DPDK stops a netvsc device (e.g. on testpmd quit), the data path
was left pointing to the VF/MANA device. If the kernel netvsc driver
subsequently reloads the MANA device and opens it, incoming traffic
arrives on the MANA device immediately, before the queues are fully
initialized. This causes bogus RX completion events to appear on the
TX completion queue, triggering a kernel WARNING in mana_poll_tx_cq().

Fix this by switching the data path back to synthetic (via
NVS_DATAPATH_SYNTHETIC) in hn_vf_stop() before stopping the VF device.
This tells the host to route traffic through the synthetic path, so
that when the MANA driver recreates its queues, no unexpected traffic
arrives until netvsc explicitly switches back to VF.

Also update hn_vf_start() to switch the data path back to VF after the
VF device is started, enabling correct stop/start cycling.

Both functions now use write locks instead of read locks since they
modify vf_vsc_switched state.

Fixes: dc7680e8597c ("net/netvsc: support integrated VF")
Cc: stable@dpdk.org

Signed-off-by: Long Li <longli@microsoft.com>
---

v2:
- hn_vf_stop(): only clear vf_vsc_switched on successful datapath switch
- hn_vf_start(): stop VF device if datapath switch to VF fails

 drivers/net/netvsc/hn_vf.c | 58 ++++++++++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 8 deletions(-)

diff --git a/drivers/net/netvsc/hn_vf.c b/drivers/net/netvsc/hn_vf.c
index 99e8086afa..c3f024b378 100644
--- a/drivers/net/netvsc/hn_vf.c
+++ b/drivers/net/netvsc/hn_vf.c
@@ -314,9 +314,17 @@ int hn_vf_add_unlocked(struct rte_eth_dev *dev, struct hn_data *hv)
 	}
 
 switch_data_path:
-	ret = hn_nvs_set_datapath(hv, NVS_DATAPATH_VF);
-	if (ret == 0)
-		hv->vf_ctx.vf_vsc_switched = true;
+	/* Only switch data path to VF if the device is started.
+	 * Otherwise defer to hn_vf_start() to avoid routing traffic
+	 * to the VF before queues are set up.
+	 */
+	if (dev->data->dev_started) {
+		ret = hn_nvs_set_datapath(hv, NVS_DATAPATH_VF);
+		if (ret)
+			PMD_DRV_LOG(ERR, "Failed to switch to VF: %d", ret);
+		else
+			hv->vf_ctx.vf_vsc_switched = true;
+	}
 
 exit:
 	return ret;
@@ -521,11 +529,31 @@ int hn_vf_start(struct rte_eth_dev *dev)
 	struct rte_eth_dev *vf_dev;
 	int ret = 0;
 
-	rte_rwlock_read_lock(&hv->vf_lock);
+	rte_rwlock_write_lock(&hv->vf_lock);
 	vf_dev = hn_get_vf_dev(hv);
-	if (vf_dev)
+	if (vf_dev) {
 		ret = rte_eth_dev_start(vf_dev->data->port_id);
-	rte_rwlock_read_unlock(&hv->vf_lock);
+		if (ret == 0) {
+			/* Re-switch data path to VF if VSP has reported
+			 * VF is present and we haven't switched yet
+			 * (e.g. after a stop/start cycle).
+			 */
+			if (hv->vf_ctx.vf_vsp_reported &&
+			    !hv->vf_ctx.vf_vsc_switched) {
+				ret = hn_nvs_set_datapath(hv,
+							  NVS_DATAPATH_VF);
+				if (ret) {
+					PMD_DRV_LOG(ERR,
+						    "Failed to switch to VF: %d",
+						    ret);
+					rte_eth_dev_stop(vf_dev->data->port_id);
+				} else {
+					hv->vf_ctx.vf_vsc_switched = true;
+				}
+			}
+		}
+	}
+	rte_rwlock_write_unlock(&hv->vf_lock);
 	return ret;
 }
 
@@ -535,15 +563,29 @@ int hn_vf_stop(struct rte_eth_dev *dev)
 	struct rte_eth_dev *vf_dev;
 	int ret = 0;
 
-	rte_rwlock_read_lock(&hv->vf_lock);
+	rte_rwlock_write_lock(&hv->vf_lock);
 	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev) {
+		/* Switch data path back to synthetic before stopping VF,
+		 * so the host stops routing traffic to the VF device.
+		 */
+		if (hv->vf_ctx.vf_vsc_switched) {
+			ret = hn_nvs_set_datapath(hv, NVS_DATAPATH_SYNTHETIC);
+			if (ret) {
+				PMD_DRV_LOG(ERR,
+					    "Failed to switch to synthetic: %d",
+					    ret);
+			} else {
+				hv->vf_ctx.vf_vsc_switched = false;
+			}
+		}
+
 		ret = rte_eth_dev_stop(vf_dev->data->port_id);
 		if (ret != 0)
 			PMD_DRV_LOG(ERR, "Failed to stop device on port %u",
 				    vf_dev->data->port_id);
 	}
-	rte_rwlock_read_unlock(&hv->vf_lock);
+	rte_rwlock_write_unlock(&hv->vf_lock);
 
 	return ret;
 }
-- 
2.43.0