> -----Ursprüngliche Nachricht----- > Von: Loktionov, Aleksandr <[email protected]> > Gesendet: Dienstag, 13. Januar 2026 10:00 > 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 v5] igb: Fix trigger of incorrect irq in > igb_xsk_wakeup > > > > > -----Original Message----- > > From: Vivek Behera <[email protected]> > > Sent: Monday, January 12, 2026 2:04 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 v5] 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 Loktionov <[email protected]> > > --- > > v1: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore > > .kernel.org%2Fintel-wired-lan%2F20251212131454.124116-1-&data=05%7C02% > > > 7Cvivek.behera%40siemens.com%7C89e4331d9dd54d83b19108de5282346c%7C38 > ae > > > 3bcd95794fd4addab42e1495d55a%7C1%7C0%7C639038916338104430%7CUnknow > n%7C > > > TWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4 > zMi > > > IsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=m3LL7aO3Tvj9 > Kimm > > uKo9Fgw6dEqAg%2F1pUBRzk3haioo%3D&reserved=0 > > [email protected]/ > > v2: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore > > .kernel.org%2Fintel-wired-lan%2F20251215115416.410619-1-&data=05%7C02% > > > 7Cvivek.behera%40siemens.com%7C89e4331d9dd54d83b19108de5282346c%7C38 > ae > > > 3bcd95794fd4addab42e1495d55a%7C1%7C0%7C639038916338132851%7CUnknow > n%7C > > > TWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4 > zMi > > > IsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=KaVmMrku6oX > yzlRh > > Ii5qvgm47j5HWJ4ma601OvHysxM%3D&reserved=0 > > [email protected]/ > > v3: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore > > .kernel.org%2Fintel-wired-lan%2F20251220114936.140473-1-&data=05%7C02% > > > 7Cvivek.behera%40siemens.com%7C89e4331d9dd54d83b19108de5282346c%7C38 > ae > > > 3bcd95794fd4addab42e1495d55a%7C1%7C0%7C639038916338151419%7CUnknow > n%7C > > > TWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4 > zMi > > > IsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=lTjnm324in4Zap > H4 > > veRCyAqnkYpAgfzi1fcDpAxqmZE%3D&reserved=0 > > [email protected]/ > > v4: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore > > .kernel.org%2Fintel-wired-lan%2F20251222115747.230521-1-&data=05%7C02% > > > 7Cvivek.behera%40siemens.com%7C89e4331d9dd54d83b19108de5282346c%7C38 > ae > > > 3bcd95794fd4addab42e1495d55a%7C1%7C0%7C639038916338169212%7CUnknow > n%7C > > > TWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4 > zMi > > > IsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=YE9U7UzDDE > %2FyNc > > tyeMkJgD8Jmdhch4TlpuovCNa4bY0%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 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 > > --- > > .../net/ethernet/intel/igb/e1000_defines.h | 1 + > > drivers/net/ethernet/intel/igb/igb_xsk.c | 75 +++++++++++++++--- > > - > > 2 files changed, 61 insertions(+), 15 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..6e51b5b6f131 100644 > > --- a/drivers/net/ethernet/intel/igb/igb_xsk.c > > +++ b/drivers/net/ethernet/intel/igb/igb_xsk.c > > @@ -529,6 +529,13 @@ 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; > > + 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; > > u32 eics = 0; > > > > if (test_bit(__IGB_DOWN, &adapter->state)) @@ -536,27 +543,65 @@ 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; > But the number may differ in case of reconfiguration. Sorry but I think we had this discussion some weeks ago. The number of rx and tx queues can never be different for igb. The driver's ethtool only supports setting combined queues. Here is the snippet from the igb_set_channels from igb_ethtool.c /* Verify they are not requesting separate vectors */ if (!count || ch->rx_count || ch->tx_count) return -EINVAL; Consequently, it is not possible to configure different number of rx and tx queues. sudo ethtool -L eno2 rx 2 tx 1 netlink error: requested channel count exceeds maximum (offset 36) netlink error: Invalid argument only combined option is supported eddx@mvs:~/linux-6.18$ sudo ethtool -L eno2 combined 4 eddx@mvs:~/linux-6.18$ cat /proc/interrupts | grep eno2 162: 1 0 0 0 IR-PCI-MSIX-0000:08:00.0 0-edge eno2 163: 0 0 0 0 IR-PCI-MSIX-0000:08:00.0 1-edge eno2-TxRx-0 164: 2 0 0 0 IR-PCI-MSIX-0000:08:00.0 2-edge eno2-TxRx-1 165: 0 0 0 0 IR-PCI-MSIX-0000:08:00.0 3-edge eno2-TxRx-2 166: 0 0 0 0 IR-PCI-MSIX-0000:08:00.0 4-edge eno2-TxRx-3 However, the irq's are split over the available irq's depending on the number of combined pairs active sudo ethtool -L eno2 combined 2 eddx@mvs:~/linux-6.18$ cat /proc/interrupts | grep eno2 162: 1 0 0 0 IR-PCI-MSIX-0000:08:00.0 0-edge eno2 163: 0 0 0 0 IR-PCI-MSIX-0000:08:00.0 1-edge eno2-rx-0 164: 2 0 0 0 IR-PCI-MSIX-0000:08:00.0 2-edge eno2-rx-1 165: 3 0 0 0 IR-PCI-MSIX-0000:08:00.0 3-edge eno2-tx-0 166: 5 0 0 0 IR-PCI-MSIX-0000:08:00.0 4-edge eno2-tx-1 eddx@mvs:~/linux-6.18$ sudo ethtool -L eno2 combined 3 eddx@mvs:~/linux-6.18$ cat /proc/interrupts | grep eno2 162: 1 0 0 0 IR-PCI-MSIX-0000:08:00.0 0-edge eno2 163: 0 0 0 0 IR-PCI-MSIX-0000:08:00.0 1-edge eno2-TxRx-0 164: 5 0 0 0 IR-PCI-MSIX-0000:08:00.0 2-edge eno2-TxRx-1 165: 2 0 0 0 IR-PCI-MSIX-0000:08:00.0 3-edge eno2-TxRx-2 eddx@mvs:~/linux-6.18$ sudo ethtool -L eno2 combined 1 eddx@mvs:~/linux-6.18$ cat /proc/interrupts | grep eno2 162: 1 0 0 0 IR-PCI-MSIX-0000:08:00.0 0-edge eno2 163: 0 0 0 0 IR-PCI-MSIX-0000:08:00.0 1-edge eno2-rx-0 164: 2 0 0 0 IR-PCI-MSIX-0000:08:00.0 2-edge eno2-tx-0 So, unless I am missing something I think the current logic should be sufficient. > Why not: > if (qid >= adapter->num_rx_queues || qid >= adapter->num_tx_queues) > return -EINVAL; > Or add assertion that they're equal: > if (WARN_ON(adapter->num_rx_queues != adapter->num_tx_queues)) > return -EINVAL; Okay. I can add this. > if (qid >= adapter->num_rx_queues) > return -EINVAL; > > > > - > > - ring = adapter->tx_ring[qid]; > > - > > ... > > > -- > > 2.34.1
Re: [Intel-wired-lan] [PATCH iwl-net v5] igb: Fix trigger of incorrect irq in igb_xsk_wakeup
Behera, VIVEK via Intel-wired-lan Tue, 13 Jan 2026 02:02:36 -0800
- [Intel-wired-lan] [PATCH iwl-net v5] igb... Vivek Behera via Intel-wired-lan
- Re: [Intel-wired-lan] [PATCH iwl-ne... Paul Menzel
- Re: [Intel-wired-lan] [PATCH iw... Behera, VIVEK via Intel-wired-lan
- Re: [Intel-wired-lan] [PATCH iwl-ne... Kwapulinski, Piotr
- Re: [Intel-wired-lan] [PATCH iw... Behera, VIVEK via Intel-wired-lan
- Re: [Intel-wired-lan] [PATCH iwl-ne... Behera, VIVEK via Intel-wired-lan
- Re: [Intel-wired-lan] [PATCH iwl-ne... Loktionov, Aleksandr
- Re: [Intel-wired-lan] [PATCH iw... Behera, VIVEK via Intel-wired-lan
- Re: [Intel-wired-lan] [PATCH iwl-ne... Maciej Fijalkowski
- Re: [Intel-wired-lan] [PATCH iw... Behera, VIVEK via Intel-wired-lan
- Re: [Intel-wired-lan] [PATC... Maciej Fijalkowski
- Re: [Intel-wired-lan] [... Behera, VIVEK via Intel-wired-lan
- Re: [Intel-wired-l... Maciej Fijalkowski
- Re: [Intel-wir... Behera, VIVEK via Intel-wired-lan
