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

[madhukar mythri]

Thanks for the comments.

This issue occurs only with the query functions (like: device info get and
link-status) and not at the time of initialization(hn_rndis_set()).
So, to make it simple, just use the spinlocks for the queries only.

Regards,
Madhuker.

On Mon, Jan 12, 2026 at 8:28 AM Stephen Hemminger <
stephen at networkplumber.org> wrote:

> On Fri, 12 Dec 2025 17:22:38 +0530
> Madhuker Mythri <madhukar.mythri at 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 at 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 at 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 at 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 at dpdk.org
>
> Signed-off-by: Madhuker Mythri <madhukar.mythri at 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.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mails.dpdk.org/archives/dev/attachments/20260113/f7081582/attachment-0001.htm>