On Friday, September 19, 2025 12:03 AM, Paul Menzel <[email protected]> wrote: >Dear Chwee-Lin,
Dear Paul, > > >Thank you for your patch. > >Am 18.09.25 um 20:38 schrieb Chwee-Lin Choong: >> The current HW bug workaround checks the TXTT_0 ready bit first, then >> reads LOW -> HIGH -> LOW from register 0 to detect if a timestamp was >> captured. >> >> This sequence has a race: if a new timestamp is latched after reading >> the TXTT mask but before the first LOW read, both old and new >> timestamp match, causing the driver to drop a valid timestamp. > >Reading the TXTT mask is `rd32(IGC_TSYNCTXCTL)`, correct? > Yes, rd32(IGC_TSYNCTXCTL) is the read of the TXTT mask (bits like TXTT_0, TXTT_ANY). >> Fix by reading the LOW register first, then the TXTT mask, so a newly >> latched timestamp will always be detected. >> >> This fix also prevents TX unit hangs observed under heavy timestamping >> load. > >The unit shouldn’t hang, even if valid timestamps are dropped? > This only happens if TX timestamping is requested on an AF_XDP zero-copy queue. In that case the driver holds the TX completion until the timestamp is ready (or until IGC_PTP_TX_TIMEOUT, 15s). Example debug output: [146320.288672] igc 0000:01:00.0 enp1s0: Blocking cleanup: XSK timestamp pending on desc idx=0, age=15889 ms [146320.288681] igc 0000:01:00.0 enp1s0: Tx timestamp timeout [146320.288699] igc 0000:01:00.0 enp1s0: Detected Tx Unit Hang [146320.288699] Tx Queue <0> [146320.288699] TDH <d61> [146320.288699] TDT <d63> [146320.288699] next_to_use <d63> [146320.288699] next_to_clean <de3> [146320.288699] buffer_info[next_to_clean] [146320.288699] time_stamp <108b3fb2f> [146320.288699] next_to_watch <000000002c34ce76> [146320.288699] jiffies <108b438c0> [146320.288699] desc.status <200001> >Do you have a reproducer? > The issue can be reproduced with the Linutronix RTC-testbench, e.g. by running a Profinet test with ptp4l (non-XDP) running concurrently with TsnHigh traffic on an AF_XDP zero-copy queue with TX hardware timestamping enabled. https://github.com/Linutronix/RTC-Testbench >> Fixes: c789ad7cbebc ("igc: Work around HW bug causing missing >> timestamps") >> Suggested-by: Avi Shalev <[email protected]> >> Signed-off-by: Song Yoong Siang <[email protected]> >> Signed-off-by: Chwee-Lin Choong <[email protected]> >> --- >> drivers/net/ethernet/intel/igc/igc_ptp.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c >> b/drivers/net/ethernet/intel/igc/igc_ptp.c >> index b7b46d863bee..930486b02fc1 100644 >> --- a/drivers/net/ethernet/intel/igc/igc_ptp.c >> +++ b/drivers/net/ethernet/intel/igc/igc_ptp.c >> @@ -774,10 +774,17 @@ static void igc_ptp_tx_reg_to_stamp(struct >igc_adapter *adapter, >> static void igc_ptp_tx_hwtstamp(struct igc_adapter *adapter) >> { >> struct igc_hw *hw = &adapter->hw; >> + u32 txstmpl_old; >> u64 regval; >> u32 mask; >> int i; >> >> + /* Read the "low" register 0 first to establish a baseline value. >> + * This avoids a race where a new timestamp could be latched >> + * after checking the TXTT mask. >> + */ >> + txstmpl_old = rd32(IGC_TXSTMPL); >> + >> mask = rd32(IGC_TSYNCTXCTL) & IGC_TSYNCTXCTL_TXTT_ANY; >> if (mask & IGC_TSYNCTXCTL_TXTT_0) { >> regval = rd32(IGC_TXSTMPL); >> @@ -801,9 +808,8 @@ static void igc_ptp_tx_hwtstamp(struct igc_adapter >*adapter) >> * timestamp was captured, we can read the "high" >> * register again. >> */ >> - u32 txstmpl_old, txstmpl_new; >> + u32 txstmpl_new; >> >> - txstmpl_old = rd32(IGC_TXSTMPL); >> rd32(IGC_TXSTMPH); >> txstmpl_new = rd32(IGC_TXSTMPL); >> > >Kind regards, > >Paul Best regards, CL
