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.
To avoid an IRQ storm in the event that we really do 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 check for stale packets in the
auxiliary work thread. This way we do not check in a tight loop waiting for
a timestamp that may never come.
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.
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 | 51 ++++++++++++++++---------------
3 files changed, 34 insertions(+), 27 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
+/* Maximum number of timestamp indexes across all devices */
+#define INDEX_PER_PORT_MAX INDEX_PER_PORT
+
/**
* struct ice_ptp_port - data used to initialize an external port for PTP
*
@@ -314,7 +317,7 @@ void ice_ptp_req_tx_single_tstamp(struct ice_ptp_tx *tx, u8
idx);
void ice_ptp_complete_tx_single_tstamp(struct ice_ptp_tx *tx);
void ice_ptp_process_ts(struct ice_pf *pf);
irqreturn_t ice_ptp_ts_irq(struct ice_pf *pf);
-bool ice_ptp_tx_tstamps_pending(struct ice_pf *pf);
+bool ice_ptp_tx_tstamps_pending(struct ice_pf *pf, bool in_irq);
u64 ice_ptp_read_src_clk_reg(struct ice_pf *pf,
struct ptp_system_timestamp *sts);
@@ -362,7 +365,8 @@ static inline irqreturn_t ice_ptp_ts_irq(struct ice_pf *pf)
return IRQ_HANDLED;
}
-static inline bool ice_ptp_tx_tstamps_pending(struct ice_pf *pf)
+static inline bool
+ice_ptp_tx_tstamps_pending(struct ice_pf *pf, bool in_irq)
{
return false;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
b/drivers/net/ethernet/intel/ice/ice_main.c
index f32041dd8b27..d12952171a99 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3248,7 +3248,7 @@ static irqreturn_t ice_misc_intr_thread_fn(int
__always_unused irq, void *data)
ice_irq_dynamic_ena(hw, NULL, NULL);
ice_flush(hw);
- if (ice_ptp_tx_tstamps_pending(pf)) {
+ if (ice_ptp_tx_tstamps_pending(pf, true)) {
/* If any new Tx timestamps happened while in interrupt,
* re-arm the interrupt to trigger it again.
*/
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 142d9e1c1f2e..06f383415d07 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -365,9 +365,12 @@ static u64 ice_ptp_extend_40b_ts(struct ice_pf *pf, u64
in_tstamp)
static bool
ice_ptp_is_tx_tracker_up(struct ice_ptp_tx *tx)
{
+ struct ice_ptp_port *ptp_port =
+ container_of(tx, struct ice_ptp_port, tx);
+
lockdep_assert_held(&tx->lock);
- return tx->init && !tx->calibrating;
+ return tx->init && !tx->calibrating && READ_ONCE(ptp_port->link_up);
}
/**
@@ -564,7 +567,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 +584,11 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx
*tx)
return;
}
- /* Drop packets if the link went down */
- link_up = READ_ONCE(ptp_port->link_up);
-
for_each_set_bit(idx, tx->in_use, tx->len) {
struct skb_shared_hwtstamps shhwtstamps = {};
u8 phy_idx = idx + tx->offset;
u64 raw_tstamp = 0, tstamp;
- bool drop_ts = !link_up;
+ bool drop_ts = false;
struct sk_buff *skb;
/* Prevent speculative re-ordering of start and skb */
@@ -1399,6 +1398,10 @@ void ice_ptp_link_change(struct ice_pf *pf, bool linkup)
if (pf->hw.reset_ongoing) {
mutex_lock(&pf->adapter->ps_lock);
WRITE_ONCE(ptp_port->link_up, linkup);
+
+ if (!linkup)
+ ice_ptp_mark_tx_tracker_stale(&ptp_port->tx);
+
mutex_unlock(&pf->adapter->ps_lock);
return;
}
@@ -1448,6 +1451,9 @@ void ice_ptp_link_change(struct ice_pf *pf, bool linkup)
*/
WRITE_ONCE(ptp_port->link_up, linkup);
+ if (!linkup)
+ ice_ptp_mark_tx_tracker_stale(&ptp_port->tx);
+
switch (hw->mac_type) {
case ICE_MAC_E810:
case ICE_MAC_E830:
@@ -2804,21 +2810,22 @@ void ice_ptp_process_ts(struct ice_pf *pf)
}
}
-static bool ice_port_has_timestamps(struct ice_ptp_tx *tx)
+static bool ice_port_has_timestamps(struct ice_ptp_tx *tx, bool in_irq)
{
- bool more_timestamps;
+ DECLARE_BITMAP(tstamps, INDEX_PER_PORT_MAX) = {};
scoped_guard(spinlock_irqsave, &tx->lock) {
if (!tx->init)
return false;
- more_timestamps = !bitmap_empty(tx->in_use, tx->len);
+ if (in_irq)
+ return bitmap_andnot(tstamps, tx->in_use, tx->stale,
tx->len);
+ else
+ return !bitmap_empty(tx->in_use, tx->len);
}
-
- return more_timestamps;
}
-static bool ice_any_port_has_timestamps(struct ice_pf *pf)
+static bool ice_any_port_has_timestamps(struct ice_pf *pf, bool in_irq)
{
struct ice_port_list *ports = &pf->adapter->ports;
bool have_tstamps = false;
@@ -2832,7 +2839,7 @@ static bool ice_any_port_has_timestamps(struct ice_pf *pf)
if (!kref_get_unless_zero(&port->ref))
continue;
- if (ice_port_has_timestamps(&port->tx))
+ if (ice_port_has_timestamps(&port->tx, in_irq))
have_tstamps = true;
kref_put(&port->ref, ice_ptp_release_port_srcu);
@@ -2846,7 +2853,7 @@ static bool ice_any_port_has_timestamps(struct ice_pf *pf)
return have_tstamps;
}
-bool ice_ptp_tx_tstamps_pending(struct ice_pf *pf)
+bool ice_ptp_tx_tstamps_pending(struct ice_pf *pf, bool in_irq)
{
struct ice_hw *hw = &pf->hw;
int ret;
@@ -2856,11 +2863,11 @@ bool ice_ptp_tx_tstamps_pending(struct ice_pf *pf)
case ICE_PTP_TX_INTERRUPT_NONE:
return false;
case ICE_PTP_TX_INTERRUPT_SELF:
- if (ice_port_has_timestamps(&pf->ptp.port.tx))
+ if (ice_port_has_timestamps(&pf->ptp.port.tx, in_irq))
return true;
break;
case ICE_PTP_TX_INTERRUPT_ALL:
- if (ice_any_port_has_timestamps(pf))
+ if (ice_any_port_has_timestamps(pf, in_irq))
return true;
break;
default:
@@ -2936,7 +2943,7 @@ irqreturn_t ice_ptp_ts_irq(struct ice_pf *pf)
/* E830 can read timestamps in the top half using rd32() */
ice_ptp_process_ts(pf);
- if (ice_ptp_tx_tstamps_pending(pf)) {
+ if (ice_ptp_tx_tstamps_pending(pf, true)) {
/* Process outstanding Tx timestamps. If there
* is more work, re-arm the interrupt to trigger again.
*/
@@ -2966,19 +2973,15 @@ static void ice_ptp_maybe_trigger_tx_interrupt(struct
ice_pf *pf)
{
struct device *dev = ice_pf_to_dev(pf);
struct ice_hw *hw = &pf->hw;
- int ret;
if (!pf->ptp.port.tx.has_ready_bitmap)
return;
- if (!ice_pf_src_tmr_owned(pf))
+ if (pf->ptp.tx_interrupt_mode != ICE_PTP_TX_INTERRUPT_SELF &&
+ !ice_pf_src_tmr_owned(pf))
return;
- ret = ice_check_phy_tx_tstamp_ready(hw);
- if (ret < 0) {
- dev_dbg(dev, "PTP periodic task unable to read PHY timestamp
ready bitmap, err %d\n",
- ret);
- } else if (ret) {
+ if (ice_ptp_tx_tstamps_pending(pf, false)) {
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
