Skip to content

Commit ac1ebc5

Browse files
Fix incorrect snapshotting of MRP config by CASE client. (project-chip#32380)
* Fix incorrect snapshotting of MRP config by CASE client. Right now, we snapshot the MRP config as part of the CASEClientInitParams during stack startup. After that, we will use that snapshotted config whenever we are the CASE initiator. This config will not match the parameters we use as CASE responder or advertise over DNS-SD if the local MRP configuration ever changes. Which for an ICD it can. The fix is to stop the incorrect snapshotting and get the information we need from the right source of truth when we need it. * Address review comments. * Address issue with CASE session not assuming the right things when NullOptional is passed in.
1 parent a986606 commit ac1ebc5

File tree

4 files changed

+15
-4
lines changed

4 files changed

+15
-4
lines changed

src/app/CASEClient.cpp

+4-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717

1818
#include <app/CASEClient.h>
19+
#include <messaging/ReliableMessageProtocolConfig.h>
1920

2021
namespace chip {
2122

@@ -49,10 +50,12 @@ CHIP_ERROR CASEClient::EstablishSession(const CASEClientInitParams & params, con
4950
Messaging::ExchangeContext * exchange = params.exchangeMgr->NewContext(session.Value(), &mCASESession);
5051
VerifyOrReturnError(exchange != nullptr, CHIP_ERROR_INTERNAL);
5152

53+
const Optional<ReliableMessageProtocolConfig> & mrpLocalConfig =
54+
params.mrpLocalConfig.HasValue() ? params.mrpLocalConfig : GetLocalMRPConfig();
5255
mCASESession.SetGroupDataProvider(params.groupDataProvider);
5356
ReturnErrorOnFailure(mCASESession.EstablishSession(*params.sessionManager, params.fabricTable, peer, exchange,
5457
params.sessionResumptionStorage, params.certificateValidityPolicy, delegate,
55-
params.mrpLocalConfig));
58+
mrpLocalConfig));
5659

5760
return CHIP_NO_ERROR;
5861
}

src/app/CASEClient.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,11 @@ struct CASEClientInitParams
3434
Messaging::ExchangeManager * exchangeMgr = nullptr;
3535
FabricTable * fabricTable = nullptr;
3636
Credentials::GroupDataProvider * groupDataProvider = nullptr;
37-
Optional<ReliableMessageProtocolConfig> mrpLocalConfig = Optional<ReliableMessageProtocolConfig>::Missing();
37+
38+
// mrpLocalConfig should not generally be set to anything other than
39+
// NullOptional. Doing that can lead to different parts of the system
40+
// claiming different MRP parameters for the same node.
41+
Optional<ReliableMessageProtocolConfig> mrpLocalConfig = NullOptional;
3842

3943
CHIP_ERROR Validate() const
4044
{

src/app/server/Server.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,9 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
313313
.exchangeMgr = &mExchangeMgr,
314314
.fabricTable = &mFabrics,
315315
.groupDataProvider = mGroupsProvider,
316-
.mrpLocalConfig = GetLocalMRPConfig(),
316+
// Don't provide an MRP local config, so each CASE initiation will use
317+
// the then-current value.
318+
.mrpLocalConfig = NullOptional,
317319
},
318320
.clientPool = &mCASEClientPool,
319321
.sessionSetupPool = &mSessionSetupPool,

src/controller/CHIPDeviceControllerFactory.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,9 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
270270
.exchangeMgr = stateParams.exchangeMgr,
271271
.fabricTable = stateParams.fabricTable,
272272
.groupDataProvider = stateParams.groupDataProvider,
273-
.mrpLocalConfig = GetLocalMRPConfig(),
273+
// Don't provide an MRP local config, so each CASE initiation will use
274+
// the then-current value.
275+
.mrpLocalConfig = NullOptional,
274276
};
275277

276278
CASESessionManagerConfig sessionManagerConfig = {

0 commit comments

Comments
 (0)