Message-ID: <CADEbmW0roa=NKwB2kE7DJ0n_W5=Rqk1LJu3kri4u1Rkc8h-KvA@mail.gmail.com>
Date: Mon, 26 Jan 2026 09:30:29 +0100
From: Michal Schmidt <mschmidt@...hat.com>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: Intel Wired LAN <intel-wired-lan@...ts.osuosl.org>, 
	Przemyslaw Korba <przemyslaw.korba@...el.com>, netdev@...r.kernel.org, 
	Aleksandr Loktionov <aleksandr.loktionov@...el.com>, Anthony Nguyen <anthony.l.nguyen@...el.com>
Subject: Re: [PATCH iwl-net] ice: PTP: fix missing timestamps on E825 hardware

On Wed, Jan 21, 2026 at 8:01 PM Jacob Keller <jacob.e.keller@...el.com> wrote:
> The E825 hardware currently has each PF handle the PFINT_TSYN_TX cause of
> the miscellaneous OICR interrupt vector. The actual interrupt cause
> underlying this is shared by all ports on the same quad:
>
>   ┌─────────────────────────────────┐
>   │                                 │
>   │   ┌────┐ ┌────┐ ┌────┐ ┌────┐   │
>   │   │PF 0│ │PF 1│ │PF 2│ │PF 3│   │
>   │   └────┘ └────┘ └────┘ └────┘   │
>   │                                 │
>   └────────────────▲────────────────┘
>                    │
>                    │
>   ┌────────────────┼────────────────┐
>   │             PHY QUAD            │
>   └───▲────────▲────────▲────────▲──┘
>       │        │        │        │
>   ┌───┼──┐ ┌───┴──┐ ┌───┼──┐ ┌───┼──┐
>   │Port 0│ │Port 1│ │Port 2│ │Port 3│
>   └──────┘ └──────┘ └──────┘ └──────┘
>
> If multiple PFs issue Tx timestamp requests near simultaneously, it is
> possible that the correct PF will not be interrupted and will miss its
> timestamp. Understanding why is somewhat complex.
>
> Consider the following sequence of events:
>
>   CPU 0:
>   Send Tx packet on PF 0
>   ...
>   PF 0 enqueues packet with Tx request          CPU 1, PF1:
>   ...                                           Send Tx packet on PF1
>   ...                                           PF 1 enqueues packet with Tx request
>
>   HW:
>   PHY Port 0 sends packet
>   PHY raises Tx timestamp event interrupt
>   MAC raises each PF interrupt
>
>   CPU 0, PF0:                                   CPU 1, PF1:
>   ice_misc_intr() checks for Tx timestamps      ice_misc_intr() checks for Tx timestamp
>   Sees packet ready bit set                     Sees nothing available
>   ...                                           Exits
>   ...
>   ...
>   HW:
>   PHY port 1 sends packet
>   PHY interrupt ignored because not all packet timestamps read yet.
>   ...
>   Read timestamp, report to stack
>
> Because the interrupt event is shared for all ports on the same quad, the
> PHY will not raise a new interrupt for any PF until all timestamps are
> read.
>
> In the example above, the second timestamp comes in for port 1 before the
> timestamp from port 0 is read. At this point, there is no longer an
> interrupt thread running that will read the timestamps, because each PF has
> checked and found that there was no work to do. Applications such as ptp4l
> will timeout after waiting a few milliseconds. Eventually, the watchdog
> service task will re-check for all quads and notice that there are
> outstanding timestamps, and issue a software interrupt to recover. However,
> by this point it is far too late, and applications have already failed.
>
> All of this occurs because of the underlying hardware behavior. The PHY
> cannot raise a new interrupt signal until all outstanding timestamps have
> been read.
>
> As a first step to fix this, switch the E825C hardware to the
> ICE_PTP_TX_INTERRUPT_ALL mode. In this mode, only the clock owner PF will
> respond to the PFINT_TSYN_TX cause. Other PFs disable this cause and will
> not wake. In this mode, the clock owner will iterate over all ports and
> handle timestamps for each connected port.
>
> This matches the E822 behavior, and is a necessary but insufficient step to
> resolve the missing timestamps.
>
> Even with use of the ICE_PTP_TX_INTERRUPT_ALL mode, we still sometimes miss
> a timestamp event. The ice_ptp_tx_tstamp_owner() does re-check the ready
> bitmap, but does so before re-enabling the OICR interrupt vector. It also
> only checks the ready bitmap, but not the software Tx timestamp tracker.
>
> To avoid risk of losing a timestamp, refactor the logic to check both the
> software Tx timestamp tracker bitmap *and* the hardware ready bitmap.
> Additionally, do this outside of ice_ptp_process_ts() after we have already
> re-enabled the OICR interrupt.
>
> Remove the checks from the ice_ptp_tx_tstamp(), ice_ptp_tx_tstamp_owner(),
> and the ice_ptp_process_ts() functions. This results in ice_ptp_tx_tstamp()
> being nothing more than a wrapper around ice_ptp_process_tx_tstamp() so we
> can remove it.
>
> Add the ice_ptp_tx_tstamps_pending() function which returns a boolean
> indicating if there are any pending Tx timestamps. First, check the
> software timestamp tracker bitmap. In ICE_PTP_TX_INTERRUPT_ALL mode, check
> *all* ports software trackers. If a tracker has outstanding timestamp
> requests, return true. Additionally, check the PHY ready bitmap to confirm
> if the PHY indicates any outstanding timestamps.
>
> In the ice_misc_thread_fn(), call ice_ptp_tx_tstamps_pending() just before
> returning from the IRQ thread handler. If it returns true, write to
> PFINT_OICR to trigger a PFINT_OICR_TSYN_TX_M software interrupt. This will
> force the handler to interrupt again and complete the work even if the PHY
> hardware did not interrupt for any reason.
>
> This results in the following new flow for handling Tx timestamps:
>
> 1) send Tx packet
> 2) PHY captures timestamp
> 3) PHY triggers MAC interrupt
> 4) clock owner executes ice_misc_intr() with PFINT_OICR_TSYN_TX flag set
> 5) ice_ptp_ts_irq() returns IRQ_WAKE_THREAD
> 7) The interrupt thread wakes up and kernel calls ice_misc_intr_thread_fn()
> 8) ice_ptp_process_ts() is called to handle any outstanding timestamps
> 9) ice_irq_dynamic_ena() is called to re-enable the OICR hardware interrupt
>    cause
> 10) ice_ptp_tx_tstamps_pending() is called to check if we missed any more
>     outstanding timestamps, checking both software and hardware indicators.
>
> With this change, it should no longer be possible for new timestamps to
> come in such a way that we lose an interrupt. If a timestamp comes in
> before the ice_ptp_tx_tstamps_pending() call, it will be noticed by at
> least one of the software bitmap check or the hardware bitmap check. If the
> timestamp comes in *after* this check, it should cause a timestamp
> interrupt as we have already read all timestamps from the PHY and the OICR
> vector has been re-enabled.
>
> Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products")
> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>

I applied this patch to a RHEL 9.6 kernel and my colleague has tested it.
He wrote:
> Tested the build: kernel-5.14.0-570.81.1.iceptpstamps.el9 by running
> G8273.2 noise generation test (7.1) for more than 4 hours -  have not
> observed any issues. Packet rates normal and inter-message intervals
> are regular. No delays, timeouts, failures.

You can add:

Tested-by: Vitaly Grinberg <vgrinber@...hat.com>

Powered by blists - more mailing lists