[Group] Add source Node Id to group session (#15038)

* Add source Node Id to group session * Fix nrf CI
project-chip · Aug 28, 2023 · 1582836 · 1582836
1 parent 34be39d
commit 1582836
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 14 deletions.
diff --git a/examples/chip-tool/commands/common/CommandInvoker.h b/examples/chip-tool/commands/common/CommandInvoker.h
@@ -107,7 +107,7 @@ class CommandInvoker final : public ResponseReceiver<typename RequestType::Respo
  }
 
  CHIP_ERROR InvokeGroupCommand(Messaging::ExchangeManager * exchangeManager, FabricIndex fabric, GroupId groupId,
- const RequestType & aRequestData)
+ NodeId sourceNodeId, const RequestType & aRequestData)
  {
  app::CommandPathParams commandPath = { 0 /* endpoint */, groupId, RequestType::GetClusterId(), RequestType::GetCommandId(),
  (app::CommandPathFlags::kGroupIdValid) };
@@ -117,7 +117,7 @@ class CommandInvoker final : public ResponseReceiver<typename RequestType::Respo
 
  ReturnErrorOnFailure(commandSender->AddRequestData(commandPath, aRequestData));
 
- Optional<SessionHandle> session = exchangeManager->GetSessionManager()->CreateGroupSession(groupId, fabric);
+ Optional<SessionHandle> 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
@@ -53,7 +53,8 @@ CHIP_ERROR ClusterBase::AssociateWithGroup(DeviceProxy * device, GroupId groupId
  {
  // Local copy to preserve original SessionHandle for future Unicast communication.
  Optional<SessionHandle> 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
@@ -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
@@ -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
@@ -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<SessionHandle> AllocEntry(GroupId group, FabricIndex fabricIndex)
+ Optional<SessionHandle> 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<SessionHandle>(*entry);

diff --git 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<SessionHandle> session = CreateGroupSession(groupContext.group_id, groupContext.fabric_index);
+ Optional<SessionHandle> 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
@@ -217,9 +217,9 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate
  }
 
  // TODO: implements group sessions
- Optional<SessionHandle> CreateGroupSession(GroupId group, chip::FabricIndex fabricIndex)
+ Optional<SessionHandle> CreateGroupSession(GroupId group, chip::FabricIndex fabricIndex, NodeId sourceNodeId)
  {
- return mGroupSessions.AllocEntry(group, fabricIndex);
+ return mGroupSessions.AllocEntry(group, fabricIndex, sourceNodeId);
  }
  Optional<SessionHandle> FindGroupSession(GroupId group, chip::FabricIndex fabricIndex)
  {