diff --git a/src/core/net/dns_client.cpp b/src/core/net/dns_client.cpp index 49a0d4d5a..1d41c87c6 100644 --- a/src/core/net/dns_client.cpp +++ b/src/core/net/dns_client.cpp @@ -791,6 +791,8 @@ void Client::Stop(void) IgnoreError(mEndpoint.Deinitialize()); } #endif + + mLimitedQueryServers.Clear(); } #if OPENTHREAD_CONFIG_DNS_CLIENT_OVER_TCP_ENABLE @@ -938,6 +940,8 @@ Error Client::Resolve(const char *aInstanceLabel, info.mConfig.SetFrom(aConfig, mDefaultConfig); info.mShouldResolveHostAddr = aShouldResolveHostAddr; + CheckAndUpdateServiceMode(info.mConfig, aConfig); + switch (info.mConfig.GetServiceMode()) { case QueryConfig::kServiceModeSrvTxtSeparate: @@ -1402,6 +1406,11 @@ Error Client::ParseResponse(const Message &aResponseMessage, Query *&aQuery, Err aResponseError = Header::ResponseCodeToError(header.GetResponseCode()); + if ((aResponseError == kErrorNone) && (info.mQueryType == kServiceQuerySrvTxt)) + { + RecordServerAsCapableOfMultiQuestions(info.mConfig.GetServerSockAddr().GetAddress()); + } + exit: return error; } @@ -1591,6 +1600,60 @@ exit: #if OPENTHREAD_CONFIG_DNS_CLIENT_SERVICE_DISCOVERY_ENABLE +void Client::CheckAndUpdateServiceMode(QueryConfig &aConfig, const QueryConfig *aRequestConfig) const +{ + // If the user explicitly requested "optimize" mode, we honor that + // request. Otherwise, if "optimize" is chosen from the default + // config, we check if the DNS server is known to have trouble + // with multiple-question queries. If so, we switch to "separate" + // mode. + + if ((aRequestConfig != nullptr) && (aRequestConfig->GetServiceMode() == QueryConfig::kServiceModeSrvTxtOptimize)) + { + ExitNow(); + } + + VerifyOrExit(aConfig.GetServiceMode() == QueryConfig::kServiceModeSrvTxtOptimize); + + if (mLimitedQueryServers.Contains(aConfig.GetServerSockAddr().GetAddress())) + { + aConfig.SetServiceMode(QueryConfig::kServiceModeSrvTxtSeparate); + } + +exit: + return; +} + +void Client::RecordServerAsLimitedToSingleQuestion(const Ip6::Address &aServerAddress) +{ + VerifyOrExit(!aServerAddress.IsUnspecified()); + + VerifyOrExit(!mLimitedQueryServers.Contains(aServerAddress)); + + if (mLimitedQueryServers.IsFull()) + { + uint8_t randomIndex = Random::NonCrypto::GetUint8InRange(0, mLimitedQueryServers.GetMaxSize()); + + mLimitedQueryServers.Remove(mLimitedQueryServers[randomIndex]); + } + + IgnoreError(mLimitedQueryServers.PushBack(aServerAddress)); + +exit: + return; +} + +void Client::RecordServerAsCapableOfMultiQuestions(const Ip6::Address &aServerAddress) +{ + Ip6::Address *entry = mLimitedQueryServers.Find(aServerAddress); + + VerifyOrExit(entry != nullptr); + mLimitedQueryServers.Remove(*entry); + +exit: + return; +} + Error Client::ReplaceWithSeparateSrvTxtQueries(Query &aQuery) { Error error = kErrorFailed; @@ -1602,6 +1665,8 @@ Error Client::ReplaceWithSeparateSrvTxtQueries(Query &aQuery) VerifyOrExit(info.mQueryType == kServiceQuerySrvTxt); VerifyOrExit(info.mConfig.GetServiceMode() == QueryConfig::kServiceModeSrvTxtOptimize); + RecordServerAsLimitedToSingleQuestion(info.mConfig.GetServerSockAddr().GetAddress()); + secondQuery = aQuery.Clone(); VerifyOrExit(secondQuery != nullptr); diff --git a/src/core/net/dns_client.hpp b/src/core/net/dns_client.hpp index 9ac6a3868..5a86058d1 100644 --- a/src/core/net/dns_client.hpp +++ b/src/core/net/dns_client.hpp @@ -770,7 +770,8 @@ public: #endif // OPENTHREAD_CONFIG_DNS_CLIENT_SERVICE_DISCOVERY_ENABLE private: - static constexpr uint16_t kMaxCnameAliasNameChanges = 40; + static constexpr uint16_t kMaxCnameAliasNameChanges = 40; + static constexpr uint8_t kLimitedQueryServersArraySize = 3; enum QueryType : uint8_t { @@ -858,6 +859,9 @@ private: void *aContext, const QueryConfig *aConfig, bool aShouldResolveHostAddr); + void CheckAndUpdateServiceMode(QueryConfig &aConfig, const QueryConfig *aRequestConfig) const; + void RecordServerAsLimitedToSingleQuestion(const Ip6::Address &aServerAddress); + void RecordServerAsCapableOfMultiQuestions(const Ip6::Address &aServerAddress); Error ReplaceWithSeparateSrvTxtQueries(Query &aQuery); void ResolveHostAddressIfNeeded(Query &aQuery, const Message &aResponseMessage); #endif @@ -925,6 +929,7 @@ private: #if OPENTHREAD_CONFIG_DNS_CLIENT_DEFAULT_SERVER_ADDRESS_AUTO_SET_ENABLE bool mUserDidSetDefaultAddress; #endif + Array mLimitedQueryServers; }; } // namespace Dns diff --git a/src/core/net/dnssd_server.hpp b/src/core/net/dnssd_server.hpp index 4174ac5ae..e20e71f4e 100644 --- a/src/core/net/dnssd_server.hpp +++ b/src/core/net/dnssd_server.hpp @@ -49,6 +49,7 @@ #include "border_router/infra_if.hpp" #include "common/as_core_type.hpp" #include "common/callback.hpp" +#include "common/equatable.hpp" #include "common/message.hpp" #include "common/non_copyable.hpp" #include "common/owned_ptr.hpp" @@ -96,6 +97,26 @@ public: */ class Counters : public otDnssdCounters, public Clearable { + public: + /** + * Returns the total number of processed queries (successful or failed responses). + * + * @return The total number of queries. + * + */ + uint32_t GetTotalQueries(void) const { return mSuccessResponse + GetTotalFailedQueries(); } + + /** + * Returns the total number of failed queries (any error response code). + * + * @return The total number of failed queries. + * + */ + uint32_t GetTotalFailedQueries(void) const + { + return mServerFailureResponse + mFormatErrorResponse + mNameErrorResponse + mNotImplementedResponse + + mOtherResponse; + } }; #if OPENTHREAD_CONFIG_DNS_UPSTREAM_QUERY_ENABLE diff --git a/tests/unit/test_dns_client.cpp b/tests/unit/test_dns_client.cpp index e91376689..b4947f585 100644 --- a/tests/unit/test_dns_client.cpp +++ b/tests/unit/test_dns_client.cpp @@ -497,15 +497,17 @@ void TestDnsClient(void) Dns::Client::QueryConfig::kServiceModeSrvTxtOptimize, }; - Array addresses; - Srp::Server *srpServer; - Srp::Client *srpClient; - Srp::Client::Service service1; - Srp::Client::Service service2; - Dns::Client *dnsClient; - Dns::Client::QueryConfig queryConfig; - Dns::ServiceDiscovery::Server *dnsServer; - uint16_t heapAllocations; + Array addresses; + Srp::Server *srpServer; + Srp::Client *srpClient; + Srp::Client::Service service1; + Srp::Client::Service service2; + Dns::Client *dnsClient; + Dns::Client::QueryConfig queryConfig; + Dns::ServiceDiscovery::Server *dnsServer; + Dns::ServiceDiscovery::Server::Counters oldServerCounters; + Dns::ServiceDiscovery::Server::Counters newServerCounters; + uint16_t heapAllocations; Log("--------------------------------------------------------------------------------------------"); Log("TestDnsClient"); @@ -882,6 +884,8 @@ void TestDnsClient(void) queryConfig.Clear(); queryConfig.mServiceMode = static_cast(Dns::Client::QueryConfig::kServiceModeSrvTxtOptimize); + oldServerCounters = dnsServer->GetCounters(); + sResolveServiceInfo.Reset(); SuccessOrQuit(dnsClient->ResolveServiceAndHostAddress(kInstance1Label, kService1FullName, ServiceCallback, sInstance, &queryConfig)); @@ -907,6 +911,55 @@ void TestDnsClient(void) VerifyOrQuit(addresses.Contains(sResolveServiceInfo.mHostAddresses[index])); } + newServerCounters = dnsServer->GetCounters(); + + Log("Validate (using server counter) that client first tried to query SRV/TXT together and failed"); + Log("and then send separate queries (for SRV, TXT and AAAA)"); + Log(" Total : %2u -> %2u", oldServerCounters.GetTotalQueries(), newServerCounters.GetTotalQueries()); + Log(" Failed: %2u -> %2u", oldServerCounters.GetTotalFailedQueries(), newServerCounters.GetTotalFailedQueries()); + + VerifyOrQuit(newServerCounters.GetTotalFailedQueries() == 1 + oldServerCounters.GetTotalFailedQueries()); + VerifyOrQuit(newServerCounters.GetTotalQueries() == 4 + oldServerCounters.GetTotalQueries()); + + Log("- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - "); + Log("Resolve service again now using `kServiceModeSrvTxtOptimize` as default config"); + Log("Client should already know that server is not capable of handling multi-question query"); + + queryConfig.Clear(); + queryConfig.mServiceMode = static_cast(Dns::Client::QueryConfig::kServiceModeSrvTxtOptimize); + + dnsClient->SetDefaultConfig(queryConfig); + + Log("ResolveService(%s,%s)", kInstance1Label, kService1FullName); + + oldServerCounters = dnsServer->GetCounters(); + + sResolveServiceInfo.Reset(); + SuccessOrQuit(dnsClient->ResolveService(kInstance1Label, kService1FullName, ServiceCallback, sInstance, nullptr)); + + AdvanceTime(100); + + VerifyOrQuit(sResolveServiceInfo.mCallbackCount == 1); + SuccessOrQuit(sResolveServiceInfo.mError); + + VerifyOrQuit(sResolveServiceInfo.mInfo.mTtl != 0); + VerifyOrQuit(sResolveServiceInfo.mInfo.mPort == service1.mPort); + VerifyOrQuit(sResolveServiceInfo.mInfo.mWeight == service1.mWeight); + VerifyOrQuit(strcmp(sResolveServiceInfo.mInfo.mHostNameBuffer, kHostFullName) == 0); + + VerifyOrQuit(sResolveServiceInfo.mInfo.mTxtDataTtl != 0); + VerifyOrQuit(sResolveServiceInfo.mInfo.mTxtDataSize != 0); + + newServerCounters = dnsServer->GetCounters(); + + Log("Client should already know that server is not capable of handling multi-question query"); + Log("Check server counters to validate that client did send separate queries for TXT and SRV"); + Log(" Total : %2u -> %2u", oldServerCounters.GetTotalQueries(), newServerCounters.GetTotalQueries()); + Log(" Failed: %2u -> %2u", oldServerCounters.GetTotalFailedQueries(), newServerCounters.GetTotalFailedQueries()); + + VerifyOrQuit(newServerCounters.GetTotalFailedQueries() == oldServerCounters.GetTotalFailedQueries()); + VerifyOrQuit(newServerCounters.GetTotalQueries() == 2 + oldServerCounters.GetTotalQueries()); + dnsServer->SetTestMode(Dns::ServiceDiscovery::Server::kTestModeDisabled); Log("- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - ");