Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SED] Take SED polling intervals into account in all MRP contexts #14417

Merged
merged 1 commit into from
Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/app/DeviceProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class DLL_EXPORT DeviceProxy

virtual uint8_t GetNextSequenceNumber() = 0;

ReliableMessageProtocolConfig mMRPConfig = gDefaultMRPConfig;
ReliableMessageProtocolConfig mMRPConfig = GetLocalMRPConfig();
};

} // namespace chip
4 changes: 2 additions & 2 deletions src/app/server/CommissioningWindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow()
ReturnErrorOnFailure(SetTemporaryDiscriminator(mECMDiscriminator));
ReturnErrorOnFailure(
mPairingSession.WaitForPairing(mECMPASEVerifier, mECMIterations, ByteSpan(mECMSalt, mECMSaltLength), mECMPasscodeID,
keyID, Optional<ReliableMessageProtocolConfig>::Value(gDefaultMRPConfig), this));
keyID, Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), this));
}
else
{
Expand All @@ -190,7 +190,7 @@ CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow()
ReturnErrorOnFailure(mPairingSession.WaitForPairing(
pinCode, kSpake2p_Iteration_Count,
ByteSpan(reinterpret_cast<const uint8_t *>(kSpake2pKeyExchangeSalt), strlen(kSpake2pKeyExchangeSalt)), keyID,
Optional<ReliableMessageProtocolConfig>::Value(gDefaultMRPConfig), this));
Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), this));
}

ReturnErrorOnFailure(StartAdvertisement());
Expand Down
4 changes: 2 additions & 2 deletions src/app/server/Dnssd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ CHIP_ERROR DnssdServer::AdvertiseOperational()
.SetPeerId(fabricInfo.GetPeerId())
.SetMac(mac)
.SetPort(GetSecuredPort())
.SetMRPConfig(gDefaultMRPConfig)
.SetMRPConfig(GetLocalMRPConfig())
.SetTcpSupported(Optional<bool>(INET_CONFIG_ENABLE_TCP_ENDPOINT))
.EnableIpV4(true);

Expand Down Expand Up @@ -341,7 +341,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
advertiseParameters.SetRotatingDeviceId(chip::Optional<const char *>::Value(rotatingDeviceIdHexBuffer));
#endif

advertiseParameters.SetMRPConfig(gDefaultMRPConfig).SetTcpSupported(Optional<bool>(INET_CONFIG_ENABLE_TCP_ENDPOINT));
advertiseParameters.SetMRPConfig(GetLocalMRPConfig()).SetTcpSupported(Optional<bool>(INET_CONFIG_ENABLE_TCP_ENDPOINT));

if (mode != chip::Dnssd::CommissioningMode::kEnabledEnhanced)
{
Expand Down
4 changes: 2 additions & 2 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params)
.idAllocator = &mIDAllocator,
.fabricTable = params.systemState->Fabrics(),
.clientPool = &mCASEClientPool,
.mrpLocalConfig = Optional<ReliableMessageProtocolConfig>::Value(mMRPConfig),
.mrpLocalConfig = Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()),
};

CASESessionManagerConfig sessionManagerConfig = {
Expand Down Expand Up @@ -886,7 +886,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<ReliableMessageProtocolConfig>::Value(mMRPConfig), exchangeCtxt, this);
Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), exchangeCtxt, this);
Damian-Nordic marked this conversation as resolved.
Show resolved Hide resolved
SuccessOrExit(err);

// Immediately persist the updated mNextKeyID value
Expand Down
2 changes: 0 additions & 2 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,6 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate

uint16_t mVendorId;

ReliableMessageProtocolConfig mMRPConfig = gDefaultMRPConfig;

//////////// SessionRecoveryDelegate Implementation ///////////////
void OnFirstMessageDeliveryFailed(const SessionHandle & session) override;

Expand Down
14 changes: 1 addition & 13 deletions src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
12 changes: 0 additions & 12 deletions src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 3 additions & 2 deletions src/lib/dnssd/Resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<System::Clock::Milliseconds32> GetMrpRetryIntervalIdle() const { return mMrpRetryIntervalIdle; }
Optional<System::Clock::Milliseconds32> GetMrpRetryIntervalActive() const { return mMrpRetryIntervalActive; }
Expand Down
22 changes: 20 additions & 2 deletions src/messaging/ReliableMessageProtocolConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,31 @@

#include <messaging/ReliableMessageProtocolConfig.h>

#include <platform/CHIPDeviceLayer.h>
#include <system/SystemClock.h>

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
2 changes: 1 addition & 1 deletion src/messaging/ReliableMessageProtocolConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,6 @@ struct ReliableMessageProtocolConfig
System::Clock::Milliseconds32 mActiveRetransTimeout;
};

extern const ReliableMessageProtocolConfig gDefaultMRPConfig;
ReliableMessageProtocolConfig GetLocalMRPConfig();

} // namespace chip
4 changes: 2 additions & 2 deletions src/messaging/tests/MessagingContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,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);
}

Expand Down
4 changes: 2 additions & 2 deletions src/messaging/tests/TestReliableMessageProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1347,8 +1347,8 @@ void CheckLostStandaloneAck(nlTestSuite * inSuite, void * inContext)
int InitializeTestCase(void * inContext)
{
TestContext & ctx = *static_cast<TestContext *>(inContext);
ctx.GetSessionAliceToBob()->AsSecureSession()->SetMRPConfig(gDefaultMRPConfig);
ctx.GetSessionBobToAlice()->AsSecureSession()->SetMRPConfig(gDefaultMRPConfig);
ctx.GetSessionAliceToBob()->AsSecureSession()->SetMRPConfig(GetLocalMRPConfig());
ctx.GetSessionBobToAlice()->AsSecureSession()->SetMRPConfig(GetLocalMRPConfig());
return SUCCESS;
}

Expand Down
2 changes: 1 addition & 1 deletion src/protocols/secure_channel/CASEServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<ReliableMessageProtocolConfig>::Value(gDefaultMRPConfig)));
mSessionKeyId, mFabrics, this, Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig())));

// Hand over the exchange context to the CASE session.
ec->SetDelegate(&GetSession());
Expand Down
3 changes: 2 additions & 1 deletion src/transport/GroupSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/transport/PairingSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ class DLL_EXPORT PairingSession

Optional<uint16_t> mPeerSessionId;

ReliableMessageProtocolConfig mMRPConfig = gDefaultMRPConfig;
ReliableMessageProtocolConfig mMRPConfig = GetLocalMRPConfig();
};

} // namespace chip
2 changes: 1 addition & 1 deletion src/transport/SessionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ void SessionManager::RefreshSessionOperationalData(const SessionHandle & session
void SessionManager::MessageDispatch(const PacketHeader & packetHeader, const Transport::PeerAddress & peerAddress,
System::PacketBufferHandle && msg)
{
Optional<SessionHandle> optionalSession = mUnauthenticatedSessions.FindOrAllocateEntry(peerAddress, gDefaultMRPConfig);
Optional<SessionHandle> optionalSession = mUnauthenticatedSessions.FindOrAllocateEntry(peerAddress, GetLocalMRPConfig());
if (!optionalSession.HasValue())
{
ChipLogError(Inet, "UnauthenticatedSession exhausted");
Expand Down
4 changes: 2 additions & 2 deletions src/transport/tests/TestPairingSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 10 additions & 10 deletions src/transport/tests/TestPeerConnections.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
}
Expand All @@ -104,15 +104,15 @@ 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());
NL_TEST_ASSERT(inSuite, connections.FindSecureSessionByLocalKey(2).HasValue());

// 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());
Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand Down Expand Up @@ -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());
Expand Down