netdev - Re: [PATCH iwl-next v2] igb: Convert Tx timestamping to PTP aux worker

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <87ikie7a88.fsf@jax.kurt.home>
Date: Sat, 23 Aug 2025 09:44:55 +0200
From: Kurt Kanzenbach <kurt@...utronix.de>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>, Tony Nguyen
 <anthony.l.nguyen@...el.com>, Przemek Kitszel
 <przemyslaw.kitszel@...el.com>
Cc: 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>, Vinicius Costa Gomes
 <vinicius.gomes@...el.com>, Sebastian Andrzej Siewior
 <bigeasy@...utronix.de>, Paul Menzel <pmenzel@...gen.mpg.de>, Miroslav
 Lichvar <mlichvar@...hat.com>, Jacob Keller <jacob.e.keller@...el.com>,
 intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org
Subject: Re: [PATCH iwl-next v2] igb: Convert Tx timestamping to PTP aux worker

On Fri Aug 22 2025, Vadim Fedorenko wrote:
> On 22/08/2025 08:28, Kurt Kanzenbach wrote:
>> 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.
>> 
>>    * 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;
>
> do_aux_work is expected to return delay in jiffies to re-schedule the
> work. it would be cleaner to use msec_to_jiffies macros to tell how much
> time the driver has to wait before the next try of retrieving the
> timestamp. AFAIU, the timestamp may come ASAP in this case, so it's
> actually reasonable to request immediate re-schedule of the worker by
> returning 0.

I don't think so. The 'return 1' is only executed for 82576. For all
other NICs the TS is already available. For 82576 the packet is queued
to Tx ring and the worker is scheduled immediately. For example, in case
of other Tx traffic there's a chance that the TS is not available
yet. So we comeback one jiffy later, which is 10ms at worst. That looked
reasonable to me.

Thanks,
Kurt

Download attachment "signature.asc" of type "application/pgp-signature" (862 bytes)