From b1309c08018112f184a30e0991f42bb3a84e8028 Mon Sep 17 00:00:00 2001 From: Milan Lenco Date: Mon, 1 Jul 2024 19:06:10 +0200 Subject: [PATCH] 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 (cherry picked from commit f208c6763fad4609cdbdc5c76ff846b3cec1da4a) --- pkg/pillar/dpcmanager/dpcmanager.go | 3 +- pkg/pillar/dpcmanager/dpcmanager_test.go | 31 ++++++++++--------- pkg/pillar/dpcmanager/verify.go | 39 +++++++++++++++++++++++- pkg/pillar/dpcmanager/wwan.go | 6 +++- pkg/pillar/types/dpc.go | 14 +++++++-- 5 files changed, 74 insertions(+), 19 deletions(-) diff --git a/pkg/pillar/dpcmanager/dpcmanager.go b/pkg/pillar/dpcmanager/dpcmanager.go index b9718e4176..6bab06a948 100644 --- a/pkg/pillar/dpcmanager/dpcmanager.go +++ b/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. diff --git a/pkg/pillar/dpcmanager/dpcmanager_test.go b/pkg/pillar/dpcmanager/dpcmanager_test.go index 741e0f7dbf..b462fe8726 100644 --- a/pkg/pillar/dpcmanager/dpcmanager_test.go +++ b/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. diff --git a/pkg/pillar/dpcmanager/verify.go b/pkg/pillar/dpcmanager/verify.go index 7a5fdae9ed..c2ef9fbdef 100644 --- a/pkg/pillar/dpcmanager/verify.go +++ b/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 diff --git a/pkg/pillar/dpcmanager/wwan.go b/pkg/pillar/dpcmanager/wwan.go index 9be4317a91..b9e1360bb7 100644 --- a/pkg/pillar/dpcmanager/wwan.go +++ b/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") + } } } diff --git a/pkg/pillar/types/dpc.go b/pkg/pillar/types/dpc.go index e034ab74f5..bad568e090 100644 --- a/pkg/pillar/types/dpc.go +++ b/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) ||