ice: protect init and calibrating check in ice_ptp_request_ts

When requesting a new timestamp, the ice_ptp_request_ts function does not
hold the Tx tracker lock while checking init and calibrating. This means
that we might issue a new timestamp request just after the Tx timestamp
tracker starts being deinitialized. This could lead to incorrect access of
the timestamp structures. Correct this by moving the init and calibrating
checks under the lock, and updating the flows which modify these fields to
use the lock.

Note that we do not need to hold the lock while checking for tx->init in
ice_ptp_tx_tstamp. This is because the teardown function will use
synchronize_irq after clearing the flag to ensure that the threaded
interrupt completes. Either a) the tx->init flag will be cleared before the
ice_ptp_tx_tstamp function starts, thus it will exit immediately, or b) the
threaded interrupt will be executing and the synchronize_irq will wait
until the threaded interrupt has completed at which point we know the init
field has definitely been set and new interrupts will not execute the Tx
timestamp thread function.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Gurucharan G <gurucharanx.g@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>

diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 0282ccc55819ff9754573e57515add8d384ca1e7..481492d84e0ec9107bb302f26c21fcc8dfbff872 100644 (file)
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -599,6 +599,23 @@ static u64 ice_ptp_extend_40b_ts(struct ice_pf *pf, u64 in_tstamp)
                                     (in_tstamp >> 8) & mask);
 }
 
+/**
+ * ice_ptp_is_tx_tracker_up - Check if Tx tracker is ready for new timestamps
+ * @tx: the PTP Tx timestamp tracker to check
+ *
+ * Check that a given PTP Tx timestamp tracker is up, i.e. that it is ready
+ * to accept new timestamp requests.
+ *
+ * Assumes the tx->lock spinlock is already held.
+ */
+static bool
+ice_ptp_is_tx_tracker_up(struct ice_ptp_tx *tx)
+{
+       lockdep_assert_held(&tx->lock);
+
+       return tx->init && !tx->calibrating;
+}
+
 /**
  * ice_ptp_tx_tstamp - Process Tx timestamps for a port
  * @tx: the PTP Tx timestamp tracker
@@ -788,10 +805,10 @@ ice_ptp_alloc_tx_tracker(struct ice_ptp_tx *tx)
                return -ENOMEM;
        }
 
-       spin_lock_init(&tx->lock);
-
        tx->init = 1;
 
+       spin_lock_init(&tx->lock);
+
        return 0;
 }
 
@@ -833,7 +850,9 @@ ice_ptp_flush_tx_tracker(struct ice_pf *pf, struct ice_ptp_tx *tx)
 static void
 ice_ptp_release_tx_tracker(struct ice_pf *pf, struct ice_ptp_tx *tx)
 {
+       spin_lock(&tx->lock);
        tx->init = 0;
+       spin_unlock(&tx->lock);
 
        /* wait for potentially outstanding interrupt to complete */
        synchronize_irq(pf->msix_entries[pf->oicr_idx].vector);
@@ -1327,7 +1346,9 @@ ice_ptp_port_phy_restart(struct ice_ptp_port *ptp_port)
        kthread_cancel_delayed_work_sync(&ptp_port->ov_work);
 
        /* temporarily disable Tx timestamps while calibrating PHY offset */
+       spin_lock(&ptp_port->tx.lock);
        ptp_port->tx.calibrating = true;
+       spin_unlock(&ptp_port->tx.lock);
        ptp_port->tx_fifo_busy_cnt = 0;
 
        /* Start the PHY timer in Vernier mode */
@@ -1336,7 +1357,9 @@ ice_ptp_port_phy_restart(struct ice_ptp_port *ptp_port)
                goto out_unlock;
 
        /* Enable Tx timestamps right away */
+       spin_lock(&ptp_port->tx.lock);
        ptp_port->tx.calibrating = false;
+       spin_unlock(&ptp_port->tx.lock);
 
        kthread_queue_delayed_work(pf->ptp.kworker, &ptp_port->ov_work, 0);
 
@@ -2330,11 +2353,14 @@ s8 ice_ptp_request_ts(struct ice_ptp_tx *tx, struct sk_buff *skb)
 {
        u8 idx;
 
-       /* Check if this tracker is initialized */
-       if (!tx->init || tx->calibrating)
+       spin_lock(&tx->lock);
+
+       /* Check that this tracker is accepting new timestamp requests */
+       if (!ice_ptp_is_tx_tracker_up(tx)) {
+               spin_unlock(&tx->lock);
                return -1;
+       }
 
-       spin_lock(&tx->lock);
        /* Find and set the first available index */
        idx = find_first_zero_bit(tx->in_use, tx->len);
        if (idx < tx->len) {

diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
index 5052fc41bed347cd76c66a94dc014bfd000d846b..0bfafaaab6c7280c3703e97a1381fa0f212920a2 100644 (file)
--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
@@ -110,7 +110,7 @@ struct ice_tx_tstamp {
 
 /**
  * struct ice_ptp_tx - Tracking structure for all Tx timestamp requests on a port
- * @lock: lock to prevent concurrent write to in_use bitmap
+ * @lock: lock to prevent concurrent access to fields of this struct
  * @tstamps: array of len to store outstanding requests
  * @in_use: bitmap of len to indicate which slots are in use
  * @block: which memory block (quad or port) the timestamps are captured in