diff --git a/fbclock/daemon/daemon.go b/fbclock/daemon/daemon.go index 4a44a00e..24b501e2 100644 --- a/fbclock/daemon/daemon.go +++ b/fbclock/daemon/daemon.go @@ -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 @@ -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 @@ -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) @@ -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) @@ -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 @@ -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) @@ -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 { diff --git a/fbclock/daemon/daemon_test.go b/fbclock/daemon/daemon_test.go index a32183b7..76668f76 100644 --- a/fbclock/daemon/daemon_test.go +++ b/fbclock/daemon/daemon_test.go @@ -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 } @@ -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)) @@ -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") + }) + } +}