diff mbox series

[iwl-net,v3,11/15] ice: wait for in-flight Tx timestamps before flushing the tracker

Message ID 20260925-jk-e825c-timestamp-processing-logic-fixes-srcu-v3-11-6532598e8da8@intel.com
State Under Review
Delegated to: Anthony Nguyen
Headers show
Series ice: E82x: timestamp processing logic fixes | expand

Commit Message

Jacob Keller Sept. 25, 2026, 11:56 p.m. UTC 
From: Petr Oros <poros@redhat.com>

ice_ptp_flush_tx_tracker() frees every tracked request, but a request
whose timestamp is still being captured by the PHY at that moment is
freed without touching the PHY entry. The ready bit published shortly
after has no tracked owner, and the PHY does not raise another Tx
timestamp interrupt until every outstanding ready bit is read, so
delivery for the whole quad degrades to the periodic work.

Wait up to 10 ms (per port) for in-flight captures to publish their ready
bits before flushing, so the flush clears them together with the rest.

Note that the ice_ptp_flush_tx_tracker() function was introduced along with
the original E810 support, but that device does not have a ready bitmap.
Only later devices (E822, E825, E830) have the bitmap and potential issues
with internal tracking. Thus, skip the wait for E810 by checking the
tx->has_ready_bitmap flag.

Fixes: 10e4b4a3a3e1 ("ice: check Tx timestamp memory register for ready timestamps")
Signed-off-by: Petr Oros <poros@redhat.com>
Reviewed-by: Maciek Machnikowski <maciej.machnikowski@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_ptp.c | 64 ++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

Comments

Loktionov, Aleksandr Oct. 5, 2026, 11:05 a.m. UTC | #1 
> -----Original Message-----
> From: Jacob Keller <jacob.e.keller@intel.com>
> Sent: Saturday, September 26, 2026 1:57 AM
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>; Machnikowski,
> Maciej <maciej.machnikowski@intel.com>; Keller, Jacob E
> <jacob.e.keller@intel.com>; Korba, Przemyslaw
> <przemyslaw.korba@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Nitka, Grzegorz
> <grzegorz.nitka@intel.com>; Kubalewski, Arkadiusz
> <arkadiusz.kubalewski@intel.com>
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; Oros, Petr
> <poros@redhat.com>; Machnikowski, Maciej
> <maciej.machnikowski@intel.com>
> Subject: [PATCH iwl-net v3 11/15] ice: wait for in-flight Tx
> timestamps before flushing the tracker
> 
> From: Petr Oros <poros@redhat.com>
> 
> ice_ptp_flush_tx_tracker() frees every tracked request, but a request
> whose timestamp is still being captured by the PHY at that moment is
> freed without touching the PHY entry. The ready bit published shortly
> after has no tracked owner, and the PHY does not raise another Tx
> timestamp interrupt until every outstanding ready bit is read, so
> delivery for the whole quad degrades to the periodic work.
> 
> Wait up to 10 ms (per port) for in-flight captures to publish their
> ready bits before flushing, so the flush clears them together with the
> rest.
> 
> Note that the ice_ptp_flush_tx_tracker() function was introduced along
> with the original E810 support, but that device does not have a ready
> bitmap.
> Only later devices (E822, E825, E830) have the bitmap and potential
> issues with internal tracking. Thus, skip the wait for E810 by
> checking the
> tx->has_ready_bitmap flag.
> 
> Fixes: 10e4b4a3a3e1 ("ice: check Tx timestamp memory register for
> ready timestamps")
> Signed-off-by: Petr Oros <poros@redhat.com>
> Reviewed-by: Maciek Machnikowski <maciej.machnikowski@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_ptp.c | 64
> ++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
> b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index 818e2e265a7e..5220de274819 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -744,6 +744,68 @@ ice_ptp_alloc_tx_tracker(struct ice_ptp_tx *tx)
>  	return 0;
>  }
> 
> +/**
> + * ice_ptp_is_tracker_drained - Check for outstanding timestamps
> + * @pf: Board private structure
> + * @tx: Timestamp tracker structure
> + *
> + * Return: False if there are any timestamps still waiting for
> hardware;
> + *         otherwise true, including when unable to read the ready
> bitmap.
> + */
> +static bool
> +ice_ptp_is_tracker_drained(struct ice_pf *pf, struct ice_ptp_tx *tx)
> {
> +	struct ice_hw *hw = &pf->hw;
> +	bool pending = false;
> +	unsigned long flags;
> +	u64 tstamp_ready;
> +	u8 idx;
> +
> +	/* If HW reset is ongoing, we can't access SBQ */
> +	if (hw->reset_ongoing)
> +		return true;
> +
> +	if (ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready))
> +		return true;
> +
> +	spin_lock_irqsave(&tx->lock, flags);
> +	for_each_set_bit(idx, tx->in_use, tx->len) {
> +		if (!(tstamp_ready & BIT_ULL(idx + tx->offset))) {
> +			pending = true;
> +			break;
> +		}
> +	}
> +	spin_unlock_irqrestore(&tx->lock, flags);
> +
> +	return !pending;
> +}
> +
> +/**
> + * ice_ptp_wait_for_tracker_drain - Wait for PHY to complete
> timestamps
> + * @pf: Board private structure
> + * @tx: Timestamp tracker structure
> + *
> + * Wait for up to 10 milliseconds for the PHY to complete any
> +outstanding
> + * timestamps before flushing.
> + */
> +static void
> +ice_ptp_wait_for_tracker_drain(struct ice_pf *pf, struct ice_ptp_tx
> +*tx) {
> +	bool drained;
> +	int err;
> +
> +	if (!tx->has_ready_bitmap)
> +		return;
> +
> +	err = read_poll_timeout(ice_ptp_is_tracker_drained,
> +				drained, drained, 500, 10 * USEC_PER_MSEC,
> false,
> +				pf, tx);
> +	if (err) {
> +		dev_dbg(ice_pf_to_dev(pf), "Timed out waiting for in-
> flight Tx timestamps on block %u\n",
> +			tx->block);
> +	}
> +}
> +
>  /**
>   * ice_ptp_flush_tx_tracker - Flush any remaining timestamps from the
> tracker
>   * @pf: Board private structure
> @@ -760,6 +822,8 @@ ice_ptp_flush_tx_tracker(struct ice_pf *pf, struct
> ice_ptp_tx *tx)
>  	int err;
>  	u8 idx;
> 
> +	ice_ptp_wait_for_tracker_drain(pf, tx);
> +
>  	err = ice_get_phy_tx_tstamp_ready(hw, tx->block,
> &tstamp_ready);
>  	if (err) {
>  		dev_dbg(ice_pf_to_dev(pf), "Failed to get the Tx tstamp
> ready bitmap for block %u, err %d\n",
> 
> --
> 2.56.0.rc0.395.gd1f3524e15dc

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Nowlin, Alexander Oct. 6, 2026, 1:42 a.m. UTC | #2 
> -----Original Message-----
> From: Jacob Keller <jacob.e.keller@intel.com> 
> Sent: Friday, September 25, 2026 4:57 PM
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>; Machnikowski, Maciej <maciej.machnikowski@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>; Korba, Przemyslaw <przemyslaw.korba@intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Nitka, Grzegorz <grzegorz.nitka@intel.com>; Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; Oros, Petr <poros@redhat.com>; Machnikowski, Maciej <maciej.machnikowski@intel.com>
> Subject: [PATCH iwl-net v3 11/15] ice: wait for in-flight Tx timestamps before flushing the tracker
> 
> From: Petr Oros <poros@redhat.com>
> 
> ice_ptp_flush_tx_tracker() frees every tracked request, but a request whose timestamp is still being captured by the PHY at that moment is freed without touching the PHY entry. The ready bit published shortly after has no tracked owner, and the PHY does not raise another Tx timestamp interrupt until every outstanding ready bit is read, so delivery for the whole quad degrades to the periodic work.
> 
> Wait up to 10 ms (per port) for in-flight captures to publish their ready bits before flushing, so the flush clears them together with the rest.
> 
> Note that the ice_ptp_flush_tx_tracker() function was introduced along with the original E810 support, but that device does not have a ready bitmap.
> Only later devices (E822, E825, E830) have the bitmap and potential issues with internal tracking. Thus, skip the wait for E810 by checking the
> tx->has_ready_bitmap flag.
> 
> Fixes: 10e4b4a3a3e1 ("ice: check Tx timestamp memory register for ready timestamps")
> Signed-off-by: Petr Oros <poros@redhat.com>
> Reviewed-by: Maciek Machnikowski <maciej.machnikowski@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_ptp.c | 64 ++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)

Tested-by: Alexander Nowlin <alexander.nowlin@intel.com>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 818e2e265a7e..5220de274819 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -744,6 +744,68 @@  ice_ptp_alloc_tx_tracker(struct ice_ptp_tx *tx)
 	return 0;
 }
 
+/**
+ * ice_ptp_is_tracker_drained - Check for outstanding timestamps
+ * @pf: Board private structure
+ * @tx: Timestamp tracker structure
+ *
+ * Return: False if there are any timestamps still waiting for hardware;
+ *         otherwise true, including when unable to read the ready bitmap.
+ */
+static bool
+ice_ptp_is_tracker_drained(struct ice_pf *pf, struct ice_ptp_tx *tx)
+{
+	struct ice_hw *hw = &pf->hw;
+	bool pending = false;
+	unsigned long flags;
+	u64 tstamp_ready;
+	u8 idx;
+
+	/* If HW reset is ongoing, we can't access SBQ */
+	if (hw->reset_ongoing)
+		return true;
+
+	if (ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready))
+		return true;
+
+	spin_lock_irqsave(&tx->lock, flags);
+	for_each_set_bit(idx, tx->in_use, tx->len) {
+		if (!(tstamp_ready & BIT_ULL(idx + tx->offset))) {
+			pending = true;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&tx->lock, flags);
+
+	return !pending;
+}
+
+/**
+ * ice_ptp_wait_for_tracker_drain - Wait for PHY to complete timestamps
+ * @pf: Board private structure
+ * @tx: Timestamp tracker structure
+ *
+ * Wait for up to 10 milliseconds for the PHY to complete any outstanding
+ * timestamps before flushing.
+ */
+static void
+ice_ptp_wait_for_tracker_drain(struct ice_pf *pf, struct ice_ptp_tx *tx)
+{
+	bool drained;
+	int err;
+
+	if (!tx->has_ready_bitmap)
+		return;
+
+	err = read_poll_timeout(ice_ptp_is_tracker_drained,
+				drained, drained, 500, 10 * USEC_PER_MSEC, false,
+				pf, tx);
+	if (err) {
+		dev_dbg(ice_pf_to_dev(pf), "Timed out waiting for in-flight Tx timestamps on block %u\n",
+			tx->block);
+	}
+}
+
 /**
  * ice_ptp_flush_tx_tracker - Flush any remaining timestamps from the tracker
  * @pf: Board private structure
@@ -760,6 +822,8 @@  ice_ptp_flush_tx_tracker(struct ice_pf *pf, struct ice_ptp_tx *tx)
 	int err;
 	u8 idx;
 
+	ice_ptp_wait_for_tracker_drain(pf, tx);
+
 	err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready);
 	if (err) {
 		dev_dbg(ice_pf_to_dev(pf), "Failed to get the Tx tstamp ready bitmap for block %u, err %d\n",