From 1179787a59a3bc6b822a48cfc4617cdcd65196c1 Mon Sep 17 00:00:00 2001 From: jepenven-silabs <67962328+jepenven-silabs@users.noreply.github.com> Date: Fri, 5 Nov 2021 13:51:12 -0400 Subject: [PATCH] Update session handle for Group message (#11390) * Update session handle for Group message --- src/messaging/ExchangeContext.cpp | 4 ++- src/messaging/ExchangeMgr.cpp | 43 +++++++++++++++++-------------- src/transport/SessionHandle.h | 7 +++++ src/transport/SessionManager.cpp | 23 +++++++---------- 4 files changed, 44 insertions(+), 33 deletions(-) diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp index e2aa6d7db3f181..d86c001907d30d 100644 --- a/src/messaging/ExchangeContext.cpp +++ b/src/messaging/ExchangeContext.cpp @@ -275,7 +275,9 @@ ExchangeContext::ExchangeContext(ExchangeManager * em, uint16_t ExchangeId, Sess SetDropAckDebug(false); SetAckPending(false); SetMsgRcvdFromPeer(false); - SetAutoRequestAck(true); + + // Do not request Ack for multicast + SetAutoRequestAck(!session.IsGroupSession()); #if defined(CHIP_EXCHANGE_CONTEXT_DETAIL_LOGGING) ChipLogDetail(ExchangeManager, "ec++ id: " ChipLogFormatExchange, ChipLogValueExchange(this)); diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index 5d88c689ed7e50..07a11a5be24dbc 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -213,29 +213,34 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const msgFlags.Set(MessageFlagValues::kDuplicateMessage); } - // Search for an existing exchange that the message applies to. If a match is found... - bool found = false; - mContextPool.ForEachActiveObject([&](auto * ec) { - if (ec->MatchExchange(session, packetHeader, payloadHeader)) - { - // Found a matching exchange. Set flag for correct subsequent MRP - // retransmission timeout selection. - if (!ec->HasRcvdMsgFromPeer()) + // Skip retrieval of exchange for group message since no exchange is stored + // for group msg (optimization) + if (!packetHeader.IsGroupSession()) + { + // Search for an existing exchange that the message applies to. If a match is found... + bool found = false; + mContextPool.ForEachActiveObject([&](auto * ec) { + if (ec->MatchExchange(session, packetHeader, payloadHeader)) { - ec->SetMsgRcvdFromPeer(true); + // Found a matching exchange. Set flag for correct subsequent MRP + // retransmission timeout selection. + if (!ec->HasRcvdMsgFromPeer()) + { + ec->SetMsgRcvdFromPeer(true); + } + + // Matched ExchangeContext; send to message handler. + ec->HandleMessage(packetHeader.GetMessageCounter(), payloadHeader, source, msgFlags, std::move(msgBuf)); + found = true; + return false; } + return true; + }); - // Matched ExchangeContext; send to message handler. - ec->HandleMessage(packetHeader.GetMessageCounter(), payloadHeader, source, msgFlags, std::move(msgBuf)); - found = true; - return false; + if (found) + { + return; } - return true; - }); - - if (found) - { - return; } // If it's not a duplicate message, search for an unsolicited message handler if it is marked as being sent by an initiator. diff --git a/src/transport/SessionHandle.h b/src/transport/SessionHandle.h index 0b738011af3469..3502e9ae3bbb9f 100644 --- a/src/transport/SessionHandle.h +++ b/src/transport/SessionHandle.h @@ -43,6 +43,11 @@ class SessionHandle mPeerSessionId.SetValue(peerSessionId); } + SessionHandle(NodeId peerNodeId, GroupId groupId, FabricIndex fabric) : mPeerNodeId(peerNodeId), mFabric(fabric) + { + mGroupId.SetValue(groupId); + } + bool IsSecure() const { return !mUnauthenticatedSessionHandle.HasValue(); } bool HasFabricIndex() const { return (mFabric != kUndefinedFabricIndex); } @@ -68,6 +73,7 @@ class SessionHandle } NodeId GetPeerNodeId() const { return mPeerNodeId; } + bool IsGroupSession() const { return mGroupId.HasValue(); } const Optional & GetPeerSessionId() const { return mPeerSessionId; } const Optional & GetLocalSessionId() const { return mLocalSessionId; } @@ -85,6 +91,7 @@ class SessionHandle NodeId mPeerNodeId; Optional mLocalSessionId; Optional mPeerSessionId; + Optional mGroupId; // TODO: Re-evaluate the storing of Fabric ID in SessionHandle // The Fabric ID will not be available for PASE and group sessions. So need // to identify an approach that'll allow looking up the corresponding information for diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index 3b0bb58bc00bde..ad291f5054f2c9 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -459,6 +459,7 @@ void SessionManager::SecureGroupMessageDispatch(const PacketHeader & packetHeade CHIP_ERROR err = CHIP_NO_ERROR; PayloadHeader payloadHeader; SessionManagerDelegate::DuplicateMessage isDuplicate = SessionManagerDelegate::DuplicateMessage::No; + FabricIndex fabricIndex = 0; // TODO : remove initialization once GroupDataProvider->Decrypt is implemented // Credentials::GroupDataProvider * groups = Credentials::GetGroupDataProvider(); VerifyOrExit(!msg.IsNull(), ChipLogError(Inet, "Secure transport received NULL packet, discarding")); @@ -471,6 +472,7 @@ void SessionManager::SecureGroupMessageDispatch(const PacketHeader & packetHeade } // Trial decryption with GroupDataProvider. TODO: Implement the GroupDataProvider Class + // TODO retrieve also the fabricIndex with the GroupDataProvider. // VerifyOrExit(CHIP_NO_ERROR == groups->DecryptMessage(packetHeader, payloadHeader, msg), // ChipLogError(Inet, "Secure transport received group message, but failed to decode it, discarding")); @@ -488,29 +490,24 @@ void SessionManager::SecureGroupMessageDispatch(const PacketHeader & packetHeade // TODO: Handle Group message counter here spec 4.7.3 // spec 4.5.1.2 for msg counter - - if (isDuplicate == SessionManagerDelegate::DuplicateMessage::Yes && !payloadHeader.NeedsAck()) + if (isDuplicate == SessionManagerDelegate::DuplicateMessage::Yes) { ChipLogDetail(Inet, "Received a duplicate message with MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchangeId, - packetHeader.GetMessageCounter(), ChipLogValueExchangeIdFromReceivedHeader(payloadHeader)); - if (!payloadHeader.NeedsAck()) - { - // If it's a duplicate message, but doesn't require an ack, let's drop it right here to save CPU - // cycles on further message processing. - ExitNow(err = CHIP_NO_ERROR); - } + packetHeader.GetMessageCounter(), ChipLogValueExchangeIdFromSentHeader(payloadHeader)); + + // If it's a duplicate message, let's drop it right here to save CPU + // cycles on further message processing. + ExitNow(err = CHIP_NO_ERROR); } // TODO: Commit Group Message Counter if (mCB != nullptr) { - // TODO: Update Session Handle for Group messages. - // SessionHandle session(session->GetPeerNodeId(), session->GetLocalSessionId(), session->GetPeerSessionId(), - // session->GetFabricIndex()); - // mCB->OnMessageReceived(packetHeader, payloadHeader, session, peerAddress, isDuplicate, std::move(msg)); + SessionHandle session(packetHeader.GetSourceNodeId().Value(), packetHeader.GetDestinationGroupId().Value(), fabricIndex); + mCB->OnMessageReceived(packetHeader, payloadHeader, session, peerAddress, isDuplicate, std::move(msg)); } exit: