datasets: add cidr dataset type - #16085
Closed
jlucovsky wants to merge 2 commits into
Closed
Conversation
Backed by a pair of radix trees (SCRadix4Tree for IPv4, SCRadix6Tree for IPv6). Address data comes from the ip.src or ip.dst sticky buffer -- 4 bytes for IPv4, 16 for IPv6. All four commands are supported: isset/isnotset: best-match lookup, so a /16 in the dataset matches any host in that block. set: adds the matched address as a /32 (IPv4) or /128 (IPv6). With mask, the address is masked to the specified prefix length first, so the dataset accumulates /N netblocks rather than individual hosts. unset: removes the exact host entry, or with mask, removes the /N netblock that a corresponding set would have added. mask accepts a decimal prefix length (24), a hex prefix length (0x18), or an IPv4 bitmask (0xffffff00). It is only valid on set and unset. Prefix length 0 (match-everything) is rejected at rule load. DetectDatasetBufferMatch dispatches CIDR sets directly to CIDRLookupIPv4, CIDRAddIPv4Netblock, and CIDRRemoveIPv4Netblock (and the IPv6 counterparts) so the per-packet path does not build a temporary 5/17-byte encoding. The generic SCDatasetAdd/DatasetLookup /DatasetRemove entry points still accept the encoding for callers outside the hot path. Concurrency: CIDRLookupIPv4 and CIDRLookupIPv6 take only the read lock, so a rule using dataset:isset on ip.src or ip.dst scales across workers. Add and remove use a rdlock-first pattern -- probe under the read lock, and take the write lock only if the entry needs to change. Under a spoofed-source flood driving dataset:set on ip.src, addresses already present short-circuit on the rdlock without ever contending the writer. The per-family byte counter is stored atomically so a future stats consumer can sample it without contending the lock either. memcap is enforced per family and is approximate. Each entry counts as ~128 bytes (IPv4) or ~192 bytes (IPv6); the tree's internal split nodes are not counted, so real memory use will be somewhat higher than the tracked total. Once the tracked total for a family exceeds memcap, further set operations for that family are rejected and one warning is logged per family. Restrictions: only load is supported; save and state are rejected at rule-load time. datarep is rejected at rule load in detect-datarep, in SCDatasetAddwRep, and in DatasetLookupwRep. dataset-dump is a no-op for CIDR. JSON/NDJSON format is rejected at DatajsonGet. Unix socket dataset-add, dataset-remove, dataset-lookup, and dataset-clear are wired through the CIDR helpers. DatasetClear() takes both family write locks for the whole operation and reinitializes the trees in place, so the operation is atomic from a reader's point of view and the CIDRType struct and embedded locks are never freed while workers hold a reference. Lua: switch from DatasetFind(name, DATASET_TYPE_STRING) to DatasetSearchByName(name) so Lua scripts can retrieve a CIDR set via dataset.get(), and route dataset:add() through DatasetAddCIDRString when the set type is CIDR so the Lua string is parsed as a CIDR value rather than pushed as raw bytes. Rust: add DSCIDR to the dataset type enum and process_cidr_set() so the Rust dataset loader can parse CIDR entries from a load file. The "reputation not supported for CIDR" warning is emitted once per file. Also fixes a pre-existing bug in DetectDatasetSetup where the parse return-code check only caught the value 0, not negative error codes, so any parse-time -1 return (the new mask validation, and older paths for duplicate save, bad type, oversize keys) silently loaded rules with zeroed defaults. Docs updated: type description, file format, example rules, mask option, memcap semantics. Issue: 8124
Takes one write lock per address family for the whole batch, so an
admin import of N CIDR entries drops from N wrlock acquisitions to
at most 2 (one per family). The existing dataset-add is one wrlock
acquisition per entry; 10k prefixes means 10k wrlock cycles, each
blocking the packet-path rdlock for one tree walk.
The new command takes a JSON array of values instead of a single
data argument:
{"command": "dataset-add-batch",
"arguments": {"setname": "blocklist", "settype": "cidr",
"values": ["10.0.0.0/8", "192.168.0.0/16", ...]}}
The reply reports how each value was handled:
{"return": "OK",
"message": {"added": N, "existed": N, "failed": N,
"rejected_memcap": N}}
rejected_memcap is only present for CIDR since it's the only type
whose memcap can be exhausted mid-batch on this path. The batch is
not atomic: a malformed value or a memcap rejection is counted in its
own bucket while the rest of the batch proceeds.
DatasetAddCIDRBatch parses each input into per-family buckets without
any lock held, then takes the wrlock on each non-empty family and
walks the bucket in one hold. The rdlock-first optimization in the
single-add path is dropped inside the batch: the writer already holds
the lock, so a read-side probe would be wasted work.
For non-CIDR dataset types the handler falls back to looping through
DatasetAddSerialized. Hash-based sets already use per-bucket locks,
so batching them doesn't consolidate locks; the win is one
unix-socket round trip instead of N.
Trade-off called out in the docs: for CIDR the single continuous
wrlock hold is worse tail latency for the packet path during a bulk
import than the current per-entry pattern would be, but the total
blocked-reader time is smaller. Schedule bulk imports during quiet
windows.
Issue: 8124
jlucovsky
requested review from
jasonish,
jufajardini and
victorjulien
as code owners
August 23, 2026 14:01
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #16085 +/- ##
==========================================
- Coverage 83.05% 82.92% -0.14%
==========================================
Files 1004 1006 +2
Lines 277359 278007 +648
==========================================
+ Hits 230370 230535 +165
- Misses 46989 47472 +483
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Information: QA ran without warnings. Pipeline = 33247 |
Contributor
Author
|
Continued in #16090 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
type cidrdataset backed by radix trees for IPv4 and IPv6 network ranges, with longest-prefix-match lookups.with
rfc1918.lstcontaining10.0.0.0/8,172.16.0.0/12,192.168.0.0/16, and any packet whoseip.srcfalls under one of those prefixes matches. Same shape forip.dst. All four dataset commands are supported:isset,isnotset,set,unset.setandunsetaccept an optionalmaskin decimal (mask 24), hex prefix (mask 0x18), or IPv4 bitmask (mask 0xffffff00) form so rules can accumulate/Nnetblocks instead of individual hosts.Link to ticket: https://redmine.openinfosecfoundation.org/issues/8124
Describe changes:
Storage is one
SCRadix4Treeand oneSCRadix6Tree, each behind anSCRWLock.issetandisnotsettake the read lock only.setprobes under the read lock first and only escalates to the write lock when the entry is new. A steady-state flood of already-seen sources never blocks on writes. A microbenchmark across 16 workers measured 13-14M aggregate lookups/s.Memcap is per family and approximate. Each entry counts as ~128 bytes for IPv4 or ~192 bytes for IPv6; internal split nodes are not counted, so memory can run over.
datasets.rstdocuments the gap. When memcap rejects an add, a per family counter increments and a warning re-logs on the first rejection and every millionth after, so operators see sustained pressure rather than a single lost line.The new unix-socket command
dataset-add-batchtakes the write lock once per family for the whole batch. A 10k-entry admin import drops from 10k wrlock acquisitions to at most 2. Hash-based types benefit too, though the win is smaller since they already lock per bucket.SV_REPO=
SV_BRANCH=OISF/suricata-verify#3304
SU_REPO=
SU_BRANCH=