From 75522066c1cc47b0d9d2a1681987a5da4ef8ce7a Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 17 Sep 2018 15:05:18 -0700 Subject: [PATCH] [network-data] remove all matching entries from `RemoveRloc` methods (#3082) This commit makes changes to ensure the `Leader::RemoveRloc` methods remove all matching entries from a `BorderRouterTlv` or `HasRouteTlv` It adds new methods to `BorderRouterTlv` and `HasRouteTlv` to get the first entry or last entry and allow an easier way to iterate over all the entries. This commit also adds a new helper method `RlocMatch()` to simplify the comparison of RLOC16 values (exact match vs router id match). --- src/core/thread/network_data_leader_ftd.cpp | 54 +++++++++---------- src/core/thread/network_data_leader_ftd.hpp | 2 + src/core/thread/network_data_tlvs.hpp | 58 +++++++++++++++++++++ 3 files changed, 84 insertions(+), 30 deletions(-) diff --git a/src/core/thread/network_data_leader_ftd.cpp b/src/core/thread/network_data_leader_ftd.cpp index 71a12460b..2a0b3bb46 100644 --- a/src/core/thread/network_data_leader_ftd.cpp +++ b/src/core/thread/network_data_leader_ftd.cpp @@ -433,6 +433,12 @@ exit: } } +bool Leader::RlocMatch(uint16_t aFirstRloc16, uint16_t aSecondRloc16, bool aExactMatch) +{ + return ((aExactMatch && aFirstRloc16 == aSecondRloc16) || + (!aExactMatch && Mle::Mle::RouterIdMatch(aFirstRloc16, aSecondRloc16))); +} + otError Leader::RlocLookup(uint16_t aRloc16, bool & aIn, bool & aStable, @@ -485,8 +491,7 @@ otError Leader::RlocLookup(uint16_t aRloc16, { borderRouterEntry = borderRouter->GetEntry(i); - if ((aExactMatch && borderRouterEntry->GetRloc() == aRloc16) || - (!aExactMatch && (Mle::Mle::RouterIdMatch(borderRouterEntry->GetRloc(), aRloc16)))) + if (RlocMatch(borderRouterEntry->GetRloc(), aRloc16, aExactMatch)) { aIn = true; @@ -510,8 +515,7 @@ otError Leader::RlocLookup(uint16_t aRloc16, { hasRouteEntry = hasRoute->GetEntry(i); - if ((aExactMatch && hasRouteEntry->GetRloc() == aRloc16) || - (!aExactMatch && (Mle::Mle::RouterIdMatch(hasRouteEntry->GetRloc(), aRloc16)))) + if (RlocMatch(hasRouteEntry->GetRloc(), aRloc16, aExactMatch)) { aIn = true; @@ -564,8 +568,7 @@ otError Leader::RlocLookup(uint16_t aRloc16, server = static_cast(subCur); VerifyOrExit(server->IsValid(), error = OT_ERROR_PARSE); - if ((aExactMatch && server->GetServer16() == aRloc16) || - (!aExactMatch && (Mle::Mle::RouterIdMatch(server->GetServer16(), aRloc16)))) + if (RlocMatch(server->GetServer16(), aRloc16, aExactMatch)) { aIn = true; @@ -1387,8 +1390,7 @@ otError Leader::RemoveRloc(ServiceTlv &service, uint16_t aRloc16, bool aExactMat case NetworkDataTlv::kTypeServer: server = static_cast(cur); - if ((aExactMatch && server->GetServer16() == aRloc16) || - (!aExactMatch && (Mle::Mle::RouterIdMatch(server->GetServer16(), aRloc16)))) + if (RlocMatch(server->GetServer16(), aRloc16, aExactMatch)) { removeLength = sizeof(ServerTlv) + server->GetServerDataLength(); service.SetSubTlvsLength(service.GetSubTlvsLength() - removeLength); @@ -1411,23 +1413,19 @@ otError Leader::RemoveRloc(ServiceTlv &service, uint16_t aRloc16, bool aExactMat otError Leader::RemoveRloc(PrefixTlv &aPrefix, HasRouteTlv &aHasRoute, uint16_t aRloc16, bool aExactMatch) { - HasRouteEntry *entry; + HasRouteEntry *entry = aHasRoute.GetFirstEntry(); - // remove rloc from has route tlv - for (uint8_t i = 0; i < aHasRoute.GetNumEntries(); i++) + while (entry <= aHasRoute.GetLastEntry()) { - entry = aHasRoute.GetEntry(i); - - if ((aExactMatch && entry->GetRloc() != aRloc16) || - (!aExactMatch && !(Mle::Mle::RouterIdMatch(entry->GetRloc(), aRloc16)))) + if (RlocMatch(entry->GetRloc(), aRloc16, aExactMatch)) { + aHasRoute.SetLength(aHasRoute.GetLength() - sizeof(HasRouteEntry)); + aPrefix.SetSubTlvsLength(aPrefix.GetSubTlvsLength() - sizeof(HasRouteEntry)); + Remove(reinterpret_cast(entry), sizeof(HasRouteEntry)); continue; } - aHasRoute.SetLength(aHasRoute.GetLength() - sizeof(HasRouteEntry)); - aPrefix.SetSubTlvsLength(aPrefix.GetSubTlvsLength() - sizeof(HasRouteEntry)); - Remove(reinterpret_cast(entry), sizeof(*entry)); - break; + entry = entry->GetNext(); } return OT_ERROR_NONE; @@ -1435,23 +1433,19 @@ otError Leader::RemoveRloc(PrefixTlv &aPrefix, HasRouteTlv &aHasRoute, uint16_t otError Leader::RemoveRloc(PrefixTlv &aPrefix, BorderRouterTlv &aBorderRouter, uint16_t aRloc16, bool aExactMatch) { - BorderRouterEntry *entry; + BorderRouterEntry *entry = aBorderRouter.GetFirstEntry(); - // remove rloc from border router tlv - for (uint8_t i = 0; i < aBorderRouter.GetNumEntries(); i++) + while (entry <= aBorderRouter.GetLastEntry()) { - entry = aBorderRouter.GetEntry(i); - - if ((aExactMatch && entry->GetRloc() != aRloc16) || - (!aExactMatch && !(Mle::Mle::RouterIdMatch(entry->GetRloc(), aRloc16)))) + if (RlocMatch(entry->GetRloc(), aRloc16, aExactMatch)) { + aBorderRouter.SetLength(aBorderRouter.GetLength() - sizeof(BorderRouterEntry)); + aPrefix.SetSubTlvsLength(aPrefix.GetSubTlvsLength() - sizeof(BorderRouterEntry)); + Remove(reinterpret_cast(entry), sizeof(*entry)); continue; } - aBorderRouter.SetLength(aBorderRouter.GetLength() - sizeof(BorderRouterEntry)); - aPrefix.SetSubTlvsLength(aPrefix.GetSubTlvsLength() - sizeof(BorderRouterEntry)); - Remove(reinterpret_cast(entry), sizeof(*entry)); - break; + entry = entry->GetNext(); } return OT_ERROR_NONE; diff --git a/src/core/thread/network_data_leader_ftd.hpp b/src/core/thread/network_data_leader_ftd.hpp index b29e01803..aa5d12055 100644 --- a/src/core/thread/network_data_leader_ftd.hpp +++ b/src/core/thread/network_data_leader_ftd.hpp @@ -191,6 +191,8 @@ private: otError RemoveRloc(PrefixTlv &aPrefix, HasRouteTlv &aHasRoute, uint16_t aRloc16, bool aExactMatch); otError RemoveRloc(PrefixTlv &aPrefix, BorderRouterTlv &aBorderRouter, uint16_t aRloc16, bool aExactMatch); + static bool RlocMatch(uint16_t aFirstRloc16, uint16_t aSecondRloc16, bool aExactMatch); + otError RlocLookup(uint16_t aRloc16, bool & aIn, bool & aStable, diff --git a/src/core/thread/network_data_tlvs.hpp b/src/core/thread/network_data_tlvs.hpp index f0330fee0..293aa68ae 100644 --- a/src/core/thread/network_data_tlvs.hpp +++ b/src/core/thread/network_data_tlvs.hpp @@ -226,6 +226,14 @@ public: mFlags = (mFlags & ~kPreferenceMask) | ((aPrf << kPreferenceOffset) & kPreferenceMask); } + /** + * This method returns a pointer to the next HasRouteEntry. + * + * @returns A pointer to the next HasRouteEntry. + * + */ + HasRouteEntry *GetNext(void) { return (this + 1); } + private: enum { @@ -276,6 +284,27 @@ public: { return reinterpret_cast(GetValue() + (i * sizeof(HasRouteEntry))); } + + /** + * This method returns a pointer to the first HasRouteEntry (at index 0'th). + * + * @returns A pointer to the first HasRouteEntry. + * + */ + HasRouteEntry *GetFirstEntry(void) { return reinterpret_cast(GetValue()); } + + /** + * This method returns a pointer to the last HasRouteEntry. + * + * If there are no entries the pointer will be invalid but guaranteed to be before the `GetFirstEntry()` pointer. + * + * @returns A pointer to the last HasRouteEntry. + * + */ + HasRouteEntry *GetLastEntry(void) + { + return reinterpret_cast(GetValue() + GetLength() - sizeof(HasRouteEntry)); + } } OT_TOOL_PACKED_END; /** @@ -586,6 +615,14 @@ public: */ void SetOnMesh(void) { mFlags |= kOnMeshFlag; } + /** + * This method returns a pointer to the next BorderRouterEntry + * + * @returns A pointer to the next BorderRouterEntry. + * + */ + BorderRouterEntry *GetNext(void) { return (this + 1); } + private: uint16_t mRloc; uint8_t mFlags; @@ -631,6 +668,27 @@ public: { return reinterpret_cast(GetValue() + (i * sizeof(BorderRouterEntry))); } + + /** + * This method returns a pointer to the first BorderRouterEntry (at index 0'th). + * + * @returns A pointer to the first BorderRouterEntry. + * + */ + BorderRouterEntry *GetFirstEntry(void) { return reinterpret_cast(GetValue()); } + + /** + * This method returns a pointer to the last BorderRouterEntry. + * + * If there are no entries the pointer will be invalid but guaranteed to be before the `GetFirstEntry()` pointer. + * + * @returns A pointer to the last BorderRouterEntry. + * + */ + BorderRouterEntry *GetLastEntry(void) + { + return reinterpret_cast(GetValue() + GetLength() - sizeof(BorderRouterEntry)); + } } OT_TOOL_PACKED_END; /**