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: <20250826125912.q0OhVCZJ@linutronix.de>
Date: Tue, 26 Aug 2025 14:59:12 +0200
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: Kurt Kanzenbach <kurt@...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>,
	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 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.

> > 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?

> > Thanks,
> > Kurt

Sebastian