[PATCH] net/netvsc: Fix on race condition of multiple commands

[PATCH] net/netvsc: Fix on race condition of multiple commands

[Madhuker Mythri]

When multiple processes issue command requests(like: device info get
and link-status) at same-time, then we could see the command request
failures, due to race-condition of common function execution.

So, to avoid such race-condition introduced a new spin-lock to protect
the code of critical section.

Fixes: 4e9c73e96e83 ("net/netvsc: add Hyper-V network device")
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: [EXTERNAL] [PATCH] net/netvsc: Fix on race condition of multiple commands

[Long Li]

> When multiple processes issue command requests(like: device info get and
> link-status) at same-time, then we could see the command request failures,
> due to race-condition of common function execution.

Hi Madhuker,

I'm not sure if we should use a lock in the driver for this. It's not clear in DPDK documents but in general the calls to query device status are not thread safe.

Is it possible that the application uses a lock to sync calling to this?

Thanks,
Long

> 
> So, to avoid such race-condition introduced a new spin-lock to protect the
> code of critical section.
> 
> Fixes: 4e9c73e96e83 ("net/netvsc: add Hyper-V network device")
> 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: [EXTERNAL] [PATCH] net/netvsc: Fix on race condition of multiple commands

[Stephen Hemminger]

On Fri, 19 Dec 2025 17:35:33 +0000
Long Li <longli@microsoft.com> wrote:

> > When multiple processes issue command requests(like: device info get and
> > link-status) at same-time, then we could see the command request failures,
> > due to race-condition of common function execution.  
> 
> Hi Madhuker,
> 
> I'm not sure if we should use a lock in the driver for this. It's not clear in DPDK documents but in general the calls to query device status are not thread safe.
> 
> Is it possible that the application uses a lock to sync calling to this?
> 

I do not know of any restrictions about threads calling query operations.

For info_get() the transaction is in rndis_get_offload().
There are couple of ways to handle this better. One would to do
the query during probe and remember the result. The hypervisor is
not going to change supported offload. The other and simpler way
would be to just have hardcoded offload values. The code for query
got compute offloads is inherited for BSD and unless someone was trying
to run on Windows 2012 or earlier version of Hyper-V it would never change.

Link status is a little more complex. Does the hyper-visor ever report
that the software path is down? And reading through the hn_rdis_exec code
it looks like if multiple operations are in process the second one
should return -EBUSY. Application could retry in that case.

Re: [EXTERNAL] [PATCH] net/netvsc: Fix on race condition of multiple commands

[madhukar mythri]

[-- Attachment #1: Type: text/plain, Size: 2204 bytes --]

Hi Li and Stephen,

We have a common DPDK application for all the PMD's, in which we are seeing
issue for this Netvsc PMD only.
I mean, for KVM hypervisor with Intel or Mellanox NICs we did not see such
sync issues. Also, with failsafe PMD on hyper-v did not seen such sync
issues.

So, i thought this would be better to fix at PMD level using spinlock.

@Stephen Hemminger <stephen@networkplumber.org> , yes we can store the
device info get details after probe and reuse it later.
For Link-status get with multiple threads we can go with retry mechanism.

However, w.r.t all other PMD's this device info get and Link-status get has
issues in multi threaded application.

Regards,
Madhuker.

On Sat, 20 Dec, 2025, 23:55 Stephen Hemminger, <stephen@networkplumber.org>
wrote:

> On Fri, 19 Dec 2025 17:35:33 +0000
> Long Li <longli@microsoft.com> wrote:
>
> > > When multiple processes issue command requests(like: device info get
> and
> > > link-status) at same-time, then we could see the command request
> failures,
> > > due to race-condition of common function execution.
> >
> > Hi Madhuker,
> >
> > I'm not sure if we should use a lock in the driver for this. It's not
> clear in DPDK documents but in general the calls to query device status are
> not thread safe.
> >
> > Is it possible that the application uses a lock to sync calling to this?
> >
>
> I do not know of any restrictions about threads calling query operations.
>
> For info_get() the transaction is in rndis_get_offload().
> There are couple of ways to handle this better. One would to do
> the query during probe and remember the result. The hypervisor is
> not going to change supported offload. The other and simpler way
> would be to just have hardcoded offload values. The code for query
> got compute offloads is inherited for BSD and unless someone was trying
> to run on Windows 2012 or earlier version of Hyper-V it would never change.
>
> Link status is a little more complex. Does the hyper-visor ever report
> that the software path is down? And reading through the hn_rdis_exec code
> it looks like if multiple operations are in process the second one
> should return -EBUSY. Application could retry in that case.
>

[-- Attachment #2: Type: text/html, Size: 3158 bytes --]

RE: [EXTERNAL] [PATCH] net/netvsc: Fix on race condition of multiple commands

[Long Li]

[-- Attachment #1: Type: text/plain, Size: 3315 bytes --]

Hi Madhukar,

I suggest caching the result of hn_rndis_query_hwcaps(), as suggested by Stephen. This can be done inside PMD. Do you want me to submit a patch?

For querying link status, the lock should be implemented inside hn_rndis_exec1(). The drawback is that this function can potentially wait for up to 60 seconds on response from host, maybe not suitable for spinlock in production use. But I think it's better to have application retry on BUSY (with some delay logic), as the netvsc is designed in this way since introduced.

Thanks,
Long



From: madhukar mythri <madhukar.mythri@gmail.com>
Sent: Saturday, December 20, 2025 9:37 PM
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: Long Li <longli@microsoft.com>; dev@dpdk.org; Madhuker Mythri <madhuker.mythri@oracle.com>
Subject: Re: [EXTERNAL] [PATCH] net/netvsc: Fix on race condition of multiple commands

You don't often get email from madhukar.mythri@gmail.com<mailto:madhukar.mythri@gmail.com>. Learn why this is important<https://aka.ms/LearnAboutSenderIdentification>
Hi Li and Stephen,

We have a common DPDK application for all the PMD's, in which we are seeing issue for this Netvsc PMD only.
I mean, for KVM hypervisor with Intel or Mellanox NICs we did not see such sync issues. Also, with failsafe PMD on hyper-v did not seen such sync issues.

So, i thought this would be better to fix at PMD level using spinlock.

@Stephen Hemminger<mailto:stephen@networkplumber.org> , yes we can store the device info get details after probe and reuse it later.
For Link-status get with multiple threads we can go with retry mechanism.

However, w.r.t all other PMD's this device info get and Link-status get has issues in multi threaded application.

Regards,
Madhuker.

On Sat, 20 Dec, 2025, 23:55 Stephen Hemminger, <stephen@networkplumber.org<mailto:stephen@networkplumber.org>> wrote:
On Fri, 19 Dec 2025 17:35:33 +0000
Long Li <longli@microsoft.com<mailto:longli@microsoft.com>> wrote:

> > When multiple processes issue command requests(like: device info get and
> > link-status) at same-time, then we could see the command request failures,
> > due to race-condition of common function execution.
>
> Hi Madhuker,
>
> I'm not sure if we should use a lock in the driver for this. It's not clear in DPDK documents but in general the calls to query device status are not thread safe.
>
> Is it possible that the application uses a lock to sync calling to this?
>

I do not know of any restrictions about threads calling query operations.

For info_get() the transaction is in rndis_get_offload().
There are couple of ways to handle this better. One would to do
the query during probe and remember the result. The hypervisor is
not going to change supported offload. The other and simpler way
would be to just have hardcoded offload values. The code for query
got compute offloads is inherited for BSD and unless someone was trying
to run on Windows 2012 or earlier version of Hyper-V it would never change.

Link status is a little more complex. Does the hyper-visor ever report
that the software path is down? And reading through the hn_rdis_exec code
it looks like if multiple operations are in process the second one
should return -EBUSY. Application could retry in that case.

[-- Attachment #2: Type: text/html, Size: 10998 bytes --]

Re: [EXTERNAL] [PATCH] net/netvsc: Fix on race condition of multiple commands

[madhukar mythri]

[-- Attachment #1: Type: text/plain, Size: 3756 bytes --]

Hi Long,

Yes, got it.
By caching the results of 'hn_rndis_query_hwcaps()' inside PMD will be
better.
Yes, you can submit the patch 'Or' let me know, if you are busy.

Thanks,
Madhukar.

On Thu, Jan 1, 2026 at 4:52 AM Long Li <longli@microsoft.com> wrote:

> Hi Madhukar,
>
>
>
> I suggest caching the result of hn_rndis_query_hwcaps(), as suggested by
> Stephen. This can be done inside PMD. Do you want me to submit a patch?
>
>
>
> For querying link status, the lock should be implemented inside
> hn_rndis_exec1(). The drawback is that this function can potentially wait
> for up to 60 seconds on response from host, maybe not suitable for spinlock
> in production use. But I think it’s better to have application retry on
> BUSY (with some delay logic), as the netvsc is designed in this way since
> introduced.
>
>
>
> Thanks,
>
> Long
>
>
>
>
>
>
>
> *From:* madhukar mythri <madhukar.mythri@gmail.com>
> *Sent:* Saturday, December 20, 2025 9:37 PM
> *To:* Stephen Hemminger <stephen@networkplumber.org>
> *Cc:* Long Li <longli@microsoft.com>; dev@dpdk.org; Madhuker Mythri <
> madhuker.mythri@oracle.com>
> *Subject:* Re: [EXTERNAL] [PATCH] net/netvsc: Fix on race condition of
> multiple commands
>
>
>
> You don't often get email from madhukar.mythri@gmail.com. Learn why this
> is important <https://aka.ms/LearnAboutSenderIdentification>
>
> Hi Li and Stephen,
>
>
>
> We have a common DPDK application for all the PMD's, in which we are
> seeing issue for this Netvsc PMD only.
>
> I mean, for KVM hypervisor with Intel or Mellanox NICs we did not see such
> sync issues. Also, with failsafe PMD on hyper-v did not seen such sync
> issues.
>
>
>
> So, i thought this would be better to fix at PMD level using spinlock.
>
>
>
> @Stephen Hemminger <stephen@networkplumber.org> , yes we can store the
> device info get details after probe and reuse it later.
>
> For Link-status get with multiple threads we can go with retry mechanism.
>
>
>
> However, w.r.t all other PMD's this device info get and Link-status get
> has issues in multi threaded application.
>
>
>
> Regards,
>
> Madhuker.
>
>
>
> On Sat, 20 Dec, 2025, 23:55 Stephen Hemminger, <stephen@networkplumber.org>
> wrote:
>
> On Fri, 19 Dec 2025 17:35:33 +0000
> Long Li <longli@microsoft.com> wrote:
>
> > > When multiple processes issue command requests(like: device info get
> and
> > > link-status) at same-time, then we could see the command request
> failures,
> > > due to race-condition of common function execution.
> >
> > Hi Madhuker,
> >
> > I'm not sure if we should use a lock in the driver for this. It's not
> clear in DPDK documents but in general the calls to query device status are
> not thread safe.
> >
> > Is it possible that the application uses a lock to sync calling to this?
> >
>
> I do not know of any restrictions about threads calling query operations.
>
> For info_get() the transaction is in rndis_get_offload().
> There are couple of ways to handle this better. One would to do
> the query during probe and remember the result. The hypervisor is
> not going to change supported offload. The other and simpler way
> would be to just have hardcoded offload values. The code for query
> got compute offloads is inherited for BSD and unless someone was trying
> to run on Windows 2012 or earlier version of Hyper-V it would never change.
>
> Link status is a little more complex. Does the hyper-visor ever report
> that the software path is down? And reading through the hn_rdis_exec code
> it looks like if multiple operations are in process the second one
> should return -EBUSY. Application could retry in that case.
>
>

[-- Attachment #2: Type: text/html, Size: 8765 bytes --]

Re: [PATCH] net/netvsc: Fix on race condition of multiple commands

[Stephen Hemminger]

On Fri, 12 Dec 2025 17:22:38 +0530
Madhuker Mythri <madhukar.mythri@gmail.com> wrote:

> When multiple processes issue command requests(like: device info get
> and link-status) at same-time, then we could see the command request
> failures, due to race-condition of common function execution.
> 
> So, to avoid such race-condition introduced a new spin-lock to protect
> the code of critical section.
> 
> Fixes: 4e9c73e96e83 ("net/netvsc: add Hyper-V network device")
> Signed-off-by: Madhuker Mythri <madhukar.mythri@gmail.com>

AI patch review spotted some minor things and raises vaild question
about overall races in hn_rndis_exec

## Patch Review: net/netvsc Race Condition Fix

Based on the AGENTS.md DPDK review criteria, here's my assessment:

---

### **ERRORS (Must Fix)**

1. **Subject line capitalization** — "Fix" should be lowercase "fix"
   - Current: `net/netvsc: Fix on race condition of multiple commands`
   - Should be: `net/netvsc: fix race condition in multiple commands`

2. **Missing blank line before Signed-off-by** — Per tag order requirements, there must be a blank line between the `Fixes:` block and the `Signed-off-by:` block:
   ```
   Fixes: 4e9c73e96e83 ("net/netvsc: add Hyper-V network device")

   Signed-off-by: Madhuker Mythri <madhukar.mythri@gmail.com>
   ```

---

### **WARNINGS (Should Fix)**

1. **Awkward grammar in subject** — "Fix on race condition" is grammatically incorrect. Should be "fix race condition" (removing "on").

2. **Missing `Cc: stable@dpdk.org`** — This is a bug fix with a `Fixes:` tag, so it's a candidate for stable backport.

3. **Body formatting nit** — Line 89 has `requests(like:` — missing space before the parenthesis. Should be `requests (like:`.

4. **Incomplete protection?** — The spinlock only wraps `hn_rndis_execute()` inside `hn_rndis_query()`, but `hn_rndis_execute()` is likely called from other paths (e.g., `hn_rndis_set()`). If the race condition is in `hn_rndis_execute()` itself, all callers should be protected — or the lock should be inside `hn_rndis_execute()` rather than in `hn_rndis_query()`.

---

### **INFO (Minor/Consider)**

1. **Commit body could be clearer** — Consider specifying which exact functions race (e.g., "concurrent calls to `hn_rndis_query()` from different control paths can corrupt the shared response buffer").

2. **Lock granularity** — Holding a spinlock across `hn_rndis_execute()` which appears to wait for completion could block other threads for extended periods. A mutex might be more appropriate if this isn't a fast-path operation.

---

### **What's Correct** ✓

- Subject length (53 chars ≤ 60)
- Correct component prefix (`net/netvsc:`)
- Imperative mood
- No trailing period
- Body lines within 75 characters
- Body doesn't start with "It"
- `Signed-off-by:` present with real name and email
- `Fixes:` tag present with 12-char SHA and quoted subject
- Code uses proper DPDK APIs (`rte_spinlock_*`)
- Code changes are minimal and focused

---

### **Suggested Revised Commit Message**

```
net/netvsc: fix race condition in RNDIS command execution

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

---

### **Technical Question for Author**

Is `hn_rndis_query()` the only caller of `hn_rndis_execute()` that can race, or should `hn_rndis_set()` and other callers also be protected? If the race is in the execute function itself, moving the lock inside `hn_rndis_execute()` would provide complete protection.

[PATCH] net/netvsc: Fix on race condition of multiple commands

[Madhuker Mythri]

When multiple processes issue command requests(like: device info get
and link-status) at same-time, then we could see the command request
failures, due to race-condition of common function execution.

So, to avoid such race-condition introduced a new spin-lock to protect
the code of critical section.

Fixes: 4e9c73e ("net/netvsc: add Hyper-V network device")
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] net/netvsc: Fix on race condition of multiple commands

[Patrick Robb]

[-- Attachment #1: Type: text/plain, Size: 103 bytes --]

Recheck-request: iol-intel-Performance

I am putting in a CI retest - no action is needed on your end.

[-- Attachment #2: Type: text/html, Size: 150 bytes --]