> -----Original Message----- > From: Tony Nguyen <[email protected]> > Sent: Thursday, December 18, 2025 11:44 PM > To: Behera, Vivek (DI FA DSP ICC PRC1) <[email protected]>; > Kwapulinski, Piotr <[email protected]>; Loktionov, Aleksandr > <[email protected]>; Keller, Jacob E <[email protected]>; > Kitszel, Przemyslaw <[email protected]>; > [email protected] > Cc: [email protected]; [email protected] > Subject: Re: [Intel-wired-lan] [PATCH iwl-net v8] igc: Fix trigger of > incorrect irq in > igc_xsk_wakeup function > > > > On 12/17/2025 11:21 PM, Behera, VIVEK wrote: > > 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]> > > Unless you have received a Reviewed-by tag explicitly from the person, you > shouldn't add it. > > ... > > >>> 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. */ > > nit: This can just be one comment, no need to break it into two comment blocks > > /* 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; > > This should be RCT, longest argument to shortest, so the structs should be > first. > > >>> + > >>> + 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 > > For this patch, I don't think I would re-add them. As part of normal process > (future > patches), yes, but this one was asked to be excluded while the kinks of the > upstream process were worked out. As this is the list you're targeting your > patch > for and I'll be sending this to netdev following our validation, it will get > its review > from netdev at that time. > > Also, adding Siang who is one of the authors from the Fixes: > > Thanks, > Tony > > > > Regards > > > > Vivek
Hi tony. Okay , understood. Please let me know about the status of this patch Regards Vivek
