> -----Original Message----- > From: Intel-wired-lan <[email protected]> On Behalf > Of Kurt Kanzenbach > Sent: Friday, August 22, 2025 9:28 AM > To: Nguyen, Anthony L <[email protected]>; Kitszel, > Przemyslaw <[email protected]> > Cc: Paul Menzel <[email protected]>; Vadim Fedorenko > <[email protected]>; Gomes, Vinicius > <[email protected]>; [email protected]; Richard Cochran > <[email protected]>; Kurt Kanzenbach <[email protected]>; > Andrew Lunn <[email protected]>; Eric Dumazet > <[email protected]>; [email protected]; Keller, Jacob > E <[email protected]>; Jakub Kicinski <[email protected]>; Paolo > Abeni <[email protected]>; David S. Miller <[email protected]>; > Sebastian Andrzej Siewior <[email protected]> > 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 <[email protected]> Reviewed-by: Aleksandr Loktionov <[email protected]>
> --- > 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- > [email protected] > --- > 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 <[email protected]>
