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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1baf249d-c9c4-4952-a00f-6294dbb71794@intel.com>
Date: Mon, 25 Aug 2025 16:24:37 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>, Kurt Kanzenbach
	<kurt@...utronix.de>, 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>,
	<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 8/25/2025 6:18 AM, Vadim Fedorenko wrote:
> On 23/08/2025 08:44, Kurt Kanzenbach wrote:
>> 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.
> 
> Ok, LGTM
> Reviewed-by: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
> 

Agreed.

Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>


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