tidy up SetSync usage

Summary:
Tidy things up a bit, making sure all functions in `clock.go` take
clockid as an argument and keeping sptp client tests from changing sys clock state.

Reviewed By: leoleovich

Differential Revision: D50081869

fbshipit-source-id: f7c066208ce0d65dca300ef52e19c4aba316f662

clock/clock.go

@@ -125,11 +125,10 @@ func MaxFreqPPB(clockid int32) (freqPPB float64, state int, err error) {
 }
 
 // SetSync sets clock status to TIME_OK
-func SetSync() error {
+func SetSync(clockid int32) error {
 	tx := &unix.Timex{}
-	// man(2) clock_adjtime, turn ppb to ppm
 	tx.Modes = AdjStatus | AdjMaxError
-	state, err := Adjtime(unix.CLOCK_REALTIME, tx)
+	state, err := Adjtime(clockid, tx)
 
 	if err == nil && state != unix.TIME_OK {
 		return fmt.Errorf("clock state %d is not TIME_OK after setting sync state", state)

cmd/ptpcheck/cmd/phc2phc.go

@@ -24,6 +24,7 @@ import (
 	"github.com/facebook/time/servo"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
+	"golang.org/x/sys/unix"
 )
 
 var (
@@ -93,7 +94,7 @@ func phc2phcRun(srcDevice string, dstDevice string, interval time.Duration, step
 				log.Errorf("failed to adjust freq to %v: %v", -freqAdj, err)
 				continue
 			}
-			if err := clock.SetSync(); err != nil {
+			if err := clock.SetSync(unix.CLOCK_REALTIME); err != nil {
 				log.Errorf("failed to set sys clock sync state: %v", err)
 			}
 		}

ptp/sptp/client/clock.go

@@ -32,6 +32,7 @@ type Clock interface {
 	Step(step time.Duration) error
 	FrequencyPPB() (float64, error)
 	MaxFreqPPB() (float64, error)
+	SetSync() error
 }
 
 // PHC groups methods for interactions with PHC devices
@@ -70,6 +71,11 @@ func (p *PHC) MaxFreqPPB() (float64, error) {
 	return phc.MaxFreqAdjPPBFromDevice(p.devicePath)
 }
 
+// SetSync is a no-op for PHC
+func (p *PHC) SetSync() error {
+	return nil
+}
+
 // SysClock groups methods for interacting with system clock
 type SysClock struct{}
 
@@ -82,6 +88,11 @@ func (c *SysClock) AdjFreqPPB(freqPPB float64) error {
 	return err
 }
 
+// SetSync sets clock status to TIME_OK
+func (c *SysClock) SetSync() error {
+	return clock.SetSync(unix.CLOCK_REALTIME)
+}
+
 // Step jumps time on PHC
 func (c *SysClock) Step(step time.Duration) error {
 	state, err := clock.Step(unix.CLOCK_REALTIME, step)
@@ -131,3 +142,8 @@ func (c *FreeRunningClock) FrequencyPPB() (float64, error) {
 func (c *FreeRunningClock) MaxFreqPPB() (float64, error) {
 	return 0.0, nil
 }
+
+// SetSync sets clock status to TIME_OK
+func (c *FreeRunningClock) SetSync() error {
+	return nil
+}

ptp/sptp/client/sptp.go

@@ -28,7 +28,6 @@ import (
 	"golang.org/x/sync/errgroup"
 	"golang.org/x/sys/unix"
 
-	"github.com/facebook/time/clock"
 	"github.com/facebook/time/dscp"
 	"github.com/facebook/time/phc"
 	ptp "github.com/facebook/time/ptp/protocol"
@@ -396,8 +395,8 @@ func (p *SPTP) processResults(results map[string]*RunResult) {
 		if err := p.clock.AdjFreqPPB(-1 * freqAdj); err != nil {
 			log.Errorf("failed to adjust freq to %v: %v", -1*freqAdj, err)
 		}
-		if err := clock.SetSync(); err != nil {
-			log.Errorf("failed to set sys clock sync state")
+		if err := p.clock.SetSync(); err != nil {
+			log.Errorf("failed to set clock sync state")
 		}
 	}
 }

ptp/sptp/client/sptp_test.go

@@ -81,8 +81,9 @@ func TestProcessResultsEmptyResult(t *testing.T) {
 	results := map[string]*RunResult{
 		"192.168.0.10": {},
 	}
-	mockServo.EXPECT().MeanFreq()
-	mockClock.EXPECT().AdjFreqPPB(gomock.Any())
+	meanFreq := 10.2
+	mockServo.EXPECT().MeanFreq().Return(meanFreq)
+	mockClock.EXPECT().AdjFreqPPB(-1 * meanFreq)
 	mockStatsServer.EXPECT().SetCounter("ptp.sptp.gms.total", int64(1))
 	mockStatsServer.EXPECT().SetCounter("ptp.sptp.gms.available_pct", int64(0))
 	mockStatsServer.EXPECT().SetGMStats(gomock.Any())
@@ -98,6 +99,7 @@ func TestProcessResultsSingle(t *testing.T) {
 	mockClock := NewMockClock(ctrl)
 	mockClock.EXPECT().AdjFreqPPB(gomock.Any()).Return(nil)
 	mockClock.EXPECT().Step(gomock.Any()).Return(nil)
+	mockClock.EXPECT().SetSync()
 	mockServo := NewMockServo(ctrl)
 	mockServo.EXPECT().Sample(int64(-200002000), gomock.Any()).Return(12.3, servo.StateJump)
 	mockServo.EXPECT().Sample(int64(-100001000), gomock.Any()).Return(14.2, servo.StateLocked)
@@ -152,6 +154,7 @@ func TestProcessResultsMulti(t *testing.T) {
 	mockClock := NewMockClock(ctrl)
 	mockClock.EXPECT().AdjFreqPPB(gomock.Any()).Return(nil)
 	mockClock.EXPECT().Step(gomock.Any()).Return(nil)
+	mockClock.EXPECT().SetSync()
 	mockServo := NewMockServo(ctrl)
 	mockServo.EXPECT().Sample(int64(-200002000), gomock.Any()).Return(12.3, servo.StateJump)
 	mockServo.EXPECT().Sample(int64(-104002000), gomock.Any()).Return(14.2, servo.StateLocked)