> -----Original Message----- > From: Jacob Keller <[email protected]> > Sent: Saturday, September 26, 2026 1:57 AM > To: Intel Wired LAN <[email protected]>; Machnikowski, > Maciej <[email protected]>; Keller, Jacob E > <[email protected]>; Korba, Przemyslaw > <[email protected]>; Nguyen, Anthony L > <[email protected]>; Nitka, Grzegorz > <[email protected]>; Kubalewski, Arkadiusz > <[email protected]> > Cc: Keller, Jacob E <[email protected]>; Oros, Petr > <[email protected]>; Machnikowski, Maciej > <[email protected]> > Subject: [PATCH iwl-net v3 12/15] ice: keep Tx timestamp slots tracked > until completion or timeout > > From: Petr Oros <[email protected]> > > When the link goes down the processing loop drops every outstanding > request, and a request whose timestamp is not ready yet is freed > without reading the PHY slot. The hardware completes the capture a > moment later, the orphaned ready bit blocks the port interrupt until > the next link-up sweep, and the freed index can meanwhile be reused by > a new request whose slot the hardware then overwrites. Captured on a > reproducer as ready bits with no in_use owner right after a link > bounce. > > Stop dropping on link down. Mark the outstanding requests stale so > their completions are read and discarded, reject new requests while > the link is down, and free a not yet ready slot only after the two > second timeout. This way an index is never reused while the hardware > can still write it and never left untracked while a completion can > still arrive. > > Since we're adding a new call to ice_ptp_mark_tx_tracker, notice that > the function previously did not check the tracker initialization > state. Fix this by only accessing the bitmaps if the init field is > set. > > To avoid continuously re-triggering the IRQ at the end of a processing > loop when we have a stale packet that is not timestamped, modify > ice_ptp_tx_tstamps_pending() to ignore stale timestamps when checking > for whether to re-arm the IRQ from the miscellaneous thread function. > Instead, only count stale packets as part of the check in the > auxiliary work thread. > This way we do not keep spamming the IRQ when the timestamp won't be > reported anyways. > > To ensure that forward progress is made on clearing stale timestamps, > fix the check for ice_ptp_maybe_trigger_tx_interrupt to properly apply > for devices that manage their own interrupt, instead of only checking > on the clock owner. In the case of a timestamp which never completes, > this does result in one extra IRQ every 500 msec until the timeout. In > some sense this is extra work, but we need to recheck to ensure that > timestamp slots do not remain locked forever, and the software can't > reliably know whether a given index will or won't complete. > > Note that E810 does not have the ready bitmap in its hardware. It is > currently excluded from the ice_ptp_maybe_trigger_tx_interrupt() by a > check against the has_ready_bitmap field. This could lead to an > interrupt stall if every single Tx timestamp index slot becomes stale. > However, just removing the check against has_ready_bitmap would be > problematic for the low latency path which currently is not protected > against another IRQ happening while a request is outstanding. That is > a pre-existing issue, but we should avoid making it worse until the > path can be addressed by another fix. The low latency path also > completely bypasses the > ice_ptp_process_tx_tstamp() flow. For now, skip the > ice_ptp_maybe_trigger_tx_interrupt() when ts_ll_int_read is set and > the low latency path is in use. The path still has preexisting issues > but this avoids making it worse. > > This effectively reverts commit fcc2cef37fed ("ice/ptp: fix the PTP > worker retrying indefinitely if the link went down"), which tried to > release an index before this 2 second wait period. > > Fixes: fcc2cef37fed ("ice/ptp: fix the PTP worker retrying > indefinitely if the link went down") > Suggested-by: Jacob Keller <[email protected]> > Signed-off-by: Petr Oros <[email protected]> > Reviewed-by: Maciek Machnikowski <[email protected]> > Signed-off-by: Jacob Keller <[email protected]> > --- > drivers/net/ethernet/intel/ice/ice_ptp.h | 8 +++-- > drivers/net/ethernet/intel/ice/ice_main.c | 2 +- > drivers/net/ethernet/intel/ice/ice_ptp.c | 57 ++++++++++++++++------- > -------- > 3 files changed, 37 insertions(+), 30 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h > b/drivers/net/ethernet/intel/ice/ice_ptp.h > index 029ee4612d76..7810fefc546e 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.h > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.h > @@ -132,6 +132,9 @@ struct ice_ptp_tx { > #define INDEX_PER_PORT_E82X 16 > #define INDEX_PER_PORT 64 >
... > >link_up); > } > > /** > @@ -554,7 +557,9 @@ void ice_ptp_complete_tx_single_tstamp(struct > ice_ptp_tx *tx) > * extremely unlikely that a packet will ever take this long to > timestamp. If > * we detect a Tx timestamp request that has waited for this long we > assume > * the packet will never be sent by hardware and discard it without > reading > - * the timestamp register. > + * the timestamp register. Note that in the unusual case where the > PHY > + * continuously fails to clear the ready bitmap index, the slot may > + remained Nit: ' may remained' -> 'may remain locked' Reviewed-by: Aleksandr Loktionov <[email protected]> > + * locked for more than two seconds. > */ > static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx) { @@ - > 564,7 +569,6 @@ static void ice_ptp_process_tx_tstamp(struct > ice_ptp_tx *tx) > struct ice_pf *pf; > struct ice_hw *hw; > u64 tstamp_ready; > - bool link_up; > int err; > u8 idx; > > @@ -582,14 +586,11 @@ static void ice_ptp_process_tx_tstamp(struct > ice_ptp_tx *tx) > return; > } > ... > dev_dbg(dev, "PTP periodic task detected waiting > timestamps. Triggering Tx timestamp interrupt now.\n"); > > wr32(hw, PFINT_OICR, PFINT_OICR_TSYN_TX_M); > > -- > 2.56.0.rc0.395.gd1f3524e15dc
