Skip to content

Commit

Permalink
Move CASESessionManager to CHIPDeviceControllerSystemState.
Browse files Browse the repository at this point in the history 
We should have one CASESessionManager, not one per DeviceController.

Fixes project-chip#16174
  • Loading branch information
@bzbarsky-apple
bzbarsky-apple committed Mar 15, 2022
1 parent c33b1a3 commit 92baee6
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 55 deletions.
58 changes: 16 additions & 42 deletions src/controller/CHIPDeviceController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,28 +148,6 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params)
}
}

DeviceProxyInitParams deviceInitParams = {
.sessionManager = params.systemState->SessionMgr(),
.exchangeMgr = params.systemState->ExchangeMgr(),
.idAllocator = &mIDAllocator,
.fabricTable = params.systemState->Fabrics(),
.clientPool = &mCASEClientPool,
.mrpLocalConfig = Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()),
};

CASESessionManagerConfig sessionManagerConfig = {
.sessionInitParams = deviceInitParams,
#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0
.dnsCache = &mDNSCache,
#endif
.devicePool = &mDevicePool,
};

mCASESessionManager = chip::Platform::New<CASESessionManager>(sessionManagerConfig);
VerifyOrReturnError(mCASESessionManager != nullptr, CHIP_ERROR_NO_MEMORY);

ReturnErrorOnFailure(mCASESessionManager->Init(params.systemState->SystemLayer()));

mSystemState = params.systemState->Retain();
mState = State::Initialized;
return CHIP_NO_ERROR;
Expand Down Expand Up @@ -243,11 +221,9 @@ CHIP_ERROR DeviceController::Shutdown()

if (mFabricInfo != nullptr)
{
// Shut down any ongoing CASE session activity we have. Note that we
// own the mCASESessionManager, so shutting down everything on it is
// fine. If we ever end up sharing the CASE session manager with other
// DeviceController instances we may need to be more targeted here.
mCASESessionManager->ReleaseAllSessions();
// Shut down any ongoing CASE session activity we have. We're going to
// assume that all sessions for our fabric belong to us here.
mSystemState->CASESessionMgr()->ReleaseSessionsForFabric(GetCompressedFabricId());

// TODO: The CASE session manager does not shut down existing CASE
// sessions. It just shuts down any ongoing CASE session establishment
Expand All @@ -268,9 +244,6 @@ CHIP_ERROR DeviceController::Shutdown()
mDNSResolver.Shutdown();
mDeviceDiscoveryDelegate = nullptr;

chip::Platform::Delete(mCASESessionManager);
mCASESessionManager = nullptr;

return CHIP_NO_ERROR;
}

Expand All @@ -288,14 +261,14 @@ void DeviceController::ReleaseOperationalDevice(NodeId remoteDeviceId)
{
VerifyOrReturn(mState == State::Initialized && mFabricInfo != nullptr,
ChipLogError(Controller, "ReleaseOperationalDevice was called in incorrect state"));
mCASESessionManager->ReleaseSession(mFabricInfo->GetPeerIdForNode(remoteDeviceId));
mSystemState->CASESessionMgr()->ReleaseSession(mFabricInfo->GetPeerIdForNode(remoteDeviceId));
}

CHIP_ERROR DeviceController::DisconnectDevice(NodeId nodeId)
{
ChipLogProgress(Controller, "Force close session for node 0x%" PRIx64, nodeId);

OperationalDeviceProxy * proxy = mCASESessionManager->FindExistingSession(mFabricInfo->GetPeerIdForNode(nodeId));
OperationalDeviceProxy * proxy = mSystemState->CASESessionMgr()->FindExistingSession(mFabricInfo->GetPeerIdForNode(nodeId));
if (proxy == nullptr)
{
ChipLogProgress(Controller, "Attempted to close a session that does not exist.");
Expand Down Expand Up @@ -395,7 +368,7 @@ CHIP_ERROR DeviceController::GetPeerAddressAndPort(PeerId peerId, Inet::IPAddres
{
VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE);
Transport::PeerAddress peerAddr;
ReturnErrorOnFailure(mCASESessionManager->GetPeerAddress(peerId, peerAddr));
ReturnErrorOnFailure(mSystemState->CASESessionMgr()->GetPeerAddress(peerId, peerAddr));
addr = peerAddr.GetIPAddress();
port = peerAddr.GetPort();
return CHIP_NO_ERROR;
Expand All @@ -422,7 +395,7 @@ void DeviceController::OnVIDReadResponse(void * context, VendorId value)
controller->mSetupPayload.vendorID = value;

OperationalDeviceProxy * device =
controller->mCASESessionManager->FindExistingSession(controller->GetPeerIdWithCommissioningWindowOpen());
controller->mSystemState->CASESessionMgr()->FindExistingSession(controller->GetPeerIdWithCommissioningWindowOpen());
if (device == nullptr)
{
ChipLogError(Controller, "Could not find device for opening commissioning window");
Expand Down Expand Up @@ -510,7 +483,8 @@ CHIP_ERROR DeviceController::OpenCommissioningWindowWithCallback(NodeId deviceId

if (callback != nullptr && mCommissioningWindowOption != CommissioningWindowOption::kOriginalSetupCode && readVIDPIDAttributes)
{
OperationalDeviceProxy * device = mCASESessionManager->FindExistingSession(GetPeerIdWithCommissioningWindowOpen());
OperationalDeviceProxy * device =
mSystemState->CASESessionMgr()->FindExistingSession(GetPeerIdWithCommissioningWindowOpen());
VerifyOrReturnError(device != nullptr, CHIP_ERROR_INVALID_ARGUMENT);

constexpr EndpointId kBasicClusterEndpoint = 0;
Expand All @@ -528,7 +502,7 @@ CHIP_ERROR DeviceController::OpenCommissioningWindowInternal()
ChipLogProgress(Controller, "OpenCommissioningWindow for device ID %" PRIu64, mDeviceWithCommissioningWindowOpen);
VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE);

OperationalDeviceProxy * device = mCASESessionManager->FindExistingSession(GetPeerIdWithCommissioningWindowOpen());
OperationalDeviceProxy * device = mSystemState->CASESessionMgr()->FindExistingSession(GetPeerIdWithCommissioningWindowOpen());
VerifyOrReturnError(device != nullptr, CHIP_ERROR_INVALID_ARGUMENT);

constexpr EndpointId kAdministratorCommissioningClusterEndpoint = 0;
Expand Down Expand Up @@ -595,7 +569,7 @@ ControllerDeviceInitParams DeviceController::GetControllerDeviceInitParams()
.exchangeMgr = mSystemState->ExchangeMgr(),
.udpEndPointManager = mSystemState->UDPEndPointManager(),
.storageDelegate = mStorageDelegate,
.idAllocator = &mIDAllocator,
.idAllocator = mSystemState->SessionIDAlloc(),
.fabricsTable = mSystemState->Fabrics(),
};
}
Expand Down Expand Up @@ -835,7 +809,7 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re
session = mSystemState->SessionMgr()->CreateUnauthenticatedSession(params.GetPeerAddress(), device->GetMRPConfig());
VerifyOrExit(session.HasValue(), err = CHIP_ERROR_NO_MEMORY);

err = mIDAllocator.Allocate(keyID);
err = mSystemState->SessionIDAlloc()->Allocate(keyID);
SuccessOrExit(err);

// TODO - Remove use of SetActive/IsActive from CommissioneeDeviceProxy
Expand Down Expand Up @@ -1495,7 +1469,7 @@ void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, PeerId peer
error = CHIP_ERROR_INTERNAL;
}

commissioner->mCASESessionManager->ReleaseSession(peerId);
commissioner->mSystemState->CASESessionMgr()->ReleaseSession(peerId);
if (commissioner->mCommissioningStage == CommissioningStage::kFindOperational &&
commissioner->mCommissioningDelegate != nullptr)
{
Expand Down Expand Up @@ -2086,13 +2060,13 @@ CHIP_ERROR DeviceController::UpdateDevice(NodeId deviceId)

OperationalDeviceProxy * DeviceController::GetDeviceSession(const PeerId & peerId)
{
return mCASESessionManager->FindExistingSession(peerId);
return mSystemState->CASESessionMgr()->FindExistingSession(peerId);
}

OperationalDeviceProxy * DeviceCommissioner::GetDeviceSession(const PeerId & peerId)
{
CHIP_ERROR err =
mCASESessionManager->FindOrEstablishSession(peerId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback);
CHIP_ERROR err = mSystemState->CASESessionMgr()->FindOrEstablishSession(peerId, &mOnDeviceConnectedCallback,
&mOnDeviceConnectionFailureCallback);

if (err != CHIP_NO_ERROR)
{
Expand Down
13 changes: 2 additions & 11 deletions src/controller/CHIPDeviceController.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,8 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, public Abstr
chip::Callback::Callback<OnDeviceConnectionFailure> * onFailure)
{
VerifyOrReturnError(mState == State::Initialized && mFabricInfo != nullptr, CHIP_ERROR_INCORRECT_STATE);
return mCASESessionManager->FindOrEstablishSession(mFabricInfo->GetPeerIdForNode(deviceId), onConnection, onFailure);
return mSystemState->CASESessionMgr()->FindOrEstablishSession(mFabricInfo->GetPeerIdForNode(deviceId), onConnection,
onFailure);
}

/**
Expand Down Expand Up @@ -352,14 +353,6 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, public Abstr

State mState;

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;

SerializableU64Set<kNumMaxPairedDevices> mPairedDevices;
bool mPairedDevicesInitialized;

Expand All @@ -379,8 +372,6 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, public Abstr

OperationalCredentialsDelegate * mOperationalCredentialsDelegate;

SessionIDAllocator mIDAllocator;

uint16_t mVendorId;

/// Fetches the session to use for the current device. Allows overriding
Expand Down
57 changes: 57 additions & 0 deletions src/controller/CHIPDeviceControllerFactory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@

#include <controller/CHIPDeviceControllerFactory.h>

#include <app/OperationalDeviceProxy.h>
#include <app/util/DataModelHandler.h>
#include <lib/support/ErrorStr.h>
#include <messaging/ReliableMessageProtocolConfig.h>

#if CONFIG_DEVICE_LAYER
#include <platform/CHIPDeviceLayer.h>
Expand Down Expand Up @@ -201,6 +203,30 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
chip::app::DnssdServer::Instance().StartServer();
}

stateParams.sessionIDAllocator = Platform::New<SessionIDAllocator>();
stateParams.operationalDevicePool = Platform::New<DeviceControllerSystemStateParams::OperationalDevicePool>();

DeviceProxyInitParams deviceInitParams = {
.sessionManager = stateParams.sessionMgr,
.exchangeMgr = stateParams.exchangeMgr,
.idAllocator = stateParams.sessionIDAllocator,
.fabricTable = stateParams.fabricTable,
.clientPool = stateParams.caseClientPool,
.mrpLocalConfig = Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()),
};

CASESessionManagerConfig sessionManagerConfig = {
.sessionInitParams = deviceInitParams,
#if CHIP_CONFIG_MDNS_CACHE_SIZE > 0
.dnsCache = NoSuchThingWeWouldNeedToAddIt,
#endif
.devicePool = stateParams.operationalDevicePool,
};

// TODO: Need to be able to create a CASESessionManagerConfig here!
stateParams.caseSessionManager = Platform::New<CASESessionManager>(sessionManagerConfig);
ReturnErrorOnFailure(stateParams.caseSessionManager->Init(stateParams.systemLayer));

// store the system state
mSystemState = chip::Platform::New<DeviceControllerSystemState>(stateParams);
ChipLogDetail(Controller, "System State Initialized...");
Expand Down Expand Up @@ -287,6 +313,33 @@ CHIP_ERROR DeviceControllerSystemState::Shutdown()
mCASEServer = nullptr;
}

if (mCASESessionManager != nullptr)
{
mCASESessionManager->Shutdown();
Platform::Delete(mCASESessionManager);
mCASESessionManager = nullptr;
}

// mSessionIDAllocator, mCASEClientPool, and mDevicePool must be deallocated
// after mCASESessionManager, which uses them.
if (mSessionIDAllocator != nullptr)
{
Platform::Delete(mSessionIDAllocator);
mSessionIDAllocator = nullptr;
}

if (mOperationalDevicePool != nullptr)
{
Platform::Delete(mOperationalDevicePool);
mOperationalDevicePool = nullptr;
}

if (mCASEClientPool != nullptr)
{
Platform::Delete(mCASEClientPool);
mCASEClientPool = nullptr;
}

Dnssd::Resolver::Instance().Shutdown();

// Shut down the interaction model
Expand Down Expand Up @@ -324,7 +377,11 @@ CHIP_ERROR DeviceControllerSystemState::Shutdown()
}

mSystemLayer = nullptr;
mTCPEndPointManager = nullptr;
mUDPEndPointManager = nullptr;
#if CONFIG_NETWORK_LAYER_BLE
mBleLayer = nullptr;
#endif // CONFIG_NETWORK_LAYER_BLE

if (mMessageCounterManager != nullptr)
{
Expand Down
32 changes: 30 additions & 2 deletions src/controller/CHIPDeviceControllerSystemState.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,13 @@

#pragma once

#include <app/CASEClientPool.h>
#include <app/CASESessionManager.h>
#include <credentials/FabricTable.h>
#include <lib/core/CHIPConfig.h>
#include <protocols/secure_channel/CASEServer.h>
#include <protocols/secure_channel/MessageCounterManager.h>
#include <protocols/secure_channel/SessionIDAllocator.h>

#include <transport/TransportMgr.h>
#include <transport/raw/UDP.h>
Expand Down Expand Up @@ -63,32 +67,48 @@ namespace Controller {

struct DeviceControllerSystemStateParams
{
using OperationalDevicePool = OperationalDeviceProxyPool<CHIP_CONFIG_CONTROLLER_MAX_ACTIVE_DEVICES>;
using CASEClientPool = chip::CASEClientPool<CHIP_CONFIG_CONTROLLER_MAX_ACTIVE_CASE_CLIENTS>;

// Params that can outlive the DeviceControllerSystemState
System::Layer * systemLayer = nullptr;
Inet::EndPointManager<Inet::TCPEndPoint> * tcpEndPointManager = nullptr;
Inet::EndPointManager<Inet::UDPEndPoint> * udpEndPointManager = nullptr;
#if CONFIG_NETWORK_LAYER_BLE
Ble::BleLayer * bleLayer = nullptr;
#endif

// Params that will be deallocated via Platform::Delete in
// DeviceControllerSystemState::Shutdown.
DeviceTransportMgr * transportMgr = nullptr;
SessionManager * sessionMgr = nullptr;
Messaging::ExchangeManager * exchangeMgr = nullptr;
secure_channel::MessageCounterManager * messageCounterManager = nullptr;
FabricTable * fabricTable = nullptr;
CASEServer * caseServer = nullptr;
CASESessionManager * caseSessionManager = nullptr;
SessionIDAllocator * sessionIDAllocator = nullptr;
OperationalDevicePool * operationalDevicePool = nullptr;
CASEClientPool * caseClientPool = nullptr;
};

// A representation of the internal state maintained by the DeviceControllerFactory
// and refcounted by Device Controllers.
// Expects that the creator of this object is the last one to release it.
class DeviceControllerSystemState
{
using OperationalDevicePool = DeviceControllerSystemStateParams::OperationalDevicePool;
using CASEClientPool = DeviceControllerSystemStateParams::CASEClientPool;

public:
~DeviceControllerSystemState(){};
DeviceControllerSystemState(DeviceControllerSystemStateParams params) :
mSystemLayer(params.systemLayer), mTCPEndPointManager(params.tcpEndPointManager),
mUDPEndPointManager(params.udpEndPointManager), mTransportMgr(params.transportMgr), mSessionMgr(params.sessionMgr),
mExchangeMgr(params.exchangeMgr), mMessageCounterManager(params.messageCounterManager), mFabrics(params.fabricTable),
mCASEServer(params.caseServer)
mCASEServer(params.caseServer), mCASESessionManager(params.caseSessionManager),
mSessionIDAllocator(params.sessionIDAllocator), mOperationalDevicePool(params.operationalDevicePool),
mCASEClientPool(params.caseClientPool)
{
#if CONFIG_NETWORK_LAYER_BLE
mBleLayer = params.bleLayer;
Expand Down Expand Up @@ -120,7 +140,9 @@ class DeviceControllerSystemState
bool IsInitialized()
{
return mSystemLayer != nullptr && mUDPEndPointManager != nullptr && mTransportMgr != nullptr && mSessionMgr != nullptr &&
mExchangeMgr != nullptr && mMessageCounterManager != nullptr && mFabrics != nullptr;
mExchangeMgr != nullptr && mMessageCounterManager != nullptr && mFabrics != nullptr && mCASEServer != nullptr &&
mCASESessionManager != nullptr && mSessionIDAllocator != nullptr && mOperationalDevicePool != nullptr &&
mCASEClientPool != nullptr;
};

System::Layer * SystemLayer() { return mSystemLayer; };
Expand All @@ -134,6 +156,8 @@ class DeviceControllerSystemState
#if CONFIG_NETWORK_LAYER_BLE
Ble::BleLayer * BleLayer() { return mBleLayer; };
#endif
CASESessionManager * CASESessionMgr() const { return mCASESessionManager; }
SessionIDAllocator * SessionIDAlloc() const { return mSessionIDAllocator; }

private:
DeviceControllerSystemState(){};
Expand All @@ -150,6 +174,10 @@ class DeviceControllerSystemState
secure_channel::MessageCounterManager * mMessageCounterManager = nullptr;
FabricTable * mFabrics = nullptr;
CASEServer * mCASEServer = nullptr;
CASESessionManager * mCASESessionManager = nullptr;
SessionIDAllocator * mSessionIDAllocator = nullptr;
OperationalDevicePool * mOperationalDevicePool = nullptr;
CASEClientPool * mCASEClientPool = nullptr;

std::atomic<uint32_t> mRefCount{ 1 };

Expand Down

0 comments on commit 92baee6

Please sign in to comment.