Skip to content

Commit

Permalink
fix: offset calculation for NTP responses
Browse files Browse the repository at this point in the history
The problem here is that `time.Duration` and `time.Time` types don't
exactly match the NTP 64-bit time value, so the expected calculations
for big time jumps (including across NTP eras) don't work properly.

See:

* https://www.eecis.udel.edu/~mills/y2k.html
* https://www.eecis.udel.edu/~mills/time.html

Ref: siderolabs/talos#8771

Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
  • Loading branch information
smira authored and beevik committed May 23, 2024
1 parent f4f0ed1 commit a393ae8
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
15 changes: 12 additions & 3 deletions ntp.go
Original file line number Diff line number Diff line change
Expand Up @@ -749,9 +749,18 @@ func rtt(org, rec, xmt, dst ntpTime) time.Duration {
func offset(org, rec, xmt, dst ntpTime) time.Duration {
// local clock offset
// offset = ((rec-org) + (xmt-dst)) / 2
a := rec.Time().Sub(org.Time())
b := xmt.Time().Sub(dst.Time())
return (a + b) / time.Duration(2)
// The inputs are 64-bit unsigned ints. The output is a 63-bit signed int.
// Need to handle conversions with care. See:
// https://www.eecis.udel.edu/~mills/time.html
a := int64(rec) - int64(org)
b := int64(xmt) - int64(dst)
d := (a + b) / 2

if d > 0 {
return ntpTime(d).Duration()
} else {
return -ntpTime(-d).Duration()
}
}

func minError(org, rec, xmt, dst ntpTime) time.Duration {
Expand Down
21 changes: 21 additions & 0 deletions ntp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// The NTP server to use for online unit tests. May be overridden by the
Expand Down Expand Up @@ -388,6 +389,26 @@ func TestOfflineOffsetCalculationNegative(t *testing.T) {
assert.Equal(t, expectedOffset, offset)
}

func TestOfflineOffsetCalculationNegativeBig(t *testing.T) {
// these timestamps are in different NTP epochs,
// see:
// * https://www.eecis.udel.edu/~mills/y2k.html
serverNow, err := time.Parse("2006-01-02 15:04:05", "2024-05-22 11:30:15")
require.NoError(t, err)

clientNow, err := time.Parse("2006-01-02 15:04:05", "2047-01-01 00:00:00")
require.NoError(t, err)

org := toNtpTime(clientNow)
rec := toNtpTime(serverNow)
xmt := toNtpTime(serverNow.Add(1 * time.Second))
dst := toNtpTime(clientNow.Add(1 * time.Second))

expectedOffset := -clientNow.Sub(serverNow)
offset := offset(org, rec, xmt, dst)
assert.Equal(t, expectedOffset, offset)
}

func TestOfflineReferenceString(t *testing.T) {
cases := []struct {
Stratum byte
Expand Down

0 comments on commit a393ae8

Please sign in to comment.