espressif/openthread
1
0
Fork 0
mirror of https://github.com/espressif/openthread.git synced 2026-07-29 23:27:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

[srp-client] use sequential message IDs & allow older response IDs (#10353)

Browse Source 
This commit updates `Srp::Client` to use sequential message IDs,
replacing the previous model where message IDs were generated
randomly.

When processing responses from the server, older message IDs are
accepted as long as the corresponding older message was identical to
the latest one. This helps in situations where the server's response
may be delayed  (e.g., due to network congestion) and the client retry
mechanism is retransmitting the same update message.

This commit also adds a unit test `TestSrpClientDelayedResponse()`
to validate the new behavior.
This commit is contained in:
Abtin Keshavarzian
2024-06-11 08:43:49 -07:00
committed by GitHub
parent d9bb516f0d
commit 7ba7fb2eb6
3 changed files with 276 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/net/srp_client.cpp
+76 -28
View File
@@ -68,13 +68,19 @@ void Client::HostInfo::Clear(void)
SetState(kRemoved);
}
void Client::HostInfo::SetState(ItemState aState)
bool Client::HostInfo::SetState(ItemState aState)
{
if (aState != GetState())
{
LogInfo("HostInfo %s -> %s", ItemStateToString(GetState()), ItemStateToString(aState));
mState = MapEnum(aState);
}
bool didChange;
VerifyOrExit(aState != GetState(), didChange = false);
LogInfo("HostInfo %s -> %s", ItemStateToString(GetState()), ItemStateToString(aState));
mState = MapEnum(aState);
didChange = true;
exit:
return didChange;
}
void Client::HostInfo::EnableAutoAddress(void)
@@ -121,9 +127,11 @@ exit:
return error;
}
void Client::Service::SetState(ItemState aState)
bool Client::Service::SetState(ItemState aState)
{
VerifyOrExit(GetState() != aState);
bool didChange;
VerifyOrExit(GetState() != aState, didChange = false);
LogInfo("Service %s -> %s, \"%s\" \"%s\"", ItemStateToString(GetState()), ItemStateToString(aState),
GetInstanceName(), GetName());
@@ -150,10 +158,11 @@ void Client::Service::SetState(ItemState aState)
GetPriority(), GetNumTxtEntries());
}
mState = MapEnum(aState);
mState = MapEnum(aState);
didChange = true;
exit:
return;
return didChange;
}
bool Client::Service::Matches(const Service &aOther) const
@@ -249,7 +258,8 @@ Client::Client(Instance &aInstance)
, mServiceKeyRecordEnabled(false)
, mUseShortLeaseOption(false)
#endif
, mUpdateMessageId(0)
, mNextMessageId(0)
, mResponseMessageId(0)
, mAutoHostAddressCount(0)
, mRetryWaitInterval(kMinRetryWaitInterval)
, mTtl(0)
@@ -345,6 +355,7 @@ void Client::Stop(Requester aRequester, StopMode aMode)
mShouldRemoveKeyLease = false;
mTxFailureRetryCount = 0;
mResponseMessageId = mNextMessageId;
if (aMode == kResetRetryInterval)
{
@@ -739,13 +750,15 @@ exit:
return;
}
void Client::ChangeHostAndServiceStates(const ItemState *aNewStates, ServiceStateChangeMode aMode)
bool Client::ChangeHostAndServiceStates(const ItemState *aNewStates, ServiceStateChangeMode aMode)
{
bool anyChanged;
#if OPENTHREAD_CONFIG_SRP_CLIENT_AUTO_START_API_ENABLE && OPENTHREAD_CONFIG_SRP_CLIENT_SAVE_SELECTED_SERVER_ENABLE
ItemState oldHostState = mHostInfo.GetState();
#endif
mHostInfo.SetState(aNewStates[mHostInfo.GetState()]);
anyChanged = mHostInfo.SetState(aNewStates[mHostInfo.GetState()]);
for (Service &service : mServices)
{
@@ -754,7 +767,7 @@ void Client::ChangeHostAndServiceStates(const ItemState *aNewStates, ServiceStat
continue;
}
service.SetState(aNewStates[service.GetState()]);
anyChanged |= service.SetState(aNewStates[service.GetState()]);
}
#if OPENTHREAD_CONFIG_SRP_CLIENT_AUTO_START_API_ENABLE && OPENTHREAD_CONFIG_SRP_CLIENT_SAVE_SELECTED_SERVER_ENABLE
@@ -781,6 +794,8 @@ void Client::ChangeHostAndServiceStates(const ItemState *aNewStates, ServiceStat
}
}
#endif // OPENTHREAD_CONFIG_SRP_CLIENT_AUTO_START_API_ENABLE && OPENTHREAD_CONFIG_SRP_CLIENT_SAVE_SELECTED_SERVER_ENABLE
return anyChanged;
}
void Client::InvokeCallback(Error aError) const { InvokeCallback(aError, mHostInfo, nullptr); }
@@ -806,6 +821,7 @@ void Client::SendUpdate(void)
Error error = kErrorNone;
Message *message = mSocket.NewMessage();
uint32_t length;
bool anyChanged;
VerifyOrExit(message != nullptr, error = kErrorNoBufs);
SuccessOrExit(error = PrepareUpdateMessage(*message));
@@ -822,14 +838,35 @@ void Client::SendUpdate(void)
SuccessOrExit(error = mSocket.SendTo(*message, Ip6::MessageInfo()));
LogInfo("Send update");
LogInfo("Send update, msg-id:0x%x", mNextMessageId);
// State changes:
// kToAdd -> kAdding
// kToRefresh -> kRefreshing
// kToRemove -> kRemoving
ChangeHostAndServiceStates(kNewStateOnMessageTx, kForServicesAppendedInMessage);
anyChanged = ChangeHostAndServiceStates(kNewStateOnMessageTx, kForServicesAppendedInMessage);
// `mNextMessageId` tracks the message ID used in the prepared
// update message. It is incremented after a successful
// `mSocket.SendTo()` call. If unsuccessful, the same ID can be
// reused for the next update.
//
// Acceptable response message IDs fall within the range starting
// at `mResponseMessageId ` and ending before `mNextMessageId`.
//
// `anyChanged` tracks if any host or service states have changed.
// If not, the prepared message is identical to the last one with
// the same hosts/services, allowing us to accept earlier message
// IDs. If changes occur, `mResponseMessageId ` is updated to
// ensure only responses to the latest message are accepted.
if (anyChanged)
{
mResponseMessageId = mNextMessageId;
}
mNextMessageId++;
// Remember the update message tx time to use later to determine the
// lease renew time.
@@ -902,13 +939,7 @@ Error Client::PrepareUpdateMessage(Message &aMessage)
SuccessOrExit(error = ReadOrGenerateKey(info.mKeyPair));
#endif
// Generate random Message ID and ensure it is different from last one
do
{
SuccessOrExit(error = header.SetRandomMessageId());
} while (header.GetMessageId() == mUpdateMessageId);
mUpdateMessageId = header.GetMessageId();
header.SetMessageId(mNextMessageId);
// SRP Update (DNS Update) message must have exactly one record in
// Zone section, no records in Prerequisite Section, can have
@@ -1578,22 +1609,32 @@ void Client::ProcessResponse(Message &aMessage)
uint16_t recordCount;
LinkedList<Service> removedServices;
VerifyOrExit(GetState() == kStateUpdating);
switch (GetState())
{
case kStateToUpdate:
case kStateUpdating:
case kStateToRetry:
break;
case kStateStopped:
case kStatePaused:
case kStateUpdated:
ExitNow();
}
SuccessOrExit(error = aMessage.Read(offset, header));
VerifyOrExit(header.GetType() == Dns::Header::kTypeResponse, error = kErrorParse);
VerifyOrExit(header.GetQueryType() == Dns::Header::kQueryTypeUpdate, error = kErrorParse);
VerifyOrExit(header.GetMessageId() == mUpdateMessageId, error = kErrorDrop);
VerifyOrExit(IsResponseMessageIdValid(header.GetMessageId()), error = kErrorDrop);
mResponseMessageId = header.GetMessageId() + 1;
if (!Get<Mle::Mle>().IsRxOnWhenIdle())
{
Get<DataPollSender>().StopFastPolls();
}
// Response is for the earlier request message.
LogInfo("Received response");
LogInfo("Received response, msg-id:0x%x", header.GetMessageId());
#if OPENTHREAD_CONFIG_SRP_CLIENT_AUTO_START_API_ENABLE && OPENTHREAD_CONFIG_SRP_CLIENT_SWITCH_SERVER_ON_FAILURE
mAutoStart.ResetTimeoutFailureCount();
@@ -1722,6 +1763,13 @@ exit:
}
}
bool Client::IsResponseMessageIdValid(uint16_t aId) const
{
// Semantically equivalent to `(aId >= mResponseMessageId) && (aId < mNextMessageId)`
return !SerialNumber::IsLess(aId, mResponseMessageId) && SerialNumber::IsLess(aId, mNextMessageId);
}
void Client::HandleUpdateDone(void)
{
HostInfo hostInfoCopy = mHostInfo;
src/core/net/srp_client.hpp
+17 -4
View File
@@ -167,7 +167,7 @@ public:
private:
void SetName(const char *aName) { mName = aName; }
void SetState(ItemState aState);
bool SetState(ItemState aState);
void SetAddresses(const Ip6::Address *aAddresses, uint8_t aNumAddresses);
void EnableAutoAddress(void);
};
@@ -303,7 +303,7 @@ public:
static constexpr uint32_t kAppendedInMsgFlag = (1U << 31);
static constexpr uint32_t kLeaseMask = ~kAppendedInMsgFlag;
void SetState(ItemState aState);
bool SetState(ItemState aState);
TimeMilli GetLeaseRenewTime(void) const { return TimeMilli(mData); }
void SetLeaseRenewTime(TimeMilli aTime) { mData = aTime.GetValue(); }
bool IsAppendedInMessage(void) const { return mLease & kAppendedInMsgFlag; }
@@ -788,6 +788,17 @@ public:
*
*/
bool GetUseShortLeaseOption(void) const { return mUseShortLeaseOption; }
/**
* Set the next DNS message ID for client to use.
*
* This is intended for testing only.
*
* @pram[in] aMessageId A message ID.
*
*/
void SetNextMessageId(uint16_t aMessageId) { mNextMessageId = aMessageId; }
#endif // OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
private:
@@ -1007,7 +1018,7 @@ private:
void UpdateServiceStateToRemove(Service &aService);
State GetState(void) const { return mState; }
void SetState(State aState);
void ChangeHostAndServiceStates(const ItemState *aNewStates, ServiceStateChangeMode aMode);
bool ChangeHostAndServiceStates(const ItemState *aNewStates, ServiceStateChangeMode aMode);
void InvokeCallback(Error aError) const;
void InvokeCallback(Error aError, const HostInfo &aHostInfo, const Service *aRemovedServices) const;
void HandleHostInfoOrServiceChange(void);
@@ -1031,6 +1042,7 @@ private:
void UpdateRecordLengthInMessage(Dns::ResourceRecord &aRecord, uint16_t aOffset, Message &aMessage) const;
static void HandleUdpReceive(void *aContext, otMessage *aMessage, const otMessageInfo *aMessageInfo);
void ProcessResponse(Message &aMessage);
bool IsResponseMessageIdValid(uint16_t aId) const;
void HandleUpdateDone(void);
void GetRemovedServices(LinkedList<Service> &aRemovedServices);
static Error ReadResourceRecord(const Message &aMessage, uint16_t &aOffset, Dns::ResourceRecord &aRecord);
@@ -1073,7 +1085,8 @@ private:
bool mUseShortLeaseOption : 1;
#endif
uint16_t mUpdateMessageId;
uint16_t mNextMessageId;
uint16_t mResponseMessageId;
uint16_t mAutoHostAddressCount;
uint32_t mRetryWaitInterval;
tests/unit/test_srp_server.cpp
+183
View File
@@ -1024,6 +1024,187 @@ void TestUpdateLeaseShortVariant(void)
Log("End of TestUpdateLeaseShortVariant");
}
static uint16_t sServerRxCount;
static Ip6::MessageInfo sServerMsgInfo;
static uint16_t sServerLastMsgId;
void HandleServerUdpReceive(void *aContext, otMessage *aMessage, const otMessageInfo *aMessageInfo)
{
Dns::Header header;
VerifyOrQuit(aContext == nullptr);
VerifyOrQuit(aMessage != nullptr);
VerifyOrQuit(aMessageInfo != nullptr);
SuccessOrQuit(AsCoreType(aMessage).Read(0, header));
sServerMsgInfo = AsCoreType(aMessageInfo);
sServerLastMsgId = header.GetMessageId();
sServerRxCount++;
Log("HandleServerUdpReceive(), message-id: 0x%x", header.GetMessageId());
}
void TestSrpClientDelayedResponse(void)
{
static constexpr uint16_t kServerPort = 53535;
Srp::Client *srpClient;
Srp::Client::Service service1;
Srp::Client::Service service2;
Log("--------------------------------------------------------------------------------------------");
Log("TestSrpClientDelayedResponse");
InitTest();
srpClient = &sInstance->Get<Srp::Client>();
for (uint8_t testIter = 0; testIter < 3; testIter++)
{
Log("- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -");
Log("testIter = %u", testIter);
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Prepare a socket to act as SRP server.
Ip6::Udp::Socket udpSocket(*sInstance);
Ip6::SockAddr serverSockAddr;
uint16_t firstMsgId;
Message *response;
Dns::UpdateHeader header;
sServerRxCount = 0;
SuccessOrQuit(udpSocket.Open(HandleServerUdpReceive, nullptr));
SuccessOrQuit(udpSocket.Bind(kServerPort, Ip6::kNetifThread));
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Manually start the client with a message ID based on `testIter`
// We use zero in the first iteration, `0xffff` in the second
// iteration to test wrapping of 16-bit message ID.
switch (testIter)
{
case 0:
srpClient->SetNextMessageId(0);
break;
case 1:
srpClient->SetNextMessageId(0xffff);
break;
case 2:
srpClient->SetNextMessageId(0xaaaa);
break;
}
serverSockAddr.SetAddress(sInstance->Get<Mle::Mle>().GetMeshLocal16());
serverSockAddr.SetPort(kServerPort);
SuccessOrQuit(srpClient->Start(serverSockAddr));
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Register a service
SuccessOrQuit(srpClient->SetHostName(kHostName));
SuccessOrQuit(srpClient->EnableAutoHostAddress());
PrepareService1(service1);
SuccessOrQuit(srpClient->AddService(service1));
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Wait for short time and make sure server receives an SRP
// update message from client.
AdvanceTime(1 * 1000);
VerifyOrQuit(sServerRxCount == 1);
firstMsgId = sServerLastMsgId;
switch (testIter)
{
case 0:
VerifyOrQuit(firstMsgId == 0);
break;
case 1:
VerifyOrQuit(firstMsgId == 0xffff);
break;
case 2:
VerifyOrQuit(firstMsgId == 0xaaaa);
break;
}
if (testIter == 2)
{
AdvanceTime(2 * 1000);
PrepareService2(service2);
SuccessOrQuit(srpClient->AddService(service2));
}
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Wait for longer to allow client to retry a bunch of times
AdvanceTime(20 * 1000);
VerifyOrQuit(sServerRxCount > 1);
VerifyOrQuit(sServerLastMsgId != firstMsgId);
VerifyOrQuit(srpClient->GetHostInfo().GetState() != Srp::Client::kRegistered);
VerifyOrQuit(service1.GetState() != Srp::Client::kRegistered);
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Now send a delayed response from server using the first
// message ID.
response = udpSocket.NewMessage();
VerifyOrQuit(response != nullptr);
Log("Sending response with msg-id: 0x%x", firstMsgId);
header.SetMessageId(firstMsgId);
header.SetType(Dns::UpdateHeader::kTypeResponse);
header.SetResponseCode(Dns::UpdateHeader::kResponseSuccess);
SuccessOrQuit(response->Append(header));
SuccessOrQuit(udpSocket.SendTo(*response, sServerMsgInfo));
AdvanceTime(10);
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// In the first two iterations, we ensure that client
// did successfully accept the response with older message ID.
// This should not be the case in the third iteration due to
// changes to client services after first UPdate message was
// sent by client.
switch (testIter)
{
case 0:
case 1:
VerifyOrQuit(srpClient->GetHostInfo().GetState() == Srp::Client::kRegistered);
VerifyOrQuit(service1.GetState() == Srp::Client::kRegistered);
break;
case 2:
VerifyOrQuit(srpClient->GetHostInfo().GetState() != Srp::Client::kRegistered);
VerifyOrQuit(service1.GetState() != Srp::Client::kRegistered);
break;
}
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Remove service and close socket.
srpClient->ClearHostAndServices();
srpClient->Stop();
SuccessOrQuit(udpSocket.Close());
}
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Finalize OT instance
Log("Finalizing OT instance");
FinalizeTest();
Log("End of TestSrpClientDelayedResponse");
}
#endif // OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
#endif // ENABLE_SRP_TEST
@@ -1040,7 +1221,9 @@ int main(void)
ot::TestSrpServerClientRemove(/* aShouldRemoveKeyLease */ false);
#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
ot::TestUpdateLeaseShortVariant();
ot::TestSrpClientDelayedResponse();
#endif
printf("All tests passed\n");
#else
printf("SRP_SERVER or SRP_CLIENT feature is not enabled\n");