From 40ebd8d07f329ce0404b6f27390dab75a4dddaad Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 5 May 2026 07:49:35 -0700 Subject: [PATCH] [router-table] append Route TLV directly to message (#13042) This commit updates the way Route TLV is constructed and appended to messages. Previously, a `RouteTlv` object with a large fixed-size `mRouteData` array was allocated on the stack, filled by `RouterTable`, and then appended to the message. To simplify the code and improve efficiently a new helper method `RouterTable::AppendRouteTlv()` is introduced which appends the TLV content directly in the `Message`. It uses `Tlv::StartTlv()` and `Tlv::EndTlv()` to encapsulate the `RouterIdMask` and the iteratively appended route data entries. A helper method `RouteTlv::AppendRouteDataEntry()` is added which handles encoding and adding a Route Data Entry, including the the bit-packing logic(the staggered 1.5-byte packing under `OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE`). --- src/core/thread/mle.cpp | 7 +- src/core/thread/mle_tlvs.cpp | 69 +++++++++++++++++++ src/core/thread/mle_tlvs.hpp | 93 ++++++++++---------------- src/core/thread/network_diagnostic.cpp | 8 +-- src/core/thread/router_table.cpp | 65 +++++++++++------- src/core/thread/router_table.hpp | 16 +++-- 6 files changed, 160 insertions(+), 98 deletions(-) diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index 344645b68..54133ae99 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -3957,12 +3957,7 @@ Error Mle::TxMessage::AppendCompactRouteTlv(uint16_t aDestRloc16) { return Appen Error Mle::TxMessage::AppendFullOrCompactRouteTlv(uint16_t aDestRloc16) { - RouteTlv tlv; - - tlv.Init(); - Get().FillRouteTlv(tlv, aDestRloc16); - - return tlv.AppendTo(*this); + return Get().AppendRouteTlv(*this, RouteTlv::kType, aDestRloc16); } Error Mle::TxMessage::AppendActiveDatasetTlv(void) { return AppendDatasetTlv(MeshCoP::Dataset::kActive); } diff --git a/src/core/thread/mle_tlvs.cpp b/src/core/thread/mle_tlvs.cpp index 1ff588e17..79e3bba1e 100644 --- a/src/core/thread/mle_tlvs.cpp +++ b/src/core/thread/mle_tlvs.cpp @@ -65,6 +65,75 @@ exit: return isValid; } +Error RouteTlv::AppendRouteDataEntry(Message &aMessage, + LinkQuality aLqIn, + LinkQuality aLqOut, + uint8_t aRouteCost +#if OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE + , + bool aIsEven +#endif +) +{ + Error error; + EntryType entry = 0; + + if (aRouteCost >= kMaxRouteCost) + { + aRouteCost = 0; + } + + WriteBits(entry, aLqOut); + WriteBits(entry, aLqIn); + WriteBits(entry, aRouteCost); + +#if !OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE + ExitNow(error = aMessage.Append(entry)); +#else + { + // Under `OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE`, each route + // data entry uses 1.5 bytes (12 bits). Two entries are packed + // into 3 bytes. + // + // For the even (first) entry, we grow the message by 2 bytes + // and write the 12 bits into the upper 12 bits of the new + // 16-bit word at the end of the message. + // + // For the odd (second) entry, we grow the message by 1 byte. We + // then read the last 16 bits (which overlap with the last byte + // of the even entry), write the new 12-bit entry into the + // lower 12 bits, and write the 16-bit word back. This + // perfectly packs the two 12-bit entries into 3 bytes. + + uint16_t offset; + uint16_t data; + + SuccessOrExit(error = aMessage.IncreaseLength(aIsEven ? sizeof(uint16_t) : sizeof(uint8_t))); + + VerifyOrExit(aMessage.GetLength() >= sizeof(uint16_t), error = kErrorParse); + offset = aMessage.GetLength() - sizeof(uint16_t); + + if (aIsEven) + { + data = 0; + WriteBits(data, entry); + } + else + { + IgnoreError(aMessage.Read(offset, data)); + data = BigEndian::HostSwap16(data); + + WriteBits(data, entry); + } + + aMessage.Write(offset, BigEndian::HostSwap16(data)); + } +#endif // OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE + +exit: + return error; +} + //--------------------------------------------------------------------------------------------------------------------- // ConnectivityTlvValue diff --git a/src/core/thread/mle_tlvs.hpp b/src/core/thread/mle_tlvs.hpp index 9029fc90b..f33558148 100644 --- a/src/core/thread/mle_tlvs.hpp +++ b/src/core/thread/mle_tlvs.hpp @@ -302,20 +302,6 @@ public: #endif } - /** - * Sets the Route Data entry count. - * - * @param[in] aCount The number of Route Data entries in the Route TLV. - */ - void SetRouteDataEntryCount(uint8_t aCount) - { -#if !OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE - SetLength(sizeof(mRouterIdMask) + aCount); -#else - SetLength(sizeof(mRouterIdMask) + aCount + (aCount + 1) / 2); -#endif - } - /** * Returns the Route Cost value for a given Router index. * @@ -364,32 +350,42 @@ public: #endif } - /** - * Sets the Route Data (Link Quality In/Out and Route Cost) for a given Router index. - * - * @param[in] aRouterIndex The Router index. - * @param[in] aLinkQualityIn The Link Quality In value. - * @param[in] aLinkQualityOut The Link Quality Out value. - * @param[in] aRouteCost The Route Cost value. - */ - void SetRouteData(uint8_t aRouterIndex, LinkQuality aLinkQualityIn, LinkQuality aLinkQualityOut, uint8_t aRouteCost) - { #if !OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE - mRouteData[aRouterIndex] = 0; - - WriteBits(mRouteData[aRouterIndex], aLinkQualityIn); - WriteBits(mRouteData[aRouterIndex], aLinkQualityOut); - WriteBits(mRouteData[aRouterIndex], aRouteCost); + /** + * Appends a Route Data entry (Link Quality In/Out and Route Cost) to a message. + * + * @param[in] aMessage The message to append to. + * @param[in] aLqIn The Link Quality In value. + * @param[in] aLqOut The Link Quality Out value. + * @param[in] aRouteCost The Route Cost value. + * + * @retval kErrorNone Successfully appended the data. + * @retval kErrorNoBufs Insufficient available buffers to grow the message. + */ + static Error AppendRouteDataEntry(Message &aMessage, LinkQuality aLqIn, LinkQuality aLqOut, uint8_t aRouteCost); #else - uint16_t data = 0; - - WriteBits(data, aLinkQualityOut); - WriteBits(data, aLinkQualityIn); - WriteBits(data, aRouteCost); - - WriteEntry(aRouterIndex, data); + /** + * Appends a Route Data entry (Link Quality In/Out and Route Cost) to a message. + * + * Under `OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE`, each route data entry uses 1.5 bytes (12 bits). Two entries + * are packed into 3 bytes. @p aIsEven is used to indicate whether this is an even (first) or an odd (second) entry. + * + * @param[in] aMessage The message to append to. + * @param[in] aLqIn The Link Quality In value. + * @param[in] aLqOut The Link Quality Out value. + * @param[in] aRouteCost The Route Cost value. + * @param[in] aIsEven Indicates whether this is an even (first) entry. + * + * @retval kErrorNone Successfully appended the data. + * @retval kErrorNoBufs Insufficient available buffers to grow the message. + * @retval kErrorParse Message length is invalid for parsing route data. + */ + static Error AppendRouteDataEntry(Message &aMessage, + LinkQuality aLqIn, + LinkQuality aLqOut, + uint8_t aRouteCost, + bool aIsEven); #endif - } private: #if !OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE @@ -398,6 +394,8 @@ private: // | LQOut | LQIn | Route Cost | // +---+---+---+---+---+---+---+---+ + typedef uint8_t EntryType; + static constexpr uint8_t kLinkQualityOutMask = 0x03 << 6; static constexpr uint8_t kLinkQualityInMask = 0x03 << 4; static constexpr uint8_t kRouteCostMask = 0x0f << 0; @@ -409,6 +407,8 @@ private: // remaining 8 bits are for the route cost. The even and odd // entries are staggered. + typedef uint16_t EntryType; + static constexpr uint16_t kEvenEntryMask = 0xfff << 4; static constexpr uint16_t kOddEntryMask = 0xfff << 0; @@ -434,25 +434,6 @@ private: return data; } - - void WriteEntry(uint8_t aRouterIndex, uint16_t aData) - { - uint16_t offset = (aRouterIndex + aRouterIndex / 2); - uint16_t existing; - - existing = BigEndian::ReadUint16(&mRouteData[offset]); - - if (aRouterIndex & 0x1) - { - WriteBits(existing, aData); - } - else - { - WriteBits(existing, aData); - } - - BigEndian::WriteUint16(existing, &mRouteData[offset]); - } #endif // OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE RouterIdMask mRouterIdMask; diff --git a/src/core/thread/network_diagnostic.cpp b/src/core/thread/network_diagnostic.cpp index 4ef1ad7c2..23ce55020 100644 --- a/src/core/thread/network_diagnostic.cpp +++ b/src/core/thread/network_diagnostic.cpp @@ -432,14 +432,8 @@ Error Server::AppendDiagTlv(uint8_t aTlvType, Message &aMessage) } case Tlv::kRoute: - { - RouteTlv tlv; - - tlv.Init(); - Get().FillRouteTlv(tlv); - SuccessOrExit(error = tlv.AppendTo(aMessage)); + SuccessOrExit(error = Get().AppendRouteTlv(aMessage, Tlv::kRoute)); break; - } case Tlv::kEnhancedRoute: error = AppendEnhancedRoute(aMessage); diff --git a/src/core/thread/router_table.cpp b/src/core/thread/router_table.cpp index 5108602f0..9bee48c2d 100644 --- a/src/core/thread/router_table.cpp +++ b/src/core/thread/router_table.cpp @@ -763,11 +763,19 @@ void RouterTable::GetRouterIdMask(Mle::RouterIdMask &aRouterIdMask) const } } -void RouterTable::FillRouteTlv(Mle::RouteTlv &aRouteTlv, uint16_t aDestRloc16) const +Error RouterTable::AppendRouteTlv(Message &aMessage, uint8_t aTlvType, uint16_t aDestRloc16) const { - uint8_t routerIndex; + Error error; + Tlv::Bookmark tlvBookmark; + Mle::RouterIdMask routerIdMask; +#if OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE + bool isEven = true; +#endif - GetRouterIdMask(aRouteTlv.GetRouterIdMask()); + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Determine the Router ID mask to use. + + GetRouterIdMask(routerIdMask); if (aDestRloc16 != Mle::kInvalidRloc16) { @@ -795,26 +803,35 @@ void RouterTable::FillRouteTlv(Mle::RouteTlv &aRouteTlv, uint16_t aDestRloc16) c continue; } - if (aRouteTlv.GetRouterIdMask().IsAllocated(routerId)) + if (routerIdMask.IsAllocated(routerId)) { - aRouteTlv.GetRouterIdMask().Remove(routerId); + routerIdMask.Remove(routerId); routerCount--; } } // Ensure that the neighbor will process the current // Route TLV in a subsequent message exchange - aRouteTlv.GetRouterIdMask().SetSequence(GetRouterIdSequence() - kLinkAcceptSequenceRollback); + routerIdMask.SetSequence(GetRouterIdSequence() - kLinkAcceptSequenceRollback); } } - routerIndex = 0; + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Append TLV: Router ID Mask followed by Route Data Entries per + // allocated router in the mask + + SuccessOrExit(error = Tlv::StartTlv(aMessage, aTlvType, tlvBookmark)); + + SuccessOrExit(error = aMessage.Append(routerIdMask)); for (uint8_t routerId = 0; routerId <= Mle::kMaxRouterId; routerId++) { - uint16_t routerRloc16; + uint16_t routerRloc16; + LinkQuality lqIn; + LinkQuality lqOut; + uint8_t routeCost; - if (!aRouteTlv.GetRouterIdMask().IsAllocated(routerId)) + if (!routerIdMask.IsAllocated(routerId)) { continue; } @@ -823,29 +840,31 @@ void RouterTable::FillRouteTlv(Mle::RouteTlv &aRouteTlv, uint16_t aDestRloc16) c if (Get().HasRloc16(routerRloc16)) { - aRouteTlv.SetRouteData(routerIndex, kLinkQuality0, kLinkQuality0, 1); + lqIn = kLinkQuality0; + lqOut = kLinkQuality0; + routeCost = 1; } else { const Router *router = FindRouterById(routerId); - uint8_t pathCost; - OT_ASSERT(router != nullptr); - - pathCost = GetPathCost(routerRloc16); - - if (pathCost >= Mle::kMaxRouteCost) - { - pathCost = 0; - } - - aRouteTlv.SetRouteData(routerIndex, router->GetLinkQualityIn(), router->GetLinkQualityOut(), pathCost); + lqIn = router->GetLinkQualityIn(); + lqOut = router->GetLinkQualityOut(); + routeCost = GetPathCost(routerRloc16); } - routerIndex++; +#if !OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE + SuccessOrExit(error = Mle::RouteTlv::AppendRouteDataEntry(aMessage, lqIn, lqOut, routeCost)); +#else + SuccessOrExit(error = Mle::RouteTlv::AppendRouteDataEntry(aMessage, lqIn, lqOut, routeCost, isEven)); + isEven = !isEven; +#endif } - aRouteTlv.SetRouteDataEntryCount(routerIndex); + error = Tlv::EndTlv(aMessage, tlvBookmark); + +exit: + return error; } void RouterTable::HandleTimeTick(void) diff --git a/src/core/thread/router_table.hpp b/src/core/thread/router_table.hpp index 7ffda35fb..1bee0dfb9 100644 --- a/src/core/thread/router_table.hpp +++ b/src/core/thread/router_table.hpp @@ -371,17 +371,21 @@ public: void GetRouterIdMask(Mle::RouterIdMask &aRouterIdMask) const; /** - * Fills a Route TLV. + * Appends a Route TLV to a given message. * * 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. + * case, we ensure that entries for this device, the leader, and the destination router (itself or its parent if it + * is a child) are always included. * - * @param[out] aRouteTlv A Route TLV to be filled. - * @param[in] aDestRloc16 The destination RLOC16 (used for compact format when not `Mle::kInvalidRloc16)` + * @param[in,out] aMessage The message to append the Route TLV to. + * @param[in] aTlvType The TLV type to use (e.g., `Tlv::kRoute`). + * @param[in] aDestRloc16 The destination RLOC16. Used to determine whether to use the compact format. + * + * @retval kErrorNone Successfully appended the Route TLV. + * @retval kErrorNoBufs Insufficient available buffers to append the TLV. */ - void FillRouteTlv(Mle::RouteTlv &aRouteTlv, uint16_t aDestRloc16 = Mle::kInvalidRloc16) const; + Error AppendRouteTlv(Message &aMessage, uint8_t aTlvType, uint16_t aDestRloc16 = Mle::kInvalidRloc16) const; /** * Updates the router table and must be called with a one second period.