From 3043f2e1967df2f05f1ae1a210fbc2833ead6f11 Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Tue, 3 Oct 2023 15:46:37 -0700
Subject: [PATCH] [bbr-local] simplify `AddService()` and its use (#9477)

This commit contains smaller changes in `BackbonRouter::Local`:

- `AddService()` now gets a `RegisterationMode` enum as its input
  either decide based on current state or force registration.
- We remove the `NetworkData::Notifier::HandleServerDataUpdated()`
  calls after calling `AddService()` since `AddService()` method
  itself will already call this (when successfully adds the service
  to Network Data)
- We also remove the extra state check before calling `AddService()`
  as it is done by the `AddService()` method itself.
---
 src/core/api/backbone_router_ftd_api.cpp |  2 +-
 src/core/backbone_router/bbr_local.cpp   | 25 ++++++++++++------------
 src/core/backbone_router/bbr_local.hpp   | 16 +++++++++++----
 src/core/thread/mle_router.cpp           |  9 +--------
 4 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/src/core/api/backbone_router_ftd_api.cpp b/src/core/api/backbone_router_ftd_api.cpp
index e24c642ef..bd3a3c350 100644
--- a/src/core/api/backbone_router_ftd_api.cpp
+++ b/src/core/api/backbone_router_ftd_api.cpp
@@ -69,7 +69,7 @@ otError otBackboneRouterSetConfig(otInstance *aInstance, const otBackboneRouterC
 
 otError otBackboneRouterRegister(otInstance *aInstance)
 {
-    return AsCoreType(aInstance).Get<BackboneRouter::Local>().AddService(true /* Force registration */);
+    return AsCoreType(aInstance).Get<BackboneRouter::Local>().AddService(BackboneRouter::Local::kForceRegistration);
 }
 
 uint8_t otBackboneRouterGetRegistrationJitter(otInstance *aInstance)
diff --git a/src/core/backbone_router/bbr_local.cpp b/src/core/backbone_router/bbr_local.cpp
index 430ac906c..a6dae0971 100644
--- a/src/core/backbone_router/bbr_local.cpp
+++ b/src/core/backbone_router/bbr_local.cpp
@@ -87,7 +87,7 @@ void Local::SetEnabled(bool aEnable)
     {
         SetState(kStateSecondary);
         AddDomainPrefixToNetworkData();
-        IgnoreError(AddService());
+        IgnoreError(AddService(kDecideBasedOnState));
     }
     else
     {
@@ -163,7 +163,7 @@ Error Local::SetConfig(const Config &aConfig)
     {
         Get<Notifier>().Signal(kEventThreadBackboneRouterLocalChanged);
 
-        IgnoreError(AddService());
+        IgnoreError(AddService(kDecideBasedOnState));
     }
 
 exit:
@@ -171,18 +171,22 @@ exit:
     return error;
 }
 
-Error Local::AddService(bool aForce)
+Error Local::AddService(RegisterMode aMode)
 {
     Error                                            error = kErrorInvalidState;
     NetworkData::Service::BackboneRouter::ServerData serverData;
 
     VerifyOrExit(mState != kStateDisabled && Get<Mle::Mle>().IsAttached());
 
-    VerifyOrExit(aForce /* if register by force */ ||
-                 !Get<BackboneRouter::Leader>().HasPrimary() /* if no available Backbone Router service */ ||
-                 Get<BackboneRouter::Leader>().GetServer16() == Get<Mle::MleRouter>().GetRloc16()
-                 /* If the device itself should be BBR. */
-    );
+    switch (aMode)
+    {
+    case kDecideBasedOnState:
+        VerifyOrExit(!Get<BackboneRouter::Leader>().HasPrimary() ||
+                     Get<BackboneRouter::Leader>().GetServer16() == Get<Mle::MleRouter>().GetRloc16());
+        break;
+    case kForceRegistration:
+        break;
+    }
 
     serverData.SetSequenceNumber(mSequenceNumber);
     serverData.SetReregistrationDelay(mReregistrationDelay);
@@ -272,10 +276,7 @@ void Local::HandleBackboneRouterPrimaryUpdate(Leader::State aState, const Config
         mMlrTimeout          = aConfig.mMlrTimeout;
         SequenceNumberIncrease();
         Get<Notifier>().Signal(kEventThreadBackboneRouterLocalChanged);
-        if (AddService(true /* Force registration to refresh and restore Primary state */) == kErrorNone)
-        {
-            Get<NetworkData::Notifier>().HandleServerDataUpdated();
-        }
+        IgnoreError(AddService(kForceRegistration));
     }
     else
     {
diff --git a/src/core/backbone_router/bbr_local.hpp b/src/core/backbone_router/bbr_local.hpp
index b2ba94e37..a1d470f38 100644
--- a/src/core/backbone_router/bbr_local.hpp
+++ b/src/core/backbone_router/bbr_local.hpp
@@ -84,6 +84,16 @@ public:
         kStatePrimary   = OT_BACKBONE_ROUTER_STATE_PRIMARY,   ///< The Primary Backbone Router.
     };
 
+    /**
+     * Represents registration mode used as input to `AddService()` method.
+     *
+     */
+    enum RegisterMode : uint8_t
+    {
+        kDecideBasedOnState, ///< Decide based on current state.
+        kForceRegistration,  ///< Force registration regardless of current state.
+    };
+
     /**
      * Initializes the local Backbone Router.
      *
@@ -137,16 +147,14 @@ public:
     /**
      * Registers Backbone Router Dataset to Leader.
      *
-     * @param[in]  aForce True to force registration regardless of current state.
-     *                    False to decide based on current state.
-     *
+     * @param[in]  aMode  The registration mode to use (decide based on current state or force registration).
      *
      * @retval kErrorNone            Successfully added the Service entry.
      * @retval kErrorInvalidState    Not in the ready state to register.
      * @retval kErrorNoBufs          Insufficient space to add the Service entry.
      *
      */
-    Error AddService(bool aForce = false);
+    Error AddService(RegisterMode aMode);
 
     /**
      * Indicates whether or not the Backbone Router is Primary.
diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp
index cb96350a2..190db0815 100644
--- a/src/core/thread/mle_router.cpp
+++ b/src/core/thread/mle_router.cpp
@@ -1550,14 +1550,7 @@ void MleRouter::HandleTimeTick(void)
 
         if (mBackboneRouterRegistrationDelay == 0)
         {
-            // If no Backbone Router service after jitter, try to register its own Backbone Router Service.
-            if (!Get<BackboneRouter::Leader>().HasPrimary())
-            {
-                if (Get<BackboneRouter::Local>().AddService() == kErrorNone)
-                {
-                    Get<NetworkData::Notifier>().HandleServerDataUpdated();
-                }
-            }
+            IgnoreError(Get<BackboneRouter::Local>().AddService(BackboneRouter::Local::kDecideBasedOnState));
         }
     }
 #endif