Skip to content

Commit

Permalink
[Dnssd] Sort IPs by scores for commissionable node discovery (#23292)
Browse files Browse the repository at this point in the history
  • Loading branch information
vivien-apple authored Oct 21, 2022
1 parent 559c7d5 commit 131b8c9
Show file tree
Hide file tree
Showing 7 changed files with 173 additions and 89 deletions.
94 changes: 6 additions & 88 deletions src/lib/address_resolve/AddressResolve_DefaultImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,96 +24,14 @@ namespace {

static constexpr System::Clock::Timeout kInvalidTimeout{ System::Clock::Timeout::max() };

// IP addess "suitability"
// - Larger value means "more suitable"
// - Enum ordered ascending for easier read. Note however that order of
// checks MUST match in ScoreIpAddress below.
enum class IpScore : unsigned
{
kInvalid = 0, // No address available

// "Other" IPv6 include:
// - invalid addresses (have seen router bugs during interop testing)
// - embedded IPv4 (::/80)
kOtherIpv6 = 1,
kIpv4 = 2, // Not Matter SPEC, so low priority
#ifdef __APPLE__
kUniqueLocal = 3, // ULA. Thread devices use this
kGlobalUnicast = 4, // Maybe routable, not local subnet
kUniqueLocalWithSharedPrefix = 5, // Prefix seems to match a local interface
kGlobalUnicastWithSharedPrefix = 6, // Prefix seems to match a local interface
kLinkLocal = 7, // Valid only on an interface
#else
kLinkLocal = 3, // Valid only on an interface
kUniqueLocal = 4, // ULA. Thread devices use this
kGlobalUnicast = 5, // Maybe routable, not local subnet
kUniqueLocalWithSharedPrefix = 6, // Prefix seems to match a local interface
kGlobalUnicastWithSharedPrefix = 7, // Prefix seems to match a local interface
#endif // __APPLE__
};

constexpr unsigned ScoreValue(IpScore score)
{
return static_cast<unsigned>(score);
}

/**
* Gives a score for an IP address, generally related to "how good" the address
* is and how likely it is for it to be reachable.
*/
IpScore ScoreIpAddress(const Inet::IPAddress & ip, Inet::InterfaceId interfaceId)
{
if (ip.IsIPv6())
{
#ifdef __APPLE__
if (ip.IsIPv6LinkLocal())
{
return IpScore::kLinkLocal;
}
#endif // __APPLE__

if (interfaceId.MatchLocalIPv6Subnet(ip))
{
if (ip.IsIPv6GlobalUnicast())
{
return IpScore::kGlobalUnicastWithSharedPrefix;
}
if (ip.IsIPv6ULA())
{
return IpScore::kUniqueLocalWithSharedPrefix;
}
}
if (ip.IsIPv6GlobalUnicast())
{
return IpScore::kGlobalUnicast;
}

if (ip.IsIPv6ULA())
{
return IpScore::kUniqueLocal;
}

#ifndef __APPLE__
if (ip.IsIPv6LinkLocal())
{
return IpScore::kLinkLocal;
}
#endif // __APPLE__

return IpScore::kOtherIpv6;
}

return IpScore::kIpv4;
}

} // namespace

void NodeLookupHandle::ResetForLookup(System::Clock::Timestamp now, const NodeLookupRequest & request)
{
mRequestStartTime = now;
mRequest = request;
mBestResult = ResolveResult();
mBestAddressScore = ScoreValue(IpScore::kInvalid);
mBestAddressScore = Dnssd::IPAddressSorter::IpScore::kInvalid;
}

void NodeLookupHandle::LookupResult(const ResolveResult & result)
Expand All @@ -123,8 +41,8 @@ void NodeLookupHandle::LookupResult(const ResolveResult & result)
result.address.ToString(addr_string);
#endif

unsigned newScore = ScoreValue(ScoreIpAddress(result.address.GetIPAddress(), result.address.GetInterface()));
if (newScore > mBestAddressScore)
auto newScore = Dnssd::IPAddressSorter::ScoreIpAddress(result.address.GetIPAddress(), result.address.GetInterface());
if (to_underlying(newScore) > to_underlying(mBestAddressScore))
{
mBestResult = result;
mBestAddressScore = newScore;
Expand All @@ -140,11 +58,11 @@ void NodeLookupHandle::LookupResult(const ResolveResult & result)
}

#if CHIP_PROGRESS_LOGGING
ChipLogProgress(Discovery, "%s: new best score: %u", addr_string, mBestAddressScore);
ChipLogProgress(Discovery, "%s: new best score: %u", addr_string, to_underlying(mBestAddressScore));
}
else
{
ChipLogProgress(Discovery, "%s: score has not improved: %u", addr_string, newScore);
ChipLogProgress(Discovery, "%s: score has not improved: %u", addr_string, to_underlying(newScore));
#endif
}
}
Expand Down Expand Up @@ -180,7 +98,7 @@ NodeLookupAction NodeLookupHandle::NextAction(System::Clock::Timestamp now)
}

// Minimal time to search reached. If any IP available, ready to return it.
if (mBestAddressScore > ScoreValue(IpScore::kInvalid))
if (mBestAddressScore != Dnssd::IPAddressSorter::IpScore::kInvalid)
{
return NodeLookupAction::Success(mBestResult);
}
Expand Down
3 changes: 2 additions & 1 deletion src/lib/address_resolve/AddressResolve_DefaultImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#pragma once

#include <lib/address_resolve/AddressResolve.h>
#include <lib/dnssd/IPAddressSorter.h>
#include <lib/dnssd/Resolver.h>
#include <system/TimeSource.h>
#include <transport/raw/PeerAddress.h>
Expand Down Expand Up @@ -123,7 +124,7 @@ class NodeLookupHandle : public NodeLookupHandleBase
System::Clock::Timestamp mRequestStartTime;
NodeLookupRequest mRequest; // active request to process
AddressResolve::ResolveResult mBestResult;
unsigned mBestAddressScore = 0;
Dnssd::IPAddressSorter::IpScore mBestAddressScore;
};

class Resolver : public ::chip::AddressResolve::Resolver, public Dnssd::OperationalResolveDelegate
Expand Down
2 changes: 2 additions & 0 deletions src/lib/dnssd/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ static_library("dnssd") {

sources = [
"Advertiser.h",
"IPAddressSorter.cpp",
"IPAddressSorter.h",
"Resolver.h",
"ServiceNaming.cpp",
"ServiceNaming.h",
Expand Down
3 changes: 3 additions & 0 deletions src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <crypto/RandUtils.h>
#include <lib/core/CHIPConfig.h>
#include <lib/core/CHIPSafeCasts.h>
#include <lib/dnssd/IPAddressSorter.h>
#include <lib/dnssd/ServiceNaming.h>
#include <lib/dnssd/TxtFields.h>
#include <lib/dnssd/platform/Dnssd.h>
Expand Down Expand Up @@ -53,6 +54,8 @@ static void HandleNodeResolve(void * context, DnssdService * result, const Span<

nodeData.resolutionData.interfaceId = result->mInterface;

IPAddressSorter::Sort(addresses, result->mInterface);

size_t addressesFound = 0;
for (auto & ip : addresses)
{
Expand Down
91 changes: 91 additions & 0 deletions src/lib/dnssd/IPAddressSorter.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
*
* Copyright (c) 2022 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include <lib/dnssd/IPAddressSorter.h>
#include <lib/support/SortUtils.h>

namespace chip {
namespace Dnssd {
namespace IPAddressSorter {

void Sort(Inet::IPAddress * addresses, size_t count, Inet::InterfaceId interfaceId)
{
Sorting::BubbleSort(addresses, count, [interfaceId](const Inet::IPAddress & a, const Inet::IPAddress & b) -> bool {
auto scoreA = to_underlying(ScoreIpAddress(a, interfaceId));
auto scoreB = to_underlying(ScoreIpAddress(b, interfaceId));
return scoreA > scoreB;
});
}

void Sort(const Span<Inet::IPAddress> & addresses, Inet::InterfaceId interfaceId)
{
Sorting::BubbleSort(addresses.begin(), addresses.size(),
[interfaceId](const Inet::IPAddress & a, const Inet::IPAddress & b) -> bool {
auto scoreA = to_underlying(ScoreIpAddress(a, interfaceId));
auto scoreB = to_underlying(ScoreIpAddress(b, interfaceId));
return scoreA > scoreB;
});
}

IpScore ScoreIpAddress(const Inet::IPAddress & ip, Inet::InterfaceId interfaceId)
{
if (ip.IsIPv6())
{
#ifdef __APPLE__
if (ip.IsIPv6LinkLocal())
{
return IpScore::kLinkLocal;
}
#endif // __APPLE__

if (interfaceId.MatchLocalIPv6Subnet(ip))
{
if (ip.IsIPv6GlobalUnicast())
{
return IpScore::kGlobalUnicastWithSharedPrefix;
}
if (ip.IsIPv6ULA())
{
return IpScore::kUniqueLocalWithSharedPrefix;
}
}
if (ip.IsIPv6GlobalUnicast())
{
return IpScore::kGlobalUnicast;
}

if (ip.IsIPv6ULA())
{
return IpScore::kUniqueLocal;
}

#ifndef __APPLE__
if (ip.IsIPv6LinkLocal())
{
return IpScore::kLinkLocal;
}
#endif // __APPLE__

return IpScore::kOtherIpv6;
}

return IpScore::kIpv4;
}

} // namespace IPAddressSorter
} // namespace Dnssd
} // namespace chip
66 changes: 66 additions & 0 deletions src/lib/dnssd/IPAddressSorter.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
*
* Copyright (c) 2022 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#pragma once

#include <inet/IPAddress.h>
#include <lib/support/Span.h>

namespace chip {
namespace Dnssd {
namespace IPAddressSorter {

// IP addess "suitability"
// - Larger value means "more suitable"
// - Enum ordered ascending for easier read. Note however that order of
// checks MUST match in ScoreIpAddress below.
enum class IpScore : unsigned
{
kInvalid = 0, // No address available

// "Other" IPv6 include:
// - invalid addresses (have seen router bugs during interop testing)
// - embedded IPv4 (::/80)
kOtherIpv6 = 1,
kIpv4 = 2, // Not Matter SPEC, so low priority
#ifdef __APPLE__
kUniqueLocal = 3, // ULA. Thread devices use this
kGlobalUnicast = 4, // Maybe routable, not local subnet
kUniqueLocalWithSharedPrefix = 5, // Prefix seems to match a local interface
kGlobalUnicastWithSharedPrefix = 6, // Prefix seems to match a local interface
kLinkLocal = 7, // Valid only on an interface
#else
kLinkLocal = 3, // Valid only on an interface
kUniqueLocal = 4, // ULA. Thread devices use this
kGlobalUnicast = 5, // Maybe routable, not local subnet
kUniqueLocalWithSharedPrefix = 6, // Prefix seems to match a local interface
kGlobalUnicastWithSharedPrefix = 7, // Prefix seems to match a local interface
#endif // __APPLE__
};

void Sort(Inet::IPAddress * addresses, size_t count, Inet::InterfaceId interfaceId);

void Sort(const Span<Inet::IPAddress> & addresses, Inet::InterfaceId interfaceId);

/**
* Gives a score for an IP address, generally related to "how good" the address
* is and how likely it is for it to be reachable.
*/
IpScore ScoreIpAddress(const Inet::IPAddress & ip, Inet::InterfaceId interfaceId);

} // namespace IPAddressSorter
} // namespace Dnssd
} // namespace chip
3 changes: 3 additions & 0 deletions src/lib/dnssd/IncrementalResolve.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
#include <lib/dnssd/IncrementalResolve.h>

#include <lib/dnssd/IPAddressSorter.h>
#include <lib/dnssd/ServiceNaming.h>
#include <lib/dnssd/TxtFields.h>
#include <lib/dnssd/minimal_mdns/Logging.h>
Expand Down Expand Up @@ -352,6 +353,8 @@ CHIP_ERROR IncrementalResolver::Take(DiscoveredNodeData & outputData)
{
VerifyOrReturnError(IsActiveCommissionParse(), CHIP_ERROR_INCORRECT_STATE);

IPAddressSorter::Sort(mCommonResolutionData.ipAddress, mCommonResolutionData.numIPs, mCommonResolutionData.interfaceId);

outputData.resolutionData = mCommonResolutionData;
outputData.commissionData = mSpecificResolutionData.Get<CommissionNodeData>();

Expand Down

0 comments on commit 131b8c9

Please sign in to comment.