> -----Ursprüngliche Nachricht----- > Von: Loktionov, Aleksandr <[email protected]> > Gesendet: Montag, 22. Dezember 2025 10:50 > 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] > Betreff: RE: [PATCH iwl-net v3] igb: Fix trigger of incorrect irq in > igb_xsk_wakeup > > > > > -----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%2 > > > F&data=05%7C02%7Cvivek.behera%40siemens.com%7Cefe284bc96394ac4faec08 > de > > > 413f8258%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C6390199381861 > 762 > > > 35%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuM > DAwMCI > > > sIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdat > a=s > > 1ibDomK94wJm36PXD5zZI30WDRPX5uvCN9cs%2FCjAbE%3D&reserved=0 > > > > 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%2 > > > F&data=05%7C02%7Cvivek.behera%40siemens.com%7Cefe284bc96394ac4faec08 > de > > > 413f8258%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C6390199381862 > 000 > > > 95%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuM > DAwMCI > > > sIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdat > a=B > > W0eXA2EA7t8cu1Rzshdn8MngOgwlNPu5UcXNbrTch8%3D&reserved=0 > > > > 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 */ > Yes. I agree the comment would make ii clearer. I would include it in the next version Regards Vivek > 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
Re: [Intel-wired-lan] [PATCH iwl-net v3] igb: Fix trigger of incorrect irq in igb_xsk_wakeup
Behera, VIVEK via Intel-wired-lan Mon, 22 Dec 2025 02:59:16 -0800
- [Intel-wired-lan] [PATCH iwl-net v3] igb... Vivek Behera 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] [PATC... Loktionov, Aleksandr
- Re: [Intel-wired-lan] [... Behera, VIVEK via Intel-wired-lan
