From ee32965f7ef3a13d878b437536dbb70f485baade Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 28 Feb 2023 10:26:56 -0800 Subject: [PATCH] [mle] smaller enhancements (#8812) This commit contains smaller enhancements in `MleRouter` class, defining `kChallengeTimeout` constant, using ternary `? :` replacing `if/else` checks, combining `#if` blocks, etc. --- src/core/thread/mle.cpp | 5 +-- src/core/thread/mle_router.cpp | 70 +++++++++++----------------------- src/core/thread/mle_router.hpp | 8 ++-- src/core/thread/mle_types.hpp | 4 +- 4 files changed, 28 insertions(+), 59 deletions(-) diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index 26ee7cfe2..75b2a0b64 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -4940,10 +4940,7 @@ Error Mle::RxMessage::ReadChallengeOrResponse(uint8_t aTlvType, Challenge &aBuff SuccessOrExit(error = Tlv::FindTlvValueOffset(*this, aTlvType, offset, length)); VerifyOrExit(length >= kMinChallengeSize, error = kErrorParse); - if (length > kMaxChallengeSize) - { - length = kMaxChallengeSize; - } + length = Min(length, kMaxChallengeSize); ReadBytes(offset, aBuffer.mBuffer, length); aBuffer.mLength = static_cast(length); diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index bf4d7fdf0..5d9e5c857 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -184,20 +184,6 @@ exit: return error; } -// If the router was a leader or had more than 5 children prior to reset, -// the multicast link request is retransmitted as a critical message. -void MleRouter::SetLinkRequestTransmissionCounter(void) -{ - uint16_t numOfChildren = mChildTable.GetNumChildren(Child::kInStateValidOrRestoring); - - mLinkRequestAttempts = kMaxTransmissionCount; - - if (mWasLeader || numOfChildren >= kMinCriticalChildrenCount) - { - mLinkRequestAttempts = kMaxCriticalTransmissionCount; - } -} - Error MleRouter::BecomeRouter(ThreadStatusTlv::Status aStatus) { Error error = kErrorNone; @@ -215,7 +201,14 @@ Error MleRouter::BecomeRouter(ThreadStatusTlv::Status aStatus) switch (mRole) { case kRoleDetached: - SetLinkRequestTransmissionCounter(); + // If router had more than `kMinCriticalChildrenCount` children + // or was a leader prior to reset we treat the multicast Link + // Request as a critical message. + mLinkRequestAttempts = + (mWasLeader || mChildTable.GetNumChildren(Child::kInStateValidOrRestoring) >= kMinCriticalChildrenCount) + ? kMaxCriticalTransmissionCount + : kMaxTransmissionCount; + SuccessOrExit(error = SendLinkRequest(nullptr)); mLinkRequestAttempts--; ScheduleMessageTransmissionTimer(); @@ -240,10 +233,6 @@ Error MleRouter::BecomeLeader(void) Router *router; uint32_t partitionId; uint8_t leaderId; -#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE - uint8_t minRouterId; - uint8_t maxRouterId; -#endif VerifyOrExit(!Get().IsPartiallyComplete(), error = kErrorInvalidState); VerifyOrExit(!IsDisabled(), error = kErrorInvalidState); @@ -253,22 +242,18 @@ Error MleRouter::BecomeLeader(void) mRouterTable.Clear(); #if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE - partitionId = mPreferredLeaderPartitionId ? mPreferredLeaderPartitionId : Random::NonCrypto::GetUint32(); + { + uint8_t minId; + uint8_t maxId; + + mRouterTable.GetRouterIdRange(minId, maxId); + partitionId = mPreferredLeaderPartitionId ? mPreferredLeaderPartitionId : Random::NonCrypto::GetUint32(); + leaderId = (IsRouterIdValid(mPreviousRouterId) && minId <= mPreviousRouterId && mPreviousRouterId <= maxId) + ? mPreviousRouterId + : Random::NonCrypto::GetUint8InRange(minId, maxId + 1); + } #else partitionId = Random::NonCrypto::GetUint32(); -#endif - -#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE - mRouterTable.GetRouterIdRange(minRouterId, maxRouterId); - if (IsRouterIdValid(mPreviousRouterId) && minRouterId <= mPreviousRouterId && mPreviousRouterId <= maxRouterId) - { - leaderId = mPreviousRouterId; - } - else - { - leaderId = Random::NonCrypto::GetUint8InRange(minRouterId, maxRouterId + 1); - } -#else leaderId = IsRouterIdValid(mPreviousRouterId) ? mPreviousRouterId : Random::NonCrypto::GetUint8InRange(0, kMaxRouterId + 1); #endif @@ -327,13 +312,11 @@ void MleRouter::HandleChildStart(AttachMode aMode) case kDowngradeToReed: SendAddressRelease(); - // reset children info if any if (HasChildren()) { RemoveChildren(); } - // reset routerId info SetRouterId(kInvalidRouterId); break; @@ -594,7 +577,7 @@ Error MleRouter::SendLinkRequest(Neighbor *aNeighbor) if (aNeighbor == nullptr) { mChallenge.GenerateRandom(); - mChallengeTimeout = (((2 * kMaxResponseDelay) + kStateUpdatePeriod - 1) / kStateUpdatePeriod); + mChallengeTimeout = kChallengeTimeout; SuccessOrExit(error = message->AppendChallengeTlv(mChallenge)); destination.SetToLinkLocalAllRoutersMulticast(); @@ -1771,24 +1754,15 @@ void MleRouter::SendParentResponse(Child *aChild, const Challenge &aChallenge, b #endif aChild->GenerateChallenge(); - SuccessOrExit(error = message->AppendChallengeTlv(aChild->GetChallenge(), aChild->GetChallengeSize())); - error = message->AppendLinkMarginTlv(aChild->GetLinkInfo().GetLinkMargin()); - SuccessOrExit(error); - + SuccessOrExit(error = message->AppendLinkMarginTlv(aChild->GetLinkInfo().GetLinkMargin())); SuccessOrExit(error = message->AppendConnectivityTlv()); SuccessOrExit(error = message->AppendVersionTlv()); destination.SetToLinkLocalAddress(aChild->GetExtAddress()); - if (aRoutersOnlyRequest) - { - delay = 1 + Random::NonCrypto::GetUint16InRange(0, kParentResponseMaxDelayRouters); - } - else - { - delay = 1 + Random::NonCrypto::GetUint16InRange(0, kParentResponseMaxDelayAll); - } + delay = 1 + Random::NonCrypto::GetUint16InRange(0, aRoutersOnlyRequest ? kParentResponseMaxDelayRouters + : kParentResponseMaxDelayAll); SuccessOrExit(error = message->SendAfterDelay(destination, delay)); diff --git a/src/core/thread/mle_router.hpp b/src/core/thread/mle_router.hpp index 1d34380be..251efdb0f 100644 --- a/src/core/thread/mle_router.hpp +++ b/src/core/thread/mle_router.hpp @@ -542,9 +542,9 @@ public: #endif private: - static constexpr uint16_t kDiscoveryMaxJitter = 250; // Max jitter delay Discovery Responses (in msec). - static constexpr uint32_t kStateUpdatePeriod = 1000; // State update period (in msec). - static constexpr uint16_t kUnsolicitedDataResponseJitter = 500; // Max delay for unsol Data Response (in msec). + static constexpr uint16_t kDiscoveryMaxJitter = 250; // Max jitter delay Discovery Responses (in msec). + static constexpr uint16_t kChallengeTimeout = 2; // Challenge timeout (in sec). + static constexpr uint16_t kUnsolicitedDataResponseJitter = 500; // Max delay for unsol Data Response (in msec). // Threshold to accept a router upgrade request with reason // `kBorderRouterRequest` (number of BRs acting as router in @@ -629,8 +629,6 @@ private: void HandleAdvertiseTrickleTimer(void); void HandleTimeTick(void); - void SetLinkRequestTransmissionCounter(void); - TrickleTimer mAdvertiseTrickleTimer; ChildTable mChildTable; diff --git a/src/core/thread/mle_types.hpp b/src/core/thread/mle_types.hpp index 076f83379..33e84bf18 100644 --- a/src/core/thread/mle_types.hpp +++ b/src/core/thread/mle_types.hpp @@ -127,8 +127,8 @@ constexpr uint16_t kMaxChildId = 511; ///< Maximum Child ID constexpr uint8_t kRouterIdOffset = 10; ///< Bit offset of Router ID in RLOC16 constexpr uint8_t kRlocPrefixLength = 14; ///< Prefix length of RLOC in bytes -constexpr uint8_t kMinChallengeSize = 4; ///< Minimum Challenge size in bytes. -constexpr uint8_t kMaxChallengeSize = 8; ///< Maximum Challenge size in bytes. +constexpr uint16_t kMinChallengeSize = 4; ///< Minimum Challenge size in bytes. +constexpr uint16_t kMaxChallengeSize = 8; ///< Maximum Challenge size in bytes. /* * Routing Protocol Constants