RE: [Intel-wired-lan] [PATCH iwl-net v1] igc: fix race condition in TX timestamp read for register 0

[Choong, Chwee Lin]

On Friday, September 19, 2025 12:03 AM, Paul Menzel <pmenzel@xxxxxxxxxxxxx> 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 <avi.shalev@xxxxxxxxx>
>> Signed-off-by: Song Yoong Siang <yoong.siang.song@xxxxxxxxx>
>> Signed-off-by: Chwee-Lin Choong <chwee.lin.choong@xxxxxxxxx>
>> ---
>>   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