Skip to content

Commit

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

Fixes #16174
  • Loading branch information
@bzbarsky-apple @pull
bzbarsky-apple authored and pull[bot] committed Nov 30, 2023
1 parent 097367e commit 2356697
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 @@ -419,7 +392,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 @@ -446,7 +419,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 @@ -534,7 +507,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 @@ -552,7 +526,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 @@ -619,7 +593,7 @@ ControllerDeviceInitParams DeviceController::GetControllerDeviceInitParams()
.exchangeMgr = mSystemState->ExchangeMgr(),
.udpEndPointManager = mSystemState->UDPEndPointManager(),
.storageDelegate = mStorageDelegate,
.idAllocator = &mIDAllocator,
.idAllocator = mSystemState->SessionIDAlloc(),
.fabricsTable = mSystemState->Fabrics(),
};
}
Expand Down Expand Up @@ -867,7 +841,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 @@ -1522,7 +1496,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 @@ -2105,13 +2079,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 @@ -211,7 +211,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 @@ -350,14 +351,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 @@ -377,8 +370,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
58 changes: 58 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,31 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
chip::app::DnssdServer::Instance().StartServer();
}

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

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 +314,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 +378,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
31 changes: 29 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,8 @@ 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 && mCASESessionManager != nullptr &&
mSessionIDAllocator != nullptr && mOperationalDevicePool != nullptr && mCASEClientPool != nullptr;
};

System::Layer * SystemLayer() { return mSystemLayer; };
Expand All @@ -134,6 +155,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 +173,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 2356697

Please sign in to comment.