From f6e34f41e235c3677155b6bda34c6c31e59956ad Mon Sep 17 00:00:00 2001 From: yunhanw-google Date: Mon, 23 Sep 2024 19:02:50 -0700 Subject: [PATCH] [SVE][ICD] Bump MaxICDMonitoringEntrySize to reasonable size (#35737) * Fix icd monitor entry size * Restyled by whitespace * Restyled by clang-format * port test from pr 35734 * update documentation * Update src/app/icd/server/ICDMonitoringTable.h Co-authored-by: Boris Zbarsky --------- Co-authored-by: Restyled.io Co-authored-by: Boris Zbarsky --- src/app/icd/server/ICDMonitoringTable.cpp | 1 + src/app/icd/server/ICDMonitoringTable.h | 17 +++++++++++++++-- .../icd/server/tests/TestICDMonitoringTable.cpp | 16 ++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/app/icd/server/ICDMonitoringTable.cpp b/src/app/icd/server/ICDMonitoringTable.cpp index c5543c74125f52..a6adddde2e7404 100644 --- a/src/app/icd/server/ICDMonitoringTable.cpp +++ b/src/app/icd/server/ICDMonitoringTable.cpp @@ -53,6 +53,7 @@ CHIP_ERROR ICDMonitoringEntry::Serialize(TLV::TLVWriter & writer) const ReturnErrorOnFailure(writer.Put(TLV::ContextTag(Fields::kClientType), clientType)); ReturnErrorOnFailure(writer.EndContainer(outer)); + ReturnErrorOnFailure(writer.Finalize()); return CHIP_NO_ERROR; } diff --git a/src/app/icd/server/ICDMonitoringTable.h b/src/app/icd/server/ICDMonitoringTable.h index 942c56fda45e71..dca9fd21972341 100644 --- a/src/app/icd/server/ICDMonitoringTable.h +++ b/src/app/icd/server/ICDMonitoringTable.h @@ -34,11 +34,24 @@ using SymmetricKeystore = SessionKeystore; namespace chip { -inline constexpr size_t kICDMonitoringBufferSize = 60; +static constexpr size_t MaxICDMonitoringEntrySize() +{ + // All the fields added together + return TLV::EstimateStructOverhead(sizeof(NodeId) /*checkInNodeID*/, sizeof(uint64_t) /*monitoredSubject*/, + sizeof(Crypto::Symmetric128BitsKeyByteArray) /*aes_key_handle*/, + sizeof(Crypto::Symmetric128BitsKeyByteArray) /*hmac_key_handle*/, + sizeof(uint8_t) /*client_type*/) * + // Provide 50% extra space to make a firmware upgrade that starts storing + // more data followed by a downgrade work easily and reliably. + // The 50% number is chosen fairly randomly; storage increases larger than that are + // possible but need to be staged carefully. + 3 / 2; +} + +inline constexpr size_t kICDMonitoringBufferSize = MaxICDMonitoringEntrySize(); struct ICDMonitoringEntry : public PersistentData { - ICDMonitoringEntry(FabricIndex fabric = kUndefinedFabricIndex, NodeId nodeId = kUndefinedNodeId) { this->fabricIndex = fabric; diff --git a/src/app/icd/server/tests/TestICDMonitoringTable.cpp b/src/app/icd/server/tests/TestICDMonitoringTable.cpp index 177f8caba507e1..81fcb2af18a250 100644 --- a/src/app/icd/server/tests/TestICDMonitoringTable.cpp +++ b/src/app/icd/server/tests/TestICDMonitoringTable.cpp @@ -43,6 +43,8 @@ constexpr uint64_t kClientNodeId13 = 0x100003; constexpr uint64_t kClientNodeId21 = 0x200001; constexpr uint64_t kClientNodeId22 = 0x200002; +constexpr uint64_t kClientNodeMaxValue = std::numeric_limits::max(); + constexpr uint8_t kKeyBuffer0a[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; constexpr uint8_t kKeyBuffer0b[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; @@ -98,6 +100,20 @@ TEST(TestICDMonitoringTable, TestEntryAssignationOverload) EXPECT_TRUE(entry2.IsKeyEquivalent(ByteSpan(kKeyBuffer1a))); } +TEST(TestICDMonitoringTable, TestEntryMaximumSize) +{ + TestPersistentStorageDelegate storage; + TestSessionKeystoreImpl keystore; + ICDMonitoringTable table(storage, kTestFabricIndex1, kMaxTestClients1, &keystore); + + ICDMonitoringEntry entry(&keystore); + entry.checkInNodeID = kClientNodeMaxValue; + entry.monitoredSubject = kClientNodeMaxValue; + entry.clientType = ClientTypeEnum::kPermanent; + EXPECT_EQ(CHIP_NO_ERROR, entry.SetKey(ByteSpan(kKeyBuffer1a))); + EXPECT_EQ(CHIP_NO_ERROR, table.Set(0, entry)); +} + TEST(TestICDMonitoringTable, TestEntryKeyFunctions) { TestSessionKeystoreImpl keystore;