> -----Original Message-----
> From: Behera, VIVEK <[email protected]>
> Sent: Monday, December 22, 2025 9:21 AM
> To: Loktionov, Aleksandr <[email protected]>; Keller,
> Jacob E <[email protected]>; Nguyen, Anthony L
> <[email protected]>; Kitszel, Przemyslaw
> <[email protected]>; Fijalkowski, Maciej
> <[email protected]>; [email protected];
> [email protected]
> Cc: [email protected]
> Subject: AW: [PATCH iwl-net v3] igb: Fix trigger of incorrect irq in
> igb_xsk_wakeup
>
>
>
> > -----Ursprüngliche Nachricht-----
> > Von: Loktionov, Aleksandr <[email protected]>
> > Gesendet: Montag, 22. Dezember 2025 08:31
> > An: Behera, Vivek (DI FA DSP ICC PRC1) <[email protected]>;
> > Keller, Jacob E <[email protected]>; Nguyen, Anthony L
> > <[email protected]>; Kitszel, Przemyslaw
> > <[email protected]>; Fijalkowski, Maciej
> > <[email protected]>; [email protected];
> > [email protected]
> > Cc: [email protected]; Behera, Vivek (DI FA DSP ICC
> > PRC1) <[email protected]>
> > Betreff: RE: [PATCH iwl-net v3] igb: Fix trigger of incorrect irq in
> > igb_xsk_wakeup
> >
> >
> >
> > > -----Original Message-----
> > > From: Vivek Behera <[email protected]>
> > > Sent: Saturday, December 20, 2025 12:50 PM
> > > To: Loktionov, Aleksandr <[email protected]>; Keller,
> > > Jacob E <[email protected]>; Nguyen, Anthony L
> > > <[email protected]>; Kitszel, Przemyslaw
> > > <[email protected]>; Fijalkowski, Maciej
> > > <[email protected]>; [email protected];
> > > [email protected]
> > > Cc: [email protected]; Behera, Vivek
> > > <[email protected]>
> > > Subject: [PATCH iwl-net v3] igb: Fix trigger of incorrect irq in
> > > igb_xsk_wakeup
> > >
> > > 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]>
> > >
> > > ---
> > > v1:
> > >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > > re
> > > .kernel.org%2Fintel-wired-lan%2F20251212131454.124116-1-
> &data=05%7C0
> > > 2%
> > >
> >
> 7Cvivek.behera%40siemens.com%7C164e180ed5e443da63c408de412c1b1f%7C38a
> > e
> > >
> > 3bcd95794fd4addab42e1495d55a%7C1%7C0%7C639019854838314101%7CUnknow
> > n%7C
> > >
> > TWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4
> > zMi
> > >
> > IsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=95%2FeVnmg9
> > JiwCd
> > > oGd3sd441VWbvXi%2FBYFTtbW9ddXfA%3D&reserved=0
> > > [email protected]/
> > > v2:
> > >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > > re
> > > .kernel.org%2Fintel-wired-lan%2F20251215115416.410619-1-
> &data=05%7C0
> > > 2%
> > >
> >
> 7Cvivek.behera%40siemens.com%7C164e180ed5e443da63c408de412c1b1f%7C38a
> > e
> > >
> > 3bcd95794fd4addab42e1495d55a%7C1%7C0%7C639019854838342225%7CUnknow
> > n%7C
> > >
> > TWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4
> > zMi
> > >
> > IsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=niEnO3XufusM
> > bVGs
> > > l6QO0mj3yWC1Zr3dl5azIYRqs24%3D&reserved=0
> > > [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 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
> > > ---
> > > .../net/ethernet/intel/igb/e1000_defines.h | 1 +
> > > drivers/net/ethernet/intel/igb/igb_xsk.c | 88
> +++++++++++++++++-
> > > -
> > > 2 files changed, 81 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h
> > > b/drivers/net/ethernet/intel/igb/e1000_defines.h
> > > index fa028928482f..9357564a2d58 100644
> > > --- a/drivers/net/ethernet/intel/igb/e1000_defines.h
> > > +++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
> > > @@ -443,6 +443,7 @@
> > > #define E1000_ICS_LSC E1000_ICR_LSC /* Link Status
> Change
> > > */
> > > #define E1000_ICS_RXDMT0 E1000_ICR_RXDMT0 /* rx desc min.
> > > threshold */
> > > #define E1000_ICS_DRSTA E1000_ICR_DRSTA /* Device Reset
> > > Aserted */
> > > +#define E1000_ICS_TXDW E1000_ICR_TXDW /* Transmit desc
> > > written back */
> > >
> > > /* Extended Interrupt Cause Set */
> > > /* E1000_EITR_CNT_IGNR is only for 82576 and newer */ diff --git
> > > a/drivers/net/ethernet/intel/igb/igb_xsk.c
> > > b/drivers/net/ethernet/intel/igb/igb_xsk.c
> > > index 30ce5fbb5b77..3dbc2573b47a 100644
> > > --- a/drivers/net/ethernet/intel/igb/igb_xsk.c
> > > +++ b/drivers/net/ethernet/intel/igb/igb_xsk.c
> > > @@ -529,6 +529,7 @@ int igb_xsk_wakeup(struct net_device *dev, u32
> > > qid, u32 flags)
> > > struct igb_adapter *adapter = netdev_priv(dev);
> > > struct e1000_hw *hw = &adapter->hw;
> > > struct igb_ring *ring;
> > > + struct igb_q_vector *q_vector;
> > > u32 eics = 0;
> > >
> > > if (test_bit(__IGB_DOWN, &adapter->state)) @@ -536,14 +537,80
> @@
> > > int igb_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags)
> > >
> > > if (!igb_xdp_is_enabled(adapter))
> > > return -EINVAL;
> > > -
> > > - if (qid >= adapter->num_tx_queues)
> > > + /* Check if queue_id is valid. Tx and Rx queue numbers are
> > > always same */
> > > + if (qid >= adapter->num_rx_queues)
> > > return -EINVAL;
> > >
> > > - ring = adapter->tx_ring[qid];
> > > -
> > > - if (test_bit(IGB_RING_FLAG_TX_DISABLED, &ring->flags))
> > > - return -ENETDOWN;
> > > + if ((flags & XDP_WAKEUP_RX) && (flags & XDP_WAKEUP_TX)) {
> > > + /* If both TX and RX need to be woken up check if queue
> > > pairs are active */
> > > + if (adapter->flags & IGB_FLAG_QUEUE_PAIRS) {
> > > + /* Extract ring params from Rx */
> > > + ring = adapter->rx_ring[qid];
> > > + } else {
> > > + /* Two irqs for Rx AND Tx need to be triggered */
> > > + struct napi_struct *rx_napi;
> > > + struct napi_struct *tx_napi;
> > > + bool trigger_irq_tx = false;
> > > + bool trigger_irq_rx = false;
> > > + u32 eics_tx = 0;
> > > + u32 eics_rx = 0;
> > > + /* IRQ trigger preparation for Rx */
> > > + ring = adapter->rx_ring[qid];
> > > + if (!ring->xsk_pool)
> > > + return -ENXIO;
> > When IGB_FLAG_QUEUE_PAIRS is set, the code sets ring =
> > adapter->rx_ring[qid] and then falls through to the common path (if
> (!READ_ONCE(ring->xsk_pool))).
> > However, the common path only handles a single NAPI and single IRQ
> trigger.
> > In queue-pair mode, RX and TX share the same q_vector/IRQ, so
> > triggering only one direction might not fully satisfy the
> > XDP_WAKEUP_RX | XDP_WAKEUP_TX contract.
> > The logic appears incomplete for the queue-pair case when both flags
> are set.
>
> Okay. According to my understanding when the queue pairs are activated
> one q_vector is used for the rx and tx. And this napi instance is
> saved in the common q_vector. Below is the example of napi threads
> with queue pairs enabled for the 4 rx-tx queue pairs ps aux | grep
> eno2
> root 561 0.0 0.0 0 0 ? S 07:44 0:00
> [irq/162-eno2]
> root 565 0.0 0.0 0 0 ? S 07:44 0:00
> [irq/163-eno2-TxRx-0]
> root 566 0.0 0.0 0 0 ? S 07:44 0:00
> [irq/164-eno2-TxRx-1]
> root 567 0.0 0.0 0 0 ? S 07:44 0:00
> [irq/165-eno2-TxRx-2]
> root 568 0.0 0.0 0 0 ? S 07:44 0:00
> [irq/166-eno2-TxRx-3]
> root 1247 0.0 0.0 0 0 ? S 09:08 0:00
> [napi/eno2-8197]
> root 1248 0.0 0.0 0 0 ? S 09:08 0:00
> [napi/eno2-8196]
> root 1249 0.0 0.0 0 0 ? S 09:08 0:00
> [napi/eno2-8195]
> root 1250 0.0 0.0 0 0 ? S 09:08 0:00
> [napi/eno2-8194]
>
> So in my understanding a single q_vector and a single napi is all that
> is needed to trigger both the rx and tx. Essentially with the queue
> pairs enabled irrespective of the flags we will end up triggering a
> single interrupt for the associated queue pair and have the common
> napi to deal with.
>
> Is this not correct?
You are right.
The /proc/interrupts output confirms this with the eno2-TxRx-N naming pattern
showing combined IRQ handling.
Perhaps adding a comment in the code would make this clearer?
/* In queue-pair mode, rx_ring and tx_ring share the same q_vector,
* so a single IRQ trigger will wake both RX and TX processing
*/
With the best regards
Reviewed-by: Aleksandr Loktionov <[email protected]>
> >
> > > + q_vector = ring->q_vector;
> > > + rx_napi = &q_vector->napi;
> > > + /* Extend the BIT mask for eics */
> > > + eics_rx = ring->q_vector->eims_value;
> > > +
> > > + /* IRQ trigger preparation for Tx */
> > > + ring = adapter->tx_ring[qid];
> > > + if (test_bit(IGB_RING_FLAG_TX_DISABLED, &ring-
> > > >flags))
> > > + return -ENETDOWN;
> > > +
> > > + if (!ring->xsk_pool)
> > Shouldn't it be: !READ_ONCE(ring->xsk_pool)) ?
> Yes. indeed
> >
> > > + return -ENXIO;
> > > + q_vector = ring->q_vector;
> >
> > ...
> >
> > > --
> > > 2.34.1
