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

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <SJ5PPF4422C5374EA58BEFC6B6CA1173E00DA11A@SJ5PPF4422C5374.namprd11.prod.outlook.com>
Date: Fri, 19 Sep 2025 06:16:13 +0000
From: "Choong, Chwee Lin" <chwee.lin.choong@...el.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>
CC: "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>, Andrew Lunn <andrew+netdev@...n.ch>, "David S
 . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, "Jakub
 Kicinski" <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Richard Cochran
	<richardcochran@...il.com>, "Gomes, Vinicius" <vinicius.gomes@...el.com>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Shalev, Avi"
	<avi.shalev@...el.com>, "Song, Yoong Siang" <yoong.siang.song@...el.com>
Subject: RE: [Intel-wired-lan] [PATCH iwl-net v1] igc: fix race condition in
 TX timestamp read for register 0


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