From 611c62126a39eaee31f6afe4d4d32675b6d71660 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 4 May 2026 06:46:25 -0700 Subject: [PATCH] [mle] use compact Route TLV in Link Accept to child neighbors (#13012) This commit enhances MLE where a full Route TLV could be appended to a Link Accept message sent to a child neighbor, potentially leading to a message requiring lowpan fragmentation. Previously, `Mle::SendLinkAccept()` relied on a `Router` pointer to determine whether to use a full or compact Route TLV. When the Link Request originated from a child, this pointer was null, causing a full Route TLV to be used. The changes in this commit include: - Updating the `LinkAcceptInfo` struct to track the RLOC16 of the Link Request sender. - Updating `Mle::TxMessage::AppendRouteTlv()` and adding `AppendCompactRouteTlv()` to replace the previous single method that took a `Neighbor` pointer. This makes the intent clearer and supports both router and child neighbors. - Updating `RouterTable::FillRouteTlv()` to take an RLOC16 instead of a `Neighbor` pointer. It uses `Mle::RouterIdFromRloc16()` to ensure that if the destination is a child, its parent's Router ID is included in the compact Route TLV. - Includes new Nexus test `test_compact_route_tlv` to validate the use of compact Route TLV in Link Accept. --- src/core/thread/mle.cpp | 8 +- src/core/thread/mle.hpp | 5 +- src/core/thread/mle_ftd.cpp | 15 +-- src/core/thread/router_table.cpp | 19 ++-- src/core/thread/router_table.hpp | 11 +- tests/nexus/CMakeLists.txt | 1 + tests/nexus/test_compact_route_tlv.cpp | 136 +++++++++++++++++++++++++ 7 files changed, 171 insertions(+), 24 deletions(-) create mode 100644 tests/nexus/test_compact_route_tlv.cpp diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index bb1d94fd0..c1a43be07 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -3957,12 +3957,16 @@ exit: return error; } -Error Mle::TxMessage::AppendRouteTlv(Neighbor *aNeighbor) +Error Mle::TxMessage::AppendRouteTlv(void) { return AppendFullOrCompactRouteTlv(kInvalidRloc16); } + +Error Mle::TxMessage::AppendCompactRouteTlv(uint16_t aDestRloc16) { return AppendFullOrCompactRouteTlv(aDestRloc16); } + +Error Mle::TxMessage::AppendFullOrCompactRouteTlv(uint16_t aDestRloc16) { RouteTlv tlv; tlv.Init(); - Get().FillRouteTlv(tlv, aNeighbor); + Get().FillRouteTlv(tlv, aDestRloc16); return tlv.AppendTo(*this); } diff --git a/src/core/thread/mle.hpp b/src/core/thread/mle.hpp index 0bf724eeb..765d2f8fc 100644 --- a/src/core/thread/mle.hpp +++ b/src/core/thread/mle.hpp @@ -1603,7 +1603,8 @@ private: Error AppendCslClockAccuracyTlv(void); #endif #if OPENTHREAD_FTD - Error AppendRouteTlv(Neighbor *aNeighbor = nullptr); + Error AppendRouteTlv(void); + Error AppendCompactRouteTlv(uint16_t aDestRloc16); Error AppendActiveDatasetTlv(void); Error AppendPendingDatasetTlv(void); Error AppendConnectivityTlv(void); @@ -1625,6 +1626,7 @@ private: private: Error AppendAddressRegistrationEntry(const Ip6::Address &aAddress); Error AppendDatasetTlv(MeshCoP::Dataset::Type aDatasetType); + Error AppendFullOrCompactRouteTlv(uint16_t aDestRloc16); }; //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -1711,6 +1713,7 @@ private: Mac::ExtAddress mExtAddress; // The neighbor/router extended address. TlvList mRequestedTlvList; // The requested TLVs in Link Request. RxChallenge mRxChallenge; // The challenge in Link Request. + uint16_t mRloc16; // The neighbor/router RLOC16. uint8_t mLinkMargin; // Link margin of the received Link Request. }; #endif diff --git a/src/core/thread/mle_ftd.cpp b/src/core/thread/mle_ftd.cpp index a9ec6efe1..e3c5a3fe3 100644 --- a/src/core/thread/mle_ftd.cpp +++ b/src/core/thread/mle_ftd.cpp @@ -689,7 +689,6 @@ void Mle::HandleLinkRequest(RxInfo &aRxInfo) Neighbor *neighbor = nullptr; uint16_t version; LeaderData leaderData; - uint16_t sourceAddress; LinkAcceptInfo info; Log(kMessageReceive, kTypeLinkRequest, aRxInfo.mMessageInfo.GetPeerAddr()); @@ -717,12 +716,12 @@ void Mle::HandleLinkRequest(RxInfo &aRxInfo) info.mLinkMargin = Get().ComputeLinkMargin(aRxInfo.mMessage.GetAverageRss()); - switch (Tlv::Find(aRxInfo.mMessage, sourceAddress)) + switch (Tlv::Find(aRxInfo.mMessage, info.mRloc16)) { case kErrorNone: - if (IsRouterRloc16(sourceAddress)) + if (IsRouterRloc16(info.mRloc16)) { - Router *router = mRouterTable.FindRouterByRloc16(sourceAddress); + Router *router = mRouterTable.FindRouterByRloc16(info.mRloc16); VerifyOrExit(router != nullptr, error = kErrorParse); @@ -745,8 +744,9 @@ void Mle::HandleLinkRequest(RxInfo &aRxInfo) case kErrorNotFound: // A missing source address indicates that the router was // recently reset. - VerifyOrExit(aRxInfo.IsNeighborStateValid() && IsRouterRloc16(aRxInfo.mNeighbor->GetRloc16()), - error = kErrorDrop); + VerifyOrExit(aRxInfo.IsNeighborStateValid(), error = kErrorDrop); + info.mRloc16 = aRxInfo.mNeighbor->GetRloc16(); + VerifyOrExit(IsRouterRloc16(info.mRloc16), error = kErrorDrop); neighbor = aRxInfo.mNeighbor; break; @@ -839,7 +839,7 @@ Error Mle::SendLinkAccept(const LinkAcceptInfo &aInfo) switch (tlvType) { case Tlv::kRoute: - SuccessOrExit(error = message->AppendRouteTlv(router)); + SuccessOrExit(error = message->AppendCompactRouteTlv(aInfo.mRloc16)); break; case Tlv::kAddress16: @@ -1083,6 +1083,7 @@ void Mle::HandleLinkAcceptVariant(RxInfo &aRxInfo, MessageType aMessageType) } info.mExtAddress = router->GetExtAddress(); + info.mRloc16 = router->GetRloc16(); info.mLinkMargin = Get().ComputeLinkMargin(aRxInfo.mMessage.GetAverageRss()); SuccessOrExit(error = SendLinkAccept(info)); diff --git a/src/core/thread/router_table.cpp b/src/core/thread/router_table.cpp index 30f454f03..5108602f0 100644 --- a/src/core/thread/router_table.cpp +++ b/src/core/thread/router_table.cpp @@ -763,16 +763,16 @@ void RouterTable::GetRouterIdMask(Mle::RouterIdMask &aRouterIdMask) const } } -void RouterTable::FillRouteTlv(Mle::RouteTlv &aRouteTlv, const Neighbor *aNeighbor) const +void RouterTable::FillRouteTlv(Mle::RouteTlv &aRouteTlv, uint16_t aDestRloc16) const { uint8_t routerIndex; GetRouterIdMask(aRouteTlv.GetRouterIdMask()); - if ((aNeighbor != nullptr) && Mle::IsRouterRloc16(aNeighbor->GetRloc16())) + if (aDestRloc16 != Mle::kInvalidRloc16) { - // Sending a Link Accept message that may require truncation - // of Route64 TLV. + // If a valid destination RLOC16 is provided, we use a compact + // format for the Route TLV (used for Link Accept messages). uint8_t routerCount = mRouters.GetLength(); @@ -785,12 +785,13 @@ void RouterTable::FillRouteTlv(Mle::RouteTlv &aRouteTlv, const Neighbor *aNeighb break; } - if (Get().MatchesRouterId(routerId) || (routerId == aNeighbor->GetRouterId()) || + // The Route TLV must include entries for this device, the + // leader, and the destination router (the neighbor itself or + // its parent if it is a child). + + if (Get().MatchesRouterId(routerId) || (routerId == Mle::RouterIdFromRloc16(aDestRloc16)) || (routerId == Get().GetLeaderId())) { - // Route64 TLV must contain this device and the - // neighboring router to ensure that at least this - // link can be established. continue; } @@ -802,7 +803,7 @@ void RouterTable::FillRouteTlv(Mle::RouteTlv &aRouteTlv, const Neighbor *aNeighb } // Ensure that the neighbor will process the current - // Route64 TLV in a subsequent message exchange + // Route TLV in a subsequent message exchange aRouteTlv.GetRouterIdMask().SetSequence(GetRouterIdSequence() - kLinkAcceptSequenceRollback); } } diff --git a/src/core/thread/router_table.hpp b/src/core/thread/router_table.hpp index 33711cdfe..7ffda35fb 100644 --- a/src/core/thread/router_table.hpp +++ b/src/core/thread/router_table.hpp @@ -373,14 +373,15 @@ public: /** * Fills a Route TLV. * - * When @p aNeighbor is not `nullptr`, we limit the number of router entries to `kMaxRoutersInRouteTlvForLinkAccept` - * when populating `aRouteTlv`, so that the TLV can be appended in a Link Accept message. In this case, we ensure - * to include router entries associated with @p aNeighbor, leader, and this device itself. + * If @p aDestRloc16 is not `Mle::kInvalidRloc16`, a compact format is used for the Route TLV by limiting the + * number of router entries to `kMaxRoutersInRouteTlvForLinkAccept`. This is used for Link Accept messages. In this + * case, we ensure that entries for this device, the leader, and the @p aDestRloc16 (itself or its parent if it is + * child) are included. * * @param[out] aRouteTlv A Route TLV to be filled. - * @param[in] aNeighbor A pointer to the receiver (in case TLV is for a Link Accept message). + * @param[in] aDestRloc16 The destination RLOC16 (used for compact format when not `Mle::kInvalidRloc16)` */ - void FillRouteTlv(Mle::RouteTlv &aRouteTlv, const Neighbor *aNeighbor = nullptr) const; + void FillRouteTlv(Mle::RouteTlv &aRouteTlv, uint16_t aDestRloc16 = Mle::kInvalidRloc16) const; /** * Updates the router table and must be called with a one second period. diff --git a/tests/nexus/CMakeLists.txt b/tests/nexus/CMakeLists.txt index 04e373d88..bdf169d4c 100644 --- a/tests/nexus/CMakeLists.txt +++ b/tests/nexus/CMakeLists.txt @@ -394,6 +394,7 @@ ot_nexus_test(child_supervision "core;nexus") ot_nexus_test(coap_block "core;nexus") ot_nexus_test(coap_observe "core;nexus") ot_nexus_test(coaps "core;nexus") +ot_nexus_test(compact_route_tlv "core;nexus") ot_nexus_test(dataset_updater "core;nexus") ot_nexus_test(discover_scan "core;nexus") ot_nexus_test(dnssd "core;nexus") diff --git a/tests/nexus/test_compact_route_tlv.cpp b/tests/nexus/test_compact_route_tlv.cpp new file mode 100644 index 000000000..d8d22b720 --- /dev/null +++ b/tests/nexus/test_compact_route_tlv.cpp @@ -0,0 +1,136 @@ +/* + * Copyright (c) 2026, 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 +#include +#include + +#include "platform/nexus_core.hpp" +#include "platform/nexus_node.hpp" + +namespace ot { +namespace Nexus { + +void TestCompactRouteTlv(void) +{ + static constexpr uint16_t kNumRouters = 31; + + Core nexus; + Node &leader = nexus.CreateNode(); + Node *routers[kNumRouters]; + + Log("---------------------------------------------------------------------------------------"); + Log("TestCompactRouteTlv"); + + // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + Log("Form topology - leader + %u routers", kNumRouters); + + leader.Get().SetRouterUpgradeThreshold(32); + leader.Get().SetRouterDowngradeThreshold(33); + + leader.Form(); + nexus.AdvanceTime(100 * Time::kOneSecondInMsec); + VerifyOrQuit(leader.Get().IsLeader()); + + for (uint16_t i = 0; i < kNumRouters; i++) + { + Node &router = nexus.CreateNode(); + + routers[i] = &router; + + router.Get().SetRouterUpgradeThreshold(32); + router.Get().SetRouterDowngradeThreshold(33); + router.Get().SetRouterSelectionJitter(20); + + router.Join(leader, Node::kAsFtd); + nexus.AdvanceTime(30 * Time::kOneSecondInMsec); + + VerifyOrQuit(router.Get().IsRouter()); + + Log("Router %-2u -> rloc16: 0x%04x", i, router.Get().GetRloc16()); + } + + // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + Log("Check Active Router Count on all routers"); + + nexus.AdvanceTime(300 * Time::kOneSecondInMsec); + + VerifyOrQuit(leader.Get().GetActiveRouterCount() == kNumRouters + 1); + + for (const Node *router : routers) + { + VerifyOrQuit(router->Get().GetActiveRouterCount() == kNumRouters + 1); + } + + // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + Log("Reset `routers[0]` and validate that it is restored quickly"); + + routers[0]->Reset(); + + routers[0]->Get().SetRouterUpgradeThreshold(32); + routers[0]->Get().SetRouterDowngradeThreshold(33); + routers[0]->Get().SetRouterSelectionJitter(20); + + routers[0]->Get().Up(); + SuccessOrQuit(routers[0]->Get().Start()); + + for (uint16_t i = 0; i < 5000; i++) + { + nexus.AdvanceTime(1); + + if (routers[0]->Get().IsRouter()) + { + Log("Node `routers[0]` restored it role after reset"); + Log("Validate that it got a compact Route TLV and therefore does not yet see all routers"); + VerifyOrQuit(routers[0]->Get().GetActiveRouterCount() < kNumRouters + 1); + break; + } + + VerifyOrQuit(routers[0]->Get().IsDetached()); + } + + VerifyOrQuit(routers[0]->Get().IsRouter()); + + Log("Validate that `routes[0]` discovered all routers"); + + nexus.AdvanceTime(100 * Time::kOneSecondInMsec); + + VerifyOrQuit(routers[0]->Get().IsRouter()); + VerifyOrQuit(routers[0]->Get().GetActiveRouterCount() == kNumRouters + 1); +} + +} // namespace Nexus +} // namespace ot + +int main(void) +{ + ot::Nexus::TestCompactRouteTlv(); + printf("All tests passed\n"); + + return 0; +}