diff --git a/src/core/BUILD.gn b/src/core/BUILD.gn index 99c7818ac..d04ec4b9e 100644 --- a/src/core/BUILD.gn +++ b/src/core/BUILD.gn @@ -422,6 +422,7 @@ openthread_core_files = [ "common/random_manager.cpp", "common/random_manager.hpp", "common/retain_ptr.hpp", + "common/serial_number.hpp", "common/settings.cpp", "common/settings.hpp", "common/settings_driver.hpp", diff --git a/src/core/Makefile.am b/src/core/Makefile.am index bd37bcf41..e60cc29ed 100644 --- a/src/core/Makefile.am +++ b/src/core/Makefile.am @@ -455,6 +455,7 @@ HEADERS_COMMON = \ common/random.hpp \ common/random_manager.hpp \ common/retain_ptr.hpp \ + common/serial_number.hpp \ common/settings.hpp \ common/settings_driver.hpp \ common/string.hpp \ diff --git a/src/core/common/serial_number.hpp b/src/core/common/serial_number.hpp new file mode 100644 index 000000000..39f116643 --- /dev/null +++ b/src/core/common/serial_number.hpp @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2022, The OpenThread Authors. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. Neither the name of the copyright holder nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +/** + * @file + * This file includes definitions for serial number comparison similar to RFC-1982. + */ + +#ifndef SERIAL_NUMBER_HPP_ +#define SERIAL_NUMBER_HPP_ + +#include "openthread-core-config.h" + +#include + +#include "common/numeric_limits.hpp" +#include "common/type_traits.hpp" + +namespace ot { + +class SerialNumber +{ +public: + /** + * This static method indicates whether or not a first serial number is strictly less than a second serial number. + * + * The comparison takes into account the wrapping of serial number values (similar to RFC-1982). It is semantically + * equivalent to `aFirst < aSecond`. + * + * @tparam UintType The unsigned integer type. + * + * @param[in] aFirst The first serial number. + * @param[in] aSecond The second serial number. + * + * @retval TRUE If @p aFirst is less than @p aSecond. + * @retval FALSE If @p aFirst is not less than @p aSecond. + * + */ + template static bool IsLess(UintType aFirst, UintType aSecond) + { + static_assert(TypeTraits::IsSame::kValue || TypeTraits::IsSame::kValue || + TypeTraits::IsSame::kValue || + TypeTraits::IsSame::kValue, + "UintType MUST be an 8, 16, 32, or 64 bit `uint` type"); + + static constexpr UintType kNegativeMask = (NumericLimits::kMax >> 1) + 1; + + return ((aFirst - aSecond) & kNegativeMask) != 0; + } + + /** + * This static method indicates whether or not a first serial number is strictly greater than a second serial + * number. + * + * The comparison takes into account the wrapping of serial number values (similar to RFC-1982). It is semantically + * equivalent to `aFirst > aSecond`. + * + * @tparam UintType The unsigned integer type. + * + * @param[in] aFirst The first serial number. + * @param[in] aSecond The second serial number. + * + * @retval TRUE If @p aFirst is greater than @p aSecond. + * @retval FALSE If @p aFirst is not greater than @p aSecond. + * + */ + template static bool IsGreater(UintType aFirst, UintType aSecond) + { + return IsLess(aSecond, aFirst); + } +}; + +} // namespace ot + +#endif // SERIAL_NUMBER_HPP_ diff --git a/src/core/common/time.hpp b/src/core/common/time.hpp index bff16f78f..6c07fa3c8 100644 --- a/src/core/common/time.hpp +++ b/src/core/common/time.hpp @@ -40,6 +40,7 @@ #include #include "common/equatable.hpp" +#include "common/serial_number.hpp" namespace ot { @@ -178,7 +179,7 @@ public: * @retval FALSE This `Time` instance is not strictly before @p aOther. * */ - bool operator<(const Time &aOther) const { return ((mValue - aOther.mValue) & (1UL << 31)) != 0; } + bool operator<(const Time &aOther) const { return SerialNumber::IsLess(mValue, aOther.mValue); } /** * This method indicates whether this `Time` instance is after or equal to another one. diff --git a/src/core/net/ip6_mpl.cpp b/src/core/net/ip6_mpl.cpp index 7ed4a4c03..a41932a92 100644 --- a/src/core/net/ip6_mpl.cpp +++ b/src/core/net/ip6_mpl.cpp @@ -38,6 +38,7 @@ #include "common/locator_getters.hpp" #include "common/message.hpp" #include "common/random.hpp" +#include "common/serial_number.hpp" #include "net/ip6.hpp" namespace ot { @@ -187,14 +188,12 @@ Error Mpl::UpdateSeedSet(uint16_t aSeedId, uint8_t aSequence) { // have existing entries for aSeedId - int8_t diff = static_cast(aSequence - mSeedSet[i].mSequence); - - if (diff == 0) + if (aSequence == mSeedSet[i].mSequence) { // already received, drop message ExitNow(error = kErrorDrop); } - else if (insert == nullptr && diff < 0) + else if (insert == nullptr && SerialNumber::IsLess(aSequence, mSeedSet[i].mSequence)) { // insert in order of sequence insert = &mSeedSet[i]; diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index d95b8eada..421e2966d 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -44,6 +44,7 @@ #include "common/locator_getters.hpp" #include "common/logging.hpp" #include "common/random.hpp" +#include "common/serial_number.hpp" #include "common/settings.hpp" #include "crypto/aes_ccm.hpp" #include "meshcop/meshcop.hpp" @@ -3140,10 +3141,8 @@ exit: bool Mle::IsNetworkDataNewer(const LeaderData &aLeaderData) { - int8_t diff = static_cast(aLeaderData.GetDataVersion(GetNetworkDataType()) - - Get().GetVersion(GetNetworkDataType())); - - return (diff > 0); + return SerialNumber::IsGreater(aLeaderData.GetDataVersion(GetNetworkDataType()), + Get().GetVersion(GetNetworkDataType())); } Error Mle::HandleLeaderData(const Message &aMessage, const Ip6::MessageInfo &aMessageInfo) @@ -3495,27 +3494,28 @@ void Mle::HandleParentResponse(const Message &aMessage, const Ip6::MessageInfo & #if OPENTHREAD_FTD if (IsFullThreadDevice() && !IsDetached()) { - int8_t diff = static_cast(connectivity.GetIdSequence() - Get().GetRouterIdSequence()); + bool isPartitionIdSame = (leaderData.GetPartitionId() == mLeaderData.GetPartitionId()); + bool isIdSequenceSame = (connectivity.GetIdSequence() == Get().GetRouterIdSequence()); + bool isIdSequenceGreater = + SerialNumber::IsGreater(connectivity.GetIdSequence(), Get().GetRouterIdSequence()); switch (mParentRequestMode) { case kAttachAny: - VerifyOrExit(leaderData.GetPartitionId() != mLeaderData.GetPartitionId() || diff > 0); + VerifyOrExit(!isPartitionIdSame || isIdSequenceGreater); break; case kAttachSame1: case kAttachSame2: - VerifyOrExit(leaderData.GetPartitionId() == mLeaderData.GetPartitionId()); - VerifyOrExit(diff > 0); + VerifyOrExit(isPartitionIdSame && isIdSequenceGreater); break; case kAttachSameDowngrade: - VerifyOrExit(leaderData.GetPartitionId() == mLeaderData.GetPartitionId()); - VerifyOrExit(diff >= 0); + VerifyOrExit(isPartitionIdSame && (isIdSequenceSame || isIdSequenceGreater)); break; case kAttachBetter: - VerifyOrExit(leaderData.GetPartitionId() != mLeaderData.GetPartitionId()); + VerifyOrExit(!isPartitionIdSame); VerifyOrExit(MleRouter::ComparePartitions(connectivity.GetActiveRouters() <= 1, leaderData, Get().IsSingleton(), mLeaderData) > 0); diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index 9365b9af0..5c76f3a0c 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -42,6 +42,7 @@ #include "common/locator_getters.hpp" #include "common/logging.hpp" #include "common/random.hpp" +#include "common/serial_number.hpp" #include "common/settings.hpp" #include "mac/mac_types.hpp" #include "meshcop/meshcop.hpp" @@ -965,8 +966,8 @@ Error MleRouter::HandleLinkAccept(const Message & aMessage, VerifyOrExit(leaderData.GetPartitionId() == mLeaderData.GetPartitionId()); if (mRetrieveNewNetworkData || - (static_cast(leaderData.GetDataVersion(NetworkData::kFullSet) - - Get().GetVersion(NetworkData::kFullSet)) > 0)) + SerialNumber::IsGreater(leaderData.GetDataVersion(NetworkData::kFullSet), + Get().GetVersion(NetworkData::kFullSet))) { IgnoreError(SendDataRequest(aMessageInfo.GetPeerAddr(), dataRequestTlvs, sizeof(dataRequestTlvs), 0)); } @@ -1218,7 +1219,7 @@ Error MleRouter::HandleAdvertisement(const Message &aMessage, const Ip6::Message if (route.IsValid() && IsFullThreadDevice() && (mPreviousPartitionIdTimeout > 0) && (partitionId == mPreviousPartitionId)) { - VerifyOrExit((static_cast(route.GetRouterIdSequence() - mPreviousPartitionRouterIdSequence) > 0), + VerifyOrExit(SerialNumber::IsGreater(route.GetRouterIdSequence(), mPreviousPartitionRouterIdSequence), error = kErrorDrop); } @@ -1262,7 +1263,7 @@ Error MleRouter::HandleAdvertisement(const Message &aMessage, const Ip6::Message if (IsFullThreadDevice() && (aNeighbor && aNeighbor->IsStateValid()) && ((mRouterTable.GetActiveRouterCount() == 0) || - (static_cast(route.GetRouterIdSequence() - mRouterTable.GetRouterIdSequence()) > 0))) + SerialNumber::IsGreater(route.GetRouterIdSequence(), mRouterTable.GetRouterIdSequence()))) { bool processRouteTlv = false; diff --git a/src/core/thread/network_data_service.hpp b/src/core/thread/network_data_service.hpp index 954437ee9..8c0c0606e 100644 --- a/src/core/thread/network_data_service.hpp +++ b/src/core/thread/network_data_service.hpp @@ -42,6 +42,7 @@ #include "common/encoding.hpp" #include "common/locator.hpp" #include "common/non_copyable.hpp" +#include "common/serial_number.hpp" #include "net/socket.hpp" #include "thread/network_data_tlvs.hpp" @@ -189,7 +190,7 @@ public: */ bool IsSequenceNumberAheadOf(const Info &aOther) const { - return (((aOther.mSequenceNumber - mSequenceNumber) & (1U << 7)) != 0); + return SerialNumber::IsGreater(mSequenceNumber, aOther.mSequenceNumber); } Ip6::Address mAnycastAddress; ///< The anycast address associated with the DNS/SRP servers. diff --git a/src/core/thread/topology.hpp b/src/core/thread/topology.hpp index d4858e082..218a24c39 100644 --- a/src/core/thread/topology.hpp +++ b/src/core/thread/topology.hpp @@ -45,6 +45,7 @@ #include "common/locator.hpp" #include "common/message.hpp" #include "common/random.hpp" +#include "common/serial_number.hpp" #include "common/timer.hpp" #include "mac/mac_types.hpp" #include "net/ip6.hpp" @@ -546,7 +547,7 @@ public: * before @p aTag. * */ - bool IsLastRxFragmentTagAfter(uint16_t aTag) const { return ((aTag - mLastRxFragmentTag) & (1U << 15)) != 0; } + bool IsLastRxFragmentTagAfter(uint16_t aTag) const { return SerialNumber::IsGreater(mLastRxFragmentTag, aTag); } #endif // OPENTHREAD_CONFIG_MULTI_RADIO diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 1b2827fac..8db8fbf19 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -769,6 +769,27 @@ target_link_libraries(ot-test-meshcop add_test(NAME ot-test-meshcop COMMAND ot-test-meshcop) +add_executable(ot-test-serial-number + test_serial_number.cpp +) + +target_include_directories(ot-test-serial-number + PRIVATE + ${COMMON_INCLUDES} +) + +target_compile_options(ot-test-serial-number + PRIVATE + ${COMMON_COMPILE_OPTIONS} +) + +target_link_libraries(ot-test-serial-number + PRIVATE + ${COMMON_LIBS} +) + +add_test(NAME ot-test-serial-number COMMAND ot-test-serial-number) + add_executable(ot-test-string test_string.cpp ) diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am index 14da7fff3..863548095 100644 --- a/tests/unit/Makefile.am +++ b/tests/unit/Makefile.am @@ -140,6 +140,7 @@ check_PROGRAMS += \ ot-test-pool \ ot-test-priority-queue \ ot-test-pskc \ + ot-test-serial-number \ ot-test-smart-ptrs \ ot-test-string \ ot-test-timer \ @@ -288,6 +289,9 @@ ot_test_smart_ptrs_SOURCES = $(COMMON_SOURCES) test_smart_ptrs.cpp ot_test_meshcop_LDADD = $(COMMON_LDADD) ot_test_meshcop_SOURCES = $(COMMON_SOURCES) test_meshcop.cpp +ot_test_serial_number_LDADD = $(COMMON_LDADD) +ot_test_serial_number_SOURCES = $(COMMON_SOURCES) test_serial_number.cpp + ot_test_string_LDADD = $(COMMON_LDADD) ot_test_string_SOURCES = $(COMMON_SOURCES) test_string.cpp diff --git a/tests/unit/test_serial_number.cpp b/tests/unit/test_serial_number.cpp new file mode 100644 index 000000000..dc8d7a1eb --- /dev/null +++ b/tests/unit/test_serial_number.cpp @@ -0,0 +1,78 @@ +/* + * Copyright (c) 2022, The OpenThread Authors. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. Neither the name of the copyright holder nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +#include "test_platform.h" + +#include + +#include "test_util.h" +#include "common/code_utils.hpp" +#include "common/numeric_limits.hpp" +#include "common/serial_number.hpp" + +namespace ot { + +template void TestSerialNumber(const char *aName) +{ + static constexpr UintType kMax = NumericLimits::kMax; + static constexpr UintType kMid = kMax / 2; + + static const UintType kNumbers[] = {0, 1, 20, kMid - 1, kMid, kMid + 1, kMax - 20, kMax - 1, kMax}; + + for (UintType number : kNumbers) + { + VerifyOrQuit(!SerialNumber::IsGreater(number, number)); + VerifyOrQuit(!SerialNumber::IsLess(number, number)); + + VerifyOrQuit(SerialNumber::IsGreater(number + 1, number)); + VerifyOrQuit(SerialNumber::IsGreater(number + kMid - 1, number)); + VerifyOrQuit(SerialNumber::IsGreater(number + kMid, number)); + VerifyOrQuit(!SerialNumber::IsGreater(number + kMid + 2, number)); + VerifyOrQuit(!SerialNumber::IsGreater(number + kMax - 1, number)); + + VerifyOrQuit(SerialNumber::IsLess(number - 1, number)); + VerifyOrQuit(SerialNumber::IsLess(number - kMid + 1, number)); + VerifyOrQuit(SerialNumber::IsLess(number - kMid, number)); + VerifyOrQuit(!SerialNumber::IsLess(number - kMid - 2, number)); + VerifyOrQuit(!SerialNumber::IsLess(number - kMax + 1, number)); + } + + printf("TestSerialNumber<%s>() passed\n", aName); +} + +} // namespace ot + +int main(void) +{ + ot::TestSerialNumber("uint8_t"); + ot::TestSerialNumber("uint16_t"); + ot::TestSerialNumber("uint32_t"); + ot::TestSerialNumber("uint64_t"); + printf("\nAll tests passed.\n"); + return 0; +}