From: Sasha Levin <[email protected]>
Date: Sat, 25 Oct 2025 11:57:49 -0400

> From: Alexander Lobakin <[email protected]>
> 
> [ Upstream commit bd74a86bc75d35adefbebcec7c3a743d02c06230 ]
> 
> Add the missing linking of NAPIs to netdev queues when enabling
> interrupt vectors in order to support NAPI configuration and
> interfaces requiring get_rx_queue()->napi to be set (like XSk
> busy polling).
> 
> As currently, idpf_vport_{start,stop}() is called from several flows
> with inconsistent RTNL locking, we need to synchronize them to avoid
> runtime assertions. Notably:
> 
> * idpf_{open,stop}() -- regular NDOs, RTNL is always taken;
> * idpf_initiate_soft_reset() -- usually called under RTNL;
> * idpf_init_task -- called from the init work, needs RTNL;
> * idpf_vport_dealloc -- called without RTNL taken, needs it.
> 
> Expand common idpf_vport_{start,stop}() to take an additional bool
> telling whether we need to manually take the RTNL lock.
> 
> Suggested-by: Maciej Fijalkowski <[email protected]> # helper
> Signed-off-by: Alexander Lobakin <[email protected]>
> Tested-by: Ramu R <[email protected]>
> Signed-off-by: Tony Nguyen <[email protected]>
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> 
> LLM Generated explanations, may be completely bogus:
> 
> YES
> 
> Why this is a bug-fix
> - The driver was missing the association between netdev queues and their
>   NAPI instances. This breaks NAPI-aware configuration and features that
>   require queue->napi to be set, e.g., AF_XDP busy polling. The patch
>   adds the missing linkage and corresponding unlinkage, which is clearly
>   a functional fix rather than a feature.
> 
> What changed
> - Link/unlink netdev queues to the NAPI of each q_vector:
>   - Adds `idpf_q_vector_set_napi()` and uses it to associate both RX and
>     TX queues with the q_vector’s `napi`:
>     - Link on IRQ request:
>       drivers/net/ethernet/intel/idpf/idpf_txrx.c:4043
>     - Unlink on IRQ free:
>       drivers/net/ethernet/intel/idpf/idpf_txrx.c:3852
>   - Helper implementation:
>     drivers/net/ethernet/intel/idpf/idpf_txrx.c:3818
> 
> - Ensure correct locking for netif_queue_set_napi:
>   - `netif_queue_set_napi()` asserts RTNL or invisibility
>     (net/core/dev.c:7167), so the patch adds an `rtnl` parameter to the
>     vport bring-up/tear-down paths and acquires RTNL where it previously
>     wasn’t guaranteed:
>     - `idpf_vport_open(struct idpf_vport *vport, bool rtnl)` acquires
>       RTNL when `rtnl=true`
>       (drivers/net/ethernet/intel/idpf/idpf_lib.c:1397–1400), and
>       releases on both success and error paths (1528–1531).
>     - `idpf_vport_stop(struct idpf_vport *vport, bool rtnl)` does the
>       same for teardown (900–927).
>   - Callers updated according to their RTNL context, avoiding double-
>     lock or missing-lock situations:
>     - NDO stop: passes `false` (called under RTNL):
>       drivers/net/ethernet/intel/idpf/idpf_lib.c:951
>     - NDO open: passes `false` (called under RTNL):
>       drivers/net/ethernet/intel/idpf/idpf_lib.c:2275
>     - init work (not under RTNL): `idpf_init_task()` passes `true`:
>       drivers/net/ethernet/intel/idpf/idpf_lib.c:1607
>     - vport dealloc (not under RTNL): passes `true`:
>       drivers/net/ethernet/intel/idpf/idpf_lib.c:1044
>     - soft reset (usually under RTNL via ndo contexts): passes `false`:
>       drivers/net/ethernet/intel/idpf/idpf_lib.c:1997 and reopen at
>       2027, 2037
> 
> - Order of operations remains sane:
>   - Add NAPI and map vectors, then request IRQs, then link queues to
>     NAPI, then enable NAPI/IRQs
>     (drivers/net/ethernet/intel/idpf/idpf_txrx.c:4598–4607, 4043,
>     4619–4621).
>   - On teardown disable interrupts/NAPI, delete NAPI, unlink queues,
>     free IRQs (drivers/net/ethernet/intel/idpf/idpf_txrx.c:4119–4125,
>     3852).
> 
> Impact and risk
> - User-visible bug fixed: AF_XDP busy-polling and other NAPI-aware paths
>   can now retrieve the correct NAPI via get_rx_queue()->napi.
> - Change is tightly scoped to the idpf driver; no UAPI or architectural
>   changes.
> - Locking adjustments are minimal and consistent with net core
>   expectations for `netif_queue_set_napi()`.
> - Similar pattern exists in other drivers (e.g., ice, igb, igc) that use
>   `netif_queue_set_napi`, which supports the approach’s correctness.
> - Note: In the rare request_irq failure unwind, the code frees any
>   requested IRQs but doesn’t explicitly clear queue->napi for
>   previously-linked vectors; however, `napi_del()` runs and the
>   q_vector/napi storage remains valid, and normal teardown does clear
>   associations. This is a minor edge and does not outweigh the benefit
>   of the fix.
> 
> Stable backport suitability
> - Meets stable criteria: fixes a real functional bug, small and self-
>   contained, limited to a single driver, low regression risk, and
>   conforms to net core locking rules.
> - Dependency: requires `netif_queue_set_napi()` (present in this branch,
>   net/core/dev.c:7159). For older stable series lacking this API, a
>   backport would need equivalent infrastructure or adaptation.
> 
> Conclusion
> - This is a clear, necessary bug fix enabling expected NAPI-aware
>   behavior in idpf. It is safe and appropriate to backport.

While it's more of a feature and a prereq for XDP support in idpf, this
generated explanation is actually good and precise. I'm perfectly fine
with backporting this.

> 
>  drivers/net/ethernet/intel/idpf/idpf_lib.c  | 38 +++++++++++++++------
>  drivers/net/ethernet/intel/idpf/idpf_txrx.c | 17 +++++++++
>  2 files changed, 45 insertions(+), 10 deletions(-)
Thanks,
Olek

Reply via email to