Add step_threshold flag (#401)

Summary:
Pull Request resolved: #401

WHAT?

Adds step_threshold flag, allowing users to specify a threshold for adjustments, causing larger offsets to step the clock instead of adjusting it.

WHY?

Help clock time converge faster when needed.

Reviewed By: abulimov

Differential Revision: D63966619

fbshipit-source-id: 73ac6dcf340d5b5593f3d34a9c5c4cfc9e29be26

cmd/ptpcheck/cmd/ts2phc.go

@@ -27,27 +27,29 @@ import (
 )
 
 var (
-	srcDeviceTS2PHCFlag string
-	dstDeviceTS2PHCFlag string
-	intervalTS2PHCFlag  time.Duration
-	firstStepTS2PHCFlag time.Duration
-	srcPinTS2PHCFlag    uint
-	dstPinTS2PHCFlag    uint
-	maxFreqTS2PHCFlag   float64
+	srcDeviceTS2PHCFlag     string
+	dstDeviceTS2PHCFlag     string
+	intervalTS2PHCFlag      time.Duration
+	srcPinTS2PHCFlag        uint
+	dstPinTS2PHCFlag        uint
+	maxFreqTS2PHCFlag       float64
+	firstStepTS2PHCFlag     time.Duration
+	stepThresholdTS2PHCFlag time.Duration
 )
 
 func init() {
 	RootCmd.AddCommand(ts2phcCmd)
 	ts2phcCmd.Flags().StringVarP(&srcDeviceTS2PHCFlag, "source", "s", "/dev/ptp_tcard", "Source for Time of Day (ToD) data. Only PHC devices are supported at the moment")
 	ts2phcCmd.Flags().StringVarP(&dstDeviceTS2PHCFlag, "destination", "d", "eth0", "PHC to be synchronized. The clock may be identified by its character device (like /dev/ptp0) or its associated network interface (like eth0).")
 	ts2phcCmd.Flags().DurationVarP(&intervalTS2PHCFlag, "interval", "i", time.Second, "Interval between syncs in nanosseconds")
-	ts2phcCmd.Flags().DurationVarP(&firstStepTS2PHCFlag, "first_step", "f", 20*time.Microsecond, "The maximum offset, specified in seconds, that the servo will correct by changing the clock frequency instead of stepping the clock. This is only applied on the first update. When set to 0.0, the servo will not step the clock on start.")
+	ts2phcCmd.Flags().DurationVarP(&firstStepTS2PHCFlag, "first_step", "f", 20*time.Microsecond, "The maximum offset that the servo will correct by changing the clock frequency instead of stepping the clock. This is only applied on the first update. When set to 0, the servo will not step the clock on start.")
 	ts2phcCmd.Flags().UintVarP(&srcPinTS2PHCFlag, "out-pin", "n", phc.DefaultTs2PhcIndex, "output pin number of the PPS signal on source device.")
 	ts2phcCmd.Flags().UintVarP(&dstPinTS2PHCFlag, "in-pin", "o", phc.DefaultTs2PhcSinkIndex, "input pin number of the PPS signal on destination device. (default 0)")
 	ts2phcCmd.Flags().Float64VarP(&maxFreqTS2PHCFlag, "max_frequency", "m", 0, "maximum frequency in parts per billion (PPB) that the servo will correct by changing the clock frequency instead of stepping the clock. If unset, uses the maximum frequency reported by the PHC device.")
+	ts2phcCmd.Flags().DurationVarP(&stepThresholdTS2PHCFlag, "step_threshold", "t", 0, "The maximum offset that the servo will correct by changing the clock frequency instead of stepping the clock. When set to 0, the servo will never step the clock except on start.")
 }
 
-func ts2phcRun(srcDevicePath string, dstDeviceName string, interval time.Duration, stepth time.Duration, srcPinIndex uint) error {
+func ts2phcRun(srcDevicePath string, dstDeviceName string, interval time.Duration, firstStepth time.Duration, stepth time.Duration, srcPinIndex uint) error {
 	ppsSource, err := getPPSSourceFromPath(srcDevicePath, srcPinIndex)
 	if err != nil {
 		return fmt.Errorf("error opening source phc device: %w", err)
@@ -61,7 +63,7 @@ func ts2phcRun(srcDevicePath string, dstDeviceName string, interval time.Duratio
 		return fmt.Errorf("error setting target device as PPS sink: %w", err)
 	}
 
-	pi, err := phc.NewPiServo(interval, stepth, dstDevice, maxFreqTS2PHCFlag)
+	pi, err := phc.NewPiServo(interval, firstStepth, stepth, dstDevice, maxFreqTS2PHCFlag)
 	if err != nil {
 		return fmt.Errorf("error getting servo: %w", err)
 	}
@@ -117,7 +119,7 @@ var ts2phcCmd = &cobra.Command{
 	Short: "Sync PHC with external timestamps",
 	Run: func(_ *cobra.Command, _ []string) {
 		ConfigureVerbosity()
-		if err := ts2phcRun(srcDeviceTS2PHCFlag, dstDeviceTS2PHCFlag, intervalTS2PHCFlag, firstStepTS2PHCFlag, srcPinTS2PHCFlag); err != nil {
+		if err := ts2phcRun(srcDeviceTS2PHCFlag, dstDeviceTS2PHCFlag, intervalTS2PHCFlag, firstStepTS2PHCFlag, stepThresholdTS2PHCFlag, srcPinTS2PHCFlag); err != nil {
 			log.Fatal(err)
 		}
 	},

phc/pps_source.go

@@ -182,14 +182,14 @@ func (ppsSource *PPSSource) Timestamp() (*time.Time, error) {
 }
 
 // NewPiServo returns a servo.PiServo object configure for synchronizing the given device. maxFreq 0 is equivalent to no maxFreq
-func NewPiServo(interval time.Duration, stepth time.Duration, device FrequencyGetter, maxFreq float64) (*servo.PiServo, error) {
+func NewPiServo(interval time.Duration, firstStepth time.Duration, stepth time.Duration, device FrequencyGetter, maxFreq float64) (*servo.PiServo, error) {
 	servoCfg := servo.DefaultServoConfig()
-	if stepth != 0 {
+	if firstStepth != 0 {
 		// allow stepping clock on first update
 		servoCfg.FirstUpdate = true
-		servoCfg.FirstStepThreshold = int64(stepth)
+		servoCfg.FirstStepThreshold = int64(firstStepth)
 	}
-
+	servoCfg.StepThreshold = int64(stepth)
 	freq, err := device.FreqPPB()
 	if err != nil {
 		return nil, err

phc/pps_source_test.go

@@ -346,7 +346,7 @@ func TestNewPiServo(t *testing.T) {
 		mockFrequencyGetter.EXPECT().MaxFreqAdjPPB().Return(3.0, nil),
 	)
 
-	servo, err := NewPiServo(time.Duration(1), time.Duration(1), mockFrequencyGetter, 0.0)
+	servo, err := NewPiServo(time.Duration(1), time.Duration(1), time.Duration(0), mockFrequencyGetter, 0.0)
 
 	require.NoError(t, err)
 	require.Equal(t, int64(1), servo.Servo.FirstStepThreshold)
@@ -364,7 +364,7 @@ func TestNewPiServoFreqPPBError(t *testing.T) {
 		mockFrequencyGetter.EXPECT().FreqPPB().Return(1.0, fmt.Errorf("error")),
 	)
 
-	_, err := NewPiServo(time.Duration(1), time.Duration(1), mockFrequencyGetter, 0.0)
+	_, err := NewPiServo(time.Duration(1), time.Duration(1), time.Duration(0), mockFrequencyGetter, 0.0)
 
 	require.Error(t, err)
 }
@@ -378,7 +378,7 @@ func TestNewPiServoMaxFreqError(t *testing.T) {
 		mockFrequencyGetter.EXPECT().MaxFreqAdjPPB().Return(12345.0, fmt.Errorf("error")),
 	)
 
-	servo, err := NewPiServo(time.Duration(1), time.Duration(1), mockFrequencyGetter, 0.0)
+	servo, err := NewPiServo(time.Duration(1), time.Duration(1), time.Duration(0), mockFrequencyGetter, 0.0)
 
 	require.NoError(t, err)
 	require.Equal(t, int64(1), servo.Servo.FirstStepThreshold)
@@ -396,7 +396,7 @@ func TestNewPiServoUseMaxFreq(t *testing.T) {
 		mockFrequencyGetter.EXPECT().FreqPPB().Return(1.0, nil),
 	)
 
-	servo, err := NewPiServo(time.Duration(1), time.Duration(1), mockFrequencyGetter, 2.0)
+	servo, err := NewPiServo(time.Duration(1), time.Duration(1), time.Duration(0), mockFrequencyGetter, 2.0)
 
 	require.NoError(t, err)
 	require.Equal(t, int64(1), servo.Servo.FirstStepThreshold)
@@ -406,6 +406,25 @@ func TestNewPiServoUseMaxFreq(t *testing.T) {
 	require.Equal(t, 2.0, servo.GetMaxFreq())
 }
 
+func TestNewPiServoStepth(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	mockFrequencyGetter := NewMockFrequencyGetter(ctrl)
+	gomock.InOrder(
+		mockFrequencyGetter.EXPECT().FreqPPB().Return(1.0, nil),
+	)
+
+	servo, err := NewPiServo(time.Duration(1), time.Duration(1), time.Duration(10), mockFrequencyGetter, 2.0)
+
+	require.NoError(t, err)
+	require.Equal(t, int64(1), servo.Servo.FirstStepThreshold)
+	require.Equal(t, true, servo.Servo.FirstUpdate)
+	require.Equal(t, -1.0, servo.MeanFreq())
+	require.Equal(t, "INIT", servo.GetState().String())
+	require.Equal(t, 2.0, servo.GetMaxFreq())
+	require.Equal(t, int64(10), servo.Servo.StepThreshold)
+}
+
 func TestNewPiServoNoFirstStep(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
@@ -414,7 +433,7 @@ func TestNewPiServoNoFirstStep(t *testing.T) {
 		mockFrequencyGetter.EXPECT().FreqPPB().Return(1.0, nil),
 	)
 
-	servo, err := NewPiServo(time.Duration(1), time.Duration(0), mockFrequencyGetter, 2.0)
+	servo, err := NewPiServo(time.Duration(1), time.Duration(0), time.Duration(0), mockFrequencyGetter, 2.0)
 
 	require.NoError(t, err)
 	require.Equal(t, false, servo.Servo.FirstUpdate)