From 4202297a70f1c822a9827c0c3bb6ee34ee9a8fa9 Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Wed, 3 May 2023 20:21:03 -0400
Subject: [PATCH] Try next IP address when sending first session establishment
 message fails. (#26308)

If DNS-SD discovery comes back with an IP that is not routable for some reason,
or if there os some other synchronous error when trying to kick off session
establishment, move on to the next IP (just like we would for an async session
establishment error), instead of just aborting the session establishment process
altogether.

The change in SetUpCodePairer::ConnectToDiscoveredDevice just moves on to the
next element in our list, if any.

The change in OperationalSessionSetup::UpdateDeviceData adds the TryNextResult
call, but also restructures things to have early returns instead of deep nesting
of ifs.

Fixes https://github.com/project-chip/connectedhomeip/issues/26307
---
 src/app/OperationalSessionSetup.cpp | 52 ++++++++++++++++++-----------
 src/controller/SetUpCodePairer.cpp  |  4 +--
 2 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/src/app/OperationalSessionSetup.cpp b/src/app/OperationalSessionSetup.cpp
index 92c53d9b5329e2..cded2b4f404fb5 100644
--- a/src/app/OperationalSessionSetup.cpp
+++ b/src/app/OperationalSessionSetup.cpp
@@ -193,7 +193,6 @@ void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & ad
                   mPeerId.GetFabricIndex(), ChipLogValueX64(mPeerId.GetNodeId()), peerAddrBuff, static_cast<int>(mState));
 #endif
 
-    CHIP_ERROR err = CHIP_NO_ERROR;
     mDeviceAddress = addr;
 
     // Initialize CASE session state with any MRP parameters that DNS-SD has provided.
@@ -203,31 +202,46 @@ void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & ad
         mCASEClient->SetRemoteMRPIntervals(config);
     }
 
-    if (mState == State::ResolvingAddress)
+    if (mState != State::ResolvingAddress)
     {
-        MoveToState(State::HasAddress);
-        mInitParams.sessionManager->UpdateAllSessionsPeerAddress(mPeerId, addr);
-        if (!mPerformingAddressUpdate)
-        {
-            err = EstablishConnection(config);
-            if (err != CHIP_NO_ERROR)
-            {
-                DequeueConnectionCallbacks(err);
-                // Do not touch `this` instance anymore; it has been destroyed in DequeueConnectionCallbacks.
-                return;
-            }
-            // We expect to get a callback via OnSessionEstablished or OnSessionEstablishmentError to continue
-            // the state machine forward.
-            return;
-        }
+        ChipLogError(Discovery, "Received UpdateDeviceData in incorrect state");
+        DequeueConnectionCallbacks(CHIP_ERROR_INCORRECT_STATE);
+        // Do not touch `this` instance anymore; it has been destroyed in
+        // DequeueConnectionCallbacks.
+        return;
+    }
 
+    MoveToState(State::HasAddress);
+    mInitParams.sessionManager->UpdateAllSessionsPeerAddress(mPeerId, addr);
+
+    if (mPerformingAddressUpdate)
+    {
+        // Nothing else to do here.
         DequeueConnectionCallbacks(CHIP_NO_ERROR);
         // Do not touch `this` instance anymore; it has been destroyed in DequeueConnectionCallbacks.
         return;
     }
 
-    ChipLogError(Discovery, "Received UpdateDeviceData in incorrect state");
-    DequeueConnectionCallbacks(CHIP_ERROR_INCORRECT_STATE);
+    CHIP_ERROR err = EstablishConnection(config);
+    LogErrorOnFailure(err);
+    if (err == CHIP_NO_ERROR)
+    {
+        // We expect to get a callback via OnSessionEstablished or OnSessionEstablishmentError to continue
+        // the state machine forward.
+        return;
+    }
+
+    // Move to the ResolvingAddress state, in case we have more results,
+    // since we expect to receive results in that state.
+    MoveToState(State::ResolvingAddress);
+    if (CHIP_NO_ERROR == Resolver::Instance().TryNextResult(mAddressLookupHandle))
+    {
+        // No need to NotifyRetryHandlers, since we never actually
+        // spent any time trying the previous result.
+        return;
+    }
+
+    DequeueConnectionCallbacks(err);
     // Do not touch `this` instance anymore; it has been destroyed in DequeueConnectionCallbacks.
 }
 
diff --git a/src/controller/SetUpCodePairer.cpp b/src/controller/SetUpCodePairer.cpp
index a257fa7d92bf8d..ee5a31365ad82c 100644
--- a/src/controller/SetUpCodePairer.cpp
+++ b/src/controller/SetUpCodePairer.cpp
@@ -217,7 +217,7 @@ bool SetUpCodePairer::ConnectToDiscoveredDevice()
         return false;
     }
 
-    if (!mDiscoveredParameters.empty())
+    while (!mDiscoveredParameters.empty())
     {
         // Grab the first element from the queue and try connecting to it.
         // Remove it from the queue before we try to connect, in case the
@@ -258,7 +258,7 @@ bool SetUpCodePairer::ConnectToDiscoveredDevice()
             return true;
         }
 
-        // Failed to start establishing PASE.
+        // Failed to start establishing PASE.  Move on to the next item.
         PASEEstablishmentComplete();
     }