netdev - RE: [PATCH] igc: Fix trigger of incorrect irq in igc_xsk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <AS1PR10MB53929B574F1D9A8645B6CA938FA4A@AS1PR10MB5392.EURPRD10.PROD.OUTLOOK.COM>
Date: Sat, 6 Dec 2025 12:03:08 +0000
From: "Behera, VIVEK" <vivek.behera@...mens.com>
To: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>, "Nguyen, Anthony
 L" <anthony.l.nguyen@...el.com>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>, Andrew Lunn <andrew+netdev@...n.ch>, "\"David
 S. Miller\"" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] igc: Fix trigger of incorrect irq in igc_xsk_wakeup
 function

Hi Aleksandr ,

Thanks for the review and the valuable feedback!

On Fri, Dec 5, 2025 at 1:40 PM, Intel-wired-lan <intel-wired-lan-bounces@...osl.org> wrote:
> From 4e3ebdc0af6baa83ccfc17c61c1eb61408095ffd Mon Sep 17 00:00:00 2001
> From: Vivek Behera <vivek.behera@...mens.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)
>
>
> ...
>
> Signed-off-by: Vivek Behera <vivek.behera@...mens.com>
> ---
> 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
> +                                                          */
> It looks like not a malformed input, but as unsupported operation for the current device/IRQ configuration. In net drivers, -EOPNOTSUPP is the expected errno for "the device cannot perform this requested operation in this configuration," while -EINVAL signals a bad argument.
> Am I right?

When considering the split configuration, it would still be possible to trigger the individual IRQs for TX and RX. It is just about setting the correct bits in the EICS register. However, when I refer to the `ndo_xsk_wakeup` documentation, it isn't clear to me whether triggering multiple IRQs would be correct or desired. Referring to other netdev drivers implementing this hook, I couldn't find any instance of triggering individual IRQs in one call of the function. This could also be due to the fact that none of the drivers use the `flags` argument when the function is called. This is why I am inclined to handle this case as an error. You are right, the `-EOPNOTSUPP` would be the correct return value in this case. I will update this in the next version of the patch (v2) to reflect this change.

> +                      /* 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;

> ...

Thanks again for catching this!

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