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

Add support for re-using CASE sessions #17266

Merged
merged 4 commits into from
Apr 15, 2022
+192 −80
Merged

Add support for re-using CASE sessions #17266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
df6dacd
Add support for re-using CASE sessions.
mrjerryjohns Apr 9, 2022
c4b886e
Review feedback
mrjerryjohns Apr 14, 2022
1cbfe02
Merge branch 'master' into reuse-case
mrjerryjohns Apr 15, 2022
8965367
Fixed up the tv-casting-app to somewhat function
mrjerryjohns Apr 15, 2022
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
12 changes: 11 additions & 1 deletion examples/chip-tool/commands/tests/TestCommand.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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);
mrjerryjohns marked this conversation as resolved.
Show resolved Hide resolved
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"device", not "this": this is a static method.

// 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