Skip to content

Commit

Permalink
Add support for re-using CASE sessions (#17266)
Browse files Browse the repository at this point in the history 
* Add support for re-using CASE sessions.

* Review feedback

* Fixed up the tv-casting-app to somewhat function
  • Loading branch information
@mrjerryjohns @pull
mrjerryjohns authored and pull[bot] committed Jun 27, 2023
1 parent 70ef65a commit 2882433
Show file tree
Hide file tree
Showing 8 changed files with 192 additions and 80 deletions.
12 changes: 11 additions & 1 deletion examples/chip-tool/commands/tests/TestCommand.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,17 @@ CHIP_ERROR TestCommand::RunCommand()

CHIP_ERROR TestCommand::WaitForCommissionee(chip::NodeId nodeId)
{
CurrentCommissioner().ReleaseOperationalDevice(nodeId);
chip::FabricIndex fabricIndex;

ReturnErrorOnFailure(CurrentCommissioner().GetFabricIndex(&fabricIndex));

//
// There's a chance the commissionee may have rebooted before this call here as part of a test flow
// or is just starting out fresh outright. Let's make sure we're not re-using any cached CASE sessions
// that will now be stale and mismatched with the peer, causing subsequent interactions to fail.
//
CurrentCommissioner().SessionMgr()->ExpireAllPairings(nodeId, fabricIndex);

return CurrentCommissioner().GetConnectedDevice(nodeId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback);
}

Expand Down
48 changes: 38 additions & 10 deletions examples/tv-casting-app/linux/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,10 @@ class TargetEndpointInfo
class TargetVideoPlayerInfo
{
public:
TargetVideoPlayerInfo() :
mOnConnectedCallback(HandleDeviceConnected, this), mOnConnectionFailureCallback(HandleDeviceConnectionFailure, this)
{}

bool IsInitialized() { return mInitialized; }

CHIP_ERROR Initialize(NodeId nodeId, FabricIndex fabricIndex)
Expand Down Expand Up @@ -359,21 +363,30 @@ class TargetVideoPlayerInfo
.clientPool = &gCASEClientPool,
};

PeerId peerID = fabric->GetPeerIdForNode(nodeId);
mOperationalDeviceProxy = chip::Platform::New<chip::OperationalDeviceProxy>(initParams, peerID);
PeerId peerID = fabric->GetPeerIdForNode(nodeId);

//
// TODO: The code here is assuming that we can create an OperationalDeviceProxy instance and attach it immediately
// to a CASE session that just got established to us by the tv-app. While this will work most of the time,
// this is a dangerous assumption to make since it is entirely possible for that secure session to have been
// evicted in the time since that session was established to the point here when we desire to interact back
// with that peer. If that is the case, our `OnConnected` callback will not get invoked syncronously and
// mOperationalDeviceProxy will still have a value of null, triggering the check below to fail.
//
mOperationalDeviceProxy = nullptr;
CHIP_ERROR err =
server->GetCASESessionManager()->FindOrEstablishSession(peerID, &mOnConnectedCallback, &mOnConnectionFailureCallback);
if (err != CHIP_NO_ERROR)
{
ChipLogError(AppServer, "Could not establish a session to the peer");
return err;
}

// TODO: figure out why this doesn't work so that we can remove OperationalDeviceProxy creation above,
// and remove the FindSecureSessionForNode and SetConnectedSession calls below
// mOperationalDeviceProxy = server->GetCASESessionManager()->FindExistingSession(nodeId);
if (mOperationalDeviceProxy == nullptr)
{
ChipLogError(AppServer, "Failed in creating an instance of OperationalDeviceProxy");
ChipLogError(AppServer, "Failed to find an existing instance of OperationalDeviceProxy to the peer");
return CHIP_ERROR_INVALID_ARGUMENT;
}
ChipLogError(AppServer, "Created an instance of OperationalDeviceProxy");

SessionHandle handle = server->GetSecureSessionManager().FindSecureSessionForNode(nodeId);
mOperationalDeviceProxy->SetConnectedSession(handle);

mInitialized = true;
return CHIP_NO_ERROR;
Expand Down Expand Up @@ -451,12 +464,27 @@ class TargetVideoPlayerInfo
}

private:
static void HandleDeviceConnected(void * context, OperationalDeviceProxy * device)
{
TargetVideoPlayerInfo * _this = static_cast<TargetVideoPlayerInfo *>(context);
_this->mOperationalDeviceProxy = device;
}

static void HandleDeviceConnectionFailure(void * context, PeerId peerId, CHIP_ERROR error)
{
TargetVideoPlayerInfo * _this = static_cast<TargetVideoPlayerInfo *>(context);
_this->mOperationalDeviceProxy = nullptr;
}

static constexpr size_t kMaxNumberOfEndpoints = 5;
TargetEndpointInfo mEndpoints[kMaxNumberOfEndpoints];
NodeId mNodeId;
FabricIndex mFabricIndex;
OperationalDeviceProxy * mOperationalDeviceProxy;

Callback::Callback<OnDeviceConnected> mOnConnectedCallback;
Callback::Callback<OnDeviceConnectionFailure> mOnConnectionFailureCallback;

bool mInitialized = false;
};
TargetVideoPlayerInfo gTargetVideoPlayerInfo;
Expand Down
2 changes: 1 addition & 1 deletion src/app/CASESessionManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ CHIP_ERROR CASESessionManager::FindOrEstablishSession(PeerId peerId, Callback::C
OperationalDeviceProxy * session = FindExistingSession(peerId);
if (session == nullptr)
{
ChipLogDetail(CASESessionManager, "FindOrEstablishSession: No existing session found");
ChipLogDetail(CASESessionManager, "FindOrEstablishSession: No existing OperationalDeviceProxy instance found");

session = mConfig.devicePool->Allocate(mConfig.sessionInitParams, peerId);

Expand Down
143 changes: 94 additions & 49 deletions src/app/OperationalDeviceProxy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "CASEClient.h"
#include "CommandSender.h"
#include "ReadPrepareParams.h"
#include "transport/SecureSession.h"

#include <lib/address_resolve/AddressResolve.h>
#include <lib/core/CHIPCore.h>
Expand Down Expand Up @@ -57,10 +58,35 @@ void OperationalDeviceProxy::MoveToState(State aTargetState)
}
}

bool OperationalDeviceProxy::AttachToExistingSecureSession()
{
VerifyOrReturnError(mState == State::NeedsAddress || mState == State::Initialized, false);

ScopedNodeId peerNodeId(mPeerId.GetNodeId(), mFabricInfo->GetFabricIndex());
auto sessionHandle = mInitParams.sessionManager->FindSecureSessionForNode(peerNodeId, Transport::SecureSession::Type::kCASE);
if (sessionHandle.HasValue())
{
ChipLogProgress(Controller, "Found an existing secure session to [" ChipLogFormatX64 "-" ChipLogFormatX64 "]!",
ChipLogValueX64(mPeerId.GetCompressedFabricId()), ChipLogValueX64(mPeerId.GetNodeId()));
mSecureSession.Grab(sessionHandle.Value());
return true;
}

return false;
}

CHIP_ERROR OperationalDeviceProxy::Connect(Callback::Callback<OnDeviceConnected> * onConnection,
Callback::Callback<OnDeviceConnectionFailure> * onFailure)
{
CHIP_ERROR err = CHIP_NO_ERROR;
CHIP_ERROR err = CHIP_NO_ERROR;
bool isConnected = false;

//
// Always enqueue our user provided callbacks into our callback list.
// If anything goes wrong below, we'll trigger failures (including any queued from
// a previous iteration which in theory shouldn't happen, but this is written to be more defensive)
//
EnqueueConnectionCallbacks(onConnection, onFailure);

switch (mState)
{
Expand All @@ -69,35 +95,47 @@ CHIP_ERROR OperationalDeviceProxy::Connect(Callback::Callback<OnDeviceConnected>
break;

case State::NeedsAddress:
err = LookupPeerAddress();
EnqueueConnectionCallbacks(onConnection, onFailure);
isConnected = AttachToExistingSecureSession();
if (!isConnected)
{
err = LookupPeerAddress();
}

break;

case State::Initialized:
err = EstablishConnection();
if (err == CHIP_NO_ERROR)
isConnected = AttachToExistingSecureSession();
if (!isConnected)
{
EnqueueConnectionCallbacks(onConnection, onFailure);
err = EstablishConnection();
}

break;

case State::Connecting:
EnqueueConnectionCallbacks(onConnection, onFailure);
break;

case State::SecureConnected:
if (onConnection != nullptr)
{
onConnection->mCall(onConnection->mContext, this);
}
isConnected = true;
break;

default:
err = CHIP_ERROR_INCORRECT_STATE;
}

if (err != CHIP_NO_ERROR && onFailure != nullptr)
if (isConnected)
{
MoveToState(State::SecureConnected);
}

//
// Dequeue all our callbacks on either encountering an error
// or if we successfully connected. Both should not be set
// simultaneously.
//
if (err != CHIP_NO_ERROR || isConnected)
{
onFailure->mCall(onFailure->mContext, mPeerId, err);
DequeueConnectionCallbacks(err);
}

return err;
Expand Down Expand Up @@ -133,7 +171,7 @@ CHIP_ERROR OperationalDeviceProxy::UpdateDeviceData(const Transport::PeerAddress
err = EstablishConnection();
if (err != CHIP_NO_ERROR)
{
OnSessionEstablishmentError(err);
DequeueConnectionCallbacks(err);
}
}
else
Expand Down Expand Up @@ -194,35 +232,43 @@ void OperationalDeviceProxy::EnqueueConnectionCallbacks(Callback::Callback<OnDev
}
}

void OperationalDeviceProxy::DequeueConnectionSuccessCallbacks(bool executeCallback)
void OperationalDeviceProxy::DequeueConnectionCallbacks(CHIP_ERROR error)
{
Cancelable ready;
mConnectionSuccess.DequeueAll(ready);
while (ready.mNext != &ready)
Cancelable failureReady, successReady;

//
// Dequeue both failure and success callback lists into temporary stack args before invoking either of them.
// We do this since we may not have a valid 'this' pointer anymore upon invoking any of those callbacks
// since the callee may destroy this object as part of that callback.
//
mConnectionFailure.DequeueAll(failureReady);
mConnectionSuccess.DequeueAll(successReady);

//
// If we encountered no error, go ahead and call all success callbacks. Otherwise,
// call the failure callbacks.
//
while (failureReady.mNext != &failureReady)
{
Callback::Callback<OnDeviceConnected> * cb = Callback::Callback<OnDeviceConnected>::FromCancelable(ready.mNext);
Callback::Callback<OnDeviceConnectionFailure> * cb =
Callback::Callback<OnDeviceConnectionFailure>::FromCancelable(failureReady.mNext);

cb->Cancel();
if (executeCallback)

if (error != CHIP_NO_ERROR)
{
cb->mCall(cb->mContext, this);
cb->mCall(cb->mContext, mPeerId, error);
}
}
}

void OperationalDeviceProxy::DequeueConnectionFailureCallbacks(CHIP_ERROR error, bool executeCallback)
{
Cancelable ready;
mConnectionFailure.DequeueAll(ready);
while (ready.mNext != &ready)
while (successReady.mNext != &successReady)
{
Callback::Callback<OnDeviceConnectionFailure> * cb =
Callback::Callback<OnDeviceConnectionFailure>::FromCancelable(ready.mNext);
Callback::Callback<OnDeviceConnected> * cb = Callback::Callback<OnDeviceConnected>::FromCancelable(successReady.mNext);

cb->Cancel();
if (executeCallback)
if (error == CHIP_NO_ERROR)
{
cb->mCall(cb->mContext, mPeerId, error);
cb->mCall(cb->mContext, this);
}
}
}
Expand All @@ -234,13 +280,20 @@ void OperationalDeviceProxy::HandleCASEConnectionFailure(void * context, CASECli
ChipLogError(Controller, "HandleCASEConnectionFailure was called while the device was not initialized"));
VerifyOrReturn(client == device->mCASEClient, ChipLogError(Controller, "HandleCASEConnectionFailure for unknown CASEClient"));

//
// We don't need to reset the state all the way back to NeedsAddress since all that transpired
// was just CASE connection failure. So let's re-use the cached address to re-do CASE again
// if need-be.
//
device->MoveToState(State::Initialized);

device->CloseCASESession();
device->DequeueConnectionSuccessCallbacks(/* executeCallback */ false);
device->DequeueConnectionFailureCallbacks(error, /* executeCallback */ true);
// Do not touch device anymore; it might have been destroyed by a failure
device->DequeueConnectionCallbacks(error);

//
// Do not touch device instance anymore; it might have been destroyed by a failure
// callback.
//
}

void OperationalDeviceProxy::HandleCASEConnected(void * context, CASEClient * client)
Expand All @@ -254,19 +307,18 @@ void OperationalDeviceProxy::HandleCASEConnected(void * context, CASEClient * cl
if (err != CHIP_NO_ERROR)
{
device->HandleCASEConnectionFailure(context, client, err);
// Do not touch device anymore; it might have been destroyed by a
// HandleCASEConnectionFailure.
}
else
{
device->MoveToState(State::SecureConnected);

device->CloseCASESession();
device->DequeueConnectionFailureCallbacks(CHIP_NO_ERROR, /* executeCallback */ false);
device->DequeueConnectionSuccessCallbacks(/* executeCallback */ true);
// Do not touch device anymore; it might have been destroyed by a
// success callback.
device->DequeueConnectionCallbacks(CHIP_NO_ERROR);
}

//
// Do not touch this instance anymore; it might have been destroyed by a
// callback.
//
}

CHIP_ERROR OperationalDeviceProxy::Disconnect()
Expand All @@ -285,12 +337,6 @@ CHIP_ERROR OperationalDeviceProxy::Disconnect()
return CHIP_NO_ERROR;
}

void OperationalDeviceProxy::SetConnectedSession(const SessionHandle & handle)
{
mSecureSession.Grab(handle);
MoveToState(State::SecureConnected);
}

void OperationalDeviceProxy::Clear()
{
if (mCASEClient)
Expand Down Expand Up @@ -367,8 +413,7 @@ void OperationalDeviceProxy::OnNodeAddressResolutionFailed(const PeerId & peerId
ChipLogError(Discovery, "Operational discovery failed for 0x" ChipLogFormatX64 ": %" CHIP_ERROR_FORMAT,
ChipLogValueX64(peerId.GetNodeId()), reason.Format());

DequeueConnectionSuccessCallbacks(/* executeCallback */ false);
DequeueConnectionFailureCallbacks(reason, /* executeCallback */ true);
DequeueConnectionCallbacks(reason);
}

} // namespace chip
Loading

0 comments on commit 2882433

Please sign in to comment.