From 2bc87ce157e87fc0c98aff18a791cbaa588c37f3 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Sun, 19 Jun 2022 11:23:30 -0700 Subject: [PATCH] [netdata] simplify `Publisher` to use `HandleServerDataUpdated()` (#7823) This commit simplifies `NetworkData::Publisher` to rely and use the new `HandleServerDataUpdated()` behavior which uses a `Tasklet` to start the registration with leader. With this, we can directly call `HandleServerDataUpdated()` whenever there is a change and even multiple times during processing from `Publisher` while ensuring that all changes are still registered together. --- src/core/thread/network_data_publisher.cpp | 141 +++++---------------- src/core/thread/network_data_publisher.hpp | 20 +-- 2 files changed, 45 insertions(+), 116 deletions(-) diff --git a/src/core/thread/network_data_publisher.cpp b/src/core/thread/network_data_publisher.cpp index 5f46008d6..af25ee8f2 100644 --- a/src/core/thread/network_data_publisher.cpp +++ b/src/core/thread/network_data_publisher.cpp @@ -202,15 +202,8 @@ void Publisher::NotifyPrefixEntryChange(Event aEvent, const Ip6::Prefix &aPrefix void Publisher::HandleNotifierEvents(Events aEvents) { - OT_UNUSED_VARIABLE(aEvents); - - bool registerWithLeader = false; - #if OPENTHREAD_CONFIG_TMF_NETDATA_SERVICE_ENABLE - if (mDnsSrpServiceEntry.HandleNotifierEvents(aEvents)) - { - registerWithLeader = true; - } + mDnsSrpServiceEntry.HandleNotifierEvents(aEvents); #endif #if OPENTHREAD_CONFIG_BORDER_ROUTER_ENABLE @@ -219,11 +212,6 @@ void Publisher::HandleNotifierEvents(Events aEvents) entry.HandleNotifierEvents(aEvents); } #endif - - if (registerWithLeader) - { - Get().HandleServerDataUpdated(); - } } void Publisher::HandleTimer(Timer &aTimer) @@ -233,29 +221,16 @@ void Publisher::HandleTimer(Timer &aTimer) void Publisher::HandleTimer(void) { - bool registerWithLeader = false; - #if OPENTHREAD_CONFIG_TMF_NETDATA_SERVICE_ENABLE - if (mDnsSrpServiceEntry.HandleTimer()) - { - registerWithLeader = true; - } + mDnsSrpServiceEntry.HandleTimer(); #endif #if OPENTHREAD_CONFIG_BORDER_ROUTER_ENABLE for (PrefixEntry &entry : mPrefixEntries) { - if (entry.HandleTimer()) - { - registerWithLeader = true; - } + entry.HandleTimer(); } #endif - - if (registerWithLeader) - { - Get().HandleServerDataUpdated(); - } } //--------------------------------------------------------------------------------------------------------------------- @@ -364,22 +339,25 @@ void Publisher::Entry::UpdateState(uint8_t aNumEntries, uint8_t aNumPreferredEnt } } -bool Publisher::Entry::HandleTimer(void) +void Publisher::Entry::HandleTimer(void) { - bool registerWithLeader = false; - // Timer is used to delay adding/removing the entry. If we have // reached `mUpdateTime` add or remove the entry. Otherwise, // restart the timer (note that timer can be shared between - // different published entries). This method returns a `bool` - // indicating whether or not anything in local Network Data got - // changed so to notify the leader and register the changes. + // different published entries). VerifyOrExit((GetState() == kAdding) || (GetState() == kRemoving)); if (mUpdateTime <= TimerMilli::GetNow()) { - registerWithLeader = (GetState() == kAdding) ? Add() : Remove(/* aNextState */ kToAdd); + if (GetState() == kAdding) + { + Add(); + } + else + { + Remove(/* aNextState */ kToAdd); + } } else { @@ -387,55 +365,41 @@ bool Publisher::Entry::HandleTimer(void) } exit: - return registerWithLeader; + return; } -bool Publisher::Entry::Add(void) +void Publisher::Entry::Add(void) { - bool registerWithLeader = false; - #if OPENTHREAD_CONFIG_TMF_NETDATA_SERVICE_ENABLE if (Get().IsADnsSrpServiceEntry(*this)) { - registerWithLeader = static_cast(this)->Add(); - ExitNow(); + static_cast(this)->Add(); } #endif #if OPENTHREAD_CONFIG_BORDER_ROUTER_ENABLE if (Get().IsAPrefixEntry(*this)) { - registerWithLeader = static_cast(this)->Add(); - ExitNow(); + static_cast(this)->Add(); } #endif - -exit: - return registerWithLeader; } -bool Publisher::Entry::Remove(State aNextState) +void Publisher::Entry::Remove(State aNextState) { - bool registerWithLeader = false; - #if OPENTHREAD_CONFIG_TMF_NETDATA_SERVICE_ENABLE if (Get().IsADnsSrpServiceEntry(*this)) { - registerWithLeader = static_cast(this)->Remove(aNextState); - ExitNow(); + static_cast(this)->Remove(aNextState); } #endif #if OPENTHREAD_CONFIG_BORDER_ROUTER_ENABLE if (Get().IsAPrefixEntry(*this)) { - registerWithLeader = static_cast(this)->Remove(aNextState); - ExitNow(); + static_cast(this)->Remove(aNextState); } #endif - -exit: - return registerWithLeader; } Publisher::Entry::InfoString Publisher::Entry::ToString(bool aIncludeState) const @@ -542,8 +506,6 @@ void Publisher::DnsSrpServiceEntry::PublishUnicast(uint16_t aPort) void Publisher::DnsSrpServiceEntry::Publish(const Info &aInfo) { - bool registerWithLeader = false; - if (GetState() != kNoEntry) { if (aInfo == mInfo) @@ -552,7 +514,7 @@ void Publisher::DnsSrpServiceEntry::Publish(const Info &aInfo) ExitNow(); } - registerWithLeader = Remove(/* aNextState */ kNoEntry); + Remove(/* aNextState */ kNoEntry); } mInfo = aInfo; @@ -560,33 +522,19 @@ void Publisher::DnsSrpServiceEntry::Publish(const Info &aInfo) Process(); - if (registerWithLeader) - { - Get().HandleServerDataUpdated(); - } - exit: return; } void Publisher::DnsSrpServiceEntry::Unpublish(void) { - bool registerWithLeader; - LogInfo("Unpublishing DNS/SRP service"); - registerWithLeader = Remove(/* aNextState */ kNoEntry); - - if (registerWithLeader) - { - Get().HandleServerDataUpdated(); - } + Remove(/* aNextState */ kNoEntry); } -bool Publisher::DnsSrpServiceEntry::HandleNotifierEvents(Events aEvents) +void Publisher::DnsSrpServiceEntry::HandleNotifierEvents(Events aEvents) { - bool registerWithLeader = false; - if ((GetType() == kTypeUnicastMeshLocalEid) && aEvents.Contains(kEventThreadMeshLocalAddrChanged)) { mInfo.SetAddress(Get().GetMeshLocal64()); @@ -599,7 +547,7 @@ bool Publisher::DnsSrpServiceEntry::HandleNotifierEvents(Events aEvents) Remove(/* aNextState */ kAdding); Add(); - registerWithLeader = true; + Get().HandleServerDataUpdated(); } } @@ -607,16 +555,12 @@ bool Publisher::DnsSrpServiceEntry::HandleNotifierEvents(Events aEvents) { Process(); } - - return registerWithLeader; } -bool Publisher::DnsSrpServiceEntry::Add(void) +void Publisher::DnsSrpServiceEntry::Add(void) { // Adds the service entry to the network data. - bool registerWithLeader = false; - switch (GetType()) { case kTypeAnycast: @@ -635,20 +579,18 @@ bool Publisher::DnsSrpServiceEntry::Add(void) break; } - registerWithLeader = true; + Get().HandleServerDataUpdated(); SetState(kAdded); Notify(kEventEntryAdded); exit: - return registerWithLeader; + return; } -bool Publisher::DnsSrpServiceEntry::Remove(State aNextState) +void Publisher::DnsSrpServiceEntry::Remove(State aNextState) { // Removes the service entry from network data (if it was added). - bool registerWithLeader = false; - VerifyOrExit((GetState() == kAdded) || (GetState() == kRemoving)); switch (GetType()) @@ -668,12 +610,11 @@ bool Publisher::DnsSrpServiceEntry::Remove(State aNextState) break; } - registerWithLeader = true; + Get().HandleServerDataUpdated(); Notify(kEventEntryRemoved); exit: SetState(aNextState); - return registerWithLeader; } void Publisher::DnsSrpServiceEntry::Notify(Event aEvent) const @@ -871,16 +812,9 @@ void Publisher::PrefixEntry::Publish(const ExternalRouteConfig &aConfig) void Publisher::PrefixEntry::Unpublish(void) { - bool registerWithLeader = false; - LogInfo("Unpublishing %s", mPrefix.ToString().AsCString()); - registerWithLeader = Remove(/* aNextState */ kNoEntry); - - if (registerWithLeader) - { - Get().HandleServerDataUpdated(); - } + Remove(/* aNextState */ kNoEntry); } void Publisher::PrefixEntry::HandleNotifierEvents(Events aEvents) @@ -891,12 +825,10 @@ void Publisher::PrefixEntry::HandleNotifierEvents(Events aEvents) } } -bool Publisher::PrefixEntry::Add(void) +void Publisher::PrefixEntry::Add(void) { // Adds the prefix entry to the network data. - bool registerWithLeader = false; - switch (mType) { case kTypeOnMeshPrefix: @@ -908,12 +840,12 @@ bool Publisher::PrefixEntry::Add(void) break; } - registerWithLeader = true; + Get().HandleServerDataUpdated(); SetState(kAdded); Get().NotifyPrefixEntryChange(kEventEntryAdded, mPrefix); exit: - return registerWithLeader; + return; } Error Publisher::PrefixEntry::AddOnMeshPrefix(void) @@ -938,12 +870,10 @@ Error Publisher::PrefixEntry::AddExternalRoute(void) return Get().AddHasRoutePrefix(config); } -bool Publisher::PrefixEntry::Remove(State aNextState) +void Publisher::PrefixEntry::Remove(State aNextState) { // Remove the prefix entry from the network data. - bool registerWithLeader = false; - VerifyOrExit((GetState() == kAdded) || (GetState() == kRemoving)); switch (mType) @@ -957,12 +887,11 @@ bool Publisher::PrefixEntry::Remove(State aNextState) break; } - registerWithLeader = true; + Get().HandleServerDataUpdated(); Get().NotifyPrefixEntryChange(kEventEntryRemoved, mPrefix); exit: SetState(aNextState); - return registerWithLeader; } void Publisher::PrefixEntry::Process(void) diff --git a/src/core/thread/network_data_publisher.hpp b/src/core/thread/network_data_publisher.hpp index 1da84d70f..c52283b9b 100644 --- a/src/core/thread/network_data_publisher.hpp +++ b/src/core/thread/network_data_publisher.hpp @@ -303,15 +303,15 @@ private: const TimeMilli &GetUpdateTime(void) const { return mUpdateTime; } bool IsPreferred(uint16_t aRloc16) const; void UpdateState(uint8_t aNumEntries, uint8_t aNumPreferredEntries, uint8_t aDesiredNumEntries); - bool HandleTimer(void); + void HandleTimer(void); InfoString ToString(bool aIncludeState = true) const; public: bool IsAdded(void) const { return (mState == kAdded); } private: - bool Add(void); - bool Remove(State aNextState); + void Add(void); + void Remove(State aNextState); void LogUpdateTime(void) const; static const char *StateToString(State aState); @@ -331,8 +331,8 @@ private: void PublishUnicast(const Ip6::Address &aAddress, uint16_t aPort); void PublishUnicast(uint16_t aPort); void Unpublish(void); - bool HandleTimer(void) { return Entry::HandleTimer(); } - bool HandleNotifierEvents(Events aEvents); + void HandleTimer(void) { Entry::HandleTimer(); } + void HandleNotifierEvents(Events aEvents); private: static constexpr uint8_t kDesiredNumAnycast = @@ -374,8 +374,8 @@ private: Type GetType(void) const { return mInfo.GetType(); } void Publish(const Info &aInfo); - bool Add(void); - bool Remove(State aNextState); + void Add(void); + void Remove(State aNextState); void Notify(Event aEvent) const; void Process(void); void CountAnycastEntries(uint8_t &aNumEntries, uint8_t &aNumPreferredEntries) const; @@ -402,7 +402,7 @@ private: void Publish(const OnMeshPrefixConfig &aConfig); void Publish(const ExternalRouteConfig &aConfig); void Unpublish(void); - bool HandleTimer(void) { return Entry::HandleTimer(); } + void HandleTimer(void) { Entry::HandleTimer(); } void HandleNotifierEvents(Events aEvents); private: @@ -418,10 +418,10 @@ private: kTypeExternalRoute, }; - bool Add(void); + void Add(void); Error AddOnMeshPrefix(void); Error AddExternalRoute(void); - bool Remove(State aNextState); + void Remove(State aNextState); void Process(void); void CountOnMeshPrefixEntries(uint8_t &aNumEntries, uint8_t &aNumPreferredEntries) const; void CountExternalRouteEntries(uint8_t &aNumEntries, uint8_t &aNumPreferredEntries) const;