Skip to content

RFC: config: aggregate mpm/spm options under the detect node v1#14902

Draft
lukashino wants to merge 1 commit intoOISF:mainfrom
lukashino:feat/8324-aggregate-mpm-spm-config-paths-v1-draft
Draft

RFC: config: aggregate mpm/spm options under the detect node v1#14902
lukashino wants to merge 1 commit intoOISF:mainfrom
lukashino:feat/8324-aggregate-mpm-spm-config-paths-v1-draft

Conversation

@lukashino
Copy link
Contributor

As a follow-up on Victor's suggestions in #14838 I present the aggregated MPM/SPM configuration options under the detect node. I picked:

  • mpm-algo
  • detect.sgh-mpm-context
  • sgh-mpm-caching
  • sgh-mpm-caching-path
  • sgh-mpm-caching-max-age
  • spm-algo

Victor only suggested aggregating the caching options, but I thought I would take it a step further with the other MPM/SPM options too.

Redmine ticket https://redmine.openinfosecfoundation.org/issues/8324

The main question is whether this is "too much" or if you see it as a valid improvement. I like it this way, but I understand it might bring some unneeded hurdles when converting from one major version to another. On the other hand, upgrading to a new major version should be done with caution, ideally manually, so I don't see it as a big deal.

Copilot AI review requested due to automatic review settings February 25, 2026 10:31
@lukashino lukashino requested a review from a team as a code owner February 25, 2026 10:31
@lukashino lukashino marked this pull request as draft February 25, 2026 10:32
@lukashino lukashino added typo/doc update No code change : only doc or typo fixes skip qa labels Feb 25, 2026

This comment was marked as off-topic.

@suricata-qa
Copy link

Information: QA skipped due to tag. Set to force a run.

Pipeline = skip

cache:
# Cache MPM contexts to the disk to avoid rule compilation at the startup.
# Cache files are created in the standard library directory.
enabled: yes
Copy link
Member

Choose a reason for hiding this comment

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

Not really specific to this PR, but is the cache enabled by default even if this is commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, starting in Suricata 8, if the caching folder is available/writeable.

Copy link
Member

Choose a reason for hiding this comment

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

We can comment it out then. This helps us make sure defaults are actually defaults, and not just a default because they are set that way in the configuration file.

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

Labels

skip qa typo/doc update No code change : only doc or typo fixes

Development

Successfully merging this pull request may close these issues.

4 participants