dpdk: add initial unittests for DPDK codebase v12.7#15347
Closed
lukashino wants to merge 9 commits into
Closed
Conversation
added 9 commits
May 9, 2026 12:01
The threads field is uint16_t, so the <= 0 comparison is misleading as the value can never be negative. Change to == 0. Ticket: 6927
Rename affinity public API functions to use a consistent naming convention (e.g. GetAffinityTypeForNameAndIface to AffinityTypeGetByIfaceOrCpuset, UtilAffinityGetAffinedCPUNum to AffinityGetAffinedCPUNum, AffinitySetupLoadFromConfig to AffinityLoadFromConfig, etc.). Also change the return type of AffinityCpusOverlap from uint16_t to bool, remove dead code in that function, and make ResetAffinityForTest public as AffinityReset for unit test use. Ticket: 6927
To better control the values within the variables and to prepare for the follow-up unit tests, the variable was moved into global scope and should be accessed only by functions. This allows reinitialization of the variable value - needed for unit tests. Ticket: 6927
Needed by unit tests to properly reset the device list. Ticket: 6927
Add SKIP() macro and update the test runner to track and report skipped tests separately from passes and failures. Ticket: 6927
Some unit tests work with multiple CPUs. Suricata, in some code paths, depends on the environment on which it runs. As a result, tests requiring e.g. 4 cores failed on 2-core machines. This commit introduces a skip directive for these tests. Change AffinityLoadFromConfig from void to int, returning negative errno on failure. Callers now treat affinity configuration errors as fatal at startup with descriptive error messages. Move the CPU range bounds check (b > max) from an early parse-time error to a post-load validation that returns -ERANGE, allowing the skip directive to handle incompatible environments. Add explicit bounds checks for single CPU values exceeding the available range. Ticket: 6927
Reduce affinity unit test CPU indices to a maximum of 4 cores for broader CI compatibility on machines with fewer CPUs. Also fix assertions in ThreadingAffinityTest12 that were incorrectly checking hiprio_cpu instead of cpu_set. Ticket: 6927
Add unit tests for mempool cache size auto-calculation and for automatic thread distribution across interfaces, including management thread overlap scenarios. Ticket: 6927
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds initial DPDK-focused unit tests and strengthens CPU affinity parsing/error propagation to make affinity/threading failures explicit and tests skippable on unsupported environments.
Changes:
- Introduces a unittest
SKIP()mechanism and updates the unittest runner to track/report skipped tests. - Refactors affinity APIs (naming + error returns) and threads those errors up to runtime initialization.
- Adds DPDK runmode unit tests (thread auto-assignment + mempool cache sizing) and enables DPDK in CI unittest build.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/util-unittest.h | Adds SKIP() macro returning a dedicated skip code. |
| src/util-unittest.c | Treats return codes as pass/skip/fail and reports skipped tests. |
| src/util-ebpf.c | Updates cpuset parsing helper name to affinity-prefixed API. |
| src/util-device.c | Re-initializes device TAILQs after cleanup/finalize to avoid stale heads across tests. |
| src/util-affinity.h | Renames/export changes for affinity APIs and adds error-returning AffinityLoadFromConfig(). |
| src/util-affinity.c | Propagates parse/HW errors via negative errno, adds test init helper, new tests. |
| src/tm-threads.c | Adapts to new affinity helper names. |
| src/runmodes.c | Fatal-errors on invalid affinity config with errno-derived message. |
| src/runmode-unittests.c | Registers affinity tests under new name and adds DPDK tests under HAVE_DPDK. |
| src/runmode-dpdk.h | Exposes DPDK unittest registration when HAVE_DPDK && UNITTESTS. |
| src/runmode-dpdk.c | Adds DPDK unit tests; refactors “auto remaining threads” state handling. |
| .github/workflows/builds.yml | Installs DPDK deps and enables --enable-dpdk for the sanitizer+unittests configure step. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SCReturn; | ||
| } | ||
|
|
||
| static int32_t remaining_auto_cpus = -1; // -1 means not initialized |
Comment on lines
+388
to
+404
| static void AutoRemainingThreadsInit(void) | ||
| { | ||
| if (remaining_auto_cpus == -1) { | ||
| ThreadsAffinityType *wtaf = AffinityTypeGetByIfaceOrCpuset("worker-cpu-set", NULL); | ||
| if (wtaf == NULL) | ||
| FatalError("Worker-cpu-set not listed in the threading section"); | ||
|
|
||
| int32_t total_cpus = AffinityGetAffinedCPUNum(wtaf); | ||
| if (total_cpus == 0) | ||
| FatalError("No worker CPU cores with affinity were configured"); | ||
|
|
||
| int32_t ldevs = LiveGetDeviceCount(); | ||
| if (ldevs == 0) | ||
| FatalError("No devices configured"); | ||
| remaining_auto_cpus = total_cpus % ldevs; | ||
| } | ||
| } |
Comment on lines
+492
to
495
| AutoRemainingThreadsInit(); | ||
| if (AutoRemainingThreadsUsedOne()) { | ||
| iconf->threads++; | ||
| remaining_auto_cpus--; | ||
| } |
Comment on lines
+2090
to
+2101
| if (threading_set_cpu_affinity) { | ||
| ret = AffinityLoadFromConfig(); | ||
| if (ret < 0) { | ||
| DPDKRunmodeSetThreadsDeinit(); | ||
| return ret; | ||
| } | ||
| } | ||
|
|
||
| LiveBuildDeviceListCustom("unittest-ifaces", "iface"); | ||
| LiveDeviceFinalize(); | ||
| AutoRemainingThreadsInit(); | ||
| return 0; |
Comment on lines
+1963
to
+1966
| int ret = AffinityTestInit(config); | ||
| FAIL_IF_NOT(ret < 0); | ||
|
|
||
| PASS; |
Comment on lines
+1981
to
+1984
| int ret = AffinityTestInit(config); | ||
| FAIL_IF_NOT(ret < 0); | ||
|
|
||
| PASS; |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #15347 +/- ##
==========================================
- Coverage 82.66% 82.63% -0.04%
==========================================
Files 995 995
Lines 271046 271056 +10
==========================================
- Hits 224069 223983 -86
- Misses 46977 47073 +96
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
WARNING:
Pipeline = 31317 |
Contributor
Author
|
next in #15366 |
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.
Follow-up of #15214
Link to ticket: https://redmine.openinfosecfoundation.org/issues/6927
The PR is adding unit tests tailored to verify CPU threading logic and the automatic calculation of the mempool cache of the interface. Tests are skipped if the minimal CPU requirements (CPU count) on the executing machine are not met.
Describe changes:
v12.7:
v12.1 (Victor-greened in v11.4):
v11.4:
v11.3:
v11.2:
v10.1:
v2 (v10):
v1 - PR splitted to sub-PRs: