Hello colleagues,

> -----Original Message-----
> From: Kwapulinski, Piotr <[email protected]>
> Sent: Tuesday, December 16, 2025 6:55 PM
> To: Behera, Vivek (DI FA DSP ICC PRC1) <[email protected]>;
> Loktionov, Aleksandr <[email protected]>; Keller, Jacob E
> <[email protected]>; Nguyen, Anthony L <[email protected]>;
> Kitszel, Przemyslaw <[email protected]>
> Cc: [email protected]; [email protected]; Behera, Vivek (DI FA
> DSP ICC PRC1) <[email protected]>
> Subject: RE: [Intel-wired-lan] [PATCH iwl-net v8] igc: Fix trigger of 
> incorrect irq in
> igc_xsk_wakeup function
> 
> >-----Original Message-----
> >From: Intel-wired-lan <[email protected]> On Behalf Of
> >Vivek Behera
> >Sent: Monday, December 15, 2025 1:21 PM
> >To: Loktionov, Aleksandr <[email protected]>; Keller, Jacob
> >E <[email protected]>; Nguyen, Anthony L
> ><[email protected]>; Kitszel, Przemyslaw
> ><[email protected]>
> >Cc: [email protected]; [email protected]; Behera, Vivek
> ><[email protected]>
> >Subject: [Intel-wired-lan] [PATCH iwl-net v8] igc: Fix trigger of
> >incorrect irq in igc_xsk_wakeup function
> >
> >This patch addresses the issue where the igc_xsk_wakeup function was
> triggering an incorrect IRQ for tx-0 when the i226 is configured with only 2
> combined queues or in an environment with 2 active CPU cores.
> >This prevented XDP Zero-copy send functionality in such split IRQ
> configurations.
> >
> >The fix implements the correct logic for extracting q_vectors saved during rx
> and tx ring allocation and utilizes flags provided by the ndo_xsk_wakeup API 
> to
> trigger the appropriate IRQ.
> >
> >Fixes: fc9df2a0b520 ("igc: Enable RX via AF_XDP zero-copy")
> >Fixes: 15fd021bc427 ("igc: Add Tx hardware timestamp request for AF_XDP
> >zero-copy packet")
> >Signed-off-by: Vivek Behera <[email protected]>
> >Reviewed-by: Jacob Keller <[email protected]>
> >Reviewed-by: Aleksandr loktinov <[email protected]>
> >Reviewed-by: Przemyslaw Kitszel <[email protected]>
> >Reviewed-by: Tony Nguyen <[email protected]>
> >---
> >v1:
> >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> >kernel.org%2Fintel-wired-
> lan%2FAS1PR10MB5392B7268416DB8A1624FDB88FA7A%4
> >0AS1PR10MB5392.EURPRD10.PROD.OUTLOOK.COM%2F&data=05%7C02%7
> Cvivek.behera
> >%40siemens.com%7C8a63023eb06047539d5c08de3ccc4d88%7C38ae3bcd9579
> 4fd4add
> >ab42e1495d55a%7C1%7C0%7C639015045337851822%7CUnknown%7CTWFpb
> GZsb3d8eyJF
> >bXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTW
> FpbCI
> >sIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=rFeGiAGZ%2B04ujqFtkeYgGb
> o%2FrDIOds
> >qC9W2Ns0zfXmg%3D&reserved=0
> >v2:
> >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> >kernel.org%2Fintel-wired-
> lan%2FAS1PR10MB539280B1427DA0ABE9D65E628FA5A%4
> >0AS1PR10MB5392.EURPRD10.PROD.OUTLOOK.COM%2F&data=05%7C02%7
> Cvivek.behera
> >%40siemens.com%7C8a63023eb06047539d5c08de3ccc4d88%7C38ae3bcd9579
> 4fd4add
> >ab42e1495d55a%7C1%7C0%7C639015045337901136%7CUnknown%7CTWFpb
> GZsb3d8eyJF
> >bXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTW
> FpbCI
> >sIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=VB1%2FADgBeiczrNyjCi4EUT
> wpEPJNCEY5
> >NR2kpIHjp3s%3D&reserved=0
> >v3:
> >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> >kernel.org%2Fintel-wired-
> lan%2FIA3PR11MB8986E4ACB7F264CF2DD1D750E5A0A%4
> >0IA3PR11MB8986.namprd11.prod.outlook.com%2F&data=05%7C02%7Cvivek.be
> hera
> >%40siemens.com%7C8a63023eb06047539d5c08de3ccc4d88%7C38ae3bcd9579
> 4fd4add
> >ab42e1495d55a%7C1%7C0%7C639015045337945302%7CUnknown%7CTWFpb
> GZsb3d8eyJF
> >bXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTW
> FpbCI
> >sIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=HNXDOBjXNji%2FI2Pk2mV%2
> B1XLvpZQt7K
> >4dxnQ7DZnGmmk%3D&reserved=0
> >v4:
> >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> >kernel.org%2Fintel-wired-
> lan%2FAS1PR10MB53926CB955FBD4F9F4A018818FA0A%4
> >0AS1PR10MB5392.EURPRD10.PROD.OUTLOOK.COM%2F&data=05%7C02%7
> Cvivek.behera
> >%40siemens.com%7C8a63023eb06047539d5c08de3ccc4d88%7C38ae3bcd9579
> 4fd4add
> >ab42e1495d55a%7C1%7C0%7C639015045337988842%7CUnknown%7CTWFpb
> GZsb3d8eyJF
> >bXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTW
> FpbCI
> >sIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=HrTbjiGaps3kGt4Qo9aJhfRoP
> OGjcY4JnI
> >njRisiYns%3D&reserved=0
> >v5:
> >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> >kernel.org%2Fintel-wired-
> lan%2FAS1PR10MB5392FCA415A38B9DD7BB5F218FA0A%4
> >0AS1PR10MB5392.EURPRD10.PROD.OUTLOOK.COM%2F&data=05%7C02%7
> Cvivek.behera
> >%40siemens.com%7C8a63023eb06047539d5c08de3ccc4d88%7C38ae3bcd9579
> 4fd4add
> >ab42e1495d55a%7C1%7C0%7C639015045338031278%7CUnknown%7CTWFpb
> GZsb3d8eyJF
> >bXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTW
> FpbCI
> >sIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=CJYmzQmJAwhK4pMwNg5B4
> vnP8v7Pu%2BZy
> >KqWq1RJkXZk%3D&reserved=0
> >v6:
> >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> >kernel.org%2Fintel-wired-lan%2F20251211173916.23951-1-vivek.behera%40si
> >emens.com%2F&data=05%7C02%7Cvivek.behera%40siemens.com%7C8a6302
> 3eb06047
> >539d5c08de3ccc4d88%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7
> C6390150
> >45338064045%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUs
> IlYiOiIwLjA
> >uMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C
> %7C%7C&
> >sdata=4fhP5XhkJq8qjEqVitlGrfu3FQskjaTgWbPWFYyXGvI%3D&reserved=0
> >v7:
> >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> >kernel.org%2Fintel-wired-lan%2F20251212163208.137164-1-vivek.behera%40s
> >iemens.com%2F&data=05%7C02%7Cvivek.behera%40siemens.com%7C8a6302
> 3eb0604
> >7539d5c08de3ccc4d88%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7
> C639015
> >045338092038%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWU
> sIlYiOiIwLj
> >AuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7
> C%7C%7C
> >&sdata=Y1%2BDpVeRVIChENPfIwtLjWPsyBgnGU%2FPm47Xe%2FqHiUo%3D&
> reserved=0
> >
> >changelog:
> >v1
> >- Inital description of the Bug and steps to reproduce with RTC
> >Testbench
> >- Test results after applying the patch
> >v1 -> v2
> >- Handling of RX and TX Wakeup in igc_xsk_wakeup for a split IRQ
> >configuration
> >- Removal of igc_trigger_rxtxq_interrupt (now redundant)
> >- Added flag to igc_xsk_wakeup function call in igc_ptp_free_tx_buffer
> >v2 -> v3
> >- Added 'Fixes:' tags for the relevant commits.
> >- Added reviewer
> >v3 -> v4
> >- Added reviewer
> >v4 -> v5
> >- Updated comment style from multi-star to standard linux convention
> >v5 -> v6
> >- Resolve formatting issues highlighted by reviewers
> >- Try to include version histroy as defined in netdev guidelines
> >- Included review suggestions from Przemyslaw
> >- Added reviewers
> >v6 -> v7
> >- Included review suggestions from Przemyslaw missed in v6
> >v7 -> v8
> >- Modified sequence to complete all error checks for rx and tx
> >  before updating napi states and triggering irq
> >---
> > drivers/net/ethernet/intel/igc/igc_main.c | 90 ++++++++++++++++++-----
> >drivers/net/ethernet/intel/igc/igc_ptp.c  |  2 +-
> > 2 files changed, 73 insertions(+), 19 deletions(-)
> >
> >diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> >b/drivers/net/ethernet/intel/igc/igc_main.c
> >index 7aafa60ba0c8..76e4790bd3c0 100644
> >--- a/drivers/net/ethernet/intel/igc/igc_main.c
> >+++ b/drivers/net/ethernet/intel/igc/igc_main.c
> >@@ -6908,21 +6908,13 @@ static int igc_xdp_xmit(struct net_device *dev, int
> num_frames,
> >     return nxmit;
> > }
> >
> >-static void igc_trigger_rxtxq_interrupt(struct igc_adapter *adapter,
> >-                                    struct igc_q_vector *q_vector)
> >-{
> >-    struct igc_hw *hw = &adapter->hw;
> >-    u32 eics = 0;
> >-
> >-    eics |= q_vector->eims_value;
> >-    wr32(IGC_EICS, eics);
> >-}
> >-
> > int igc_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags)  {
> >     struct igc_adapter *adapter = netdev_priv(dev);
> >+    struct igc_hw *hw = &adapter->hw;
> >     struct igc_q_vector *q_vector;
> >     struct igc_ring *ring;
> >+    u32 eics = 0;
> >
> >     if (test_bit(__IGC_DOWN, &adapter->state))
> >             return -ENETDOWN;
> >@@ -6930,18 +6922,80 @@ int igc_xsk_wakeup(struct net_device *dev, u32
> queue_id, u32 flags)
> >     if (!igc_xdp_is_enabled(adapter))
> >             return -ENXIO;
> >
> >-    if (queue_id >= adapter->num_rx_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 & IGC_FLAG_QUEUE_PAIRS)) {
> >+                    /* Just get the ring params from Rx */
> >+                    if (queue_id >= adapter->num_rx_queues)
> >+                            return -EINVAL;
> >+                    ring = adapter->rx_ring[queue_id];
> >+            } 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;
> >+
> >+                    if (queue_id >= adapter->num_rx_queues)
> >+                            return -EINVAL;
> >+
> >+                    if (queue_id >= adapter->num_tx_queues)
> >+                            return -EINVAL;
> >+
> >+                    /* IRQ trigger preparation for Rx */
> >+                    ring = adapter->rx_ring[queue_id];
> >+                    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[queue_id];
> >+                    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))
> >+                            eics |= eics_rx;
> >+                    if (!napi_if_scheduled_mark_missed(tx_napi))
> >+                            eics |= eics_tx;
> >+
> >+                    /* Now we trigger the required irqs for Rx and Tx */
> >+                    if (eics)
> >+                            wr32(IGC_EICS, eics);
> >+
> >+                    return 0;
> >+            }
> >+    } else if (flags & XDP_WAKEUP_TX) {
> >+            if (queue_id >= adapter->num_tx_queues)
> >+                    return -EINVAL;
> >+            /* Get the ring params from Tx */
> >+            ring = adapter->tx_ring[queue_id];
> >+    } else if (flags & XDP_WAKEUP_RX) {
> >+            if (queue_id >= adapter->num_rx_queues)
> >+                    return -EINVAL;
> >+            /* Get the ring params from Rx */
> >+            ring = adapter->rx_ring[queue_id];
> >+    } else {
> >+            /* Invalid Flags */
> >             return -EINVAL;
> >-
> >-    ring = adapter->rx_ring[queue_id];
> >-
> >+    }
> >+    /* Prepare to trigger single irq */
> >     if (!ring->xsk_pool)
> >             return -ENXIO;
> >
> >-    q_vector = adapter->q_vector[queue_id];
> >-    if (!napi_if_scheduled_mark_missed(&q_vector->napi))
> >-            igc_trigger_rxtxq_interrupt(adapter, q_vector);
> >-
> >+    q_vector = ring->q_vector;
> >+    if (!napi_if_scheduled_mark_missed(&q_vector->napi)) {
> >+            eics |= q_vector->eims_value;
> >+            wr32(IGC_EICS, eics);
> >+    }
> >     return 0;
> > }
> >
> >diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c
> >b/drivers/net/ethernet/intel/igc/igc_ptp.c
> >index b7b46d863bee..6d8c2d639cd7 100644
> >--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
> >+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
> >@@ -550,7 +550,7 @@ static void igc_ptp_free_tx_buffer(struct igc_adapter
> *adapter,
> >             tstamp->buffer_type = 0;
> >
> >             /* Trigger txrx interrupt for transmit completion */
> >-            igc_xsk_wakeup(adapter->netdev, tstamp->xsk_queue_index, 0);
> >+            igc_xsk_wakeup(adapter->netdev, tstamp->xsk_queue_index,
> >+XDP_WAKEUP_TX);
> >
> >             return;
> >     }
> >--
> >2.34.1
> 
> Reviewed-by: Piotr Kwapulinski <[email protected]>

Thanks for all the reviews. Are there any suggestions before I add netdev@ and 
linux-kernel@ to the list for further review

Regards

Vivek

Reply via email to