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: <IA3PR11MB898642CE8EA4BBA755E76812E53EA@IA3PR11MB8986.namprd11.prod.outlook.com>
Date: Mon, 25 Aug 2025 10:58:40 +0000
From: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>
To: Kurt Kanzenbach <kurt@...utronix.de>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>
CC: Paul Menzel <pmenzel@...gen.mpg.de>, Vadim Fedorenko
	<vadim.fedorenko@...ux.dev>, "Gomes, Vinicius" <vinicius.gomes@...el.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, Richard Cochran
	<richardcochran@...il.com>, Andrew Lunn <andrew+netdev@...n.ch>, Eric Dumazet
	<edumazet@...gle.com>, "intel-wired-lan@...ts.osuosl.org"
	<intel-wired-lan@...ts.osuosl.org>, "Keller, Jacob E"
	<jacob.e.keller@...el.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, "David S. Miller" <davem@...emloft.net>, "Sebastian
 Andrzej Siewior" <bigeasy@...utronix.de>
Subject: RE: [Intel-wired-lan] [PATCH iwl-next v2] igb: Convert Tx
 timestamping to PTP aux worker



> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf
> Of Kurt Kanzenbach
> Sent: Friday, August 22, 2025 9:28 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@...el.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@...el.com>
> Cc: Paul Menzel <pmenzel@...gen.mpg.de>; Vadim Fedorenko
> <vadim.fedorenko@...ux.dev>; Gomes, Vinicius
> <vinicius.gomes@...el.com>; netdev@...r.kernel.org; Richard Cochran
> <richardcochran@...il.com>; Kurt Kanzenbach <kurt@...utronix.de>;
> Andrew Lunn <andrew+netdev@...n.ch>; Eric Dumazet
> <edumazet@...gle.com>; intel-wired-lan@...ts.osuosl.org; Keller, Jacob
> E <jacob.e.keller@...el.com>; Jakub Kicinski <kuba@...nel.org>; Paolo
> Abeni <pabeni@...hat.com>; David S. Miller <davem@...emloft.net>;
> Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> Subject: [Intel-wired-lan] [PATCH iwl-next v2] igb: Convert Tx
> timestamping to PTP aux worker
> 
> 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, switch to the PTP aux worker which can be prioritized and
> pinned according to use case. Tested on Intel i210.
> 
> Signed-off-by: Kurt Kanzenbach <kurt@...utronix.de>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>


> ---
> Changes in v2:
> - Switch from IRQ to PTP aux worker due to NTP performance regression
> (Miroslav)
> - Link to v1: https://lore.kernel.org/r/20250815-igb_irq_ts-v1-1-
> 8c6fc0353422@...utronix.de
> ---
>  drivers/net/ethernet/intel/igb/igb.h      |  1 -
>  drivers/net/ethernet/intel/igb/igb_main.c |  6 +++---
> drivers/net/ethernet/intel/igb/igb_ptp.c  | 28 +++++++++++++++--------
> -----
>  3 files changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb.h
> b/drivers/net/ethernet/intel/igb/igb.h
> index
> c3f4f7cd264e9b2ff70f03b580f95b15b528028c..f285def61f7a778f66944d6c52bb
> 31f11ff803cf 100644
> --- a/drivers/net/ethernet/intel/igb/igb.h
> +++ b/drivers/net/ethernet/intel/igb/igb.h
> @@ -624,7 +624,6 @@ struct igb_adapter {
>  	struct ptp_clock *ptp_clock;
>  	struct ptp_clock_info ptp_caps;
>  	struct delayed_work ptp_overflow_work;
> -	struct work_struct ptp_tx_work;
>  	struct sk_buff *ptp_tx_skb;
>  	struct kernel_hwtstamp_config tstamp_config;
>  	unsigned long ptp_tx_start;
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> b/drivers/net/ethernet/intel/igb/igb_main.c
> index
> a9a7a94ae61e93aa737b0103e00580e73601d62b..76467f0e53305188fcbbff27e21e
> 478e764ca552 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -6576,7 +6576,7 @@ netdev_tx_t igb_xmit_frame_ring(struct sk_buff
> *skb,
>  			adapter->ptp_tx_skb = skb_get(skb);
>  			adapter->ptp_tx_start = jiffies;
>  			if (adapter->hw.mac.type == e1000_82576)
> -				schedule_work(&adapter->ptp_tx_work);
> +				ptp_schedule_worker(adapter->ptp_clock, 0);
>  		} else {
>  			adapter->tx_hwtstamp_skipped++;
>  		}
> @@ -6612,7 +6612,7 @@ netdev_tx_t igb_xmit_frame_ring(struct sk_buff
> *skb,
>  		dev_kfree_skb_any(adapter->ptp_tx_skb);
>  		adapter->ptp_tx_skb = NULL;
>  		if (adapter->hw.mac.type == e1000_82576)
> -			cancel_work_sync(&adapter->ptp_tx_work);
> +			ptp_cancel_worker_sync(adapter->ptp_clock);
>  		clear_bit_unlock(__IGB_PTP_TX_IN_PROGRESS, &adapter-
> >state);
>  	}
> 
> @@ -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);
> +		ptp_schedule_worker(adapter->ptp_clock, 0);
>  	}
> 
>  	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..8dabde01d09dcacc13e19fa4ce7a
> d0327077190a 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
> @@ -798,20 +798,20 @@ static int igb_ptp_verify_pin(struct
> ptp_clock_info *ptp, unsigned int pin,
> 
>  /**
>   * igb_ptp_tx_work
> - * @work: pointer to work struct
> + * @ptp: pointer to ptp clock information
>   *
>   * This work function polls the TSYNCTXCTL valid bit to determine
> when a
>   * timestamp has been taken for the current stored skb.
>   **/
> -static void igb_ptp_tx_work(struct work_struct *work)
> +static long igb_ptp_tx_work(struct ptp_clock_info *ptp)
>  {
> -	struct igb_adapter *adapter = container_of(work, struct
> igb_adapter,
> -						   ptp_tx_work);
> +	struct igb_adapter *adapter = container_of(ptp, struct
> igb_adapter,
> +						   ptp_caps);
>  	struct e1000_hw *hw = &adapter->hw;
>  	u32 tsynctxctl;
> 
>  	if (!adapter->ptp_tx_skb)
> -		return;
> +		return -1;
> 
>  	if (time_is_before_jiffies(adapter->ptp_tx_start +
>  				   IGB_PTP_TX_TIMEOUT)) {
> @@ -824,15 +824,17 @@ static void igb_ptp_tx_work(struct work_struct
> *work)
>  		 */
>  		rd32(E1000_TXSTMPH);
>  		dev_warn(&adapter->pdev->dev, "clearing Tx timestamp
> hang\n");
> -		return;
> +		return -1;
>  	}
> 
>  	tsynctxctl = rd32(E1000_TSYNCTXCTL);
> -	if (tsynctxctl & E1000_TSYNCTXCTL_VALID)
> +	if (tsynctxctl & E1000_TSYNCTXCTL_VALID) {
>  		igb_ptp_tx_hwtstamp(adapter);
> -	else
> -		/* reschedule to check later */
> -		schedule_work(&adapter->ptp_tx_work);
> +		return -1;
> +	}
> +
> +	/* reschedule to check later */
> +	return 1;
>  }
> 
>  static void igb_ptp_overflow_check(struct work_struct *work) @@ -
> 915,7 +917,7 @@ void igb_ptp_tx_hang(struct igb_adapter *adapter)
>  	 * timestamp bit when this occurs.
>  	 */
>  	if (timeout) {
> -		cancel_work_sync(&adapter->ptp_tx_work);
> +		ptp_cancel_worker_sync(adapter->ptp_clock);
>  		dev_kfree_skb_any(adapter->ptp_tx_skb);
>  		adapter->ptp_tx_skb = NULL;
>  		clear_bit_unlock(__IGB_PTP_TX_IN_PROGRESS, &adapter-
> >state); @@ -1381,6 +1383,7 @@ void igb_ptp_init(struct igb_adapter
> *adapter)
>  		return;
>  	}
> 
> +	adapter->ptp_caps.do_aux_work = igb_ptp_tx_work;
>  	adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps,
>  						&adapter->pdev->dev);
>  	if (IS_ERR(adapter->ptp_clock)) {
> @@ -1392,7 +1395,6 @@ void igb_ptp_init(struct igb_adapter *adapter)
>  		adapter->ptp_flags |= IGB_PTP_ENABLED;
> 
>  		spin_lock_init(&adapter->tmreg_lock);
> -		INIT_WORK(&adapter->ptp_tx_work, igb_ptp_tx_work);
> 
>  		if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
>  			INIT_DELAYED_WORK(&adapter->ptp_overflow_work,
> @@ -1437,7 +1439,7 @@ void igb_ptp_suspend(struct igb_adapter
> *adapter)
>  	if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
>  		cancel_delayed_work_sync(&adapter->ptp_overflow_work);
> 
> -	cancel_work_sync(&adapter->ptp_tx_work);
> +	ptp_cancel_worker_sync(adapter->ptp_clock);
>  	if (adapter->ptp_tx_skb) {
>  		dev_kfree_skb_any(adapter->ptp_tx_skb);
>  		adapter->ptp_tx_skb = NULL;
> 
> ---
> base-commit: a7bd72158063740212344fad5d99dcef45bc70d6
> change-id: 20250813-igb_irq_ts-1aa77cc7b4cb
> 
> Best regards,
> --
> Kurt Kanzenbach <kurt@...utronix.de>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ