diff mbox series

[net] sfc: fix timestamp reconstruction at 16-bit rollover points

Message ID ec166593-f68f-7834-e260-cd8ec6533054@solarflare.com
State Accepted
Delegated to: David Miller
Headers show
Series [net] sfc: fix timestamp reconstruction at 16-bit rollover points | expand

Commit Message

Alex Maftei (amaftei) Feb. 26, 2020, 5:33 p.m. UTC
We can't just use the top bits of the last sync event as they could be
off-by-one every 65,536 seconds, giving an error in reconstruction of
65,536 seconds.

This patch uses the difference in the bottom 16 bits (mod 2^16) to
calculate an offset that needs to be applied to the last sync event to
get to the current time.

Signed-off-by: Alexandru-Mihai Maftei <amaftei@solarflare.com>
---
 drivers/net/ethernet/sfc/ptp.c | 38 +++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

Comments

Martin Habets Feb. 27, 2020, 5:49 p.m. UTC | #1
On 26/02/2020 17:33, Alex Maftei (amaftei) wrote:
> We can't just use the top bits of the last sync event as they could be
> off-by-one every 65,536 seconds, giving an error in reconstruction of
> 65,536 seconds.
> 
> This patch uses the difference in the bottom 16 bits (mod 2^16) to
> calculate an offset that needs to be applied to the last sync event to
> get to the current time.
> 
> Signed-off-by: Alexandru-Mihai Maftei <amaftei@solarflare.com>

Acked-by: Martin Habets <mhabets@solarflare.com>

> ---
>  drivers/net/ethernet/sfc/ptp.c | 38 +++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
> index af15a737c675..59b4f16896a8 100644
> --- a/drivers/net/ethernet/sfc/ptp.c
> +++ b/drivers/net/ethernet/sfc/ptp.c
> @@ -560,13 +560,45 @@ efx_ptp_mac_nic_to_ktime_correction(struct efx_nic *efx,
>  				    u32 nic_major, u32 nic_minor,
>  				    s32 correction)
>  {
> +	u32 sync_timestamp;
>  	ktime_t kt = { 0 };
> +	s16 delta;
>  
>  	if (!(nic_major & 0x80000000)) {
>  		WARN_ON_ONCE(nic_major >> 16);
> -		/* Use the top bits from the latest sync event. */
> -		nic_major &= 0xffff;
> -		nic_major |= (last_sync_timestamp_major(efx) & 0xffff0000);
> +
> +		/* Medford provides 48 bits of timestamp, so we must get the top
> +		 * 16 bits from the timesync event state.
> +		 *
> +		 * We only have the lower 16 bits of the time now, but we do
> +		 * have a full resolution timestamp at some point in past. As
> +		 * long as the difference between the (real) now and the sync
> +		 * is less than 2^15, then we can reconstruct the difference
> +		 * between those two numbers using only the lower 16 bits of
> +		 * each.
> +		 *
> +		 * Put another way
> +		 *
> +		 * a - b = ((a mod k) - b) mod k
> +		 *
> +		 * when -k/2 < (a-b) < k/2. In our case k is 2^16. We know
> +		 * (a mod k) and b, so can calculate the delta, a - b.
> +		 *
> +		 */
> +		sync_timestamp = last_sync_timestamp_major(efx);
> +
> +		/* Because delta is s16 this does an implicit mask down to
> +		 * 16 bits which is what we need, assuming
> +		 * MEDFORD_TX_SECS_EVENT_BITS is 16. delta is signed so that
> +		 * we can deal with the (unlikely) case of sync timestamps
> +		 * arriving from the future.
> +		 */
> +		delta = nic_major - sync_timestamp;
> +
> +		/* Recover the fully specified time now, by applying the offset
> +		 * to the (fully specified) sync time.
> +		 */
> +		nic_major = sync_timestamp + delta;
>  
>  		kt = ptp->nic_to_kernel_time(nic_major, nic_minor,
>  					     correction);
>
David Miller Feb. 27, 2020, 8:05 p.m. UTC | #2
From: "Alex Maftei (amaftei)" <amaftei@solarflare.com>
Date: Wed, 26 Feb 2020 17:33:19 +0000

> We can't just use the top bits of the last sync event as they could be
> off-by-one every 65,536 seconds, giving an error in reconstruction of
> 65,536 seconds.
> 
> This patch uses the difference in the bottom 16 bits (mod 2^16) to
> calculate an offset that needs to be applied to the last sync event to
> get to the current time.
> 
> Signed-off-by: Alexandru-Mihai Maftei <amaftei@solarflare.com>

Applied and queued up for -stable, thank you.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index af15a737c675..59b4f16896a8 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -560,13 +560,45 @@  efx_ptp_mac_nic_to_ktime_correction(struct efx_nic *efx,
 				    u32 nic_major, u32 nic_minor,
 				    s32 correction)
 {
+	u32 sync_timestamp;
 	ktime_t kt = { 0 };
+	s16 delta;
 
 	if (!(nic_major & 0x80000000)) {
 		WARN_ON_ONCE(nic_major >> 16);
-		/* Use the top bits from the latest sync event. */
-		nic_major &= 0xffff;
-		nic_major |= (last_sync_timestamp_major(efx) & 0xffff0000);
+
+		/* Medford provides 48 bits of timestamp, so we must get the top
+		 * 16 bits from the timesync event state.
+		 *
+		 * We only have the lower 16 bits of the time now, but we do
+		 * have a full resolution timestamp at some point in past. As
+		 * long as the difference between the (real) now and the sync
+		 * is less than 2^15, then we can reconstruct the difference
+		 * between those two numbers using only the lower 16 bits of
+		 * each.
+		 *
+		 * Put another way
+		 *
+		 * a - b = ((a mod k) - b) mod k
+		 *
+		 * when -k/2 < (a-b) < k/2. In our case k is 2^16. We know
+		 * (a mod k) and b, so can calculate the delta, a - b.
+		 *
+		 */
+		sync_timestamp = last_sync_timestamp_major(efx);
+
+		/* Because delta is s16 this does an implicit mask down to
+		 * 16 bits which is what we need, assuming
+		 * MEDFORD_TX_SECS_EVENT_BITS is 16. delta is signed so that
+		 * we can deal with the (unlikely) case of sync timestamps
+		 * arriving from the future.
+		 */
+		delta = nic_major - sync_timestamp;
+
+		/* Recover the fully specified time now, by applying the offset
+		 * to the (fully specified) sync time.
+		 */
+		nic_major = sync_timestamp + delta;
 
 		kt = ptp->nic_to_kernel_time(nic_major, nic_minor,
 					     correction);