From 4cdf08dc1502dc7cb0d185f4c7866fdc5fb664b0 Mon Sep 17 00:00:00 2001 From: Yixin Wei Date: Fri, 30 Oct 2026 17:46:16 -0700 Subject: [PATCH] start without PTP when /dev/fbclock/ptp or the PHC is missing (#592) Summary: This diff adds the conditions for what the sidecar writes. It writes NTP on every host, with -v3. It writes PTP and PRTC, and everything else PTP needs (v1, v2 and the linearizability tests), only on a PTP host: one where /dev/fbclock/ptp exists and -iface's PHC opens. A PTP host runs exactly as today; only hosts without PTP change. Today the sidecar can't run on a host without PTP. It exits at startup without a PHC, and it always writes v1, runs the sptp loop and starts the linearizability tests. That blocks -v3 on those hosts. Chrony mode used to cover them, and D127943753 removed it. The daemon checks the conditions at startup. The device is what marks a PTP host, since non-PTP NICs can have a PHC too. Any error from either check counts as no PTP, and the startup log line says why. A host without PTP then checks again every 10 s, the pace at which systemd restarts a sidecar that exits, and once the device exists and the PHC opens, it exits so that the restart runs PTP. So a link that udev makes after the sidecar started, at boot or on a turnup, or a PHC that comes back after a NIC reset, gets PTP without anyone restarting the sidecar, and ntp keeps flowing meanwhile. That matters once -manage goes off: today, with -manage on, main exits when -iface has no PHC and systemd retries. Without PTP the sidecar writes only v3's ntp section (header 0x4) and keeps running. There's no PHC setup, no v1 or v2 file, no sptp loop, no PTP counters and no linearizability tests, whose failures fbclock.mon alerts on and FBAR answers by restarting sptp. It exits at startup when it has nothing to write: without -v3, or when /run/fbclock/data can't be opened. With -manage on, main exits before the daemon starts when -iface has no PHC, so for the v3 rollout those hosts run with -manage=false. How the sidecar starts, before and after: https://pxl.cl/hX3GC In code, New checks the conditions and keeps the result, openDataV3 builds v3's header from it (0x7 on a PTP host, 0x4 without), and Run starts the ntp loop on every host. On a host without PTP, Run then waits there and re-checks every 10 s. Everything below that stop is today's Run, except that it no longer opens v3 itself. The open-source fbclock-daemon shares this code. A host with a PHC, -manage=false and no /dev/fbclock/ptp now writes only ntp, or fails without -v3, where today's daemon runs PTP. The open-source client library doesn't init there on default options, so those setups don't change. A caller that opted into NTP or ANY loses data_v2 there. No new files or flags. Without PTP the daemon creates only /run/fbclock/data, which it already creates with -v3. Differential Revision: D128232627 --- fbclock/daemon/daemon.go | 96 ++++++++++++++++++++++++++++++----- fbclock/daemon/daemon_test.go | 89 ++++++++++++++++++++++++++++++-- 2 files changed, 167 insertions(+), 18 deletions(-) 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") + }) + } +}