<div dir="ltr">Thanks for the comments.<div> <div>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()).</div><div>So, to make it simple, just use the spinlocks for the queries only.</div><div> </div><div>Regards,</div></div><div>Madhuker.</div></div> <div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Mon, Jan 12, 2026 at 8:28 AM Stephen Hemminger <<a href="mailto:stephen@networkplumber.org">stephen@networkplumber.org</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, 12 Dec 2025 17:22:38 +0530 Madhuker Mythri <<a href="mailto:madhukar.mythri@gmail.com" target="_blank">madhukar.mythri@gmail.com</a>> 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 <<a href="mailto:madhukar.mythri@gmail.com" target="_blank">madhukar.mythri@gmail.com</a>> 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 <<a href="mailto:madhukar.mythri@gmail.com" target="_blank">madhukar.mythri@gmail.com</a>> ``` --- ### **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: <a href="mailto:stable@dpdk.org" target="_blank">stable@dpdk.org</a>`** — 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: <a href="mailto:stable@dpdk.org" target="_blank">stable@dpdk.org</a> Signed-off-by: Madhuker Mythri <<a href="mailto:madhukar.mythri@gmail.com" target="_blank">madhukar.mythri@gmail.com</a>> ``` --- ### **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. </blockquote></div>