Re: [Intel-wired-lan] [PATCH v2] igc: fix page fault in XDP TX timestamps handling

[Paul Menzel <pmenzel@molgen.mpg.de>]

Dear Zdenek,


Thank you for your patch.

Am 25.02.26 um 10:58 schrieb Zdenek Bouska via Intel-wired-lan:
> If an XDP application that requested TX timestamping is shutting down
> while the link of the interface in use is still up the following kernel
> splat is reported:
> 
> [  883.803618] [   T1554] BUG: unable to handle page fault for address: ffffcfb6200fd008
> ...
> [  883.803650] [   T1554] Call Trace:
> [  883.803652] [   T1554]  <TASK>
> [  883.803654] [   T1554]  igc_ptp_tx_tstamp_event+0xdf/0x160 [igc]
> [  883.803660] [   T1554]  igc_tsync_interrupt+0x2d5/0x300 [igc]
> ...
> 
> During shutdown of the TX ring the xsk_meta pointers are left behind, so
> that the IRQ handler is trying to touch them.
> 
> This issue is now being fixed by cleaning up the stale xsk meta data on
> TX shutdown. TX timestamps on other queues remain unaffected.

If you have the commands to reproduce this, that’d be great to have.

> Fixes: 15fd021bc427 ("igc: Add Tx hardware timestamp request for AF_XDP zero-copy packet")
> Signed-off-by: Zdenek Bouska <zdenek.bouska@siemens.com>
> ---
> Changes in v2:
> - Moved 'adapter' variable declaration into the if block (Vinicius)
> - Link to v1: https://lore.kernel.org/r/20260224-igc-fix-xdp-tx-tstamp-pagefault-v1-1-7c729ef61ee5@siemens.com
> ---
>   drivers/net/ethernet/intel/igc/igc.h      |  2 ++
>   drivers/net/ethernet/intel/igc/igc_main.c |  7 +++++++
>   drivers/net/ethernet/intel/igc/igc_ptp.c  | 33 +++++++++++++++++++++++++++++++
>   3 files changed, 42 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
> index a427f05814c1ae7330c6f7034cd0f2b40b74dab6..17236813965d334f14eba928affbd4f91b96ecd4 100644
> --- a/drivers/net/ethernet/intel/igc/igc.h
> +++ b/drivers/net/ethernet/intel/igc/igc.h
> @@ -781,6 +781,8 @@ int igc_ptp_hwtstamp_set(struct net_device *netdev,
>   			 struct kernel_hwtstamp_config *config,
>   			 struct netlink_ext_ack *extack);
>   void igc_ptp_tx_hang(struct igc_adapter *adapter);
> +void igc_ptp_clear_xsk_tx_tstamp_queue(struct igc_adapter *adapter,
> +				       u16 queue_id);
>   void igc_ptp_read(struct igc_adapter *adapter, struct timespec64 *ts);
>   void igc_ptp_tx_tstamp_event(struct igc_adapter *adapter);
>   
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 89a321a344d263ace5c66f7ade782b40cc482566..1931fcb659354d5009e0ea02316bf3a47b66b04d 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -264,6 +264,13 @@ static void igc_clean_tx_ring(struct igc_ring *tx_ring)
>   	/* reset next_to_use and next_to_clean */
>   	tx_ring->next_to_use = 0;
>   	tx_ring->next_to_clean = 0;
> +
> +	/* Clear any lingering XSK TX timestamp requests */
> +	if (test_bit(IGC_RING_FLAG_TX_HWTSTAMP, &tx_ring->flags)) {
> +		struct igc_adapter *adapter = netdev_priv(tx_ring->netdev);
> +
> +		igc_ptp_clear_xsk_tx_tstamp_queue(adapter, tx_ring->queue_index);
> +	}
>   }
>   
>   /**
> diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
> index 7aae83c108fd7611b00bf075592f93a902b83422..98491346d21b80925ce42ba276d851d4318e66b7 100644
> --- a/drivers/net/ethernet/intel/igc/igc_ptp.c
> +++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
> @@ -576,6 +576,39 @@ static void igc_ptp_clear_tx_tstamp(struct igc_adapter *adapter)
>   	spin_unlock_irqrestore(&adapter->ptp_tx_lock, flags);
>   }
>   
> +/**
> + * igc_ptp_clear_xsk_tx_tstamp_queue - Clear pending XSK TX timestamps for a queue
> + * @adapter: Board private structure
> + * @queue_id: TX queue index to clear timestamps for
> + *
> + * Iterates over all TX timestamp registers and releases any pending
> + * timestamp requests associated with the given TX queue. This is
> + * called when an XDP pool is being disabled to ensure no stale
> + * timestamp references remain.
> + */
> +void igc_ptp_clear_xsk_tx_tstamp_queue(struct igc_adapter *adapter, u16 queue_id)
> +{
> +	unsigned long flags;
> +	int i;
> +
> +	spin_lock_irqsave(&adapter->ptp_tx_lock, flags);
> +
> +	for (i = 0; i < IGC_MAX_TX_TSTAMP_REGS; i++) {
> +		struct igc_tx_timestamp_request *tstamp = &adapter->tx_tstamp[i];
> +
> +		if (tstamp->buffer_type != IGC_TX_BUFFER_TYPE_XSK)
> +			continue;
> +		if (tstamp->xsk_queue_index != queue_id)
> +			continue;
> +		if (!tstamp->xsk_tx_buffer)
> +			continue;
> +
> +		igc_ptp_free_tx_buffer(adapter, tstamp);
> +	}
> +
> +	spin_unlock_irqrestore(&adapter->ptp_tx_lock, flags);
> +}
> +
>   static void igc_ptp_disable_tx_timestamp(struct igc_adapter *adapter)
>   {
>   	struct igc_hw *hw = &adapter->hw;
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul