From f9765f2cc253fefbfa42254c2db8379c5bf92961 Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Mon, 9 Aug 2021 14:03:34 -0700
Subject: [PATCH] [srp-client] config to allow/disallow server switch if host
 is registered (#6896)

This commit adds a new config in `Srp::Client` which specifies whether
the SRP client is allowed to switch server (when auto-start feature
is enabled) on timeouts when host or service info were previously
successfully registered with the currently selected server. Note that
independent of this config, auto-start will always switch and select
a new server when the entry in the Network Data corresponding to the
currently selected server gets removed.
---
 src/core/config/srp_client.h | 24 ++++++++++++++++++++++++
 src/core/net/srp_client.cpp  | 18 +++++++++++-------
 src/core/net/srp_client.hpp  |  4 +++-
 3 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/src/core/config/srp_client.h b/src/core/config/srp_client.h
index 28ea42cc7..e13ed7d78 100644
--- a/src/core/config/srp_client.h
+++ b/src/core/config/srp_client.h
@@ -85,6 +85,30 @@
 #define OPENTHREAD_CONFIG_SRP_CLIENT_SWITCH_SERVER_ON_FAILURE 1
 #endif
 
+/**
+ * @def OPENTHREAD_CONFIG_SRP_CLIENT_DISALLOW_SERVER_SWITCH_WITH_REGISTERED_HOST
+ *
+ * Define to 1 to disallow SRP client to switch server (when auto-start is used) on failure if host (or any services)
+ * were previously successfully registered with the currently selected server.
+ *
+ * This config is only applicable when `OPENTHREAD_CONFIG_SRP_CLIENT_SWITCH_SERVER_ON_FAILURE` is enabled.
+ *
+ * If this is enabled, once SRP client successfully registers with an auto-start selected SRP server, it stays with
+ * the selected SRP server (even if future SRP updates fail or timeout) while the related server entry remains present
+ * in the Thread Network Data. If the entry is removed from Network Data, then SRP client will try to select a new
+ * server.
+ *
+ * Enabling this behavior would be useful allowing more aggressive switch logic (e.g., switch after fewer timeouts
+ * `OPENTHREAD_CONFIG_SRP_CLIENT_MAX_TIMEOUT_FAILURES_TO_SWITCH_SERVER`) while not allowing switch due to temporary
+ * failures on the currently selected server. This can help avoid situation where the old server may still remember the
+ * registered host/service info and client switching to a new server then failing due to name conflict (when new server
+ * probes for the host/service name original server would not allow new registration).
+ *
+ */
+#ifndef OPENTHREAD_CONFIG_SRP_CLIENT_DISALLOW_SERVER_SWITCH_WITH_REGISTERED_HOST
+#define OPENTHREAD_CONFIG_SRP_CLIENT_DISALLOW_SERVER_SWITCH_WITH_REGISTERED_HOST 1
+#endif
+
 /**
  * @def OPENTHREAD_CONFIG_SRP_CLIENT_MAX_TIMEOUT_FAILURES_TO_SWITCH_SERVER
  *
diff --git a/src/core/net/srp_client.cpp b/src/core/net/srp_client.cpp
index 66f6701f8..ac9e12856 100644
--- a/src/core/net/srp_client.cpp
+++ b/src/core/net/srp_client.cpp
@@ -1251,7 +1251,7 @@ void Client::ProcessResponse(Message &aMessage)
             // running and auto-start is enabled and selected the
             // server.
 
-            SelectNextServer();
+            SelectNextServer(/* aDisallowSwitchOnRegisteredHost */ true);
         }
 #endif
         ExitNow(error = kErrorNone);
@@ -1633,7 +1633,7 @@ void Client::HandleTimer(void)
 
         if (mTimoutFailureCount >= kMaxTimeoutFailuresToSwitchServer)
         {
-            SelectNextServer();
+            SelectNextServer(kDisallowSwitchOnRegisteredHost);
         }
 #endif
         break;
@@ -1769,7 +1769,7 @@ exit:
 }
 
 #if OPENTHREAD_CONFIG_SRP_CLIENT_SWITCH_SERVER_ON_FAILURE
-void Client::SelectNextServer(void)
+void Client::SelectNextServer(bool aDisallowSwitchOnRegisteredHost)
 {
     // This method tries to find the next server info entry in the
     // Network Data after the current one selected. If found, it
@@ -1782,12 +1782,16 @@ void Client::SelectNextServer(void)
     serverSockAddr.Clear();
 
     // Ensure that client is running, auto-start is enabled and
-    // auto-start selected the server and that host info is not yet
-    // registered (indicating that no service has yet been registered
-    // either).
+    // auto-start selected the server.
 
     VerifyOrExit(IsRunning() && mAutoStartModeEnabled && mAutoStartDidSelectServer);
-    VerifyOrExit((mHostInfo.GetState() == kAdding) || (mHostInfo.GetState() == kToAdd));
+
+    if (aDisallowSwitchOnRegisteredHost)
+    {
+        // Ensure that host info is not yet registered (indicating that no
+        // service has yet been registered either).
+        VerifyOrExit((mHostInfo.GetState() == kAdding) || (mHostInfo.GetState() == kToAdd));
+    }
 
     // We go through all entries to find the one matching the currently
     // selected one, then set `selectNext` to `true` so to select the
diff --git a/src/core/net/srp_client.hpp b/src/core/net/srp_client.hpp
index b24d0caa8..87eeddf64 100644
--- a/src/core/net/srp_client.hpp
+++ b/src/core/net/srp_client.hpp
@@ -750,6 +750,8 @@ private:
     };
 
     static constexpr bool kAutoStartDefaultMode = OPENTHREAD_CONFIG_SRP_CLIENT_AUTO_START_DEFAULT_MODE;
+    static constexpr bool kDisallowSwitchOnRegisteredHost =
+        OPENTHREAD_CONFIG_SRP_CLIENT_DISALLOW_SERVER_SWITCH_WITH_REGISTERED_HOST;
 
     // Port number to use when server is discovered using "network data anycast service".
     static constexpr uint16_t kAnycastServerPort = 53;
@@ -824,7 +826,7 @@ private:
 #if OPENTHREAD_CONFIG_SRP_CLIENT_AUTO_START_API_ENABLE
     void ProcessAutoStart(void);
 #if OPENTHREAD_CONFIG_SRP_CLIENT_SWITCH_SERVER_ON_FAILURE
-    void SelectNextServer(void);
+    void SelectNextServer(bool aDisallowSwitchOnRegisteredHost);
 #endif
 #endif