From 205312463dc824ede1bffa777dd9f1c6f9338222 Mon Sep 17 00:00:00 2001 From: Zang MingJie Date: Fri, 24 Sep 2021 18:33:16 +0800 Subject: [PATCH] Migrate ReliableMessageMgr::mRetransTable to Pool --- src/messaging/ReliableMessageContext.cpp | 10 - src/messaging/ReliableMessageContext.h | 1 - src/messaging/ReliableMessageMgr.cpp | 267 +++++++++-------------- src/messaging/ReliableMessageMgr.h | 11 +- 4 files changed, 114 insertions(+), 175 deletions(-) diff --git a/src/messaging/ReliableMessageContext.cpp b/src/messaging/ReliableMessageContext.cpp index cedd923495b062..c9d8ff77aa7b1b 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); diff --git a/src/messaging/ReliableMessageContext.h b/src/messaging/ReliableMessageContext.h index 410d1d01173bcb..2cb5e724140265 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 diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp index e887dc1fed6e5a..9c415411febfd2 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->SetOccupied(true); +} + +ReliableMessageMgr::RetransTableEntry::~RetransTableEntry() +{ + ec->SetOccupied(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,16 +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.messageCounter, entry.nextRetransTimeTick); - } - } + mRetransTable.ForEachActiveObject([&](auto * entry) { + ChipLogDetail(ExchangeManager, + "EC:" ChipLogFormatExchange " MessageCounter:" ChipLogFormatMessageCounter " NextRetransTimeCtr:%04" PRIX16, + ChipLogValueExchange(entry->rc->GetExchangeContext()), entry->messageCounter, entry->nextRetransTimeTick); + return true; + }); } #else void ReliableMessageMgr::TicklessDebugDumpRetransTable(const char * log) @@ -126,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) { @@ -155,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"); } @@ -220,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); @@ -264,35 +260,16 @@ 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 && !rc->IsOccupied()); - 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 = mRetransTable.CreateObject(rc); - *rEntry = &entry; + // Expire any virtual ticks that have expired so all wakeup sources reflect the current time + ExpireTicks(); - // Increment the reference count - rc->RetainContext(); - rc->SetOccupied(true); - added = true; - - break; - } - } - - if (!added) + if (rEntry == nullptr) { ChipLogError(ExchangeManager, "mRetransTable Already Full"); err = CHIP_ERROR_RETRANS_TABLE_FULL; @@ -303,10 +280,7 @@ CHIP_ERROR ReliableMessageMgr::AddToRetransTable(ReliableMessageContext * rc, Re 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. @@ -315,36 +289,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, @@ -352,35 +327,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) { @@ -394,7 +365,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); @@ -404,48 +375,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() @@ -467,22 +422,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 %u", nextWakeTimeTick); + ChipLogDetail(ExchangeManager, "ReliableMessageMgr::StartTimer RetransTime %u", nextWakeTimeTick); #endif - } } - } + return true; + }); if (foundWake) { @@ -538,14 +489,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