hyperscan: add caching mechanism for hyperscan contexts v11 #12658

lukashino · 2025-02-22T19:05:39Z

Followup of #12394

Cache Hyperscan serialized databases to disk to prevent compilation of the same databases when Suricata is run again with the same ruleset.
Hyperscan binary files are stored per rulegroup in the designated folder, by default in the cached library folder.
Since caching is per signature group heads, some chunk of the ruleset can change and it still can reuse part of
the unchanged signature groups.

Loading fresh ET Open ruleset: 19 seconds
Loading cached ET Open ruleset: 07 seconds

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

Describe changes:
v11:

adjusted install permissions to 770 from 660 - couldn't write by non-root users
adjusted HS pattern database hashing algorithm to include sids and pattern ids in the hash calculation - while not necessary, MPM DBs could collide when the same patterns were linked to different SIDs (different PDs == different MPMs even though the MPM patterns can be the same).

v10:

the default path to the caching directory is not HS tied and is code-embedded through a macro
minor user/code adjustments
commit cleanups
rebased

v9:

rebase
split HS MPM codebase into multiple files - core functions, Hyperscan MPM functions, HS caching functions
recommitted the code
minor tweaks from the previous PR, docs update
made DetectEngineMpmCachingEnabled private (possible also for DetectEngineMpmCachingGetPath), but checkout v8 thread - decision needed
some discussion threads in v8 can/should be brought up again and be decided

v7: (v6 was private)

fix docs and add ticket number to the commit
fix privilege drop issue, files are created after privilege drop, tested also rule reload - it worked fine
refactor the util-mpm-hs code, primarily prepare function
rebase

v5:

rebased
commit message update
docs update

v4:

rebased
changed the default caching directory to somewhere /var/lib/suricata/cache/hs
custom cache directory path option added
docs added
the default settings changed - enabled on the config generation, disabled when the option is not present in the config

v3

rebased
MPM caching is still left on by default.

v2

improved styling to follow Suricata code styleguide
increased cache file name length from 10 to 20 characters
cache file name is a hash of the patterns - now only HS relevant fields are hashed - as long as the group of patterns itself is not changed then it is reused
minor refactors
added a safe variant of littlehash2 function
added suricata.yaml option to enable/disable caching
changed the storage location to the configured logging directory

v1

initial work to cache and load Hyperscan databases from the disk

This variant of hashlittle2() ensures that it avoids accesses beyond the last byte of the string, which will cause warnings from tools like Valgrind or Address Sanitizer.

…functions

Cache Hyperscan serialized databases to disk to prevent compilation of the same databases when Suricata is run again with the same ruleset. Hyperscan binary files are stored per rulegroup in the designated folder, by default in the cached library folder. Since caching is per signature group heads, some chunk of the ruleset can change and it still can reuse part of the unchanged signature groups. Loading *fresh* ET Open ruleset: 19 seconds Loading *cached* ET Open ruleset: 07 seconds Ticket: 7170

codecov · 2025-02-22T19:36:56Z

Codecov Report

Attention: Patch coverage is 30.53691% with 414 lines in your changes missing coverage. Please review.

Project coverage is 80.65%. Comparing base (3bc2a14) to head (aaa1233).

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #12658      +/-   ##
==========================================
- Coverage   80.77%   80.65%   -0.13%     
==========================================
  Files         932      934       +2     
  Lines      259517   259935     +418     
==========================================
+ Hits       209629   209641      +12     
- Misses      49888    50294     +406

Flag	Coverage Δ
fuzzcorpus	`56.93% <10.24%> (-0.06%)`	⬇️
livemode	`19.35% <11.21%> (-0.03%)`	⬇️
pcap	`44.06% <10.73%> (-0.10%)`	⬇️
suricata-verify	`63.44% <11.70%> (-0.07%)`	⬇️
unittests	`58.24% <29.69%> (-0.08%)`	⬇️

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

lukashino · 2025-02-22T19:47:01Z

@victorjulien wrote in #12394 :

How will this behave around rule reloads and testing? The recommended way with suricata-update is to run suri on the new rules with -T before issuing the reload to the running suricata. What behavior will there be around caching here? Also if the -T run fails, what happens?

The cache files are created by running the suricata -T. Then on rule reload the cache files are picked up.
I am not sure how Suricata tests the ruleset but if the rules are incorrect then it does not even reach the MPM compilation stage - so no cache files on an error.
The save-to-disk caching part also happens only after all patterns are compiled.

suricata-qa · 2025-02-22T23:45:03Z

Information: QA ran without warnings.

Pipeline 24865

victorjulien · 2025-02-25T18:23:29Z

Side note: this PR broke on one of my machines, but it was due to its hyperscan version being very old (4.6). We don't seem to check, or the check doesn't work well.

jasonish · 2025-02-25T19:25:41Z

Side note: this PR broke on one of my machines, but it was due to its hyperscan version being very old (4.6). We don't seem to check, or the check doesn't work well.

Looks like there is no check.

victorjulien

See inline.

Additionally, I think it would be good to count the successfully loaded caches, and output that number. Right now I don't know from the output if the cache works.

victorjulien · 2025-02-25T18:47:16Z

doc/userguide/performance/hyperscan.rst

+  # Cache MPM contexts to the disk to avoid rule compilation at the startup.
+  # Cache files are created in the standard library directory.
+  sgh-mpm-caching: yes


it's not clear that this is under detect, so

detect: sgh-mpm-caching....

victorjulien · 2025-02-25T19:03:26Z

src/util-mpm-hs-cache.c

+    FILE *db_cache_out = fopen(hash_file_static, "w");
+    if (!db_cache_out) {
+        if (!notified) {
+            SCLogWarning("Failed to create Hyperscan cache file, make sure the folder exist and is "


This needs to also have the strerror(errno) output as it's very unclear to me why I get this failure

Feb 25 19:57:17 c2758 suricata[231574]: Warning: mpm-hs-cache: Failed to create Hyperscan cache file, make sure the folder exist and is writable or adjust sgh-mpm-caching-path setting (/var/lib/suricata/cache/sgh/ids/00445770507689860353_v1.hs) [HSSaveCache:util-mpm-hs-cache.c:197]

The path exists and is owned by the same user as the process.

Ok, it looks like it was landlock getting in the way. Wonder if we should automatically add this path to the read and write allow list.

victorjulien · 2025-02-25T19:35:32Z

src/app-layer-detect-proto.c

@@ -1297,7 +1297,7 @@ static int AppLayerProtoDetectPMPrepareMpm(AppLayerProtoDetectPMCtx *ctx)
    int ret = 0;
    MpmCtx *mpm_ctx = &ctx->mpm_ctx;

-    if (mpm_table[mpm_ctx->mpm_type].Prepare(mpm_ctx) < 0)
+    if (mpm_table[mpm_ctx->mpm_type].Prepare(mpm_ctx, false) < 0)


I'm not too excited about this bool everywhere, that isn't really clear. Wonder if it should be a flags field with named flags, like MPM_PREPARE_FLAG_NONE, MPM_PREPARE_FLAG_USE_CACHE, etc or similar.

victorjulien · 2025-02-25T19:39:08Z

src/util-mpm.h

@@ -162,7 +162,8 @@ typedef struct MpmTableElmt_ {
     */
    int  (*AddPattern)(struct MpmCtx_ *, uint8_t *, uint16_t, uint16_t, uint16_t, uint32_t, SigIntId, uint8_t);
    int  (*AddPatternNocase)(struct MpmCtx_ *, uint8_t *, uint16_t, uint16_t, uint16_t, uint32_t, SigIntId, uint8_t);
-    int  (*Prepare)(struct MpmCtx_ *);
+    int (*Prepare)(struct MpmCtx_ *, bool);
+    int (*CacheRuleset)(void);


it's odd that this doesn't take any args for a ruleset or detect engine, for which to write the cache

lukashino · 2025-02-28T10:01:25Z

Additionally, I think it would be good to count the successfully loaded caches, and output that number. Right now I don't know from the output if the cache works.

There is https://github.com/OISF/suricata/pull/12658/files#diff-ccc788de76082a7cbf738db043a78c0d6332af6055a1ed7311f434820f714d9dR838

Should I make it a "Notice" level?

Lukas Sismis added 7 commits February 22, 2025 12:34

hashlittle: add a safe variant of hashlittle2 function

9e314ec

This variant of hashlittle2() ensures that it avoids accesses beyond the last byte of the string, which will cause warnings from tools like Valgrind or Address Sanitizer.

util-hash: add iterator for hash function

777b32b

util-path: remove dead code

6205fa2

detect-engine: remove commented out code

db2b588

util-mpm-hs: refactor Hyperscan Pattern DB initialization to smaller …

4f04883

…functions

util-mpm: prepare MPM codebase for ruleset caching

e3a268f

lukashino requested review from jufajardini, victorjulien and a team as code owners February 22, 2025 19:05

lukashino mentioned this pull request Feb 22, 2025

hyperscan: add caching mechanism for hyperscan contexts v10 #12394

Closed

victorjulien requested changes Feb 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hyperscan: add caching mechanism for hyperscan contexts v11 #12658

hyperscan: add caching mechanism for hyperscan contexts v11 #12658

lukashino commented Feb 22, 2025 •

edited

Loading

codecov bot commented Feb 22, 2025 •

edited

Loading

lukashino commented Feb 22, 2025

suricata-qa commented Feb 22, 2025

victorjulien commented Feb 25, 2025

jasonish commented Feb 25, 2025

victorjulien left a comment

victorjulien Feb 25, 2025

victorjulien Feb 25, 2025

victorjulien Feb 25, 2025

victorjulien Feb 25, 2025

victorjulien Feb 25, 2025

victorjulien Feb 25, 2025

lukashino commented Feb 28, 2025

hyperscan: add caching mechanism for hyperscan contexts v11 #12658

Are you sure you want to change the base?

hyperscan: add caching mechanism for hyperscan contexts v11 #12658

Conversation

lukashino commented Feb 22, 2025 • edited Loading

codecov bot commented Feb 22, 2025 • edited Loading

Codecov Report

lukashino commented Feb 22, 2025

suricata-qa commented Feb 22, 2025

victorjulien commented Feb 25, 2025

jasonish commented Feb 25, 2025

victorjulien left a comment

Choose a reason for hiding this comment

victorjulien Feb 25, 2025

Choose a reason for hiding this comment

victorjulien Feb 25, 2025

Choose a reason for hiding this comment

victorjulien Feb 25, 2025

Choose a reason for hiding this comment

victorjulien Feb 25, 2025

Choose a reason for hiding this comment

victorjulien Feb 25, 2025

Choose a reason for hiding this comment

victorjulien Feb 25, 2025

Choose a reason for hiding this comment

lukashino commented Feb 28, 2025

lukashino commented Feb 22, 2025 •

edited

Loading

codecov bot commented Feb 22, 2025 •

edited

Loading