* [PATCH v2 2/2] net/netvsc: support runtime queue count reconfiguration
2026-02-26 4:24 [PATCH v2 0/2] net/netvsc: runtime queue reconfiguration and fixes Long Li
2026-02-26 4:24 ` [PATCH v2 1/2] net/netvsc: fix subchannel leak on device removal Long Li
@ 2026-02-26 4:24 ` Long Li
2026-03-02 19:38 ` [PATCH v2 0/2] net/netvsc: runtime queue reconfiguration and fixes Stephen Hemminger
2 siblings, 0 replies; 5+ messages in thread
From: Long Li @ 2026-02-26 4:24 UTC (permalink / raw)
To: dev; +Cc: stephen, longli, weh, stable
Add support for changing the number of RX/TX queues at runtime
via port stop/configure/start. When the queue count changes,
perform a full NVS/RNDIS teardown and reinit to allocate fresh
VMBus subchannels matching the new queue count, then reconfigure
RSS indirection table accordingly.
Key changes:
- hn_dev_configure: detect queue count changes and perform full
NVS session reinit with subchannel teardown/recreation
- hn_dev_stop: drain pending TX completions (up to 1s) to prevent
stale completions from corrupting queue state after reconfig
- hn_vf_tx/rx_queue_release: use write lock when nulling VF queue
pointers to prevent use-after-free with concurrent fast-path
readers
Signed-off-by: Long Li <longli@microsoft.com>
---
v2:
- Fix reinit_failed recovery: re-map device before chan_open when
device is unmapped
- Move hn_rndis_conf_offload() to after reinit block
- Use write lock in hn_vf_tx/rx_queue_release()
- Reset RSS indirection table in subchan_cleanup error path
- Fix multi-line comment style
drivers/net/netvsc/hn_ethdev.c | 171 +++++++++++++++++++++++++++++++--
drivers/net/netvsc/hn_vf.c | 16 +--
2 files changed, 171 insertions(+), 16 deletions(-)
diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index 798b4c9023..e0885b74b7 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -745,6 +745,9 @@ netvsc_hotadd_callback(const char *device_name, enum rte_dev_event_type type,
}
}
+static void hn_detach(struct hn_data *hv);
+static int hn_attach(struct hn_data *hv, unsigned int mtu);
+
static int hn_dev_configure(struct rte_eth_dev *dev)
{
struct rte_eth_conf *dev_conf = &dev->data->dev_conf;
@@ -754,6 +757,8 @@ static int hn_dev_configure(struct rte_eth_dev *dev)
struct hn_data *hv = dev->data->dev_private;
uint64_t unsupported;
int i, err, subchan;
+ uint32_t old_subchans = 0;
+ bool device_unmapped = false;
PMD_INIT_FUNC_TRACE();
@@ -778,36 +783,95 @@ static int hn_dev_configure(struct rte_eth_dev *dev)
hv->vlan_strip = !!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_STRIP);
- err = hn_rndis_conf_offload(hv, txmode->offloads,
- rxmode->offloads);
- if (err) {
- PMD_DRV_LOG(NOTICE,
- "offload configure failed");
- return err;
- }
+ /* If queue count unchanged, skip subchannel teardown/reinit */
+ if (RTE_MAX(dev->data->nb_rx_queues,
+ dev->data->nb_tx_queues) == hv->num_queues)
+ goto skip_reinit;
hv->num_queues = RTE_MAX(dev->data->nb_rx_queues,
dev->data->nb_tx_queues);
+ /* Close all existing subchannels */
+ for (i = 1; i < HN_MAX_CHANNELS; i++) {
+ if (hv->channels[i] != NULL) {
+ rte_vmbus_chan_close(hv->channels[i]);
+ hv->channels[i] = NULL;
+ old_subchans++;
+ }
+ }
+
+ /*
+ * If subchannels existed, do a full NVS/RNDIS teardown
+ * and vmbus re-init to ensure a clean NVS session.
+ * Cannot re-send NVS subchannel request on the same
+ * session without invalidating the data path.
+ */
+ if (old_subchans > 0) {
+ PMD_DRV_LOG(NOTICE,
+ "reinit NVS (had %u subchannels)",
+ old_subchans);
+
+ hn_detach(hv);
+
+ rte_vmbus_chan_close(hv->channels[0]);
+ rte_free(hv->channels[0]);
+ hv->channels[0] = NULL;
+
+ rte_vmbus_unmap_device(hv->vmbus);
+ device_unmapped = true;
+ err = rte_vmbus_map_device(hv->vmbus);
+ if (err) {
+ PMD_DRV_LOG(ERR,
+ "Could not re-map vmbus device!");
+ goto reinit_failed;
+ }
+ device_unmapped = false;
+
+ hv->rxbuf_res = hv->vmbus->resource[HV_RECV_BUF_MAP];
+ hv->chim_res = hv->vmbus->resource[HV_SEND_BUF_MAP];
+
+ err = rte_vmbus_chan_open(hv->vmbus, &hv->channels[0]);
+ if (err) {
+ PMD_DRV_LOG(ERR,
+ "Could not re-open vmbus channel!");
+ goto reinit_failed;
+ }
+
+ rte_vmbus_set_latency(hv->vmbus, hv->channels[0],
+ hv->latency);
+
+ err = hn_attach(hv, dev->data->mtu);
+ if (err) {
+ rte_vmbus_chan_close(hv->channels[0]);
+ rte_free(hv->channels[0]);
+ hv->channels[0] = NULL;
+ PMD_DRV_LOG(ERR,
+ "NVS reinit failed: %d", err);
+ goto reinit_failed;
+ }
+ }
+
for (i = 0; i < NDIS_HASH_INDCNT; i++)
hv->rss_ind[i] = i % dev->data->nb_rx_queues;
hn_rss_hash_init(hv, rss_conf);
subchan = hv->num_queues - 1;
+
+ /* Allocate fresh subchannels and configure RSS */
if (subchan > 0) {
err = hn_subchan_configure(hv, subchan);
if (err) {
PMD_DRV_LOG(NOTICE,
"subchannel configuration failed");
- return err;
+ goto subchan_cleanup;
}
err = hn_rndis_conf_rss(hv, NDIS_RSS_FLAG_DISABLE);
if (err) {
PMD_DRV_LOG(NOTICE,
"rss disable failed");
- return err;
+ goto subchan_cleanup;
}
if (rss_conf->rss_hf != 0) {
@@ -815,12 +879,75 @@ static int hn_dev_configure(struct rte_eth_dev *dev)
if (err) {
PMD_DRV_LOG(NOTICE,
"initial RSS config failed");
- return err;
+ goto subchan_cleanup;
}
}
}
+skip_reinit:
+ /* Apply offload config after reinit so it targets the final RNDIS session */
+ err = hn_rndis_conf_offload(hv, txmode->offloads,
+ rxmode->offloads);
+ if (err) {
+ PMD_DRV_LOG(NOTICE,
+ "offload configure failed");
+ return err;
+ }
+
return hn_vf_configure_locked(dev, dev_conf);
+
+subchan_cleanup:
+ for (i = 1; i < HN_MAX_CHANNELS; i++) {
+ if (hv->channels[i] != NULL) {
+ rte_vmbus_chan_close(hv->channels[i]);
+ hv->channels[i] = NULL;
+ }
+ }
+ hv->num_queues = 1;
+ for (i = 0; i < NDIS_HASH_INDCNT; i++)
+ hv->rss_ind[i] = 0;
+ return err;
+
+reinit_failed:
+ /*
+ * Device is in a broken state after failed reinit.
+ * Try to re-establish minimal connectivity.
+ */
+ PMD_DRV_LOG(ERR,
+ "reinit failed (err %d), attempting recovery", err);
+ if (hv->channels[0] == NULL) {
+ if (device_unmapped) {
+ if (rte_vmbus_map_device(hv->vmbus)) {
+ hv->num_queues = 0;
+ PMD_DRV_LOG(ERR,
+ "recovery failed, could not re-map device");
+ return err;
+ }
+ hv->rxbuf_res = hv->vmbus->resource[HV_RECV_BUF_MAP];
+ hv->chim_res = hv->vmbus->resource[HV_SEND_BUF_MAP];
+ }
+ if (rte_vmbus_chan_open(hv->vmbus, &hv->channels[0]) == 0) {
+ if (hn_attach(hv, dev->data->mtu) == 0) {
+ hv->num_queues = 1;
+ PMD_DRV_LOG(NOTICE,
+ "recovery successful on primary channel");
+ } else {
+ rte_vmbus_chan_close(hv->channels[0]);
+ rte_free(hv->channels[0]);
+ hv->channels[0] = NULL;
+ hv->num_queues = 0;
+ PMD_DRV_LOG(ERR,
+ "recovery failed, device unusable");
+ }
+ } else {
+ hv->num_queues = 0;
+ PMD_DRV_LOG(ERR,
+ "recovery failed, device unusable");
+ }
+ } else {
+ hv->num_queues = 1;
+ }
+ return err;
}
static int hn_dev_stats_get(struct rte_eth_dev *dev,
@@ -1067,6 +1194,7 @@ hn_dev_stop(struct rte_eth_dev *dev)
{
struct hn_data *hv = dev->data->dev_private;
int i, ret;
+ unsigned int retry;
PMD_INIT_FUNC_TRACE();
dev->data->dev_started = 0;
@@ -1075,6 +1203,29 @@ hn_dev_stop(struct rte_eth_dev *dev)
hn_rndis_set_rxfilter(hv, 0);
ret = hn_vf_stop(dev);
+ /*
+ * Drain pending TX completions to prevent stale completions
+ * from corrupting queue state after port reconfiguration.
+ */
+ for (retry = 0; retry < 100; retry++) {
+ uint32_t pending = 0;
+
+ for (i = 0; i < hv->num_queues; i++) {
+ struct hn_tx_queue *txq = dev->data->tx_queues[i];
+
+ if (txq == NULL)
+ continue;
+ hn_process_events(hv, i, 0);
+ pending += rte_mempool_in_use_count(txq->txdesc_pool);
+ }
+ if (pending == 0)
+ break;
+ rte_delay_ms(10);
+ }
+ if (retry >= 100)
+ PMD_DRV_LOG(WARNING,
+ "Failed to drain all TX completions");
+
for (i = 0; i < hv->num_queues; i++) {
dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
diff --git a/drivers/net/netvsc/hn_vf.c b/drivers/net/netvsc/hn_vf.c
index 0ecfaf54ea..e77232bfb3 100644
--- a/drivers/net/netvsc/hn_vf.c
+++ b/drivers/net/netvsc/hn_vf.c
@@ -637,12 +637,14 @@ void hn_vf_tx_queue_release(struct hn_data *hv, uint16_t queue_id)
{
struct rte_eth_dev *vf_dev;
- rte_rwlock_read_lock(&hv->vf_lock);
+ rte_rwlock_write_lock(&hv->vf_lock);
vf_dev = hn_get_vf_dev(hv);
- if (vf_dev && vf_dev->dev_ops->tx_queue_release)
+ if (vf_dev && vf_dev->dev_ops->tx_queue_release) {
(*vf_dev->dev_ops->tx_queue_release)(vf_dev, queue_id);
+ vf_dev->data->tx_queues[queue_id] = NULL;
+ }
- rte_rwlock_read_unlock(&hv->vf_lock);
+ rte_rwlock_write_unlock(&hv->vf_lock);
}
int hn_vf_rx_queue_setup(struct rte_eth_dev *dev,
@@ -669,11 +671,13 @@ void hn_vf_rx_queue_release(struct hn_data *hv, uint16_t queue_id)
{
struct rte_eth_dev *vf_dev;
- rte_rwlock_read_lock(&hv->vf_lock);
+ rte_rwlock_write_lock(&hv->vf_lock);
vf_dev = hn_get_vf_dev(hv);
- if (vf_dev && vf_dev->dev_ops->rx_queue_release)
+ if (vf_dev && vf_dev->dev_ops->rx_queue_release) {
(*vf_dev->dev_ops->rx_queue_release)(vf_dev, queue_id);
- rte_rwlock_read_unlock(&hv->vf_lock);
+ vf_dev->data->rx_queues[queue_id] = NULL;
+ }
+ rte_rwlock_write_unlock(&hv->vf_lock);
}
int hn_vf_stats_get(struct rte_eth_dev *dev,
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2 0/2] net/netvsc: runtime queue reconfiguration and fixes
2026-02-26 4:24 [PATCH v2 0/2] net/netvsc: runtime queue reconfiguration and fixes Long Li
2026-02-26 4:24 ` [PATCH v2 1/2] net/netvsc: fix subchannel leak on device removal Long Li
2026-02-26 4:24 ` [PATCH v2 2/2] net/netvsc: support runtime queue count reconfiguration Long Li
@ 2026-03-02 19:38 ` Stephen Hemminger
2026-03-03 2:55 ` [EXTERNAL] " Long Li
2 siblings, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2026-03-02 19:38 UTC (permalink / raw)
To: Long Li; +Cc: dev, weh, stable
On Wed, 25 Feb 2026 20:24:12 -0800
Long Li <longli@microsoft.com> wrote:
> This series adds support for runtime queue count reconfiguration in the
> netvsc PMD, along with a prerequisite bug fix.
>
> Patch 1 fixes a pre-existing subchannel resource leak in
> eth_hn_dev_uninit() where subchannels are never closed before the
> primary channel.
>
> Patch 2 adds runtime queue count reconfiguration via port
> stop/configure/start, with full NVS/RNDIS session teardown and reinit
> when the queue count changes.
>
> v2:
> - Split subchannel leak fix into separate patch with Fixes tag (patch 1)
> - Fix reinit_failed recovery: re-map device before chan_open when device
> is unmapped to prevent undefined behavior on unmapped ring buffers
> - Move hn_rndis_conf_offload() to after reinit block so offload config
> targets the final RNDIS session instead of being lost on teardown
> - Use write lock in hn_vf_tx/rx_queue_release() to prevent race with
> concurrent fast-path readers holding read lock
> - Reset RSS indirection table to queue 0 in subchan_cleanup error path
> - Fix multi-line comment style to follow DPDK convention
>
> Long Li (2):
> net/netvsc: fix subchannel leak on device removal
> net/netvsc: support runtime queue count reconfiguration
>
> drivers/net/netvsc/hn_ethdev.c | 181 +++++++++++++++++++++++++++++++--
> drivers/net/netvsc/hn_vf.c | 16 +--
> 2 files changed, 181 insertions(+), 16 deletions(-)
>
After a couple of cycles with Claude, reduced the issues to more concise summary.
Work is still needed.
---
## Patch 1: "fix subchannel leak on device removal"
Adds a loop in `eth_hn_dev_uninit` to close subchannels before closing the primary channel. This fixes a real resource leak I didn't explicitly call out (subchannels allocated in `hn_subchan_configure` were never closed on removal).
## Patch 2: "support runtime queue count reconfiguration"
Major rework of `hn_dev_configure` to support changing queue counts at runtime, plus TX drain in `hn_dev_stop`, plus write-lock fix in VF queue release.
---
## Re-evaluation of my original findings
### Finding #1: Double-free of `hv->primary` — STILL PRESENT
Neither patch changes this. The flow is still:
1. `eth_hn_dev_uninit` → `hn_dev_close` → `hn_dev_free_queues` which calls `hn_rx_queue_free(rxq, false)` — this frees `hv->primary` via `hn_rx_queue_free_common`
2. `eth_hn_dev_uninit` then does `rte_free(hv->primary)` at line 1448 — double-free
### Finding #2: Init error path leaks `hv->primary` and `channels[0]` — STILL PRESENT
The `failed:` label in `eth_hn_dev_init` still only calls `hn_chim_uninit` and `hn_detach`. It never frees `hv->primary` or closes `hv->channels[0]`.
### Finding #3: `hv->primary` alloc failure leaks `channels[0]` — STILL PRESENT
Lines 1376-1380 are untouched.
### Finding #4: `max_chan <= 0` jumps to `failed` with `err` possibly zero — STILL PRESENT
Lines 1406-1407 are untouched. `err` is 0 from the successful `hn_rndis_get_eaddr` call, so the `failed:` path tears down the device and returns success.
### Finding #5: `hn_dev_start` leaks event callback on rxfilter failure — STILL PRESENT
Lines 1035-1047 are untouched.
---
## New issues introduced by Patch 2
### New Issue A: `hn_dev_configure` reinit path doesn't update `hv->primary->chan`
When the reinit path closes the primary channel and opens a new one (lines in the patch around `rte_vmbus_chan_close(hv->channels[0])` ... `rte_vmbus_chan_open(hv->vmbus, &hv->channels[0])`), the `hv->primary` rx queue struct still holds a stale `chan` pointer from the old channel. `hv->primary->chan` was set during `hn_rx_queue_alloc` at init time and is never updated here. The next control operation using `hv->primary` (which uses `hn_primary_chan(hv)` indirectly through `hn_nvs_execute` → `rxq->ring_lock`) would operate on the new channel correctly because `hn_primary_chan()` reads from `hv->channels[0]`, but `hv->primary->chan` is stale. This matters if anything uses `rxq->chan` directly for the primary queue.
### New Issue B: `hn_chim_uninit`/`hn_chim_init` not called during reinit
The reinit path in `hn_dev_configure` calls `hn_detach` (which calls `hn_nvs_detach` → `hn_nvs_disconn_rxbuf` + `hn_nvs_disconn_chim`) and then `hn_attach` (which calls `hn_nvs_attach` → `hn_nvs_conn_chim`). After reinit, `hv->chim_cnt` and `hv->chim_szmax` get updated by `hn_nvs_conn_chim`, but the chimney bitmap (`hv->chim_bmap`) still reflects the old allocation state. If `chim_cnt` changes across the reinit, the bitmap is the wrong size. This could lead to out-of-bounds bitmap access.
### New Issue C: `reinit_failed` recovery doesn't restore `hv->primary->rxbuf_info`
When `hn_attach` succeeds in the reinit path, `hn_nvs_conn_rxbuf` allocates a *new* `rxbuf_info` for `hv->primary`. But `hn_detach` (called earlier) doesn't free the old `rxbuf_info`. So the old one leaks, and the pointer is overwritten. In the recovery path, if `hn_attach` succeeds there too, another `rxbuf_info` is allocated, leaking the previous one.
### New Issue D: VF queue release NULL-sets VF queues but VF setup doesn't re-set them
In `hn_vf_tx_queue_release`, the patch adds `vf_dev->data->tx_queues[queue_id] = NULL`. This is good for preventing use-after-free. However, the corresponding `hn_vf_tx_queue_setup` calls `rte_eth_tx_queue_setup` on the VF which internally sets the VF's queue pointer. So this should be fine on re-setup — not a bug, just noting the asymmetry is intentional.
---
## Summary
All five original findings remain unfixed by these patches. The patches address different (legitimate) problems: subchannel leaks and runtime queue reconfig. The most critical outstanding issue is still the double-free of `hv->primary` (#1), and the new reinit path in Patch 2 introduces potential concerns around stale chimney bitmap state (#B) and leaked `rxbuf_info` (#C).
^ permalink raw reply [flat|nested] 5+ messages in thread