> -----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://lore.kernel.org/intel-wired-lan/20251212131454.124116-1-
> [email protected]/
> v2: https://lore.kernel.org/intel-wired-lan/20251215115416.410619-1-
> [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.
> + 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)) ?
> + return -ENXIO;
> + q_vector = ring->q_vector;
...
> --
> 2.34.1
