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: <509af4ec-93b0-44b4-a10f-abcab029617d@intel.com>
Date: Wed, 27 Aug 2025 09:22:29 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>, Kurt Kanzenbach <kurt@...utronix.de>
CC: Sebastian Andrzej Siewior <bigeasy@...utronix.de>, 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>, 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/27/2025 6:39 AM, Paul Menzel wrote:
> Dear Linux folks,
> 
> 
> A very interesting issue.
> 
> Am 27.08.25 um 14:57 schrieb Kurt Kanzenbach:
>> On Tue Aug 26 2025, Jacob Keller wrote:
>>> On 8/26/2025 5:59 AM, Sebastian Andrzej Siewior wrote:
>>>> On 2025-08-25 16:28:38 [-0700], Jacob Keller wrote:
>>>>> 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.
>>>>
>>>> From logical point of view it makes sense to retrieve the HW timestamp
>>>> immediately when it becomes available and feed it to the stack. I can't
>>>> imagine how delaying it to yet another thread improves the situation.
>>>> The benchmark is about > 1k packets/ second while in reality you have
>>>> less than 20 packets a second. With multiple applications you usually
>>>> need a "second timestamp register" or you may lose packets.
>>>>
>>>> Delaying it to the AUX worker makes sense for hardware which can't fire
>>>> an interrupt and polling is the only option left. This is sane in this
>>>> case but I don't like this solution as some kind compromise for
>>>> everyone. Simply because it adds overhead and requires additional
>>>> configuration.
>>>
>>> I agree. Its just frustrating that doing so appears to cause a
>>> regression in at least one test setup on hardware which uses this method.
>>>
>>>>>> 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 :(
>>>>
>>>> Could we please use the direct retrieval/ submission for HW which
>>>> supports it and fallback to the AUX worker (instead of the kworker) for
>>>> HW which does not have an interrupt for it?
>>>
>>> I have no objection. Perhaps we could assume the high end of the ntpperf
>>> benchmark is not reflective of normal use case? We *are* limited to only
>>> one timestamp register, which the igb driver does protect by bitlock.
>>
>> Does that mean we're going back to v1 + the AUX worker for 82576? Let me
>> prepare v3 then.
> 
> Good question. Personally, I’d interpret Linux’ no-regression-policy 
> that, if a possible regression is known, even for a synthetic benchmark, 
> it must not be introduced unrelated how upsetting this is. So the 
> current approach needs to be taken.
> 
> 
> Kind regards,
> 
> Paul

Another option is a v3 with AUX worker for 82575 and i350, but direct
in-interrupt for i210 since so far that doesn't seem to have a regression.

Or perhaps Sebastian can figure out something further about the
reproduction.


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