diff --git a/src/app/DeviceProxy.h b/src/app/DeviceProxy.h index 6a3e214b89d166..4aee009199959f 100644 --- a/src/app/DeviceProxy.h +++ b/src/app/DeviceProxy.h @@ -68,7 +68,7 @@ class DLL_EXPORT DeviceProxy virtual uint8_t GetNextSequenceNumber() = 0; - ReliableMessageProtocolConfig mMRPConfig = gDefaultMRPConfig; + ReliableMessageProtocolConfig mMRPConfig = GetLocalMRPConfig(); }; } // namespace chip diff --git a/src/app/server/CommissioningWindowManager.cpp b/src/app/server/CommissioningWindowManager.cpp index 1ce4658343dcfd..52ad2406c1f466 100644 --- a/src/app/server/CommissioningWindowManager.cpp +++ b/src/app/server/CommissioningWindowManager.cpp @@ -182,7 +182,7 @@ CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow() ReturnErrorOnFailure(SetTemporaryDiscriminator(mECMDiscriminator)); ReturnErrorOnFailure( mPairingSession.WaitForPairing(mECMPASEVerifier, mECMIterations, ByteSpan(mECMSalt, mECMSaltLength), mECMPasscodeID, - keyID, Optional::Value(gDefaultMRPConfig), this)); + keyID, Optional::Value(GetLocalMRPConfig()), this)); } else { @@ -192,7 +192,7 @@ CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow() ReturnErrorOnFailure(mPairingSession.WaitForPairing( pinCode, kSpake2p_Iteration_Count, ByteSpan(reinterpret_cast(kSpake2pKeyExchangeSalt), strlen(kSpake2pKeyExchangeSalt)), keyID, - Optional::Value(gDefaultMRPConfig), this)); + Optional::Value(GetLocalMRPConfig()), this)); } ReturnErrorOnFailure(StartAdvertisement()); diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 20c5dae999fc10..1d079593b1d51d 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -262,7 +262,7 @@ CHIP_ERROR DnssdServer::AdvertiseOperational() .SetPeerId(fabricInfo.GetPeerId()) .SetMac(mac) .SetPort(GetSecuredPort()) - .SetMRPConfig(gDefaultMRPConfig) + .SetMRPConfig(GetLocalMRPConfig()) .SetTcpSupported(Optional(INET_CONFIG_ENABLE_TCP_ENDPOINT)) .EnableIpV4(true); @@ -341,7 +341,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi advertiseParameters.SetRotatingDeviceId(chip::Optional::Value(rotatingDeviceIdHexBuffer)); #endif - advertiseParameters.SetMRPConfig(gDefaultMRPConfig).SetTcpSupported(Optional(INET_CONFIG_ENABLE_TCP_ENDPOINT)); + advertiseParameters.SetMRPConfig(GetLocalMRPConfig()).SetTcpSupported(Optional(INET_CONFIG_ENABLE_TCP_ENDPOINT)); if (mode != chip::Dnssd::CommissioningMode::kEnabledEnhanced) { diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 814d7a9e8993f1..2ca8476c93d7de 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -144,7 +144,7 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params) .idAllocator = &mIDAllocator, .fabricTable = params.systemState->Fabrics(), .clientPool = &mCASEClientPool, - .mrpLocalConfig = Optional::Value(mMRPConfig), + .mrpLocalConfig = Optional::Value(GetLocalMRPConfig()), }; CASESessionManagerConfig sessionManagerConfig = { @@ -894,7 +894,7 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re VerifyOrExit(exchangeCtxt != nullptr, err = CHIP_ERROR_INTERNAL); err = device->GetPairing().Pair(params.GetPeerAddress(), params.GetSetupPINCode(), keyID, - Optional::Value(mMRPConfig), exchangeCtxt, this); + Optional::Value(GetLocalMRPConfig()), exchangeCtxt, this); SuccessOrExit(err); // Immediately persist the updated mNextKeyID value diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 15c4e47692d9d0..1a9664097d90a9 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -394,8 +394,6 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate uint16_t mVendorId; - ReliableMessageProtocolConfig mMRPConfig = gDefaultMRPConfig; - //////////// SessionRecoveryDelegate Implementation /////////////// void OnFirstMessageDeliveryFailed(const SessionHandle & session) override; diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp index 7d3bfce3a48917..e9b9137c45eaa5 100644 --- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp @@ -158,19 +158,7 @@ class AdvertiserMinMdns : public ServiceAdvertiser, char ** txtFields, size_t & numTxtFields) { auto optionalMrp = params.GetMRPConfig(); - // TODO: Issue #5833 - MRP retry intervals should be updated on the poll period value change or device type change. -#if CHIP_DEVICE_CONFIG_ENABLE_SED - chip::DeviceLayer::ConnectivityManager::SEDPollingConfig sedPollingConfig; - ReturnErrorOnFailure(chip::DeviceLayer::ConnectivityMgr().GetSEDPollingConfig(sedPollingConfig)); - // Increment default MRP retry intervals by SED poll period to be on the safe side - // and avoid unnecessary retransmissions. - if (optionalMrp.HasValue()) - { - auto mrp = optionalMrp.Value(); - optionalMrp.SetValue(ReliableMessageProtocolConfig(mrp.mIdleRetransTimeout + sedPollingConfig.SlowPollingIntervalMS, - mrp.mActiveRetransTimeout + sedPollingConfig.FastPollingIntervalMS)); - } -#endif + if (optionalMrp.HasValue()) { auto mrp = optionalMrp.Value(); diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 916bf27bc78c05..d805d2199511e9 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -236,18 +236,6 @@ CHIP_ERROR CopyTextRecordValue(char * buffer, size_t bufferLen, const chip::Opti auto retryInterval = isIdle ? optional.Value().mIdleRetransTimeout : optional.Value().mActiveRetransTimeout; - // TODO: Issue #5833 - MRP retry intervals should be updated on the poll period value - // change or device type change. - // TODO: Is this really the best place to set these? Seems like it should be passed - // in with the correct values and set one level up from here. -#if CHIP_DEVICE_CONFIG_ENABLE_SED - chip::DeviceLayer::ConnectivityManager::SEDPollingConfig sedPollingConfig; - ReturnErrorOnFailure(chip::DeviceLayer::ConnectivityMgr().GetSEDPollingConfig(sedPollingConfig)); - // Increment default MRP retry intervals by SED poll period to be on the safe side - // and avoid unnecessary retransmissions. - retryInterval += isIdle ? sedPollingConfig.SlowPollingIntervalMS : sedPollingConfig.FastPollingIntervalMS; -#endif - if (retryInterval > kMaxRetryInterval) { ChipLogProgress(Discovery, "MRP retry interval %s value exceeds allowed range of 1 hour, using maximum available", diff --git a/src/lib/dnssd/Resolver.h b/src/lib/dnssd/Resolver.h index 86020e551b0264..6cf4c7a55ea243 100644 --- a/src/lib/dnssd/Resolver.h +++ b/src/lib/dnssd/Resolver.h @@ -56,8 +56,9 @@ struct ResolvedNodeData ReliableMessageProtocolConfig GetMRPConfig() const { - return ReliableMessageProtocolConfig(GetMrpRetryIntervalIdle().ValueOr(gDefaultMRPConfig.mIdleRetransTimeout), - GetMrpRetryIntervalActive().ValueOr(gDefaultMRPConfig.mActiveRetransTimeout)); + const ReliableMessageProtocolConfig defaultConfig = GetLocalMRPConfig(); + return ReliableMessageProtocolConfig(GetMrpRetryIntervalIdle().ValueOr(defaultConfig.mIdleRetransTimeout), + GetMrpRetryIntervalActive().ValueOr(defaultConfig.mActiveRetransTimeout)); } Optional GetMrpRetryIntervalIdle() const { return mMrpRetryIntervalIdle; } Optional GetMrpRetryIntervalActive() const { return mMrpRetryIntervalActive; } diff --git a/src/messaging/ReliableMessageProtocolConfig.cpp b/src/messaging/ReliableMessageProtocolConfig.cpp index 76c56a8b5a96ac..3404ad97623957 100644 --- a/src/messaging/ReliableMessageProtocolConfig.cpp +++ b/src/messaging/ReliableMessageProtocolConfig.cpp @@ -24,13 +24,31 @@ #include +#include #include namespace chip { using namespace System::Clock::Literals; -const ReliableMessageProtocolConfig gDefaultMRPConfig(CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL, - CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL); +ReliableMessageProtocolConfig GetLocalMRPConfig() +{ + ReliableMessageProtocolConfig config(CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL, + CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL); + +#if CHIP_DEVICE_CONFIG_ENABLE_SED + DeviceLayer::ConnectivityManager::SEDPollingConfig sedPollingConfig; + + if (DeviceLayer::ConnectivityMgr().GetSEDPollingConfig(sedPollingConfig) == CHIP_NO_ERROR) + { + // Increase default MRP retry intervals by SED polling intervals. That is, intervals for + // which the device can be at sleep and not be able to receive any messages). + config.mIdleRetransTimeout += sedPollingConfig.SlowPollingIntervalMS; + config.mActiveRetransTimeout += sedPollingConfig.FastPollingIntervalMS; + } +#endif + + return config; +} } // namespace chip diff --git a/src/messaging/ReliableMessageProtocolConfig.h b/src/messaging/ReliableMessageProtocolConfig.h index f03641f0139ed9..ac8d599cc954d2 100644 --- a/src/messaging/ReliableMessageProtocolConfig.h +++ b/src/messaging/ReliableMessageProtocolConfig.h @@ -117,6 +117,6 @@ struct ReliableMessageProtocolConfig System::Clock::Milliseconds32 mActiveRetransTimeout; }; -extern const ReliableMessageProtocolConfig gDefaultMRPConfig; +ReliableMessageProtocolConfig GetLocalMRPConfig(); } // namespace chip diff --git a/src/messaging/tests/MessagingContext.cpp b/src/messaging/tests/MessagingContext.cpp index b74024174d9840..4a288177de79d6 100644 --- a/src/messaging/tests/MessagingContext.cpp +++ b/src/messaging/tests/MessagingContext.cpp @@ -119,13 +119,13 @@ void MessagingContext::ExpireSessionBobToFriends() Messaging::ExchangeContext * MessagingContext::NewUnauthenticatedExchangeToAlice(Messaging::ExchangeDelegate * delegate) { - return mExchangeManager.NewContext(mSessionManager.CreateUnauthenticatedSession(mAliceAddress, gDefaultMRPConfig).Value(), + return mExchangeManager.NewContext(mSessionManager.CreateUnauthenticatedSession(mAliceAddress, GetLocalMRPConfig()).Value(), delegate); } Messaging::ExchangeContext * MessagingContext::NewUnauthenticatedExchangeToBob(Messaging::ExchangeDelegate * delegate) { - return mExchangeManager.NewContext(mSessionManager.CreateUnauthenticatedSession(mBobAddress, gDefaultMRPConfig).Value(), + return mExchangeManager.NewContext(mSessionManager.CreateUnauthenticatedSession(mBobAddress, GetLocalMRPConfig()).Value(), delegate); } diff --git a/src/messaging/tests/TestReliableMessageProtocol.cpp b/src/messaging/tests/TestReliableMessageProtocol.cpp index 4eaa0f6f5c9d77..6941514d5db33f 100644 --- a/src/messaging/tests/TestReliableMessageProtocol.cpp +++ b/src/messaging/tests/TestReliableMessageProtocol.cpp @@ -1347,8 +1347,8 @@ void CheckLostStandaloneAck(nlTestSuite * inSuite, void * inContext) int InitializeTestCase(void * inContext) { TestContext & ctx = *static_cast(inContext); - ctx.GetSessionAliceToBob()->AsSecureSession()->SetMRPConfig(gDefaultMRPConfig); - ctx.GetSessionBobToAlice()->AsSecureSession()->SetMRPConfig(gDefaultMRPConfig); + ctx.GetSessionAliceToBob()->AsSecureSession()->SetMRPConfig(GetLocalMRPConfig()); + ctx.GetSessionBobToAlice()->AsSecureSession()->SetMRPConfig(GetLocalMRPConfig()); return SUCCESS; } diff --git a/src/protocols/secure_channel/CASEServer.cpp b/src/protocols/secure_channel/CASEServer.cpp index fb524277d602ab..28f5d5f94d3969 100644 --- a/src/protocols/secure_channel/CASEServer.cpp +++ b/src/protocols/secure_channel/CASEServer.cpp @@ -72,7 +72,7 @@ CHIP_ERROR CASEServer::InitCASEHandshake(Messaging::ExchangeContext * ec) // Setup CASE state machine using the credentials for the current fabric. ReturnErrorOnFailure(GetSession().ListenForSessionEstablishment( - mSessionKeyId, mFabrics, this, Optional::Value(gDefaultMRPConfig))); + mSessionKeyId, mFabrics, this, Optional::Value(GetLocalMRPConfig()))); // Hand over the exchange context to the CASE session. ec->SetDelegate(&GetSession()); diff --git a/src/transport/GroupSession.h b/src/transport/GroupSession.h index 41ee154ed91361..6b5b25ff56d025 100644 --- a/src/transport/GroupSession.h +++ b/src/transport/GroupSession.h @@ -47,8 +47,9 @@ class GroupSession : public Session const ReliableMessageProtocolConfig & GetMRPConfig() const override { + static const ReliableMessageProtocolConfig cfg(GetLocalMRPConfig()); VerifyOrDie(false); - return gDefaultMRPConfig; + return cfg; } System::Clock::Milliseconds32 GetAckTimeout() const override diff --git a/src/transport/PairingSession.h b/src/transport/PairingSession.h index b33dc52d726222..fa692bcb69bec7 100644 --- a/src/transport/PairingSession.h +++ b/src/transport/PairingSession.h @@ -198,7 +198,7 @@ class DLL_EXPORT PairingSession Optional mPeerSessionId; - ReliableMessageProtocolConfig mMRPConfig = gDefaultMRPConfig; + ReliableMessageProtocolConfig mMRPConfig = GetLocalMRPConfig(); }; } // namespace chip diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index d15ec918535d3b..518cc1ebbc9ddc 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -440,7 +440,7 @@ void SessionManager::RefreshSessionOperationalData(const SessionHandle & session void SessionManager::MessageDispatch(const PacketHeader & packetHeader, const Transport::PeerAddress & peerAddress, System::PacketBufferHandle && msg) { - Optional optionalSession = mUnauthenticatedSessions.FindOrAllocateEntry(peerAddress, gDefaultMRPConfig); + Optional optionalSession = mUnauthenticatedSessions.FindOrAllocateEntry(peerAddress, GetLocalMRPConfig()); if (!optionalSession.HasValue()) { ChipLogError(Inet, "UnauthenticatedSession exhausted"); diff --git a/src/transport/tests/TestPairingSession.cpp b/src/transport/tests/TestPairingSession.cpp index 1701d42015d3c3..6a9f89756abb83 100644 --- a/src/transport/tests/TestPairingSession.cpp +++ b/src/transport/tests/TestPairingSession.cpp @@ -108,8 +108,8 @@ void PairingSessionTryDecodeMissingMRPParams(nlTestSuite * inSuite, void * inCon NL_TEST_ASSERT(inSuite, reader.Next() == CHIP_NO_ERROR); NL_TEST_ASSERT(inSuite, session.DecodeMRPParametersIfPresent(TLV::ContextTag(2), reader) == CHIP_NO_ERROR); - NL_TEST_ASSERT(inSuite, session.GetMRPConfig().mIdleRetransTimeout == gDefaultMRPConfig.mIdleRetransTimeout); - NL_TEST_ASSERT(inSuite, session.GetMRPConfig().mActiveRetransTimeout == gDefaultMRPConfig.mActiveRetransTimeout); + NL_TEST_ASSERT(inSuite, session.GetMRPConfig().mIdleRetransTimeout == GetLocalMRPConfig().mIdleRetransTimeout); + NL_TEST_ASSERT(inSuite, session.GetMRPConfig().mActiveRetransTimeout == GetLocalMRPConfig().mActiveRetransTimeout); } // Test Suite diff --git a/src/transport/tests/TestPeerConnections.cpp b/src/transport/tests/TestPeerConnections.cpp index e716f8604778db..02501624a53eba 100644 --- a/src/transport/tests/TestPeerConnections.cpp +++ b/src/transport/tests/TestPeerConnections.cpp @@ -71,7 +71,7 @@ void TestBasicFunctionality(nlTestSuite * inSuite, void * inContext) // Node ID 1, peer key 1, local key 2 auto optionalSession = connections.CreateNewSecureSession(kPeer1SessionType, 2, kPeer1NodeId, kPeer1CATs, 1, - 0 /* fabricIndex */, gDefaultMRPConfig); + 0 /* fabricIndex */, GetLocalMRPConfig()); NL_TEST_ASSERT(inSuite, optionalSession.HasValue()); NL_TEST_ASSERT(inSuite, optionalSession.Value()->AsSecureSession()->GetSecureSessionType() == kPeer1SessionType); NL_TEST_ASSERT(inSuite, optionalSession.Value()->AsSecureSession()->GetPeerNodeId() == kPeer1NodeId); @@ -80,7 +80,7 @@ void TestBasicFunctionality(nlTestSuite * inSuite, void * inContext) // Node ID 2, peer key 3, local key 4 optionalSession = connections.CreateNewSecureSession(kPeer2SessionType, 4, kPeer2NodeId, kPeer2CATs, 3, 0 /* fabricIndex */, - gDefaultMRPConfig); + GetLocalMRPConfig()); NL_TEST_ASSERT(inSuite, optionalSession.HasValue()); NL_TEST_ASSERT(inSuite, optionalSession.Value()->AsSecureSession()->GetSecureSessionType() == kPeer2SessionType); NL_TEST_ASSERT(inSuite, optionalSession.Value()->AsSecureSession()->GetPeerNodeId() == kPeer2NodeId); @@ -90,7 +90,7 @@ void TestBasicFunctionality(nlTestSuite * inSuite, void * inContext) // Insufficient space for new connections. Object is max size 2 optionalSession = connections.CreateNewSecureSession(kPeer3SessionType, 6, kPeer3NodeId, kPeer3CATs, 5, 0 /* fabricIndex */, - gDefaultMRPConfig); + GetLocalMRPConfig()); NL_TEST_ASSERT(inSuite, !optionalSession.HasValue()); System::Clock::Internal::SetSystemClockForTesting(realClock); } @@ -104,7 +104,7 @@ void TestFindByKeyId(nlTestSuite * inSuite, void * inContext) // Node ID 1, peer key 1, local key 2 auto optionalSession = connections.CreateNewSecureSession(kPeer1SessionType, 2, kPeer1NodeId, kPeer1CATs, 1, - 0 /* fabricIndex */, gDefaultMRPConfig); + 0 /* fabricIndex */, GetLocalMRPConfig()); NL_TEST_ASSERT(inSuite, optionalSession.HasValue()); NL_TEST_ASSERT(inSuite, !connections.FindSecureSessionByLocalKey(1).HasValue()); @@ -112,7 +112,7 @@ void TestFindByKeyId(nlTestSuite * inSuite, void * inContext) // Node ID 2, peer key 3, local key 4 optionalSession = connections.CreateNewSecureSession(kPeer2SessionType, 4, kPeer2NodeId, kPeer2CATs, 3, 0 /* fabricIndex */, - gDefaultMRPConfig); + GetLocalMRPConfig()); NL_TEST_ASSERT(inSuite, optionalSession.HasValue()); NL_TEST_ASSERT(inSuite, !connections.FindSecureSessionByLocalKey(3).HasValue()); @@ -141,21 +141,21 @@ void TestExpireConnections(nlTestSuite * inSuite, void * inContext) // Node ID 1, peer key 1, local key 2 auto optionalSession = connections.CreateNewSecureSession(kPeer1SessionType, 2, kPeer1NodeId, kPeer1CATs, 1, - 0 /* fabricIndex */, gDefaultMRPConfig); + 0 /* fabricIndex */, GetLocalMRPConfig()); NL_TEST_ASSERT(inSuite, optionalSession.HasValue()); optionalSession.Value()->AsSecureSession()->SetPeerAddress(kPeer1Addr); clock.SetMonotonic(200_ms64); // Node ID 2, peer key 3, local key 4 optionalSession = connections.CreateNewSecureSession(kPeer2SessionType, 4, kPeer2NodeId, kPeer2CATs, 3, 0 /* fabricIndex */, - gDefaultMRPConfig); + GetLocalMRPConfig()); NL_TEST_ASSERT(inSuite, optionalSession.HasValue()); optionalSession.Value()->AsSecureSession()->SetPeerAddress(kPeer2Addr); // cannot add before expiry clock.SetMonotonic(300_ms64); optionalSession = connections.CreateNewSecureSession(kPeer3SessionType, 6, kPeer3NodeId, kPeer3CATs, 5, 0 /* fabricIndex */, - gDefaultMRPConfig); + GetLocalMRPConfig()); NL_TEST_ASSERT(inSuite, !optionalSession.HasValue()); // at time 300, this expires ip addr 1 @@ -173,7 +173,7 @@ void TestExpireConnections(nlTestSuite * inSuite, void * inContext) clock.SetMonotonic(300_ms64); // Node ID 3, peer key 5, local key 6 optionalSession = connections.CreateNewSecureSession(kPeer3SessionType, 6, kPeer3NodeId, kPeer3CATs, 5, 0 /* fabricIndex */, - gDefaultMRPConfig); + GetLocalMRPConfig()); NL_TEST_ASSERT(inSuite, optionalSession.HasValue()); optionalSession.Value()->AsSecureSession()->SetPeerAddress(kPeer3Addr); @@ -206,7 +206,7 @@ void TestExpireConnections(nlTestSuite * inSuite, void * inContext) // Node ID 1, peer key 1, local key 2 optionalSession = connections.CreateNewSecureSession(kPeer1SessionType, 2, kPeer1NodeId, kPeer1CATs, 1, 0 /* fabricIndex */, - gDefaultMRPConfig); + GetLocalMRPConfig()); NL_TEST_ASSERT(inSuite, optionalSession.HasValue()); NL_TEST_ASSERT(inSuite, connections.FindSecureSessionByLocalKey(2).HasValue()); NL_TEST_ASSERT(inSuite, connections.FindSecureSessionByLocalKey(4).HasValue());