> -----Original Message-----
> From: Fijalkowski, Maciej <[email protected]>
> Sent: Friday, January 9, 2026 1:15 AM
> To: Behera, Vivek <[email protected]>
> Cc: Loktionov, Aleksandr <[email protected]>; Keller,
> Jacob E <[email protected]>; Nguyen, Anthony L
> <[email protected]>; Kitszel, Przemyslaw
> <[email protected]>; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH iwl-net v4] igb: Fix trigger of incorrect irq in
> igb_xsk_wakeup
> 
> On Mon, Dec 22, 2025 at 12:57:47PM +0100, Vivek Behera wrote:
> > The current implementation in the igb_xsk_wakeup expects the Rx and
> Tx
> > queues to share the same irq. This would lead to triggering of
> > incorrect irq in split irq configuration.
> > This patch addresses this issue which could impact environments with
> 2
> > active cpu cores or when the number of queues is reduced to 2 or
> less
> >
> > cat /proc/interrupts | grep eno2
> >  167:          0          0          0          0 IR-PCI-MSIX-
> 0000:08:00.0
> >  0-edge      eno2
> >  168:          0          0          0          0 IR-PCI-MSIX-
> 0000:08:00.0
> >  1-edge      eno2-rx-0
> >  169:          0          0          0          0 IR-PCI-MSIX-
> 0000:08:00.0
> >  2-edge      eno2-rx-1
> >  170:          0          0          0          0 IR-PCI-MSIX-
> 0000:08:00.0
> >  3-edge      eno2-tx-0
> >  171:          0          0          0          0 IR-PCI-MSIX-
> 0000:08:00.0
> >  4-edge      eno2-tx-1
> >
> > Furthermore it uses the flags input argument to trigger either rx,
> tx
> > or both rx and tx irqs as specified in the ndo_xsk_wakeup api
> > documentation
> >
> > Fixes: 80f6ccf9f116 ("igb: Introduce XSK data structures and
> helpers")
> > Signed-off-by: Vivek Behera <[email protected]>
> > Reviewed-by: Aleksandr Loktionov <[email protected]>
> > ---
> > v1:
> > https://lore.kernel.org/intel-wired-lan/20251212131454.124116-1-
> vivek.
> > [email protected]/
> > v2:
> > https://lore.kernel.org/intel-wired-lan/20251215115416.410619-1-
> vivek.
> > [email protected]/
> > v3:
> > https://lore.kernel.org/intel-wired-lan/20251220114936.140473-1-
> vivek.
> > [email protected]/
> >
> > changelog:
> > v1
> > - Inital description of the Bug and fixes made in the patch
> >
> > v1 -> v2
> > - Handling of RX and TX Wakeup in igc_xsk_wakeup for a split IRQ
> > configuration
> > - Review suggestions by Aleksander: Modified sequence to complete
> all
> >   error checks for rx and tx before updating napi states and
> > triggering irqs
> > - Corrected trigger of TX and RX interrupts over E1000_ICS (non msix
> > use case)
> > - Added define for Tx interrupt trigger bit mask for E1000_ICS
> >
> > v2 -> v3
> > - Included applicable feedback and suggestions from igc patch
> > - Fixed logic in updating eics value when  both TX and RX need
> wakeup
> >
> > v3 -> v4
> > - Added comments to explain trigerring of both TX and RX with active
> > queue pairs
> > - Fixed check of xsk pools in if statement
> > ---
> >  .../net/ethernet/intel/igb/e1000_defines.h    |  1 +
> >  drivers/net/ethernet/intel/igb/igb_xsk.c      | 90
> +++++++++++++++++--
> >  2 files changed, 83 insertions(+), 8 deletions(-)
> >

...

> > +   } else {
> > +           /* Invalid Flags */
> > +           return -EINVAL;
> > +   }
> 
> This is too complicated IMHO. Wouldn't something like this work:
> - if wakeup rx, pick napi from rx ring's q_vector
>   * napi_if_scheduled_mark_missed() logic that exists in
> igc_xsk_wakeup()
> 
> repeat for tx; if IGB_FLAG_QUEUE_PAIRS then the branch of second
> napi_if_scheduled_mark_missed() call would not be executed as we had
> previously marked the missed bit in napi state;
> 

I agree, it's too complicated.

...

> > --
> > 2.34.1
> >

Reply via email to