Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert TimeSource to safer Clock types #11449

Merged
merged 1 commit into from
Nov 5, 2021
Merged

Convert TimeSource to safer Clock types #11449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/app/server/Dnssd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ void OnPlatformEventWrapper(const DeviceLayer::ChipDeviceEvent * event, intptr_t

} // namespace

constexpr System::Clock::Timestamp DnssdServer::kTimeoutCleared;

#if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY

constexpr const char kExtendedDiscoveryTimeoutKeypairStorage[] = "ExtDiscKey";
Expand Down Expand Up @@ -110,15 +112,15 @@ void HandleExtendedDiscoveryExpiration(System::Layer * aSystemLayer, void * aApp

void DnssdServer::OnExtendedDiscoveryExpiration(System::Layer * aSystemLayer, void * aAppState)
{
if (!DnssdServer::OnExpiration(mExtendedDiscoveryExpirationMs))
if (!DnssdServer::OnExpiration(mExtendedDiscoveryExpiration))
{
ChipLogDetail(Discovery, "OnExtendedDiscoveryExpiration callback for cleared session");
return;
}

ChipLogDetail(Discovery, "OnExtendedDiscoveryExpiration callback for valid session");

mExtendedDiscoveryExpirationMs = TIMEOUT_CLEARED;
mExtendedDiscoveryExpiration = kTimeoutCleared;
}
#endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY

Expand All @@ -128,14 +130,14 @@ void HandleDiscoveryExpiration(System::Layer * aSystemLayer, void * aAppState)
DnssdServer::Instance().OnDiscoveryExpiration(aSystemLayer, aAppState);
}

bool DnssdServer::OnExpiration(uint64_t expirationMs)
bool DnssdServer::OnExpiration(System::Clock::Timestamp expirationMs)
{
if (expirationMs == TIMEOUT_CLEARED)
if (expirationMs == kTimeoutCleared)
{
ChipLogDetail(Discovery, "OnExpiration callback for cleared session");
return false;
}
uint64_t now = mTimeSource.GetCurrentMonotonicTimeMs();
System::Clock::Timestamp now = mTimeSource.GetMonotonicTimestamp();
if (expirationMs > now)
{
ChipLogDetail(Discovery, "OnExpiration callback for reset session");
Expand Down Expand Up @@ -183,7 +185,7 @@ bool DnssdServer::OnExpiration(uint64_t expirationMs)

void DnssdServer::OnDiscoveryExpiration(System::Layer * aSystemLayer, void * aAppState)
{
if (!DnssdServer::OnExpiration(mDiscoveryExpirationMs))
if (!DnssdServer::OnExpiration(mDiscoveryExpiration))
{
ChipLogDetail(Discovery, "OnDiscoveryExpiration callback for cleared session");
return;
Expand All @@ -205,7 +207,7 @@ void DnssdServer::OnDiscoveryExpiration(System::Layer * aSystemLayer, void * aAp
}
#endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY

mDiscoveryExpirationMs = TIMEOUT_CLEARED;
mDiscoveryExpiration = kTimeoutCleared;
}

CHIP_ERROR DnssdServer::ScheduleDiscoveryExpiration()
Expand All @@ -216,7 +218,7 @@ CHIP_ERROR DnssdServer::ScheduleDiscoveryExpiration()
}
ChipLogDetail(Discovery, "Scheduling Discovery timeout in secs=%d", mDiscoveryTimeoutSecs);

mDiscoveryExpirationMs = mTimeSource.GetCurrentMonotonicTimeMs() + static_cast<uint64_t>(mDiscoveryTimeoutSecs) * 1000;
mDiscoveryExpiration = mTimeSource.GetMonotonicTimestamp() + System::Clock::Seconds16(mDiscoveryTimeoutSecs);

return DeviceLayer::SystemLayer().StartTimer(System::Clock::Seconds16(mDiscoveryTimeoutSecs), HandleDiscoveryExpiration,
nullptr);
Expand All @@ -232,8 +234,7 @@ CHIP_ERROR DnssdServer::ScheduleExtendedDiscoveryExpiration()
}
ChipLogDetail(Discovery, "Scheduling Extended Discovery timeout in secs=%d", extendedDiscoveryTimeoutSecs);

mExtendedDiscoveryExpirationMs =
mTimeSource.GetCurrentMonotonicTimeMs() + static_cast<uint64_t>(extendedDiscoveryTimeoutSecs) * 1000;
mExtendedDiscoveryExpiration = mTimeSource.GetMonotonicTimestamp() + System::Clock::Seconds16(extendedDiscoveryTimeoutSecs);

return DeviceLayer::SystemLayer().StartTimer(System::Clock::Seconds16(extendedDiscoveryTimeoutSecs),
HandleExtendedDiscoveryExpiration, nullptr);
Expand Down
15 changes: 8 additions & 7 deletions src/app/server/Dnssd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@
namespace chip {
namespace app {

#define TIMEOUT_CLEARED 0
class DLL_EXPORT DnssdServer
{
public:
static constexpr System::Clock::Timestamp kTimeoutCleared = System::Clock::kZero;

/// Provides the system-wide implementation of the service advertiser
static DnssdServer & Instance()
{
Expand Down Expand Up @@ -104,9 +105,9 @@ class DLL_EXPORT DnssdServer

void ClearTimeouts()
{
mDiscoveryExpirationMs = TIMEOUT_CLEARED;
mDiscoveryExpiration = kTimeoutCleared;
#if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
mExtendedDiscoveryExpirationMs = TIMEOUT_CLEARED;
mExtendedDiscoveryExpiration = kTimeoutCleared;
#endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
}

Expand All @@ -115,11 +116,11 @@ class DLL_EXPORT DnssdServer

/// schedule next discovery expiration
CHIP_ERROR ScheduleDiscoveryExpiration();
int16_t mDiscoveryTimeoutSecs = CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS;
uint64_t mDiscoveryExpirationMs = TIMEOUT_CLEARED;
int16_t mDiscoveryTimeoutSecs = CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS;
System::Clock::Timestamp mDiscoveryExpiration = kTimeoutCleared;

/// return true if expirationMs is valid (not cleared and not in the future)
bool OnExpiration(uint64_t expirationMs);
bool OnExpiration(System::Clock::Timestamp expiration);

#if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
/// get the current extended discovery timeout (from persistent storage)
Expand All @@ -128,7 +129,7 @@ class DLL_EXPORT DnssdServer
/// schedule next extended discovery expiration
CHIP_ERROR ScheduleExtendedDiscoveryExpiration();

uint64_t mExtendedDiscoveryExpirationMs = TIMEOUT_CLEARED;
System::Clock::Timestamp mExtendedDiscoveryExpiration = kTimeoutCleared;
#endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
};

Expand Down
3 changes: 2 additions & 1 deletion src/lib/dnssd/Discovery_ImplPlatform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,8 @@ void DiscoveryImplPlatform::HandleNodeIdResolve(void * context, DnssdService * r
#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->sDnssdCache.Insert(nodeData.mPeerId, result->mAddress.Value(), result->mPort, result->mInterface, 2 * 1000);
error = mgr->sDnssdCache.Insert(nodeData.mPeerId, result->mAddress.Value(), result->mPort, result->mInterface,
System::Clock::Seconds16(2));

if (CHIP_NO_ERROR != error)
{
Expand Down
25 changes: 13 additions & 12 deletions src/lib/dnssd/DnssdCache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,19 @@ class DnssdCache
// return error if cache is full
// TODO: have an eviction policy so if the cache is full, an entry may be deleted.
// One policy may be Least-time-to-live
CHIP_ERROR Insert(PeerId peerId, const Inet::IPAddress & addr, uint16_t port, Inet::InterfaceId iface, uint32_t TTLms)
CHIP_ERROR Insert(PeerId peerId, const Inet::IPAddress & addr, uint16_t port, Inet::InterfaceId iface,
System::Clock::Timestamp TTL)
{
const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs();
const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp();

DnssdCacheEntry * entry;

entry = FindPeerId(peerId, currentTime);
if (entry)
{
// update timeout if found entry
entry->expiryTime = currentTime + TTLms;
entry->TTL = TTLms; // in case it changes */
entry->expiryTime = currentTime + TTL;
entry->TTL = TTL; // in case it changes */
return CHIP_NO_ERROR;
}

Expand All @@ -77,8 +78,8 @@ class DnssdCache
entry->ipAddr = addr;
entry->port = port;
entry->ifaceId = iface;
entry->TTL = TTLms;
entry->expiryTime = currentTime + TTLms;
entry->TTL = TTL;
entry->expiryTime = currentTime + TTL;
elementsUsed++;

return CHIP_NO_ERROR;
Expand All @@ -87,7 +88,7 @@ class DnssdCache
CHIP_ERROR Delete(PeerId peerId)
{
DnssdCacheEntry * pentry;
const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs();
const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp();

VerifyOrReturnError(pentry = FindPeerId(peerId, currentTime), CHIP_ERROR_KEY_NOT_FOUND);

Expand All @@ -99,7 +100,7 @@ class DnssdCache
CHIP_ERROR Lookup(PeerId peerId, Inet::IPAddress & addr, uint16_t & port, Inet::InterfaceId & iface)
{
DnssdCacheEntry * pentry;
const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs();
const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp();

VerifyOrReturnError(pentry = FindPeerId(peerId, currentTime), CHIP_ERROR_KEY_NOT_FOUND);

Expand Down Expand Up @@ -141,16 +142,16 @@ class DnssdCache
Inet::IPAddress ipAddr;
uint16_t port;
Inet::InterfaceId ifaceId;
uint64_t TTL; // from mdns record -- units?
uint64_t expiryTime; // units?
System::Clock::Timestamp TTL;
System::Clock::Timestamp expiryTime;
};
PeerId nullPeerId; // indicates a cache entry is unused
int elementsUsed; // running count of how many entries are used -- for a sanity check

DnssdCacheEntry mLookupTable[CACHE_SIZE];
Time::TimeSource<Time::Source::kSystem> mTimeSource;

DnssdCacheEntry * findSlot(uint64_t currentTime)
DnssdCacheEntry * findSlot(System::Clock::Timestamp currentTime)
{
for (DnssdCacheEntry & entry : mLookupTable)
{
Expand All @@ -166,7 +167,7 @@ class DnssdCache
return nullptr;
}

DnssdCacheEntry * FindPeerId(PeerId peerId, uint64_t current_time)
DnssdCacheEntry * FindPeerId(PeerId peerId, System::Clock::Timestamp current_time)
{
for (DnssdCacheEntry & entry : mLookupTable)
{
Expand Down
10 changes: 5 additions & 5 deletions src/lib/dnssd/tests/TestDnssdCache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void TestInsert(nlTestSuite * inSuite, void * inContext)

Inet::IPAddress addr;
Inet::IPAddress addrV6;
const int ttl = 2; /* seconds */
const System::Clock::Timestamp ttl = System::Clock::Seconds16(2);
Inet::InterfaceId iface_out;
Inet::IPAddress addr_out;
uint16_t port_out;
Expand All @@ -71,7 +71,7 @@ void TestInsert(nlTestSuite * inSuite, void * inContext)

// ml -- why doesn't adding 2 uint16_t give a uint16_t?
peerId.SetNodeId((NodeId) id + i);
result = tDnssdCache.Insert(peerId, addr, (uint16_t)(port + i), iface, 1000 * ttl);
result = tDnssdCache.Insert(peerId, addr, (uint16_t)(port + i), iface, ttl);
if (i < sizeOfCache)
{
NL_TEST_ASSERT(inSuite, result == CHIP_NO_ERROR);
Expand All @@ -83,15 +83,15 @@ void TestInsert(nlTestSuite * inSuite, void * inContext)
}

tDnssdCache.DumpCache();
usleep(static_cast<useconds_t>((ttl + 1) * 1000 * 1000));
usleep(static_cast<useconds_t>(std::chrono::microseconds(ttl + System::Clock::Seconds16(1)).count()));
id = 0x200;
port = 3000;
for (uint16_t i = 0; i < sizeOfCache; i++)
{
CHIP_ERROR result;

peerId.SetNodeId((NodeId) id + i);
result = tDnssdCache.Insert(peerId, addr, (uint16_t)(port + i), iface, 1000 * ttl);
result = tDnssdCache.Insert(peerId, addr, (uint16_t)(port + i), iface, ttl);
NL_TEST_ASSERT(inSuite, result == CHIP_NO_ERROR);
}
tDnssdCache.DumpCache();
Expand All @@ -115,7 +115,7 @@ void TestInsert(nlTestSuite * inSuite, void * inContext)
CHIP_ERROR result;

peerId.SetNodeId((NodeId) id + i);
result = tDnssdCache.Insert(peerId, addrV6, (uint16_t)(port + i), iface, 1000 * ttl);
result = tDnssdCache.Insert(peerId, addrV6, (uint16_t)(port + i), iface, ttl);
NL_TEST_ASSERT(inSuite, result == CHIP_NO_ERROR);
}

Expand Down
12 changes: 6 additions & 6 deletions src/protocols/user_directed_commissioning/UDCClientState.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ class UDCClientState
UDCClientProcessingState GetUDCClientProcessingState() const { return mUDCClientProcessingState; }
void SetUDCClientProcessingState(UDCClientProcessingState state) { mUDCClientProcessingState = state; }

uint64_t GetExpirationTimeMs() const { return mExpirationTimeMs; }
void SetExpirationTimeMs(uint64_t value) { mExpirationTimeMs = value; }
System::Clock::Timestamp GetExpirationTime() const { return mExpirationTime; }
void SetExpirationTime(System::Clock::Timestamp value) { mExpirationTime = value; }

bool IsInitialized(uint64_t currentTime)
bool IsInitialized(System::Clock::Timestamp currentTime)
{
// if state is not the "not-initialized" and it has not expired
return (mUDCClientProcessingState != UDCClientProcessingState::kNotInitialized && mExpirationTimeMs > currentTime);
return (mUDCClientProcessingState != UDCClientProcessingState::kNotInitialized && mExpirationTime > currentTime);
}

/**
Expand All @@ -93,7 +93,7 @@ class UDCClientState
void Reset()
{
mPeerAddress = PeerAddress::Uninitialized();
mExpirationTimeMs = 0;
mExpirationTime = System::Clock::kZero;
mUDCClientProcessingState = UDCClientProcessingState::kNotInitialized;
}

Expand All @@ -103,7 +103,7 @@ class UDCClientState
char mDeviceName[Dnssd::kMaxDeviceNameLen + 1];
uint16_t mLongDiscriminator = 0;
UDCClientProcessingState mUDCClientProcessingState;
uint64_t mExpirationTimeMs = 0;
System::Clock::Timestamp mExpirationTime = System::Clock::kZero;
};

} // namespace UserDirectedCommissioning
Expand Down
16 changes: 8 additions & 8 deletions src/protocols/user_directed_commissioning/UDCClients.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ namespace Protocols {
namespace UserDirectedCommissioning {

// UDC client state times out after 1 hour. This may need to be tweaked.
constexpr const uint64_t kUDCClientTimeoutMs = 60 * 60 * 1000;
constexpr const System::Clock::Timestamp kUDCClientTimeout = System::Clock::Milliseconds64(60 * 60 * 1000);

/**
* Handles a set of UDC Client Processing States.
Expand Down Expand Up @@ -54,7 +54,7 @@ class UDCClients
CHECK_RETURN_VALUE
CHIP_ERROR CreateNewUDCClientState(const char * instanceName, UDCClientState ** state)
{
const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs();
const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp();

CHIP_ERROR err = CHIP_ERROR_NO_MEMORY;

Expand All @@ -68,7 +68,7 @@ class UDCClients
if (!stateiter.IsInitialized(currentTime))
{
stateiter.SetInstanceName(instanceName);
stateiter.SetExpirationTimeMs(currentTime + kUDCClientTimeoutMs);
stateiter.SetExpirationTime(currentTime + kUDCClientTimeout);
stateiter.SetUDCClientProcessingState(UDCClientProcessingState::kDiscoveringNode);

if (state)
Expand Down Expand Up @@ -99,8 +99,8 @@ class UDCClients
return nullptr;
}

const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs();
UDCClientState state = mStates[index];
const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp();
UDCClientState state = mStates[index];
if (!state.IsInitialized(currentTime))
{
return nullptr;
Expand All @@ -118,7 +118,7 @@ class UDCClients
CHECK_RETURN_VALUE
UDCClientState * FindUDCClientState(const PeerAddress & address)
{
const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs();
const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp();

UDCClientState * state = nullptr;

Expand Down Expand Up @@ -147,7 +147,7 @@ class UDCClients
CHECK_RETURN_VALUE
UDCClientState * FindUDCClientState(const char * instanceName)
{
const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs();
const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp();

UDCClientState * state = nullptr;

Expand Down Expand Up @@ -180,7 +180,7 @@ class UDCClients
/// Convenience method to mark a UDC Client state as active (non-expired)
void MarkUDCClientActive(UDCClientState * state)
{
state->SetExpirationTimeMs(mTimeSource.GetCurrentMonotonicTimeMs() + kUDCClientTimeoutMs);
state->SetExpirationTime(mTimeSource.GetMonotonicTimestamp() + kUDCClientTimeout);
}

private:
Expand Down
8 changes: 4 additions & 4 deletions src/protocols/user_directed_commissioning/tests/TestUdcMessages.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,11 @@ void TestUDCClients(nlTestSuite * inSuite, void * inContext)

// test re-activation
NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR == mUdcClients.CreateNewUDCClientState(instanceName4, &state));
uint64_t expirationTime = state->GetExpirationTimeMs();
state->SetExpirationTimeMs(expirationTime - 1);
NL_TEST_ASSERT(inSuite, (expirationTime - 1) == state->GetExpirationTimeMs());
System::Clock::Timestamp expirationTime = state->GetExpirationTime();
state->SetExpirationTime(expirationTime - System::Clock::Milliseconds64(1));
NL_TEST_ASSERT(inSuite, (expirationTime - System::Clock::Milliseconds64(1)) == state->GetExpirationTime());
mUdcClients.MarkUDCClientActive(state);
NL_TEST_ASSERT(inSuite, (expirationTime - 1) < state->GetExpirationTimeMs());
NL_TEST_ASSERT(inSuite, (expirationTime - System::Clock::Milliseconds64(1)) < state->GetExpirationTime());
}

// Test Suite
Expand Down
Loading