> -----Original Message-----
> From: Behera, Vivek (DI FA DSP ICC PRC1) <[email protected]>
> Sent: Monday, December 15, 2025 12:54 PM
> To: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> Cc: [email protected]; Behera, Vivek (DI FA DSP ICC PRC1)
> <[email protected]>
> Subject: [PATCH iwl-net v2] 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]>
> Reviewed-by: Aleksandr loktinov <[email protected]>
> ---
> v1:
> https://lore.kernel.o/
> rg%2Fintel-wired-lan%2F20251212131454.124116-1-
> vivek.behera%40siemens.com%2F&data=05%7C02%7Cvivek.behera%40siemens
> .com%7C1199ec6c63494235f90408de3bd0eefa%7C38ae3bcd95794fd4addab42e1
> 495d55a%7C1%7C0%7C639013965726377756%7CUnknown%7CTWFpbGZsb3d8
> eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoi
> TWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=uTarFq1Uj3h0H1aZeN
> 06HWf0ts3BsMJkfPq9pPBegrI%3D&reserved=0
>
> 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
> ---
>  .../net/ethernet/intel/igb/e1000_defines.h    |  1 +
>  drivers/net/ethernet/intel/igb/igb_xsk.c      | 98 +++++++++++++++++--
>  2 files changed, 92 insertions(+), 7 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..d146ab629ef0 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)) @@ -537,13 +538,91 @@
> 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)
> +     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. */
> +                     if (qid >= adapter->num_rx_queues)
> +                             return -EINVAL;
> +                     ring = adapter->rx_ring[qid];
> +             } else {
> +                     /* Two irqs for Rx AND Tx need to be triggered */
> +                     u32 eics_tx = 0;
> +                     u32 eics_rx = 0;
> +                     struct napi_struct *rx_napi;
> +                     struct napi_struct *tx_napi;
> +                     bool trigger_irq_tx = false;
> +                     bool trigger_irq_rx = false;
> +
> +                     if (qid >= adapter->num_rx_queues)
> +                             return -EINVAL;
> +
> +                     if (qid >= adapter->num_tx_queues)
> +                             return -EINVAL;
> +
> +                     /* IRQ trigger preparation for Rx */
> +                     ring = adapter->rx_ring[qid];
> +                     if (!ring->xsk_pool)
> +                             return -ENXIO;
> +                     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)
> +                             return -ENXIO;
> +                     q_vector = ring->q_vector;
> +                     tx_napi  = &q_vector->napi;
> +                     /* Extend the BIT mask for eics */
> +                     eics_tx |= ring->q_vector->eims_value;
> +
> +                     /* Check and update napi states for rx and tx */
> +                     if (!napi_if_scheduled_mark_missed(rx_napi)) {
> +                             trigger_irq_rx = true;
> +                             eics |= eics_rx;
> +                     }
> +                     if (!napi_if_scheduled_mark_missed(tx_napi)) {
> +                             trigger_irq_tx = true;
> +                             eics |= eics_tx;
> +                     }
> +                     /* Now we trigger the required irqs for Rx and Tx */
> +                     if ((trigger_irq_rx) || (trigger_irq_tx)) {
> +                             if (adapter->flags & IGB_FLAG_HAS_MSIX) {
> +                                     eics |= ring->q_vector->eims_value;
> +                                     wr32(E1000_EICS, eics);
> +                             } else {
> +                                     if ((trigger_irq_rx) && 
> (trigger_irq_tx))
> +                                             wr32(E1000_ICS,
> E1000_ICS_RXDMT0 | E1000_ICS_TXDW);
> +                                     else if (trigger_irq_rx)
> +                                             wr32(E1000_ICS,
> E1000_ICS_RXDMT0);
> +                                     else
> +                                             wr32(E1000_ICS,
> E1000_ICS_TXDW);
> +                             }
> +                     }
> +                     return 0;
> +             }
> +     } else if (flags & XDP_WAKEUP_TX) {
> +             if (qid >= adapter->num_tx_queues)
> +                     return -EINVAL;
> +             /* Get the ring params from Tx */
> +             ring = adapter->tx_ring[qid];
> +             if (test_bit(IGB_RING_FLAG_TX_DISABLED, &ring->flags))
> +                     return -ENETDOWN;
> +     } else if (flags & XDP_WAKEUP_RX) {
> +             if (qid >= adapter->num_rx_queues)
> +                     return -EINVAL;
> +             /* Get the ring params from Rx */
> +             ring = adapter->rx_ring[qid];
> +     } else {
> +             /* Invalid Flags */
>               return -EINVAL;
> -
> -     ring = adapter->tx_ring[qid];
> -
> -     if (test_bit(IGB_RING_FLAG_TX_DISABLED, &ring->flags))
> -             return -ENETDOWN;
> +     }
>
>       if (!READ_ONCE(ring->xsk_pool))
>               return -EINVAL;
> @@ -554,7 +633,12 @@ int igb_xsk_wakeup(struct net_device *dev, u32 qid,
> u32 flags)
>                       eics |= ring->q_vector->eims_value;
>                       wr32(E1000_EICS, eics);
>               } else {
> -                     wr32(E1000_ICS, E1000_ICS_RXDMT0);
> +                     if ((flags & XDP_WAKEUP_RX) && (flags &
> XDP_WAKEUP_TX))
> +                             wr32(E1000_ICS, E1000_ICS_RXDMT0 |
> E1000_ICS_TXDW);
> +                     else if (flags & XDP_WAKEUP_RX)
> +                             wr32(E1000_ICS, E1000_ICS_RXDMT0);
> +                     else
> +                             wr32(E1000_ICS, E1000_ICS_TXDW);
>               }
>       }
>
> --
> 2.34.1

Hi Intel Colleagues,

Would you be submitting this patch to netdev list after your internal 
validation just like the igc patch?

Regards

Vivek

Reply via email to