diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index 5674bffb6..ce5413b4d 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -3373,9 +3373,13 @@ void Mle::HandleChildIdResponse(RxInfo &aRxInfo) #if OPENTHREAD_FTD if (IsFullThreadDevice()) { - switch (Get().ProcessRouteTlv(aRxInfo)) + RouteTlv routeTlv; + + switch (Get().ProcessRouteTlv(aRxInfo, routeTlv)) { case kErrorNone: + Get().UpdateRoutesOnFed(routeTlv, RouterIdFromRloc16(sourceAddress)); + break; case kErrorNotFound: break; default: diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index 9ddade460..c5fb11234 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -1276,8 +1276,6 @@ Error MleRouter::HandleAdvertisement(RxInfo &aRxInfo) if (IsFullThreadDevice()) { - Router *leader; - if ((mRouterSelectionJitterTimeout == 0) && (mRouterTable.GetActiveRouterCount() < mRouterUpgradeThreshold)) { @@ -1285,37 +1283,7 @@ Error MleRouter::HandleAdvertisement(RxInfo &aRxInfo) ExitNow(); } - leader = mRouterTable.GetLeader(); - - if (leader != nullptr) - { - for (uint8_t id = 0, routeCount = 0; id <= kMaxRouterId; id++) - { - if (!routeTlv.IsRouterIdSet(id)) - { - continue; - } - - if (id != GetLeaderId()) - { - routeCount++; - continue; - } - - if (routeTlv.GetRouteCost(routeCount) > 0) - { - leader->SetNextHop(id); - leader->SetCost(routeTlv.GetRouteCost(routeCount)); - } - else - { - leader->SetNextHop(kInvalidRouterId); - leader->SetCost(0); - } - - break; - } - } + mRouterTable.UpdateRoutesOnFed(routeTlv, routerId); } } else @@ -1397,7 +1365,6 @@ void MleRouter::HandleParentRequest(RxInfo &aRxInfo) uint16_t version; uint8_t scanMask; Challenge challenge; - Router *leader; Child *child; uint8_t modeBitmask; DeviceMode mode; @@ -1421,13 +1388,7 @@ void MleRouter::HandleParentRequest(RxInfo &aRxInfo) VerifyOrExit(mRouterTable.GetLeaderAge() < mNetworkIdTimeout, error = kErrorDrop); // 3. Its current routing path cost to the Leader is infinite. - leader = mRouterTable.GetLeader(); - OT_ASSERT(leader != nullptr); - - VerifyOrExit(IsLeader() || mRouterTable.GetLinkCost(GetLeaderId()) < kMaxRouteCost || - (IsChild() && leader->GetCost() + 1 < kMaxRouteCost) || - (leader->GetCost() + mRouterTable.GetLinkCost(leader->GetNextHop()) < kMaxRouteCost), - error = kErrorDrop); + VerifyOrExit(mRouterTable.GetPathCostToLeader() < kMaxRouteCost, error = kErrorDrop); // 4. It is a REED and there are already `kMaxRouters` active routers in // the network (because Leader would reject any further address solicit). @@ -3313,7 +3274,7 @@ void MleRouter::ResolveRoutingLoops(uint16_t aSourceMac, uint16_t aDestRloc16) VerifyOrExit(router != nullptr); // invalidate next hop - router->SetNextHop(kInvalidRouterId); + router->SetNextHopToInvalid(); ResetAdvertiseInterval(); exit: @@ -3490,7 +3451,7 @@ void MleRouter::HandleAddressSolicitResponse(Coap::Message *aMessage, VerifyOrExit(router != nullptr); router->SetExtAddress(Get().GetExtAddress()); - router->SetCost(0); + router->SetNextHopToInvalid(); router = mRouterTable.FindRouterById(mParent.GetRouterId()); VerifyOrExit(router != nullptr); @@ -3499,8 +3460,7 @@ void MleRouter::HandleAddressSolicitResponse(Coap::Message *aMessage, router->SetFrom(mParent); router->SetState(Neighbor::kStateValid); - router->SetNextHop(kInvalidRouterId); - router->SetCost(0); + router->SetNextHopToInvalid(); leader = mRouterTable.GetLeader(); OT_ASSERT(leader != nullptr); @@ -3508,8 +3468,7 @@ void MleRouter::HandleAddressSolicitResponse(Coap::Message *aMessage, if (leader != router) { // Keep route path to the Leader reported by the parent before it is updated. - leader->SetCost(mParent.GetLeaderCost()); - leader->SetNextHop(RouterIdFromRloc16(mParent.GetRloc16())); + leader->SetNextHopAndCost(RouterIdFromRloc16(mParent.GetRloc16()), mParent.GetLeaderCost()); } if (isRouterIdAllocated) @@ -3707,9 +3666,7 @@ exit: void MleRouter::FillConnectivityTlv(ConnectivityTlv &aTlv) { - Router *leader; - uint8_t cost; - int8_t parentPriority = kParentPriorityMedium; + int8_t parentPriority = kParentPriorityMedium; if (mParentPriority != kParentPriorityUnspecified) { @@ -3732,22 +3689,12 @@ void MleRouter::FillConnectivityTlv(ConnectivityTlv &aTlv) aTlv.SetParentPriority(parentPriority); - // compute leader cost and link qualities aTlv.SetLinkQuality1(0); aTlv.SetLinkQuality2(0); aTlv.SetLinkQuality3(0); - leader = mRouterTable.GetLeader(); - cost = (leader != nullptr) ? leader->GetCost() : static_cast(kMaxRouteCost); - - switch (mRole) + if (IsChild()) { - case kRoleDisabled: - case kRoleDetached: - cost = static_cast(kMaxRouteCost); - break; - - case kRoleChild: switch (mParent.GetLinkQualityIn()) { case kLinkQuality0: @@ -3765,30 +3712,8 @@ void MleRouter::FillConnectivityTlv(ConnectivityTlv &aTlv) aTlv.SetLinkQuality3(aTlv.GetLinkQuality3() + 1); break; } - - cost += CostForLinkQuality(mParent.GetLinkQualityIn()); - break; - - case kRoleRouter: - if (leader != nullptr) - { - cost += mRouterTable.GetLinkCost(leader->GetNextHop()); - - if (!IsRouterIdValid(leader->GetNextHop()) || mRouterTable.GetLinkCost(GetLeaderId()) < cost) - { - cost = mRouterTable.GetLinkCost(GetLeaderId()); - } - } - - break; - - case kRoleLeader: - cost = 0; - break; } - aTlv.SetActiveRouters(mRouterTable.GetActiveRouterCount()); - for (const Router &router : Get()) { if (router.GetRloc16() == GetRloc16()) @@ -3822,7 +3747,8 @@ void MleRouter::FillConnectivityTlv(ConnectivityTlv &aTlv) } } - aTlv.SetLeaderCost((cost < kMaxRouteCost) ? cost : static_cast(kMaxRouteCost)); + aTlv.SetActiveRouters(mRouterTable.GetActiveRouterCount()); + aTlv.SetLeaderCost(Min(mRouterTable.GetPathCostToLeader(), kMaxRouteCost)); aTlv.SetIdSequence(mRouterTable.GetRouterIdSequence()); aTlv.SetSedBufferSize(OPENTHREAD_CONFIG_DEFAULT_SED_BUFFER_SIZE); aTlv.SetSedDatagramCount(OPENTHREAD_CONFIG_DEFAULT_SED_DATAGRAM_COUNT); diff --git a/src/core/thread/router_table.cpp b/src/core/thread/router_table.cpp index da4539c3a..611004be2 100644 --- a/src/core/thread/router_table.cpp +++ b/src/core/thread/router_table.cpp @@ -88,7 +88,7 @@ Router *RouterTable::AddRouter(uint8_t aRouterId) router->Clear(); router->SetRloc16(Mle::Rloc16FromRouterId(aRouterId)); - router->SetNextHop(Mle::kInvalidRouterId); + router->SetNextHopToInvalid(); mRouterIdMap.SetIndex(aRouterId, mRouters.IndexOf(*router)); @@ -197,8 +197,7 @@ Error RouterTable::Release(uint8_t aRouterId) { if (otherRouter.GetNextHop() == aRouterId) { - otherRouter.SetNextHop(Mle::kInvalidRouterId); - otherRouter.SetCost(0); + otherRouter.SetNextHopToInvalid(); } } @@ -228,8 +227,7 @@ void RouterTable::RemoveRouterLink(Router &aRouter) { if (router.GetNextHop() == aRouter.GetRouterId()) { - router.SetNextHop(Mle::kInvalidRouterId); - router.SetCost(0); + router.SetNextHopToInvalid(); if (GetLinkCost(router) >= Mle::kMaxRouteCost) { @@ -324,7 +322,7 @@ exit: return error; } -Router *RouterTable::GetLeader(void) { return FindRouterById(Get().GetLeaderId()); } +const Router *RouterTable::GetLeader(void) const { return FindRouterById(Get().GetLeaderId()); } uint32_t RouterTable::GetLeaderAge(void) const { @@ -381,6 +379,8 @@ uint8_t RouterTable::GetPathCost(uint16_t aDestRloc16) const const Router *router; const Router *nextHop; + VerifyOrExit(Get().IsAttached()); + if (aDestRloc16 == Get().GetRloc16()) { // Destination is this device, return cost as zero. @@ -388,51 +388,61 @@ uint8_t RouterTable::GetPathCost(uint16_t aDestRloc16) const ExitNow(cost = 0); } - if (Get().IsChild()) - { - // If device is a child, then check if destination is our parent - // and determine cost based on the link quality to parent. - // Otherwise we cannot determine the cost. - - VerifyOrExit(aDestRloc16 == Get().GetParent().GetRloc16()); - cost = CostForLinkQuality(Get().GetParent().GetLinkQualityIn()); - ExitNow(); - } - destRouterId = Mle::RouterIdFromRloc16(aDestRloc16); - if (destRouterId == Mle::RouterIdFromRloc16(Get().GetRloc16())) + router = FindRouterById(destRouterId); + nextHop = (router != nullptr) ? FindNextHopOf(*router) : nullptr; + + if (Get().IsChild()) { - // `aDestRloc16` is a one of device's children. We know the - // device is not a child from `IsChild()` check above and - // that the destination is not device itself (from first `if` - // check above). + const Router &parent = Get().GetParent(); - const Child *child = Get().FindChild(aDestRloc16, Child::kInStateValid); + // If destination is our parent or another child of our + // parent, we use the link cost to our parent. Otherwise we + // check if we have a next hop towards the destination and + // add its cost to the link cost to parent. - VerifyOrExit(child != nullptr); - ExitNow(cost = CostForLinkQuality(child->GetLinkQualityIn())); + VerifyOrExit((destRouterId == parent.GetRouterId()) || (nextHop != nullptr)); + + cost = CostForLinkQuality(parent.GetLinkQualityIn()); + + if (destRouterId != parent.GetRouterId()) + { + cost += router->GetCost(); + } + + // The case where destination itself is a child is handled at + // the end (after `else` block). } - - router = FindRouterById(destRouterId); - - VerifyOrExit(router != nullptr); - - cost = GetLinkCost(*router); - - nextHop = FindNextHopOf(*router); - - if (nextHop != nullptr) + else // Role is router or leader { - // Determine whether direct link or forwarding hop link - // has a lower cost. - cost = Min(cost, static_cast(router->GetCost() + GetLinkCost(*nextHop))); + if (destRouterId == Mle::RouterIdFromRloc16(Get().GetRloc16())) + { + // Destination is a one of our children. + + const Child *child = Get().FindChild(aDestRloc16, Child::kInStateValid); + + VerifyOrExit(child != nullptr); + ExitNow(cost = CostForLinkQuality(child->GetLinkQualityIn())); + } + + VerifyOrExit(router != nullptr); + + cost = GetLinkCost(*router); + + if (nextHop != nullptr) + { + // Determine whether direct link or forwarding hop link + // has a lower cost. + cost = Min(cost, static_cast(router->GetCost() + GetLinkCost(*nextHop))); + } } if (!Mle::IsActiveRouter(aDestRloc16)) { // Destination is a child. we assume best link quality // between destination and its parent router. + cost += kCostForLinkQuality3; } @@ -440,6 +450,11 @@ exit: return cost; } +uint8_t RouterTable::GetPathCostToLeader(void) const +{ + return GetPathCost(Mle::Rloc16FromRouterId(Get().GetLeaderId())); +} + uint16_t RouterTable::GetNextHop(uint16_t aDestRloc16) const { uint16_t nextHopRloc16 = Mle::kInvalidRloc16; @@ -512,7 +527,7 @@ void RouterTable::UpdateRouterIdSet(uint8_t aRouterIdSequence, const Mle::Router Router *router = FindRouterById(routerId); OT_ASSERT(router != nullptr); - router->SetNextHop(Mle::kInvalidRouterId); + router->SetNextHopToInvalid(); RemoveRouterLink(*router); RemoveRouter(*router); } @@ -592,15 +607,8 @@ void RouterTable::UpdateRoutes(const Mle::RouteTlv &aRouteTlv, uint8_t aRouterId resetAdvInterval = true; } - if (router->GetNextHop() != aRouterId) + if (router->SetNextHopAndCost(aRouterId, cost)) { - router->SetNextHop(aRouterId); - changed = true; - } - - if (router->GetCost() != cost) - { - router->SetCost(cost); changed = true; } } @@ -611,8 +619,7 @@ void RouterTable::UpdateRoutes(const Mle::RouteTlv &aRouteTlv, uint8_t aRouterId resetAdvInterval = true; } - router->SetNextHop(Mle::kInvalidRouterId); - router->SetCost(0); + router->SetNextHopToInvalid(); router->SetLastHeard(TimerMilli::GetNow()); changed = true; } @@ -624,8 +631,7 @@ void RouterTable::UpdateRoutes(const Mle::RouteTlv &aRouteTlv, uint8_t aRouterId if (newCost < curCost) { - router->SetNextHop(aRouterId); - router->SetCost(cost); + router->SetNextHopAndCost(aRouterId, cost); changed = true; } } @@ -685,6 +691,44 @@ exit: return changed; } +void RouterTable::UpdateRoutesOnFed(const Mle::RouteTlv &aRouteTlv, uint8_t aParentId) +{ + bool changed = false; + + for (uint8_t routerId = 0, index = 0; routerId <= Mle::kMaxRouterId; + index += aRouteTlv.IsRouterIdSet(routerId) ? 1 : 0, routerId++) + { + Router *router; + uint8_t cost; + uint8_t nextHopId; + + if (!aRouteTlv.IsRouterIdSet(routerId) || (routerId == aParentId)) + { + continue; + } + + router = FindRouterById(routerId); + + if (router == nullptr) + { + continue; + } + + cost = aRouteTlv.GetRouteCost(index); + nextHopId = (cost == 0) ? Mle::kInvalidRouterId : aParentId; + + if (router->SetNextHopAndCost(nextHopId, cost)) + { + changed = true; + } + } + + if (changed) + { + LogRouteTable(); + } +} + void RouterTable::FillRouteTlv(Mle::RouteTlv &aRouteTlv, const Neighbor *aNeighbor) const { uint8_t routerIdSequence = mRouterIdSequence; @@ -856,6 +900,10 @@ void RouterTable::LogRouteTable(void) const { string.Append(" - me"); } + else if (Get().IsChild() && (router.GetRloc16() == Get().GetParent().GetRloc16())) + { + string.Append(" - parent"); + } else { if (router.IsStateValid()) diff --git a/src/core/thread/router_table.hpp b/src/core/thread/router_table.hpp index 99e42e182..e601a8cb3 100644 --- a/src/core/thread/router_table.hpp +++ b/src/core/thread/router_table.hpp @@ -124,7 +124,15 @@ public: * @returns A pointer to the Leader in the Thread network. * */ - Router *GetLeader(void); + Router *GetLeader(void) { return AsNonConst(AsConst(this)->GetLeader()); } + + /** + * This method returns the leader in the Thread network. + * + * @returns A pointer to the Leader in the Thread network. + * + */ + const Router *GetLeader(void) const; /** * This method returns the leader's age in seconds, i.e., seconds since the last Router ID Sequence update. @@ -164,6 +172,14 @@ public: */ uint8_t GetPathCost(uint16_t aDestRloc16) const; + /** + * This method returns the mesh path cost to leader. + * + * @returns The path cost to leader. + * + */ + uint8_t GetPathCostToLeader(void) const; + /** * This method determines the next hop towards an RLOC16 destination. * @@ -325,6 +341,17 @@ public: */ void UpdateRoutes(const Mle::RouteTlv &aRouteTlv, uint8_t aRouterId); + /** + * This method updates the routes on an FED based on a received `RouteTlv` from the parent. + * + * This method MUST be called when device is an FED child and @p aRouteTlv is received from its current parent. + * + * @param[in] aRouteTlv The received `RouteTlv` from parent. + * @param[in] aParentId The Router ID of parent. + * + */ + void UpdateRoutesOnFed(const Mle::RouteTlv &aRouteTlv, uint8_t aParentId); + /** * This method gets the allocated Router ID set. * diff --git a/src/core/thread/topology.cpp b/src/core/thread/topology.cpp index 3869a7158..15023af5f 100644 --- a/src/core/thread/topology.cpp +++ b/src/core/thread/topology.cpp @@ -567,4 +567,25 @@ void Parent::Clear(void) Init(instance); } +bool Router::SetNextHopAndCost(uint8_t aNextHop, uint8_t aCost) +{ + bool changed = false; + + if (mNextHop != aNextHop) + { + mNextHop = aNextHop; + changed = true; + } + + if (mCost != aCost) + { + mCost = aCost; + changed = true; + } + + return changed; +} + +bool Router::SetNextHopToInvalid(void) { return SetNextHopAndCost(Mle::kInvalidRouterId, 0); } + } // namespace ot diff --git a/src/core/thread/topology.hpp b/src/core/thread/topology.hpp index 4cd5cca44..52be960a3 100644 --- a/src/core/thread/topology.hpp +++ b/src/core/thread/topology.hpp @@ -1401,14 +1401,6 @@ public: */ uint8_t GetNextHop(void) const { return mNextHop; } - /** - * This method sets the router ID of the next hop to this router. - * - * @param[in] aRouterId The router ID of the next hop to this router. - * - */ - void SetNextHop(uint8_t aRouterId) { mNextHop = aRouterId; } - /** * This method gets the link quality out value for this router. * @@ -1442,12 +1434,25 @@ public: uint8_t GetCost(void) const { return mCost; } /** - * This method sets the router cost to this router. + * This method sets the next hop and cost to this router. * - * @param[in] aCost The router cost to this router. + * @param[in] aNextHop The Router ID of the next hop to this router. + * @param[in] aCost The cost to this router. + * + * @retval TRUE If there was a change, i.e., @p aNextHop or @p aCost were different from their previous values. + * @retval FALSE If no change to next hop and cost values (new values are the same as before). * */ - void SetCost(uint8_t aCost) { mCost = aCost; } + bool SetNextHopAndCost(uint8_t aNextHop, uint8_t aCost); + + /** + * This method sets the next hop to this router as invalid and clears the cost. + * + * @retval TRUE If there was a change (next hop was valid before). + * @retval FALSE No change to next hop (next hop was invalid before). + * + */ + bool SetNextHopToInvalid(void); private: uint8_t mNextHop; ///< The next hop towards this router diff --git a/tests/toranj/cli/test-008-multicast-traffic.py b/tests/toranj/cli/test-008-multicast-traffic.py index 01c8cd2d6..7623a9c5a 100755 --- a/tests/toranj/cli/test-008-multicast-traffic.py +++ b/tests/toranj/cli/test-008-multicast-traffic.py @@ -97,6 +97,7 @@ r2.join(r1) r3.join(r2) r4.join(r3) fed.join(r1, cli.JOIN_TYPE_REED) + sed.join(r3, cli.JOIN_TYPE_SLEEPY_END_DEVICE) sed.set_pollperiod(600)