Skip to content

counters: add STATS_TYPE_RATE for time-derivative counters 6410 v1 - #16204

Open
1bnBattuta wants to merge 3 commits into
OISF:mainfrom
1bnBattuta:stats-rate-counters-6410-v1
Open

1bnBattuta wants to merge 3 commits into
OISF:mainfrom
1bnBattuta:stats-rate-counters-6410-v1

Conversation

@1bnBattuta

@1bnBattuta 1bnBattuta commented Sep 9, 2026

Copy link
Copy Markdown

Contribution style:

Our Contribution agreements:

Changes (if applicable):

Link to ticket: https://redmine.openinfosecfoundation.org/issues/6410

Describe changes:
Adding a new counter type for rate values and registering two counters based on it:

  • stats.pps : packets per second
  • stats.bps : bytes per seconds (maybe misleading given that bits par second is standard, but left this way because of the ticket)

The stats management thread computes the rates once per tick (default 8s) and then output paths pick them just like any other stat.

A new type of counters was added because existing types does not support the needed functionality: a global counter computed at output time that is derived from another 'source' counter with a related timestamp. Also, adding rates in the future becomes a matter of registration.

Rate counters are registered in global_counter_ctx because rates are global and are not worker dependent, also calculating rates for each worker thread will lead to averaging of rates.

Why postpone registration for rate counters ?
Rate counters need their source counter's gid to register, but that gid isn't available at StatsInit time: counters_id_hash hasn't been built yet, and no worker has registered the source counter thus registering immediately fails.

Registering early and resolving the source gid lazily at output time doesn't work either. The counter must be in global_counter_ctx.head before StatsThreadSetupPublic sizes pc_array and copy_of_private; appending later forces those arrays (and stats_table.stats) to be resized, and that something I don't want to touch.

Things to flag here: where to register pps and bps ? timestamp source (here it is a dedicated CLOCK_MONOTONIC)
code comments are verbose intentionally, happy reading.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

NOTE: This PR may contain new authors.

@catenacyber

Copy link
Copy Markdown
Contributor

Thanks for your work.

Do we have a suricata-verify test ?

@inashivb as you worked on elephant flows, what do you think about this ?

@1bnBattuta

Copy link
Copy Markdown
Author

You are welcome.
No not yet, the problem is that the counters are computed from CLOCK_MONOTONIC deltas, so their values depend on how long the run actually takes on the host, a strict output diff can't validate this.

I could write a test to check the presence of the fields stats.pps and stats.bps if it is good enough.
For the numeric values, a unit test may be more adapted.

For the moment I prefer waiting for reviews concerning the use of the clock.

@catenacyber

Copy link
Copy Markdown
Contributor

a strict output diff can't validate this.

Can we not have a validation of an approximation ? (suricata-verify can check gt and lt for a numeric value as a recent improvement)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants