| 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: <ae675b47-1c9d-4dc3-8808-1e6cd6589acd@molgen.mpg.de>
Date: Wed, 10 Dec 2025 17:41:35 +0100
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Vivek Behera <vivek.behera@...mens.com>
Cc: Aleksandr Loktionov <aleksandr.loktionov@...el.com>,
Jacob E Keller <jacob.e.keller@...el.com>,
Anthony L Nguyen <anthony.l.nguyen@...el.com>,
Przemyslaw 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>,
intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [Intel-wired-lan] [PATCH v5 iwl-net] igc: Fix trigger of
incorrect irq in igc_xsk_wakeup function
Dear Vivek,
Thank you for your patch.
Am 10.12.25 um 08:50 schrieb Behera, VIVEK:
What is your given name? ;-) I think in French, it’s common to
capitalize the surname.
> Changes in v5:
> - Updated comment style from multi-star to standard /* */ as
> suggested by Aleksandr.
It’s common to add that below the --- and above the diffstat.
> From ab2583ff8a17405d3aa6caf4df1c4fdfb21f5e98 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 v5] [iwl-net] 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.
>
> Changed comment blocks to align with standard Linux comments
Maybe “Also, change the comment blocks to …” and a dot/period at the end.
Do you have a reproducer for this? If yes, it’d be great if you
documented this in the commit message.
>
> Fixes: fc9df2a0b520d7d439ecf464794d53e91be74b93 ("igc: Enable RX via AF_XDP zero-copy")
It’s common to use 14(?) characters of the hash.
> Fixes: 15fd021bc4270273d8f4b7f58fdda8a16214a377 ("igc: Add Tx hardware timestamp request for AF_XDP zero-copy packet")
Ditto.
> Signed-off-by: Vivek Behera <vivek.behera@...mens.com>
> Reviewed-by: Jacob Keller <jacob.keller@...el.com>
> Reviewed-by: Aleksandr loktinov <aleksandr.loktionov@...el.com>
Please capitalize the last name.
> ---
> drivers/net/ethernet/intel/igc/igc_main.c | 81 ++++++++++++++++++-----
> drivers/net/ethernet/intel/igc/igc_ptp.c | 2 +-
> 2 files changed, 64 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..c7bf5a4b89e9 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,71 @@ 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)
> - return -EINVAL;
> + 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 */
> + if (queue_id >= adapter->num_rx_queues)
> + return -EINVAL; /*queue_id is invalid*/
>
> - ring = adapter->rx_ring[queue_id];
> + if (queue_id >= adapter->num_tx_queues)
> + return -EINVAL; /* queue_id invalid */
>
> - if (!ring->xsk_pool)
> - return -ENXIO;
> + /* IRQ trigger preparation for Rx */
> + ring = adapter->rx_ring[queue_id];
> + 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);
> + /* Retrieve the q_vector saved in the ring */
> + q_vector = ring->q_vector;
> + if (!napi_if_scheduled_mark_missed(&q_vector->napi))
> + eics |= q_vector->eims_value;
> + /* IRQ trigger preparation for Tx */
> + ring = adapter->tx_ring[queue_id];
>
> + if (!ring->xsk_pool)
> + return -ENXIO;
> +
> + /* Retrieve the q_vector saved in the ring */
> + q_vector = ring->q_vector;
> + if (!napi_if_scheduled_mark_missed(&q_vector->napi))
> + eics |= q_vector->eims_value; /* Extend the BIT mask for eics */
> +
> + /* Now we trigger the split irqs for Rx and Tx over eics */
> + if (eics != 0)
> + 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;
> + }
> + /* Prepare to trigger single irq */
> + if (!ring->xsk_pool)
> + return -ENXIO;
> + /* Retrieve the q_vector saved in the ring */
> + 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;
> }
Kind regards,
Paul
Powered by blists - more mailing lists