New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
stats: Do not expand dots of tm_name #10316
Conversation
When an interface with dots is used, per worker stats are nested by the dot-separated-components of the interface due to the usage of OutputStats2Json(). Prevent this by using OutputStats2Json() on a per-thread specific object and setting this object into the threads object using the json_object_set_new() which won't do the dot expansion. This was tested by creating an interface with dots in the name and checking the stats. ip link add name a.b.c type dummy With Suricata 7.0.2, sniffing on the a.b.c interface results in the following worker stats format: "threads": { "W#01-a": { "b": { "c": { "capture": { "kernel_packets": 0, After this fix, the output looks as follows: "threads": { "W#01-a.b.c": { "capture": { "kernel_packets": 0, Ticket: OISF#6732
src/output-json-stats.c
Outdated
/* 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. | ||
assert(strcmp(st->tstats[u].tm_name, st->tstats[u].tm_name) == 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ewh, this was meant to be - let me know whether to open v2 with that and any more suggestions or it can be fixed during the merge. Thanks.
assert(strcmp(st->tstats[u].tm_name, st->tstats[u].tm_name) == 0); | |
assert(strcmp(st->tstats[offset].tm_name, st->tstats[u].tm_name) == 0); |
Use DEBUG_VALIDATE_BUG_ON instead of plain assert()
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #10316 +/- ##
===========================================
+ Coverage 73.31% 82.34% +9.02%
===========================================
Files 895 978 +83
Lines 148215 272035 +123820
===========================================
+ Hits 108666 224010 +115344
- Misses 39549 48025 +8476
Flags with carried forward coverage won't be shown. Click here to find out more. |
Replaced by #10338 |
Make sure these boxes are signed before submitting your Pull Request -- thank you.
https://docs.suricata.io/en/latest/devguide/contributing/contribution-process.html
https://suricata.io/about/contribution-agreement/ (note: this is only required once)
Link to redmine 6732 ticket:
Describe changes:
When an interface with dots is used, per worker stats are nested by the dot-separated-components of the interface due to the usage of OutputStats2Json().
Prevent this by using OutputStats2Json() on a per-thread specific object and setting this object into the threads object using the json_object_set_new() which won't do the dot expansion.
This was tested by creating an interface with dots in the name and checking the stats.
With Suricata 7.0.2, sniffing on the a.b.c interface results in the following worker stats format:
After this fix, the output looks as follows:
Provide values to any of the below to override the defaults.
To use a pull request use a branch name like
pr/N
whereN
is thepull request number.
Alternatively,
SV_BRANCH
may also be a link to anOISF/suricata-verify pull-request.