From f42c814052bebe6e7fffd1f6f3f090bec53c4a53 Mon Sep 17 00:00:00 2001 From: Li Cao Date: Mon, 25 Nov 2024 08:06:14 +0000 Subject: [PATCH] [thread host] Update Leave method --- src/dbus/server/dbus_thread_object_ncp.cpp | 2 +- src/ncp/ncp_host.cpp | 13 +++- src/ncp/ncp_host.hpp | 2 +- src/ncp/rcp_host.cpp | 67 ++++++++++++++++++--- src/ncp/rcp_host.hpp | 16 ++++- src/ncp/thread_host.hpp | 4 +- tests/gtest/test_rcp_host_api.cpp | 69 ++++++++++++++++++++++ 7 files changed, 158 insertions(+), 15 deletions(-) diff --git a/src/dbus/server/dbus_thread_object_ncp.cpp b/src/dbus/server/dbus_thread_object_ncp.cpp index 526a477f6d9..bd869f79b65 100644 --- a/src/dbus/server/dbus_thread_object_ncp.cpp +++ b/src/dbus/server/dbus_thread_object_ncp.cpp @@ -125,7 +125,7 @@ void DBusThreadObjectNcp::JoinHandler(DBusRequest &aRequest) void DBusThreadObjectNcp::LeaveHandler(DBusRequest &aRequest) { - mHost.Leave([aRequest](otError aError, const std::string &aErrorInfo) mutable { + mHost.Leave(true /* aEraseDataset */, [aRequest](otError aError, const std::string &aErrorInfo) mutable { OT_UNUSED_VARIABLE(aErrorInfo); aRequest.ReplyOtResult(aError); }); diff --git a/src/ncp/ncp_host.cpp b/src/ncp/ncp_host.cpp index b37a4a51b98..347de367553 100644 --- a/src/ncp/ncp_host.cpp +++ b/src/ncp/ncp_host.cpp @@ -157,14 +157,23 @@ void NcpHost::Join(const otOperationalDatasetTlvs &aActiveOpDatasetTlvs, const A task->Run(); } -void NcpHost::Leave(const AsyncResultReceiver &aReceiver) +void NcpHost::Leave(bool aEraseDataset, const AsyncResultReceiver &aReceiver) { AsyncTaskPtr task; auto errorHandler = [aReceiver](otError aError, const std::string &aErrorInfo) { aReceiver(aError, aErrorInfo); }; task = std::make_shared(errorHandler); task->First([this](AsyncTaskPtr aNext) { mNcpSpinel.ThreadDetachGracefully(std::move(aNext)); }) - ->Then([this](AsyncTaskPtr aNext) { mNcpSpinel.ThreadErasePersistentInfo(std::move(aNext)); }); + ->Then([this, aEraseDataset](AsyncTaskPtr aNext) { + if (aEraseDataset) + { + mNcpSpinel.ThreadErasePersistentInfo(std::move(aNext)); + } + else + { + aNext->SetResult(OT_ERROR_NONE, ""); + } + }); task->Run(); } diff --git a/src/ncp/ncp_host.hpp b/src/ncp/ncp_host.hpp index 9a94f2fd870..3e68a524b59 100644 --- a/src/ncp/ncp_host.hpp +++ b/src/ncp/ncp_host.hpp @@ -91,7 +91,7 @@ class NcpHost : public MainloopProcessor, public ThreadHost, public NcpNetworkPr // ThreadHost methods void Join(const otOperationalDatasetTlvs &aActiveOpDatasetTlvs, const AsyncResultReceiver &aReceiver) override; - void Leave(const AsyncResultReceiver &aReceiver) override; + void Leave(bool aEraseDataset, const AsyncResultReceiver &aReceiver) override; void ScheduleMigration(const otOperationalDatasetTlvs &aPendingOpDatasetTlvs, const AsyncResultReceiver aReceiver) override; void SetThreadEnabled(bool aEnabled, const AsyncResultReceiver aReceiver) override; diff --git a/src/ncp/rcp_host.cpp b/src/ncp/rcp_host.cpp index 0061e29f597..a6377f6cf82 100644 --- a/src/ncp/rcp_host.cpp +++ b/src/ncp/rcp_host.cpp @@ -333,6 +333,7 @@ void RcpHost::Deinit(void) mSetThreadEnabledReceiver = nullptr; mScheduleMigrationReceiver = nullptr; + mDetachGracefullyCallbacks.clear(); } void RcpHost::HandleStateChanged(otChangedFlags aFlags) @@ -445,10 +446,35 @@ void RcpHost::Join(const otOperationalDatasetTlvs &aActiveOpDatasetTlvs, const A mTaskRunner.Post([aReceiver](void) { aReceiver(OT_ERROR_NOT_IMPLEMENTED, "Not implemented!"); }); } -void RcpHost::Leave(const AsyncResultReceiver &aReceiver) +void RcpHost::Leave(bool aEraseDataset, const AsyncResultReceiver &aReceiver) { - // TODO: Implement Leave under RCP mode. - mTaskRunner.Post([aReceiver](void) { aReceiver(OT_ERROR_NOT_IMPLEMENTED, "Not implemented!"); }); + otError error = OT_ERROR_NONE; + std::string errorMsg; + bool receiveResultHere = true; + + VerifyOrExit(mInstance != nullptr, error = OT_ERROR_INVALID_STATE, errorMsg = "OT is not initialized"); + VerifyOrExit(mThreadEnabledState != ThreadEnabledState::kStateDisabling, error = OT_ERROR_BUSY, + errorMsg = "Thread is disabling"); + + if (mThreadEnabledState == ThreadEnabledState::kStateDisabled) + { + ConditionalErasePersistentInfo(aEraseDataset); + ExitNow(); + } + + ThreadDetachGracefully([aEraseDataset, aReceiver, this] { + ConditionalErasePersistentInfo(aEraseDataset); + if (aReceiver) + { + aReceiver(OT_ERROR_NONE, ""); + } + }); + +exit: + if (receiveResultHere) + { + mTaskRunner.Post([aReceiver, error, errorMsg](void) { aReceiver(error, errorMsg); }); + } } void RcpHost::ScheduleMigration(const otOperationalDatasetTlvs &aPendingOpDatasetTlvs, @@ -529,7 +555,7 @@ void RcpHost::SetThreadEnabled(bool aEnabled, const AsyncResultReceiver aReceive { UpdateThreadEnabledState(ThreadEnabledState::kStateDisabling); - SuccessOrExit(error = otThreadDetachGracefully(mInstance, DisableThreadAfterDetach, this)); + ThreadDetachGracefully([this](void) { DisableThreadAfterDetach(); }); mSetThreadEnabledReceiver = aReceiver; receiveResultHere = false; } @@ -537,7 +563,7 @@ void RcpHost::SetThreadEnabled(bool aEnabled, const AsyncResultReceiver aReceive exit: if (receiveResultHere) { - mTaskRunner.Post([aReceiver, error, errorMsg](void) { aReceiver(error, errorMsg); }); + mTaskRunner.Post([aReceiver, error, errorMsg](void) { SafeInvoke(aReceiver, error, errorMsg); }); } } @@ -595,9 +621,36 @@ void RcpHost::SetChannelMaxPowers(const std::vector &aChannelMa } #endif // OTBR_ENABLE_POWER_CALIBRATION -void RcpHost::DisableThreadAfterDetach(void *aContext) +void RcpHost::ThreadDetachGracefully(const DetachGracefullyCallback &aCallback) +{ + mDetachGracefullyCallbacks.push_back(aCallback); + + // Ignores the OT_ERROR_BUSY error if a detach has already been requested + OT_UNUSED_VARIABLE(otThreadDetachGracefully(mInstance, ThreadDetachGracefullyCallback, this)); +} + +void RcpHost::ThreadDetachGracefullyCallback(void *aContext) +{ + static_cast(aContext)->ThreadDetachGracefullyCallback(); +} + +void RcpHost::ThreadDetachGracefullyCallback(void) { - static_cast(aContext)->DisableThreadAfterDetach(); + SafeInvokeAndClear(mScheduleMigrationReceiver, OT_ERROR_ABORT, "Aborted by leave/disable operation"); + + for (auto &callback : mDetachGracefullyCallbacks) + { + callback(); + } + mDetachGracefullyCallbacks.clear(); +} + +void RcpHost::ConditionalErasePersistentInfo(bool aErase) +{ + if (aErase) + { + OT_UNUSED_VARIABLE(otInstanceErasePersistentInfo(mInstance)); + } } void RcpHost::DisableThreadAfterDetach(void) diff --git a/src/ncp/rcp_host.hpp b/src/ncp/rcp_host.hpp index e1f19675cac..8efdbf15895 100644 --- a/src/ncp/rcp_host.hpp +++ b/src/ncp/rcp_host.hpp @@ -199,7 +199,7 @@ class RcpHost : public MainloopProcessor, public ThreadHost, public OtNetworkPro // Thread Control virtual methods void Join(const otOperationalDatasetTlvs &aActiveOpDatasetTlvs, const AsyncResultReceiver &aRecevier) override; - void Leave(const AsyncResultReceiver &aRecevier) override; + void Leave(bool aEraseDataset, const AsyncResultReceiver &aRecevier) override; void ScheduleMigration(const otOperationalDatasetTlvs &aPendingOpDatasetTlvs, const AsyncResultReceiver aReceiver) override; void SetThreadEnabled(bool aEnabled, const AsyncResultReceiver aReceiver) override; @@ -231,6 +231,13 @@ class RcpHost : public MainloopProcessor, public ThreadHost, public OtNetworkPro aReceiver = nullptr; } } + static void SafeInvoke(const AsyncResultReceiver &aReceiver, otError aError, const std::string &aErrorInfo = "") + { + if (aReceiver) + { + aReceiver(aError, aErrorInfo); + } + } static void HandleStateChanged(otChangedFlags aFlags, void *aContext) { @@ -251,7 +258,11 @@ class RcpHost : public MainloopProcessor, public ThreadHost, public OtNetworkPro void HandleBackboneRouterNdProxyEvent(otBackboneRouterNdProxyEvent aEvent, const otIp6Address *aAddress); #endif - static void DisableThreadAfterDetach(void *aContext); + using DetachGracefullyCallback = std::function; + void ThreadDetachGracefully(const DetachGracefullyCallback &aCallback); + static void ThreadDetachGracefullyCallback(void *aContext); + void ThreadDetachGracefullyCallback(void); + void ConditionalErasePersistentInfo(bool aErase); void DisableThreadAfterDetach(void); static void SendMgmtPendingSetCallback(otError aError, void *aContext); void SendMgmtPendingSetCallback(otError aError); @@ -278,6 +289,7 @@ class RcpHost : public MainloopProcessor, public ThreadHost, public OtNetworkPro ThreadEnabledState mThreadEnabledState; AsyncResultReceiver mSetThreadEnabledReceiver; AsyncResultReceiver mScheduleMigrationReceiver; + std::vector mDetachGracefullyCallbacks; #if OTBR_ENABLE_FEATURE_FLAGS // The applied FeatureFlagList in ApplyFeatureFlagList call, used for debugging purpose. diff --git a/src/ncp/thread_host.hpp b/src/ncp/thread_host.hpp index d364a14c739..5348c7c68d6 100644 --- a/src/ncp/thread_host.hpp +++ b/src/ncp/thread_host.hpp @@ -166,13 +166,13 @@ class ThreadHost : virtual public NetworkProperties * be called. * 2. If this device is not in disabled state, OTBR sends Address Release Notification (i.e. ADDR_REL.ntf) * to gracefully detach from the current network and it takes 1 second to finish. - * 3. Then Operational Dataset will be removed from persistent storage. + * 3. Then Operational Dataset will be removed from persistent storage if @p aEraseDataset is true. * 4. If everything goes fine, @p aReceiver will be invoked with OT_ERROR_NONE. Otherwise, other errors * will be passed to @p aReceiver when the error happens. * * @param[in] aReceiver A receiver to get the async result of this operation. */ - virtual void Leave(const AsyncResultReceiver &aRecevier) = 0; + virtual void Leave(bool aEraseDataset, const AsyncResultReceiver &aRecevier) = 0; /** * This method migrates this device to the new network specified by @p aPendingOpDatasetTlvs. diff --git a/tests/gtest/test_rcp_host_api.cpp b/tests/gtest/test_rcp_host_api.cpp index 59e84140c4c..9ba5bd1eb9c 100644 --- a/tests/gtest/test_rcp_host_api.cpp +++ b/tests/gtest/test_rcp_host_api.cpp @@ -194,6 +194,75 @@ TEST(RcpHostApi, SetCountryCodeWorkCorrectly) host.Deinit(); } +TEST(RcpHostApi, StateChangesCorrectlyAfterLeave) +{ + otError error = OT_ERROR_NONE; + std::string errorMsg = ""; + bool resultReceived = false; + otbr::MainloopContext mainloop; + otbr::Ncp::ThreadHost::AsyncResultReceiver receiver = [&resultReceived, &error, + &errorMsg](otError aError, const std::string &aErrorMsg) { + resultReceived = true; + error = aError; + errorMsg = aErrorMsg; + }; + + otbr::Ncp::RcpHost host("wpan0", std::vector(), /* aBackboneInterfaceName */ "", /* aDryRun */ false, + /* aEnableAutoAttach */ false); + + // 1. Call Leave when host hasn't been initialized. + otbr::MainloopManager::GetInstance().RemoveMainloopProcessor( + &host); // Temporarily remove RcpHost because it's not initialized yet. + host.Leave(/* aEraseDataset */ true, receiver); + MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; }); + EXPECT_EQ(error, OT_ERROR_INVALID_STATE); + EXPECT_STREQ(errorMsg.c_str(), "OT is not initialized"); + otbr::MainloopManager::GetInstance().AddMainloopProcessor(&host); + + host.Init(); + + // 2. Call Leave when disabling Thread. + error = OT_ERROR_NONE; + resultReceived = false; + host.SetThreadEnabled(false, nullptr); + host.Leave(/* aEraseDataset */ true, receiver); + MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; }); + EXPECT_EQ(error, OT_ERROR_BUSY); + EXPECT_STREQ(errorMsg.c_str(), "Thread is disabling"); + + // 3. Call Leave when Thread is disabled. + error = OT_ERROR_NONE; + resultReceived = false; + otOperationalDataset dataset; + otOperationalDatasetTlvs datasetTlvs; + OT_UNUSED_VARIABLE(otDatasetCreateNewNetwork(ot::FakePlatform::CurrentInstance(), &dataset)); + otDatasetConvertToTlvs(&dataset, &datasetTlvs); + OT_UNUSED_VARIABLE(otDatasetSetActiveTlvs(ot::FakePlatform::CurrentInstance(), &datasetTlvs)); + host.Leave(/* aEraseDataset */ true, receiver); + MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; }); + EXPECT_EQ(error, OT_ERROR_NONE); + + error = otDatasetGetActive(ot::FakePlatform::CurrentInstance(), &dataset); + EXPECT_EQ(error, OT_ERROR_NOT_FOUND); + + // 4. Call Leave when Thread is enabled. + error = OT_ERROR_NONE; + resultReceived = false; + OT_UNUSED_VARIABLE(otDatasetSetActiveTlvs(ot::FakePlatform::CurrentInstance(), &datasetTlvs)); + host.SetThreadEnabled(true, nullptr); + MainloopProcessUntil(mainloop, /* aTimeoutSec */ 1, + [&host]() { return host.GetDeviceRole() != OT_DEVICE_ROLE_DETACHED; }); + EXPECT_EQ(host.GetDeviceRole(), OT_DEVICE_ROLE_LEADER); + host.Leave(/* aEraseDataset */ false, receiver); + MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; }); + EXPECT_EQ(error, OT_ERROR_NONE); + + error = otDatasetGetActive(ot::FakePlatform::CurrentInstance(), &dataset); // Dataset should still be there. + EXPECT_EQ(error, OT_ERROR_NONE); + + host.Deinit(); +} + TEST(RcpHostApi, StateChangesCorrectlyAfterScheduleMigration) { otError error = OT_ERROR_NONE;