Skip to content

Commit

Permalink
Try next IP address when sending first session establishment message …
Browse files Browse the repository at this point in the history
…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 #26307
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Jul 20, 2023
1 parent ec80536 commit 4202297
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 21 deletions.
52 changes: 33 additions & 19 deletions src/app/OperationalSessionSetup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
}

Expand Down
4 changes: 2 additions & 2 deletions src/controller/SetUpCodePairer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
}

Expand Down

0 comments on commit 4202297

Please sign in to comment.