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: <aefa86a1-8959-4e5f-8203-78ce4c50b3bd@molgen.mpg.de>
Date: Wed, 27 Aug 2025 15:39:34 +0200
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Kurt Kanzenbach <kurt@...utronix.de>
Cc: Jacob Keller <jacob.e.keller@...el.com>,
 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

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