Addressed review Comments

shripad621git · Dec 7, 2023 · 3ebd82e · 3ebd82e
1 parent 525db41
commit 3ebd82e
Show file tree

Hide file tree

Showing 4 changed files with 121 additions and 6 deletions.
diff --git a/src/tracing/esp32_trace/BUILD.gn b/src/tracing/esp32_trace/BUILD.gn
@@ -50,5 +50,4 @@ source_set("esp32_trace_tracing") {
   public = [ "include/matter/tracing/macros_impl.h" ]
   public_configs = [ ":tracing" ]
   deps = [ ":backend" ]
-
 }
diff --git a/src/tracing/esp32_trace/esp32_tracing.cpp b/src/tracing/esp32_trace/esp32_tracing.cpp
@@ -17,6 +17,7 @@
  */
 
 #include "esp32_tracing.h"
+#include <algorithm>
 #include <esp_heap_caps.h>
 #include <esp_insights.h>
 #include <esp_log.h>
@@ -26,6 +27,107 @@
 namespace chip {
 namespace Tracing {
 namespace Insights {
+namespace {
+
+constexpr size_t kPermitListMaxSize = 10;
+using HashValue                     = uint32_t;
+
+// Implements a murmurhash with 0 seed.
+uint32_t MurmurHash(const void * key)
+{
+    const uint32_t kMultiplier = 0x5bd1e995;
+    const uint32_t kShift      = 24;
+    const unsigned char * data = (const unsigned char *) key;
+    uint32_t hash              = 0;
+
+    while (*data)
+    {
+        uint32_t value = *data++;
+        value *= kMultiplier;
+        value ^= value >> kShift;
+        value *= kMultiplier;
+        hash *= kMultiplier;
+        hash ^= value;
+    }
+
+    hash ^= hash >> 13;
+    hash *= kMultiplier;
+    hash ^= hash >> 15;
+
+    if (hash == 0)
+    {
+        ESP_LOGW("Tracing", "MurmurHash resulted in a hash value of 0");
+    }
+
+    return hash;
+}
+
+/* PASESession,CASESession,NetworkCommissioning,GeneralCommissioning,OperationalCredentials
+ * are well known permitted entries.
+ */
+
+HashValue gPermitList[kPermitListMaxSize] = {
+    MurmurHash("PASESession"),
+    MurmurHash("CASESession"),
+    MurmurHash("NetworkCommissioning"),
+    MurmurHash("GeneralCommissioning"),
+    MurmurHash("OperationalCredentials"),
+};
+
+bool IsPermitted(HashValue hashValue)
+{
+    for (HashValue permitted : gPermitList)
+    {
+        if (permitted == 0)
+        {
+            break;
+        }
+        if (hashValue == permitted)
+        {
+            return true;
+        }
+    }
+    return false;
+}
+
+} // namespace
+
+namespace ESP32Filter {
+
+CHIP_ERROR AddHashToPermitlist(const char * str)
+{
+    HashValue hashValue = MurmurHash(str);
+    if (hashValue == 0)
+    {
+        ESP_LOGW("TRC", "Hash value for '%s' is 0", str);
+        return CHIP_ERROR_INCORRECT_STATE;
+    }
+
+    for (HashValue & permitted : gPermitList)
+    {
+        if (permitted == 0)
+        {
+            permitted = hashValue;
+            return CHIP_NO_ERROR;
+        }
+        if (hashValue == permitted)
+        {
+            ESP_LOGW("TRC", "Hash value for '%s' is colliding with an existing entry", str);
+            return CHIP_ERROR_DUPLICATE_KEY_ID;
+        }
+    }
+    return CHIP_ERROR_NO_MEMORY;
+}
+
+void RemoveHashFromPermitlist(const char * str)
+{
+    HashValue hashValue = MurmurHash(str);
+
+    auto * end = gPermitList + kPermitListMaxSize;
+    std::fill(std::remove(gPermitList, end, hashValue), end, 0);
+}
+
+} // namespace ESP32Filter
 
 #define LOG_HEAP_INFO(label, group, entry_exit)                                                                                    \
     do                                                                                                                             \
@@ -48,12 +150,20 @@ void ESP32Backend::LogNodeDiscoveryFailed(NodeDiscoveryFailedInfo & info) {}
 
 void ESP32Backend::TraceBegin(const char * label, const char * group)
 {
-    LOG_HEAP_INFO(label, group, "Entry");
+    HashValue hashValue = MurmurHash(group);
+    if (IsPermitted(hashValue))
+    {
+        LOG_HEAP_INFO(label, group, "Entry");
+    }
 }
 
 void ESP32Backend::TraceEnd(const char * label, const char * group)
 {
-    LOG_HEAP_INFO(label, group, "Exit");
+    HashValue hashValue = MurmurHash(group);
+    if (IsPermitted(hashValue))
+    {
+        LOG_HEAP_INFO(label, group, "Exit");
+    }
 }
 
 void ESP32Backend::TraceInstant(const char * label, const char * group)
@@ -63,4 +173,3 @@ void ESP32Backend::TraceInstant(const char * label, const char * group)
 } // namespace Insights
 } // namespace Tracing
 } // namespace chip
-// namespace chip
diff --git a/src/tracing/esp32_trace/esp32_tracing.h b/src/tracing/esp32_trace/esp32_tracing.h
@@ -1,11 +1,18 @@
+#include <lib/core/CHIPError.h>
 #include <tracing/backend.h>
 
 #include <memory>
-
 namespace chip {
 namespace Tracing {
 namespace Insights {
 
+// Provide the user the ability to add/remove trace filters.
+namespace ESP32Filter {
+
+CHIP_ERROR AddHashToPermitlist(const char * alabel);
+void RemoveHashFromPermitlist(const char * alabel);
+} // namespace ESP32Filter
+
 /// A Backend that outputs data to chip logging.
 ///
 /// Structured data is formatted as json strings.

diff --git a/src/tracing/esp32_trace/include/matter/tracing/macros_impl.h b/src/tracing/esp32_trace/include/matter/tracing/macros_impl.h
@@ -37,11 +37,11 @@ class Scoped
 public:
     inline Scoped(const char * label, const char * group) : mLabel(label), mGroup(group) { MATTER_TRACE_BEGIN(label, group); }
     inline ~Scoped() { MATTER_TRACE_END(mLabel, mGroup); }
+
 private:
     const char * mLabel;
     const char * mGroup;
 };
-
 } // namespace Insights
 } // namespace Tracing
 } // namespace chip