Skip to content

Commit

Permalink
fix: calculate log2i properly
Browse files Browse the repository at this point in the history
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 <andrey.smirnov@talos-systems.com>
  • Loading branch information
smira committed Aug 3, 2023
1 parent bcf2845 commit 4eab301
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 88 deletions.
3 changes: 2 additions & 1 deletion internal/pkg/ntp/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/beevik/ntp"
"golang.org/x/sys/unix"

"github.com/siderolabs/talos/internal/pkg/timex"
)
Expand All @@ -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)
31 changes: 20 additions & 11 deletions internal/pkg/ntp/ntp.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ import (
"net"
"reflect"
"sync"
"syscall"
"time"

"github.com/beevik/ntp"
"github.com/siderolabs/gen/slices"
"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"
)
Expand Down Expand Up @@ -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
)

Expand All @@ -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),
},
Expand All @@ -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,
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/ntp/ntp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ import (
"fmt"
"log"
"sync"
"syscall"
"testing"
"time"

beevikntp "github.com/beevik/ntp"
"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"
Expand Down Expand Up @@ -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 {
Expand Down
114 changes: 41 additions & 73 deletions internal/pkg/timex/timex.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -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
}

0 comments on commit 4eab301

Please sign in to comment.