From 69197995e7c4df5338d0a559caccf8cda5fd2cd3 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Sat, 17 Feb 2024 18:19:27 +0100 Subject: [PATCH 1/2] stats: Fix non-worker stats missing Commit b8b8aa69b49ac0dd222446c28d00a50f9fd7d716 used tm_name of the first StatsRecord of a thread block as key for the "threads" object. However, depending on the type of thread, tm_name can be NULL and would result in no entry being included for that thread at all. This caused non-worker metrics to vanish from the "threads" object in the dump-counters output. This patch fixes this by remembering the first occurrence of a valid tm_name within the per-thread block and adds another unittest to cover this scenario. (cherry picked from commit f17204191d3bb2201e6b6b1c4cf2e7a96148e8cd) --- src/output-json-stats.c | 34 +++++++++++------ src/tests/output-json-stats.c | 69 ++++++++++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 14 deletions(-) diff --git a/src/output-json-stats.c b/src/output-json-stats.c index 33f98afa2dda..7cc880727dce 100644 --- a/src/output-json-stats.c +++ b/src/output-json-stats.c @@ -265,23 +265,30 @@ json_t *StatsToJSON(const StatsTable *st, uint8_t flags) uint32_t x; for (x = 0; x < st->ntstats; x++) { uint32_t offset = x * st->nstats; - - // Stats for for this thread. - json_t *thread = json_object(); - if (unlikely(thread == NULL)) { - json_decref(js_stats); - json_decref(threads); - return NULL; - } + const char *tm_name = NULL; + json_t *thread = NULL; /* for each counter */ for (u = offset; u < (offset + st->nstats); u++) { if (st->tstats[u].name == NULL) continue; - // Seems this holds, but assert in debug builds. - DEBUG_VALIDATE_BUG_ON( - strcmp(st->tstats[offset].tm_name, st->tstats[u].tm_name) != 0); + DEBUG_VALIDATE_BUG_ON(st->tstats[u].tm_name == NULL); + + if (tm_name == NULL) { + // First time we see a set tm_name. Remember it + // and allocate the stats object for this thread. + tm_name = st->tstats[u].tm_name; + thread = json_object(); + if (unlikely(thread == NULL)) { + json_decref(js_stats); + json_decref(threads); + return NULL; + } + } else { + DEBUG_VALIDATE_BUG_ON(strcmp(tm_name, st->tstats[u].tm_name) != 0); + DEBUG_VALIDATE_BUG_ON(thread == NULL); + } json_t *js_type = NULL; const char *stat_name = st->tstats[u].short_name; @@ -303,7 +310,10 @@ json_t *StatsToJSON(const StatsTable *st, uint8_t flags) } } } - json_object_set_new(threads, st->tstats[offset].tm_name, thread); + if (tm_name != NULL) { + DEBUG_VALIDATE_BUG_ON(thread == NULL); + json_object_set_new(threads, tm_name, thread); + } } json_object_set_new(js_stats, "threads", threads); } diff --git a/src/tests/output-json-stats.c b/src/tests/output-json-stats.c index ac1336eff898..332a819fee86 100644 --- a/src/tests/output-json-stats.c +++ b/src/tests/output-json-stats.c @@ -23,7 +23,7 @@ static int OutputJsonStatsTest01(void) { - StatsRecord global_records[] = { { 0 }, { 0 } }; + StatsRecord total_records[] = { { 0 }, { 0 } }; StatsRecord thread_records[2]; thread_records[0].name = "capture.kernel_packets"; thread_records[0].short_name = "kernel_packets"; @@ -36,7 +36,7 @@ static int OutputJsonStatsTest01(void) StatsTable table = { .nstats = 2, - .stats = &global_records[0], + .stats = &total_records[0], .ntstats = 1, .tstats = &thread_records[0], }; @@ -64,7 +64,72 @@ static int OutputJsonStatsTest01(void) return cmp_result == 0; } +static int OutputJsonStatsTest02(void) +{ + StatsRecord total_records[4] = { 0 }; + StatsRecord thread_records[8] = { 0 }; + + // Totals + total_records[0].name = "tcp.syn"; + total_records[0].short_name = "syn"; + total_records[0].tm_name = NULL; + total_records[0].value = 1234; + + // Worker + // thread_records[0] is a global counter + thread_records[1].name = "capture.kernel_packets"; + thread_records[1].short_name = "kernel_packets"; + thread_records[1].tm_name = "W#01-bond0.30"; + thread_records[1].value = 42; + thread_records[2].name = "capture.kernel_drops"; + thread_records[2].short_name = "kernel_drops"; + thread_records[2].tm_name = "W#01-bond0.30"; + thread_records[2].value = 4711; + // thread_records[3] is a FM specific counter + + // Flow manager + // thread_records[4] is a global counter + // thread_records[5] is a worker specific counter + // thread_records[6] is a worker specific counter + thread_records[7].name = "flow.mgr.full_hash_passes"; + thread_records[7].short_name = "full_hash_passes"; + thread_records[7].tm_name = "FM#01"; + thread_records[7].value = 10; + + StatsTable table = { + .nstats = 4, + .stats = &total_records[0], + .ntstats = 2, + .tstats = &thread_records[0], + }; + + json_t *r = StatsToJSON(&table, JSON_STATS_TOTALS | JSON_STATS_THREADS); + if (!r) + return 0; + + // Remove variable content + json_object_del(r, "uptime"); + + char *serialized = json_dumps(r, 0); + + // Cheesy comparison + const char *expected = "{\"tcp\": {\"syn\": 1234}, \"threads\": {\"W#01-bond0.30\": " + "{\"capture\": {\"kernel_packets\": " + "42, \"kernel_drops\": 4711}}, \"FM#01\": {\"flow\": {\"mgr\": " + "{\"full_hash_passes\": 10}}}}}"; + + int cmp_result = strcmp(expected, serialized); + if (cmp_result != 0) + printf("unexpected result\nexpected=%s\ngot=%s\n", expected, serialized); + + free(serialized); + json_decref(r); + + return cmp_result == 0; +} + void OutputJsonStatsRegisterTests(void) { UtRegisterTest("OutputJsonStatsTest01", OutputJsonStatsTest01); + UtRegisterTest("OutputJsonStatsTest02", OutputJsonStatsTest02); } From a25683b928c9f26718d36cbbd884f7cc9f25fdba Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Tue, 20 Feb 2024 12:50:40 +0100 Subject: [PATCH 2/2] schema: Add stats.capture and in_iface properties New suricata-verify test listens on loopback interface, resulting in the capture and in_iface fields in the stats and event objects. (cherry picked from commit f9cf87a003d273ec175590e2ffec053d2672af95) --- etc/schema.json | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/etc/schema.json b/etc/schema.json index e01c6828c3c8..960de2136fa4 100644 --- a/etc/schema.json +++ b/etc/schema.json @@ -51,6 +51,9 @@ "icmp_type": { "type": "integer" }, + "in_iface": { + "type": "string" + }, "log_level": { "type": "string" }, @@ -3697,6 +3700,20 @@ "uptime": { "type": "integer" }, + "capture": { + "type": "object", + "properties": { + "kernel_packets": { + "type": "integer" + }, + "kernel_drops": { + "type": "integer" + }, + "kernel_ifdrops": { + "type": "integer" + } + } + }, "memcap_pressure": { "type": "integer" },