diff --git a/src/messaging/ReliableMessageContext.cpp b/src/messaging/ReliableMessageContext.cpp index fe45f08052b73c..4f94e11bf68d85 100644 --- a/src/messaging/ReliableMessageContext.cpp +++ b/src/messaging/ReliableMessageContext.cpp @@ -43,16 +43,6 @@ ReliableMessageContext::ReliableMessageContext() : mConfig(gDefaultReliableMessageProtocolConfig), mNextAckTimeTick(0), mPendingPeerAckMessageCounter(0) {} -void ReliableMessageContext::RetainContext() -{ - GetExchangeContext()->Retain(); -} - -void ReliableMessageContext::ReleaseContext() -{ - GetExchangeContext()->Release(); -} - bool ReliableMessageContext::AutoRequestAck() const { return mFlags.Has(Flags::kFlagAutoRequestAck); @@ -88,14 +78,14 @@ void ReliableMessageContext::SetDropAckDebug(bool inDropAckDebug) mFlags.Set(Flags::kFlagDropAckDebug, inDropAckDebug); } -bool ReliableMessageContext::IsOccupied() const +bool ReliableMessageContext::IsMessageNotAcked() const { - return mFlags.Has(Flags::kFlagOccupied); + return mFlags.Has(Flags::kFlagMesageNotAcked); } -void ReliableMessageContext::SetOccupied(bool inOccupied) +void ReliableMessageContext::SetMessageNotAcked(bool messageNotAcked) { - mFlags.Set(Flags::kFlagOccupied, inOccupied); + mFlags.Set(Flags::kFlagMesageNotAcked, messageNotAcked); } bool ReliableMessageContext::ShouldDropAckDebug() const diff --git a/src/messaging/ReliableMessageContext.h b/src/messaging/ReliableMessageContext.h index 606d7c781116c7..247e7d3e4e3796 100644 --- a/src/messaging/ReliableMessageContext.h +++ b/src/messaging/ReliableMessageContext.h @@ -41,7 +41,6 @@ namespace Messaging { class ChipMessageInfo; class ExchangeContext; enum class MessageFlagValues : uint32_t; -class ReliableMessageContext; class ReliableMessageMgr; class ReliableMessageContext @@ -166,20 +165,11 @@ class ReliableMessageContext */ void SetMsgRcvdFromPeer(bool inMsgRcvdFromPeer); - /** - * Determine whether there is already an acknowledgment pending to be sent to the peer on this exchange. - * - * @return Returns 'true' if there is already an acknowledgment pending on this exchange, else 'false'. - */ - bool IsOccupied() const; + /// Determine whether there is message hasn't been acknowledged. + bool IsMessageNotAcked() const; - /** - * Set whether there is an acknowledgment panding to be send to the peer on - * this exchange. - * - * @param[in] inOccupied Whether there is a pending acknowledgment. - */ - void SetOccupied(bool inOccupied); + /// Set whether there is a message hasn't been acknowledged. + void SetMessageNotAcked(bool messageNotAcked); /** * Get the reliable message manager that corresponds to this reliable @@ -202,8 +192,8 @@ class ReliableMessageContext /// Internal and debug only: when set, the exchange layer does not send an acknowledgment. kFlagDropAckDebug = (1u << 3), - /// When set, signifies current reliable message context is in usage. - kFlagOccupied = (1u << 4), + /// When set, signifies there is a message which hasn't been acknowledged. + kFlagMesageNotAcked = (1u << 4), /// When set, signifies that there is an acknowledgment pending to be sent back. kFlagAckPending = (1u << 5), @@ -229,8 +219,6 @@ class ReliableMessageContext BitFlags mFlags; // Internal state flags private: - void RetainContext(); - void ReleaseContext(); void HandleRcvdAck(uint32_t ackMessageCounter); CHIP_ERROR HandleNeedsAck(uint32_t messageCounter, BitFlags messageFlags); CHIP_ERROR HandleNeedsAckInner(uint32_t messageCounter, BitFlags messageFlags); diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp index d8040b9fbef269..cd9326a7472ebe 100644 --- a/src/messaging/ReliableMessageMgr.cpp +++ b/src/messaging/ReliableMessageMgr.cpp @@ -37,10 +37,19 @@ namespace chip { namespace Messaging { -ReliableMessageMgr::RetransTableEntry::RetransTableEntry() : rc(nullptr), nextRetransTimeTick(0), sendCount(0) {} +ReliableMessageMgr::RetransTableEntry::RetransTableEntry(ReliableMessageContext * rc) : + ec(*rc->GetExchangeContext()), retainedBuf(EncryptedPacketBufferHandle()), nextRetransTimeTick(0), sendCount(0) +{ + ec->SetMessageNotAcked(true); +} + +ReliableMessageMgr::RetransTableEntry::~RetransTableEntry() +{ + ec->SetMessageNotAcked(false); +} ReliableMessageMgr::ReliableMessageMgr(BitMapObjectPool & contextPool) : - mContextPool(contextPool), mSystemLayer(nullptr), mSessionManager(nullptr), mCurrentTimerExpiry(0), + mContextPool(contextPool), mSystemLayer(nullptr), mCurrentTimerExpiry(0), mTimerIntervalShift(CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT) {} @@ -48,9 +57,7 @@ ReliableMessageMgr::~ReliableMessageMgr() {} void ReliableMessageMgr::Init(chip::System::Layer * systemLayer, SessionManager * sessionManager) { - mSystemLayer = systemLayer; - mSessionManager = sessionManager; - + mSystemLayer = systemLayer; mTimeStampBase = System::Clock::GetMonotonicMilliseconds(); mCurrentTimerExpiry = 0; } @@ -59,14 +66,13 @@ void ReliableMessageMgr::Shutdown() { StopTimer(); - mSystemLayer = nullptr; - mSessionManager = nullptr; - // Clear the retransmit table - for (RetransTableEntry & rEntry : mRetransTable) - { - ClearRetransTable(rEntry); - } + mRetransTable.ForEachActiveObject([&](auto * entry) { + ClearRetransTable(*entry); + return true; + }); + + mSystemLayer = nullptr; } uint64_t ReliableMessageMgr::GetTickCounterFromTimePeriod(uint64_t period) @@ -84,17 +90,12 @@ void ReliableMessageMgr::TicklessDebugDumpRetransTable(const char * log) { ChipLogDetail(ExchangeManager, log); - for (RetransTableEntry & entry : mRetransTable) - { - if (entry.rc) - { - ChipLogDetail(ExchangeManager, - "EC:" ChipLogFormatExchange " MessageCounter:" ChipLogFormatMessageCounter - " NextRetransTimeCtr:%04" PRIX16, - ChipLogValueExchange(entry.rc->GetExchangeContext()), entry.retainedBuf.GetMessageCounter(), - entry.nextRetransTimeTick); - } - } + mRetransTable.ForEachActiveObject([&](auto * entry) { + ChipLogDetail(ExchangeManager, + "EC:" ChipLogFormatExchange " MessageCounter:" ChipLogFormatMessageCounter " NextRetransTimeCtr:%04" PRIX16, + ChipLogValueExchange(&entry->ec.Get()), entry->retainedBuf.GetMessageCounter(), entry->nextRetransTimeTick); + return true; + }); } #else void ReliableMessageMgr::TicklessDebugDumpRetransTable(const char * log) @@ -127,27 +128,25 @@ void ReliableMessageMgr::ExecuteActions() // Retransmit / cancel anything in the retrans table whose retrans timeout // has expired - for (RetransTableEntry & entry : mRetransTable) - { - ReliableMessageContext * rc = entry.rc; - CHIP_ERROR err = CHIP_NO_ERROR; + mRetransTable.ForEachActiveObject([&](auto * entry) { + CHIP_ERROR err = CHIP_NO_ERROR; - if (!rc || entry.nextRetransTimeTick != 0) - continue; + if (entry->nextRetransTimeTick != 0) + return true; - if (entry.retainedBuf.IsNull()) + if (entry->retainedBuf.IsNull()) { // We generally try to prevent entries with a null buffer being in a table, but it could happen // if the message dispatch (which is supposed to fill in the buffer) fails to do so _and_ returns // success (so its caller doesn't clear out the bogus table entry). // // If that were to happen, we would crash in the code below. Guard against it, just in case. - ClearRetransTable(entry); - continue; + ClearRetransTable(*entry); + return true; } - uint8_t sendCount = entry.sendCount; - uint32_t messageCounter = entry.retainedBuf.GetMessageCounter(); + uint8_t sendCount = entry->sendCount; + uint32_t messageCounter = entry->retainedBuf.GetMessageCounter(); if (sendCount == CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS) { @@ -156,29 +155,30 @@ void ReliableMessageMgr::ExecuteActions() ChipLogError(ExchangeManager, "Failed to Send CHIP MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchange " sendCount: %" PRIu8 " max retries: %d", - messageCounter, ChipLogValueExchange(rc->GetExchangeContext()), sendCount, - CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS); + messageCounter, ChipLogValueExchange(&entry->ec.Get()), sendCount, CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS); // Remove from Table - ClearRetransTable(entry); + ClearRetransTable(*entry); } // Resend from Table (if the operation fails, the entry is cleared) if (err == CHIP_NO_ERROR) - err = SendFromRetransTable(&entry); + err = SendFromRetransTable(entry); if (err == CHIP_NO_ERROR) { // If the retransmission was successful, update the passive timer - entry.nextRetransTimeTick = static_cast(rc->GetActiveRetransmitTimeoutTick()); + entry->nextRetransTimeTick = static_cast(entry->ec->GetActiveRetransmitTimeoutTick()); #if !defined(NDEBUG) ChipLogDetail(ExchangeManager, "Retransmitted MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchange " Send Cnt %d", - messageCounter, ChipLogValueExchange(rc->GetExchangeContext()), entry.sendCount); + messageCounter, ChipLogValueExchange(&entry->ec.Get()), entry->sendCount); #endif } - } + + return true; + }); TicklessDebugDumpRetransTable("ReliableMessageMgr::ExecuteActions Dumping mRetransTable entries after processing"); } @@ -221,19 +221,14 @@ void ReliableMessageMgr::ExpireTicks() } }); - for (RetransTableEntry & entry : mRetransTable) - { - ReliableMessageContext * rc = entry.rc; - if (rc) - { - // Decrement Retransmit timeout by elapsed timeticks - TickProceed(entry.nextRetransTimeTick, deltaTicks); + mRetransTable.ForEachActiveObject([&](auto * entry) { + // Decrement Retransmit timeout by elapsed timeticks + TickProceed(entry->nextRetransTimeTick, deltaTicks); #if defined(RMP_TICKLESS_DEBUG) - ChipLogDetail(ExchangeManager, "ReliableMessageMgr::ExpireTicks set nextRetransTimeTick to %u", - entry.nextRetransTimeTick); + ChipLogDetail(ExchangeManager, "ReliableMessageMgr::ExpireTicks set nextRetransTimeTick to %u", entry->nextRetransTimeTick); #endif - } // rc entry is allocated - } + return true; + }); // Re-Adjust the base time stamp to the most recent tick boundary mTimeStampBase += (deltaTicks << mTimerIntervalShift); @@ -265,58 +260,25 @@ void ReliableMessageMgr::Timeout(System::Layer * aSystemLayer, void * aAppState) CHIP_ERROR ReliableMessageMgr::AddToRetransTable(ReliableMessageContext * rc, RetransTableEntry ** rEntry) { - bool added = false; - CHIP_ERROR err = CHIP_NO_ERROR; - - VerifyOrDie(rc != nullptr); - - if (rc->IsOccupied()) - { - // This can happen if we have a misbehaving peer that is not sending - // acks with its application-level responses when it should, so we end - // up with two outstanding app-level messages both waiting for an ack. - // Just give up and error out in that case. - return CHIP_ERROR_INCORRECT_STATE; - } - - for (RetransTableEntry & entry : mRetransTable) - { - // Check the exchContext pointer for finding an empty slot in Table - if (!entry.rc) - { - // Expire any virtual ticks that have expired so all wakeup sources reflect the current time - ExpireTicks(); - - entry.rc = rc; - entry.sendCount = 0; - entry.retainedBuf = EncryptedPacketBufferHandle(); - - *rEntry = &entry; + VerifyOrDie(!rc->IsMessageNotAcked()); - // Increment the reference count - rc->RetainContext(); - rc->SetOccupied(true); - added = true; + // Expire any virtual ticks that have expired so all wakeup sources reflect the current time + ExpireTicks(); - break; - } - } + *rEntry = mRetransTable.CreateObject(rc); - if (!added) + if (*rEntry == nullptr) { ChipLogError(ExchangeManager, "mRetransTable Already Full"); - err = CHIP_ERROR_RETRANS_TABLE_FULL; + return CHIP_ERROR_RETRANS_TABLE_FULL; } - return err; + return CHIP_NO_ERROR; } void ReliableMessageMgr::StartRetransmision(RetransTableEntry * entry) { - VerifyOrReturn(entry != nullptr && entry->rc != nullptr, - ChipLogError(ExchangeManager, "StartRetransmission was called for invalid entry")); - - entry->nextRetransTimeTick = static_cast(entry->rc->GetInitialRetransmitTimeoutTick() + + entry->nextRetransTimeTick = static_cast(entry->ec->GetInitialRetransmitTimeoutTick() + GetTickCounterFromTimeDelta(System::Clock::GetMonotonicMilliseconds())); // Check if the timer needs to be started and start it. @@ -325,36 +287,37 @@ void ReliableMessageMgr::StartRetransmision(RetransTableEntry * entry) void ReliableMessageMgr::PauseRetransmision(ReliableMessageContext * rc, uint32_t PauseTimeMillis) { - for (RetransTableEntry & entry : mRetransTable) - { - if (entry.rc == rc) + mRetransTable.ForEachActiveObject([&](auto * entry) { + if (entry->ec->GetReliableMessageContext() == rc) { - entry.nextRetransTimeTick = static_cast(entry.nextRetransTimeTick + (PauseTimeMillis >> mTimerIntervalShift)); - break; + entry->nextRetransTimeTick = + static_cast(entry->nextRetransTimeTick + (PauseTimeMillis >> mTimerIntervalShift)); + return false; } - } + return true; + }); } void ReliableMessageMgr::ResumeRetransmision(ReliableMessageContext * rc) { - for (RetransTableEntry & entry : mRetransTable) - { - if (entry.rc == rc) + mRetransTable.ForEachActiveObject([&](auto * entry) { + if (entry->ec->GetReliableMessageContext() == rc) { - entry.nextRetransTimeTick = 0; - break; + entry->nextRetransTimeTick = 0; + return false; } - } + return true; + }); } bool ReliableMessageMgr::CheckAndRemRetransTable(ReliableMessageContext * rc, uint32_t ackMessageCounter) { - for (RetransTableEntry & entry : mRetransTable) - { - if ((entry.rc == rc) && entry.retainedBuf.GetMessageCounter() == ackMessageCounter) + bool removed = false; + mRetransTable.ForEachActiveObject([&](auto * entry) { + if (entry->ec->GetReliableMessageContext() == rc && entry->retainedBuf.GetMessageCounter() == ackMessageCounter) { // Clear the entry from the retransmision table. - ClearRetransTable(entry); + ClearRetransTable(*entry); #if !defined(NDEBUG) ChipLogDetail(ExchangeManager, @@ -362,35 +325,31 @@ bool ReliableMessageMgr::CheckAndRemRetransTable(ReliableMessageContext * rc, ui " from Retrans Table on exchange " ChipLogFormatExchange, ackMessageCounter, ChipLogValueExchange(rc->GetExchangeContext())); #endif - return true; + removed = true; + return false; } - } + return true; + }); - return false; + return removed; } CHIP_ERROR ReliableMessageMgr::SendFromRetransTable(RetransTableEntry * entry) { - ReliableMessageContext * rc = entry->rc; - if (rc == nullptr) - { - return CHIP_NO_ERROR; - } - - const ExchangeMessageDispatch * dispatcher = rc->GetExchangeContext()->GetMessageDispatch(); - if (dispatcher == nullptr || !rc->GetExchangeContext()->HasSecureSession()) + const ExchangeMessageDispatch * dispatcher = entry->ec->GetMessageDispatch(); + if (dispatcher == nullptr || !entry->ec->HasSecureSession()) { // Using same error message for all errors to reduce code size. ChipLogError(ExchangeManager, "Crit-err %" CHIP_ERROR_FORMAT " when sending CHIP MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchange ", send tries: %d", CHIP_ERROR_INCORRECT_STATE.Format(), entry->retainedBuf.GetMessageCounter(), - ChipLogValueExchange(rc->GetExchangeContext()), entry->sendCount); + ChipLogValueExchange(&entry->ec.Get()), entry->sendCount); ClearRetransTable(*entry); return CHIP_ERROR_INCORRECT_STATE; } - CHIP_ERROR err = dispatcher->SendPreparedMessage(rc->GetExchangeContext()->GetSecureSession(), entry->retainedBuf); + CHIP_ERROR err = dispatcher->SendPreparedMessage(entry->ec->GetSecureSession(), entry->retainedBuf); if (err == CHIP_NO_ERROR) { @@ -404,7 +363,7 @@ CHIP_ERROR ReliableMessageMgr::SendFromRetransTable(RetransTableEntry * entry) ChipLogError(ExchangeManager, "Crit-err %" CHIP_ERROR_FORMAT " when sending CHIP MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchange ", send tries: %d", - err.Format(), entry->retainedBuf.GetMessageCounter(), ChipLogValueExchange(rc->GetExchangeContext()), + err.Format(), entry->retainedBuf.GetMessageCounter(), ChipLogValueExchange(&entry->ec.Get()), entry->sendCount); ClearRetransTable(*entry); @@ -414,48 +373,32 @@ CHIP_ERROR ReliableMessageMgr::SendFromRetransTable(RetransTableEntry * entry) void ReliableMessageMgr::ClearRetransTable(ReliableMessageContext * rc) { - for (RetransTableEntry & entry : mRetransTable) - { - if (entry.rc == rc) + RetransTableEntry * result = nullptr; + mRetransTable.ForEachActiveObject([&](auto * entry) { + if (entry->ec->GetReliableMessageContext() == rc) { - // Clear the retransmit table entry. - ClearRetransTable(entry); + result = entry; + return false; } + return true; + }); + if (result != nullptr) + { + ClearRetransTable(*result); } } -void ReliableMessageMgr::ClearRetransTable(RetransTableEntry & rEntry) +void ReliableMessageMgr::ClearRetransTable(RetransTableEntry & entry) { - if (rEntry.rc) - { - VerifyOrDie(rEntry.rc->IsOccupied() == true); - - // Expire any virtual ticks that have expired so all wakeup sources reflect the current time - ExpireTicks(); - - rEntry.rc->SetOccupied(false); - rEntry.rc->ReleaseContext(); - rEntry.rc = nullptr; - - // Clear all other fields - rEntry = RetransTableEntry(); - - // Schedule next physical wakeup, unless shutting down - if (mSystemLayer) - StartTimer(); - } + mRetransTable.ReleaseObject(&entry); + // Expire any virtual ticks that have expired so all wakeup sources reflect the current time + ExpireTicks(); + StartTimer(); } void ReliableMessageMgr::FailRetransTableEntries(ReliableMessageContext * rc, CHIP_ERROR err) { - for (RetransTableEntry & entry : mRetransTable) - { - if (entry.rc == rc) - { - // Remove the entry from the retransmission table. - ClearRetransTable(entry); - } - } + ClearRetransTable(rc); } void ReliableMessageMgr::StartTimer() @@ -477,22 +420,18 @@ void ReliableMessageMgr::StartTimer() } }); - for (RetransTableEntry & entry : mRetransTable) - { - ReliableMessageContext * rc = entry.rc; - if (rc) + mRetransTable.ForEachActiveObject([&](auto * entry) { + // When do we need to next wake up for ReliableMessageProtocol retransmit? + if (entry->nextRetransTimeTick < nextWakeTimeTick) { - // When do we need to next wake up for ReliableMessageProtocol retransmit? - if (entry.nextRetransTimeTick < nextWakeTimeTick) - { - nextWakeTimeTick = entry.nextRetransTimeTick; - foundWake = true; + nextWakeTimeTick = entry->nextRetransTimeTick; + foundWake = true; #if defined(RMP_TICKLESS_DEBUG) - ChipLogDetail(ExchangeManager, "ReliableMessageMgr::StartTimer RetransTime %" PRIu64, nextWakeTimeTick); + ChipLogDetail(ExchangeManager, "ReliableMessageMgr::StartTimer RetransTime %" PRIu64, nextWakeTimeTick); #endif - } } - } + return true; + }); if (foundWake) { @@ -548,14 +487,10 @@ void ReliableMessageMgr::StopTimer() int ReliableMessageMgr::TestGetCountRetransTable() { int count = 0; - - for (RetransTableEntry & entry : mRetransTable) - { - ReliableMessageContext * rc = entry.rc; - if (rc) - count++; - } - + mRetransTable.ForEachActiveObject([&](auto * entry) { + count++; + return true; + }); return count; } #endif // CHIP_CONFIG_TEST diff --git a/src/messaging/ReliableMessageMgr.h b/src/messaging/ReliableMessageMgr.h index 9a151f2b0f6f27..5befa502b0b753 100644 --- a/src/messaging/ReliableMessageMgr.h +++ b/src/messaging/ReliableMessageMgr.h @@ -39,6 +39,9 @@ namespace chip { namespace Messaging { +class ExchangeContext; +using ExchangeHandle = ReferenceCountedHandle; + enum class SendMessageFlags : uint16_t; class ReliableMessageContext; @@ -57,9 +60,10 @@ class ReliableMessageMgr */ struct RetransTableEntry { - RetransTableEntry(); + RetransTableEntry(ReliableMessageContext * rc); + ~RetransTableEntry(); - ReliableMessageContext * rc; /**< The context for the stored CHIP message. */ + ExchangeHandle ec; /**< The context for the stored CHIP message. */ EncryptedPacketBufferHandle retainedBuf; /**< The packet buffer holding the CHIP message. */ uint16_t nextRetransTimeTick; /**< A counter representing the next retransmission time for the message. */ uint8_t sendCount; /**< A counter representing the number of times the message has been sent. */ @@ -226,7 +230,6 @@ class ReliableMessageMgr private: BitMapObjectPool & mContextPool; chip::System::Layer * mSystemLayer; - SessionManager * mSessionManager; uint64_t mTimeStampBase; // ReliableMessageProtocol timer base value to add offsets to evaluate timeouts System::Clock::MonotonicMilliseconds mCurrentTimerExpiry; // Tracks when the ReliableMessageProtocol timer will next expire uint16_t mTimerIntervalShift; // ReliableMessageProtocol Timer tick period shift @@ -244,7 +247,7 @@ class ReliableMessageMgr void TicklessDebugDumpRetransTable(const char * log); // ReliableMessageProtocol Global tables for timer context - RetransTableEntry mRetransTable[CHIP_CONFIG_RMP_RETRANS_TABLE_SIZE]; + BitMapObjectPool mRetransTable; }; } // namespace Messaging