Re: [PATCH] igc: Fix trigger of incorrect irq in igc_xsk_wakeup function

[Jacob Keller <jacob.e.keller@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 7396 bytes --]



On 12/5/2025 4:39 AM, Behera, VIVEK wrote:
> From 4e3ebdc0af6baa83ccfc17c61c1eb61408095ffd Mon Sep 17 00:00:00 2001
> From: Vivek Behera <vivek.behera@siemens.com>
> Date: Fri, 5 Dec 2025 10:26:05 +0100
> Subject: [PATCH] igc: Fix trigger of incorrect irq in igc_xsk_wakeup function
> 
> When the i226 is configured to use only 2 combined queues using ethtool
> or in an environment with only 2 active CPU cores the 4 irq lines
> are used in a split configuration with one irq
> assigned to each of the two rx and tx queues
> (see console output below)
> 
> sudo ethtool -l enp1s0
> Channel parameters for enp1s0:
> Pre-set maximums:
> RX:                        n/a
> TX:                         n/a
> Other:                  1
> Combined:        4
> Current hardware settings:
> RX:                        n/a
> TX:                         n/a
> Other:                  1
> Combined:        2
> eddx@mvs:~$ cat /proc/interrupts | grep enp1s0
> 147:          1          0  IR-PCI-MSIX-0000:01:00.0   0-edge      enp1s0
> 148:          8          0  IR-PCI-MSIX-0000:01:00.0   1-edge      enp1s0-rx-0
> 149:          0          0  IR-PCI-MSIX-0000:01:00.0   2-edge      enp1s0-rx-1
> 150:         26          0  IR-PCI-MSIX-0000:01:00.0   3-edge      enp1s0-tx-0
> 151:          0          0  IR-PCI-MSIX-0000:01:00.0   4-edge      enp1s0-tx-1
> 
> While testing with the RTC Testbench it was noticed
> using the bpftrace that the
> igc_xsk_wakeup when triggered by xsk_sendmsg
> was triggering the incorrect irq for
> tx-0(see trace below)
> 
> TIMESTAMP: 456992309829 | FUNCTION: igc_xsk_wakeup | ENTRY: RtcTxThread (PID: 945) - queue_id: 0
> TIMESTAMP: 456992317157 | FUNCTION: igc_poll | ENTRY: irq/148-enp1s0- (PID: 948)
> TIMESTAMP: 456993309408 | FUNCTION: igc_xsk_wakeup | ENTRY: RtcTxThread (PID: 945) - queue_id: 0
> TIMESTAMP: 456993316591 | FUNCTION: igc_poll | ENTRY: irq/148-enp1s0- (PID: 948)
> TIMESTAMP: 456994309630 | FUNCTION: igc_xsk_wakeup | ENTRY: RtcTxThread (PID: 945) - queue_id: 0
> TIMESTAMP: 456994316674 | FUNCTION: igc_poll | ENTRY: irq/148-enp1s0- (PID: 948)
> TIMESTAMP: 456995309493 | FUNCTION: igc_xsk_wakeup | ENTRY: RtcTxThread (PID: 945) - queue_id: 0
> TIMESTAMP: 456995316593 | FUNCTION: igc_poll | ENTRY: irq/148-enp1s0- (PID: 948)
> 
> Due to this bug no XDP Zc send is possible in this split irq configuration.
> This patch implements the correct logic of extracting the q_vectors saved
> duirng the rx and tx ring allocation.
> Furthermore the patch includes usage of flags provided by the ndo_xsk_wakeup
> api to trigger the required irq. With this patch correct irqs are triggered
> 
> cat /proc/interrupts | grep enp1s0
> 161:          1          0          0          0 IR-PCI-MSIX-0000:01:00.0    0-edge      enp1s0
> 162:          2          0          0          0 IR-PCI-MSIX-0000:01:00.0    1-edge      enp1s0-rx-0
> 163:        359          0          0          0 IR-PCI-MSIX-0000:01:00.0    2-edge      enp1s0-rx-1
> 164:     872005          0          0          0 IR-PCI-MSIX-0000:01:00.0    3-edge      enp1s0-tx-0
> 165:         71          0          0          0 IR-PCI-MSIX-0000:01:00.0    4-edge      enp1s0-tx-1
> 
> TIMESTAMP: 149658589239205 | FUNCTION: igc_xsk_wakeup | ENTRY: RtcTxThread (PID: 10633) - queue_id: 0
> TIMESTAMP: 149658589244662 | FUNCTION: igc_poll | ENTRY: irq/164-enp1s0- (PID: 10593)
> TIMESTAMP: 149658589293396 | FUNCTION: igc_poll | ENTRY: irq/164-enp1s0- (PID: 10593)
> TIMESTAMP: 149658589295357 | FUNCTION: xsk_tx_completed | ENTRY: irq/164-enp1s0- (PID: 10593) - num_entries: 61
> TIMESTAMP: 149658589342151 | FUNCTION: igc_poll | ENTRY: irq/164-enp1s0- (PID: 10593)
> TIMESTAMP: 149658589343881 | FUNCTION: xsk_tx_completed | ENTRY: irq/164-enp1s0- (PID: 10593) - num_entries: 3
> TIMESTAMP: 149658589391394 | FUNCTION: igc_poll | ENTRY: irq/164-enp1s0- (PID: 10593)
> TIMESTAMP: 149658590239215 | FUNCTION: igc_xsk_wakeup | ENTRY: RtcTxThread (PID: 10633) - queue_id: 0
> 

I appreciate the detailed outline of how to configure the system so this
fails, and the steps taken to verify the change fixes the issue.

> Signed-off-by: Vivek Behera <vivek.behera@siemens.com>

This is a bug fix, so it should be targeted at net. You will need a
Fixes tag associating which commit this fixes as well. Alternatively,
since this is for an Intel networking driver and you sent it to Intel
Wired LAN, it would be "iwl-net" so that it gets picked up by Tony for
testing along with our other igc changes.


> ---
> drivers/net/ethernet/intel/igc/igc_main.c | 31 +++++++++++++++++++----
> 1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 7aafa60ba0c8..0cfcd20a2536 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -6930,21 +6930,42 @@ 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 queue pair per IRQ is needed */
> +                      if (!(adapter->flags & IGC_FLAG_QUEUE_PAIRS))
> +                                  return -EINVAL; /* igc queue pairs are not activated.
> +                                                          * Can't trigger irq
> +                                                          */

We only have to check for queue pairs if we want to wake both. Makes sense.

> +                      /* Just get the ring params from Rx */
> +                      if (queue_id >= adapter->num_rx_queues)
> +                                  return -EINVAL;
> +                      ring = adapter->rx_ring[queue_id];
> +          } 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];
> +          }
>             if (!ring->xsk_pool)
>                        return -ENXIO;
> -
> -           q_vector = adapter->q_vector[queue_id];
> +          /* Retrieve the q_vector saved in the ring */
> +          q_vector = ring->q_vector;
>            if (!napi_if_scheduled_mark_missed(&q_vector->napi))
>                        igc_trigger_rxtxq_interrupt(adapter, q_vector);

The actual code changes seem correct to me, so you may add my review tag
on a version which has the Fixes and the appropriate tree tag i.e.
[iwl-net] or [PATCH iwl-net].

Reviewed-by: Jacob Keller <jacob.keller@intel.com>

Thanks,
Jake

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]