From 787d6d3763d6bbd66594b4a339dde49a0cb5dfcc Mon Sep 17 00:00:00 2001 From: Rongli Sun Date: Fri, 3 Apr 2020 08:16:54 +0800 Subject: [PATCH] [core] use the common `RouterIdSet` class (#4756) Currently there are four similar copies regarding Router Id set separately in 1) RouteTlv in mle_tlvs.hpp; 2) ThreadRouterMaskTlv in thread_tlvs.hpp; 3) RouterIdSet in router_table.hpp; 4) RouteTlv in network_diagnostic_tlvs.hpp. This commit extracts the common RouterIdSet class and apply it in the four components, meanwhile keep one copy for RouteTlv and ThreadRouterMaskTlv process in route_table. --- src/core/thread/mle_router.cpp | 22 ++----- src/core/thread/mle_tlvs.hpp | 66 ++++++++++---------- src/core/thread/mle_types.hpp | 67 ++++++++++++++++++++ src/core/thread/network_diagnostic_tlvs.hpp | 20 ++---- src/core/thread/router_table.cpp | 68 ++++----------------- src/core/thread/router_table.hpp | 41 +++++-------- src/core/thread/thread_tlvs.hpp | 32 ++++------ 7 files changed, 152 insertions(+), 164 deletions(-) diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index 0c414fed7..6853f3074 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -1081,7 +1081,7 @@ otError MleRouter::ProcessRouteTlv(const RouteTlv &aRoute) { otError error = OT_ERROR_NONE; - mRouterTable.ProcessTlv(aRoute); + mRouterTable.UpdateRouterIdSet(aRoute.GetRouterIdSequence(), aRoute.GetRouterIdMask()); if (mRole == OT_DEVICE_ROLE_ROUTER && !mRouterTable.IsAllocated(mRouterId)) { @@ -3307,7 +3307,7 @@ void MleRouter::RemoveRouterLink(Router &aRouter) #if OPENTHREAD_FTD case OT_DEVICE_ROLE_ROUTER: case OT_DEVICE_ROLE_LEADER: - mRouterTable.RemoveNeighbor(aRouter); + mRouterTable.RemoveRouterLink(aRouter); break; #endif @@ -3346,7 +3346,7 @@ void MleRouter::RemoveNeighbor(Neighbor &aNeighbor) else if (aNeighbor.IsStateValid()) { Signal(OT_NEIGHBOR_TABLE_EVENT_ROUTER_REMOVED, aNeighbor); - mRouterTable.RemoveNeighbor(static_cast(aNeighbor)); + mRouterTable.RemoveRouterLink(static_cast(aNeighbor)); } aNeighbor.GetLinkInfo().Clear(); @@ -4086,7 +4086,7 @@ void MleRouter::HandleAddressSolicitResponse(Coap::Message * aMessage, SetStateRouter(Rloc16FromRouterId(mRouterId)); mRouterTable.Clear(); - mRouterTable.ProcessTlv(routerMaskTlv); + mRouterTable.UpdateRouterIdSet(routerMaskTlv.GetIdSequence(), routerMaskTlv.GetAssignedRouterIdMask()); router = mRouterTable.GetRouter(routerId); VerifyOrExit(router != NULL); @@ -4244,15 +4244,7 @@ void MleRouter::SendAddressSolicitResponse(const Coap::Message & aRequest, routerMaskTlv.Init(); routerMaskTlv.SetIdSequence(mRouterTable.GetRouterIdSequence()); - routerMaskTlv.ClearAssignedRouterIdMask(); - - for (uint8_t routerId = 0; routerId <= kMaxRouterId; routerId++) - { - if (mRouterTable.IsAllocated(routerId)) - { - routerMaskTlv.SetAssignedRouterId(routerId); - } - } + routerMaskTlv.SetAssignedRouterIdMask(mRouterTable.GetRouterIdSet()); SuccessOrExit(error = routerMaskTlv.AppendTo(*message)); } @@ -4491,14 +4483,12 @@ void MleRouter::FillRouteTlv(RouteTlv &aTlv) uint8_t routerCount = 0; aTlv.SetRouterIdSequence(mRouterTable.GetRouterIdSequence()); - aTlv.ClearRouterIdMask(); + aTlv.SetRouterIdMask(mRouterTable.GetRouterIdSet()); for (RouterTable::Iterator iter(GetInstance()); !iter.IsDone(); iter++, routerCount++) { Router &router = *iter.GetRouter(); - aTlv.SetRouterId(router.GetRouterId()); - if (router.GetRloc16() == GetRloc16()) { aTlv.SetLinkQualityIn(routerCount, 0); diff --git a/src/core/thread/mle_tlvs.hpp b/src/core/thread/mle_tlvs.hpp index d19bff8d4..1cb6e4e8f 100644 --- a/src/core/thread/mle_tlvs.hpp +++ b/src/core/thread/mle_tlvs.hpp @@ -172,7 +172,7 @@ public: #if !OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE /** - * This class implements Source Address TLV generation and parsing. + * This class implements Route TLV generation and parsing. * */ OT_TOOL_PACKED_BEGIN @@ -187,7 +187,7 @@ public: { SetType(kRoute); SetLength(sizeof(*this) - sizeof(Tlv)); - memset(mRouterIdMask, 0, sizeof(mRouterIdMask)); + mRouterIdMask.Clear(); memset(mRouteData, 0, sizeof(mRouteData)); } @@ -217,32 +217,29 @@ public: void SetRouterIdSequence(uint8_t aSequence) { mRouterIdSequence = aSequence; } /** - * This method clears the Router ID Mask. + * This method gets the Router ID Mask. * */ - void ClearRouterIdMask(void) { memset(mRouterIdMask, 0, sizeof(mRouterIdMask)); } + const RouterIdSet &GetRouterIdMask(void) const { return mRouterIdMask; } + + /** + * This method sets the Router ID Mask. + * + * @param[in] aRouterIdSet The Router ID Mask to set. + * + */ + void SetRouterIdMask(const RouterIdSet &aRouterIdSet) { mRouterIdMask = aRouterIdSet; } /** * This method indicates whether or not a Router ID bit is set. * - * @param[in] aRouterId The Router ID. + * @param[in] aRouterId The Router ID bit. * * @retval TRUE If the Router ID bit is set. * @retval FALSE If the Router ID bit is not set. * */ - bool IsRouterIdSet(uint8_t aRouterId) const - { - return (mRouterIdMask[aRouterId / 8] & (0x80 >> (aRouterId % 8))) != 0; - } - - /** - * This method sets the Router ID bit. - * - * @param[in] aRouterId The Router ID bit to set. - * - */ - void SetRouterId(uint8_t aRouterId) { mRouterIdMask[aRouterId / 8] |= 0x80 >> (aRouterId % 8); } + bool IsRouterIdSet(uint8_t aRouterId) const { return mRouterIdMask.Contains(aRouterId); } /** * This method returns the Route Data Length value. @@ -344,15 +341,15 @@ private: kRouteCostOffset = 0, kRouteCostMask = 0xf << kRouteCostOffset, }; - uint8_t mRouterIdSequence; - uint8_t mRouterIdMask[BitVectorBytes(kMaxRouterId + 1)]; - uint8_t mRouteData[kMaxRouterId + 1]; + uint8_t mRouterIdSequence; + RouterIdSet mRouterIdMask; + uint8_t mRouteData[kMaxRouterId + 1]; } OT_TOOL_PACKED_END; #else // OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE /** - * This class implements Source Address TLV generation and parsing. + * This class implements Route TLV generation and parsing. * */ OT_TOOL_PACKED_BEGIN @@ -395,10 +392,18 @@ public: void SetRouterIdSequence(uint8_t aSequence) { mRouterIdSequence = aSequence; } /** - * This method clears the Router ID Mask. + * This method gets the Router ID Mask. * */ - void ClearRouterIdMask(void) { memset(mRouterIdMask, 0, sizeof(mRouterIdMask)); } + const RouterIdSet &GetRouterIdMask(void) const { return mRouterIdMask; } + + /** + * This method sets the Router ID Mask. + * + * @param[in] aRouterIdSet The Router ID Mask to set. + * + */ + void SetRouterIdMask(const RouterIdSet &aRouterIdSet) { mRouterIdMask = aRouterIdSet; } /** * This method indicates whether or not a Router ID bit is set. @@ -409,10 +414,7 @@ public: * @retval FALSE If the Router ID bit is not set. * */ - bool IsRouterIdSet(uint8_t aRouterId) const - { - return (mRouterIdMask[aRouterId / 8] & (0x80 >> (aRouterId % 8))) != 0; - } + bool IsRouterIdSet(uint8_t aRouterId) const { return mRouterIdMask.Contains(aRouterId); } /** * This method sets the Router ID bit. @@ -420,7 +422,7 @@ public: * @param[in] aRouterId The Router ID bit to set. * */ - void SetRouterId(uint8_t aRouterId) { mRouterIdMask[aRouterId / 8] |= 0x80 >> (aRouterId % 8); } + void SetRouterId(uint8_t aRouterId) { mRouterIdMask.Add(aRouterId); } /** * This method returns the Route Data Length value. @@ -560,10 +562,10 @@ private: kRouteCostMask = 0xf << kRouteCostOffset, kOddEntryOffset = 4, }; - uint8_t mRouterIdSequence; - uint8_t mRouterIdMask[BitVectorBytes(kMaxRouterId + 1)]; - // Since we do hold 12 (compressible to 11) bits of data per router, each entry occupies 1.5 bytes, consecutively. - // First 4 bits are link qualities, remaining 8 bits are route cost. + uint8_t mRouterIdSequence; + RouterIdSet mRouterIdMask; + // Since we do hold 12 (compressible to 11) bits of data per router, each entry occupies 1.5 bytes, + // consecutively. First 4 bits are link qualities, remaining 8 bits are route cost. uint8_t mRouteData[kMaxRouterId + 1 + kMaxRouterId / 2 + 1]; } OT_TOOL_PACKED_END; diff --git a/src/core/thread/mle_types.hpp b/src/core/thread/mle_types.hpp index 5664e5b41..4022bcab7 100644 --- a/src/core/thread/mle_types.hpp +++ b/src/core/thread/mle_types.hpp @@ -42,6 +42,7 @@ #include +#include "common/encoding.hpp" #include "common/string.hpp" #include "mac/mac_types.hpp" @@ -567,6 +568,72 @@ public: void SetLeaderRouterId(uint8_t aRouterId) { mLeaderRouterId = aRouterId; } }; +OT_TOOL_PACKED_BEGIN +class RouterIdSet +{ +public: + /** + * This method clears the Router Id Set. + * + */ + void Clear(void) { memset(mRouterIdSet, 0, sizeof(mRouterIdSet)); } + + /** + * This method indicates whether or not a Router ID bit is set. + * + * @param[in] aRouterId The Router ID. + * + * @retval TRUE If the Router ID bit is set. + * @retval FALSE If the Router ID bit is not set. + * + */ + bool Contains(uint8_t aRouterId) const { return (mRouterIdSet[aRouterId / 8] & (0x80 >> (aRouterId % 8))) != 0; } + + /** + * This method sets a given Router ID. + * + * @param[in] aRouterId The Router ID to set. + * + */ + void Add(uint8_t aRouterId) { mRouterIdSet[aRouterId / 8] |= 0x80 >> (aRouterId % 8); } + + /** + * This method removes a given Router ID. + * + * @param[in] aRouterId The Router ID to remove. + * + */ + void Remove(uint8_t aRouterId) { mRouterIdSet[aRouterId / 8] &= ~(0x80 >> (aRouterId % 8)); } + + /** + * This method returns whether or not the Router ID sets are equal. + * + * @param[in] aOther The other Router ID Set to compare with. + * + * @retval TRUE If the Router ID sets are equal. + * @retval FALSE If the Router ID sets are not equal. + * + */ + bool operator==(const RouterIdSet &aOther) const + { + return memcmp(mRouterIdSet, aOther.mRouterIdSet, sizeof(mRouterIdSet)) == 0; + } + + /** + * This method returns whether or not the Router ID sets are not equal. + * + * @param[in] aOther The other Router ID Set to compare with. + * + * @retval TRUE If the Router ID sets are not equal. + * @retval FALSE If the Router ID sets are equal. + * + */ + bool operator!=(const RouterIdSet &aOther) const { return !(*this == aOther); } + +private: + uint8_t mRouterIdSet[BitVectorBytes(Mle::kMaxRouterId + 1)]; +} OT_TOOL_PACKED_END; + /** * @} * diff --git a/src/core/thread/network_diagnostic_tlvs.hpp b/src/core/thread/network_diagnostic_tlvs.hpp index 6953e110a..7489b1e6d 100644 --- a/src/core/thread/network_diagnostic_tlvs.hpp +++ b/src/core/thread/network_diagnostic_tlvs.hpp @@ -578,6 +578,7 @@ public: { SetType(kRoute); SetLength(sizeof(*this) - sizeof(NetworkDiagnosticTlv)); + mRouterIdMask.Clear(); } /** @@ -605,12 +606,6 @@ public: */ void SetRouterIdSequence(uint8_t aSequence) { mRouterIdSequence = aSequence; } - /** - * This method clears the Router ID Mask. - * - */ - void ClearRouterIdMask(void) { memset(mRouterIdMask, 0, sizeof(mRouterIdMask)); } - /** * This method indicates whether or not a Router ID bit is set. * @@ -620,10 +615,7 @@ public: * @retval FALSE If the Router ID bit is not set. * */ - bool IsRouterIdSet(uint8_t aRouterId) const - { - return (mRouterIdMask[aRouterId / 8] & (0x80 >> (aRouterId % 8))) != 0; - } + bool IsRouterIdSet(uint8_t aRouterId) const { return mRouterIdMask.Contains(aRouterId); } /** * This method sets the Router ID bit. @@ -631,7 +623,7 @@ public: * @param[in] aRouterId The Router ID bit to set. * */ - void SetRouterId(uint8_t aRouterId) { mRouterIdMask[aRouterId / 8] |= 0x80 >> (aRouterId % 8); } + void SetRouterId(uint8_t aRouterId) { mRouterIdMask.Add(aRouterId); } /** * This method returns the Route Data Length value. @@ -733,9 +725,9 @@ private: kRouteCostOffset = 0, kRouteCostMask = 0xf << kRouteCostOffset, }; - uint8_t mRouterIdSequence; - uint8_t mRouterIdMask[BitVectorBytes(Mle::kMaxRouterId + 1)]; - uint8_t mRouteData[Mle::kMaxRouterId + 1]; + uint8_t mRouterIdSequence; + Mle::RouterIdSet mRouterIdMask; + uint8_t mRouteData[Mle::kMaxRouterId + 1]; } OT_TOOL_PACKED_END; /** diff --git a/src/core/thread/router_table.cpp b/src/core/thread/router_table.cpp index 27f9fa703..4a22d3fb6 100644 --- a/src/core/thread/router_table.cpp +++ b/src/core/thread/router_table.cpp @@ -306,7 +306,7 @@ exit: return error; } -void RouterTable::RemoveNeighbor(Router &aRouter) +void RouterTable::RemoveRouterLink(Router &aRouter) { aRouter.SetLinkQualityOut(0); aRouter.SetLastHeard(TimerMilli::GetNow()); @@ -502,76 +502,34 @@ exit: return rval; } -void RouterTable::ProcessTlv(const Mle::RouteTlv &aTlv) +void RouterTable::UpdateRouterIdSet(uint8_t aRouterIdSequence, const Mle::RouterIdSet &aRouterIdSet) { - bool allocationChanged = false; - - mRouterIdSequence = aTlv.GetRouterIdSequence(); + mRouterIdSequence = aRouterIdSequence; mRouterIdSequenceLastUpdated = TimerMilli::GetNow(); + VerifyOrExit(mAllocatedRouterIds != aRouterIdSet); + for (uint8_t routerId = 0; routerId <= Mle::kMaxRouterId; routerId++) { - if (aTlv.IsRouterIdSet(routerId) == IsAllocated(routerId)) - { - continue; - } - - allocationChanged = true; - - if (aTlv.IsRouterIdSet(routerId)) - { - mAllocatedRouterIds.Add(routerId); - } - else + // If was allocated but removed in new Router Id Set + if (IsAllocated(routerId) && !aRouterIdSet.Contains(routerId)) { Router *router = GetRouter(routerId); OT_ASSERT(router != NULL); router->SetNextHop(Mle::kInvalidRouterId); - RemoveNeighbor(*router); + RemoveRouterLink(*router); mAllocatedRouterIds.Remove(routerId); } } - if (allocationChanged) - { - UpdateAllocation(); - Get().ResetAdvertiseInterval(); - } -} + mAllocatedRouterIds = aRouterIdSet; + UpdateAllocation(); + Get().ResetAdvertiseInterval(); -void RouterTable::ProcessTlv(const ThreadRouterMaskTlv &aTlv) -{ - bool allocationChanged = false; - - mRouterIdSequence = aTlv.GetIdSequence(); - mRouterIdSequenceLastUpdated = TimerMilli::GetNow(); - - for (uint8_t routerId = 0; routerId <= Mle::kMaxRouterId; routerId++) - { - if (aTlv.IsAssignedRouterIdSet(routerId) == IsAllocated(routerId)) - { - continue; - } - - allocationChanged = true; - - if (aTlv.IsAssignedRouterIdSet(routerId)) - { - mAllocatedRouterIds.Add(routerId); - } - else - { - mAllocatedRouterIds.Remove(routerId); - } - } - - if (allocationChanged) - { - UpdateAllocation(); - Get().ResetAdvertiseInterval(); - } +exit: + return; } void RouterTable::ProcessTimerTick(void) diff --git a/src/core/thread/router_table.hpp b/src/core/thread/router_table.hpp index f4f09233f..b3916a87f 100644 --- a/src/core/thread/router_table.hpp +++ b/src/core/thread/router_table.hpp @@ -163,12 +163,12 @@ public: otError Release(uint8_t aRouterId); /** - * This method removes a neighboring router link. + * This method removes a router link. * * @param[in] aRouter A reference to the router. * */ - void RemoveNeighbor(Router &aRouter); + void RemoveRouterLink(Router &aRouter); /** * This method returns the number of active routers in the Thread network. @@ -312,20 +312,21 @@ public: bool IsAllocated(uint8_t aRouterId) const; /** - * This method updates the router table with a received Route TLV. + * This method updates the Router ID allocation. * - * @param[in] aTlv A reference to the Route TLV. + * @param[in] aRouterIdSequence The Router Id Sequence. + * @param[in] aRouterIdSet A reference to the Router Id Set. * */ - void ProcessTlv(const Mle::RouteTlv &aTlv); + void UpdateRouterIdSet(uint8_t aRouterIdSequence, const Mle::RouterIdSet &aRouterIdSet); /** - * This method updates the router table with a received Router Mask TLV. + * This method gets the allocated Router ID set. * - * @param[in] aTlv A reference to the Router Mask TLV. + * @returns The allocated Router ID set. * */ - void ProcessTlv(const ThreadRouterMaskTlv &aTlv); + const Mle::RouterIdSet &GetRouterIdSet(void) const { return mAllocatedRouterIds; } /** * This method updates the router table and must be called with a one second period. @@ -334,18 +335,6 @@ public: void ProcessTimerTick(void); private: - class RouterIdSet - { - public: - void Clear(void) { memset(mRouterIdSet, 0, sizeof(mRouterIdSet)); } - bool Contains(uint8_t aRouterId) const { return (mRouterIdSet[aRouterId / 8] & (1 << (aRouterId % 8))) != 0; } - void Add(uint8_t aRouterId) { mRouterIdSet[aRouterId / 8] |= 1 << (aRouterId % 8); } - void Remove(uint8_t aRouterId) { mRouterIdSet[aRouterId / 8] &= ~(1 << (aRouterId % 8)); } - - private: - uint8_t mRouterIdSet[BitVectorBytes(Mle::kMaxRouterId + 1)]; - }; - void UpdateAllocation(void); const Router *GetFirstEntry(void) const; const Router *GetNextEntry(const Router *aRouter) const; @@ -355,12 +344,12 @@ private: return const_cast(const_cast(this)->GetNextEntry(aRouter)); } - Router mRouters[Mle::kMaxRouters]; - RouterIdSet mAllocatedRouterIds; - uint8_t mRouterIdReuseDelay[Mle::kMaxRouterId + 1]; - TimeMilli mRouterIdSequenceLastUpdated; - uint8_t mRouterIdSequence; - uint8_t mActiveRouterCount; + Router mRouters[Mle::kMaxRouters]; + Mle::RouterIdSet mAllocatedRouterIds; + uint8_t mRouterIdReuseDelay[Mle::kMaxRouterId + 1]; + TimeMilli mRouterIdSequenceLastUpdated; + uint8_t mRouterIdSequence; + uint8_t mActiveRouterCount; }; } // namespace ot diff --git a/src/core/thread/thread_tlvs.hpp b/src/core/thread/thread_tlvs.hpp index 2c4a4c692..47f6cf338 100644 --- a/src/core/thread/thread_tlvs.hpp +++ b/src/core/thread/thread_tlvs.hpp @@ -41,6 +41,7 @@ #include "common/tlvs.hpp" #include "net/ip6_address.hpp" #include "thread/mle.hpp" +#include "thread/mle_types.hpp" namespace ot { @@ -149,6 +150,7 @@ public: { SetType(kRouterMask); SetLength(sizeof(*this) - sizeof(ThreadTlv)); + mAssignedRouterIdMask.Clear(); } /** @@ -177,36 +179,24 @@ public: void SetIdSequence(uint8_t aSequence) { mIdSequence = aSequence; } /** - * This method clears the Assigned Router ID Mask. + * This method gets the Assigned Router ID Mask. + * + * @returns The Assigned Router ID Mask. * */ - void ClearAssignedRouterIdMask(void) { memset(mAssignedRouterIdMask, 0, sizeof(mAssignedRouterIdMask)); } + const Mle::RouterIdSet &GetAssignedRouterIdMask(void) const { return mAssignedRouterIdMask; } /** - * This method indicates whether or not a given Router ID is set in the Assigned Router ID Mask. + * This method sets the Assigned Router ID Mask. * - * @param[in] aRouterId The Router ID. - * - * @retval TRUE If the given Router ID is set in the Assigned Router ID Mask. - * @retval FALSE If the given Router ID is not set in the Assigned Router ID Mask. + * @param[in] aRouterIdSet A reference to the Assigned Router ID Mask. * */ - bool IsAssignedRouterIdSet(uint8_t aRouterId) const - { - return (mAssignedRouterIdMask[aRouterId / 8] & (0x80 >> (aRouterId % 8))) != 0; - } - - /** - * This method clears the Assigned Router ID Mask. - * - * @param[in] aRouterId The Router ID. - * - */ - void SetAssignedRouterId(uint8_t aRouterId) { mAssignedRouterIdMask[aRouterId / 8] |= 0x80 >> (aRouterId % 8); } + void SetAssignedRouterIdMask(const Mle::RouterIdSet &aRouterIdSet) { mAssignedRouterIdMask = aRouterIdSet; } private: - uint8_t mIdSequence; - uint8_t mAssignedRouterIdMask[BitVectorBytes(Mle::kMaxRouterId + 1)]; + uint8_t mIdSequence; + Mle::RouterIdSet mAssignedRouterIdMask; }; /**