Skip to content

Commit

Permalink
Added comment on CHIP_CONFIG_MDNS_CACHE_SIZE that if 0, builtin cache
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Marty Leisner authored and Marty Leisner committed Aug 26, 2021
1 parent 68139a7 commit eba1f00
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 14 deletions.
6 changes: 5 additions & 1 deletion src/lib/core/CHIPConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion src/lib/mdns/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include <inttypes.h>

#include "core/CHIPConfig.h"
#include "ServiceNaming.h"
#include "lib/core/CHIPSafeCasts.h"
#include "lib/mdns/TxtFields.h"
Expand All @@ -36,7 +37,9 @@ namespace chip {
namespace Mdns {

DiscoveryImplPlatform DiscoveryImplPlatform::sManager;
#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0
MdnsCache<CHIP_CONFIG_MDNS_CACHE_SIZE> DiscoveryImplPlatform::sMdnsCache;
#endif

DiscoveryImplPlatform::DiscoveryImplPlatform() = default;

Expand Down Expand Up @@ -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;
Expand All @@ -457,6 +462,7 @@ CHIP_ERROR DiscoveryImplPlatform::ResolveNodeId(const PeerId & peerId, Inet::IPA

return CHIP_NO_ERROR;
}
#endif

MdnsService service;

Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions src/lib/mdns/Discovery_ImplPlatform.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@
#pragma once

#include <core/CHIPError.h>
#include <core/CHIPConfig.h>
#include <inet/InetInterface.h>
#include <lib/mdns/Advertiser.h>
#include <lib/mdns/Resolver.h>
#include <lib/mdns/platform/Mdns.h>
#include <platform/CHIPDeviceConfig.h>
#include <lib/mdns/MdnsCache.h>

// Enable detailed mDNS logging for publish
#undef DETAIL_LOGGING
Expand Down Expand Up @@ -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 <CHIP_CONFIG_MDNS_CACHE_SIZE> sMdnsCache;
#endif
};

} // namespace Mdns
Expand Down
38 changes: 30 additions & 8 deletions src/lib/mdns/MdnsCache.h
Original file line number Diff line number Diff line change
@@ -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 <cstdint>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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];
Expand All @@ -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);
}
}
Expand All @@ -176,7 +198,7 @@ class MdnsCache
};

#ifndef MDNS_LOGGING
#undef ChipLogProgress
#undef MdnsLogProgress
#endif

} // namespace Mdns
Expand Down
8 changes: 4 additions & 4 deletions src/lib/mdns/tests/TestMdnsCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down

0 comments on commit eba1f00

Please sign in to comment.