From 0a16ece8d225dbc623f7fbfcbfff3989935193aa Mon Sep 17 00:00:00 2001 From: Li Cao Date: Tue, 7 May 2024 02:31:31 +0800 Subject: [PATCH 01/13] [style] add comment to indicate the which conditional endif is associated with (#10197) --- src/lib/spinel/radio_spinel.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/spinel/radio_spinel.hpp b/src/lib/spinel/radio_spinel.hpp index 2096b720d4e..8fdca562ef3 100644 --- a/src/lib/spinel/radio_spinel.hpp +++ b/src/lib/spinel/radio_spinel.hpp @@ -1066,7 +1066,7 @@ class RadioSpinel : private Logger * */ void SetVendorRestorePropertiesCallback(otRadioSpinelVendorRestorePropertiesCallback aCallback, void *aContext); -#endif +#endif // OPENTHREAD_SPINEL_CONFIG_VENDOR_HOOK_ENABLE private: enum From 35a777a2952db5bc7600dcd39ce09d7ffbfce371 Mon Sep 17 00:00:00 2001 From: Yuwen Lan Date: Tue, 7 May 2024 02:44:39 +0800 Subject: [PATCH 02/13] [cli] fix `ipaddr` and `ipmaddr` commands in debug cli (#10200) This commit corrects "ipaddr" and "ipmaddr" commands used in debug cli --- src/cli/cli.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cli/cli.cpp b/src/cli/cli.cpp index e10198a46aa..d70978872bf 100644 --- a/src/cli/cli.cpp +++ b/src/cli/cli.cpp @@ -7043,8 +7043,8 @@ template <> otError Interpreter::Process(Arg aArgs[]) "state", "rloc16", "extaddr", - "ipaddrs", - "ipmaddrs", + "ipaddr", + "ipmaddr", "channel", "panid", "extpanid", From 973e657c6b35e71e8bd437e4b43ebb77753bf025 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 May 2024 11:45:19 -0700 Subject: [PATCH 03/13] github-actions: bump codecov/codecov-action from 4.0.2 to 4.3.1 (#10195) Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 4.0.2 to 4.3.1. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/codecov/codecov-action/compare/0cfda1dd0a4ad9efc75517f399d859cd1ea4ced1...5ecb98a3c6b747ed38dc09f787459979aebb39be) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/otbr.yml | 2 +- .github/workflows/posix.yml | 2 +- .github/workflows/simulation-1.1.yml | 2 +- .github/workflows/simulation-1.2.yml | 2 +- .github/workflows/toranj.yml | 2 +- .github/workflows/unit.yml | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/otbr.yml b/.github/workflows/otbr.yml index ac04744358c..2730614f713 100644 --- a/.github/workflows/otbr.yml +++ b/.github/workflows/otbr.yml @@ -255,7 +255,7 @@ jobs: script/test combine_coverage - name: Upload Coverage continue-on-error: true - uses: codecov/codecov-action@0cfda1dd0a4ad9efc75517f399d859cd1ea4ced1 # v4.0.2 + uses: codecov/codecov-action@5ecb98a3c6b747ed38dc09f787459979aebb39be # v4.3.1 env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} with: diff --git a/.github/workflows/posix.yml b/.github/workflows/posix.yml index fd8e7d89900..c06ed69abd5 100644 --- a/.github/workflows/posix.yml +++ b/.github/workflows/posix.yml @@ -315,7 +315,7 @@ jobs: run: | script/test combine_coverage - name: Upload Coverage - uses: codecov/codecov-action@0cfda1dd0a4ad9efc75517f399d859cd1ea4ced1 # v4.0.2 + uses: codecov/codecov-action@5ecb98a3c6b747ed38dc09f787459979aebb39be # v4.3.1 env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} with: diff --git a/.github/workflows/simulation-1.1.yml b/.github/workflows/simulation-1.1.yml index ee08f39547f..c2042a2eaf2 100644 --- a/.github/workflows/simulation-1.1.yml +++ b/.github/workflows/simulation-1.1.yml @@ -395,7 +395,7 @@ jobs: run: | script/test combine_coverage - name: Upload Coverage - uses: codecov/codecov-action@0cfda1dd0a4ad9efc75517f399d859cd1ea4ced1 # v4.0.2 + uses: codecov/codecov-action@5ecb98a3c6b747ed38dc09f787459979aebb39be # v4.3.1 env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} with: diff --git a/.github/workflows/simulation-1.2.yml b/.github/workflows/simulation-1.2.yml index fbd70f00322..2e037451aaa 100644 --- a/.github/workflows/simulation-1.2.yml +++ b/.github/workflows/simulation-1.2.yml @@ -424,7 +424,7 @@ jobs: run: | script/test combine_coverage - name: Upload Coverage - uses: codecov/codecov-action@0cfda1dd0a4ad9efc75517f399d859cd1ea4ced1 # v4.0.2 + uses: codecov/codecov-action@5ecb98a3c6b747ed38dc09f787459979aebb39be # v4.3.1 env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} with: diff --git a/.github/workflows/toranj.yml b/.github/workflows/toranj.yml index 6b79b13c523..1b6fd720427 100644 --- a/.github/workflows/toranj.yml +++ b/.github/workflows/toranj.yml @@ -206,7 +206,7 @@ jobs: run: | script/test combine_coverage - name: Upload Coverage - uses: codecov/codecov-action@0cfda1dd0a4ad9efc75517f399d859cd1ea4ced1 # v4.0.2 + uses: codecov/codecov-action@5ecb98a3c6b747ed38dc09f787459979aebb39be # v4.3.1 env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} with: diff --git a/.github/workflows/unit.yml b/.github/workflows/unit.yml index bf685cac747..4185237c803 100644 --- a/.github/workflows/unit.yml +++ b/.github/workflows/unit.yml @@ -123,7 +123,7 @@ jobs: run: | script/test combine_coverage - name: Upload Coverage - uses: codecov/codecov-action@0cfda1dd0a4ad9efc75517f399d859cd1ea4ced1 # v4.0.2 + uses: codecov/codecov-action@5ecb98a3c6b747ed38dc09f787459979aebb39be # v4.3.1 env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} with: From 42ccf281fbca6bd7ca5b531b9e4debc760f866f2 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 6 May 2024 12:18:06 -0700 Subject: [PATCH 04/13] [routing-manager] ensure correct handling of shorter PD prefix (#10145) This commit corrects the order of calls to prefix methods `Tidy()` and `SetLength(kOmrPrefixLength)`. This was unintentionally reversed in PR #10000. PD prefixes can be shorter than `kOmrPrefixLength` (as allowed by `IsValidPdPrefix()`). To use them as OMR prefixes, `Tidy()` is called first to clear any extra bits before potentially extending the prefix to `kOmrPrefixLength`. --- src/core/border_router/routing_manager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index c64e289e1b7..8bb814f2b5e 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -3775,8 +3775,8 @@ bool RoutingManager::PdPrefixManager::ProcessPrefixEntry(DiscoveredPrefixTable:: ExitNow(); } - aEntry.mPrefix.SetLength(kOmrPrefixLength); aEntry.mPrefix.Tidy(); + aEntry.mPrefix.SetLength(kOmrPrefixLength); // Check if there is an update to the current prefix. The valid or // preferred lifetime may have changed. From 6f0b7631e9394dc4274038ca4e1821fb7b81e80f Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 7 May 2024 12:24:37 -0700 Subject: [PATCH 05/13] [routing-manager] enhance and streamline `PdPrefixManager` (#10146) This commit updates the `PdPrefixManager` class for improved readability and efficiency: - Relocated all Prefix PD methods in the header file for logical grouping within the same `#if` block. - Reordered method definitions for better organization (e.g., moved `SetEnabled()` closer to the constructor, and logging-related methods to the end). - Renamed variables and types for conciseness (e.g., `Dhcp6PdState` to `State`). - Simplified prefix processing logic, regardless of whether it's from RA or directly set. - Moved additional functions into `PdPrefixManager::Process()`, including checking `mEnabled`, logging failures, and updating counters. - Optimized `Process()` to update the timer only when the prefix changes. - Introduced a new nested `PrefixEntry` class with helper methods like `IsFavoredOver()` and `IsEmpty()`, simplifying the code and improving readability. - The `IsFavoredOver()` method determines if one PD prefix is favored over another. --- src/core/border_router/routing_manager.cpp | 212 ++++++++++++--------- src/core/border_router/routing_manager.hpp | 172 +++++++---------- 2 files changed, 191 insertions(+), 193 deletions(-) diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index 8bb814f2b5e..ebd5d019e3c 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -3537,21 +3537,6 @@ void RoutingManager::RsSender::HandleTimer(void) #if OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE -const char *RoutingManager::PdPrefixManager::StateToString(Dhcp6PdState aState) -{ - static const char *const kStateStrings[] = { - "Disabled", // (0) kDisabled - "Stopped", // (1) kStopped - "Running", // (2) kRunning - }; - - static_assert(0 == kDhcp6PdStateDisabled, "kDhcp6PdStateDisabled value is incorrect"); - static_assert(1 == kDhcp6PdStateStopped, "kDhcp6PdStateStopped value is incorrect"); - static_assert(2 == kDhcp6PdStateRunning, "kDhcp6PdStateRunning value is incorrect"); - - return kStateStrings[aState]; -} - RoutingManager::PdPrefixManager::PdPrefixManager(Instance &aInstance) : InstanceLocator(aInstance) , mEnabled(false) @@ -3561,12 +3546,23 @@ RoutingManager::PdPrefixManager::PdPrefixManager(Instance &aInstance) , mLastPlatformRaTime(0) , mTimer(aInstance) { - mPrefix.Clear(); +} + +void RoutingManager::PdPrefixManager::SetEnabled(bool aEnabled) +{ + State oldState = GetState(); + + VerifyOrExit(mEnabled != aEnabled); + mEnabled = aEnabled; + EvaluateStateChange(oldState); + +exit: + return; } void RoutingManager::PdPrefixManager::StartStop(bool aStart) { - Dhcp6PdState oldState = GetState(); + State oldState = GetState(); VerifyOrExit(aStart != mIsRunning); mIsRunning = aStart; @@ -3576,9 +3572,9 @@ void RoutingManager::PdPrefixManager::StartStop(bool aStart) return; } -RoutingManager::Dhcp6PdState RoutingManager::PdPrefixManager::GetState(void) const +RoutingManager::PdPrefixManager::State RoutingManager::PdPrefixManager::GetState(void) const { - Dhcp6PdState state = kDhcp6PdStateDisabled; + State state = kDhcp6PdStateDisabled; if (mEnabled) { @@ -3590,7 +3586,7 @@ RoutingManager::Dhcp6PdState RoutingManager::PdPrefixManager::GetState(void) con void RoutingManager::PdPrefixManager::EvaluateStateChange(Dhcp6PdState aOldState) { - Dhcp6PdState newState = GetState(); + State newState = GetState(); VerifyOrExit(aOldState != newState); LogInfo("PdPrefixManager: %s -> %s", StateToString(aOldState), StateToString(newState)); @@ -3605,7 +3601,7 @@ void RoutingManager::PdPrefixManager::EvaluateStateChange(Dhcp6PdState aOldState break; } - mExternalCallback.InvokeIfSet(static_cast(newState)); + mStateCallback.InvokeIfSet(static_cast(newState)); exit: return; @@ -3644,7 +3640,7 @@ void RoutingManager::PdPrefixManager::WithdrawPrefix(void) { VerifyOrExit(HasPrefix()); - LogInfo("Withdrew platform provided outdated prefix: %s", mPrefix.GetPrefix().ToString().AsCString()); + LogInfo("Withdrew DHCPv6 PD prefix %s", mPrefix.GetPrefix().ToString().AsCString()); mPrefix.Clear(); mTimer.Stop(); @@ -3655,64 +3651,51 @@ void RoutingManager::PdPrefixManager::WithdrawPrefix(void) return; } -void RoutingManager::PdPrefixManager::ProcessPlatformGeneratedRa(const uint8_t *aRouterAdvert, const uint16_t aLength) +void RoutingManager::PdPrefixManager::ProcessRa(const uint8_t *aRouterAdvert, const uint16_t aLength) { - Error error = kErrorNone; - RouterAdvert::Icmp6Packet packet; - - if (mEnabled) - { - packet.Init(aRouterAdvert, aLength); - RouterAdvert::RxMessage aMessage = RouterAdvert::RxMessage(packet); + // Processes a Router Advertisement (RA) message received on the + // platform's Thread interface. This RA message, generated by + // software entities like dnsmasq, radvd, or systemd-networkd, is + // part of the DHCPv6 prefix delegation process for distributing + // prefixes to interfaces. - error = Process(&aMessage, nullptr); - mNumPlatformRaReceived++; - mLastPlatformRaTime = TimerMilli::GetNow(); - } - else - { - LogWarn("Ignore platform generated RA since PD is disabled."); - } + RouterAdvert::Icmp6Packet packet; - if (error != kErrorNone) - { - LogCrit("Failed to process platform generated ND OnMeshPrefix: %s", ErrorToString(error)); - } + packet.Init(aRouterAdvert, aLength); + Process(&packet, nullptr); } -void RoutingManager::PdPrefixManager::ProcessDhcpPdPrefix(const PrefixTableEntry &aPrefixTableEntry) +void RoutingManager::PdPrefixManager::ProcessPrefix(const PrefixTableEntry &aPrefixTableEntry) { - Error error = kErrorNone; - - VerifyOrExit(mEnabled, LogWarn("Ignore DHCPv6 delegated prefix since PD is disabled.")); + // Processes a prefix delegated by a DHCPv6 Prefix Delegation + // (PD) server. Similar to `ProcessRa()`, but sets the prefix + // directly instead of parsing an RA message. Calling this method + // again with new values can update the prefix's lifetime. - error = Process(nullptr, &aPrefixTableEntry); - -exit: - - if (error != kErrorNone) - { - LogCrit("Failed to process DHCPv6 delegated prefix: %s", ErrorToString(error)); - } + Process(nullptr, &aPrefixTableEntry); } -Error RoutingManager::PdPrefixManager::Process(const RouterAdvert::RxMessage *aMessage, - const PrefixTableEntry *aPrefixTableEntry) +void RoutingManager::PdPrefixManager::Process(const RouterAdvert::Icmp6Packet *aRaPacket, + const PrefixTableEntry *aPrefixTableEntry) { - bool currentPrefixUpdated = false; - Error error = kErrorNone; - DiscoveredPrefixTable::Entry favoredEntry; - DiscoveredPrefixTable::Entry entry; + // Processes DHCPv6 Prefix Delegation (PD) prefixes, either from + // an RA message or directly set. Requires either `aRaPacket` or + // `aPrefixTableEntry` to be non-null. - favoredEntry.Clear(); + bool currentPrefixUpdated = false; + Error error = kErrorNone; + PrefixEntry favoredEntry; + PrefixEntry entry; - // Either `aMessage` or `aPrefixTableEntry` must be non-null. + VerifyOrExit(mEnabled, error = kErrorInvalidState); - if (aMessage != nullptr) + if (aRaPacket != nullptr) { - VerifyOrExit(aMessage->IsValid(), error = kErrorParse); + RouterAdvert::RxMessage raMsg = RouterAdvert::RxMessage(*aRaPacket); - for (const Option &option : *aMessage) + VerifyOrExit(raMsg.IsValid(), error = kErrorParse); + + for (const Option &option : raMsg) { if (option.GetType() != Option::kTypePrefixInfo || !static_cast(option).IsValid()) { @@ -3723,6 +3706,9 @@ Error RoutingManager::PdPrefixManager::Process(const RouterAdvert::RxMessage *aM entry.SetFrom(static_cast(option)); currentPrefixUpdated |= ProcessPrefixEntry(entry, favoredEntry); } + + mNumPlatformRaReceived++; + mLastPlatformRaTime = TimerMilli::GetNow(); } else // aPrefixTableEntry != nullptr { @@ -3732,46 +3718,46 @@ Error RoutingManager::PdPrefixManager::Process(const RouterAdvert::RxMessage *aM if (currentPrefixUpdated && mPrefix.IsDeprecated()) { - LogInfo("PdPrefixManager: Prefix %s is deprecated", mPrefix.GetPrefix().ToString().AsCString()); + LogInfo("DHCPv6 PD prefix %s is deprecated", mPrefix.GetPrefix().ToString().AsCString()); mPrefix.Clear(); Get().ScheduleRoutingPolicyEvaluation(kImmediately); } - if (!HasPrefix() || (favoredEntry.GetPrefix().GetLength() != 0 && favoredEntry.GetPrefix() < mPrefix.GetPrefix())) + if (favoredEntry.IsFavoredOver(mPrefix)) { - mPrefix = favoredEntry; + mPrefix = favoredEntry; + currentPrefixUpdated = true; + LogInfo("DHCPv6 PD prefix set to %s", mPrefix.GetPrefix().ToString().AsCString()); Get().ScheduleRoutingPolicyEvaluation(kImmediately); } -exit: - if (HasPrefix()) + if (HasPrefix() && currentPrefixUpdated) { - // If prefix has been set from aPrefixTableEntry use only preferred lifetime to calculate stale time - if (aPrefixTableEntry) - { - mTimer.FireAt(mPrefix.GetStaleTimeFromPreferredLifetime()); - } - else - { - mTimer.FireAt(mPrefix.GetStaleTime()); - } + // If the prefix is obtained from an RA message, use + // `GetStaleTime()` to apply the minimum `RA_STABLE_TIME`. + // Otherwise, calculate it directly from the prefix's + // preferred lifetime. + + mTimer.FireAt((aPrefixTableEntry != nullptr) ? mPrefix.GetStaleTimeFromPreferredLifetime() + : mPrefix.GetStaleTime()); } else { mTimer.Stop(); } - return error; +exit: + LogWarnOnError(error, "process DHCPv6 delegated prefix"); + OT_UNUSED_VARIABLE(error); } -bool RoutingManager::PdPrefixManager::ProcessPrefixEntry(DiscoveredPrefixTable::Entry &aEntry, - DiscoveredPrefixTable::Entry &aFavoredEntry) +bool RoutingManager::PdPrefixManager::ProcessPrefixEntry(PrefixEntry &aEntry, PrefixEntry &aFavoredEntry) { bool currentPrefixUpdated = false; - if (!IsValidPdPrefix(aEntry.GetPrefix())) + if (!aEntry.IsValidPdPrefix()) { - LogWarn("PdPrefixManager: Ignore invalid prefix entry %s", aEntry.GetPrefix().ToString().AsCString()); + LogWarn("Ignore invalid DHCPv6 PD prefix %s", aEntry.GetPrefix().ToString().AsCString()); ExitNow(); } @@ -3781,7 +3767,7 @@ bool RoutingManager::PdPrefixManager::ProcessPrefixEntry(DiscoveredPrefixTable:: // Check if there is an update to the current prefix. The valid or // preferred lifetime may have changed. - if (aEntry.GetPrefix() == GetPrefix()) + if (HasPrefix() && (mPrefix.GetPrefix() == aEntry.GetPrefix())) { currentPrefixUpdated = true; mPrefix = aEntry; @@ -3794,7 +3780,7 @@ bool RoutingManager::PdPrefixManager::ProcessPrefixEntry(DiscoveredPrefixTable:: // smaller than ULA prefixes (`fc00::/7`). This rule prefers GUA // prefixes over ULA. - if (aFavoredEntry.GetPrefix().GetLength() == 0 || aEntry.GetPrefix() < aFavoredEntry.GetPrefix()) + if (aEntry.IsFavoredOver(aFavoredEntry)) { aFavoredEntry = aEntry; } @@ -3803,21 +3789,59 @@ bool RoutingManager::PdPrefixManager::ProcessPrefixEntry(DiscoveredPrefixTable:: return currentPrefixUpdated; } -void RoutingManager::PdPrefixManager::SetEnabled(bool aEnabled) +bool RoutingManager::PdPrefixManager::PrefixEntry::IsValidPdPrefix(void) const { - Dhcp6PdState oldState = GetState(); + // We should accept ULA prefix since it could be used by the internet infrastructure like NAT64. - VerifyOrExit(mEnabled != aEnabled); - mEnabled = aEnabled; - EvaluateStateChange(oldState); + return !IsEmpty() && (GetPrefix().GetLength() <= kOmrPrefixLength) && !GetPrefix().IsLinkLocal() && + !GetPrefix().IsMulticast(); +} + +bool RoutingManager::PdPrefixManager::PrefixEntry::IsFavoredOver(const PrefixEntry &aOther) const +{ + bool isFavored; + + if (IsEmpty()) + { + // Empty prefix is not favored over any (including another + // empty prefix). + isFavored = false; + ExitNow(); + } + + if (aOther.IsEmpty()) + { + // A non-empty prefix is favored over an empty one. + isFavored = true; + ExitNow(); + } + + // Numerically smaller prefix is favored. + + isFavored = GetPrefix() < aOther.GetPrefix(); exit: - return; + return isFavored; +} + +const char *RoutingManager::PdPrefixManager::StateToString(State aState) +{ + static const char *const kStateStrings[] = { + "Disabled", // (0) kDisabled + "Stopped", // (1) kStopped + "Running", // (2) kRunning + }; + + static_assert(0 == kDhcp6PdStateDisabled, "kDhcp6PdStateDisabled value is incorrect"); + static_assert(1 == kDhcp6PdStateStopped, "kDhcp6PdStateStopped value is incorrect"); + static_assert(2 == kDhcp6PdStateRunning, "kDhcp6PdStateRunning value is incorrect"); + + return kStateStrings[aState]; } extern "C" void otPlatBorderRoutingProcessIcmp6Ra(otInstance *aInstance, const uint8_t *aMessage, uint16_t aLength) { - AsCoreType(aInstance).Get().ProcessPlatformGeneratedRa(aMessage, aLength); + AsCoreType(aInstance).Get().mPdPrefixManager.ProcessRa(aMessage, aLength); } extern "C" void otPlatBorderRoutingProcessDhcp6PdPrefix(otInstance *aInstance, @@ -3825,7 +3849,7 @@ extern "C" void otPlatBorderRoutingProcessDhcp6PdPrefix(otInstance { AssertPointerIsNotNull(aPrefixInfo); - AsCoreType(aInstance).Get().ProcessDhcpPdPrefix(*aPrefixInfo); + AsCoreType(aInstance).Get().mPdPrefixManager.ProcessPrefix(*aPrefixInfo); } #endif // OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE diff --git a/src/core/border_router/routing_manager.hpp b/src/core/border_router/routing_manager.hpp index 37acf82a8f3..a38860ccb01 100644 --- a/src/core/border_router/routing_manager.hpp +++ b/src/core/border_router/routing_manager.hpp @@ -75,6 +75,10 @@ namespace ot { namespace BorderRouter { +extern "C" void otPlatBorderRoutingProcessIcmp6Ra(otInstance *aInstance, const uint8_t *aMessage, uint16_t aLength); +extern "C" void otPlatBorderRoutingProcessDhcp6PdPrefix(otInstance *aInstance, + const otBorderRoutingPrefixTableEntry *aPrefixInfo); + /** * Implements bi-directional routing between Thread and Infrastructure networks. * @@ -87,6 +91,12 @@ class RoutingManager : public InstanceLocator friend class ot::Notifier; friend class ot::Instance; +#if OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE + friend void otPlatBorderRoutingProcessIcmp6Ra(otInstance *aInstance, const uint8_t *aMessage, uint16_t aLength); + friend void otPlatBorderRoutingProcessDhcp6PdPrefix(otInstance *aInstance, + const otBorderRoutingPrefixTableEntry *aPrefixInfo); +#endif + public: typedef NetworkData::RoutePreference RoutePreference; ///< Route preference (high, medium, low). typedef otBorderRoutingPrefixTableIterator PrefixTableIterator; ///< Prefix Table Iterator. @@ -300,34 +310,6 @@ class RoutingManager : public InstanceLocator */ Error GetOmrPrefix(Ip6::Prefix &aPrefix) const; -#if OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE - /** - * Returns the platform provided off-mesh-routable (OMR) prefix. - * - * The prefix is extracted from the platform generated RA messages handled by `ProcessPlatformGeneratedNd()`. - * - * @param[out] aPrefixInfo A reference to where the prefix info will be output to. - * - * @retval kErrorNone Successfully retrieved the OMR prefix. - * @retval kErrorNotFound There are no valid PD prefix on this BR. - * @retval kErrorInvalidState The Border Routing Manager is not initialized yet. - * - */ - Error GetPdOmrPrefix(PrefixTableEntry &aPrefixInfo) const; - - /** - * Returns platform generated RA message processed information. - * - * @param[out] aPdProcessedRaInfo A reference to where the PD processed RA info will be output to. - * - * @retval kErrorNone Successfully retrieved the Info. - * @retval kErrorNotFound There are no valid RA process info on this BR. - * @retval kErrorInvalidState The Border Routing Manager is not initialized yet. - * - */ - Error GetPdProcessedRaInfo(PdProcessedRaInfo &aPdProcessedRaInfo); -#endif - /** * Returns the currently favored off-mesh-routable (OMR) prefix. * @@ -528,64 +510,56 @@ class RoutingManager : public InstanceLocator #if OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE /** - * Handles a router advertisement message received on platform's Thread interface. - * - * Note: This method is a part of DHCPv6 PD support on Thread border routers. The message should be generated by the - * software like dnamasq, radvd, systemd-networkd on the platform as a part of the DHCPv6 prefix delegation process - * for distributing the prefix to the interfaces (links). + * Enables / Disables the DHCPv6 Prefix Delegation. * - * @param[in] aRouterAdvert A pointer to the buffer of the router advertisement message. - * @param[in] aLength The length of the router advertisement message. + * @param[in] aEnabled Whether to enable or disable. * */ - void ProcessPlatformGeneratedRa(const uint8_t *aRouterAdvert, uint16_t aLength) - { - mPdPrefixManager.ProcessPlatformGeneratedRa(aRouterAdvert, aLength); - } + void SetDhcp6PdEnabled(bool aEnabled) { return mPdPrefixManager.SetEnabled(aEnabled); } /** - * Handles a prefix delegated from a DHCPv6 PD server. The prefix is received on the DHCPv6 PD client callback and - * then this method can be used to configure the prefix in the Routing Manager module. - * - * Note: This method is a part of DHCPv6 PD support on Thread border routers. For platforms where it doesn't make - * sense to generate a RA to set a DHCPv6 PD prefix this method can be used to set the prefix directly. The lifetime - * of the prefix can be updated by calling the function again with updated values. + * Returns the state DHCPv6 Prefix Delegation manager. * - * @param[in] aPrefixInfo Prefix information structure received from the DHCPv6 PD server. + * @returns The DHCPv6 PD state. * */ - void ProcessDhcpPdPrefix(const PrefixTableEntry &aPrefixInfo) { mPdPrefixManager.ProcessDhcpPdPrefix(aPrefixInfo); } + Dhcp6PdState GetDhcp6PdState(void) const { return mPdPrefixManager.GetState(); } /** - * Enables / Disables the functions for DHCPv6 PD. + * Sets the callback to notify when DHCPv6 Prefix Delegation manager state gets changed. * - * @param[in] aEnabled Whether to accept platform generated RA messages. + * @param[in] aCallback A pointer to a callback function + * @param[in] aContext A pointer to arbitrary context information. * */ - void SetDhcp6PdEnabled(bool aEnabled) { return mPdPrefixManager.SetEnabled(aEnabled); } + void SetRequestDhcp6PdCallback(PdCallback aCallback, void *aContext) + { + mPdPrefixManager.SetStateCallback(aCallback, aContext); + } /** - * Returns the state of accpeting RouterAdvertisement messages on platform interface. + * Returns the DHCPv6-PD based off-mesh-routable (OMR) prefix. + * + * @param[out] aPrefixInfo A reference to where the prefix info will be output to. * - * @retval kDhcp6PdStateRunning DHCPv6 PD should be enabled and running on this border router. - * @retval kDhcp6PdStateDisabled DHCPv6 PD should be disabled on this border router.. + * @retval kErrorNone Successfully retrieved the OMR prefix. + * @retval kErrorNotFound There are no valid PD prefix on this BR. + * @retval kErrorInvalidState The Border Routing Manager is not initialized yet. * */ - Dhcp6PdState GetDhcp6PdState(void) const { return mPdPrefixManager.GetState(); } + Error GetPdOmrPrefix(PrefixTableEntry &aPrefixInfo) const; /** - * Sets the callback whenever the state of a prefix request or release, via the DHCPv6 Prefix Delegation (PD), - * changes on the Thread interface. + * Returns platform generated RA message processed counters and information. * - * @param[in] aCallback A pointer to a function that is called whenever the state of a prefix request or release - * changes. - * @param[in] aContext A pointer to arbitrary context information. + * @param[out] aPdProcessedRaInfo A reference to where the PD processed RA info will be output to. + * + * @retval kErrorNone Successfully retrieved the Info. + * @retval kErrorNotFound There are no valid RA process info on this BR. + * @retval kErrorInvalidState The Border Routing Manager is not initialized yet. * */ - void SetRequestDhcp6PdCallback(PdCallback aCallback, void *aContext) - { - mPdPrefixManager.SetRequestDhcp6PdCallback(aCallback, aContext); - } + Error GetPdProcessedRaInfo(PdProcessedRaInfo &aPdProcessedRaInfo); #endif // OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE @@ -1294,58 +1268,58 @@ class RoutingManager : public InstanceLocator class PdPrefixManager : public InstanceLocator { public: - // This class implements handling (including management of the lifetime) of the prefix obtained from platform's - // DHCPv6 PD client. We expect the platform will send ICMP6 RA messages to the platform's interface for the - // information of the prefix. - // This class manages the state of the PD prefix in OmrPrefixManager + // This class implements handling (including management of the + // lifetime) of the prefix obtained from platform's DHCPv6 PD + // client. + + typedef Dhcp6PdState State; explicit PdPrefixManager(Instance &aInstance); void SetEnabled(bool aEnabled); void Start(void) { StartStop(/* aStart= */ true); } void Stop(void) { StartStop(/* aStart= */ false); } - bool IsRunning(void) const { return GetState() == Dhcp6PdState::kDhcp6PdStateRunning; } + bool IsRunning(void) const { return GetState() == kDhcp6PdStateRunning; } bool HasPrefix(void) const { return IsValidOmrPrefix(mPrefix.GetPrefix()); } const Ip6::Prefix &GetPrefix(void) const { return mPrefix.GetPrefix(); } - Dhcp6PdState GetState(void) const; + State GetState(void) const; - void ProcessPlatformGeneratedRa(const uint8_t *aRouterAdvert, uint16_t aLength); - void ProcessDhcpPdPrefix(const PrefixTableEntry &aPrefixTableEntry); + void ProcessRa(const uint8_t *aRouterAdvert, uint16_t aLength); + void ProcessPrefix(const PrefixTableEntry &aPrefixTableEntry); Error GetPrefixInfo(PrefixTableEntry &aInfo) const; Error GetProcessedRaInfo(PdProcessedRaInfo &aPdProcessedRaInfo) const; void HandleTimer(void) { WithdrawPrefix(); } - void SetRequestDhcp6PdCallback(PdCallback aCallback, void *aContext) + void SetStateCallback(PdCallback aCallback, void *aContext) { mStateCallback.Set(aCallback, aContext); } + + private: + class PrefixEntry : public DiscoveredPrefixTable::Entry { - mExternalCallback.Set(aCallback, aContext); - } + public: + PrefixEntry(void) { Clear(); } + bool IsEmpty(void) const { return (GetPrefix().GetLength() == 0); } + bool IsValidPdPrefix(void) const; + bool IsFavoredOver(const PrefixEntry &aOther) const; + }; - static const char *StateToString(Dhcp6PdState aState); + void Process(const RouterAdvert::Icmp6Packet *aRaPacket, const PrefixTableEntry *aPrefixTableEntry); + bool ProcessPrefixEntry(PrefixEntry &aEntry, PrefixEntry &aFavoredEntry); + void EvaluateStateChange(State aOldState); + void WithdrawPrefix(void); + void StartStop(bool aStart); - static bool IsValidPdPrefix(const Ip6::Prefix &aPrefix) - { - // We should accept ULA prefix since it could be used by the internet infrastructure like NAT64. - return aPrefix.GetLength() != 0 && aPrefix.GetLength() <= kOmrPrefixLength && !aPrefix.IsLinkLocal() && - !aPrefix.IsMulticast(); - } + static const char *StateToString(State aState); - private: - Error Process(const RouterAdvert::RxMessage *aMessage, const PrefixTableEntry *aPrefixTableEntry); - bool ProcessPrefixEntry(DiscoveredPrefixTable::Entry &aEntry, DiscoveredPrefixTable::Entry &aFavoredEntry); - void EvaluateStateChange(Dhcp6PdState aOldState); - void WithdrawPrefix(void); - void StartStop(bool aStart); - - using PlatformOmrPrefixTimer = TimerMilliIn; - using ExternalCallback = Callback; - - bool mEnabled; - bool mIsRunning; - uint32_t mNumPlatformPioProcessed; - uint32_t mNumPlatformRaReceived; - TimeMilli mLastPlatformRaTime; - ExternalCallback mExternalCallback; - PlatformOmrPrefixTimer mTimer; - DiscoveredPrefixTable::Entry mPrefix; + using PrefixTimer = TimerMilliIn; + using StateCallback = Callback; + + bool mEnabled; + bool mIsRunning; + uint32_t mNumPlatformPioProcessed; + uint32_t mNumPlatformRaReceived; + TimeMilli mLastPlatformRaTime; + StateCallback mStateCallback; + PrefixTimer mTimer; + PrefixEntry mPrefix; }; #endif // OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE From 83af14a950f05046ebea7d1877799fbe8a85b7b3 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 7 May 2024 13:18:07 -0700 Subject: [PATCH 06/13] [dataset] send 'reject' response to `MGMT_SET` on non-leader (#10148) This commit simplifies the code and adheres to the SHOULD requirement in the Thread specification, which states: "A Thread Device that is not a Leader MUST NOT process this message. Instead it SHOULD respond with 'Reject' state value". --- src/core/meshcop/dataset_manager_ftd.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/meshcop/dataset_manager_ftd.cpp b/src/core/meshcop/dataset_manager_ftd.cpp index 077cd79e1bf..171919e23a1 100644 --- a/src/core/meshcop/dataset_manager_ftd.cpp +++ b/src/core/meshcop/dataset_manager_ftd.cpp @@ -212,6 +212,8 @@ Error DatasetManager::HandleSet(const Coap::Message &aMessage, const Ip6::Messag StateTlv::State state = StateTlv::kReject; SetRequestInfo info; + VerifyOrExit(Get().IsLeader()); + SuccessOrExit(ProcessSetRequest(aMessage, info)); if (IsActiveDataset() && info.mAffectsConnectivity) @@ -374,7 +376,6 @@ void ActiveDatasetManager::StartLeader(void) {} template <> void ActiveDatasetManager::HandleTmf(Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) { - VerifyOrExit(Get().IsLeader()); SuccessOrExit(DatasetManager::HandleSet(aMessage, aMessageInfo)); IgnoreError(ApplyConfiguration()); @@ -387,7 +388,6 @@ void PendingDatasetManager::StartLeader(void) { StartDelayTimer(); } template <> void PendingDatasetManager::HandleTmf(Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) { - VerifyOrExit(Get().IsLeader()); SuccessOrExit(DatasetManager::HandleSet(aMessage, aMessageInfo)); StartDelayTimer(); From 383d0d28158969da2f00a1de291368a46986160f Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 7 May 2024 13:52:50 -0700 Subject: [PATCH 07/13] [routing-manager] add `CalculateExpirationTime()` (#10207) This commit adds `CalculateExpirationTime()`, which simplifies the calculation of expiration times of discovered prefix entry by clamping a given lifetime to the maximum value (in seconds) first before converting to milliseconds. --- src/core/border_router/routing_manager.cpp | 22 ++++++++-------------- src/core/border_router/routing_manager.hpp | 2 +- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index ebd5d019e3c..0d001e38c7d 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -1740,7 +1740,7 @@ bool RoutingManager::DiscoveredPrefixTable::Entry::Matches(const ExpirationCheck TimeMilli RoutingManager::DiscoveredPrefixTable::Entry::GetExpireTime(void) const { - return mLastUpdateTime + CalculateExpireDelay(mValidLifetime); + return CalculateExpirationTime(mValidLifetime); } TimeMilli RoutingManager::DiscoveredPrefixTable::Entry::GetStaleTime(void) const @@ -1752,14 +1752,14 @@ TimeMilli RoutingManager::DiscoveredPrefixTable::Entry::GetStaleTime(void) const TimeMilli RoutingManager::DiscoveredPrefixTable::Entry::GetStaleTimeFromPreferredLifetime(void) const { - return mLastUpdateTime + CalculateExpireDelay(GetPreferredLifetime()); + return CalculateExpirationTime(GetPreferredLifetime()); } bool RoutingManager::DiscoveredPrefixTable::Entry::IsDeprecated(void) const { OT_ASSERT(IsOnLinkPrefix()); - return mLastUpdateTime + CalculateExpireDelay(GetPreferredLifetime()) <= TimerMilli::GetNow(); + return CalculateExpirationTime(GetPreferredLifetime()) <= TimerMilli::GetNow(); } RoutingManager::RoutePreference RoutingManager::DiscoveredPrefixTable::Entry::GetPreference(void) const @@ -1798,20 +1798,14 @@ void RoutingManager::DiscoveredPrefixTable::Entry::AdoptValidAndPreferredLifetim mLastUpdateTime = aEntry.GetLastUpdateTime(); } -uint32_t RoutingManager::DiscoveredPrefixTable::Entry::CalculateExpireDelay(uint32_t aValidLifetime) +TimeMilli RoutingManager::DiscoveredPrefixTable::Entry::CalculateExpirationTime(uint32_t aLifetime) const { - uint32_t delay; + // `aLifetime` is in unit of seconds. We clamp the lifetime to max + // interval supported by `Timer` (`2^31` msec or ~24.8 days). - if (aValidLifetime * static_cast(1000) > Timer::kMaxDelay) - { - delay = Timer::kMaxDelay; - } - else - { - delay = aValidLifetime * 1000; - } + static constexpr uint32_t kMaxLifetime = Time::MsecToSec(Timer::kMaxDelay); - return delay; + return mLastUpdateTime + Time::SecToMsec(Min(aLifetime, kMaxLifetime)); } //--------------------------------------------------------------------------------------------------------------------- diff --git a/src/core/border_router/routing_manager.hpp b/src/core/border_router/routing_manager.hpp index a38860ccb01..ef52ca65d6a 100644 --- a/src/core/border_router/routing_manager.hpp +++ b/src/core/border_router/routing_manager.hpp @@ -787,7 +787,7 @@ class RoutingManager : public InstanceLocator RoutePreference GetRoutePreference(void) const { return mShared.mRoutePreference; } private: - static uint32_t CalculateExpireDelay(uint32_t aValidLifetime); + TimeMilli CalculateExpirationTime(uint32_t aLifetime) const; Entry *mNext; Ip6::Prefix mPrefix; From 00f7c3131b8a44868ae51ea44dab374059280db6 Mon Sep 17 00:00:00 2001 From: Li Cao Date: Thu, 9 May 2024 02:29:25 +0800 Subject: [PATCH 08/13] [spinel] move definition of coprocessor type to `lib/spinel` (#10208) This commit moves the definition of coprocessor_type from posix to lib/spinel directory. There are two reasons to do so: 1. Allow platforms other than posix to access the definition. For example, nxp imt-rx. 2. Allow the user (posix/main.c) of OT system API to access the definition. Currently the posix main depends on OT system API but doesn't depend on definitions in 'platform-posix.h'. This should be kept. --- src/{posix/platform => lib/spinel}/coprocessor_type.h | 6 +++--- src/lib/spinel/spinel_driver.hpp | 2 +- src/posix/platform/platform-posix.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) rename src/{posix/platform => lib/spinel}/coprocessor_type.h (94%) diff --git a/src/posix/platform/coprocessor_type.h b/src/lib/spinel/coprocessor_type.h similarity index 94% rename from src/posix/platform/coprocessor_type.h rename to src/lib/spinel/coprocessor_type.h index 309f2d5c4b0..78fc4e19b94 100644 --- a/src/posix/platform/coprocessor_type.h +++ b/src/lib/spinel/coprocessor_type.h @@ -26,8 +26,8 @@ * POSSIBILITY OF SUCH DAMAGE. */ -#ifndef OT_PLATFORM_COPROCESSOR_MODE_H_ -#define OT_PLATFORM_COPROCESSOR_MODE_H_ +#ifndef COPROCESSOR_TYPE_H_ +#define COPROCESSOR_TYPE_H_ #ifdef __cplusplus extern "C" { @@ -47,4 +47,4 @@ typedef enum CoprocessorType #ifdef __cplusplus } #endif -#endif // OT_PLATFORM_COPROCESSOR_MODE_H_ +#endif // COPROCESSOR_TYPE_H_ diff --git a/src/lib/spinel/spinel_driver.hpp b/src/lib/spinel/spinel_driver.hpp index 2e00c694d85..1f45bc719ac 100644 --- a/src/lib/spinel/spinel_driver.hpp +++ b/src/lib/spinel/spinel_driver.hpp @@ -31,10 +31,10 @@ #include +#include "lib/spinel/coprocessor_type.h" #include "lib/spinel/logger.hpp" #include "lib/spinel/spinel.h" #include "lib/spinel/spinel_interface.hpp" -#include "posix/platform/coprocessor_type.h" namespace ot { namespace Spinel { diff --git a/src/posix/platform/platform-posix.h b/src/posix/platform/platform-posix.h index 93ef4ac162f..ca897973bb3 100644 --- a/src/posix/platform/platform-posix.h +++ b/src/posix/platform/platform-posix.h @@ -52,8 +52,8 @@ #include #include -#include "coprocessor_type.h" #include "lib/platform/exit_code.h" +#include "lib/spinel/coprocessor_type.h" #include "lib/url/url.hpp" /** From 226f239c5d85bfc0eb02cd771ddca65a34ca8f73 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Wed, 8 May 2024 11:32:42 -0700 Subject: [PATCH 09/13] [owning-list] add `RemoveAndFreeAllMatching()` to `OwningList` class (#10210) This commit introduces a new method, `RemoveAndFreeAllMatching()`, to the `OwningList` class. This method removes and frees all entries in the list that match a given indicator. This is used to simplify the native mDNS implementation. The unit test `test_linked_list` is also updated to validate the newly added helper method. --- src/core/common/owning_list.hpp | 23 ++++++++++++++ src/core/net/mdns.cpp | 55 ++++++++++++--------------------- tests/unit/test_linked_list.cpp | 37 ++++++++++++++++++++++ 3 files changed, 79 insertions(+), 36 deletions(-) diff --git a/src/core/common/owning_list.hpp b/src/core/common/owning_list.hpp index 583bc685996..3d1ea5544a7 100644 --- a/src/core/common/owning_list.hpp +++ b/src/core/common/owning_list.hpp @@ -149,6 +149,29 @@ template class OwningList : public LinkedList { LinkedList::RemoveAllMatching(aIndicator, aRemovedList); } + + /** + * Removes and frees all entries in the list matching a given entry indicator. + * + * The template type `Indicator` specifies the type of @p aIndicator object which is used to match against entries + * in the list. To check that an entry matches the given indicator, the `Matches()` method is invoked on each + * `Type` entry in the list. The `Matches()` method should be provided by `Type` class accordingly: + * + * bool Type::Matches(const Indicator &aIndicator) const + * + * @param[in] aIndicator An entry indicator to match against entries in the list. + * + * @retval TRUE At least one matching entry was removed. + * @retval FALSE No matching entry was found. + * + */ + template bool RemoveAndFreeAllMatching(const Indicator &aIndicator) + { + OwningList removedList; + + RemoveAllMatching(aIndicator, removedList); + return !removedList.IsEmpty(); + } }; } // namespace ot diff --git a/src/core/net/mdns.cpp b/src/core/net/mdns.cpp index ddf699539c6..996fa1c3525 100644 --- a/src/core/net/mdns.cpp +++ b/src/core/net/mdns.cpp @@ -313,11 +313,8 @@ void Core::HandleEntryTimer(void) void Core::RemoveEmptyEntries(void) { - OwningList removedHosts; - OwningList removedServices; - - mHostEntries.RemoveAllMatching(Entry::kRemoving, removedHosts); - mServiceEntries.RemoveAllMatching(Entry::kRemoving, removedServices); + mHostEntries.RemoveAndFreeAllMatching(Entry::kRemoving); + mServiceEntries.RemoveAndFreeAllMatching(Entry::kRemoving); } void Core::HandleEntryTask(void) @@ -1874,9 +1871,7 @@ void Core::ServiceEntry::ClearService(void) void Core::ServiceEntry::ScheduleToRemoveIfEmpty(void) { - OwningList removedSubTypes; - - mSubTypes.RemoveAllMatching(EmptyChecker(), removedSubTypes); + mSubTypes.RemoveAndFreeAllMatching(EmptyChecker()); if (IsEmpty()) { @@ -2149,8 +2144,6 @@ void Core::ServiceEntry::PrepareResponseRecords(TxMessage &aResponse, TimeMilli void Core::ServiceEntry::UpdateRecordsState(const TxMessage &aResponse) { - OwningList removedSubTypes; - Entry::UpdateRecordsState(aResponse); mPtrRecord.UpdateStateAfterAnswer(aResponse); @@ -2162,7 +2155,7 @@ void Core::ServiceEntry::UpdateRecordsState(const TxMessage &aResponse) subType.mPtrRecord.UpdateStateAfterAnswer(aResponse); } - mSubTypes.RemoveAllMatching(EmptyChecker(), removedSubTypes); + mSubTypes.RemoveAndFreeAllMatching(EmptyChecker()); if (IsEmpty()) { @@ -4142,11 +4135,10 @@ void Core::TxMessageHistory::CalculateHash(const Message &aMessage, Hash &aHash) void Core::TxMessageHistory::HandleTimer(void) { - TimeMilli now = TimerMilli::GetNow(); - TimeMilli nextTime = now.GetDistantFuture(); - OwningList expiredEntries; + TimeMilli now = TimerMilli::GetNow(); + TimeMilli nextTime = now.GetDistantFuture(); - mHashEntries.RemoveAllMatching(ExpireChecker(now), expiredEntries); + mHashEntries.RemoveAndFreeAllMatching(ExpireChecker(now)); for (const HashEntry &entry : mHashEntries) { @@ -4268,21 +4260,16 @@ void Core::AddPassiveIp6AddrCache(const char *aHostName) void Core::HandleCacheTimer(void) { - CacheTimerContext context(GetInstance()); - ExpireChecker expireChecker(context.GetNow()); - OwningList expiredBrowseList; - OwningList expiredSrvList; - OwningList expiredTxtList; - OwningList expiredIp6AddrList; - OwningList expiredIp4AddrList; + CacheTimerContext context(GetInstance()); + ExpireChecker expireChecker(context.GetNow()); // First remove all expired entries. - mBrowseCacheList.RemoveAllMatching(expireChecker, expiredBrowseList); - mSrvCacheList.RemoveAllMatching(expireChecker, expiredSrvList); - mTxtCacheList.RemoveAllMatching(expireChecker, expiredTxtList); - mIp6AddrCacheList.RemoveAllMatching(expireChecker, expiredIp6AddrList); - mIp4AddrCacheList.RemoveAllMatching(expireChecker, expiredIp4AddrList); + mBrowseCacheList.RemoveAndFreeAllMatching(expireChecker); + mSrvCacheList.RemoveAndFreeAllMatching(expireChecker); + mTxtCacheList.RemoveAndFreeAllMatching(expireChecker); + mIp6AddrCacheList.RemoveAndFreeAllMatching(expireChecker); + mIp4AddrCacheList.RemoveAndFreeAllMatching(expireChecker); // Process cache types in a specific order to optimize name // compression when constructing query messages. @@ -4720,9 +4707,7 @@ void Core::CacheEntry::Remove(const ResultCallback &aCallback) void Core::CacheEntry::ClearEmptyCallbacks(void) { - CallbackList emptyCallbacks; - - mCallbacks.RemoveAllMatching(EmptyChecker(), emptyCallbacks); + mCallbacks.RemoveAndFreeAllMatching(EmptyChecker()); if (mCallbacks.IsEmpty()) { @@ -5765,13 +5750,13 @@ void Core::AddrCache::DetermineRecordFireTime(void) void Core::AddrCache::ProcessExpiredRecords(TimeMilli aNow) { - OwningList expiredEntries; Heap::Array addrArray; AddressResult result; + bool didRemoveAny; - mCommittedEntries.RemoveAllMatching(ExpireChecker(aNow), expiredEntries); + didRemoveAny = mCommittedEntries.RemoveAndFreeAllMatching(ExpireChecker(aNow)); - VerifyOrExit(!expiredEntries.IsEmpty()); + VerifyOrExit(didRemoveAny); ConstructResult(result, addrArray); InvokeCallbacks(result); @@ -5963,9 +5948,7 @@ void Core::AddrCache::CommitNewResponseEntries(void) } else { - OwningList removedEntries; - - mCommittedEntries.RemoveAllMatching(EmptyChecker(), removedEntries); + mCommittedEntries.RemoveAndFreeAllMatching(EmptyChecker()); } while (!mNewEntries.IsEmpty()) diff --git a/tests/unit/test_linked_list.cpp b/tests/unit/test_linked_list.cpp index 8c581d76f15..819f3790f12 100644 --- a/tests/unit/test_linked_list.cpp +++ b/tests/unit/test_linked_list.cpp @@ -313,6 +313,7 @@ void TestOwningList(void) OwningList list; OwningList removedList; OwnedPtr ptr; + bool didRemove; printf("TestOwningList\n"); @@ -433,6 +434,42 @@ void TestOwningList(void) VerifyOrQuit(list.IsEmpty()); VerifyLinkedListContent(&removedList, &c, &d, &f, nullptr); VerifyOrQuit(!c.WasFreed()); + + // Test `RemoveAndFreeAllMatching()` + + a.ResetTestFlags(); + b.ResetTestFlags(); + c.ResetTestFlags(); + d.ResetTestFlags(); + e.ResetTestFlags(); + f.ResetTestFlags(); + list.Push(a); + list.Push(b); + list.Push(c); + list.Push(d); + list.Push(e); + list.Push(f); + VerifyLinkedListContent(&list, &f, &e, &d, &c, &b, &a, nullptr); + + didRemove = list.RemoveAndFreeAllMatching(kAlphaType); + VerifyLinkedListContent(&list, &f, &d, &c, nullptr); + VerifyOrQuit(didRemove); + VerifyOrQuit(a.WasFreed()); + VerifyOrQuit(b.WasFreed()); + VerifyOrQuit(e.WasFreed()); + VerifyOrQuit(!c.WasFreed()); + + didRemove = list.RemoveAndFreeAllMatching(kAlphaType); + VerifyOrQuit(!didRemove); + VerifyLinkedListContent(&list, &f, &d, &c, nullptr); + VerifyOrQuit(!c.WasFreed()); + + didRemove = list.RemoveAndFreeAllMatching(kBetaType); + VerifyOrQuit(list.IsEmpty()); + VerifyOrQuit(didRemove); + VerifyOrQuit(c.WasFreed()); + VerifyOrQuit(d.WasFreed()); + VerifyOrQuit(f.WasFreed()); } } // namespace ot From 8e3f51da841a1106a15f4b327edee700f6dd701a Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Wed, 8 May 2024 18:56:02 -0700 Subject: [PATCH 10/13] [dns] add checks for multi-label looped compressed DNS names (#10215) This commit enhances `Dns::Name` to detect and handle cases where a compressed DNS name contains a loop spanning multiple labels. The `test_dns` unit test has been updated to include test cases for these invalid names. --- src/core/net/dns_types.cpp | 3 +- src/core/net/dns_types.hpp | 2 + tests/unit/test_dns.cpp | 79 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/src/core/net/dns_types.cpp b/src/core/net/dns_types.cpp index b2df69ae52f..f6b8c58b721 100644 --- a/src/core/net/dns_types.cpp +++ b/src/core/net/dns_types.cpp @@ -612,8 +612,9 @@ Error Name::LabelIterator::GetNextLabel(void) // `mMessage.GetOffset()` must point to the start of the // DNS header. nextLabelOffset = mMessage.GetOffset() + (BigEndian::HostSwap16(pointerValue) & kPointerLabelOffsetMask); - VerifyOrExit(nextLabelOffset < mNextLabelOffset, error = kErrorParse); + VerifyOrExit(nextLabelOffset < mMinLabelOffset, error = kErrorParse); mNextLabelOffset = nextLabelOffset; + mMinLabelOffset = nextLabelOffset; // Go back through the `while(true)` loop to get the next label. } diff --git a/src/core/net/dns_types.hpp b/src/core/net/dns_types.hpp index 1083ee59eea..d822bacd157 100644 --- a/src/core/net/dns_types.hpp +++ b/src/core/net/dns_types.hpp @@ -1145,6 +1145,7 @@ class Name : public Clearable : mMessage(aMessage) , mNextLabelOffset(aLabelOffset) , mNameEndOffset(kUnsetNameEndOffset) + , mMinLabelOffset(aLabelOffset) { } @@ -1162,6 +1163,7 @@ class Name : public Clearable uint8_t mLabelLength; // Length of current label (number of chars). uint16_t mNextLabelOffset; // Offset in `mMessage` to the start of the next label. uint16_t mNameEndOffset; // Offset in `mMessage` to the byte after the end of domain name field. + uint16_t mMinLabelOffset; // Offset in `mMessage` to the start of the earliest parsed label. }; Name(const char *aString, const Message *aMessage, uint16_t aOffset) diff --git a/tests/unit/test_dns.cpp b/tests/unit/test_dns.cpp index e3bac5c4120..c11545dcd97 100644 --- a/tests/unit/test_dns.cpp +++ b/tests/unit/test_dns.cpp @@ -1031,6 +1031,85 @@ void TestDnsCompressedName(void) printf("- Name3 after `AppendTo()`: \"%s\"\n", name); // `ReadName()` for name-4 will fail due to first label containing dot char. + printf("----------------------------------------------------------------\n"); + printf("Improperly formatted names (looped)\n"); + + { + // Pointer label jumps forward to offset 6. + static const uint8_t kEncodedName[] = {3, 'F', 'O', 'O', 0xc0, 6, 2, 'A', 'B', 0}; + + SuccessOrQuit(message->SetLength(0)); + SuccessOrQuit(message->Append(kEncodedName)); + DumpBuffer("pointer-moving-forward", kEncodedName, sizeof(kEncodedName)); + offset = 0; + VerifyOrQuit(Dns::Name::ParseName(*message, offset) == kErrorParse); + VerifyOrQuit(Dns::Name::ReadName(*message, offset, name) == kErrorParse); + } + + { + // Pointer label jumps back to offset 0 creating a loop. + static const uint8_t kEncodedName[] = {0xc0, 0}; + + SuccessOrQuit(message->SetLength(0)); + SuccessOrQuit(message->Append(kEncodedName)); + DumpBuffer("looped-name", kEncodedName, sizeof(kEncodedName)); + offset = 0; + VerifyOrQuit(Dns::Name::ParseName(*message, offset) == kErrorParse); + VerifyOrQuit(Dns::Name::ReadName(*message, offset, name) == kErrorParse); + } + + { + // After first label, we have a pointer label jumping back to + // offset 0, creating a loop. + + static const uint8_t kEncodedName[] = {3, 'F', 'O', 'O', 0xc0, 0}; + + SuccessOrQuit(message->SetLength(0)); + SuccessOrQuit(message->Append(kEncodedName)); + DumpBuffer("looped-one-label", kEncodedName, sizeof(kEncodedName)); + offset = 0; + VerifyOrQuit(Dns::Name::ParseName(*message, offset) == kErrorParse); + VerifyOrQuit(Dns::Name::ReadName(*message, offset, name) == kErrorParse); + VerifyOrQuit(Dns::Name::CompareName(*message, offset, "FOO.") == kErrorParse); + } + + { + // After two labels we have a pointer label jumping back to + // start of the first label. Name itself starts at offset 3 + // (first 3 bytes are unused). + + static const uint8_t kEncodedName[] = {0, 0, 0, 3, 'F', 'O', 'O', 2, 'B', 'A', 0xc0, 3}; + + SuccessOrQuit(message->SetLength(0)); + SuccessOrQuit(message->Append(kEncodedName)); + DumpBuffer("looped-two-labels", kEncodedName, sizeof(kEncodedName)); + offset = 3; + VerifyOrQuit(Dns::Name::ParseName(*message, offset) == kErrorParse); + VerifyOrQuit(Dns::Name::ReadName(*message, offset, name) == kErrorParse); + VerifyOrQuit(Dns::Name::CompareName(*message, offset, "FOO.BA.") == kErrorParse); + } + + { + // Same as last case, but pointer label now points to + // immediately before the first label (at offset 2). The name + // is now valid. + + static const uint8_t kEncodedName[] = {0, 0, 0, 3, 'F', 'O', 'O', 2, 'B', 'A', 0xc0, 2}; + + SuccessOrQuit(message->SetLength(0)); + SuccessOrQuit(message->Append(kEncodedName)); + DumpBuffer("valid-name", kEncodedName, sizeof(kEncodedName)); + offset = 3; + SuccessOrQuit(Dns::Name::ParseName(*message, offset)); + VerifyOrQuit(offset == sizeof(kEncodedName)); + + offset = 3; + SuccessOrQuit(Dns::Name::ReadName(*message, offset, name)); + VerifyOrQuit(offset == sizeof(kEncodedName)); + VerifyOrQuit(strcmp(name, "FOO.BA.") == 0); + printf("Read name =\"%s\"\n", name); + } + message->Free(); message2->Free(); testFreeInstance(instance); From 74573b5d3f65ef7c0168ea53a636d48b2e3ee368 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Wed, 8 May 2024 19:53:25 -0700 Subject: [PATCH 11/13] [routing-manager] use `OwningList` for `Router` and `Entry` lists (#10214) This change leverages `OwningList<>` to manage discovered routers and prefix entries within `RoutingManager`. This ensures automatic deallocation of these items, streamlining memory management. It works independently of whether the items are heap-allocated or from a `Pool`. --- src/core/border_router/routing_manager.cpp | 96 ++++++++++++---------- src/core/border_router/routing_manager.hpp | 28 ++++--- 2 files changed, 69 insertions(+), 55 deletions(-) diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index 0d001e38c7d..84db52d54f6 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -1199,6 +1199,32 @@ void RoutingManager::DiscoveredPrefixTable::ProcessRaFlagsExtOption(const RaFlag return; } +#if !OPENTHREAD_CONFIG_BORDER_ROUTING_USE_HEAP_ENABLE + +RoutingManager::DiscoveredPrefixTable::Router *RoutingManager::DiscoveredPrefixTable::AllocateRouter(void) +{ + Router *router = mRouterPool.Allocate(); + + VerifyOrExit(router != nullptr); + router->Init(GetInstance()); + +exit: + return router; +} + +RoutingManager::DiscoveredPrefixTable::Entry *RoutingManager::DiscoveredPrefixTable::AllocateEntry(void) +{ + Entry *entry = mEntryPool.Allocate(); + + VerifyOrExit(entry != nullptr); + entry->Init(GetInstance()); + +exit: + return entry; +} + +#endif // !OPENTHREAD_CONFIG_BORDER_ROUTING_USE_HEAP_ENABLE + bool RoutingManager::DiscoveredPrefixTable::Contains(const Entry::Checker &aChecker) const { bool contains = false; @@ -1270,16 +1296,15 @@ void RoutingManager::DiscoveredPrefixTable::RemovePrefix(const Entry::Matcher &a { // Removes all entries matching a given prefix from the table. - LinkedList removedEntries; + bool didRemove = false; for (Router &router : mRouters) { - router.mEntries.RemoveAllMatching(aMatcher, removedEntries); + didRemove |= router.mEntries.RemoveAndFreeAllMatching(aMatcher); } - VerifyOrExit(!removedEntries.IsEmpty()); + VerifyOrExit(didRemove); - FreeEntries(removedEntries); RemoveRoutersWithNoEntriesOrFlags(); SignalTableChanged(); @@ -1292,12 +1317,7 @@ void RoutingManager::DiscoveredPrefixTable::RemoveAllEntries(void) { // Remove all entries from the table. - for (Router &router : mRouters) - { - FreeEntries(router.mEntries); - } - - FreeRouters(mRouters); + mRouters.Free(); mEntryTimer.Stop(); SignalTableChanged(); @@ -1398,58 +1418,29 @@ TimeMilli RoutingManager::DiscoveredPrefixTable::CalculateNextStaleTime(TimeMill void RoutingManager::DiscoveredPrefixTable::RemoveRoutersWithNoEntriesOrFlags(void) { - LinkedList routersToFree; - - mRouters.RemoveAllMatching(Router::kContainsNoEntriesOrFlags, routersToFree); - FreeRouters(routersToFree); -} - -void RoutingManager::DiscoveredPrefixTable::FreeRouters(LinkedList &aRouters) -{ - // Frees all routers in the given list `aRouters` - - Router *router; - - while ((router = aRouters.Pop()) != nullptr) - { - FreeRouter(*router); - } -} - -void RoutingManager::DiscoveredPrefixTable::FreeEntries(LinkedList &aEntries) -{ - // Frees all entries in the given list `aEntries`. - - Entry *entry; - - while ((entry = aEntries.Pop()) != nullptr) - { - FreeEntry(*entry); - } + mRouters.RemoveAndFreeAllMatching(Router::kContainsNoEntriesOrFlags); } void RoutingManager::DiscoveredPrefixTable::HandleEntryTimer(void) { RemoveExpiredEntries(); } void RoutingManager::DiscoveredPrefixTable::RemoveExpiredEntries(void) { - TimeMilli now = TimerMilli::GetNow(); - TimeMilli nextExpireTime = now.GetDistantFuture(); - LinkedList expiredEntries; + TimeMilli now = TimerMilli::GetNow(); + TimeMilli nextExpireTime = now.GetDistantFuture(); + bool didRemove = false; for (Router &router : mRouters) { - router.mEntries.RemoveAllMatching(Entry::ExpirationChecker(now), expiredEntries); + didRemove |= router.mEntries.RemoveAndFreeAllMatching(Entry::ExpirationChecker(now)); } RemoveRoutersWithNoEntriesOrFlags(); - if (!expiredEntries.IsEmpty()) + if (didRemove) { SignalTableChanged(); } - FreeEntries(expiredEntries); - // Determine the next expire time and schedule timer. for (const Router &router : mRouters) @@ -1808,6 +1799,13 @@ TimeMilli RoutingManager::DiscoveredPrefixTable::Entry::CalculateExpirationTime( return mLastUpdateTime + Time::SecToMsec(Min(aLifetime, kMaxLifetime)); } +#if !OPENTHREAD_CONFIG_BORDER_ROUTING_USE_HEAP_ENABLE +void RoutingManager::DiscoveredPrefixTable::Entry::Free(void) +{ + Get().mDiscoveredPrefixTable.mEntryPool.Free(*this); +} +#endif + //--------------------------------------------------------------------------------------------------------------------- // DiscoveredPrefixTable::Router @@ -1840,6 +1838,14 @@ void RoutingManager::DiscoveredPrefixTable::Router::CopyInfoTo(RouterEntry &aEnt aEntry.mStubRouterFlag = mStubRouterFlag; } +#if !OPENTHREAD_CONFIG_BORDER_ROUTING_USE_HEAP_ENABLE +void RoutingManager::DiscoveredPrefixTable::Router::Free(void) +{ + mEntries.Free(); + Get().mDiscoveredPrefixTable.mRouterPool.Free(*this); +} +#endif + //--------------------------------------------------------------------------------------------------------------------- // FavoredOmrPrefix diff --git a/src/core/border_router/routing_manager.hpp b/src/core/border_router/routing_manager.hpp index ef52ca65d6a..ec68ae73447 100644 --- a/src/core/border_router/routing_manager.hpp +++ b/src/core/border_router/routing_manager.hpp @@ -62,6 +62,7 @@ #include "common/locator.hpp" #include "common/message.hpp" #include "common/notifier.hpp" +#include "common/owning_list.hpp" #include "common/pool.hpp" #include "common/string.hpp" #include "common/timer.hpp" @@ -700,6 +701,8 @@ class RoutingManager : public InstanceLocator public Unequatable, #if OPENTHREAD_CONFIG_BORDER_ROUTING_USE_HEAP_ENABLE public Heap::Allocatable, +#else + public InstanceLocatorInit, #endif private Clearable { @@ -757,6 +760,10 @@ class RoutingManager : public InstanceLocator TimeMilli mNow; }; +#if !OPENTHREAD_CONFIG_BORDER_ROUTING_USE_HEAP_ENABLE + void Init(Instance &aInstance) { InstanceLocatorInit::Init(aInstance); } + void Free(void); +#endif void SetFrom(const RouterAdvert::Header &aRaHeader); void SetFrom(const PrefixInfoOption &aPio); void SetFrom(const RouteInfoOption &aRio); @@ -804,6 +811,8 @@ class RoutingManager : public InstanceLocator struct Router : public LinkedListEntry, #if OPENTHREAD_CONFIG_BORDER_ROUTING_USE_HEAP_ENABLE public Heap::Allocatable, +#else + public InstanceLocatorInit, #endif public Clearable { @@ -823,13 +832,18 @@ class RoutingManager : public InstanceLocator kContainsNoEntriesOrFlags }; +#if !OPENTHREAD_CONFIG_BORDER_ROUTING_USE_HEAP_ENABLE + void Init(Instance &aInstance) { InstanceLocatorInit::Init(aInstance); } + void Free(void); +#endif + bool Matches(const Ip6::Address &aAddress) const { return aAddress == mAddress; } bool Matches(EmptyChecker aChecker) const; void CopyInfoTo(RouterEntry &aEntry) const; Router *mNext; Ip6::Address mAddress; - LinkedList mEntries; + OwningList mEntries; TimeMilli mTimeout; uint8_t mNsProbeCount; bool mManagedAddressConfigFlag : 1; @@ -867,8 +881,6 @@ class RoutingManager : public InstanceLocator void RemovePrefix(const Entry::Matcher &aMatcher); void RemoveOrDeprecateEntriesFromInactiveRouters(void); void RemoveRoutersWithNoEntriesOrFlags(void); - void FreeRouters(LinkedList &aRouters); - void FreeEntries(LinkedList &aEntries); void UpdateNetworkDataOnChangeTo(Entry &aEntry); void RemoveExpiredEntries(void); void SignalTableChanged(void); @@ -877,20 +889,16 @@ class RoutingManager : public InstanceLocator #if OPENTHREAD_CONFIG_BORDER_ROUTING_USE_HEAP_ENABLE Router *AllocateRouter(void) { return Router::Allocate(); } Entry *AllocateEntry(void) { return Entry::Allocate(); } - void FreeRouter(Router &aRouter) { aRouter.Free(); } - void FreeEntry(Entry &aEntry) { aEntry.Free(); } #else - Router *AllocateRouter(void) { return mRouterPool.Allocate(); } - Entry *AllocateEntry(void) { return mEntryPool.Allocate(); } - void FreeRouter(Router &aRouter) { mRouterPool.Free(aRouter); } - void FreeEntry(Entry &aEntry) { mEntryPool.Free(aEntry); } + Router *AllocateRouter(void); + Entry *AllocateEntry(void); #endif using SignalTask = TaskletIn; using EntryTimer = TimerMilliIn; using RouterTimer = TimerMilliIn; - LinkedList mRouters; + OwningList mRouters; EntryTimer mEntryTimer; RouterTimer mRouterTimer; SignalTask mSignalTask; From 0b114821d9ea8a6c2765e4e75416f6e97bc3be11 Mon Sep 17 00:00:00 2001 From: Mason Tran Date: Thu, 9 May 2024 10:39:19 -0400 Subject: [PATCH 12/13] [spinel] only request crash logs on RCP recovery if capable (#10213) During RCP recovery, the Host does not check if the RCP supports logging a crash dump. This causes RCP recovery to fail when the RCP is built with `OPENTHREAD_CONFIG_PLATFORM_LOG_CRASH_DUMP_ENABLE = 0`. This bug was introduced in #10061 This commit will make the Host only request crash logs from the RCP if the RCP supports it. --- src/lib/spinel/radio_spinel.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/lib/spinel/radio_spinel.cpp b/src/lib/spinel/radio_spinel.cpp index 73507ff5a44..38ce7f3188d 100644 --- a/src/lib/spinel/radio_spinel.cpp +++ b/src/lib/spinel/radio_spinel.cpp @@ -2002,7 +2002,11 @@ void RadioSpinel::RecoverFromRcpFailure(void) --mRcpFailureCount; - SuccessOrDie(Set(SPINEL_PROP_RCP_LOG_CRASH_DUMP, nullptr)); + if (sSupportsLogCrashDump) + { + LogDebg("RCP supports crash dump logging. Requesting crash dump."); + SuccessOrDie(Set(SPINEL_PROP_RCP_LOG_CRASH_DUMP, nullptr)); + } LogNote("RCP recovery is done"); From 1fceb225b3858a990ffb6ce28f30b8b3dfba1614 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Feh=C3=A9r?= <82935051+feherdave@users.noreply.github.com> Date: Thu, 9 May 2024 16:41:54 +0200 Subject: [PATCH 13/13] [cli] add UDP socket openness check before sending with `udp send` (#10206) Problem: Executing the `udp send` command in CLI without previously opening the example udp socket results in an unhandled null-pointer exception. Solution: Before anything is processed in the `UdpExample::Process(Arg aArgs[])` method, the UDP socket is checked whether it's open or not. If not, the method returns with OT_ERROR_INVALID_STATE error avoiding the null-pointer exception. --- src/cli/cli_udp.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cli/cli_udp.cpp b/src/cli/cli_udp.cpp index 65e774f59d5..fba14f1c075 100644 --- a/src/cli/cli_udp.cpp +++ b/src/cli/cli_udp.cpp @@ -256,6 +256,8 @@ template <> otError UdpExample::Process(Arg aArgs[]) otMessageInfo messageInfo; otMessageSettings messageSettings = {mLinkSecurityEnabled, OT_MESSAGE_PRIORITY_NORMAL}; + VerifyOrExit(otUdpIsOpen(GetInstancePtr(), &mSocket), error = OT_ERROR_INVALID_STATE); + ClearAllBytes(messageInfo); // Possible argument formats: