Skip to content

Commit

Permalink
Disable DNSSD cache globally in preparation for address resolve (#16029)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
andy31415 authored and pull[bot] committed Nov 1, 2023
1 parent 885c0e7 commit 69373be
Show file tree
Hide file tree
Showing 19 changed files with 47 additions and 25 deletions.
19 changes: 15 additions & 4 deletions src/app/CASESessionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand All @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions src/app/CASESessionManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@ namespace chip {
struct CASESessionManagerConfig
{
DeviceProxyInitParams sessionInitParams;
#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0
Dnssd::DnssdCache<CHIP_CONFIG_MDNS_CACHE_SIZE> * dnsCache = nullptr;
OperationalDeviceProxyPoolDelegate * devicePool = nullptr;
Dnssd::ResolverProxy * dnsResolver = nullptr;
#endif
OperationalDeviceProxyPoolDelegate * devicePool = nullptr;
Dnssd::ResolverProxy * dnsResolver = nullptr;
};

/**
Expand Down
7 changes: 5 additions & 2 deletions src/app/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 7 additions & 3 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<CASESessionManager>(sessionManagerConfig);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,9 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate,

CASESessionManager * mCASESessionManager = nullptr;

#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0
Dnssd::DnssdCache<CHIP_CONFIG_MDNS_CACHE_SIZE> mDNSCache;
#endif
CASEClientPool<CHIP_CONFIG_CONTROLLER_MAX_ACTIVE_CASE_CLIENTS> mCASEClientPool;
OperationalDeviceProxyPool<CHIP_CONFIG_CONTROLLER_MAX_ACTIVE_DEVICES> mDevicePool;

Expand Down
2 changes: 1 addition & 1 deletion src/lib/core/CHIPConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Ameba/SystemPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/platform/EFR32/SystemPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/platform/ESP32/SystemPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/platform/Linux/SystemPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/platform/P6/SystemPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/platform/Tizen/SystemPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/platform/cc13x2_26x2/SystemPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/platform/mbed/SystemPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/platform/nrfconnect/SystemPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/platform/nxp/k32w/k32w0/SystemPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/platform/qpg/SystemPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/platform/telink/SystemPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/platform/webos/SystemPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 69373be

Please sign in to comment.