Re: [Intel-wired-lan] [PATCH iwl-net v2] igb: Fix trigger of incorrect irq in igb_xsk_wakeup

2025-12-20 Thread Behera, VIVEK via Intel-wired-lan 
Hi Tony

> -Original Message-
> From: Tony Nguyen 
> Sent: Friday, December 19, 2025 10:07 PM
> To: Behera, Vivek (DI FA DSP ICC PRC1) ;
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]
> Subject: Re: [PATCH iwl-net v2] igb: Fix trigger of incorrect irq in
> igb_xsk_wakeup
> 
> 
> 
> On 12/18/2025 11:05 PM, Behera, VIVEK wrote:
> >
> >
> >> -Original Message-
> >> From: Behera, Vivek (DI FA DSP ICC PRC1) 
> >> 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) 
> >> 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-:08:00.0
> >>   0-edge  eno2
> >>   168:  0  0  0  0 IR-PCI-MSIX-:08:00.0
> >>   1-edge  eno2-rx-0
> >>   169:  0  0  0  0 IR-PCI-MSIX-:08:00.0
> >>   2-edge  eno2-rx-1
> >>   170:  0  0  0  0 IR-PCI-MSIX-:08:00.0
> >>   3-edge  eno2-tx-0
> >>   171:  0  0  0  0 IR-PCI-MSIX-: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 
> >> Reviewed-by: Aleksandr loktinov 
> >> ---
> >> v1:
> >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flor
> >>
> e.kernel.o%2F&data=05%7C02%7Cvivek.behera%40siemens.com%7C1540e21c5
> 79
> >>
> 548f5fddc08de3f429f20%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7
> C63
> >>
> 9017752565580869%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRyd
> WUsIlYi
> >>
> OiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C
> 0%
> >>
> 7C%7C%7C&sdata=MPBbmZMTBMzxIthA3AnmBB9tQhJiY8bUj4S8JtNsV4s%3D
> &reserve
> >> d=0
> >> 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_RXDMT0E1000_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
> >> @@ -

Re: [Intel-wired-lan] [PATCH iwl-net v2] igb: Fix trigger of incorrect irq in igb_xsk_wakeup

2025-12-19 Thread Tony Nguyen 




On 12/18/2025 11:05 PM, Behera, VIVEK wrote:




-Original Message-
From: Behera, Vivek (DI FA DSP ICC PRC1) 
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)

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-:08:00.0
  0-edge  eno2
  168:  0  0  0  0 IR-PCI-MSIX-:08:00.0
  1-edge  eno2-rx-0
  169:  0  0  0  0 IR-PCI-MSIX-:08:00.0
  2-edge  eno2-rx-1
  170:  0  0  0  0 IR-PCI-MSIX-:08:00.0
  3-edge  eno2-tx-0
  171:  0  0  0  0 IR-PCI-MSIX-: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 
Reviewed-by: Aleksandr loktinov 
---
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_RXDMT0E1000_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;
+
+

Re: [Intel-wired-lan] [PATCH iwl-net v2] igb: Fix trigger of incorrect irq in igb_xsk_wakeup

2025-12-18 Thread Behera, VIVEK via Intel-wired-lan 



> -Original Message-
> From: Behera, Vivek (DI FA DSP ICC PRC1) 
> 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)
> 
> 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-:08:00.0
>  0-edge  eno2
>  168:  0  0  0  0 IR-PCI-MSIX-:08:00.0
>  1-edge  eno2-rx-0
>  169:  0  0  0  0 IR-PCI-MSIX-:08:00.0
>  2-edge  eno2-rx-1
>  170:  0  0  0  0 IR-PCI-MSIX-:08:00.0
>  3-edge  eno2-tx-0
>  171:  0  0  0  0 IR-PCI-MSIX-: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 
> Reviewed-by: Aleksandr loktinov 
> ---
> 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_RXDMT0E1000_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_s