RE: [Intel-wired-lan] [PATCH iwl-next v4] igb: Retrieve Tx timestamp from BH workqueue

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@xxxxxxxxxx> On Behalf
> Of Kurt Kanzenbach
> Sent: Tuesday, March 3, 2026 12:49 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@xxxxxxxxx>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@xxxxxxxxx>
> Cc: Paul Menzel <pmenzel@xxxxxxxxxxxxx>; Vadim Fedorenko
> <vadim.fedorenko@xxxxxxxxx>; Gomes, Vinicius
> <vinicius.gomes@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; Richard Cochran
> <richardcochran@xxxxxxxxx>; Kurt Kanzenbach <kurt@xxxxxxxxxxxxx>;
> linux-kernel@xxxxxxxxxxxxxxx; Andrew Lunn <andrew+netdev@xxxxxxx>;
> Eric Dumazet <edumazet@xxxxxxxxxx>; intel-wired-lan@xxxxxxxxxxxxxxxx;
> Keller, Jacob E <jacob.e.keller@xxxxxxxxx>; Jakub Kicinski
> <kuba@xxxxxxxxxx>; Paolo Abeni <pabeni@xxxxxxxxxx>; David S. Miller
> <davem@xxxxxxxxxxxxx>; Sebastian Andrzej Siewior
> <bigeasy@xxxxxxxxxxxxx>
> Subject: [Intel-wired-lan] [PATCH iwl-next v4] igb: Retrieve Tx
> timestamp from BH workqueue
> 
> Retrieve Tx timestamp from system BH instead of regular system
> workqueue.
> 
> The current implementation uses schedule_work() which is executed by
> the system work queue and kworkers to retrieve Tx timestamps. This
> increases latency and can lead to timeouts in case of heavy system
> load. i210 is often used in industrial systems, where timestamp
> timeouts can be fatal.
> 
> Therefore, switch to the system BH workqueues which are executed
> directly at the end of the IRQ handler.
> 
I'd recommend "executed directly at the end of the IRQ handler" -> "executed in softirq context shortly after the IRQ handler returns"
But anyway.
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@xxxxxxxxx>

> Tested on Intel i210 and i350 with ptp4l.
> 
> Signed-off-by: Kurt Kanzenbach <kurt@xxxxxxxxxxxxx>
> ---
> Changes in v4:
> - Use BH workqueue (tasklet) instead of doing timestamping in IRQ path
> (Jakub Kicinski)
> - Link to v3: https://patch.msgid.link/20260205-igb_irq_ts-v3-1-
> 2efc7bc4b885@xxxxxxxxxxxxx
> 
> Changes in v3:
> - Switch back to IRQ, but for i210 only
> - Keep kworker for all other NICs like i350 (Miroslav)
> - Link to v2: https://lore.kernel.org/r/20250822-igb_irq_ts-v2-1-
> 1ac37078a7a4@xxxxxxxxxxxxx
> 
> 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@xxxxxxxxxxxxx
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 4 ++--
> drivers/net/ethernet/intel/igb/igb_ptp.c  | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> b/drivers/net/ethernet/intel/igb/igb_main.c
> index ee99fd8fd513..9fd29fedb9f5 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -6572,7 +6572,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);
> +				queue_work(system_bh_wq, &adapter-
> >ptp_tx_work);
>  		} else {
>  			adapter->tx_hwtstamp_skipped++;
>  		}
> @@ -7076,7 +7076,7 @@ static void igb_tsync_interrupt(struct
> igb_adapter *adapter)
> 
>  	if (tsicr & E1000_TSICR_TXTS) {
>  		/* retrieve hardware timestamp */
> -		schedule_work(&adapter->ptp_tx_work);
> +		queue_work(system_bh_wq, &adapter->ptp_tx_work);
>  	}
> 
>  	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 bd85d02ecadd..7b44f9090631 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
> @@ -832,7 +832,7 @@ static void igb_ptp_tx_work(struct work_struct
> *work)
>  		igb_ptp_tx_hwtstamp(adapter);
>  	else
>  		/* reschedule to check later */
> -		schedule_work(&adapter->ptp_tx_work);
> +		queue_work(system_bh_wq, &adapter->ptp_tx_work);
>  }
> 
>  static void igb_ptp_overflow_check(struct work_struct *work)
> 
> ---
> base-commit: a0e8c9a5060fbdb72fca767164467a3cf2b8fc30
> change-id: 20250813-igb_irq_ts-1aa77cc7b4cb
> 
> Best regards,
> --
> Kurt Kanzenbach <kurt@xxxxxxxxxxxxx>