diff --git a/src/core/net/srp_client.cpp b/src/core/net/srp_client.cpp index 572b2ea3b..d43bd52b6 100644 --- a/src/core/net/srp_client.cpp +++ b/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(); diff --git a/src/core/net/srp_client.hpp b/src/core/net/srp_client.hpp index afaeb05ad..0d81411bc 100644 --- a/src/core/net/srp_client.hpp +++ b/src/core/net/srp_client.hpp @@ -990,6 +990,7 @@ private: static constexpr uint16_t kUnknownOffset = 0; OwnedPtr 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; diff --git a/tests/unit/test_srp_server.cpp b/tests/unit/test_srp_server.cpp index d9db36a08..2c5f50943 100644 --- a/tests/unit/test_srp_server.cpp +++ b/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); - sServerLastMsgId = header.GetMessageId(); + sServerMsgInfo = AsCoreType(aMessageInfo); + 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(); + + { + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // 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().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