From ec49522077e8b1adacde51c731adcfaf635785fb Mon Sep 17 00:00:00 2001 From: Marty Leisner Date: Thu, 26 Aug 2021 12:24:15 -0400 Subject: [PATCH] Added comment on CHIP_CONFIG_MDNS_CACHE_SIZE that if 0, builtin cache is not used adding a cast to tests for (NodeId) for clang++. fixed typos and missing header files for darwin. removed initializing nullPeerID as per suggestion by as per suggestion by Damian-Nordic. restyled whitespace restyle clang-format --- src/lib/core/CHIPConfig.h | 6 +++- src/lib/mdns/Discovery_ImplPlatform.cpp | 10 ++++++- src/lib/mdns/Discovery_ImplPlatform.h | 5 ++++ src/lib/mdns/MdnsCache.h | 38 +++++++++++++++++++------ src/lib/mdns/tests/TestMdnsCache.cpp | 8 +++--- 5 files changed, 53 insertions(+), 14 deletions(-) diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h index 2a6aae119c03b2..166ca03e15f677 100644 --- a/src/lib/core/CHIPConfig.h +++ b/src/lib/core/CHIPConfig.h @@ -2397,7 +2397,11 @@ extern const char CHIP_NON_PRODUCTION_MARKER[]; /** * @def CHIP_CONFIG_MDNS_CACHE_SIZE * - * @brief Define the size of the MDNS cache + * @brief + * Define the size of the MDNS cache + * + * If CHIP_CONFIG_MDNS_CACHE_SIZE is 0, the builtin cache is not used. + * */ #ifndef CHIP_CONFIG_MDNS_CACHE_SIZE #define CHIP_CONFIG_MDNS_CACHE_SIZE 20 diff --git a/src/lib/mdns/Discovery_ImplPlatform.cpp b/src/lib/mdns/Discovery_ImplPlatform.cpp index 45a486901d2501..476d1b44eee386 100644 --- a/src/lib/mdns/Discovery_ImplPlatform.cpp +++ b/src/lib/mdns/Discovery_ImplPlatform.cpp @@ -21,6 +21,7 @@ #include #include "ServiceNaming.h" +#include "core/CHIPConfig.h" #include "lib/core/CHIPSafeCasts.h" #include "lib/mdns/TxtFields.h" #include "lib/mdns/platform/Mdns.h" @@ -36,7 +37,9 @@ namespace chip { namespace Mdns { DiscoveryImplPlatform DiscoveryImplPlatform::sManager; +#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 MdnsCache DiscoveryImplPlatform::sMdnsCache; +#endif DiscoveryImplPlatform::DiscoveryImplPlatform() = default; @@ -439,11 +442,13 @@ CHIP_ERROR DiscoveryImplPlatform::ResolveNodeId(const PeerId & peerId, Inet::IPA { ReturnErrorOnFailure(Init()); - Inet::IPaAddress addr; +#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 + Inet::IPAddress addr; uint16_t port; Inet::InterfaceId iface; /* see if the entry is cached and use it.... */ + if (sMdnsCache.Lookup(peerId, addr, port, iface) == CHIP_NO_ERROR) { ResolvedNodeData nodeData; @@ -457,6 +462,7 @@ CHIP_ERROR DiscoveryImplPlatform::ResolveNodeId(const PeerId & peerId, Inet::IPA return CHIP_NO_ERROR; } +#endif MdnsService service; @@ -560,6 +566,7 @@ void DiscoveryImplPlatform::HandleNodeIdResolve(void * context, MdnsService * re return; } +#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 // TODO -- define appropriate TTL, for now use 2000 msec (rfc default) // figure out way to use TTL value from mDNS packet in future update error = mgr->sMdnsCache.Insert(nodeData.mPeerId, result->mAddress.Value(), result->mPort, result->mInterface, 2 * 1000); @@ -568,6 +575,7 @@ void DiscoveryImplPlatform::HandleNodeIdResolve(void * context, MdnsService * re { ChipLogError(Discovery, "MdnsCache insert failed with %s", chip::ErrorStr(error)); } +#endif Platform::CopyString(nodeData.mHostName, result->mHostName); nodeData.mInterfaceId = result->mInterface; diff --git a/src/lib/mdns/Discovery_ImplPlatform.h b/src/lib/mdns/Discovery_ImplPlatform.h index 36da664ab835b5..bedbbfbcf4140a 100644 --- a/src/lib/mdns/Discovery_ImplPlatform.h +++ b/src/lib/mdns/Discovery_ImplPlatform.h @@ -17,9 +17,11 @@ #pragma once +#include #include #include #include +#include #include #include #include @@ -97,6 +99,9 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver ResolverDelegate * mResolverDelegate = nullptr; static DiscoveryImplPlatform sManager; +#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 + static MdnsCache sMdnsCache; +#endif }; } // namespace Mdns diff --git a/src/lib/mdns/MdnsCache.h b/src/lib/mdns/MdnsCache.h index c47ebd7ce4d767..c4dd1422155e00 100644 --- a/src/lib/mdns/MdnsCache.h +++ b/src/lib/mdns/MdnsCache.h @@ -1,3 +1,20 @@ +/* + * + * Copyright (c) 2021 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #pragma once #include @@ -26,9 +43,6 @@ class MdnsCache public: MdnsCache() : elementsUsed(CACHE_SIZE) { - nullPeerId.SetFabricId(kUndefinedFabricId); - nullPeerId.SetNodeId(kUndefinedNodeId); - for (MdnsCacheEntry & e : mLookupTable) { // each unused entry decrements the count @@ -130,7 +144,7 @@ class MdnsCache uint64_t TTL; // from mdns record -- units? uint64_t expiryTime; // units? }; - PeerId nullPeerId; // indicates a cache entr is unused + PeerId nullPeerId; // indicates a cache entry is unused int elementsUsed; // running count of how many entries are used -- for a sanity check MdnsCacheEntry mLookupTable[CACHE_SIZE]; @@ -157,9 +171,17 @@ class MdnsCache for (MdnsCacheEntry & entry : mLookupTable) { if (entry.peerId == peerId) - return &entry; - if (entry.peerId != nullPeerId && entry.expiryTime <= current_time) - { // ml -- should I use < or <=? + { + if (entry.expiryTime < current_time) + { + MarkEntryUnused(entry); + break; // return nullptr + } + else + return &entry; + } + if (entry.peerId != nullPeerId && entry.expiryTime < current_time) + { MarkEntryUnused(entry); } } @@ -176,7 +198,7 @@ class MdnsCache }; #ifndef MDNS_LOGGING -#undef ChipLogProgress +#undef MdnsLogProgress #endif } // namespace Mdns diff --git a/src/lib/mdns/tests/TestMdnsCache.cpp b/src/lib/mdns/tests/TestMdnsCache.cpp index facb1c9efbed6d..6cb5a4580728f6 100644 --- a/src/lib/mdns/tests/TestMdnsCache.cpp +++ b/src/lib/mdns/tests/TestMdnsCache.cpp @@ -69,7 +69,7 @@ void TestInsert(nlTestSuite * inSuite, void * inContext) CHIP_ERROR result; // ml -- why doesn't adding 2 uint16_t give a uint16_t? - peerId.SetNodeId(id + i); + peerId.SetNodeId((NodeId) id + i); result = tMdnsCache.Insert(peerId, addr, (uint16_t)(port + i), iface, 1000 * ttl); if (i < sizeOfCache) { @@ -89,7 +89,7 @@ void TestInsert(nlTestSuite * inSuite, void * inContext) { CHIP_ERROR result; - peerId.SetNodeId(id + i); + peerId.SetNodeId((NodeId) id + i); result = tMdnsCache.Insert(peerId, addr, (uint16_t)(port + i), iface, 1000 * ttl); NL_TEST_ASSERT(inSuite, result == CHIP_NO_ERROR); } @@ -99,7 +99,7 @@ void TestInsert(nlTestSuite * inSuite, void * inContext) { CHIP_ERROR result; - peerId.SetNodeId(id + i); + peerId.SetNodeId((NodeId) id + i); result = tMdnsCache.Delete(peerId); NL_TEST_ASSERT(inSuite, result == CHIP_NO_ERROR); } @@ -113,7 +113,7 @@ void TestInsert(nlTestSuite * inSuite, void * inContext) { CHIP_ERROR result; - peerId.SetNodeId(id + i); + peerId.SetNodeId((NodeId) id + i); result = tMdnsCache.Insert(peerId, addrV6, (uint16_t)(port + i), iface, 1000 * ttl); NL_TEST_ASSERT(inSuite, result == CHIP_NO_ERROR); }