Skip to content

Feat/12168 hugepages v2 - #35

Open
Davihan11 wants to merge 2 commits into
CESNET:mainfrom
Davihan11:feat/12168-hugepages-v2
Open

Feat/12168 hugepages v2#35
Davihan11 wants to merge 2 commits into
CESNET:mainfrom
Davihan11:feat/12168-hugepages-v2

Conversation

@Davihan11

Copy link
Copy Markdown
Collaborator

Implements this.

Fixed a error propagation issue from v1.

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.
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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refines hugepages handling in the Suricata Test Suite by making allocation decisions depend on the requested hugepage memory size (not just whether any hugepages are present) and by validating the --suricata-hugepages CLI option early for clearer error reporting.

Changes:

  • Add upfront validation for --suricata-hugepages and a helper to parse size strings (e.g., 6G, 512M) into bytes.
  • Update hugepage allocation detection to compare mounted hugepage memory (HugePages_Total × Hugepagesize) against the requested amount.
  • Document the new behavior in the README and in pytest_start.sh help output.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
README.md Documents size-aware hugepages allocation behavior and clarifies setup step behavior.
pytest_start.sh Updates CLI help text to reflect re-allocation behavior when mounted hugepages are below the requested amount.
conftest.py Implements size parsing/validation and updates hugepage allocation gating logic to be requested-size aware.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Davihan11

Copy link
Copy Markdown
Collaborator Author

Good to see copilot be satisfied completely for once!

@Davihan11 Davihan11 added the enhancement New feature or request label Aug 11, 2026
Comment thread conftest.py


def pytest_configure(config):
_validate_hugepages_option(config)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shouldn't validation happen in the parser?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It should not be validated in the parser, because we get a string. This string gets used in check_hugepages(). I could make the type=_parse_size_to_bytes . I could change the code further down the range to take int instead of string though if needed.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I could make the type=_parse_size_to_bytes

That is exactly what I meant. Is there any benefit to how it is done right now?

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants