Skip to content

start without PTP when /dev/fbclock/ptp or the PHC is missing (#592) #592

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

Closed
ESoapW wants to merge 1 commit into from
Closed

start without PTP when /dev/fbclock/ptp or the PHC is missing (#592) #592

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
96 changes: 82 additions & 14 deletions fbclock/daemon/daemon.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,15 @@ var lastLeapSmearing = clockSmearing{
utcOffsetPostS: utcOffsetS,
}

var (
// ptpDevicePath is where New and Run's re-check look for the PTP device; tests point it
// elsewhere.
ptpDevicePath = ManagedPTPDevicePath
// ptpRecheckInterval is how often a host without PTP checks for it again: the pace at
// which systemd restarts the sidecar after it exits. Tests shorten it.
ptpRecheckInterval = 10 * time.Second
)

// DataPoint is what we store in DataPoint ring buffer
type DataPoint struct {
// IngressTimeNS represents ingress time in NanoSeconds
Expand Down Expand Up @@ -138,6 +147,9 @@ type Daemon struct {
stats stats.Server
l Logger

// sources is what this host offers, decided once in New; v3's header lists exactly these
sources fbclock.Sources

// function to get PHC time from configured PHC device
getPHCTime func() (time.Time, error)
// getPHCAndSysTime returns PHC time, system time, system clock id, and the
Expand Down Expand Up @@ -221,7 +233,8 @@ func (s *Daemon) setupPHC(iface string) error {
return nil
}

// New creates new fbclock-daemon
// New creates new fbclock-daemon. It runs PTP only where ManagedPTPDevicePath exists and
// cfg.Iface has a PHC, and offers ntp everywhere.
func New(cfg *Config, stats stats.Server, l Logger) (*Daemon, error) {
// we need at least 1m of samples for aggregate values
effectiveRingSize := minRingSize(cfg.RingSize, cfg.Interval)
Expand All @@ -237,9 +250,22 @@ func New(cfg *Config, stats stats.Server, l Logger) (*Daemon, error) {
s.DataFetcher = &SockFetcher{}
}

if err := s.setupPHC(cfg.Iface); err != nil {
return nil, err
// chrony runs on every host the sidecar runs on
s.sources = fbclock.SourceNTP
// the device marks a PTP host, which a PHC alone doesn't: non-PTP NICs have one too
_, err := os.Stat(ptpDevicePath)
if err == nil {
err = s.setupPHC(cfg.Iface)
}
if err != nil {
if !cfg.EnableDataV3 {
return nil, fmt.Errorf("nothing to write without PTP and -v3: %w", err)
}
log.Infof("running without PTP, writing only ntp to %s: %v", fbclock.DataPath, err)
return s, nil
}
s.sources |= fbclock.SourcePTP | fbclock.SourcePRTC
log.Infof("running with PTP: %s exists and %s has a PHC", ptpDevicePath, cfg.Iface)
// calculated values
s.stats.SetCounter("m_ns", 0)
s.stats.SetCounter("w_ns", 0)
Expand Down Expand Up @@ -598,17 +624,19 @@ func (s *Daemon) meanCoeffPPBV3(prev, cur *fbclock.DataClock) int64 {
return s.state.getMeanCoeffPPBV3(coefPPB)
}

// openDataV3 opens the v3 file at path for -v3. It returns nil when -v3 is off or
// the open fails: v3 must never stop the daemon.
// openDataV3 opens the v3 file at path for -v3, with a section for each of the host's sources.
// It returns nil when -v3 is off or the open fails, which stops only a host without PTP.
func (s *Daemon) openDataV3(path string) *fbclock.DataFile {
if !s.cfg.EnableDataV3 {
return nil
}
v3, err := fbclock.OpenFBClockDataCustom(path, map[fbclock.Sources]fbclock.DataSectionConfig{
fbclock.SourcePTP: {},
fbclock.SourcePRTC: {},
fbclock.SourceNTP: {},
})
sections := map[fbclock.Sources]fbclock.DataSectionConfig{}
for _, source := range []fbclock.Sources{fbclock.SourcePTP, fbclock.SourcePRTC, fbclock.SourceNTP} {
if s.sources&source != 0 {
sections[source] = fbclock.DataSectionConfig{}
}
}
v3, err := fbclock.OpenFBClockDataCustom(path, sections)
if err != nil {
log.Errorf("not writing fbclock data v3: %v", err)
return nil
Expand Down Expand Up @@ -876,8 +904,50 @@ func sdNotify() error {
return nil
}

// Run a daemon
// ptpAvailable reports whether New would now run PTP: the device exists and -iface's PHC opens.
func (s *Daemon) ptpAvailable() bool {
if _, err := os.Stat(ptpDevicePath); err != nil {
return false
}
return s.setupPHC(s.cfg.Iface) == nil
}

// waitForPTP checks every ptpRecheckInterval whether New would now run PTP, and returns an
// error once it would, so the process exits and systemd's restart runs PTP. It returns nil if
// ctx is done first.
func (s *Daemon) waitForPTP(ctx context.Context) error {
ticker := time.NewTicker(ptpRecheckInterval)
defer ticker.Stop()
for {
select {
case <-ctx.Done():
return nil
case <-ticker.C:
}
if s.ptpAvailable() {
return errors.New("PTP is available now: exiting so the restart runs it")
}
}
}

// Run writes v3's ntp on every host and, where New found PTP, everything PTP writes.
func (s *Daemon) Run(ctx context.Context) error {
// v3 isn't closed: the loops write to its mapping until the process exits
v3 := s.openDataV3(fbclock.DataPath)
if v3 != nil {
go s.populateNTPDataV3(ctx, &ChronyFetcher{}, v3)
}
if s.sources&fbclock.SourcePTP == 0 {
if v3 == nil {
return errors.New("nothing to write without PTP: fbclock data v3 didn't open")
}
if err := sdNotify(); err != nil {
log.Error(err)
}
// PTP can come later: the link once udev makes it, the PHC once it opens
return s.waitForPTP(ctx)
}

shm, err := fbclock.OpenFBClockSHMv1()
if err != nil {
return fmt.Errorf("opening fbclock shm: %w", err)
Expand All @@ -888,10 +958,8 @@ func (s *Daemon) Run(ctx context.Context) error {
go s.runLinearizabilityTests(ctx)
}

// v3 isn't closed: the loops below write to its mapping until the process exits
if v3 := s.openDataV3(fbclock.DataPath); v3 != nil {
if v3 != nil {
go s.populatePTPDataV3(v3)
go s.populateNTPDataV3(ctx, &ChronyFetcher{}, v3)
}

if s.cfg.EnableDataV2 {
Expand Down
89 changes: 85 additions & 4 deletions fbclock/daemon/daemon_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ func newTestDaemon(cfg *Config, stats stats.Server) *Daemon {
state: newDaemonState(cfg.RingSize),
l: &testLogger{samples: []*LogSample{}},
DataFetcher: &SockFetcher{},
// what New sets on a PTP host
sources: fbclock.SourcePTP | fbclock.SourcePRTC | fbclock.SourceNTP,
}
return s
}
Expand Down Expand Up @@ -1089,20 +1091,24 @@ func dataSections(path string) (*fbclock.DataFileSnapshot, map[string]fbclock.Da
}

func TestOpenDataV3(t *testing.T) {
const ptpHost = fbclock.SourcePTP | fbclock.SourcePRTC | fbclock.SourceNTP
testCases := []struct {
name string
sources fbclock.Sources
v2, v3 bool
failOpen bool // a directory where the file goes
wantSources uint32 // 0: no file
}{
{name: "all sections with -v3", v2: true, v3: true, wantSources: uint32(fbclock.SourcePTP | fbclock.SourcePRTC | fbclock.SourceNTP)},
{name: "all sections with -v3, -v2 off", v3: true, wantSources: uint32(fbclock.SourcePTP | fbclock.SourcePRTC | fbclock.SourceNTP)},
{name: "no file with -v3 off", v2: true},
{name: "nil when the open fails", v2: true, v3: true, failOpen: true},
{name: "PTP host: ptp, prtc and ntp", sources: ptpHost, v2: true, v3: true, wantSources: 0x7},
{name: "PTP host, -v2 off: still all three", sources: ptpHost, v3: true, wantSources: 0x7},
{name: "no PTP: ntp only", sources: fbclock.SourceNTP, v2: true, v3: true, wantSources: 0x4},
{name: "no file with -v3 off", sources: ptpHost, v2: true},
{name: "nil when the open fails", sources: ptpHost, v2: true, v3: true, failOpen: true},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
s := newTestDaemon(&Config{RingSize: 1, Interval: time.Second, EnableDataV2: tc.v2, EnableDataV3: tc.v3}, stats.NewStats())
s.sources = tc.sources
path := filepath.Join(t.TempDir(), "data")
if tc.failOpen {
require.NoError(t, os.Mkdir(path, 0o755))
Expand Down Expand Up @@ -1258,3 +1264,78 @@ func TestPopulatePTPDataV3(t *testing.T) {
})
}
}

// withPTPDevice points New and Run's re-check at path until t ends.
func withPTPDevice(t *testing.T, path string) {
old := ptpDevicePath
ptpDevicePath = path
t.Cleanup(func() { ptpDevicePath = old })
}

func TestNewWithoutPTP(t *testing.T) {
// noPHCIface doesn't exist, so it has no PHC
const noPHCIface = "fbclock-test"
dir := t.TempDir()
present, absent := filepath.Join(dir, "ptp"), filepath.Join(dir, "none")
require.NoError(t, os.WriteFile(present, nil, 0o600))
for _, tc := range []struct {
name string
device string
v3 bool
wantErr string // none: New runs without PTP
}{
{name: "no device", device: absent, v3: true},
{name: "device but no PHC", device: present, v3: true},
// the error names what's missing, and the PHC is only looked for once the device is there
{name: "no device, v3 off", device: absent, wantErr: "nothing to write without PTP and -v3: stat " + absent},
{name: "no PHC, v3 off", device: present, wantErr: `nothing to write without PTP and -v3: finding PHC device for "fbclock-test"`},
} {
t.Run(tc.name, func(t *testing.T) {
cfg := &Config{Iface: noPHCIface, RingSize: 1, Interval: time.Second, EnableDataV2: true, EnableDataV3: tc.v3}
st := stats.NewStats()
withPTPDevice(t, tc.device)
s, err := New(cfg, st, &testLogger{})
if tc.wantErr != "" {
require.ErrorContains(t, err, tc.wantErr)
return
}
require.NoError(t, err)
require.Equal(t, fbclock.SourceNTP, s.sources)
// none of PTP's counters, such as m_ns, are seeded
require.Empty(t, st.Get())
})
}
}

// withRecheckInterval shortens Run's re-check of PTP until t ends.
func withRecheckInterval(t *testing.T, d time.Duration) {
old := ptpRecheckInterval
ptpRecheckInterval = d
t.Cleanup(func() { ptpRecheckInterval = old })
}

func TestWaitForPTP(t *testing.T) {
dir := t.TempDir()
present := filepath.Join(dir, "ptp")
require.NoError(t, os.WriteFile(present, nil, 0o600))
for _, tc := range []struct {
name string
device string
}{
{name: "no device", device: filepath.Join(dir, "none")},
// the link is there but -iface has no PHC, so New still wouldn't run PTP
{name: "device but no PHC", device: present},
} {
t.Run(tc.name, func(t *testing.T) {
withPTPDevice(t, tc.device)
withRecheckInterval(t, time.Millisecond)
s := newTestDaemon(&Config{Iface: "fbclock-test", RingSize: 1, Interval: time.Second}, stats.NewStats())
require.False(t, s.ptpAvailable())
// it keeps checking and only returns, without an error, once ctx is done
ctx, cancel := context.WithTimeout(t.Context(), 50*time.Millisecond)
defer cancel()
require.NoError(t, s.waitForPTP(ctx))
require.Error(t, ctx.Err(), "returned before ctx was done")
})
}
}
Loading