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

* Add source Node Id to group session * Fix nrf CI
project-chip · Nov 20, 2023 · 3368438 · 3368438
1 parent fdeed69
commit 3368438
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)
     {