On Sat, Jan 17, 2026 at 03:51:12PM +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]>
> Suggested-by: Maciej Fijalkowski <[email protected]>
> ---
> v1: 
> https://lore.kernel.org/intel-wired-lan/[email protected]/
> v2: 
> https://lore.kernel.org/intel-wired-lan/[email protected]/
> v3: 
> https://lore.kernel.org/intel-wired-lan/[email protected]/
> v4: 
> https://lore.kernel.org/intel-wired-lan/[email protected]/
> v5: 
> https://lore.kernel.org/intel-wired-lan/[email protected]/
> 
> changelog:
> v1
> - Initial 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
> 
> v4 -> v5
> - Introduced a simplified logic for sequential check for RX and TX
> 
> v5 -> v6
> - Further simplifications suggested by Maciej
> - Included review suggestions from reviewers
> ---
>  drivers/net/ethernet/intel/igb/igb_xsk.c | 31 ++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_xsk.c 
> b/drivers/net/ethernet/intel/igb/igb_xsk.c
> index 30ce5fbb5b77..f8bddb7a2af8 100644
> --- a/drivers/net/ethernet/intel/igb/igb_xsk.c
> +++ b/drivers/net/ethernet/intel/igb/igb_xsk.c
> @@ -524,6 +524,16 @@ bool igb_xmit_zc(struct igb_ring *tx_ring, struct 
> xsk_buff_pool *xsk_pool)
>       return nb_pkts < budget;
>  }
>  
> +static u32 igb_sw_irq_prep(struct igb_q_vector *q_vector)
> +{
> +     u32 eics = 0;
> +
> +     if (!napi_if_scheduled_mark_missed(&q_vector->napi))
> +             eics = q_vector->eims_value;
> +
> +     return eics;
> +}
> +
>  int igb_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags)
>  {
>       struct igb_adapter *adapter = netdev_priv(dev);
> @@ -542,16 +552,27 @@ int igb_xsk_wakeup(struct net_device *dev, u32 qid, u32 
> flags)
>  
>       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;
>  
> -     if (!napi_if_scheduled_mark_missed(&ring->q_vector->napi)) {
> +     if (flags & XDP_WAKEUP_TX) {
> +             if (test_bit(IGB_RING_FLAG_TX_DISABLED, &ring->flags))
> +                     return -ENETDOWN;
> +
> +             eics |= igb_sw_irq_prep(ring->q_vector);
> +     }
> +
> +     if (flags & XDP_WAKEUP_RX) {
> +             /* for IGB_FLAG_QUEUE_PAIRS, this will be NOP as NAPI has
> +              * been already marked with NAPIF_STATE_MISSED
> +              */
> +             ring = adapter->rx_ring[qid];
> +             eics |= igb_sw_irq_prep(ring->q_vector);
> +     }
> +
> +     if (eics) {
>               /* Cause software interrupt */
>               if (adapter->flags & IGB_FLAG_HAS_MSIX) {

braces are redundant now as we have single line bodies of branches.
Besides:

Acked-by: Maciej Fijalkowski <[email protected]>

Thanks for co-op!

> -                     eics |= ring->q_vector->eims_value;
>                       wr32(E1000_EICS, eics);
>               } else {
>                       wr32(E1000_ICS, E1000_ICS_RXDMT0);
> -- 
> 2.34.1
> 

Reply via email to