[srp-client] enhance single service mode (#11634)

This commit enhances the `Srp::Client` "single service mode". This
mode is enabled when a prepared SRP update message exceeds the IPv6
MTU size. In this mode, the client registers its services one by one,
with each SRP update containing only a single service.

The implementation is simplified by changing the `mSingleServiceMode`
flag from a persistent member variable of the `Client` class to a
field within the `MsgInfo` struct, making its scope
message-specific.

State transitions are now correctly applied to host and service
entries when operating in single service mode. This, in turn, helps
ensure that SRP message transaction IDs are managed correctly: the
same ID is used for retries of an unchanged service, while a new ID
is used if the service information has changed.

Finally, a new test case, `TestSrpClientSingleServiceMode`, is added
to `test_srp_server` to cover this behavior and its associated retry
logic in detail.

src/core/net/srp_client.cpp

@@ -350,7 +350,6 @@ Client::Client(Instance &aInstance)
     , mState(kStateStopped)
     , mTxFailureRetryCount(0)
     , mShouldRemoveKeyLease(false)
-    , mSingleServiceMode(false)
 #if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
     , mServiceKeyRecordEnabled(false)
     , mUseShortLeaseOption(false)
@@ -453,8 +452,6 @@ void Client::Stop(Requester aRequester, StopMode aMode)
 
     VerifyOrExit(GetState() != kStateStopped);
 
-    mSingleServiceMode = false;
-
     // State changes:
     //   kAdding     -> kToRefresh
     //   kRefreshing -> kToRefresh
@@ -517,8 +514,6 @@ void Client::Pause(void)
         /* (7) kRemoved    -> */ kRemoved,
     };
 
-    mSingleServiceMode = false;
-
     // State changes:
     //   kAdding     -> kToRefresh
     //   kRefreshing -> kToRefresh
@@ -986,6 +981,17 @@ void Client::SendUpdate(void)
         /* (7) kRemoved    -> */ kRemoved,
     };
 
+    static const ItemState kNewStateOnSingleServiceMode[]{
+        /* (0) kToAdd      -> */ kToAdd,
+        /* (1) kAdding     -> */ kToRefresh,
+        /* (2) kToRefresh  -> */ kToRefresh,
+        /* (3) kRefreshing -> */ kToRefresh,
+        /* (4) kToRemove   -> */ kToRemove,
+        /* (5) kRemoving   -> */ kToRemove,
+        /* (6) kRegistered -> */ kRegistered,
+        /* (7) kRemoved    -> */ kRemoved,
+    };
+
     Error    error = kErrorNone;
     MsgInfo  info;
     uint32_t length;
@@ -994,6 +1000,7 @@ void Client::SendUpdate(void)
     info.mMessage.Reset(mSocket.NewMessage());
     VerifyOrExit(info.mMessage != nullptr, error = kErrorNoBufs);
 
+    info.mSingleServiceMode = false;
     SuccessOrExit(error = PrepareUpdateMessage(info));
 
     length = info.mMessage->GetLength() + sizeof(Ip6::Udp::Header) + sizeof(Ip6::Header);
@@ -1001,9 +1008,17 @@ void Client::SendUpdate(void)
     if (length >= Ip6::kMaxDatagramLength)
     {
         LogInfo("Msg len %lu is larger than MTU, enabling single service mode", ToUlong(length));
-        mSingleServiceMode = true;
+
-        SelectNewMessageId();
+        info.mSingleServiceMode = true;
         IgnoreError(info.mMessage->SetLength(0));
+
+        // State changes:
+        //   kAdding     -> kToRefresh
+        //   kRefreshing -> kToRefresh
+        //   kRemoving   -> kToRemove
+
+        ChangeHostAndServiceStates(kNewStateOnSingleServiceMode, kForServicesAppendedInMessage);
+
         SuccessOrExit(error = PrepareUpdateMessage(info));
     }
 
@@ -1056,8 +1071,6 @@ exit:
 
         LogInfo("Failed to send update: %s", ErrorToString(error));
 
-        mSingleServiceMode = false;
-
         SetState(kStateToRetry);
 
         if (mTxFailureRetryCount < kMaxTxFailureRetries)
@@ -1282,7 +1295,7 @@ Error Client::AppendServiceInstructions(MsgInfo &aInfo)
         {
             SuccessOrExit(error = AppendServiceInstruction(service, aInfo));
 
-            if (mSingleServiceMode)
+            if (aInfo.mSingleServiceMode)
             {
                 // In "single service mode", we allow only one service
                 // to be appended in the message.
@@ -1291,7 +1304,7 @@ Error Client::AppendServiceInstructions(MsgInfo &aInfo)
         }
     }
 
-    if (!mSingleServiceMode)
+    if (!aInfo.mSingleServiceMode)
     {
         for (Service &service : mServices)
         {
@@ -2193,7 +2206,6 @@ void Client::HandleTimer(void)
         break;
 
     case kStateUpdating:
-        mSingleServiceMode = false;
         LogRetryWaitInterval();
         LogInfo("Timed out, no response");
         GrowRetryWaitInterval();

src/core/net/srp_client.hpp

@@ -990,6 +990,7 @@ private:
         static constexpr uint16_t kUnknownOffset = 0;
 
         OwnedPtr<Message> mMessage;
+        bool              mSingleServiceMode;
         uint16_t          mDomainNameOffset;
         uint16_t          mHostNameOffset;
         uint16_t          mRecordCount;
@@ -1074,7 +1075,6 @@ private:
     State   mState;
     uint8_t mTxFailureRetryCount : 4;
     bool    mShouldRemoveKeyLease : 1;
-    bool    mSingleServiceMode : 1;
 #if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
     bool mServiceKeyRecordEnabled : 1;
     bool mUseShortLeaseOption : 1;

tests/unit/test_srp_server.cpp

@@ -1030,6 +1030,7 @@ void TestUpdateLeaseShortVariant(void)
 static uint16_t         sServerRxCount;
 static Ip6::MessageInfo sServerMsgInfo;
 static uint16_t         sServerLastMsgId;
+static uint16_t         sServerLastMsgLength;
 
 void HandleServerUdpReceive(void *aContext, otMessage *aMessage, const otMessageInfo *aMessageInfo)
 {
@@ -1041,11 +1042,12 @@ void HandleServerUdpReceive(void *aContext, otMessage *aMessage, const otMessage
 
     SuccessOrQuit(AsCoreType(aMessage).Read(0, header));
 
-    sServerMsgInfo   = AsCoreType(aMessageInfo);
+    sServerMsgInfo       = AsCoreType(aMessageInfo);
-    sServerLastMsgId = header.GetMessageId();
+    sServerLastMsgId     = header.GetMessageId();
+    sServerLastMsgLength = AsCoreType(aMessage).GetLength();
     sServerRxCount++;
 
-    Log("HandleServerUdpReceive(), message-id: 0x%x", header.GetMessageId());
+    Log("HandleServerUdpReceive(), message-id:0x%x, message-len:%u", sServerLastMsgId, sServerLastMsgLength);
 }
 
 void TestSrpClientDelayedResponse(void)
@@ -1198,6 +1200,229 @@ void TestSrpClientDelayedResponse(void)
     Log("End of TestSrpClientDelayedResponse");
 }
 
+void TestSrpClientSingleServiceMode(void)
+{
+    static constexpr uint16_t kNumServices = 5;
+    static constexpr uint16_t kServerPort  = 53535;
+
+    static const char *kSubLabels[] = {"_longsubtypelebel11111", "_longsubtypelebel2222222", nullptr};
+
+    Srp::Client           *srpClient;
+    Srp::Client::Service   services[kNumServices];
+    Dns::Name::LabelBuffer serviceInstnaces[kNumServices];
+
+    Log("--------------------------------------------------------------------------------------------");
+    Log("TestSrpClientSingleServiceMode");
+
+    InitTest();
+
+    srpClient = &sInstance->Get<Srp::Client>();
+
+    {
+        //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+        // Prepare a socket to act as SRP server.
+
+        Ip6::Udp::Socket  udpSocket(*sInstance, HandleServerUdpReceive, nullptr);
+        Ip6::SockAddr     serverSockAddr;
+        uint16_t          firstMsgId;
+        uint16_t          secondMsgId;
+        uint16_t          firstMsgLength;
+        uint16_t          numServices;
+        Message          *response;
+        Dns::UpdateHeader header;
+
+        sServerRxCount = 0;
+
+        SuccessOrQuit(udpSocket.Open(Ip6::kNetifThreadInternal));
+        SuccessOrQuit(udpSocket.Bind(kServerPort));
+
+        serverSockAddr.SetAddress(sInstance->Get<Mle::Mle>().GetMeshLocalRloc());
+        serverSockAddr.SetPort(kServerPort);
+        SuccessOrQuit(srpClient->Start(serverSockAddr));
+
+        //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+        // Prepare five services with long service names and multiple sub-types.
+
+        for (uint16_t i = 0; i < GetArrayLength(services); i++)
+        {
+            StringWriter writer(serviceInstnaces[i], sizeof(Dns::Name::LabelBuffer));
+
+            writer.Append("IncrediblyLongServiceInstanceName-001122334455667788-%02X", i);
+
+            ClearAllBytes(services[i]);
+            services[i].mName          = "_longsrvname._udp";
+            services[i].mInstanceName  = serviceInstnaces[i];
+            services[i].mSubTypeLabels = kSubLabels;
+            services[i].mTxtEntries    = nullptr;
+            services[i].mNumTxtEntries = 0;
+            services[i].mPort          = 5536 + i;
+        }
+
+        //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+        // Register four services (they should still fit in an IPv6 MTU).
+
+        SuccessOrQuit(srpClient->SetHostName("SuperLongHostNameAABBCCDDEEFF001122334455667788990123457889"));
+        SuccessOrQuit(srpClient->EnableAutoHostAddress());
+
+        SuccessOrQuit(srpClient->AddService(services[0]));
+        SuccessOrQuit(srpClient->AddService(services[1]));
+        SuccessOrQuit(srpClient->AddService(services[2]));
+        SuccessOrQuit(srpClient->AddService(services[3]));
+
+        //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+        // Wait for a short time for the server to receive the SRP
+        // update from the client. Verify that the message is smaller
+        // than the IPv6 MTU and includes all services.
+
+        AdvanceTime(1 * 1000);
+
+        VerifyOrQuit(sServerRxCount == 1);
+        firstMsgId     = sServerLastMsgId;
+        firstMsgLength = sServerLastMsgLength;
+        sServerRxCount = 0;
+
+        VerifyOrQuit(services[0].GetState() == Srp::Client::kAdding);
+        VerifyOrQuit(services[1].GetState() == Srp::Client::kAdding);
+        VerifyOrQuit(services[2].GetState() == Srp::Client::kAdding);
+        VerifyOrQuit(services[3].GetState() == Srp::Client::kAdding);
+
+        //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+        // Wait longer to allow the client to retry multiple times, and
+        // ensure the same ID is used for all retries.
+
+        AdvanceTime(60 * 1000);
+        VerifyOrQuit(sServerRxCount > 1);
+        VerifyOrQuit(sServerLastMsgId == firstMsgId);
+
+        VerifyOrQuit(services[0].GetState() == Srp::Client::kAdding);
+        VerifyOrQuit(services[1].GetState() == Srp::Client::kAdding);
+        VerifyOrQuit(services[2].GetState() == Srp::Client::kAdding);
+        VerifyOrQuit(services[3].GetState() == Srp::Client::kAdding);
+
+        sServerRxCount = 0;
+
+        //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+        // Register a fifth service, causing the SRP update to exceed
+        // the MTU limit. The client should now enter "single service
+        // mode" and register services one by one.
+
+        SuccessOrQuit(srpClient->AddService(services[4]));
+
+        AdvanceTime(60 * 1000);
+        VerifyOrQuit(sServerRxCount > 1);
+        VerifyOrQuit(sServerLastMsgId != firstMsgId);
+        VerifyOrQuit(sServerLastMsgLength < firstMsgLength);
+
+        secondMsgId = sServerLastMsgId;
+
+        // Check that only one service is included in the message.
+
+        numServices = 0;
+
+        for (const Srp::Client::Service &service : services)
+        {
+            switch (service.GetState())
+            {
+            case Srp::Client::kToAdd:
+            case Srp::Client::kToRefresh:
+                break;
+
+            case Srp::Client::kAdding:
+            case Srp::Client::kRefreshing:
+                numServices++;
+                break;
+
+            default:
+                VerifyOrQuit(false);
+            }
+        }
+
+        VerifyOrQuit(numServices == 1);
+
+        //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+        // Now send a response from server accepting the registration.
+
+        response = udpSocket.NewMessage();
+        VerifyOrQuit(response != nullptr);
+
+        Log("Sending response with msg-id: 0x%x", secondMsgId);
+
+        header.SetMessageId(secondMsgId);
+        header.SetType(Dns::UpdateHeader::kTypeResponse);
+        header.SetResponseCode(Dns::UpdateHeader::kResponseSuccess);
+        SuccessOrQuit(response->Append(header));
+        SuccessOrQuit(udpSocket.SendTo(*response, sServerMsgInfo));
+
+        sServerRxCount = 0;
+        AdvanceTime(10);
+
+        // Check that exactly one service is successfully
+        // registered.
+
+        numServices = 0;
+
+        for (const Srp::Client::Service &service : services)
+        {
+            switch (service.GetState())
+            {
+            case Srp::Client::kToAdd:
+            case Srp::Client::kToRefresh:
+            case Srp::Client::kAdding:
+            case Srp::Client::kRefreshing:
+                break;
+            case Srp::Client::kRegistered:
+                numServices++;
+                break;
+
+            default:
+                VerifyOrQuit(false);
+            }
+        }
+
+        VerifyOrQuit(numServices == 1);
+
+        //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+        // Wait for the client to register the remaining services and
+        // validate that it used a new message ID.
+
+        AdvanceTime(60 * 1000);
+
+        VerifyOrQuit(sServerRxCount > 1);
+        VerifyOrQuit(sServerLastMsgId != secondMsgId);
+
+        // Check that all remaining services are included in
+        // the message.
+
+        numServices = 0;
+
+        for (const Srp::Client::Service &service : services)
+        {
+            switch (service.GetState())
+            {
+            case Srp::Client::kAdding:
+            case Srp::Client::kRefreshing:
+                break;
+            case Srp::Client::kRegistered:
+                numServices++;
+                break;
+
+            default:
+                VerifyOrQuit(false);
+            }
+        }
+
+        VerifyOrQuit(numServices == 1);
+    }
+
+    //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+    // Finalize OT instance
+
+    Log("Finalizing OT instance");
+    FinalizeTest();
+
+    Log("End of TestSrpClientSingleServiceMode");
+}
+
 #endif // OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
 
 void TestSrpServerAddressModeForceAdd(void)
@@ -1399,6 +1624,7 @@ int main(void)
 #if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
     ot::TestUpdateLeaseShortVariant();
     ot::TestSrpClientDelayedResponse();
+    ot::TestSrpClientSingleServiceMode();
 #endif
     ot::TestSrpServerAddressModeForceAdd();
 #if OPENTHREAD_CONFIG_SRP_SERVER_FAST_START_MODE_ENABLE