Skip to content
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

Fix stats for interface with dots v2 #10338

Closed

Conversation

awelzel
Copy link
Contributor

@awelzel awelzel commented Feb 8, 2024

Replaces #10316

Changes since v1:

  • Squash the fixups for CI runs.
  • Add a unit test calling StatsToJSON() verifying the dot of bond0.30 isn't expanded into a nested object anymore.

Make sure these boxes are signed before submitting your Pull Request -- thank you.

Link to redmine: 6732

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.

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,

Provide values to any of the below to override the defaults.

To use a pull request use a branch name like pr/N where N is the
pull request number.

Alternatively, SV_BRANCH may also be a link to an
OISF/suricata-verify pull-request.

SV_REPO=
SV_BRANCH=
SU_REPO=
SU_BRANCH=
LIBHTP_REPO=
LIBHTP_BRANCH=

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
Main purpose is to validate that the 30 of bond0.30 isn't expanded into
a nested object during serialization.
Copy link

codecov bot commented Feb 8, 2024

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (80abc22) 82.32% compared to head (348fc4b) 82.33%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #10338   +/-   ##
=======================================
  Coverage   82.32%   82.33%           
=======================================
  Files         978      978           
  Lines      272142   272183   +41     
=======================================
+ Hits       224053   224091   +38     
- Misses      48089    48092    +3     
Flag Coverage Δ
fuzzcorpus 63.59% <0.00%> (-0.01%) ⬇️
suricata-verify 61.47% <0.00%> (-0.02%) ⬇️
unittests 62.85% <88.63%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Contributor

@jlucovsky jlucovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jlucovsky
Copy link
Contributor

The CI issues have been resolved; please rebase

@awelzel
Copy link
Contributor Author

awelzel commented Feb 11, 2024

Replaced by #10363

@awelzel awelzel closed this Feb 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants