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 < [email protected]> wrote: > On Fri, 12 Dec 2025 17:22:38 +0530 > Madhuker Mythri <[email protected]> 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 <[email protected]> > > 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 <[email protected]> > ``` > > --- > > ### **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: [email protected]`** — 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: [email protected] > > Signed-off-by: Madhuker Mythri <[email protected]> > ``` > > --- > > ### **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. >
