From 4eab3017b036d3229a6fa7dc9612050d1499e2b6 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Wed, 2 Aug 2023 22:19:27 +0400 Subject: [PATCH] fix: calculate log2i properly Fixes #7080 The real bug was off-by-one in `log2i` implementation, other changes are cleanups as `x/sys/unix` package now contains all the constants we need. Signed-off-by: Andrey Smirnov --- internal/pkg/ntp/interfaces.go | 3 +- internal/pkg/ntp/ntp.go | 31 +++++---- internal/pkg/ntp/ntp_test.go | 6 +- internal/pkg/timex/timex.go | 114 ++++++++++++--------------------- 4 files changed, 66 insertions(+), 88 deletions(-) diff --git a/internal/pkg/ntp/interfaces.go b/internal/pkg/ntp/interfaces.go index db7e658daf..47f6e9ed56 100644 --- a/internal/pkg/ntp/interfaces.go +++ b/internal/pkg/ntp/interfaces.go @@ -9,6 +9,7 @@ import ( "time" "github.com/beevik/ntp" + "golang.org/x/sys/unix" "github.com/siderolabs/talos/internal/pkg/timex" ) @@ -23,4 +24,4 @@ type QueryFunc func(server string) (*ntp.Response, error) type SetTimeFunc func(tv *syscall.Timeval) error // AdjustTimeFunc provides a function to adjust time. -type AdjustTimeFunc func(buf *syscall.Timex) (state timex.State, err error) +type AdjustTimeFunc func(buf *unix.Timex) (state timex.State, err error) diff --git a/internal/pkg/ntp/ntp.go b/internal/pkg/ntp/ntp.go index 90c21fd261..617de92e31 100644 --- a/internal/pkg/ntp/ntp.go +++ b/internal/pkg/ntp/ntp.go @@ -14,7 +14,6 @@ import ( "net" "reflect" "sync" - "syscall" "time" "github.com/beevik/ntp" @@ -22,6 +21,7 @@ import ( "github.com/u-root/u-root/pkg/rtc" "go.uber.org/zap" "go.uber.org/zap/zapcore" + "golang.org/x/sys/unix" "github.com/siderolabs/talos/internal/pkg/timex" ) @@ -394,13 +394,22 @@ func (syncer *Syncer) queryServer(server string) (*ntp.Response, error) { return resp, err } +// log2i returns 0 for v == 0 and v == 1. +func log2i(v uint64) int { + if v == 0 { + return 0 + } + + return 63 - bits.LeadingZeros64(v) +} + // adjustTime adds an offset to the current time. // //nolint:gocyclo func (syncer *Syncer) adjustTime(offset time.Duration, leapSecond ntp.LeapIndicator, server string, nextPollInterval time.Duration) error { var ( buf bytes.Buffer - req syscall.Timex + req unix.Timex jump bool ) @@ -409,9 +418,9 @@ func (syncer *Syncer) adjustTime(offset time.Duration, leapSecond ntp.LeapIndica fmt.Fprintf(&buf, "adjusting time (jump) by %s via %s", offset, server) - req = syscall.Timex{ - Modes: timex.ADJ_SETOFFSET | timex.ADJ_NANO | timex.ADJ_STATUS | timex.ADJ_MAXERROR | timex.ADJ_ESTERROR, - Time: syscall.Timeval{ + req = unix.Timex{ + Modes: unix.ADJ_SETOFFSET | unix.ADJ_NANO | unix.ADJ_STATUS | unix.ADJ_MAXERROR | unix.ADJ_ESTERROR, + Time: unix.Timeval{ Sec: int64(offset / time.Second), Usec: int64(offset / time.Nanosecond % time.Second), }, @@ -428,12 +437,12 @@ func (syncer *Syncer) adjustTime(offset time.Duration, leapSecond ntp.LeapIndica fmt.Fprintf(&buf, "adjusting time (slew) by %s via %s", offset, server) pollSeconds := uint64(nextPollInterval / time.Second) - log2iPollSeconds := 64 - bits.LeadingZeros64(pollSeconds) + log2iPollSeconds := log2i(pollSeconds) - req = syscall.Timex{ - Modes: timex.ADJ_OFFSET | timex.ADJ_NANO | timex.ADJ_STATUS | timex.ADJ_TIMECONST | timex.ADJ_MAXERROR | timex.ADJ_ESTERROR, + req = unix.Timex{ + Modes: unix.ADJ_OFFSET | unix.ADJ_NANO | unix.ADJ_STATUS | unix.ADJ_TIMECONST | unix.ADJ_MAXERROR | unix.ADJ_ESTERROR, Offset: int64(offset / time.Nanosecond), - Status: timex.STA_PLL, + Status: unix.STA_PLL, Maxerror: 0, Esterror: 0, Constant: int64(log2iPollSeconds) - 4, @@ -442,9 +451,9 @@ func (syncer *Syncer) adjustTime(offset time.Duration, leapSecond ntp.LeapIndica switch leapSecond { //nolint:exhaustive case ntp.LeapAddSecond: - req.Status |= timex.STA_INS + req.Status |= unix.STA_INS case ntp.LeapDelSecond: - req.Status |= timex.STA_DEL + req.Status |= unix.STA_DEL } logLevel := zapcore.DebugLevel diff --git a/internal/pkg/ntp/ntp_test.go b/internal/pkg/ntp/ntp_test.go index e45495ad4d..a9f95309f9 100644 --- a/internal/pkg/ntp/ntp_test.go +++ b/internal/pkg/ntp/ntp_test.go @@ -9,7 +9,6 @@ import ( "fmt" "log" "sync" - "syscall" "testing" "time" @@ -17,6 +16,7 @@ import ( "github.com/siderolabs/go-retry/retry" "github.com/stretchr/testify/suite" "go.uber.org/zap" + "golang.org/x/sys/unix" "github.com/siderolabs/talos/internal/pkg/ntp" "github.com/siderolabs/talos/internal/pkg/timex" @@ -57,11 +57,11 @@ func (suite *NTPSuite) getSystemClock() time.Time { return suite.systemClock } -func (suite *NTPSuite) adjustSystemClock(val *syscall.Timex) (status timex.State, err error) { +func (suite *NTPSuite) adjustSystemClock(val *unix.Timex) (status timex.State, err error) { suite.clockLock.Lock() defer suite.clockLock.Unlock() - if val.Modes&timex.ADJ_OFFSET == timex.ADJ_OFFSET { + if val.Modes&unix.ADJ_OFFSET == unix.ADJ_OFFSET { suite.T().Logf("adjustment by %s", time.Duration(val.Offset)*time.Nanosecond) suite.clockAdjustments = append(suite.clockAdjustments, time.Duration(val.Offset)*time.Nanosecond) } else { diff --git a/internal/pkg/timex/timex.go b/internal/pkg/timex/timex.go index 83cd850a16..2a4e5fed01 100644 --- a/internal/pkg/timex/timex.go +++ b/internal/pkg/timex/timex.go @@ -7,74 +7,39 @@ package timex import ( "strings" - "syscall" -) -// Values for timex.mode. -// -//nolint:golint,stylecheck,revive -const ( - ADJ_OFFSET = 0x0001 - ADJ_FREQUENCY = 0x0002 - ADJ_MAXERROR = 0x0004 - ADJ_ESTERROR = 0x0008 - ADJ_STATUS = 0x0010 - ADJ_TIMECONST = 0x0020 - ADJ_TAI = 0x0080 - ADJ_SETOFFSET = 0x0100 - ADJ_MICRO = 0x1000 - ADJ_NANO = 0x2000 - ADJ_TICK = 0x4000 + "golang.org/x/sys/unix" ) // Status is bitmask field of statuses. type Status int32 -// Clock statuses. -// -//nolint:golint,stylecheck,revive -const ( - STA_PLL = 0x0001 /* enable PLL updates (rw) */ - STA_PPSFREQ = 0x0002 /* enable PPS freq discipline (rw) */ - STA_PPSTIME = 0x0004 /* enable PPS time discipline (rw) */ - STA_FLL = 0x0008 /* select frequency-lock mode (rw) */ - STA_INS = 0x0010 /* insert leap (rw) */ - STA_DEL = 0x0020 /* delete leap (rw) */ - STA_UNSYNC = 0x0040 /* clock unsynchronized (rw) */ - STA_FREQHOLD = 0x0080 /* hold frequency (rw) */ - STA_PPSSIGNAL = 0x0100 /* PPS signal present (ro) */ - STA_PPSJITTER = 0x0200 /* PPS signal jitter exceeded (ro) */ - STA_PPSWANDER = 0x0400 /* PPS signal wander exceeded (ro) */ - STA_PPSERROR = 0x0800 /* PPS signal calibration error (ro) */ - STA_CLOCKERR = 0x1000 /* clock hardware fault (ro) */ - STA_NANO = 0x2000 /* resolution (0 = us, 1 = ns) (ro) */ - STA_MODE = 0x4000 /* mode (0 = PLL, 1 = FLL) (ro) */ - STA_CLK = 0x8000 /* clock source (0 = A, 1 = B) (ro) */ -) - func (status Status) String() string { var labels []string - for bit, label := range map[Status]string{ - STA_PLL: "STA_PLL", - STA_PPSFREQ: "STA_PPSFREQ", - STA_PPSTIME: "STA_PPSTIME", - STA_FLL: "STA_FLL", - STA_INS: "STA_INS", - STA_DEL: "STA_DEL", - STA_UNSYNC: "STA_UNSYNC", - STA_FREQHOLD: "STA_FREQHOLD", - STA_PPSSIGNAL: "STA_PPSSIGNAL", - STA_PPSJITTER: "STA_PPSJITTER", - STA_PPSWANDER: "STA_PPSWANDER", - STA_PPSERROR: "STA_PPSERROR", - STA_CLOCKERR: "STA_CLOCKERR", - STA_NANO: "STA_NANO", - STA_MODE: "STA_MODE", - STA_CLK: "STA_CLK", + for _, item := range []struct { + bit Status + label string + }{ + {unix.STA_PLL, "STA_PLL"}, /* enable PLL updates (rw) */ + {unix.STA_PPSFREQ, "STA_PPSFREQ"}, /* enable PPS freq discipline (rw) */ + {unix.STA_PPSTIME, "STA_PPSTIME"}, /* enable PPS time discipline (rw) */ + {unix.STA_FLL, "STA_FLL"}, /* select frequency-lock mode (rw) */ + {unix.STA_INS, "STA_INS"}, /* insert leap (rw) */ + {unix.STA_DEL, "STA_DEL"}, /* delete leap (rw) */ + {unix.STA_UNSYNC, "STA_UNSYNC"}, /* clock unsynchronized (rw) */ + {unix.STA_FREQHOLD, "STA_FREQHOLD"}, /* hold frequency (rw) */ + {unix.STA_PPSSIGNAL, "STA_PPSSIGNAL"}, /* PPS signal present (ro) */ + {unix.STA_PPSJITTER, "STA_PPSJITTER"}, /* PPS signal jitter exceeded (ro) */ + {unix.STA_PPSWANDER, "STA_PPSWANDER"}, /* PPS signal wander exceeded (ro) */ + {unix.STA_PPSERROR, "STA_PPSERROR"}, /* PPS signal calibration error (ro) */ + {unix.STA_CLOCKERR, "STA_CLOCKERR"}, /* clock hardware fault (ro) */ + {unix.STA_NANO, "STA_NANO"}, /* resolution (0 = us, 1 = ns) (ro) */ + {unix.STA_MODE, "STA_MODE"}, /* mode (0 = PLL, 1 = FLL) (ro) */ + {unix.STA_CLK, "STA_CLK"}, /* clock source (0 = A, 1 = B) (ro) */ } { - if (status & bit) == bit { - labels = append(labels, label) + if (status & item.bit) == item.bit { + labels = append(labels, item.label) } } @@ -84,25 +49,28 @@ func (status Status) String() string { // State is clock state. type State int -// Clock states. -// -//nolint:golint,stylecheck,revive -const ( - TIME_OK State = iota - TIME_INS - TIME_DEL - TIME_OOP - TIME_WAIT - TIME_ERROR -) - func (state State) String() string { - return [...]string{"TIME_OK", "TIME_INS", "TIME_DEL", "TIME_OOP", "TIME_WAIT", "TIME_ERROR"}[int(state)] + switch state { + case unix.TIME_OK: + return "TIME_OK" + case unix.TIME_INS: + return "TIME_INS" + case unix.TIME_DEL: + return "TIME_DEL" + case unix.TIME_OOP: + return "TIME_OOP" + case unix.TIME_WAIT: + return "TIME_WAIT" + case unix.TIME_ERROR: + return "TIME_ERROR" + default: + return "TIME_UNKNOWN" + } } // Adjtimex provides a wrapper around syscall.Adjtimex. -func Adjtimex(buf *syscall.Timex) (state State, err error) { - st, err := syscall.Adjtimex(buf) +func Adjtimex(buf *unix.Timex) (state State, err error) { + st, err := unix.ClockAdjtime(unix.CLOCK_REALTIME, buf) return State(st), err }