commit f6e34f41e235c3677155b6bda34c6c31e59956ad
author yunhanw-google <yunhanw@google.com> Mon Sep 23 19:02:50 2024 -0700
committer GitHub <noreply@github.com> Tue Sep 24 02:02:50 2024 +0000
tree ec7575b5efda9fdcd362b12eb2d22ee4fa4c732b
parent 8f80f98642d58615d0996ae55bbd4d55e70af8a7

[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 <bzbarsky@apple.com>

---------

Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

src/app/icd/server/ICDMonitoringTable.cpp

diff --git a/src/app/icd/server/ICDMonitoringTable.cpp b/src/app/icd/server/ICDMonitoringTable.cpp
index c5543c7..a6adddd 100644
--- a/src/app/icd/server/ICDMonitoringTable.cpp
+++ b/src/app/icd/server/ICDMonitoringTable.cpp
@@ -53,6 +53,7 @@
     ReturnErrorOnFailure(writer.Put(TLV::ContextTag(Fields::kClientType), clientType));
 
     ReturnErrorOnFailure(writer.EndContainer(outer));
+    ReturnErrorOnFailure(writer.Finalize());
     return CHIP_NO_ERROR;
 }
 

src/app/icd/server/ICDMonitoringTable.h

diff --git a/src/app/icd/server/ICDMonitoringTable.h b/src/app/icd/server/ICDMonitoringTable.h
index 942c56f..dca9fd2 100644
--- a/src/app/icd/server/ICDMonitoringTable.h
+++ b/src/app/icd/server/ICDMonitoringTable.h
@@ -34,11 +34,24 @@
 
 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;

src/app/icd/server/tests/TestICDMonitoringTable.cpp

diff --git a/src/app/icd/server/tests/TestICDMonitoringTable.cpp b/src/app/icd/server/tests/TestICDMonitoringTable.cpp
index 177f8ca..81fcb2a 100644
--- a/src/app/icd/server/tests/TestICDMonitoringTable.cpp
+++ b/src/app/icd/server/tests/TestICDMonitoringTable.cpp
@@ -43,6 +43,8 @@
 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 @@
     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;