Message-ID: <02d40de4-5447-45bf-b839-f22a8f062388@intel.com>
Date: Mon, 25 Aug 2025 16:28:38 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Kurt Kanzenbach <kurt@...utronix.de>, Sebastian Andrzej Siewior
	<bigeasy@...utronix.de>
CC: Tony Nguyen <anthony.l.nguyen@...el.com>, Przemek Kitszel
	<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>, Vinicius Costa Gomes <vinicius.gomes@...el.com>,
	Paul Menzel <pmenzel@...gen.mpg.de>, Vadim Fedorenko
	<vadim.fedorenko@...ux.dev>, 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/23/2025 12:29 AM, Kurt Kanzenbach wrote:
> On Fri Aug 22 2025, Sebastian Andrzej Siewior wrote:
>> On 2025-08-22 09:28:10 [+0200], 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.
>>>
>>> Signed-off-by: Kurt Kanzenbach <kurt@...utronix.de>
>>> ---
>>> Changes in v2:
>>> - Switch from IRQ to PTP aux worker due to NTP performance regression (Miroslav)
>>> - Link to v1: https://lore.kernel.org/r/20250815-igb_irq_ts-v1-1-8c6fc0353422@linutronix.de
>>
>> For the i210 it makes sense to read it directly from IRQ avoiding the
>> context switch and the delay resulting for it. For the e1000_82576 it
>> makes sense to avoid the system workqueue and use a dedicated thread
>> which is not CPU bound and could prioritized/ isolated further if
>> needed.
>> I don't understand *why* reading the TS in IRQ is causing this packet
>> loss.
> 
> Me neither. I thought it could be the irqoff time. On my test systems
> the TS IRQ takes about ~16us with reading the timestamp. In the
> kworker/ptp aux thread scenario it takes about ~6us IRQ time + ~10us run
> time for the threads. All of that looks reasonable to me.
> 

Ya, I don't think we fully understand either. Miroslav said he tested on
I350 which is a different MAC from the I210, so it could be something
there. Theoretically we could handle just I210 directly in the interrupt
and leave the other variants to the kworker.. but I don't know how much
benefit we get from that. The data sheet for the I350 appears to have
more or less the same logic for Tx timestamps. It is significantly
different for Rx timestamps though.

> Also I couldn't really see a performance degradation with ntpperf. In my
> tests the IRQ variant reached an equal or higher rate. But sometimes I
> get 'Could not send requests at rate X'. No idea what that means.
> 
> Anyway, this patch is basically a compromise. It works for Miroslav and
> my use case.
> 
>> This is also what the igc does and the performance improved
>> 	afa141583d827 ("igc: Retrieve TX timestamp during interrupt handling")
>>

igc supports several hardware variations which are all a lot similar to
i210 than i350 is to i210 in igb. I could see this working fine for i210
if it works fine in igb.. I honestly am at a loss currently why i350 is
much worse.

>> and here it causes the opposite?
> 
> As said above, I'm out of ideas here.
> 

Same. It may be one of those things where the effort to dig up precisely
what has gone wrong is so large that it becomes not feasible relative to
the gain :(

> Thanks,
> Kurt



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

Powered by blists - more mailing lists