| Message ID | 20211112135359.155502-1-karol.kolacinski@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [intel-next] ice: Don't put stale timestamps in the skb | expand |
On 11/12/2021 5:53 AM, Karol Kolacinski wrote: > The driver has to check if it does not accidentally put the timestamp in > the SKB before previous timestamp gets overwritten. > Timestamp values in the PHY are read only and do not get cleared except > at hardware reset or when a new timestamp value is captured. > The cached_tstamp field is used to detect the case where a new timestamp > has not yet been captured, ensuring that we avoid sending stale > timestamp data to the stack. Missing sign off, please run checkpatch --strict and build tests on your patches before sending to the list. > --- > drivers/net/ethernet/intel/ice/ice_ptp.c | 11 ++++------- > drivers/net/ethernet/intel/ice/ice_ptp.h | 6 ++++++ > 2 files changed, 10 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c > index 2b3b2060b504..9a1a09661c78 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.c > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c > @@ -2069,19 +2069,16 @@ static void ice_ptp_tx_tstamp_work(struct kthread_work *work) > if (err) > continue; > > - /* Check if the timestamp is valid */ > - if (!(raw_tstamp & ICE_PTP_TS_VALID)) > + /* Check if the timestamp is invalid or stale */ > + if (!(raw_tstamp & ICE_PTP_TS_VALID) || > + raw_tstamp == tx->tstamps[idx].cached_tstamp) > continue; > > - /* clear the timestamp register, so that it won't show valid > - * again when re-used. > - */ > - ice_clear_phy_tstamp(hw, tx->quad, phy_idx); > - > /* The timestamp is valid, so we'll go ahead and clear this > * index and then send the timestamp up to the stack. > */ > spin_lock(&tx->lock); > + tx->tstamps[idx].cached_tstamp = raw_tstamp; > clear_bit(idx, tx->in_use); > skb = tx->tstamps[idx].skb; > tx->tstamps[idx].skb = NULL; > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h > index 92b202ef3c15..eef8ec894871 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.h > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.h > @@ -55,15 +55,21 @@ struct ice_perout_channel { > * struct ice_tx_tstamp - Tracking for a single Tx timestamp > * @skb: pointer to the SKB for this timestamp request > * @start: jiffies when the timestamp was first requested > + * @cached_tstamp: last read timestamp > * > * This structure tracks a single timestamp request. The SKB pointer is > * provided when initiating a request. The start time is used to ensure that > * we discard old requests that were not fulfilled within a 2 second time > * window. > + * Timestamp values in the PHY are read only and do not get cleared except at > + * hardware reset or when a new timestamp value is captured. The cached_tstamp > + * field is used to detect the case where a new timestamp has not yet been > + * captured, ensuring that we avoid sending stale timestamp data to the stack. > */ > struct ice_tx_tstamp { > struct sk_buff *skb; > unsigned long start; > + u64 cached_tstamp; > }; > > /** >
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c index 2b3b2060b504..9a1a09661c78 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c @@ -2069,19 +2069,16 @@ static void ice_ptp_tx_tstamp_work(struct kthread_work *work) if (err) continue; - /* Check if the timestamp is valid */ - if (!(raw_tstamp & ICE_PTP_TS_VALID)) + /* Check if the timestamp is invalid or stale */ + if (!(raw_tstamp & ICE_PTP_TS_VALID) || + raw_tstamp == tx->tstamps[idx].cached_tstamp) continue; - /* clear the timestamp register, so that it won't show valid - * again when re-used. - */ - ice_clear_phy_tstamp(hw, tx->quad, phy_idx); - /* The timestamp is valid, so we'll go ahead and clear this * index and then send the timestamp up to the stack. */ spin_lock(&tx->lock); + tx->tstamps[idx].cached_tstamp = raw_tstamp; clear_bit(idx, tx->in_use); skb = tx->tstamps[idx].skb; tx->tstamps[idx].skb = NULL; diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h index 92b202ef3c15..eef8ec894871 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp.h +++ b/drivers/net/ethernet/intel/ice/ice_ptp.h @@ -55,15 +55,21 @@ struct ice_perout_channel { * struct ice_tx_tstamp - Tracking for a single Tx timestamp * @skb: pointer to the SKB for this timestamp request * @start: jiffies when the timestamp was first requested + * @cached_tstamp: last read timestamp * * This structure tracks a single timestamp request. The SKB pointer is * provided when initiating a request. The start time is used to ensure that * we discard old requests that were not fulfilled within a 2 second time * window. + * Timestamp values in the PHY are read only and do not get cleared except at + * hardware reset or when a new timestamp value is captured. The cached_tstamp + * field is used to detect the case where a new timestamp has not yet been + * captured, ensuring that we avoid sending stale timestamp data to the stack. */ struct ice_tx_tstamp { struct sk_buff *skb; unsigned long start; + u64 cached_tstamp; }; /**