From 1582836b55a8b5c4d96b02fe1994f06fc6614c82 Mon Sep 17 00:00:00 2001 From: Jean-Francois Penven <67962328+jepenven-silabs@users.noreply.github.com> Date: Thu, 10 Feb 2022 18:10:10 -0500 Subject: [PATCH] [Group] Add source Node Id to group session (#15038) * Add source Node Id to group session * Fix nrf CI --- examples/chip-tool/commands/common/CommandInvoker.h | 7 ++++--- src/controller/CHIPCluster.cpp | 3 ++- src/lib/core/CHIPConfig.h | 4 +++- src/messaging/tests/MessagingContext.cpp | 2 +- src/transport/GroupSession.h | 12 +++++++++--- src/transport/SessionManager.cpp | 6 +++--- src/transport/SessionManager.h | 4 ++-- 7 files changed, 24 insertions(+), 14 deletions(-) diff --git a/examples/chip-tool/commands/common/CommandInvoker.h b/examples/chip-tool/commands/common/CommandInvoker.h index 11c0c44eb71063..d7a6dd0a2ebec8 100644 --- a/examples/chip-tool/commands/common/CommandInvoker.h +++ b/examples/chip-tool/commands/common/CommandInvoker.h @@ -107,7 +107,7 @@ class CommandInvoker final : public ResponseReceiverAddRequestData(commandPath, aRequestData)); - Optional session = exchangeManager->GetSessionManager()->CreateGroupSession(groupId, fabric); + Optional session = exchangeManager->GetSessionManager()->CreateGroupSession(groupId, fabric, sourceNodeId); if (!session.HasValue()) { return CHIP_ERROR_NO_MEMORY; @@ -235,7 +235,8 @@ CHIP_ERROR InvokeGroupCommand(DeviceProxy * aDevice, void * aContext, // // We assume the aDevice already has a Case session which is way we can use he established Secure Session ReturnErrorOnFailure(invoker->InvokeGroupCommand(aDevice->GetExchangeManager(), - aDevice->GetSecureSession().Value()->GetFabricIndex(), groupId, aRequestData)); + aDevice->GetSecureSession().Value()->GetFabricIndex(), groupId, + aDevice->GetDeviceId(), aRequestData)); // invoker is already deleted and is not to be used invoker.release(); diff --git a/src/controller/CHIPCluster.cpp b/src/controller/CHIPCluster.cpp index 87c876329655c5..1be9b4e2f4094d 100644 --- a/src/controller/CHIPCluster.cpp +++ b/src/controller/CHIPCluster.cpp @@ -53,7 +53,8 @@ CHIP_ERROR ClusterBase::AssociateWithGroup(DeviceProxy * device, GroupId groupId { // Local copy to preserve original SessionHandle for future Unicast communication. Optional session = mDevice->GetExchangeManager()->GetSessionManager()->CreateGroupSession( - groupId, mDevice->GetSecureSession().Value()->GetFabricIndex()); + groupId, mDevice->GetSecureSession().Value()->GetFabricIndex(), mDevice->GetDeviceId()); + // Sanity check if (!session.HasValue() || !session.Value()->IsGroupSession()) { diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h index e6b169aa4892fb..e2a8f5d97e7164 100644 --- a/src/lib/core/CHIPConfig.h +++ b/src/lib/core/CHIPConfig.h @@ -1208,9 +1208,11 @@ * @def CHIP_CONFIG_GROUP_CONNECTION_POOL_SIZE * * @brief Define the size of the pool used for tracking CHIP groups. + * Given the ephemeral nature of groups session, no need to support + * a large pool size. */ #ifndef CHIP_CONFIG_GROUP_CONNECTION_POOL_SIZE -#define CHIP_CONFIG_GROUP_CONNECTION_POOL_SIZE 8 +#define CHIP_CONFIG_GROUP_CONNECTION_POOL_SIZE 4 #endif // CHIP_CONFIG_GROUP_CONNECTION_POOL_SIZE /** diff --git a/src/messaging/tests/MessagingContext.cpp b/src/messaging/tests/MessagingContext.cpp index 4a288177de79d6..c11b176805e1ad 100644 --- a/src/messaging/tests/MessagingContext.cpp +++ b/src/messaging/tests/MessagingContext.cpp @@ -83,7 +83,7 @@ CHIP_ERROR MessagingContext::CreateSessionAliceToBob() CHIP_ERROR MessagingContext::CreateSessionBobToFriends() { - mSessionBobToFriends.Grab(mSessionManager.CreateGroupSession(GetFriendsGroupId(), mSrcFabricIndex).Value()); + mSessionBobToFriends.Grab(mSessionManager.CreateGroupSession(GetFriendsGroupId(), mSrcFabricIndex, GetBobNodeId()).Value()); return CHIP_NO_ERROR; } diff --git a/src/transport/GroupSession.h b/src/transport/GroupSession.h index 900efae2d23272..dee5d6f5de5e1c 100644 --- a/src/transport/GroupSession.h +++ b/src/transport/GroupSession.h @@ -27,7 +27,10 @@ namespace Transport { class GroupSession : public Session { public: - GroupSession(GroupId group, FabricIndex fabricIndex) : mGroupId(group) { SetFabricIndex(fabricIndex); } + GroupSession(GroupId group, FabricIndex fabricIndex, NodeId sourceNodeId) : mGroupId(group), mSourceNodeId(sourceNodeId) + { + SetFabricIndex(fabricIndex); + } ~GroupSession() { NotifySessionReleased(); } Session::SessionType GetSessionType() const override { return Session::SessionType::kGroup; } @@ -60,8 +63,11 @@ class GroupSession : public Session GroupId GetGroupId() const { return mGroupId; } + NodeId GetSourceNodeId() { return mSourceNodeId; } + private: const GroupId mGroupId; + const NodeId mSourceNodeId; }; /* @@ -80,9 +86,9 @@ class GroupSessionTable * @return the session found or allocated, nullptr if not found and allocation failed. */ CHECK_RETURN_VALUE - Optional AllocEntry(GroupId group, FabricIndex fabricIndex) + Optional AllocEntry(GroupId group, FabricIndex fabricIndex, NodeId sourceNodeId) { - GroupSession * entry = mEntries.CreateObject(group, fabricIndex); + GroupSession * entry = mEntries.CreateObject(group, fabricIndex, sourceNodeId); if (entry != nullptr) { return MakeOptional(*entry); diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index dd276339efe006..0c4034578f6e5e 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -133,8 +133,7 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P packetHeader.SetDestinationGroupId(groupSession->GetGroupId()); packetHeader.SetFlags(Header::SecFlagValues::kPrivacyFlag); packetHeader.SetSessionType(Header::SessionType::kGroupSession); - // TODO : Replace the PeerNodeId with Our nodeId - packetHeader.SetSourceNodeId(kUndefinedNodeId); + packetHeader.SetSourceNodeId(groupSession->GetSourceNodeId()); if (!packetHeader.IsValidGroupMsg()) { @@ -693,7 +692,8 @@ void SessionManager::SecureGroupMessageDispatch(const PacketHeader & packetHeade if (mCB != nullptr) { // TODO : When MCSP is done, clean up session creation logique - Optional session = CreateGroupSession(groupContext.group_id, groupContext.fabric_index); + Optional session = + CreateGroupSession(groupContext.group_id, groupContext.fabric_index, packetHeader.GetSourceNodeId().Value()); VerifyOrReturn(session.HasValue(), ChipLogError(Inet, "Error when creating group session handle.")); Transport::GroupSession * groupSession = session.Value()->AsGroupSession(); diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h index 4adfdd90015fe5..fb84b95b48e37c 100644 --- a/src/transport/SessionManager.h +++ b/src/transport/SessionManager.h @@ -217,9 +217,9 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate } // TODO: implements group sessions - Optional CreateGroupSession(GroupId group, chip::FabricIndex fabricIndex) + Optional CreateGroupSession(GroupId group, chip::FabricIndex fabricIndex, NodeId sourceNodeId) { - return mGroupSessions.AllocEntry(group, fabricIndex); + return mGroupSessions.AllocEntry(group, fabricIndex, sourceNodeId); } Optional FindGroupSession(GroupId group, chip::FabricIndex fabricIndex) {