From cadee3ec25a5efec5131d93bb619ffe9901d2712 Mon Sep 17 00:00:00 2001 From: shripad621git Date: Fri, 22 Sep 2023 17:28:03 +0530 Subject: [PATCH] Addressed review Comments --- examples/lighting-app/esp32/main/main.cpp | 2 +- src/tracing/esp32_trace/BUILD.gn | 1 - src/tracing/esp32_trace/esp32_tracing.cpp | 115 +++++++++++++++++- src/tracing/esp32_trace/esp32_tracing.h | 9 +- .../include/matter/tracing/macros_impl.h | 2 +- 5 files changed, 122 insertions(+), 7 deletions(-) diff --git a/examples/lighting-app/esp32/main/main.cpp b/examples/lighting-app/esp32/main/main.cpp index 5957a683589c26..adb2bafd84f577 100644 --- a/examples/lighting-app/esp32/main/main.cpp +++ b/examples/lighting-app/esp32/main/main.cpp @@ -41,7 +41,7 @@ #if CONFIG_ENABLE_ESP_INSIGHTS_SYSTEM_STATS #include -#define START_TIMEOUT_MS 600000 +#define START_TIMEOUT_MS 60000 #endif #if CONFIG_ENABLE_ESP32_FACTORY_DATA_PROVIDER diff --git a/src/tracing/esp32_trace/BUILD.gn b/src/tracing/esp32_trace/BUILD.gn index 9b1551732acd5a..7548894ab072ce 100644 --- 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 index 414cdd83edcc0d..24a7166dde04eb 100644 --- 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 #include #include #include @@ -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 index 9857eb111b2c3d..7fe783d817d8d8 100644 --- a/src/tracing/esp32_trace/esp32_tracing.h +++ b/src/tracing/esp32_trace/esp32_tracing.h @@ -1,11 +1,18 @@ +#include #include #include - 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 index f5217d1d54515a..4d8a8a2a214525 100644 --- 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