[PATCH v2 0/2] net/netvsc: runtime queue reconfiguration and fixes

[PATCH v2 0/2] net/netvsc: runtime queue reconfiguration and fixes

[Long Li]

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

-- 
2.43.0

[PATCH v2 1/2] net/netvsc: fix subchannel leak on device removal

[Long Li]

eth_hn_dev_uninit() only closes the primary channel but never
closes subchannels allocated during hn_dev_configure(). This
leaks VMBus subchannel resources on device removal.

Close all subchannels before closing the primary channel to
prevent resource leaks.

Fixes: 4e9c73e96e83 ("net/netvsc: add Hyper-V network device")
Cc: stable@dpdk.org

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

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index 6584819f4f..798b4c9023 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -1433,6 +1433,7 @@ eth_hn_dev_uninit(struct rte_eth_dev *eth_dev)
 {
 	struct hn_data *hv = eth_dev->data->dev_private;
 	int ret, ret_stop;
+	int i;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -1444,6 +1445,15 @@ eth_hn_dev_uninit(struct rte_eth_dev *eth_dev)
 
 	hn_detach(hv);
 	hn_chim_uninit(eth_dev);
+
+	/* Close any subchannels before closing the primary channel */
+	for (i = 1; i < HN_MAX_CHANNELS; i++) {
+		if (hv->channels[i] != NULL) {
+			rte_vmbus_chan_close(hv->channels[i]);
+			hv->channels[i] = NULL;
+		}
+	}
+
 	rte_vmbus_chan_close(hv->channels[0]);
 	rte_free(hv->primary);
 	ret = rte_eth_dev_owner_delete(hv->owner.id);
-- 
2.43.0

[PATCH v2 2/2] net/netvsc: support runtime queue count reconfiguration

[Long Li]

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

Re: [PATCH v2 0/2] net/netvsc: runtime queue reconfiguration and fixes

[Stephen Hemminger]

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

RE: [EXTERNAL] Re: [PATCH v2 0/2] net/netvsc: runtime queue reconfiguration and fixes

[Long Li]

> Subject: [EXTERNAL] Re: [PATCH v2 0/2] net/netvsc: runtime queue
> reconfiguration and fixes
> 
> 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.

I have sent v3 addressing those issues.

Thank you,
Long





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