From 4b2c2e5566efeb11c380aef04551c107f200422a Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Wed, 17 Mar 2021 10:16:05 -0700 Subject: [PATCH] [ip6] add operator `<` overload for 'Ip6::Prefix' comparison (#6274) This commit adds an overload of operator `<` in `Ip6::Prefix` class. A prefix with shorter length is considered smaller than the one with longer length. If the prefix lengths are equal, then the prefix bytes are compared directly to determine the smaller prefix. This commit then changes `RoutingManager` to use the newly added operator `<` overload. Unit test `test_ip6_address.cpp` is updated to cover the behavior of the new operator. --- src/core/border_router/routing_manager.cpp | 19 +++-------------- src/core/border_router/routing_manager.hpp | 2 -- src/core/net/ip6_address.cpp | 17 +++++++++++++++ src/core/net/ip6_address.hpp | 14 +++++++++++++ tests/unit/test_ip6_address.cpp | 24 ++++++++++++++++++++++ 5 files changed, 58 insertions(+), 18 deletions(-) diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index a31ca0500..c109c98f2 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -361,7 +361,7 @@ uint8_t RoutingManager::EvaluateOmrPrefix(Ip6::Prefix *aNewOmrPrefixes, uint8_t } aNewOmrPrefixes[newOmrPrefixNum] = onMeshPrefixConfig.GetPrefix(); - if (smallestOmrPrefix == nullptr || IsPrefixSmallerThan(onMeshPrefixConfig.GetPrefix(), *smallestOmrPrefix)) + if (smallestOmrPrefix == nullptr || (onMeshPrefixConfig.GetPrefix() < *smallestOmrPrefix)) { smallestOmrPrefix = &aNewOmrPrefixes[newOmrPrefixNum]; } @@ -523,7 +523,7 @@ const Ip6::Prefix *RoutingManager::EvaluateOnLinkPrefix(void) continue; } - if (smallestOnLinkPrefix == nullptr || IsPrefixSmallerThan(prefix.mPrefix, *smallestOnLinkPrefix)) + if (smallestOnLinkPrefix == nullptr || (prefix.mPrefix < *smallestOnLinkPrefix)) { smallestOnLinkPrefix = &prefix.mPrefix; } @@ -547,7 +547,7 @@ const Ip6::Prefix *RoutingManager::EvaluateOnLinkPrefix(void) // the same smallest on-link prefix and the application-specific prefix is not used. else if (mAdvertisedOnLinkPrefix != nullptr) { - if (IsPrefixSmallerThan(*mAdvertisedOnLinkPrefix, *smallestOnLinkPrefix)) + if (*mAdvertisedOnLinkPrefix < *smallestOnLinkPrefix) { newOnLinkPrefix = mAdvertisedOnLinkPrefix; } @@ -788,19 +788,6 @@ void RoutingManager::SendRouterAdvertisement(const Ip6::Prefix *aNewOmrPrefixes, } } -bool RoutingManager::IsPrefixSmallerThan(const Ip6::Prefix &aFirstPrefix, const Ip6::Prefix &aSecondPrefix) -{ - uint8_t matchedLength; - - OT_ASSERT(aFirstPrefix.GetLength() == aSecondPrefix.GetLength()); - - matchedLength = - Ip6::Prefix::MatchLength(aFirstPrefix.GetBytes(), aSecondPrefix.GetBytes(), aFirstPrefix.GetBytesSize()); - - return matchedLength < aFirstPrefix.GetLength() && - aFirstPrefix.GetBytes()[matchedLength / CHAR_BIT] < aSecondPrefix.GetBytes()[matchedLength / CHAR_BIT]; -} - bool RoutingManager::IsValidOmrPrefix(const NetworkData::OnMeshPrefixConfig &aOnMeshPrefixConfig) { return IsValidOmrPrefix(aOnMeshPrefixConfig.GetPrefix()) && aOnMeshPrefixConfig.mSlaac && !aOnMeshPrefixConfig.mDp; diff --git a/src/core/border_router/routing_manager.hpp b/src/core/border_router/routing_manager.hpp index 8d5410fec..c4a9e8171 100644 --- a/src/core/border_router/routing_manager.hpp +++ b/src/core/border_router/routing_manager.hpp @@ -240,8 +240,6 @@ private: otRoutePreference aRoutePreference = OT_ROUTE_PREFERENCE_MED); bool NetworkDataContainsOmrPrefix(const Ip6::Prefix &aPrefix) const; - // Decides the first prefix is numerically smaller than the second one. - static bool IsPrefixSmallerThan(const Ip6::Prefix &aFirstPrefix, const Ip6::Prefix &aSecondPrefix); static bool IsValidOmrPrefix(const NetworkData::OnMeshPrefixConfig &aOnMeshPrefixConfig); static bool IsValidOmrPrefix(const Ip6::Prefix &aOmrPrefix); static bool IsValidOnLinkPrefix(const Ip6::Prefix &aOnLinkPrefix); diff --git a/src/core/net/ip6_address.cpp b/src/core/net/ip6_address.cpp index 2bcf5cf45..808d531ae 100644 --- a/src/core/net/ip6_address.cpp +++ b/src/core/net/ip6_address.cpp @@ -70,6 +70,23 @@ bool Prefix::IsEqual(const uint8_t *aPrefixBytes, uint8_t aPrefixLength) const return (mLength == aPrefixLength) && (MatchLength(GetBytes(), aPrefixBytes, GetBytesSize()) >= mLength); } +bool Prefix::operator<(const Prefix &aOther) const +{ + bool isSmaller; + uint8_t matchedLength; + + VerifyOrExit(GetLength() == aOther.GetLength(), isSmaller = GetLength() < aOther.GetLength()); + + matchedLength = MatchLength(GetBytes(), aOther.GetBytes(), GetBytesSize()); + + VerifyOrExit(matchedLength < GetLength(), isSmaller = false); + + isSmaller = GetBytes()[matchedLength / CHAR_BIT] < aOther.GetBytes()[matchedLength / CHAR_BIT]; + +exit: + return isSmaller; +} + uint8_t Prefix::MatchLength(const uint8_t *aPrefixA, const uint8_t *aPrefixB, uint8_t aMaxSize) { uint8_t matchedLength = 0; diff --git a/src/core/net/ip6_address.hpp b/src/core/net/ip6_address.hpp index 06f80fcc2..01bd58806 100644 --- a/src/core/net/ip6_address.hpp +++ b/src/core/net/ip6_address.hpp @@ -233,6 +233,20 @@ public: */ bool operator!=(const Prefix &aOther) const { return !(*this == aOther); } + /** + * This method overloads operator `<` to compare two prefixes. + * + * A prefix with shorter length is considered smaller than the one with longer length. If the prefix lengths are + * equal, then the prefix bytes are compared directly. + * + * @param[in] aOther The other prefix to compare against. + * + * @retval TRUE If the prefix is smaller than @p aOther. + * @retval FALSE If the prefix is not smaller than @p aOther. + * + */ + bool operator<(const Prefix &aOther) const; + /** * This static method converts a prefix length (in bits) to size (number of bytes). * diff --git a/tests/unit/test_ip6_address.cpp b/tests/unit/test_ip6_address.cpp index 0825bbe6e..0e0b1ec1d 100644 --- a/tests/unit/test_ip6_address.cpp +++ b/tests/unit/test_ip6_address.cpp @@ -252,6 +252,7 @@ void TestIp6Prefix(void) VerifyOrQuit(!address2.MatchesPrefix(prefix), "Address::MatchedPrefix() failed"); VerifyOrQuit(prefix == prefix, "Prefix::operator==() failed"); + VerifyOrQuit(!(prefix < prefix), "Prefix::operator<() failed"); for (uint8_t subPrefixLength = 1; subPrefixLength <= prefixLength; subPrefixLength++) { @@ -266,14 +267,37 @@ void TestIp6Prefix(void) VerifyOrQuit(prefix == subPrefix, "Prefix::operator==() failed"); VerifyOrQuit(prefix.IsEqual(subPrefix.GetBytes(), subPrefix.GetLength()), "Prefix::IsEqual() failed"); + VerifyOrQuit(!(subPrefix < prefix), "Prefix::operator<() failed"); } else { VerifyOrQuit(prefix != subPrefix, "Prefix::operator!= failed"); VerifyOrQuit(!prefix.IsEqual(subPrefix.GetBytes(), subPrefix.GetLength()), "Prefix::IsEqual() failed"); + VerifyOrQuit(subPrefix < prefix, "Prefix::operator<() failed"); } } + + for (uint8_t bitNumber = 0; bitNumber < prefixLength; bitNumber++) + { + ot::Ip6::Prefix prefix2; + uint8_t mask = static_cast(1U << (7 - (bitNumber & 7))); + uint8_t index = (bitNumber / 8); + bool isPrefixSmaller; + + prefix2 = prefix; + VerifyOrQuit(prefix == prefix2, "Prefix::operator==() failed"); + + // Flip the `bitNumber` bit between `prefix` and `prefix2` + + prefix2.mPrefix.mFields.m8[index] ^= mask; + VerifyOrQuit(prefix != prefix2, "Prefix::operator==() failed"); + + isPrefixSmaller = ((prefix.GetBytes()[index] & mask) == 0); + + VerifyOrQuit((prefix < prefix2) == isPrefixSmaller, "Prefix::operator<() failed"); + VerifyOrQuit((prefix2 < prefix) == !isPrefixSmaller, "Prefix::operator<() failed"); + } } } }