| 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
| ||
|
Message-ID:
<AS1PR10MB5392C71EED7AB2446036FB9F8FA3A@AS1PR10MB5392.EURPRD10.PROD.OUTLOOK.COM>
Date: Tue, 9 Dec 2025 06:03:09 +0000
From: "Behera, VIVEK" <vivek.behera@...mens.com>
To: Jacob Keller <jacob.e.keller@...el.com>, Tony Nguyen
<anthony.l.nguyen@...el.com>, Przemek Kitszel <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 Jacob,
Thank you very much for taking the time to review the patch and for the `Reviewed-by:` tag! I really appreciate your feedback.
Just wanted to let you know that I had already sent out `v2` of this patch, which included the PTP TX wakeup fix and addressed some `checkpatch.pl` comments.
For `v3`, I will incorporate your suggestions and send it out shortly.
Thanks again,
Vivek Behera
-----Original Message-----
From: Jacob Keller <jacob.e.keller@...el.com>
Sent: Tuesday, December 9, 2025 12:54 AM
To: Behera, Vivek (DI FA DSP ICC PRC1) <vivek.behera@...mens.com>; Tony Nguyen <anthony.l.nguyen@...el.com>; Przemek Kitszel <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; netdev@...r.kernel.org; linux-kernel@...r.kernel.org
Subject: Re: [PATCH] igc: Fix trigger of incorrect irq in igc_xsk_wakeup function
On 12/5/2025 4:39 AM, Behera, VIVEK 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)
>
> 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@...mens.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@...el.com>
Thanks,
Jake
Powered by blists - more mailing lists