[PATCH v2] net/netvsc: fix race condition in RNDIS command execution

[PATCH v2] net/netvsc: fix race condition in RNDIS command execution

[madhukar.mythri]

From: Madhuker Mythri <madhukar.mythri@gmail.com>

When multiple threads issue RNDIS command requests (such as device
info queries and link status checks) simultaneously, command failures
can occur due to concurrent access to shared resources in the RNDIS
execution path.

Add a spinlock to serialize RNDIS command execution and prevent
data corruption.

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

Signed-off-by: Madhuker Mythri <madhukar.mythri@gmail.com>
---
 drivers/net/netvsc/hn_ethdev.c | 1 +
 drivers/net/netvsc/hn_rndis.c  | 2 ++
 drivers/net/netvsc/hn_var.h    | 1 +
 3 files changed, 4 insertions(+)

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index 6584819f4f..b525e287fa 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -1310,6 +1310,7 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
 	PMD_INIT_FUNC_TRACE();
 
 	rte_spinlock_init(&hv->hotadd_lock);
+	rte_spinlock_init(&hv->cmd_lock);
 	LIST_INIT(&hv->hotadd_list);
 
 	vmbus = container_of(device, struct rte_vmbus_device, device);
diff --git a/drivers/net/netvsc/hn_rndis.c b/drivers/net/netvsc/hn_rndis.c
index 7c54eebcef..8a0716df89 100644
--- a/drivers/net/netvsc/hn_rndis.c
+++ b/drivers/net/netvsc/hn_rndis.c
@@ -500,8 +500,10 @@ hn_rndis_query(struct hn_data *hv, uint32_t oid,
 	/* Input data immediately follows RNDIS query. */
 	memcpy(req + 1, idata, idlen);
 
+	rte_spinlock_lock(&hv->cmd_lock);
 	error = hn_rndis_execute(hv, rid, req, reqlen,
 				 comp, comp_len, RNDIS_QUERY_CMPLT);
+	rte_spinlock_unlock(&hv->cmd_lock);
 
 	if (error)
 		goto done;
diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
index 17c1d5d07b..66ed186c0a 100644
--- a/drivers/net/netvsc/hn_var.h
+++ b/drivers/net/netvsc/hn_var.h
@@ -179,6 +179,7 @@ struct hn_data {
 	struct vmbus_channel *channels[HN_MAX_CHANNELS];
 
 	rte_spinlock_t	hotadd_lock;
+	rte_spinlock_t	cmd_lock;
 	LIST_HEAD(hotadd_list, hv_hotadd_context) hotadd_list;
 };
 
-- 
2.50.1 (Apple Git-155)

[RFT 0/2] net/netvsc: fix race conditions

[Stephen Hemminger]

The previous patch about netvsc race conditions identified a real issue,
but the change introduced another lock.

I propose the following change generated by Claude AI.
Unfortunately I no longer have free access to Azure or Hyper-V to
test this so please do more complete testing.


Stephen Hemminger (2):
  net/netvsc: fix RNDIS command concurrency issue
  net/netvsc: fix link status RNDIS command concurrency issue

 drivers/net/netvsc/hn_ethdev.c |  13 ++-
 drivers/net/netvsc/hn_rndis.c  | 175 ++++++++++++++++++++-------------
 drivers/net/netvsc/hn_rndis.h  |   2 -
 drivers/net/netvsc/hn_var.h    |   4 +
 4 files changed, 114 insertions(+), 80 deletions(-)

-- 
2.51.0

[RFT 1/2] net/netvsc: fix RNDIS command concurrency issue

[Stephen Hemminger]

When multiple threads issue RNDIS command requests (such as device
info queries) simultaneously, command failures can occur due to
concurrent access to shared resources in the RNDIS execution path.
The RNDIS layer only supports one pending request at a time, and
hn_rndis_exec1() returns -EBUSY if a request is already in progress.

Functions like hn_rndis_get_offload(), hn_rndis_conf_offload(), and
hn_rndis_get_ptypes() were querying hardware capabilities on each
call, which could race with other RNDIS commands from different
threads.

Fix this by querying the hardware offload capabilities once during
device attach in hn_rndis_attach() and caching the result in the
hn_data structure. All subsequent accesses use the cached value,
eliminating the race condition.

If the hwcaps query fails during attach, the device initialization
fails entirely. This matches the behavior of the Linux netvsc driver
(see rndis_netdev_set_hwcaps in rndis_filter.c).

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

Reported-by: Madhuker Mythri <madhukar.mythri@gmail.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/netvsc/hn_rndis.c | 110 ++++++++++++++++------------------
 drivers/net/netvsc/hn_var.h   |   4 ++
 2 files changed, 57 insertions(+), 57 deletions(-)

diff --git a/drivers/net/netvsc/hn_rndis.c b/drivers/net/netvsc/hn_rndis.c
index 7c54eebcef..ed053ee905 100644
--- a/drivers/net/netvsc/hn_rndis.c
+++ b/drivers/net/netvsc/hn_rndis.c
@@ -781,15 +781,9 @@ int hn_rndis_conf_offload(struct hn_data *hv,
 			  uint64_t tx_offloads, uint64_t rx_offloads)
 {
 	struct ndis_offload_params params;
-	struct ndis_offload hwcaps;
+	const struct ndis_offload *hwcaps = &hv->hwcaps;
 	int error;
 
-	error = hn_rndis_query_hwcaps(hv, &hwcaps);
-	if (error) {
-		PMD_DRV_LOG(ERR, "hwcaps query failed: %d", error);
-		return error;
-	}
-
 	/* NOTE: 0 means "no change" */
 	memset(&params, 0, sizeof(params));
 
@@ -803,25 +797,25 @@ int hn_rndis_conf_offload(struct hn_data *hv,
 	}
 
 	if (tx_offloads & RTE_ETH_TX_OFFLOAD_TCP_CKSUM) {
-		if (hwcaps.ndis_csum.ndis_ip4_txcsum & NDIS_TXCSUM_CAP_TCP4)
+		if (hwcaps->ndis_csum.ndis_ip4_txcsum & NDIS_TXCSUM_CAP_TCP4)
 			params.ndis_tcp4csum = NDIS_OFFLOAD_PARAM_TX;
 		else
 			goto unsupported;
 
-		if (hwcaps.ndis_csum.ndis_ip6_txcsum & NDIS_TXCSUM_CAP_TCP6)
+		if (hwcaps->ndis_csum.ndis_ip6_txcsum & NDIS_TXCSUM_CAP_TCP6)
 			params.ndis_tcp6csum = NDIS_OFFLOAD_PARAM_TX;
 		else
 			goto unsupported;
 	}
 
 	if (rx_offloads & RTE_ETH_RX_OFFLOAD_TCP_CKSUM) {
-		if ((hwcaps.ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_TCP4)
+		if ((hwcaps->ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_TCP4)
 		    == NDIS_RXCSUM_CAP_TCP4)
 			params.ndis_tcp4csum |= NDIS_OFFLOAD_PARAM_RX;
 		else
 			goto unsupported;
 
-		if ((hwcaps.ndis_csum.ndis_ip6_rxcsum & NDIS_RXCSUM_CAP_TCP6)
+		if ((hwcaps->ndis_csum.ndis_ip6_rxcsum & NDIS_RXCSUM_CAP_TCP6)
 		    == NDIS_RXCSUM_CAP_TCP6)
 			params.ndis_tcp6csum |= NDIS_OFFLOAD_PARAM_RX;
 		else
@@ -829,12 +823,12 @@ int hn_rndis_conf_offload(struct hn_data *hv,
 	}
 
 	if (tx_offloads & RTE_ETH_TX_OFFLOAD_UDP_CKSUM) {
-		if (hwcaps.ndis_csum.ndis_ip4_txcsum & NDIS_TXCSUM_CAP_UDP4)
+		if (hwcaps->ndis_csum.ndis_ip4_txcsum & NDIS_TXCSUM_CAP_UDP4)
 			params.ndis_udp4csum = NDIS_OFFLOAD_PARAM_TX;
 		else
 			goto unsupported;
 
-		if ((hwcaps.ndis_csum.ndis_ip6_txcsum & NDIS_TXCSUM_CAP_UDP6)
+		if ((hwcaps->ndis_csum.ndis_ip6_txcsum & NDIS_TXCSUM_CAP_UDP6)
 		    == NDIS_TXCSUM_CAP_UDP6)
 			params.ndis_udp6csum = NDIS_OFFLOAD_PARAM_TX;
 		else
@@ -842,38 +836,38 @@ int hn_rndis_conf_offload(struct hn_data *hv,
 	}
 
 	if (rx_offloads & RTE_ETH_TX_OFFLOAD_UDP_CKSUM) {
-		if (hwcaps.ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_UDP4)
+		if (hwcaps->ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_UDP4)
 			params.ndis_udp4csum |= NDIS_OFFLOAD_PARAM_RX;
 		else
 			goto unsupported;
 
-		if (hwcaps.ndis_csum.ndis_ip6_rxcsum & NDIS_RXCSUM_CAP_UDP6)
+		if (hwcaps->ndis_csum.ndis_ip6_rxcsum & NDIS_RXCSUM_CAP_UDP6)
 			params.ndis_udp6csum |= NDIS_OFFLOAD_PARAM_RX;
 		else
 			goto unsupported;
 	}
 
 	if (tx_offloads & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM) {
-		if ((hwcaps.ndis_csum.ndis_ip4_txcsum & NDIS_TXCSUM_CAP_IP4)
+		if ((hwcaps->ndis_csum.ndis_ip4_txcsum & NDIS_TXCSUM_CAP_IP4)
 		    == NDIS_TXCSUM_CAP_IP4)
 			params.ndis_ip4csum = NDIS_OFFLOAD_PARAM_TX;
 		else
 			goto unsupported;
 	}
 	if (rx_offloads & RTE_ETH_RX_OFFLOAD_IPV4_CKSUM) {
-		if (hwcaps.ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_IP4)
+		if (hwcaps->ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_IP4)
 			params.ndis_ip4csum |= NDIS_OFFLOAD_PARAM_RX;
 		else
 			goto unsupported;
 	}
 
 	if (tx_offloads & RTE_ETH_TX_OFFLOAD_TCP_TSO) {
-		if (hwcaps.ndis_lsov2.ndis_ip4_encap & NDIS_OFFLOAD_ENCAP_8023)
+		if (hwcaps->ndis_lsov2.ndis_ip4_encap & NDIS_OFFLOAD_ENCAP_8023)
 			params.ndis_lsov2_ip4 = NDIS_OFFLOAD_LSOV2_ON;
 		else
 			goto unsupported;
 
-		if ((hwcaps.ndis_lsov2.ndis_ip6_opts & HN_NDIS_LSOV2_CAP_IP6)
+		if ((hwcaps->ndis_lsov2.ndis_ip6_opts & HN_NDIS_LSOV2_CAP_IP6)
 		    == HN_NDIS_LSOV2_CAP_IP6)
 			params.ndis_lsov2_ip6 = NDIS_OFFLOAD_LSOV2_ON;
 		else
@@ -898,51 +892,42 @@ int hn_rndis_conf_offload(struct hn_data *hv,
 int hn_rndis_get_offload(struct hn_data *hv,
 			 struct rte_eth_dev_info *dev_info)
 {
-	struct ndis_offload hwcaps;
-	int error;
-
-	memset(&hwcaps, 0, sizeof(hwcaps));
-
-	error = hn_rndis_query_hwcaps(hv, &hwcaps);
-	if (error) {
-		PMD_DRV_LOG(ERR, "hwcaps query failed: %d", error);
-		return error;
-	}
+	const struct ndis_offload *hwcaps = &hv->hwcaps;
 
 	dev_info->tx_offload_capa = RTE_ETH_TX_OFFLOAD_MULTI_SEGS |
 				    RTE_ETH_TX_OFFLOAD_VLAN_INSERT;
 
-	if ((hwcaps.ndis_csum.ndis_ip4_txcsum & HN_NDIS_TXCSUM_CAP_IP4)
+	if ((hwcaps->ndis_csum.ndis_ip4_txcsum & HN_NDIS_TXCSUM_CAP_IP4)
 	    == HN_NDIS_TXCSUM_CAP_IP4)
 		dev_info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
 
-	if ((hwcaps.ndis_csum.ndis_ip4_txcsum & HN_NDIS_TXCSUM_CAP_TCP4)
+	if ((hwcaps->ndis_csum.ndis_ip4_txcsum & HN_NDIS_TXCSUM_CAP_TCP4)
 	    == HN_NDIS_TXCSUM_CAP_TCP4 &&
-	    (hwcaps.ndis_csum.ndis_ip6_txcsum & HN_NDIS_TXCSUM_CAP_TCP6)
+	    (hwcaps->ndis_csum.ndis_ip6_txcsum & HN_NDIS_TXCSUM_CAP_TCP6)
 	    == HN_NDIS_TXCSUM_CAP_TCP6)
 		dev_info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_TCP_CKSUM;
 
-	if ((hwcaps.ndis_csum.ndis_ip4_txcsum & NDIS_TXCSUM_CAP_UDP4) &&
-	    (hwcaps.ndis_csum.ndis_ip6_txcsum & NDIS_TXCSUM_CAP_UDP6))
+	if ((hwcaps->ndis_csum.ndis_ip4_txcsum & NDIS_TXCSUM_CAP_UDP4) &&
+	    (hwcaps->ndis_csum.ndis_ip6_txcsum & NDIS_TXCSUM_CAP_UDP6))
 		dev_info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_UDP_CKSUM;
 
-	if ((hwcaps.ndis_lsov2.ndis_ip4_encap & NDIS_OFFLOAD_ENCAP_8023) &&
-	    (hwcaps.ndis_lsov2.ndis_ip6_opts & HN_NDIS_LSOV2_CAP_IP6)
+	if ((hwcaps->ndis_lsov2.ndis_ip4_encap & NDIS_OFFLOAD_ENCAP_8023) &&
+	    (hwcaps->ndis_lsov2.ndis_ip6_opts & HN_NDIS_LSOV2_CAP_IP6)
 	    == HN_NDIS_LSOV2_CAP_IP6)
 		dev_info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
 
 	dev_info->rx_offload_capa = RTE_ETH_RX_OFFLOAD_VLAN_STRIP |
 				    RTE_ETH_RX_OFFLOAD_RSS_HASH;
 
-	if (hwcaps.ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_IP4)
+	if (hwcaps->ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_IP4)
 		dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
 
-	if ((hwcaps.ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_TCP4) &&
-	    (hwcaps.ndis_csum.ndis_ip6_rxcsum & NDIS_RXCSUM_CAP_TCP6))
+	if ((hwcaps->ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_TCP4) &&
+	    (hwcaps->ndis_csum.ndis_ip6_rxcsum & NDIS_RXCSUM_CAP_TCP6))
 		dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TCP_CKSUM;
 
-	if ((hwcaps.ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_UDP4) &&
-	    (hwcaps.ndis_csum.ndis_ip6_rxcsum & NDIS_RXCSUM_CAP_UDP6))
+	if ((hwcaps->ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_UDP4) &&
+	    (hwcaps->ndis_csum.ndis_ip6_rxcsum & NDIS_RXCSUM_CAP_UDP6))
 		dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_UDP_CKSUM;
 
 	return 0;
@@ -951,29 +936,20 @@ int hn_rndis_get_offload(struct hn_data *hv,
 uint32_t
 hn_rndis_get_ptypes(struct hn_data *hv)
 {
-	struct ndis_offload hwcaps;
+	const struct ndis_offload *hwcaps = &hv->hwcaps;
 	uint32_t ptypes;
-	int error;
-
-	memset(&hwcaps, 0, sizeof(hwcaps));
-
-	error = hn_rndis_query_hwcaps(hv, &hwcaps);
-	if (error) {
-		PMD_DRV_LOG(ERR, "hwcaps query failed: %d", error);
-		return RTE_PTYPE_L2_ETHER;
-	}
 
 	ptypes = RTE_PTYPE_L2_ETHER;
 
-	if (hwcaps.ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_IP4)
+	if (hwcaps->ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_IP4)
 		ptypes |= RTE_PTYPE_L3_IPV4;
 
-	if ((hwcaps.ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_TCP4) ||
-	    (hwcaps.ndis_csum.ndis_ip6_rxcsum & NDIS_RXCSUM_CAP_TCP6))
+	if ((hwcaps->ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_TCP4) ||
+	    (hwcaps->ndis_csum.ndis_ip6_rxcsum & NDIS_RXCSUM_CAP_TCP6))
 		ptypes |= RTE_PTYPE_L4_TCP;
 
-	if ((hwcaps.ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_UDP4) ||
-	    (hwcaps.ndis_csum.ndis_ip6_rxcsum & NDIS_RXCSUM_CAP_UDP6))
+	if ((hwcaps->ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_UDP4) ||
+	    (hwcaps->ndis_csum.ndis_ip6_rxcsum & NDIS_RXCSUM_CAP_UDP6))
 		ptypes |= RTE_PTYPE_L4_UDP;
 
 	return ptypes;
@@ -1136,8 +1112,28 @@ hn_rndis_get_linkspeed(struct hn_data *hv)
 int
 hn_rndis_attach(struct hn_data *hv)
 {
+	int error;
+
 	/* Initialize RNDIS. */
-	return hn_rndis_init(hv);
+	error = hn_rndis_init(hv);
+	if (error)
+		return error;
+
+	/*
+	 * Query and cache hardware offload capabilities.
+	 * This avoids the need to query hwcaps on each call to
+	 * hn_rndis_get_offload(), hn_rndis_conf_offload(), or
+	 * hn_rndis_get_ptypes(), which can cause failures if
+	 * multiple threads try to query simultaneously.
+	 */
+	memset(&hv->hwcaps, 0, sizeof(hv->hwcaps));
+	error = hn_rndis_query_hwcaps(hv, &hv->hwcaps);
+	if (error) {
+		PMD_DRV_LOG(ERR, "failed to query hwcaps: %d", error);
+		return error;
+	}
+
+	return 0;
 }
 
 void
diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
index 17c1d5d07b..ae7f41dc94 100644
--- a/drivers/net/netvsc/hn_var.h
+++ b/drivers/net/netvsc/hn_var.h
@@ -9,6 +9,8 @@
 #include <rte_eal_paging.h>
 #include <ethdev_driver.h>
 
+#include "ndis.h"
+
 /*
  * Tunable ethdev params
  */
@@ -151,6 +153,8 @@ struct hn_data {
 	uint16_t	num_queues;
 	uint64_t	rss_offloads;
 
+	struct ndis_offload	hwcaps;		/* Cached hardware offload caps */
+
 	rte_spinlock_t	chim_lock;
 	struct rte_mem_resource chim_res;	/* UIO resource for Tx */
 	struct rte_bitmap *chim_bmap;		/* Send buffer map */
-- 
2.51.0

[RFT 2/2] net/netvsc: fix link status RNDIS command concurrency issue

[Stephen Hemminger]

The hn_dev_link_update() function issues RNDIS queries for link status
and link speed on every call. Since this function can be called from
multiple threads concurrently, it triggers the same RNDIS command
concurrency issue where only one request can be pending at a time.

Remove the hn_rndis_get_linkstatus() and hn_rndis_get_linkspeed()
functions and modify hn_dev_link_update() to use the cached values
directly. The link status and speed are already:

1. Queried once during device attach in hn_rndis_attach()
2. Updated via RNDIS status indications (MEDIA_CONNECT/DISCONNECT
   and LINK_SPEED_CHANGE) handled in hn_rndis_link_status()

This matches the Linux netvsc driver behavior where link status is
queried once in rndis_filter_device_add() and then tracked via
status indications.

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

Reported-by: Madhuker Mythri <madhukar.mythri@gmail.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/netvsc/hn_ethdev.c | 13 ++++---
 drivers/net/netvsc/hn_rndis.c  | 65 ++++++++++++++++++++++++++--------
 drivers/net/netvsc/hn_rndis.h  |  2 --
 3 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index 6584819f4f..c4580d0bb3 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -246,16 +246,15 @@ hn_dev_link_update(struct rte_eth_dev *dev,
 {
 	struct hn_data *hv = dev->data->dev_private;
 	struct rte_eth_link link, old;
-	int error;
 
 	old = dev->data->dev_link;
 
-	error = hn_rndis_get_linkstatus(hv);
-	if (error)
-		return error;
-
-	hn_rndis_get_linkspeed(hv);
-
+	/*
+	 * Use cached link_status and link_speed which are set during
+	 * device attach and updated via RNDIS status indications
+	 * (MEDIA_CONNECT/DISCONNECT and LINK_SPEED_CHANGE).
+	 * This avoids RNDIS queries which are not thread-safe.
+	 */
 	link = (struct rte_eth_link) {
 		.link_duplex = RTE_ETH_LINK_FULL_DUPLEX,
 		.link_autoneg = RTE_ETH_LINK_SPEED_FIXED,
diff --git a/drivers/net/netvsc/hn_rndis.c b/drivers/net/netvsc/hn_rndis.c
index ed053ee905..c85953af34 100644
--- a/drivers/net/netvsc/hn_rndis.c
+++ b/drivers/net/netvsc/hn_rndis.c
@@ -299,6 +299,7 @@ static void hn_rndis_link_alarm(void *arg)
 void hn_rndis_link_status(struct rte_eth_dev *dev, const void *msg)
 {
 	const struct rndis_status_msg *indicate = msg;
+	struct hn_data *hv = dev->data->dev_private;
 
 	hn_rndis_dump(msg);
 
@@ -311,11 +312,41 @@ void hn_rndis_link_status(struct rte_eth_dev *dev, const void *msg)
 		break;
 
 	case RNDIS_STATUS_LINK_SPEED_CHANGE:
+		/*
+		 * Update link speed from the status buffer.
+		 * This matches the Linux netvsc driver behavior.
+		 */
+		if (indicate->stbuflen >= sizeof(uint32_t)) {
+			const uint8_t *p = (const uint8_t *)msg;
+			uint32_t offset = RNDIS_STBUFOFFSET_ABS(indicate->stbufoffset);
+
+			if (offset + sizeof(uint32_t) <= indicate->len) {
+				uint32_t speed;
+
+				memcpy(&speed, p + offset, sizeof(speed));
+				hv->link_speed = speed;
+				PMD_DRV_LOG(DEBUG, "link speed changed to %u",
+					    speed);
+			}
+		}
+		if (dev->data->dev_conf.intr_conf.lsc)
+			rte_eal_alarm_set(10, hn_rndis_link_alarm, dev);
+		break;
+
 	case RNDIS_STATUS_MEDIA_CONNECT:
+		hv->link_status = NDIS_MEDIA_STATE_CONNECTED;
+		PMD_DRV_LOG(INFO, "link connected");
+		if (dev->data->dev_conf.intr_conf.lsc)
+			rte_eal_alarm_set(10, hn_rndis_link_alarm, dev);
+		break;
+
 	case RNDIS_STATUS_MEDIA_DISCONNECT:
+		hv->link_status = NDIS_MEDIA_STATE_DISCONNECTED;
+		PMD_DRV_LOG(INFO, "link disconnected");
 		if (dev->data->dev_conf.intr_conf.lsc)
 			rte_eal_alarm_set(10, hn_rndis_link_alarm, dev);
 		break;
+
 	default:
 		PMD_DRV_LOG(NOTICE, "unknown RNDIS indication: %#x",
 			    indicate->status);
@@ -1095,20 +1126,6 @@ hn_rndis_get_mtu(struct hn_data *hv, uint32_t *mtu)
 			       mtu, sizeof(uint32_t));
 }
 
-int
-hn_rndis_get_linkstatus(struct hn_data *hv)
-{
-	return hn_rndis_query(hv, OID_GEN_MEDIA_CONNECT_STATUS, NULL, 0,
-			      &hv->link_status, sizeof(uint32_t));
-}
-
-int
-hn_rndis_get_linkspeed(struct hn_data *hv)
-{
-	return hn_rndis_query(hv, OID_GEN_LINK_SPEED, NULL, 0,
-			      &hv->link_speed, sizeof(uint32_t));
-}
-
 int
 hn_rndis_attach(struct hn_data *hv)
 {
@@ -1133,6 +1150,26 @@ hn_rndis_attach(struct hn_data *hv)
 		return error;
 	}
 
+	/*
+	 * Query initial link status and speed. These will be updated
+	 * via RNDIS status indications (MEDIA_CONNECT/DISCONNECT and
+	 * LINK_SPEED_CHANGE) rather than being re-queried each time.
+	 * This matches the Linux netvsc driver behavior.
+	 */
+	error = hn_rndis_query(hv, OID_GEN_MEDIA_CONNECT_STATUS, NULL, 0,
+			       &hv->link_status, sizeof(hv->link_status));
+	if (error) {
+		PMD_DRV_LOG(ERR, "failed to query link status: %d", error);
+		return error;
+	}
+
+	error = hn_rndis_query(hv, OID_GEN_LINK_SPEED, NULL, 0,
+			       &hv->link_speed, sizeof(hv->link_speed));
+	if (error) {
+		PMD_DRV_LOG(ERR, "failed to query link speed: %d", error);
+		return error;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/netvsc/hn_rndis.h b/drivers/net/netvsc/hn_rndis.h
index 7f40f6221d..26854f45a4 100644
--- a/drivers/net/netvsc/hn_rndis.h
+++ b/drivers/net/netvsc/hn_rndis.h
@@ -11,8 +11,6 @@ int	hn_rndis_attach(struct hn_data *hv);
 void	hn_rndis_detach(struct hn_data *hv);
 int	hn_rndis_get_eaddr(struct hn_data *hv, uint8_t *eaddr);
 int	hn_rndis_get_mtu(struct hn_data *hv, uint32_t *mtu);
-int	hn_rndis_get_linkstatus(struct hn_data *hv);
-int	hn_rndis_get_linkspeed(struct hn_data *hv);
 int	hn_rndis_set_rxfilter(struct hn_data *hv, uint32_t filter);
 void	hn_rndis_rx_ctrl(struct hn_data *hv, const void *data,
 			 int dlen);
-- 
2.51.0

RE: [EXTERNAL] [PATCH v2] net/netvsc: fix race condition in RNDIS command execution

[Long Li]

> From: Madhuker Mythri <madhukar.mythri@gmail.com>
> 
> When multiple threads issue RNDIS command requests (such as device info
> queries and link status checks) simultaneously, command failures can occur
> due to concurrent access to shared resources in the RNDIS execution path.
> 
> Add a spinlock to serialize RNDIS command execution and prevent data
> corruption.

This is not the correct approach as you are only protecting RNDIS_QUERY_CMPLT over RNDIS transactions. The call to hn_rndis_execute() can still fail due to other possible pending transactions over RNDIS.

Thanks,
Long

> 
> Fixes: 4e9c73e96e83 ("net/netvsc: add Hyper-V network device")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Madhuker Mythri <madhukar.mythri@gmail.com>
> ---
>  drivers/net/netvsc/hn_ethdev.c | 1 +
>  drivers/net/netvsc/hn_rndis.c  | 2 ++
>  drivers/net/netvsc/hn_var.h    | 1 +
>  3 files changed, 4 insertions(+)
> 
> diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
> index 6584819f4f..b525e287fa 100644
> --- a/drivers/net/netvsc/hn_ethdev.c
> +++ b/drivers/net/netvsc/hn_ethdev.c
> @@ -1310,6 +1310,7 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
>  	PMD_INIT_FUNC_TRACE();
> 
>  	rte_spinlock_init(&hv->hotadd_lock);
> +	rte_spinlock_init(&hv->cmd_lock);
>  	LIST_INIT(&hv->hotadd_list);
> 
>  	vmbus = container_of(device, struct rte_vmbus_device, device); diff --
> git a/drivers/net/netvsc/hn_rndis.c b/drivers/net/netvsc/hn_rndis.c index
> 7c54eebcef..8a0716df89 100644
> --- a/drivers/net/netvsc/hn_rndis.c
> +++ b/drivers/net/netvsc/hn_rndis.c
> @@ -500,8 +500,10 @@ hn_rndis_query(struct hn_data *hv, uint32_t oid,
>  	/* Input data immediately follows RNDIS query. */
>  	memcpy(req + 1, idata, idlen);
> 
> +	rte_spinlock_lock(&hv->cmd_lock);
>  	error = hn_rndis_execute(hv, rid, req, reqlen,
>  				 comp, comp_len, RNDIS_QUERY_CMPLT);
> +	rte_spinlock_unlock(&hv->cmd_lock);
> 
>  	if (error)
>  		goto done;
> diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h index
> 17c1d5d07b..66ed186c0a 100644
> --- a/drivers/net/netvsc/hn_var.h
> +++ b/drivers/net/netvsc/hn_var.h
> @@ -179,6 +179,7 @@ struct hn_data {
>  	struct vmbus_channel *channels[HN_MAX_CHANNELS];
> 
>  	rte_spinlock_t	hotadd_lock;
> +	rte_spinlock_t	cmd_lock;
>  	LIST_HEAD(hotadd_list, hv_hotadd_context) hotadd_list;  };
> 
> --
> 2.50.1 (Apple Git-155)

Re: [PATCH v2] net/netvsc: fix race condition in RNDIS command execution

[Stephen Hemminger]

On Tue, 13 Jan 2026 15:48:20 +0530
madhukar.mythri@gmail.com wrote:

> From: Madhuker Mythri <madhukar.mythri@gmail.com>
> 
> When multiple threads issue RNDIS command requests (such as device
> info queries and link status checks) simultaneously, command failures
> can occur due to concurrent access to shared resources in the RNDIS
> execution path.
> 
> Add a spinlock to serialize RNDIS command execution and prevent
> data corruption.
> 
> Fixes: 4e9c73e96e83 ("net/netvsc: add Hyper-V network device")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Madhuker Mythri <madhukar.mythri@gmail.com>
> ---

As Long (and AI review) observed this patch needs more work.
There are two issues:

1. The introduced spin lock only protects query, it does not protect
around cases where a query races with some thing else calling rndis_set.
Lock should go inside hn_rndis_execute().

2. The lock is held while spinning, and it could take several
seconds to complete, holding spinlock that long burns CPU and any
other thread trying to do operation would queue up and block.

Maybe a pthread_mutex (which sleeps) or use trylock in the
query path and return EBUSY?