All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kurt Kanzenbach <kurt@linutronix.de>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Paul Menzel <pmenzel@molgen.mpg.de>,
	Vadim Fedorenko <vadim.fedorenko@linux.dev>,
	Vinicius Costa Gomes <vinicius.gomes@intel.com>,
	Przemek Kitszel <przemyslaw.kitszel@intel.com>,
	Richard Cochran <richardcochran@gmail.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	Eric Dumazet <edumazet@google.com>,
	netdev@vger.kernel.org, Tony Nguyen <anthony.l.nguyen@intel.com>,
	intel-wired-lan@lists.osuosl.org,
	Jacob Keller <jacob.e.keller@intel.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v2] igb: Convert Tx timestamping to PTP aux worker
Date: Sat, 23 Aug 2025 09:29:36 +0200	[thread overview]
Message-ID: <87ldna7axr.fsf@jax.kurt.home> (raw)
In-Reply-To: <20250822075200.L8_GUnk_@linutronix.de>

[-- Attachment #1: Type: text/plain, Size: 1918 bytes --]

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@linutronix.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.

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")
>
> and here it causes the opposite?

As said above, I'm out of ideas here.

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Kurt Kanzenbach <kurt@linutronix.de>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>,
	Przemek Kitszel <przemyslaw.kitszel@intel.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Richard Cochran <richardcochran@gmail.com>,
	Vinicius Costa Gomes <vinicius.gomes@intel.com>,
	Paul Menzel <pmenzel@molgen.mpg.de>,
	Vadim Fedorenko <vadim.fedorenko@linux.dev>,
	Miroslav Lichvar <mlichvar@redhat.com>,
	Jacob Keller <jacob.e.keller@intel.com>,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org
Subject: Re: [PATCH iwl-next v2] igb: Convert Tx timestamping to PTP aux worker
Date: Sat, 23 Aug 2025 09:29:36 +0200	[thread overview]
Message-ID: <87ldna7axr.fsf@jax.kurt.home> (raw)
In-Reply-To: <20250822075200.L8_GUnk_@linutronix.de>

[-- Attachment #1: Type: text/plain, Size: 1918 bytes --]

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@linutronix.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.

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")
>
> and here it causes the opposite?

As said above, I'm out of ideas here.

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

  parent reply	other threads:[~2025-08-23  7:29 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-22  7:28 [Intel-wired-lan] [PATCH iwl-next v2] igb: Convert Tx timestamping to PTP aux worker Kurt Kanzenbach
2025-08-22  7:28 ` Kurt Kanzenbach
2025-08-22  7:52 ` [Intel-wired-lan] " Sebastian Andrzej Siewior
2025-08-22  7:52   ` Sebastian Andrzej Siewior
2025-08-22 23:55   ` [Intel-wired-lan] " Jacob Keller
2025-08-22 23:55     ` Jacob Keller
2025-08-23  7:29   ` Kurt Kanzenbach [this message]
2025-08-23  7:29     ` Kurt Kanzenbach
2025-08-25  7:53     ` [Intel-wired-lan] " Miroslav Lichvar
2025-08-25  7:53       ` Miroslav Lichvar
2025-08-25  9:22       ` [Intel-wired-lan] " Kurt Kanzenbach
2025-08-25  9:22         ` Kurt Kanzenbach
2025-08-25 23:23         ` [Intel-wired-lan] " Jacob Keller
2025-08-25 23:28     ` Jacob Keller
2025-08-25 23:28       ` Jacob Keller
2025-08-26 12:59       ` [Intel-wired-lan] " Sebastian Andrzej Siewior
2025-08-26 12:59         ` Sebastian Andrzej Siewior
2025-08-26 18:23         ` [Intel-wired-lan] " Jacob Keller
2025-08-26 18:23           ` Jacob Keller
2025-08-27 12:57           ` [Intel-wired-lan] " Kurt Kanzenbach
2025-08-27 12:57             ` Kurt Kanzenbach
2025-08-27 13:39             ` [Intel-wired-lan] " Paul Menzel
2025-08-27 13:39               ` Paul Menzel
2025-08-27 16:22               ` [Intel-wired-lan] " Jacob Keller
2025-08-27 16:22                 ` Jacob Keller
2025-08-27 13:57         ` [Intel-wired-lan] " Miroslav Lichvar
2025-08-27 13:57           ` Miroslav Lichvar
2025-08-27 14:05           ` [Intel-wired-lan] " Kurt Kanzenbach
2025-08-27 14:05             ` Kurt Kanzenbach
2025-08-27 14:10           ` [Intel-wired-lan] " Sebastian Andrzej Siewior
2025-08-27 14:10             ` Sebastian Andrzej Siewior
2025-08-27 14:41             ` [Intel-wired-lan] " Miroslav Lichvar
2025-08-27 14:41               ` Miroslav Lichvar
2025-08-27 14:52               ` [Intel-wired-lan] " Sebastian Andrzej Siewior
2025-08-27 14:52                 ` Sebastian Andrzej Siewior
2025-08-27 16:21                 ` [Intel-wired-lan] " Jacob Keller
2025-08-27 16:21                   ` Jacob Keller
2025-08-22 16:27 ` [Intel-wired-lan] " Vadim Fedorenko
2025-08-22 16:27   ` Vadim Fedorenko
2025-08-23  7:44   ` [Intel-wired-lan] " Kurt Kanzenbach
2025-08-23  7:44     ` Kurt Kanzenbach
2025-08-25 13:18     ` [Intel-wired-lan] " Vadim Fedorenko
2025-08-25 13:18       ` Vadim Fedorenko
2025-08-25 23:24       ` [Intel-wired-lan] " Jacob Keller
2025-08-25 23:24         ` Jacob Keller
2025-08-25 10:58 ` [Intel-wired-lan] " Loktionov, Aleksandr
2025-08-25 10:58   ` Loktionov, Aleksandr

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ldna7axr.fsf@jax.kurt.home \
    --to=kurt@linutronix.de \
    --cc=andrew+netdev@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=bigeasy@linutronix.de \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jacob.e.keller@intel.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pmenzel@molgen.mpg.de \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=richardcochran@gmail.com \
    --cc=vadim.fedorenko@linux.dev \
    --cc=vinicius.gomes@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.