Bug #9244
openthreshold/detection_filter track by_src|by_dst|by_both: global per-table free-list mutex in the entry-creation path causes severe throughput collapse under high source/destination address cardinality
Description
We observed a severe throughput regression upgrading a packet-processing sensor from
Suricata 7.0.16 to 8.0.6: at the same offered load, throughput dropped from ~35 Gbps
with 0% packet loss to ~16 Gbps with ~54% packet loss. Isolation testing identified a
single signature as the trigger: a noalert signature using
`threshold: type limit, track by_src, count 1, seconds <N>` to tag HTTP requests with a
curl User-Agent (25% of the offered test traffic). Removing the threshold keyword, or
changing `track by_src` to `track by_flow`, fully restored throughput. Tuning
detect.thresholds.hash-size and detect.thresholds.memcap in suricata.yaml had no
measurable effect.
Test traffic is generated with TRex using a sequential client address pool of
ip_range=["16.0.0.1","16.0.255.255"] (a full /16, 65536 distinct source addresses),
so the signature above is tracked against ~65k distinct by_src keys.
Root cause analysis (verified against the pristine suricata-8.0.6 and suricata-8.0.7
tags):
1. Suricata 8.0 replaced the old per-Host/per-IPPair storage used for
track by_src/by_dst/by_both (Suricata 7.0.x, src/detect-engine-threshold.c via
HostStorageRegister/IPPairStorageRegister) with one dedicated generic hash table
(THash) for all threshold/detection_filter tracking: a single static
`struct Thresholds ctx` (src/detect-engine-threshold.c:59 in 8.0.6, :67 in 8.0.7)
wrapping one `THashTableContext`.
2. Lookups for an existing tracked key are correctly sharded: the hash/bucket key
(ThresholdEntryHash, 8.0.6 tag src/detect-engine-threshold.c:143-157) includes the
source/dest address for by_src/by_dst/by_both, and THashGetFromHash
(src/util-thash.c:637-739, identical in both tags) only locks the target row
(HRLOCK_LOCK) briefly, then the individual entry's own mutex. Two different source
IPs matching the same signature do not contend for the same lock once their
entries exist.
3. However, creating a new entry — which happens on every first-ever sighting of a
given (sid, gid, rev, track, address) combination, and again every time a
previously-tracked entry expires and is recreated — goes through
THashDataGetNew() (src/util-thash.c:571-630), which first dequeues from
ctx->spare_q: a single shared free-list queue, one per THashTableContext (i.e. one
for the entire thresholds subsystem, process-wide, shared across every tracked
signature and every worker thread), protected by HQLOCK_LOCK (src/util-thash.c:89,
118), which compiles to a real pthread mutex (HQLOCK_MUTEX is defined,
src/util-thash.h:61,76-80 — not a spinlock).
4. This single mutex is on the entry-creation path unconditionally — it is not
bounded or sharded by detect.thresholds.hash-size or detect.thresholds.memcap,
which only affect bucket count and total table capacity before eviction. This
explains why tuning those settings had no effect on the regression.
5. A thread-local decision cache in front of this (CheckCache/SetupCache,
src/detect-engine-threshold.c, called from PacketAlertThreshold for
type limit/type both, IPv4, by_src/by_dst) is meant to let a worker thread skip
the locked table for repeat packets within a window. Its capacity determines how
much traffic actually reaches the mutex in step 3:
- In 8.0.6, this cache is an unbounded thread_local HashTable (no entry cap;
this is OISF ticket #8956, "decision cache can grow without bound and
terminate Suricata").
- In 8.0.7 (commit 0282a4fd81, ticket #8956), it is capped at
detect.thresholds.cache.max-entries, default and minimum 256 entries per
thread (max 1048576), evicting the earliest-expiring entry on overflow.
With traffic cycling through 65536 distinct source addresses for one tracked
signature, a 256-entry-per-thread cap guarantees continuous eviction and
cache-miss cycling, pushing a large fraction of matching packets onto the
single global mutex in step 3 — on top of whatever the unbounded cache in 8.0.6
already sent through it during cache warm-up and per-window expiry.
6. util-thash.c, util-thash.h and host.c are byte-identical between the
suricata-8.0.6 and suricata-8.0.7 tags — i.e. the global free-list mutex
identified in step 3 is unchanged between the two releases, and the regression
is not fixed by upgrading to 8.0.7. The only threshold-related changes between
the tags are the cache-bounding commit above (which, for this traffic shape,
trades a crash/memory-safety bug for more frequent hits on the global mutex)
and an unrelated hash-seed consistency fix (commit c27868a8c5, ticket #8617/#8618).
Suggested fix direction: shard or eliminate the single process-wide free-list
(ctx->spare_q / HQLOCK) on the entry-creation path in util-thash.c, e.g. with
per-row or per-thread free lists, mirroring the sharding already present on the
lookup path. As a short-term mitigation, detect.thresholds.cache.max-entries
(8.0.7+) could be raised well above the expected number of concurrently active
distinct tracked keys per worker thread, though this does not address the
underlying single-lock design.
How to reproduce:
- Suricata 8.0.6 or 8.0.7, a ruleset containing a signature with
threshold: type limit, track by_src, count 1, seconds <N> (or by_dst/by_both)
matching a reasonable fraction of traffic.
- Offered traffic at multi-Gbps line rate with source (or dest) addresses drawn
from a large pool (tens of thousands of distinct addresses or more) so that the
number of concurrently tracked by_src entries per worker thread exceeds the
decision cache's capacity.
- Observe throughput/packet loss with the signature's threshold keyword present
vs. removed, and with track by_src vs. track by_flow, to isolate the effect.
Environment:
- Suricata 7.0.16 (baseline, unaffected) vs 8.0.6 / 8.0.7 (affected)
- Multi-core/multi-worker-thread run mode (workers), traffic at ~35 Gbps offered
- Traffic generator: TRex, sequential client address pool of 65536 addresses
No data to display