> On Mon, 23 Mar 2026 16:49:57 -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.
> >
> > 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: [email protected]
> >
> > Signed-off-by: Long Li <[email protected]>
> > ---
> 
> AI spotted that ret is overwritten on error path

I have sent v3.

Thanks,
Long

> 
> 
> **Patch: [PATCH v2] net/netvsc: switch data path to synthetic on device
> stop**
> 
> The v2 addresses the two issues from v1 review: `hn_vf_stop()` now only
> clears `vf_vsc_switched` on success, and `hn_vf_start()` now stops the VF if
> the datapath switch fails. Both fixes are correct.
> 
> **Warning: `hn_vf_stop()` — `ret` from failed datapath switch is overwritten
> by `rte_eth_dev_stop()`**
> 
> When `hn_nvs_set_datapath(hv, NVS_DATAPATH_SYNTHETIC)` fails, `ret`
> holds that error. But execution falls through to `rte_eth_dev_stop()`, which
> overwrites `ret`. The caller loses the datapath switch failure — if
> `rte_eth_dev_stop()` succeeds, `hn_vf_stop()` returns 0 despite the datapath
> switch having failed. If the intent is to always stop the VF regardless of the
> switch result (reasonable), the datapath error should be preserved separately,
> or the function should return the first error. Something like:
> 
> ```c
>               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;
>                       }
>               }
> 
>               err = rte_eth_dev_stop(vf_dev->data->port_id);
>               if (err != 0)
>                       PMD_DRV_LOG(ERR, "Failed to stop device on
> port %u",
>                                   vf_dev->data->port_id);
>               if (ret == 0)
>                       ret = err;
> ```
> 
> This preserves the first error while still attempting to stop the VF.
> 
> Everything else looks correct. The lock upgrades from read to write are
> appropriate, the conditional logic in `hn_vf_add_unlocked()` correctly defers
> the datapath switch when the device isn't started, and `hn_vf_start()` 
> properly
> rolls back the VF start on datapath switch failure.

Reply via email to