* [Intel-wired-lan] [PATCH intel-next] ice: Don't put stale timestamps in the skb
@ 2021-11-16 12:07 Karol Kolacinski
2021-12-14 9:37 ` G, GurucharanX
0 siblings, 1 reply; 5+ messages in thread
From: Karol Kolacinski @ 2021-11-16 12:07 UTC (permalink / raw)
To: intel-wired-lan
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.
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
---
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 2149595cc632..df846b66a9a7 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;
};
/**
--
2.32.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Intel-wired-lan] [PATCH intel-next] ice: Don't put stale timestamps in the skb
2021-11-16 12:07 [Intel-wired-lan] [PATCH intel-next] ice: Don't put stale timestamps in the skb Karol Kolacinski
@ 2021-12-14 9:37 ` G, GurucharanX
0 siblings, 0 replies; 5+ messages in thread
From: G, GurucharanX @ 2021-12-14 9:37 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Karol
> Kolacinski
> Sent: Tuesday, November 16, 2021 5:37 PM
> To: intel-wired-lan at lists.osuosl.org
> Cc: Kolacinski, Karol <karol.kolacinski@intel.com>
> Subject: [Intel-wired-lan] [PATCH intel-next] ice: Don't put stale timestamps in
> the skb
>
> 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.
>
> Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
> ---
> 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(-)
>
Tested-by: Gurucharan G <gurucharanx.g@intel.com> (A Contingent worker at Intel)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Intel-wired-lan] [PATCH intel-next] ice: Don't put stale timestamps in the skb
2021-11-12 13:53 Karol Kolacinski
@ 2021-11-13 1:09 ` Jesse Brandeburg
0 siblings, 0 replies; 5+ messages in thread
From: Jesse Brandeburg @ 2021-11-13 1:09 UTC (permalink / raw)
To: intel-wired-lan
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;
> };
>
> /**
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Intel-wired-lan] [PATCH intel-next] ice: Don't put stale timestamps in the skb
@ 2021-11-12 13:53 Karol Kolacinski
2021-11-13 1:09 ` Jesse Brandeburg
0 siblings, 1 reply; 5+ messages in thread
From: Karol Kolacinski @ 2021-11-12 13:53 UTC (permalink / raw)
To: intel-wired-lan
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.
---
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;
};
/**
--
2.32.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Intel-wired-lan] [PATCH intel-next] ice: Don't put stale timestamps in the skb
@ 2021-11-04 13:58 Karol Kolacinski
0 siblings, 0 replies; 5+ messages in thread
From: Karol Kolacinski @ 2021-11-04 13:58 UTC (permalink / raw)
To: intel-wired-lan
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.
---
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;
};
/**
--
2.32.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-14 9:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-16 12:07 [Intel-wired-lan] [PATCH intel-next] ice: Don't put stale timestamps in the skb Karol Kolacinski
2021-12-14 9:37 ` G, GurucharanX
-- strict thread matches above, loose matches on Subject: below --
2021-11-12 13:53 Karol Kolacinski
2021-11-13 1:09 ` Jesse Brandeburg
2021-11-04 13:58 Karol Kolacinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.