OCPBUGS-111603: force freerun when non-leading DPLL loses lock

pkg/dpll/dpll.go

@@ -763,45 +763,29 @@ func (d *DpllConfig) stateDecision() {
 
 	case DPLL_HOLDOVER:
 		switch {
-		case d.hasPPSAsSource():
+		case !d.hasLeadingSource():
 			d.state = event.PTP_FREERUN
 			d.phaseOffset = FaultyPhaseOffset
-			glog.Infof("Follower card DPLL is on holdover - hardware failure or pins misconfigured")
-		case d.hasGNSSAsSource():
-			if !d.inSpec { // d.inSpec is updated by the holdover routine
-				glog.Infof("dpll is not in spec, state is DPLL_HOLDOVER, offset is out of range, state is FREERUN(%s)", d.iface)
-				d.state = event.PTP_FREERUN
-				d.phaseOffset = FaultyPhaseOffset
-				select {
-				case d.holdoverCloseCh <- true:
-					glog.Infof("closing holdover for %s since offset if out of spec", d.iface)
-				default:
-				}
-			} else if !d.onHoldover {
-				d.holdoverCloseCh = make(chan bool)
-				d.onHoldover = true
-				d.state = event.PTP_HOLDOVER
-				glog.Infof("starting holdover (%s)", d.iface)
-				go d.holdover()
-			}
-		case d.hasPTPAsSource():
-			if d.PhaseOffset() > LocalMaxHoldoverOffSet {
-				glog.Infof("dpll offset is above MaxHoldoverOffSet, state is FREERUN(%s)", d.iface)
-				d.state = event.PTP_FREERUN
-				d.phaseOffset = FaultyPhaseOffset
-				d.sourceLost = true
-				select {
-				case d.holdoverCloseCh <- true:
-					glog.Infof("closing holdover for %s since offset if above MaxHoldoverOffSet", d.iface)
-				default:
-				}
-			} else if !d.onHoldover && !d.closing {
-				d.holdoverCloseCh = make(chan bool)
-				d.onHoldover = true
-				d.state = event.PTP_HOLDOVER
-				glog.Infof("starting holdover (%s)", d.iface)
-				go d.holdover()
+			glog.Infof("non-leading DPLL %s is in holdover, reporting FREERUN", d.iface)
+		// TODO: GNSS holdover currently doesn't offer holdover out of spec. Tech Debt: should work the same as T-BC
+		// making transitions programmable by users
+		case !d.inSpec || (d.hasPTPAsSource() && math.Abs(float64(d.PhaseOffset())) > float64(LocalMaxHoldoverOffSet)):
+			glog.Infof("leading DPLL %s holdover out of spec (inSpec=%v, offset=%d, max=%d), state is FREERUN",
+				d.iface, d.inSpec, d.PhaseOffset(), LocalMaxHoldoverOffSet)
+			d.state = event.PTP_FREERUN
+			d.phaseOffset = FaultyPhaseOffset
+			d.sourceLost = true
+			select {
+			case d.holdoverCloseCh <- true:
+				glog.Infof("closing holdover for %s since holdover is out of spec", d.iface)
+			default:
 			}
+		case !d.onHoldover && !d.closing:
+			d.holdoverCloseCh = make(chan bool)
+			d.onHoldover = true
+			d.state = event.PTP_HOLDOVER
+			glog.Infof("starting holdover (%s)", d.iface)
+			go d.holdover()
 		}
 
 	case DPLL_LOCKED_HO_ACQ, DPLL_LOCKED:

pkg/event/event.go

@@ -382,6 +382,9 @@ func (e *EventHandler) updateGMState(cfgName string) clockSyncState {
 			switch d.ProcessName {
 			case DPLL:
 				dpllState = d.State
+				if e.hasNonLeadingDPLLFault(cfgName, leadingInterface) {
+					dpllState = PTP_FREERUN
+				}
 			case GNSS:
 				gnssState = d.State
 				// expecting to have at least one interface
@@ -536,6 +539,34 @@ func (e *EventHandler) isSourceLost(cfgName string) bool {
 	return false
 }
 
+// hasNonLeadingDPLLFault returns true when the leading DPLL is locked but at
+// least one non-leading DPLL is not locked, indicating a follower fault that
+// should force the composite clock to FREERUN.
+func (e *EventHandler) hasNonLeadingDPLLFault(cfgName, leadingInterface string) bool {
+	if leadingInterface == LEADING_INTERFACE_UNKNOWN {
+		return false
+	}
+	if data, ok := e.data[cfgName]; ok {
+		for _, d := range data {
+			if d.ProcessName != DPLL {
+				continue
+			}
+			leadingDetail := d.GetDataDetails(leadingInterface)
+			if leadingDetail == nil || leadingDetail.State != PTP_LOCKED {
+				return false
+			}
+			for _, dd := range d.Details {
+				if dd.IFace != leadingInterface && dd.State != PTP_LOCKED {
+					glog.Infof("non-leading DPLL %s is %s while leading %s is locked, composite DPLL forced to FREERUN",
+						dd.IFace, dd.State, leadingInterface)
+					return true
+				}
+			}
+		}
+	}
+	return false
+}
+
 func (e *EventHandler) getLeadingInterface(cfgName string) string {
 	if data, ok := e.data[cfgName]; ok {
 		for _, d := range data {

pkg/event/event_tbc.go

@@ -144,7 +144,7 @@ func (e *EventHandler) updateBCState(event EventChannel) (clockSyncState, bool)
 			updateDownstreamData = true
 		}
 	case PTP_LOCKED:
-		if e.freeRunCondition(cfgName) {
+		if e.freeRunCondition(cfgName) || e.hasNonLeadingDPLLFault(cfgName, leadingInterface) {
 			e.clkSyncState[cfgName].state = PTP_FREERUN
 			e.clkSyncState[cfgName].clockClass = protocol.ClockClassFreerun
 			glog.Info("BC FSM: LOCKED to FREERUN")
@@ -172,16 +172,18 @@ func (e *EventHandler) updateBCState(event EventChannel) (clockSyncState, bool)
 			}
 		}
 	case PTP_HOLDOVER:
-		if e.inSyncCondition(cfgName) && !e.isSourceLostBC(cfgName) {
-			e.clkSyncState[cfgName].state = PTP_LOCKED
-			glog.Info("BC FSM: HOLDOVER to LOCKED")
-			updateDownstreamData = true
-		} else if e.freeRunCondition(cfgName) {
+		nonLeadingFault := e.hasNonLeadingDPLLFault(cfgName, leadingInterface)
+		switch {
+		case nonLeadingFault || e.freeRunCondition(cfgName):
 			e.clkSyncState[cfgName].state = PTP_FREERUN
 			e.clkSyncState[cfgName].clockClass = protocol.ClockClassFreerun
 			glog.Info("BC FSM: HOLDOVER to FREERUN")
 			updateDownstreamData = true
-		} else {
+		case e.inSyncCondition(cfgName) && !e.isSourceLostBC(cfgName):
+			e.clkSyncState[cfgName].state = PTP_LOCKED
+			glog.Info("BC FSM: HOLDOVER to LOCKED")
+			updateDownstreamData = true
+		default:
 			if event.IFace == leadingInterface {
 				inSpec := false
 				if e.LeadingClockData.lastInSpec {

pkg/event/stats.go

@@ -49,14 +49,14 @@ func (d *Data) UpdateState() {
 	state := PTP_UNKNOWN
 	for _, detail := range d.Details { // 2 ts2phc or 2 dpll etc
 		switch detail.State {
-		case PTP_FREERUN: // if its free run and main state is not holdover then this is the state
-			if state != PTP_HOLDOVER {
+		case PTP_FREERUN: // FREERUN is the worst state (S0) and always takes priority
+			state = detail.State
+		case PTP_HOLDOVER: // HOLDOVER (S1) takes priority over LOCKED but not FREERUN
+			if state != PTP_FREERUN {
 				state = detail.State
 			}
-		case PTP_HOLDOVER: // if one of them is in holdover then this is the state
-			state = detail.State
-		case PTP_LOCKED: // if this is locked and none of them are in UNKNOWN or FREE run then this is the state
-			if state != PTP_FREERUN && state != PTP_HOLDOVER { // previous state
+		case PTP_LOCKED: // LOCKED (S2) is best; only sets if nothing worse exists
+			if state != PTP_FREERUN && state != PTP_HOLDOVER {
 				state = detail.State
 			}
 		}

pkg/event/stats_test.go

@@ -101,8 +101,8 @@ func Test_updateStats(t *testing.T) {
 					},
 					State: event.PTP_UNKNOWN,
 				}}},
-		wantedState: event.PTP_HOLDOVER,
-		desc:        "3. GNSS is in HOLDOVER, PPS is in FREERUN",
+		wantedState: event.PTP_FREERUN,
+		desc:        "3. GNSS is in HOLDOVER, PPS is in FREERUN - FREERUN takes priority (worst state)",
 	}, {
 		data: map[string][]*event.Data{
 			"a.0.config": {
@@ -139,3 +139,93 @@ func Test_updateStats(t *testing.T) {
 	}
 
 }
+
+func Test_updateState_LeadingFollowerMatrix(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		desc          string
+		leadingState  event.PTPState
+		followerState event.PTPState
+		wantedState   event.PTPState
+	}{
+		{
+			desc:          "both locked",
+			leadingState:  event.PTP_LOCKED,
+			followerState: event.PTP_LOCKED,
+			wantedState:   event.PTP_LOCKED,
+		},
+		{
+			desc:          "follower freerun, leader locked - follower degrades to S0",
+			leadingState:  event.PTP_LOCKED,
+			followerState: event.PTP_FREERUN,
+			wantedState:   event.PTP_FREERUN,
+		},
+		{
+			desc:          "follower freerun, leader holdover - FREERUN wins over HOLDOVER",
+			leadingState:  event.PTP_HOLDOVER,
+			followerState: event.PTP_FREERUN,
+			wantedState:   event.PTP_FREERUN,
+		},
+		{
+			desc:          "leader holdover, follower locked - HOLDOVER propagates",
+			leadingState:  event.PTP_HOLDOVER,
+			followerState: event.PTP_LOCKED,
+			wantedState:   event.PTP_HOLDOVER,
+		},
+		{
+			desc:          "both freerun",
+			leadingState:  event.PTP_FREERUN,
+			followerState: event.PTP_FREERUN,
+			wantedState:   event.PTP_FREERUN,
+		},
+		{
+			desc:          "leader freerun, follower locked - FREERUN wins",
+			leadingState:  event.PTP_FREERUN,
+			followerState: event.PTP_LOCKED,
+			wantedState:   event.PTP_FREERUN,
+		},
+		{
+			desc:          "both holdover",
+			leadingState:  event.PTP_HOLDOVER,
+			followerState: event.PTP_HOLDOVER,
+			wantedState:   event.PTP_HOLDOVER,
+		},
+		{
+			desc:          "leader locked, follower holdover",
+			leadingState:  event.PTP_LOCKED,
+			followerState: event.PTP_HOLDOVER,
+			wantedState:   event.PTP_HOLDOVER,
+		},
+		{
+			desc:          "leader freerun, follower holdover - FREERUN wins",
+			leadingState:  event.PTP_FREERUN,
+			followerState: event.PTP_HOLDOVER,
+			wantedState:   event.PTP_FREERUN,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			t.Parallel()
+			d := &event.Data{
+				ProcessName: "dpll",
+				Details: []*event.DataDetails{
+					{
+						IFace:   "leading-nic",
+						State:   tt.leadingState,
+						Metrics: map[event.ValueType]event.DataMetric{},
+					},
+					{
+						IFace:   "follower-nic",
+						State:   tt.followerState,
+						Metrics: map[event.ValueType]event.DataMetric{},
+					},
+				},
+				State: event.PTP_UNKNOWN,
+			}
+			d.UpdateState()
+			assert.Equal(t, tt.wantedState, d.State, tt.desc)
+		})
+	}
+}