[Intel-wired-lan] [PATCH AUTOSEL 6.17] idpf: link NAPIs to queues

[Sasha Levin]

On Mon, Oct 27, 2025 at 04:19:24PM +0100, Alexander Lobakin wrote:
>From: Sasha Levin <sashal at kernel.org>
>Date: Sat, 25 Oct 2025 11:57:49 -0400
>
>> From: Alexander Lobakin <aleksander.lobakin at intel.com>
>>
>> [ 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 <maciej.fijalkowski at intel.com> # helper
>> Signed-off-by: Alexander Lobakin <aleksander.lobakin at intel.com>
>> Tested-by: Ramu R <ramu.r at intel.com>
>> Signed-off-by: Tony Nguyen <anthony.l.nguyen at intel.com>
>> Signed-off-by: Sasha Levin <sashal at kernel.org>
>> ---
>>
>> 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.

Thanks for the review and feedback!

-- 
Thanks,
Sasha