Skip to contentSkip to conversation

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

PATCH Superseded Long Li 2 participants 4 messages Mar 21 – Mar 23 Network drivers

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Long Li

author
#0 lore raw

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

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")
---
 drivers/net/netvsc/hn_vf.c | 52 ++++++++++++++++++++++++++++++++------
 1 file changed, 44 insertions(+), 8 deletions(-)
drivers/net/netvsc/hn_vf.c +44 -8
index 99e8086afa..965468c58f 100644
@@ -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;
 }
Signature · 1 line
2.43.0
#1 lore raw
Quoted 16 lines · On Fri, 20 Mar 2026 17:43:37 -0700 Long Li <[email protected]> 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.

Quoted 7 lines

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

Fixes dc7680e8597c ("net/netvsc: support integrated VF")

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().

author
#2 lore raw
Quoted 60 lines
Quoted 16 lines · 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.

Quoted 7 lines

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

Fixes dc7680e8597c ("net/netvsc: support integrated VF")

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**

Quoted 6 lines

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.

Quoted 4 lines

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

Quoted 8 lines

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

Quoted 6 lines

**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().

author
#3 lore raw

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

In reply to 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")

---

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(-)
drivers/net/netvsc/hn_vf.c +50 -8
index 99e8086afa..c3f024b378 100644
@@ -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;
 }
Signature · 1 line
2.43.0