Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: calculate log2i properly #7567

Merged
merged 1 commit into from
Aug 3, 2023
Merged

fix: calculate log2i properly #7567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion internal/pkg/ntp/interfaces.go
View file
Open in desktop
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
View file
Open in desktop
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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the real fix, previous code was s/63/64.

the code in systemd for comparison: https://github.com/systemd/systemd/blob/6639ac474eb7a5325a72a3d7370492792dd00bc0/src/fundamental/logarithm.h#L39-L40

}

// 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
View file
Open in desktop
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
View file
Open in desktop
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
}