Feat/12168 hugepages v5 - #64
Merged
Merged
Conversation
hugepages_allocated() previously only checked that *some* hugepages were free (HugePages_Free != 0). If a user increased --suricata-hugepages on a machine that already had hugepages mounted, the new allocation request was silently ignored. Add _parse_size_to_bytes() helper and compare the currently allocated hugepage memory (HugePages_Total * Hugepagesize from /proc/meminfo) against the requested amount, re-running dpdk-hugepages.py --setup only when the mounted amount is lower. Also validate --suricata-hugepages in pytest_configure and raise a clean pytest.UsageError on invalid input (e.g. 6X or abc) instead of a raw ValueError traceback from the session fixture. docs: document hugepages re-allocation behavior Update README.md (note after DEFAULT_HUGEPAGES, binary-search setup step) and pytest_start.sh -sh help text to describe that hugepages are re-allocated when the currently mounted amount is lower than the requested --suricata-hugepages value. hugepages: simplify allocation check and robust size parsing - Read /proc/meminfo stdout directly instead of writing to a temp file and reading it back with a second cat command. - Parse size strings loosely (any trailing unit chars) and validate the suffix against the multiplier table, so the space-separated form from /proc/meminfo (e.g. '2048 kB') is accepted. - Catch KeyError from the deferred suffix validation in _validate_hugepages_option. rename this commit conftest: try dpdk-hugepages --reserve as last-ditch effort If dpdk-hugepages.py --setup fails, attempt --reserve before giving up so hugepages can still be pinned. --reserve still runs after a successful --setup so the pages are actually reserved for Suricata. hugepages: account for double allocation when checking; lower default descriptors docs: simplify hugepages help text; restore default rx/tx descriptors docs: clarify hugepages re-allocation behavior
- match unit with [^\s]* instead of (.+)? - use multipliers.get() with a None check - drop the --reserve last-ditch fallback (--setup includes it) - parse Hugepagesize unit via the size parser instead of assuming kB
There was a problem hiding this comment.
🟡 Changes recommended
The updated --suricata-hugepages option has a default/type mismatch and the allocation command likely passes an incompatible byte count to dpdk-hugepages.py.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR refines hugepage handling for Suricata test runs by introducing a size-to-bytes parser, improving the hugepage allocation sufficiency check, and documenting the allocation behavior.
Changes:
- Added
_parse_size_to_bytesand wired it into--suricata-hugepages. - Updated
hugepages_allocated()to compare allocated hugepage memory (bytes) against the requested amount. - Improved hugepage allocation error handling/logging and documented the “only increases” behavior in the README.
File summaries
| File | Description |
|---|---|
| README.md | Documents how DEFAULT_HUGEPAGES / --suricata-hugepages affects hugepage allocation and that allocation only increases. |
| conftest.py | Adds size parsing and updates hugepage detection/allocation logic to use byte-accurate comparisons and better failure handling. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
This pull request improves how hugepage memory allocation is handled and configured for Suricata tests. It introduces a robust mechanism for parsing memory sizes, ensures more accurate hugepage allocation checks, and enhances error handling and documentation.
Improvements to hugepage allocation and configuration:
_parse_size_to_bytesfunction inconftest.pyto robustly parse memory size strings (e.g.,6G,512M,2048 kB) into bytes, supporting various suffixes and formats.--suricata-hugepagespytest option to use the new parser, allowing users to specify hugepage memory in human-friendly formats. The help text was updated to clarify behavior.hugepages_allocatedfunction to compare the actually allocated hugepage memory in bytes against the requested amount, ensuring the allocation is sufficient and not just nonzero.check_hugepagesto log critical failures during allocation attempts and allow tests to proceed with a warning if allocation fails, rather than aborting.Documentation updates:
README.mdexplaining howDEFAULT_HUGEPAGESworks and how hugepage allocation is managed, including the fact that allocation only ever increases and never decreases.Changes from v4 - Last 2 commits