[SVE][ICD] Bump MaxICDMonitoringEntrySize to reasonable size (project…

…-chip#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 <[email protected]> --------- Co-authored-by: Restyled.io <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]>
erwinpan1 · Sep 24, 2024 · f6e34f4 · f6e34f4
1 parent 8f80f98
commit f6e34f4
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 2 deletions.
diff --git 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
@@ -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<kICDMonitoringBufferSize>
 {
-
     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
@@ -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<uint64_t>::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;