Skip to content

Commit

Permalink
Remove time source template argument of sessions
Browse files Browse the repository at this point in the history
  • Loading branch information
kghost committed Dec 9, 2021
1 parent 564fa06 commit 926198b
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 75 deletions.
3 changes: 2 additions & 1 deletion src/app/CASEClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ CHIP_ERROR CASEClient::EstablishSession(PeerId peer, const Transport::PeerAddres
{
// Create a UnauthenticatedSession for CASE pairing.
// Don't use mSecureSession here, because mSecureSession is for encrypted communication.
Optional<SessionHandle> session = mInitParams.sessionManager->CreateUnauthenticatedSession(peerAddress, mrpConfig);
Optional<SessionHandle> session = mInitParams.sessionManager->CreateUnauthenticatedSession(
peerAddress, System::SystemClock().GetMonotonicTimestamp(), mrpConfig);
VerifyOrReturnError(session.HasValue(), CHIP_ERROR_NO_MEMORY);

Messaging::ExchangeContext * exchange = mInitParams.exchangeMgr->NewContext(session.Value(), &mCASESession);
Expand Down
3 changes: 2 additions & 1 deletion src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,8 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re
}
#endif
// TODO: In some cases like PASE over IP, CRA and CRI values from commissionable node service should be used
session = mSystemState->SessionMgr()->CreateUnauthenticatedSession(params.GetPeerAddress(), device->GetMRPConfig());
session = mSystemState->SessionMgr()->CreateUnauthenticatedSession(
params.GetPeerAddress(), System::SystemClock().GetMonotonicTimestamp(), device->GetMRPConfig());
VerifyOrExit(session.HasValue(), err = CHIP_ERROR_NO_MEMORY);

exchangeCtxt = mSystemState->ExchangeMgr()->NewContext(session.Value(), &device->GetPairing());
Expand Down
13 changes: 9 additions & 4 deletions src/messaging/tests/MessagingContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,19 @@ SessionHandle MessagingContext::GetSessionBobToFriends()

Messaging::ExchangeContext * MessagingContext::NewUnauthenticatedExchangeToAlice(Messaging::ExchangeDelegate * delegate)
{
return mExchangeManager.NewContext(mSessionManager.CreateUnauthenticatedSession(mAliceAddress, gDefaultMRPConfig).Value(),
delegate);
return mExchangeManager.NewContext(
mSessionManager
.CreateUnauthenticatedSession(mAliceAddress, System::SystemClock().GetMonotonicTimestamp(), gDefaultMRPConfig)
.Value(),
delegate);
}

Messaging::ExchangeContext * MessagingContext::NewUnauthenticatedExchangeToBob(Messaging::ExchangeDelegate * delegate)
{
return mExchangeManager.NewContext(mSessionManager.CreateUnauthenticatedSession(mBobAddress, gDefaultMRPConfig).Value(),
delegate);
return mExchangeManager.NewContext(
mSessionManager.CreateUnauthenticatedSession(mBobAddress, System::SystemClock().GetMonotonicTimestamp(), gDefaultMRPConfig)
.Value(),
delegate);
}

Messaging::ExchangeContext * MessagingContext::NewExchangeToAlice(Messaging::ExchangeDelegate * delegate)
Expand Down
10 changes: 4 additions & 6 deletions src/transport/SecureSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,12 @@ class SecureSession
};

SecureSession(Type secureSessionType, uint16_t localSessionId, NodeId peerNodeId, Credentials::CATValues peerCATs,
uint16_t peerSessionId, FabricIndex fabric, const ReliableMessageProtocolConfig & config,
System::Clock::Timestamp currentTime) :
uint16_t peerSessionId, FabricIndex fabric, System::Clock::Timestamp now,
const ReliableMessageProtocolConfig & config) :
mSecureSessionType(secureSessionType),
mPeerNodeId(peerNodeId), mPeerCATs(peerCATs), mLocalSessionId(localSessionId), mPeerSessionId(peerSessionId),
mFabric(fabric), mMRPConfig(config)
{
SetLastActivityTime(currentTime);
}
mFabric(fabric), mLastActivityTime(now), mMRPConfig(config)
{}

SecureSession(SecureSession &&) = delete;
SecureSession(const SecureSession &) = delete;
Expand Down
18 changes: 6 additions & 12 deletions src/transport/SecureSessionTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ constexpr const uint16_t kAnyKeyId = 0xffff;
* - handle session active time and expiration
* - allocate and free space for sessions.
*/
template <size_t kMaxSessionCount, Time::Source kTimeSource = Time::Source::kSystem>
template <size_t kMaxSessionCount>
class SecureSessionTable
{
public:
Expand All @@ -60,10 +60,9 @@ class SecureSessionTable
CHECK_RETURN_VALUE
SecureSession * CreateNewSecureSession(SecureSession::Type secureSessionType, uint16_t localSessionId, NodeId peerNodeId,
Credentials::CATValues peerCATs, uint16_t peerSessionId, FabricIndex fabric,
const ReliableMessageProtocolConfig & config)
System::Clock::Timestamp now, const ReliableMessageProtocolConfig & config)
{
return mEntries.CreateObject(secureSessionType, localSessionId, peerNodeId, peerCATs, peerSessionId, fabric, config,
mTimeSource.GetMonotonicTimestamp());
return mEntries.CreateObject(secureSessionType, localSessionId, peerNodeId, peerCATs, peerSessionId, fabric, now, config);
}

void ReleaseSession(SecureSession * session) { mEntries.ReleaseObject(session); }
Expand Down Expand Up @@ -97,7 +96,7 @@ class SecureSessionTable
}

/// Convenience method to mark a session as active
void MarkSessionActive(SecureSession * state) { state->SetLastActivityTime(mTimeSource.GetMonotonicTimestamp()); }
void MarkSessionActive(System::Clock::Timestamp now, SecureSession * state) { state->SetLastActivityTime(now); }

/**
* Iterates through all active sessions and expires any sessions with an idle time
Expand All @@ -106,11 +105,10 @@ class SecureSessionTable
* Expiring a session involves callback execution and then clearing the internal state.
*/
template <typename Callback>
void ExpireInactiveSessions(System::Clock::Timestamp maxIdleTime, Callback callback)
void ExpireInactiveSessions(System::Clock::Timestamp now, System::Clock::Timestamp maxIdleTime, Callback callback)
{
const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp();
mEntries.ForEachActiveObject([&](auto session) {
if (session->GetLastActivityTime() + maxIdleTime < currentTime)
if (session->GetLastActivityTime() + maxIdleTime < now)
{
callback(*session);
ReleaseSession(session);
Expand All @@ -119,11 +117,7 @@ class SecureSessionTable
});
}

/// Allows access to the underlying time source used for keeping track of session active time
Time::TimeSource<kTimeSource> & GetTimeSource() { return mTimeSource; }

private:
Time::TimeSource<kTimeSource> mTimeSource;
BitMapObjectPool<SecureSession, kMaxSessionCount, OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode> mEntries;
};

Expand Down
16 changes: 9 additions & 7 deletions src/transport/SessionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ CHIP_ERROR SessionManager::SendPreparedMessage(SessionHandle sessionHandle, cons
}

// This marks any connection where we send data to as 'active'
mSecureSessions.MarkSessionActive(session);
mSecureSessions.MarkSessionActive(System::SystemClock().GetMonotonicTimestamp(), session);

destination = &session->GetPeerAddress();

Expand All @@ -237,7 +237,7 @@ CHIP_ERROR SessionManager::SendPreparedMessage(SessionHandle sessionHandle, cons
else
{
auto unauthenticated = sessionHandle.GetUnauthenticatedSession();
mUnauthenticatedSessions.MarkSessionActive(unauthenticated);
mUnauthenticatedSessions.MarkSessionActive(System::SystemClock().GetMonotonicTimestamp(), unauthenticated);
destination = &unauthenticated->GetPeerAddress();

ChipLogProgress(Inet,
Expand Down Expand Up @@ -314,7 +314,8 @@ CHIP_ERROR SessionManager::NewPairing(const Optional<Transport::PeerAddress> & p
ChipLogDetail(Inet, "New secure session created for device 0x" ChipLogFormatX64 ", key %d!!", ChipLogValueX64(peerNodeId),
peerSessionId);
session = mSecureSessions.CreateNewSecureSession(pairing->GetSecureSessionType(), localSessionId, peerNodeId,
pairing->GetPeerCATs(), peerSessionId, fabric, pairing->GetMRPConfig());
pairing->GetPeerCATs(), peerSessionId, fabric,
System::SystemClock().GetMonotonicTimestamp(), pairing->GetMRPConfig());
ReturnErrorCodeIf(session == nullptr, CHIP_ERROR_NO_MEMORY);

if (peerAddr.HasValue() && peerAddr.Value().GetIPAddress() != Inet::IPAddress::Any)
Expand Down Expand Up @@ -419,7 +420,7 @@ void SessionManager::MessageDispatch(const PacketHeader & packetHeader, const Tr
System::PacketBufferHandle && msg)
{
Optional<Transport::UnauthenticatedSessionHandle> optionalSession =
mUnauthenticatedSessions.FindOrAllocateEntry(peerAddress, gDefaultMRPConfig);
mUnauthenticatedSessions.FindOrAllocateEntry(peerAddress, System::SystemClock().GetMonotonicTimestamp(), gDefaultMRPConfig);
if (!optionalSession.HasValue())
{
ChipLogError(Inet, "UnauthenticatedSession exhausted");
Expand All @@ -438,7 +439,7 @@ void SessionManager::MessageDispatch(const PacketHeader & packetHeader, const Tr
}
VerifyOrDie(err == CHIP_NO_ERROR);

mUnauthenticatedSessions.MarkSessionActive(session);
mUnauthenticatedSessions.MarkSessionActive(System::SystemClock().GetMonotonicTimestamp(), session);

PayloadHeader payloadHeader;
ReturnOnFailure(payloadHeader.DecodeAndConsume(msg));
Expand Down Expand Up @@ -501,7 +502,7 @@ void SessionManager::SecureUnicastMessageDispatch(const PacketHeader & packetHea
return;
}

mSecureSessions.MarkSessionActive(session);
mSecureSessions.MarkSessionActive(System::SystemClock().GetMonotonicTimestamp(), session);

if (isDuplicate == SessionMessageDelegate::DuplicateMessage::Yes && !payloadHeader.NeedsAck())
{
Expand Down Expand Up @@ -627,7 +628,8 @@ void SessionManager::ExpiryTimerCallback(System::Layer * layer, void * param)
// TODO(#2279): session expiration is currently disabled until rekeying is supported
// the #ifdef should be removed after that.
mgr->mSecureSessions.ExpireInactiveSessions(
CHIP_PEER_CONNECTION_TIMEOUT_MS, [this](const Transport::SecureSession & state1) { HandleConnectionExpired(state1); });
System::SystemClock().GetMonotonicTimestamp(), System::Clock::Milliseconds32(CHIP_PEER_CONNECTION_TIMEOUT_MS),
[this](const Transport::SecureSession & state1) { HandleConnectionExpired(state1); });
#endif
mgr->ScheduleExpiryTimer(); // re-schedule the oneshot timer
}
Expand Down
4 changes: 2 additions & 2 deletions src/transport/SessionManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,11 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate
*/
void OnMessageReceived(const Transport::PeerAddress & source, System::PacketBufferHandle && msgBuf) override;

Optional<SessionHandle> CreateUnauthenticatedSession(const Transport::PeerAddress & peerAddress,
Optional<SessionHandle> CreateUnauthenticatedSession(const Transport::PeerAddress & peerAddress, System::Clock::Timestamp now,
const ReliableMessageProtocolConfig & config)
{
Optional<Transport::UnauthenticatedSessionHandle> session =
mUnauthenticatedSessions.FindOrAllocateEntry(peerAddress, config);
mUnauthenticatedSessions.FindOrAllocateEntry(peerAddress, now, config);
return session.HasValue() ? MakeOptional<SessionHandle>(session.Value()) : NullOptional;
}

Expand Down
29 changes: 13 additions & 16 deletions src/transport/UnauthenticatedSessionTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,10 @@ class UnauthenticatedSessionDeleter
class UnauthenticatedSession : public ReferenceCounted<UnauthenticatedSession, UnauthenticatedSessionDeleter, 0>
{
public:
UnauthenticatedSession(const PeerAddress & address, const ReliableMessageProtocolConfig & config) :
mPeerAddress(address), mMRPConfig(config)
UnauthenticatedSession(const PeerAddress & address, System::Clock::Timestamp now,
const ReliableMessageProtocolConfig & config) :
mPeerAddress(address),
mLastActivityTime(now), mMRPConfig(config)
{}

UnauthenticatedSession(const UnauthenticatedSession &) = delete;
Expand All @@ -69,9 +71,8 @@ class UnauthenticatedSession : public ReferenceCounted<UnauthenticatedSession, U
PeerMessageCounter & GetPeerMessageCounter() { return mPeerMessageCounter; }

private:
System::Clock::Timestamp mLastActivityTime = System::Clock::kZero;

const PeerAddress mPeerAddress;
System::Clock::Timestamp mLastActivityTime;
ReliableMessageProtocolConfig mMRPConfig;
PeerMessageCounter mPeerMessageCounter;
};
Expand All @@ -84,7 +85,7 @@ class UnauthenticatedSession : public ReferenceCounted<UnauthenticatedSession, U
* hold by using UnauthenticatedSessionHandle, which increase the reference
* count by 1. If the reference count is not 0, the entry won't be pruned.
*/
template <size_t kMaxSessionCount, Time::Source kTimeSource = Time::Source::kSystem>
template <size_t kMaxSessionCount>
class UnauthenticatedSessionTable
{
public:
Expand All @@ -94,14 +95,14 @@ class UnauthenticatedSessionTable
* @return the session found or allocated, nullptr if not found and allocation failed.
*/
CHECK_RETURN_VALUE
Optional<UnauthenticatedSessionHandle> FindOrAllocateEntry(const PeerAddress & address,
Optional<UnauthenticatedSessionHandle> FindOrAllocateEntry(const PeerAddress & address, System::Clock::Timestamp now,
const ReliableMessageProtocolConfig & config)
{
UnauthenticatedSession * result = FindEntry(address);
if (result != nullptr)
return MakeOptional<UnauthenticatedSessionHandle>(*result);

CHIP_ERROR err = AllocEntry(address, config, result);
CHIP_ERROR err = AllocEntry(address, now, config, result);
if (err == CHIP_NO_ERROR)
{
return MakeOptional<UnauthenticatedSessionHandle>(*result);
Expand All @@ -113,14 +114,11 @@ class UnauthenticatedSessionTable
}

/// Mark a session as active
void MarkSessionActive(UnauthenticatedSessionHandle session)
void MarkSessionActive(System::Clock::Timestamp now, UnauthenticatedSessionHandle session)
{
session->SetLastActivityTime(mTimeSource.GetMonotonicTimestamp());
session->SetLastActivityTime(now);
}

/// Allows access to the underlying time source used for keeping track of session active time
Time::TimeSource<kTimeSource> & GetTimeSource() { return mTimeSource; }

private:
/**
* Allocates a new session out of the internal resource pool.
Expand All @@ -129,10 +127,10 @@ class UnauthenticatedSessionTable
* CHIP_ERROR_NO_MEMORY).
*/
CHECK_RETURN_VALUE
CHIP_ERROR AllocEntry(const PeerAddress & address, const ReliableMessageProtocolConfig & config,
CHIP_ERROR AllocEntry(const PeerAddress & address, System::Clock::Timestamp now, const ReliableMessageProtocolConfig & config,
UnauthenticatedSession *& entry)
{
entry = mEntries.CreateObject(address, config);
entry = mEntries.CreateObject(address, now, config);
if (entry != nullptr)
return CHIP_NO_ERROR;

Expand All @@ -142,7 +140,7 @@ class UnauthenticatedSessionTable
return CHIP_ERROR_NO_MEMORY;
}

mEntries.ResetObject(entry, address, config);
mEntries.ResetObject(entry, address, now, config);
return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -222,7 +220,6 @@ class UnauthenticatedSessionTable
return false;
}

Time::TimeSource<Time::Source::kSystem> mTimeSource;
BitMapObjectPool<UnauthenticatedSession, kMaxSessionCount, OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode> mEntries;
};

Expand Down
Loading

0 comments on commit 926198b

Please sign in to comment.