Skip to content

Commit

Permalink
Try to set up CASE session over all discovered ips instead of only th…
Browse files Browse the repository at this point in the history
…e 'best' one that has been discovered
  • Loading branch information
vivien-apple committed Sep 21, 2022
1 parent f923113 commit 65c2141
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 12 deletions.
41 changes: 39 additions & 2 deletions src/app/OperationalSessionSetup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,21 @@ void OperationalSessionSetup::OnSessionEstablishmentError(CHIP_ERROR error)
VerifyOrReturn(mState != State::Uninitialized && mState != State::NeedsAddress,
ChipLogError(Controller, "HandleCASEConnectionFailure was called while the device was not initialized"));

DequeueConnectionCallbacks(error);
// Do not touch `this` instance anymore; it has been destroyed in DequeueConnectionCallbacks.
if (mDiscoveredResults.Allocated())
{
mDiscoveredResults.ForEachActiveObject([&](auto * entry) {
MoveToState(State::ResolvingAddress);

UpdateDeviceData(entry->address, entry->mrpRemoteConfig);
mDiscoveredResults.ReleaseObject(entry);
return Loop::Break;
});
}
else
{
DequeueConnectionCallbacks(error);
// Do not touch `this` instance anymore; it has been destroyed in DequeueConnectionCallbacks.
}
}

void OperationalSessionSetup::OnSessionEstablished(const SessionHandle & session)
Expand All @@ -320,6 +333,8 @@ void OperationalSessionSetup::OnSessionEstablished(const SessionHandle & session
if (!mSecureSession.Grab(session))
return; // Got an invalid session, do not change any state

mDiscoveredResults.ReleaseAll();

MoveToState(State::SecureConnected);

DequeueConnectionCallbacks(CHIP_NO_ERROR);
Expand Down Expand Up @@ -420,6 +435,28 @@ void OperationalSessionSetup::PerformAddressUpdate()

void OperationalSessionSetup::OnNodeAddressResolved(const PeerId & peerId, const ResolveResult & result)
{
// Loop over all the addresses that has been found and add them to the pool of potential address to try
// to connect to. The "best" result is not stored into the pool as it will be always tried first.
for (size_t i = 0; i < result.numIPs; i++)
{
auto & currentAddress = result.ipAddress[i];
if (currentAddress == result.address.GetIPAddress())
{
continue;
}

ResolveResult * resolveResult = mDiscoveredResults.CreateObject();
if (resolveResult == nullptr)
{
// It may happens if there are more results than the available number of slots in the pool. This
// is not fatal but it may indicate that the pool size must be increase.
ChipLogError(Discovery, "One discovered IP will not be tried during Operational Session Setup");
break;
}
resolveResult->address.SetIPAddress(result.ipAddress[i]);
resolveResult->mrpRemoteConfig = result.mrpRemoteConfig;
}

UpdateDeviceData(result.address, result.mrpRemoteConfig);
}

Expand Down
5 changes: 5 additions & 0 deletions src/app/OperationalSessionSetup.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include <app/util/basic-types.h>
#include <credentials/GroupDataProvider.h>
#include <lib/address_resolve/AddressResolve.h>
#include <lib/support/Pool.h>
#include <messaging/ExchangeContext.h>
#include <messaging/ExchangeDelegate.h>
#include <messaging/ExchangeMgr.h>
Expand Down Expand Up @@ -326,6 +327,10 @@ class DLL_EXPORT OperationalSessionSetup : public SessionDelegate,
* This function will set new IP address, port and MRP retransmission intervals of the device.
*/
void UpdateDeviceData(const Transport::PeerAddress & addr, const ReliableMessageProtocolConfig & config);

// This number is taken from AddressResolve.h. Please keep it in sync.
static constexpr unsigned kMaxIPAddresses = 5;
ObjectPool<AddressResolve::ResolveResult, kMaxIPAddresses> mDiscoveredResults;
};

} // namespace chip
19 changes: 19 additions & 0 deletions src/lib/address_resolve/AddressResolve.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,30 @@ namespace AddressResolve {
/// structure since not all DNSSD data is useful during operational processing.
struct ResolveResult
{
// TODO: is this count OK? Sufficient space for IPv6 LL, GUA, ULA (and maybe IPv4 if enabled)
static constexpr unsigned kMaxIPAddresses = 5;

Transport::PeerAddress address;
ReliableMessageProtocolConfig mrpRemoteConfig;
bool supportsTcp = false;

size_t numIPs = 0; // number of valid IP addresses
Inet::IPAddress ipAddress[kMaxIPAddresses];

ResolveResult() : address(Transport::Type::kUdp), mrpRemoteConfig(GetDefaultMRPConfig()) {}

#if CHIP_DETAIL_LOGGING
void LogDetail() const
{
ChipLogDetail(Discovery, "ResolveResult IP address:");
for (unsigned j = 0; j < numIPs; j++)
{
char buf[Inet::IPAddress::kMaxStringLength];
char * ipAddressOut = ipAddress[j].ToString(buf);
ChipLogDetail(Discovery, "\tIP Address #%d: %s", j + 1, ipAddressOut);
}
}
#endif // CHIP_DETAIL_LOGGING
};

/// Represents an object interested in callbacks for a resolve operation.
Expand Down
40 changes: 30 additions & 10 deletions src/lib/address_resolve/AddressResolve_DefaultImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/

#include <lib/address_resolve/AddressResolve_DefaultImpl.h>
#include <lib/support/SortUtils.h>

namespace chip {
namespace AddressResolve {
Expand Down Expand Up @@ -245,6 +246,32 @@ void Resolver::Shutdown()

void Resolver::OnOperationalNodeResolved(const Dnssd::ResolvedNodeData & nodeData)
{
ResolveResult result;

auto resolutionData = nodeData.resolutionData;
result.address.SetPort(resolutionData.port);
result.address.SetInterface(resolutionData.interfaceId);
result.mrpRemoteConfig = resolutionData.GetRemoteMRPConfig();
result.supportsTcp = resolutionData.supportsTcp;

// Copy all the IPs that has been found and sort them by score.
for (size_t i = 0; i < resolutionData.numIPs; i++)
{
#if !INET_CONFIG_ENABLE_IPV4
if (!resolutionData.ipAddress[i].IsIPv6())
{
continue;
}
#endif
result.ipAddress[result.numIPs++] = resolutionData.ipAddress[i];
}

Sorting::BubbleSort(result.ipAddress, result.numIPs, [&](const Inet::IPAddress & a, const Inet::IPAddress & b) {
unsigned scoreA = ScoreValue(ScoreIpAddress(a, resolutionData.interfaceId));
unsigned scoreB = ScoreValue(ScoreIpAddress(b, resolutionData.interfaceId));
return scoreA > scoreB;
});

auto it = mActiveLookups.begin();
while (it != mActiveLookups.end())
{
Expand All @@ -255,23 +282,16 @@ void Resolver::OnOperationalNodeResolved(const Dnssd::ResolvedNodeData & nodeDat
continue;
}

ResolveResult result;

result.address.SetPort(nodeData.resolutionData.port);
result.address.SetInterface(nodeData.resolutionData.interfaceId);
result.mrpRemoteConfig = nodeData.resolutionData.GetRemoteMRPConfig();
result.supportsTcp = nodeData.resolutionData.supportsTcp;

for (size_t i = 0; i < nodeData.resolutionData.numIPs; i++)
for (size_t i = 0; i < resolutionData.numIPs; i++)
{
#if !INET_CONFIG_ENABLE_IPV4
if (!nodeData.resolutionData.ipAddress[i].IsIPv6())
if (!resolutionData.ipAddress[i].IsIPv6())
{
ChipLogError(Discovery, "Skipping IPv4 address during operational resolve.");
continue;
}
#endif
result.address.SetIPAddress(nodeData.resolutionData.ipAddress[i]);
result.address.SetIPAddress(resolutionData.ipAddress[i]);
current->LookupResult(result);
}

Expand Down

0 comments on commit 65c2141

Please sign in to comment.