From 2a56b165c70bbc389778c4bb2fe887ec0c7d2c5c Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 11 May 2026 09:06:11 -0700 Subject: [PATCH] [mlr] extract `AddressArray` and add `FindIn()` to `Ip6AddressesTlv` (#13088) This commit moves the `AddressArray` class out of the `Mlr::Manager` and into a dedicated `mlr_types.hpp` file as `Mlr::AddressArray`. This decouples the type from the manager, making it available for broader use across the module. Additionally, the logic for parsing the `Ip6AddressesTlv` is extracted from `Mlr::Manager::ParseResponse()` into a new `FindIn()` method on the TLV class itself. This centralizes the TLV parsing logic within the TLV class, which is more idiomatic. The `FindIn()` method also provides a safety guarantee by clearing the output `AddressArray` if parsing fails. The build system configurations (`BUILD.gn` and `CMakeLists.txt`) are updated to include the newly added `mlr_types.cpp` file. Doxygen documentation is also provided for the new types and methods. --- src/core/BUILD.gn | 1 + src/core/CMakeLists.txt | 1 + src/core/thread/mlr_manager.cpp | 34 +++++++-------------- src/core/thread/mlr_manager.hpp | 6 ---- src/core/thread/mlr_types.cpp | 52 +++++++++++++++++++++++++++++++++ src/core/thread/mlr_types.hpp | 21 +++++++++++++ src/core/thread/thread_tlvs.cpp | 25 +++++++++++++++- src/core/thread/thread_tlvs.hpp | 14 +++++++++ 8 files changed, 123 insertions(+), 31 deletions(-) create mode 100644 src/core/thread/mlr_types.cpp diff --git a/src/core/BUILD.gn b/src/core/BUILD.gn index 352580603..b6a5fdf11 100644 --- a/src/core/BUILD.gn +++ b/src/core/BUILD.gn @@ -729,6 +729,7 @@ openthread_core_files = [ "thread/mle_types.hpp", "thread/mlr_manager.cpp", "thread/mlr_manager.hpp", + "thread/mlr_types.cpp", "thread/mlr_types.hpp", "thread/neighbor.cpp", "thread/neighbor.hpp", diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt index f4f99ed67..7597cab1b 100644 --- a/src/core/CMakeLists.txt +++ b/src/core/CMakeLists.txt @@ -255,6 +255,7 @@ set(COMMON_SOURCES thread/mle_tlvs.cpp thread/mle_types.cpp thread/mlr_manager.cpp + thread/mlr_types.cpp thread/neighbor.cpp thread/neighbor_table.cpp thread/network_data.cpp diff --git a/src/core/thread/mlr_manager.cpp b/src/core/thread/mlr_manager.cpp index a97130f87..f9ef39b49 100644 --- a/src/core/thread/mlr_manager.cpp +++ b/src/core/thread/mlr_manager.cpp @@ -451,8 +451,7 @@ void Manager::HandleResponse(Coap::Msg *aMsg, Error aResult) Error Manager::ParseResponse(Error aResult, Coap::Msg *aMsg, uint8_t &aStatus, AddressArray &aFailedAddresses) { - Error error = aResult; - OffsetRange offsetRange; + Error error = aResult; aStatus = kStatusGeneralFailure; aFailedAddresses.Clear(); @@ -463,17 +462,16 @@ Error Manager::ParseResponse(Error aResult, Coap::Msg *aMsg, uint8_t &aStatus, A SuccessOrExit(error = Tlv::Find(aMsg->mMessage, aStatus)); - if (Tlv::FindTlvValueOffsetRange(aMsg->mMessage, Ip6AddressesTlv::kType, offsetRange) == kErrorNone) + switch (error = Ip6AddressesTlv::FindIn(aMsg->mMessage, aFailedAddresses)) { - while (!offsetRange.IsEmpty()) - { - Ip6::Address address; - - SuccessOrExit(error = aMsg->mMessage.Read(offsetRange, address)); - offsetRange.AdvanceOffset(sizeof(Ip6::Address)); - - SuccessOrExit(error = aFailedAddresses.AddUnique(address)); - } + case kErrorNone: + break; + case kErrorNotFound: + error = kErrorNone; + aFailedAddresses.Clear(); + break; + default: + ExitNow(); } if (aStatus == kStatusSuccess) @@ -726,18 +724,6 @@ void Manager::LogMulticastAddresses(void) #endif // OT_SHOULD_LOG_AT(OT_LOG_LEVEL_DEBG) } -Error Manager::AddressArray::AddUnique(const Ip6::Address &aAddress) -{ - Error error = kErrorNone; - - if (!Contains(aAddress)) - { - error = PushBack(aAddress); - } - - return error; -} - } // namespace Mlr } // namespace ot diff --git a/src/core/thread/mlr_manager.hpp b/src/core/thread/mlr_manager.hpp index d9b0e12d6..f36dae3e8 100644 --- a/src/core/thread/mlr_manager.hpp +++ b/src/core/thread/mlr_manager.hpp @@ -152,12 +152,6 @@ private: kRenew, }; - class AddressArray : public Array - { - public: - Error AddUnique(const Ip6::Address &aAddress); - }; - void HandleNotifierEvents(Events aEvents); bool ShouldRegister(void) const; void Send(void); diff --git a/src/core/thread/mlr_types.cpp b/src/core/thread/mlr_types.cpp new file mode 100644 index 000000000..8c6dc2461 --- /dev/null +++ b/src/core/thread/mlr_types.cpp @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2020-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. + */ + +/** + * @file + * This file implements MLR types. + */ + +#include "mlr_types.hpp" + +namespace ot { +namespace Mlr { + +Error AddressArray::AddUnique(const Ip6::Address &aAddress) +{ + Error error = kErrorNone; + + if (!Contains(aAddress)) + { + error = PushBack(aAddress); + } + + return error; +} + +} // namespace Mlr +} // namespace ot diff --git a/src/core/thread/mlr_types.hpp b/src/core/thread/mlr_types.hpp index 9ec5156c5..06458ea17 100644 --- a/src/core/thread/mlr_types.hpp +++ b/src/core/thread/mlr_types.hpp @@ -36,6 +36,10 @@ #include "openthread-core-config.h" +#include "common/array.hpp" +#include "common/error.hpp" +#include "net/ip6_address.hpp" + namespace ot { namespace Mlr { @@ -77,6 +81,23 @@ enum Status : uint8_t constexpr uint8_t kMaxStatusValue = kStatusGeneralFailure; +/** + * Represents an array of IPv6 addresses. + */ +class AddressArray : public Array +{ +public: + /** + * Adds an IPv6 address to the array if it is not already present. + * + * @param[in] aAddress The IPv6 address to add. + * + * @retval kErrorNone Successfully added the address or it was already present. + * @retval kErrorNoBufs The array is full. + */ + Error AddUnique(const Ip6::Address &aAddress); +}; + } // namespace Mlr } // namespace ot diff --git a/src/core/thread/thread_tlvs.cpp b/src/core/thread/thread_tlvs.cpp index 8c72cae06..af37e0269 100644 --- a/src/core/thread/thread_tlvs.cpp +++ b/src/core/thread/thread_tlvs.cpp @@ -50,6 +50,29 @@ exit: return error; } -#endif +Error Ip6AddressesTlv::FindIn(const Message &aMessage, Mlr::AddressArray &aAddresses) +{ + Error error; + OffsetRange offsetRange; + + aAddresses.Clear(); + + SuccessOrExit(error = Tlv::FindTlvValueOffsetRange(aMessage, kType, offsetRange)); + + while (!offsetRange.IsEmpty()) + { + Ip6::Address address; + + SuccessOrExit(error = aMessage.Read(offsetRange, address)); + offsetRange.AdvanceOffset(sizeof(Ip6::Address)); + + SuccessOrExit(error = aAddresses.AddUnique(address)); + } + +exit: + return error; +} + +#endif // OPENTHREAD_CONFIG_THREAD_VERSION >= OT_THREAD_VERSION_1_2 } // namespace ot diff --git a/src/core/thread/thread_tlvs.hpp b/src/core/thread/thread_tlvs.hpp index 32209dd04..726d2ec4a 100644 --- a/src/core/thread/thread_tlvs.hpp +++ b/src/core/thread/thread_tlvs.hpp @@ -42,6 +42,7 @@ #include "meshcop/network_name.hpp" #include "net/ip6_address.hpp" #include "thread/mle_types.hpp" +#include "thread/mlr_types.hpp" namespace ot { @@ -166,6 +167,19 @@ public: */ static Error AppendTo(Message &aMessage, const Ip6::Address *aAddresses, uint16_t aNumAddresses); + /** + * Finds and parses the IPv6 Addresses TLV from a given message. + * + * @param[in] aMessage The message to parse. + * @param[out] aAddresses An `AddressArray` to output the parsed IPv6 addresses. + * + * @retval kErrorNone Successfully found and parsed the TLV. + * @retval kErrorNotFound Could not find the TLV in the message. + * @retval kErrorParse Failed to parse the TLV. + * @retval kErrorNoBufs There are more addresses in the TLV than can fit in `aAddresses`. + */ + static Error FindIn(const Message &aMessage, Mlr::AddressArray &aAddresses); + Ip6AddressesTlv(void) = delete; };