netdev - Re: [Intel-wired-lan] [PATCH iwl-next] igb: Retrieve Tx timestamp directly from interrupt

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <a1e9e37e-63da-4f1c-8ac3-36e1fde2ec0a@molgen.mpg.de>
Date: Fri, 15 Aug 2025 09:55:00 +0200
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Kurt Kanzenbach <kurt@...utronix.de>
Cc: 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>,
 Richard Cochran <richardcochran@...il.com>,
 Vinicius Costa Gomes <vinicius.gomes@...el.com>,
 Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
 intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org
Subject: Re: [Intel-wired-lan] [PATCH iwl-next] igb: Retrieve Tx timestamp
 directly from interrupt

Dear Kurt,


Thank you for your patch.

Am 15.08.25 um 08:50 schrieb Kurt Kanzenbach:
> Retrieve Tx timestamp directly from interrupt handler.
> 
> The current implementation uses schedule_work() which is executed by the
> system work queue to retrieve Tx timestamps. This increases latency and can
> lead to timeouts in case of heavy system load.
> 
> Therefore, fetch the timestamp directly from the interrupt handler.
> 
> The work queue code stays for the Intel 82576. Tested on Intel i210.

Excuse my ignorance, I do not understand the first sentence in the last 
line. Is it because the driver support different models? Why not change 
it for Intel 82576 too?

Do you have a reproducer for the issue, so others can test.

> Signed-off-by: Kurt Kanzenbach <kurt@...utronix.de>
> ---
>   drivers/net/ethernet/intel/igb/igb.h      |  1 +
>   drivers/net/ethernet/intel/igb/igb_main.c |  2 +-
>   drivers/net/ethernet/intel/igb/igb_ptp.c  | 22 ++++++++++++++++++++++
>   3 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
> index c3f4f7cd264e9b2ff70f03b580f95b15b528028c..102ca32e8979fa3203fc2ea36eac456f1943cfca 100644
> --- a/drivers/net/ethernet/intel/igb/igb.h
> +++ b/drivers/net/ethernet/intel/igb/igb.h
> @@ -776,6 +776,7 @@ int igb_ptp_hwtstamp_get(struct net_device *netdev,
>   int igb_ptp_hwtstamp_set(struct net_device *netdev,
>   			 struct kernel_hwtstamp_config *config,
>   			 struct netlink_ext_ack *extack);
> +void igb_ptp_tx_tstamp_event(struct igb_adapter *adapter);
>   void igb_set_flag_queue_pairs(struct igb_adapter *, const u32);
>   unsigned int igb_get_max_rss_queues(struct igb_adapter *);
>   #ifdef CONFIG_IGB_HWMON
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index a9a7a94ae61e93aa737b0103e00580e73601d62b..8ab6e52cb839bbb698007a74462798faaaab0071 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -7080,7 +7080,7 @@ static void igb_tsync_interrupt(struct igb_adapter *adapter)
>   
>   	if (tsicr & E1000_TSICR_TXTS) {
>   		/* retrieve hardware timestamp */
> -		schedule_work(&adapter->ptp_tx_work);
> +		igb_ptp_tx_tstamp_event(adapter);
>   	}
>   
>   	if (tsicr & TSINTR_TT0)
> diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
> index a7876882aeaf2b2a7fb9ec6ff5c83d8a1b06008a..20ecafecc60557353f8cc5ab505030246687c8e4 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
> @@ -796,6 +796,28 @@ static int igb_ptp_verify_pin(struct ptp_clock_info *ptp, unsigned int pin,
>   	return 0;
>   }
>   
> +/**
> + * igb_ptp_tx_tstamp_event
> + * @adapter: pointer to igb adapter
> + *
> + * This function checks the TSYNCTXCTL valid bit and stores the Tx hardware
> + * timestamp at the current skb.
> + **/
> +void igb_ptp_tx_tstamp_event(struct igb_adapter *adapter)
> +{
> +	struct e1000_hw *hw = &adapter->hw;
> +	u32 tsynctxctl;
> +
> +	if (!adapter->ptp_tx_skb)
> +		return;
> +
> +	tsynctxctl = rd32(E1000_TSYNCTXCTL);
> +	if (WARN_ON_ONCE(!(tsynctxctl & E1000_TSYNCTXCTL_VALID)))
> +		return;
> +
> +	igb_ptp_tx_hwtstamp(adapter);
> +}
> +
>   /**
>    * igb_ptp_tx_work
>    * @work: pointer to work struct

The diff looks fine.

Reviewed-by: Paul Menzel <pmenzel@...gen.mpg.de>


Kind regards,

Paul