Wait for wwan to apply latest config before testing connectivity

When cellular network config changes, it may take few seconds for the
wwan microservice and the modem to apply the new config.
NIM microservice should wait for the wwan config to be fully applied
when cellular connectivity is used for management and ethernet is not
available. Otherwise, it may determine the connectivity as not working
and failover to the previous config too fast, not giving the modem the
chance to connect with the latest config.

Also fixed in this commit is the Equal() method for CellularAccessPoint.
It used to compare also timestamp inside ErrorAndTime which may change
even if there is no actual config change. As a result, wwan microservice
would trigger re-connection even when there was no config change to
apply. This fix is in pillar and will be propagated into wwan/mmagent
in the next commit (with "go get") once this one is merged.

Signed-off-by: Milan Lenco <[email protected]>
(cherry picked from commit f208c67)

pkg/pillar/dpcmanager/dpcmanager.go

@@ -371,7 +371,8 @@ func (m *DpcManager) run(ctx context.Context) {
 						switch dpc.State {
 						case types.DPCStateIPDNSWait,
 							types.DPCStatePCIWait,
-							types.DPCStateIntfWait:
+							types.DPCStateIntfWait,
+							types.DPCStateWwanWait:
 							// Note that DPCStatePCIWait and DPCStateIntfWait can be returned
 							// also in scenarios where some ports are in PCIBack while others
 							// are waiting for IP addresses.

pkg/pillar/dpcmanager/dpcmanager_test.go

@@ -1085,35 +1085,29 @@ func TestWireless(test *testing.T) {
 	dpcManager.UpdateAA(aa)
 	dpcManager.AddDPC(dpc)
 
-	// Verification will wait for IP addresses.
+	// Verification will wait for wwan config to be applied.
 	t.Eventually(testingInProgressCb()).Should(BeTrue())
 	t.Eventually(dpcIdxCb()).Should(Equal(0))
 	t.Eventually(dpcKeyCb(0)).Should(Equal("zedagent"))
 	t.Eventually(dpcTimePrioCb(0, timePrio1)).Should(BeTrue())
 	t.Eventually(dnsKeyCb()).Should(Equal("zedagent"))
-	t.Expect(getDPC(0).State).To(Equal(types.DPCStateIPDNSWait))
-
-	// Simulate working wlan connectivity.
-	wlan0 = mockWlan0() // with IP
-	networkMonitor.AddOrUpdateInterface(wlan0)
-	t.Eventually(testingInProgressCb()).Should(BeFalse())
-	t.Eventually(dpcIdxCb()).Should(Equal(0))
-	t.Expect(getDPC(0).State).To(Equal(types.DPCStateSuccess))
+	t.Expect(getDPC(0).State).To(Equal(types.DPCStateWwanWait))
 
 	// Simulate working wwan connectivity.
+	rs := types.RadioSilence{}
+	wwan0Status := mockWwan0Status(dpc, rs)
+	dpcManager.ProcessWwanStatus(wwan0Status)
 	wwan0 = mockWwan0() // with IP
 	networkMonitor.AddOrUpdateInterface(wwan0)
+	t.Eventually(testingInProgressCb()).Should(BeFalse())
+	t.Eventually(dpcIdxCb()).Should(Equal(0))
+	t.Expect(getDPC(0).State).To(Equal(types.DPCStateSuccess))
 	t.Eventually(func() bool {
 		ports := getDNS().Ports
 		return len(ports) == 2 && len(ports[1].AddrInfoList) == 1 &&
 			ports[1].AddrInfoList[0].Addr.String() == "15.123.87.20"
 	}).Should(BeTrue())
 
-	// Simulate an event of receiving WwanStatus from the wwan microservice.
-	rs := types.RadioSilence{}
-	wwan0Status := mockWwan0Status(dpc, rs)
-	dpcManager.ProcessWwanStatus(wwan0Status)
-
 	// Check DNS content, it should include wwan state data.
 	t.Eventually(wwanOpModeCb(types.WwanOpModeConnected)).Should(BeTrue())
 	wwanDNS := wirelessStatusFromDNS(types.WirelessTypeCellular)
@@ -1142,6 +1136,15 @@ func TestWireless(test *testing.T) {
 	t.Expect(wwanDNS.Cellular.PhysAddrs.USB).To(Equal("1:3.3"))
 	t.Expect(wwanDNS.Cellular.PhysAddrs.PCI).To(Equal("0000:f4:00.0"))
 
+	// Simulate working wlan connectivity.
+	wlan0 = mockWlan0() // with IP
+	networkMonitor.AddOrUpdateInterface(wlan0)
+	t.Eventually(func() bool {
+		ports := getDNS().Ports
+		return len(ports) == 2 && len(ports[0].AddrInfoList) == 1 &&
+			ports[0].AddrInfoList[0].Addr.String() == "192.168.77.2"
+	}).Should(BeTrue())
+
 	// Impose radio silence.
 	// But actually there is a config error coming from upper layers,
 	// so there should be no change in the wwan config.

pkg/pillar/dpcmanager/verify.go

@@ -18,6 +18,7 @@ const (
 	waitForIfRetries    = 2
 	waitForAsyncRetries = 2
 	waitForIPDNSRetries = 5
+	waitForWwanRetries  = 3
 )
 
 func (m *DpcManager) restartVerify(ctx context.Context, reason string) {
@@ -110,7 +111,7 @@ func (m *DpcManager) runVerify(ctx context.Context, reason string) {
 			types.DPCStatePCIWait,
 			// verifyDPC has already published the new DNS for domainmgr.
 			// Wait until we hear from domainmgr or until PendTimer triggers.
-			types.DPCStateIPDNSWait, types.DPCStateIntfWait:
+			types.DPCStateIPDNSWait, types.DPCStateIntfWait, types.DPCStateWwanWait:
 			// Either addressChange or PendTimer will result in calling us again.
 			m.pendingDpcTimer = time.NewTimer(m.dpcTestDuration)
 			return
@@ -298,6 +299,22 @@ func (m *DpcManager) verifyDPC(ctx context.Context) (status types.DPCState) {
 		// Continue...
 	}
 
+	// It may take some time to reconnect cellular modem.
+	// Wait for wwan microservice to apply the latest config if wwan interface is used
+	// for management.
+	if m.waitForWwanUpdate() {
+		if elapsed < waitForWwanRetries*m.dpcTestDuration {
+			m.Log.Noticef("DPC verify: cellular connectivity reconciliation "+
+				"is still in progress (waiting for %v)", elapsed)
+			status = types.DPCStateWwanWait
+			dpc.State = status
+			return status
+		}
+		// Continue...
+		m.Log.Warnf("DPC verify: Still do not have up-to-date wwan status (waited for %v)",
+			elapsed)
+	}
+
 	availablePorts, missingPorts := m.checkMgmtPortsPresence()
 	if len(missingPorts) > 0 {
 		// Still waiting for network interface(s) to appear
@@ -582,6 +599,26 @@ func (m *DpcManager) checkMgmtPortsPresence() (available, missing []string) {
 	return available, missing
 }
 
+func (m *DpcManager) waitForWwanUpdate() bool {
+	dpc := m.currentDPC()
+	if dpc == nil {
+		return false
+	}
+	var hasMgmtWwan bool
+	for _, port := range dpc.Ports {
+		if port.IsMgmt && port.WirelessCfg.WType == types.WirelessTypeCellular {
+			hasMgmtWwan = true
+			break
+		}
+	}
+	if !hasMgmtWwan {
+		return false
+	}
+	statusIsUpToDate := dpc.Key == m.wwanStatus.DPCKey &&
+		dpc.TimePriority.Equal(m.wwanStatus.DPCTimestamp)
+	return !statusIsUpToDate
+}
+
 // If error returned from connectivity test was wrapped into PortsNotReady,
 // unwrap it before recording it into DeviceNetworkStatus and DPCL.
 // PortsNotReady error type is only useful between ConnectivityTester and DPC

pkg/pillar/dpcmanager/wwan.go

@@ -82,8 +82,12 @@ func (m *DpcManager) processWwanStatus(ctx context.Context, status types.WwanSta
 				m.publishDPCL()
 			}
 		}
-		m.restartVerify(ctx, "wwan status changed")
 		m.updateDNS()
+		if dpc.State == types.DPCStateWwanWait {
+			m.runVerify(ctx, "wwan status is up-to-date")
+		} else {
+			m.restartVerify(ctx, "wwan status changed")
+		}
 	}
 }
 

pkg/pillar/types/dpc.go

@@ -52,6 +52,9 @@ const (
 	// DPCStateAsyncWait : waiting for some config operations to finalize which are
 	// running asynchronously in the background.
 	DPCStateAsyncWait
+	// DPCStateWwanWait : waiting for the wwan microservice to apply the latest
+	// cellular configuration.
+	DPCStateWwanWait
 )
 
 // String returns the string name
@@ -75,6 +78,8 @@ func (status DPCState) String() string {
 		return "DPC_REMOTE_WAIT"
 	case DPCStateAsyncWait:
 		return "DPC_ASYNC_WAIT"
+	case DPCStateWwanWait:
+		return "DPC_WWAN_WAIT"
 	default:
 		return fmt.Sprintf("Unknown status %d", status)
 	}
@@ -726,9 +731,14 @@ func (ap CellularAccessPoint) Equal(ap2 CellularAccessPoint) bool {
 		ap.APN != ap2.APN {
 		return false
 	}
+	enc1 := ap.EncryptedCredentials
+	enc2 := ap2.EncryptedCredentials
 	if ap.AuthProtocol != ap2.AuthProtocol ||
-		// TODO (how to properly detect changed username/password ?)
-		!reflect.DeepEqual(ap.EncryptedCredentials, ap2.EncryptedCredentials) {
+		enc1.CipherBlockID != enc2.CipherBlockID ||
+		enc1.CipherContextID != enc2.CipherContextID ||
+		!bytes.Equal(enc1.InitialValue, enc2.InitialValue) ||
+		!bytes.Equal(enc1.CipherData, enc2.CipherData) ||
+		!bytes.Equal(enc1.ClearTextHash, enc2.ClearTextHash) {
 		return false
 	}
 	if !generics.EqualLists(ap.PreferredPLMNs, ap2.PreferredPLMNs) ||