From 40e363153fb49eaefa910eac41e530a5e1553d0b Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 9 Mar 2022 18:16:36 -0500 Subject: [PATCH] Disable DNSSD cache globally in preparation for address resolve (#16029) * Remove CHIP_CONFIG_MDNS_CACHE_SIZE and dns caching in general. In #15934 caching is generally not used, with the rationale of: - cache is doubled/trippled: - Platform MDNS servers are expected to cache at least on linux and mac due to avahi/bonjour being system wide and enforcing TTLs - We have cache both on CASESession and Platform, with separate caching between operational and commissionable - MinMdns does not have the dnssd cache like platform, but would likely benefit the most of it (since it would have TTLs) - we do not have a good way to clear caches and we do not update them on boot advertising (platform dnssd implementations would, however since we have an internal cache that seems to break) Looking to disable caching and either build it anew if it required/helps and at least make use of only one. * Add ifdefs for mdns cache variable * Add more conditionals for MDNS cache size * Fix typo in endif/else * Restyle --- src/app/CASESessionManager.cpp | 19 +++++++++++++++---- src/app/CASESessionManager.h | 6 ++++-- src/app/server/Server.cpp | 7 +++++-- src/controller/CHIPDeviceController.cpp | 10 +++++++--- src/controller/CHIPDeviceController.h | 2 ++ src/lib/core/CHIPConfig.h | 2 +- src/platform/Ameba/SystemPlatformConfig.h | 2 +- src/platform/EFR32/SystemPlatformConfig.h | 2 +- src/platform/ESP32/SystemPlatformConfig.h | 2 +- src/platform/Linux/SystemPlatformConfig.h | 2 +- src/platform/P6/SystemPlatformConfig.h | 2 +- src/platform/Tizen/SystemPlatformConfig.h | 2 +- .../cc13x2_26x2/SystemPlatformConfig.h | 2 +- src/platform/mbed/SystemPlatformConfig.h | 2 +- .../nrfconnect/SystemPlatformConfig.h | 2 +- .../nxp/k32w/k32w0/SystemPlatformConfig.h | 2 +- src/platform/qpg/SystemPlatformConfig.h | 2 +- src/platform/telink/SystemPlatformConfig.h | 2 +- src/platform/webos/SystemPlatformConfig.h | 2 +- 19 files changed, 47 insertions(+), 25 deletions(-) diff --git a/src/app/CASESessionManager.cpp b/src/app/CASESessionManager.cpp index 80d66a50b81cc0..e87d857c8ab229 100644 --- a/src/app/CASESessionManager.cpp +++ b/src/app/CASESessionManager.cpp @@ -37,7 +37,12 @@ CHIP_ERROR CASESessionManager::FindOrEstablishSession(PeerId peerId, Callback::C { Dnssd::ResolvedNodeData resolutionData; - bool nodeIDWasResolved = (mConfig.dnsCache != nullptr && mConfig.dnsCache->Lookup(peerId, resolutionData) == CHIP_NO_ERROR); + bool nodeIDWasResolved = +#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 + (mConfig.dnsCache != nullptr && mConfig.dnsCache->Lookup(peerId, resolutionData) == CHIP_NO_ERROR); +#else + false; +#endif OperationalDeviceProxy * session = FindExistingSession(peerId); if (session == nullptr) @@ -98,6 +103,7 @@ void CASESessionManager::OnOperationalNodeResolved(const Dnssd::ResolvedNodeData { ChipLogProgress(Controller, "Address resolved for node: 0x" ChipLogFormatX64, ChipLogValueX64(nodeData.mPeerId.GetNodeId())); +#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 if (mConfig.dnsCache != nullptr) { CHIP_ERROR err = mConfig.dnsCache->Insert(nodeData); @@ -106,6 +112,7 @@ void CASESessionManager::OnOperationalNodeResolved(const Dnssd::ResolvedNodeData ChipLogError(Controller, "DNS Cache insert: %" CHIP_ERROR_FORMAT, err.Format()); } } +#endif OperationalDeviceProxy * session = FindExistingSession(nodeData.mPeerId); VerifyOrReturn(session != nullptr, @@ -125,13 +132,17 @@ void CASESessionManager::OnOperationalNodeResolutionFailed(const PeerId & peer, CHIP_ERROR CASESessionManager::GetPeerAddress(PeerId peerId, Transport::PeerAddress & addr) { +#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 if (mConfig.dnsCache != nullptr) { Dnssd::ResolvedNodeData resolutionData; - ReturnErrorOnFailure(mConfig.dnsCache->Lookup(peerId, resolutionData)); - addr = OperationalDeviceProxy::ToPeerAddress(resolutionData); - return CHIP_NO_ERROR; + if (mConfig.dnsCache->Lookup(peerId, resolutionData) == CHIP_NO_ERROR) + { + addr = OperationalDeviceProxy::ToPeerAddress(resolutionData); + return CHIP_NO_ERROR; + } } +#endif OperationalDeviceProxy * session = FindExistingSession(peerId); VerifyOrReturnError(session != nullptr, CHIP_ERROR_NOT_CONNECTED); diff --git a/src/app/CASESessionManager.h b/src/app/CASESessionManager.h index a3c0dbe81dfe2f..2f129e9f71b08d 100644 --- a/src/app/CASESessionManager.h +++ b/src/app/CASESessionManager.h @@ -35,9 +35,11 @@ namespace chip { struct CASESessionManagerConfig { DeviceProxyInitParams sessionInitParams; +#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 Dnssd::DnssdCache * dnsCache = nullptr; - OperationalDeviceProxyPoolDelegate * devicePool = nullptr; - Dnssd::ResolverProxy * dnsResolver = nullptr; +#endif + OperationalDeviceProxyPoolDelegate * devicePool = nullptr; + Dnssd::ResolverProxy * dnsResolver = nullptr; }; /** diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 0f6666fed07297..50b5b3f4e15862 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -109,11 +109,14 @@ Server::Server() : .fabricTable = &mFabrics, .clientPool = &mCASEClientPool, }, +#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 .dnsCache = nullptr, +#endif .devicePool = &mDevicePool, .dnsResolver = nullptr, - }), mCommissioningWindowManager(this), mGroupsProvider(mDeviceStorage), - mAttributePersister(mDeviceStorage), mAccessControl(Access::Examples::GetAccessControlDelegate(&mDeviceStorage)) + }), + mCommissioningWindowManager(this), mGroupsProvider(mDeviceStorage), mAttributePersister(mDeviceStorage), + mAccessControl(Access::Examples::GetAccessControlDelegate(&mDeviceStorage)) {} CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint16_t unsecureServicePort, diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 71196d726944b2..598fa4d65585f0 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -160,9 +160,11 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params) CASESessionManagerConfig sessionManagerConfig = { .sessionInitParams = deviceInitParams, - .dnsCache = &mDNSCache, - .devicePool = &mDevicePool, - .dnsResolver = &mDNSResolver, +#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 + .dnsCache = &mDNSCache, +#endif + .devicePool = &mDevicePool, + .dnsResolver = &mDNSResolver, }; mCASESessionManager = chip::Platform::New(sessionManagerConfig); @@ -1458,7 +1460,9 @@ void DeviceCommissioner::OnOperationalNodeResolved(const chip::Dnssd::ResolvedNo return; } +#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 mDNSCache.Insert(nodeData); +#endif mCASESessionManager->FindOrEstablishSession(nodeData.mPeerId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback); DeviceController::OnOperationalNodeResolved(nodeData); diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 22117cafaa8b2d..aeb4db8c9fba2e 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -347,7 +347,9 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, CASESessionManager * mCASESessionManager = nullptr; +#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 Dnssd::DnssdCache mDNSCache; +#endif CASEClientPool mCASEClientPool; OperationalDeviceProxyPool mDevicePool; diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h index 2c3448cc53041b..144f38465369b8 100644 --- a/src/lib/core/CHIPConfig.h +++ b/src/lib/core/CHIPConfig.h @@ -1330,7 +1330,7 @@ extern const char CHIP_NON_PRODUCTION_MARKER[]; * */ #ifndef CHIP_CONFIG_MDNS_CACHE_SIZE -#define CHIP_CONFIG_MDNS_CACHE_SIZE 20 +#define CHIP_CONFIG_MDNS_CACHE_SIZE 0 #endif /** * @name Interaction Model object pool configuration. diff --git a/src/platform/Ameba/SystemPlatformConfig.h b/src/platform/Ameba/SystemPlatformConfig.h index e5ad3b3ebade61..7db088f4a77d77 100755 --- a/src/platform/Ameba/SystemPlatformConfig.h +++ b/src/platform/Ameba/SystemPlatformConfig.h @@ -61,4 +61,4 @@ struct ChipDeviceEvent; #define CHIP_SYSTEM_CONFIG_USE_SOCKETS 0 #define CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK 0 #define CHIP_SYSTEM_CONFIG_POSIX_LOCKING 0 -#define CHIP_CONFIG_MDNS_CACHE_SIZE 4 +#define CHIP_CONFIG_MDNS_CACHE_SIZE 0 diff --git a/src/platform/EFR32/SystemPlatformConfig.h b/src/platform/EFR32/SystemPlatformConfig.h index 6f5a1ebed34ed7..435ef60fa1e3f4 100644 --- a/src/platform/EFR32/SystemPlatformConfig.h +++ b/src/platform/EFR32/SystemPlatformConfig.h @@ -43,4 +43,4 @@ struct ChipDeviceEvent; #define CHIP_SYSTEM_CONFIG_NUM_TIMERS 16 #endif // CHIP_SYSTEM_CONFIG_NUM_TIMERS -#define CHIP_CONFIG_MDNS_CACHE_SIZE 4 +#define CHIP_CONFIG_MDNS_CACHE_SIZE 0 diff --git a/src/platform/ESP32/SystemPlatformConfig.h b/src/platform/ESP32/SystemPlatformConfig.h index 6838379fc15932..34ed45cc8aa76d 100644 --- a/src/platform/ESP32/SystemPlatformConfig.h +++ b/src/platform/ESP32/SystemPlatformConfig.h @@ -48,4 +48,4 @@ struct ChipDeviceEvent; #define CHIP_SYSTEM_CONFIG_NUM_TIMERS CONFIG_NUM_TIMERS #endif // CHIP_SYSTEM_CONFIG_NUM_TIMERS -#define CHIP_CONFIG_MDNS_CACHE_SIZE 4 +#define CHIP_CONFIG_MDNS_CACHE_SIZE 0 diff --git a/src/platform/Linux/SystemPlatformConfig.h b/src/platform/Linux/SystemPlatformConfig.h index 2794184ca8e0a3..1dcf357521dff3 100644 --- a/src/platform/Linux/SystemPlatformConfig.h +++ b/src/platform/Linux/SystemPlatformConfig.h @@ -46,4 +46,4 @@ struct ChipDeviceEvent; #define CHIP_SYSTEM_CONFIG_NUM_TIMERS 16 #endif // CHIP_SYSTEM_CONFIG_NUM_TIMERS -#define CHIP_CONFIG_MDNS_CACHE_SIZE 4 +#define CHIP_CONFIG_MDNS_CACHE_SIZE 0 diff --git a/src/platform/P6/SystemPlatformConfig.h b/src/platform/P6/SystemPlatformConfig.h index 1a1cda338b6dca..e37527cadfef3d 100644 --- a/src/platform/P6/SystemPlatformConfig.h +++ b/src/platform/P6/SystemPlatformConfig.h @@ -55,4 +55,4 @@ struct ChipDeviceEvent; #define CHIP_SYSTEM_CONFIG_NUM_TIMERS 16 #endif // CHIP_SYSTEM_CONFIG_NUM_TIMERS -#define CHIP_CONFIG_MDNS_CACHE_SIZE 4 +#define CHIP_CONFIG_MDNS_CACHE_SIZE 0 diff --git a/src/platform/Tizen/SystemPlatformConfig.h b/src/platform/Tizen/SystemPlatformConfig.h index 69fa7d72f37f8d..7a49ab8c277c34 100644 --- a/src/platform/Tizen/SystemPlatformConfig.h +++ b/src/platform/Tizen/SystemPlatformConfig.h @@ -48,4 +48,4 @@ struct ChipDeviceEvent; #define CHIP_SYSTEM_CONFIG_NUM_TIMERS 16 #endif // CHIP_SYSTEM_CONFIG_NUM_TIMERS -#define CHIP_CONFIG_MDNS_CACHE_SIZE 4 +#define CHIP_CONFIG_MDNS_CACHE_SIZE 0 diff --git a/src/platform/cc13x2_26x2/SystemPlatformConfig.h b/src/platform/cc13x2_26x2/SystemPlatformConfig.h index 76bb84bce8e462..e66eeaeeb1cae5 100644 --- a/src/platform/cc13x2_26x2/SystemPlatformConfig.h +++ b/src/platform/cc13x2_26x2/SystemPlatformConfig.h @@ -38,4 +38,4 @@ struct ChipDeviceEvent; #define CHIP_SYSTEM_CONFIG_EVENT_OBJECT_TYPE const struct ::chip::DeviceLayer::ChipDeviceEvent * // ========== Platform-specific Configuration Overrides ========= -#define CHIP_CONFIG_MDNS_CACHE_SIZE 4 +#define CHIP_CONFIG_MDNS_CACHE_SIZE 0 diff --git a/src/platform/mbed/SystemPlatformConfig.h b/src/platform/mbed/SystemPlatformConfig.h index a8891d351305b9..0ca39de5790290 100644 --- a/src/platform/mbed/SystemPlatformConfig.h +++ b/src/platform/mbed/SystemPlatformConfig.h @@ -48,4 +48,4 @@ struct ChipDeviceEvent; #define CHIP_SYSTEM_CONFIG_NUM_TIMERS 16 #endif // CHIP_SYSTEM_CONFIG_NUM_TIMERS -#define CHIP_CONFIG_MDNS_CACHE_SIZE 4 +#define CHIP_CONFIG_MDNS_CACHE_SIZE 0 diff --git a/src/platform/nrfconnect/SystemPlatformConfig.h b/src/platform/nrfconnect/SystemPlatformConfig.h index 9882917b1cf08c..001a90b8627258 100644 --- a/src/platform/nrfconnect/SystemPlatformConfig.h +++ b/src/platform/nrfconnect/SystemPlatformConfig.h @@ -57,4 +57,4 @@ struct ChipDeviceEvent; #define CHIP_SYSTEM_CONFIG_NUM_TIMERS 16 #endif // CHIP_SYSTEM_CONFIG_NUM_TIMERS -#define CHIP_CONFIG_MDNS_CACHE_SIZE 4 +#define CHIP_CONFIG_MDNS_CACHE_SIZE 0 diff --git a/src/platform/nxp/k32w/k32w0/SystemPlatformConfig.h b/src/platform/nxp/k32w/k32w0/SystemPlatformConfig.h index 3a3c439d2ead70..833cc88ea9b9e3 100644 --- a/src/platform/nxp/k32w/k32w0/SystemPlatformConfig.h +++ b/src/platform/nxp/k32w/k32w0/SystemPlatformConfig.h @@ -44,4 +44,4 @@ struct ChipDeviceEvent; #define CHIP_SYSTEM_CONFIG_NUM_TIMERS 16 #endif // CHIP_SYSTEM_CONFIG_NUM_TIMERS -#define CHIP_CONFIG_MDNS_CACHE_SIZE 4 +#define CHIP_CONFIG_MDNS_CACHE_SIZE 0 diff --git a/src/platform/qpg/SystemPlatformConfig.h b/src/platform/qpg/SystemPlatformConfig.h index 8ec4376d3bab36..0972796ff08b34 100644 --- a/src/platform/qpg/SystemPlatformConfig.h +++ b/src/platform/qpg/SystemPlatformConfig.h @@ -42,4 +42,4 @@ struct ChipDeviceEvent; #define CHIP_SYSTEM_CONFIG_NUM_TIMERS 16 #endif // CHIP_SYSTEM_CONFIG_NUM_TIMERS -#define CHIP_CONFIG_MDNS_CACHE_SIZE 4 +#define CHIP_CONFIG_MDNS_CACHE_SIZE 0 diff --git a/src/platform/telink/SystemPlatformConfig.h b/src/platform/telink/SystemPlatformConfig.h index c2e51709232a8a..30e47b700390e1 100644 --- a/src/platform/telink/SystemPlatformConfig.h +++ b/src/platform/telink/SystemPlatformConfig.h @@ -57,4 +57,4 @@ struct ChipDeviceEvent; #define CHIP_SYSTEM_CONFIG_NUM_TIMERS 16 #endif // CHIP_SYSTEM_CONFIG_NUM_TIMERS -#define CHIP_CONFIG_MDNS_CACHE_SIZE 4 +#define CHIP_CONFIG_MDNS_CACHE_SIZE 0 diff --git a/src/platform/webos/SystemPlatformConfig.h b/src/platform/webos/SystemPlatformConfig.h index 6a74853d884b78..bf9d871c6d89b0 100644 --- a/src/platform/webos/SystemPlatformConfig.h +++ b/src/platform/webos/SystemPlatformConfig.h @@ -46,4 +46,4 @@ struct ChipDeviceEvent; #define CHIP_SYSTEM_CONFIG_NUM_TIMERS 16 #endif // CHIP_SYSTEM_CONFIG_NUM_TIMERS -#define CHIP_CONFIG_MDNS_CACHE_SIZE 4 +#define CHIP_CONFIG_MDNS_CACHE_SIZE 0