From 42d567d089877c2c9650075e7b27093f513fe725 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 20 May 2024 08:54:16 -0700 Subject: [PATCH] [mdns] ensure callback is invoked when registering host with no address (#10264) This commit updates the native mDNS implementation to ensure the callback is invoked when a host is registered without any address (effectively unregistering the host and removing any previously registered addresses for the host-name). The implementation ensures the callback is invoked after returning from the `RegisterHost()` call (invoked from a posted tasklet), as required by the mDNS API definitions. Additionally, the `test_mdns` unit test is updated to cover registering a host with no address for the first time or updating a previous registration. --- src/core/net/mdns.cpp | 26 ++++++++++++++++ src/core/net/mdns.hpp | 2 ++ tests/unit/test_mdns.cpp | 64 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+) diff --git a/src/core/net/mdns.cpp b/src/core/net/mdns.cpp index 996fa1c35..e06b0a787 100644 --- a/src/core/net/mdns.cpp +++ b/src/core/net/mdns.cpp @@ -832,6 +832,7 @@ Core::Entry::Entry(void) , mMulticastNsecPending(false) , mUnicastNsecPending(false) , mAppendedNsec(false) + , mBypassCallbackStateCheck(false) { } @@ -903,6 +904,12 @@ void Core::Entry::SetCallback(const Callback &aCallback) ScheduleCallbackTask(); } +void Core::Entry::MarkToInvokeCallbackUnconditionally(void) +{ + mBypassCallbackStateCheck = true; + Get().mEntryTask.Post(); +} + void Core::Entry::ScheduleCallbackTask(void) { switch (GetState()) @@ -926,6 +933,16 @@ void Core::Entry::InvokeCallbacks(void) { Error error = kErrorNone; + // `mBypassCallbackStateCheck` is used when host is registered + // with no address, which is treated as unregistering the host. + // This ensures host registration callback is invoked properly. + + if (mBypassCallbackStateCheck) + { + mBypassCallbackStateCheck = false; + mCallback.InvokeAndClear(GetInstance(), error); + } + switch (GetState()) { case kConflict: @@ -1332,7 +1349,16 @@ void Core::HostEntry::Register(const Host &aHost, const Callback &aCallback) // If host is registered with no addresses, treat it // as host being unregistered and announce removal of // the old addresses. + Unregister(aHost); + + // Set the callback again as `Unregister()` may clear it. + // Also mark to invoke the callback unconditionally (bypassing + // entry state check). The callback will be invoked + // after returning from this method from the posted tasklet. + + SetCallback(aCallback); + MarkToInvokeCallbackUnconditionally(); ExitNow(); } diff --git a/src/core/net/mdns.hpp b/src/core/net/mdns.hpp index 5ee323657..6001e8f58 100644 --- a/src/core/net/mdns.hpp +++ b/src/core/net/mdns.hpp @@ -907,6 +907,7 @@ private: void Init(Instance &aInstance); void SetCallback(const Callback &aCallback); void ClearCallback(void) { mCallback.Clear(); } + void MarkToInvokeCallbackUnconditionally(void); void StartProbing(void); void SetStateToConflict(void); void SetStateToRemoving(void); @@ -939,6 +940,7 @@ private: bool mMulticastNsecPending : 1; bool mUnicastNsecPending : 1; bool mAppendedNsec : 1; + bool mBypassCallbackStateCheck : 1; TimeMilli mNsecAnswerTime; Heap::Data mKeyData; Callback mCallback; diff --git a/tests/unit/test_mdns.cpp b/tests/unit/test_mdns.cpp index 72e63ca32..aa927ea29 100644 --- a/tests/unit/test_mdns.cpp +++ b/tests/unit/test_mdns.cpp @@ -1853,6 +1853,70 @@ void TestHostReg(void) AdvanceTime(15000); VerifyOrQuit(sDnsMessages.IsEmpty()); + Log("- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -"); + Log("Register a host with no address (first time)"); + + host.mHostName = "newhost"; + host.mAddresses = nullptr; + host.mAddressesLength = 0; + host.mTtl = 1500; + + sRegCallbacks[2].Reset(); + SuccessOrQuit(mdns->RegisterHost(host, 2, HandleSuccessCallback)); + + AdvanceTime(1); + VerifyOrQuit(sRegCallbacks[2].mWasCalled); + + Log("- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -"); + Log("Register the same host now with an address"); + + host.mAddresses = &hostAddresses[0]; + host.mAddressesLength = 1; + + sRegCallbacks[3].Reset(); + SuccessOrQuit(mdns->RegisterHost(host, 3, HandleSuccessCallback)); + + AdvanceTime(15000); + VerifyOrQuit(sRegCallbacks[3].mWasCalled); + + Log("- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -"); + Log("Register the same host again now with no address"); + + host.mAddressesLength = 0; + + sRegCallbacks[4].Reset(); + sDnsMessages.Clear(); + SuccessOrQuit(mdns->RegisterHost(host, 4, HandleSuccessCallback)); + + AdvanceTime(1); + VerifyOrQuit(sRegCallbacks[4].mWasCalled); + + for (uint8_t anncCount = 0; anncCount < kNumAnnounces; anncCount++) + { + AdvanceTime((anncCount == 0) ? 0 : (1U << (anncCount - 1)) * 1000); + + VerifyOrQuit(!sDnsMessages.IsEmpty()); + dnsMsg = sDnsMessages.GetHead(); + dnsMsg->ValidateHeader(kMulticastResponse, /* Q */ 0, /* Ans */ 1, /* Auth */ 0, /* Addnl */ 0); + dnsMsg->Validate(host, kInAnswerSection, kGoodBye); + VerifyOrQuit(dnsMsg->GetNext() == nullptr); + sDnsMessages.Clear(); + } + + Log("- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -"); + Log("Register the same host again now adding an address"); + + host.mAddresses = &hostAddresses[1]; + host.mAddressesLength = 1; + + sRegCallbacks[5].Reset(); + SuccessOrQuit(mdns->RegisterHost(host, 5, HandleSuccessCallback)); + + AdvanceTime(15000); + VerifyOrQuit(sRegCallbacks[5].mWasCalled); + + Log("- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -"); + SuccessOrQuit(mdns->SetEnabled(false, kInfraIfIndex)); VerifyOrQuit(sHeapAllocatedPtrs.GetLength() <= heapAllocations);