dpdk: add initial unittests for DPDK codebase v13.2 by lukashino · Pull Request #15366 · OISF/suricata

lukashino · 2026-05-11T19:02:38Z

Follow-up of #15347

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:
v13.2:

new tests DPDKRunmodeSetThreads7 and 8
YAMLs in DPDKRunmodeSetThreads tests deindented and switched to the threading format (switching from a list to nodes)
minor fixes

v12.7:

added more error propagations -- from lower level functions to higher ones (instead of just silent accepts/fatal errors)
addressed Copilot's remarks about e.g. cpu_set overflow
dropped/replaced the cleanup callback structure, replaced by error handling in an if-call (below: int ret = AffinityLoadFromConfig();)
if a unit test returns anything except 1 or 2, it fails by default
added ThreadingAffinityTest28 and ThreadingAffinityTest29

v12.1 (Victor-greened in v11.4):

Copilot review comments addressed
limited the addition of the number of Github CI builds to 1, to one Fedora build only
the triage of not firing the function warning (unused function) in dpdk: add initial unittests for DPDK codebase v11.4 #14589 (comment)
rebased

v11.4:

reword "skip affinity" commit - explains there is no real behavior change, just the check was moved elsewhere - explain in dpdk: add initial unittests for DPDK codebase v11.3 #14150 (comment)
added have_dpdk guard where reminded
rebased

v11.3:

limit AffinityCPUConfigIsCompatible to Linux only (to exclude Darwin) as it does not contain CPU_EQUAL API
Change SKIP_INCOMPATIBLE_ENVIRONMENT macro to accept an array of deinit callbacks to not leak memory when skipping tests
rebased

v11.2:

rebased
util-affinity tests are now skipped
minor follow-up adjustments in the dpdk unittests, cleaning LiveDevList on the Init as it crashed tests when the previous test failed.

v10.1:

rebased

v2 (v10):

rebased

v1 - PR splitted to sub-PRs:

new unit tests
skip directive for unit tests for incompatible environments

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

Ticket: 6927

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds initial unit tests for DPDK runmode logic and improves CPU affinity parsing/error propagation, including a mechanism to skip tests on hosts that don’t meet minimum CPU requirements.

Changes:

Introduce SKIP() support in the unit test framework and surface skipped counts in test results.
Refactor/rename affinity APIs and propagate parse errors upwards (now fatal at runtime when affinity config is invalid).
Add DPDK runmode unit tests (thread auto-assignment + mempool cache size auto-calculation) and enable DPDK in a CI job.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/util-unittest.h	Adds `SKIP(reason)` unittest helper returning a distinct skip code.
src/util-unittest.c	Tracks/pass/fail/skip results; prints skipped count.
src/util-ebpf.c	Updates to renamed affinity cpuset builder API.
src/util-device.c	Re-initializes TAILQ heads after list cleanup/finalize.
src/util-affinity.h	Renames/reshapes affinity API, adds reset + new return types.
src/util-affinity.c	Refactors affinity parsing to return errno-style errors; adds/updates unit tests.
src/tm-threads.c	Updates callsites to renamed affinity APIs.
src/runmodes.c	Makes invalid affinity config fatal with strerror diagnostics.
src/runmode-unittests.c	Registers affinity tests under new name; registers DPDK tests under `HAVE_DPDK`.
src/runmode-dpdk.h	Adds unit test registration prototype guarded by `HAVE_DPDK && UNITTESTS`.
src/runmode-dpdk.c	Adds DPDK unit tests; refactors auto-thread remainder logic; updates affinity API usage.
.github/workflows/builds.yml	Installs DPDK deps and enables `--enable-dpdk` for a unittest build job.

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

lukashino · 2026-05-11T19:13:32Z


    for (i = 0; i < MAX_CPU_SET; i++) {
-        if (strcmp(thread_affinity[i].name, name) == 0) {
+        if (strcmp(thread_affinity[i].name, affinity_set_name) == 0) {


This thread_affinity[i].name != NULL signals bug in the code, irrelevant.

lukashino · 2026-05-11T19:14:05Z

    SCConfInit();
-    ResetAffinityForTest();
+    AffinityReset();
+    SCConfYamlLoadString(config, strlen(config));


unit tests only. this pattern is elsewhere as well.. Irrelevant

lukashino · 2026-05-11T19:14:19Z

+        SKIP("not enough cpus in the system");
+    FAIL_IF(ret < 0);
+
+    DPDKIfaceConfig iconf = { 0 };


name is not needed, irrelevant

lukashino · 2026-05-11T19:16:33Z

+    for (int i = UtilCpuGetNumProcessorsOnline() - 1; i >= 0; i--)
        if (CPU_ISSET(i, &tmpcset)) {
            return 1;
        }


potentially relevant but very minor, not addressing.

codecov · 2026-05-11T20:51:52Z

Codecov Report

❌ Patch coverage is 79.49640% with 57 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.61%. Comparing base (614c48d) to head (4a7f25d).
⚠️ Report is 38 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #15366      +/-   ##
==========================================
- Coverage   82.64%   82.61%   -0.04%     
==========================================
  Files         996      996              
  Lines      271107   271116       +9     
==========================================
- Hits       224063   223989      -74     
- Misses      47044    47127      +83

Flag	Coverage Δ
fuzzcorpus	`61.02% <1.13%> (-0.01%)`	⬇️
livemode	`18.38% <42.97%> (-0.01%)`	⬇️
netns	`22.57% <2.27%> (-0.03%)`	⬇️
pcap	`45.16% <2.27%> (-0.08%)`	⬇️
suricata-verify	`66.36% <2.27%> (-0.04%)`	⬇️
unittests	`58.54% <78.36%> (-0.02%)`	⬇️

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

suricata-qa · 2026-05-12T05:15:03Z

WARNING:

field	baseline	test	%
	SURI_TLPR1_stats_chk
.app_layer.flow.ftp_data	600	620	103.33%

Pipeline = 31395

catenacyber · 2026-05-17T19:13:03Z

Will need a rebase

catenacyber

Requesting changes as it needs a rebase

Lukas Sismis added 9 commits May 11, 2026 15:19

dpdk: adjust condition to uint type

67c862f

The threads field is uint16_t, so the <= 0 comparison is misleading as the value can never be negative. Change to == 0. Ticket: 6927

util-device: reset the device list after cleanup

a0475e6

Needed by unit tests to properly reset the device list. Ticket: 6927

unittests: add skip directive for unsupported unit tests

d9b93b8

Add SKIP() macro and update the test runner to track and report skipped tests separately from passes and failures. Ticket: 6927

dpdk: add unit tests for mempool cache and thread assignment

8e08a80

Add unit tests for mempool cache size auto-calculation and for automatic thread distribution across interfaces, including management thread overlap scenarios. Ticket: 6927

github-ci: expand unit test coverage with DPDK tests to Fedora

4a7f25d

Ticket: 6927

Copilot AI review requested due to automatic review settings May 11, 2026 19:02

lukashino requested review from jasonish and victorjulien as code owners May 11, 2026 19:02

lukashino mentioned this pull request May 11, 2026

dpdk: add initial unittests for DPDK codebase v12.7 #15347

Closed

Copilot AI reviewed May 11, 2026

View reviewed changes

Copilot started reviewing on behalf of lukashino May 11, 2026 19:32 View session

catenacyber added the needs rebase Needs rebase to main label May 17, 2026

catenacyber requested changes May 21, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

dpdk: add initial unittests for DPDK codebase v13.2#15366

dpdk: add initial unittests for DPDK codebase v13.2#15366
lukashino wants to merge 9 commits into
OISF:mainfrom
lukashino:feat/6927-add-unit-tests-v13.2

lukashino commented May 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

lukashino May 11, 2026

Uh oh!

lukashino May 11, 2026

Uh oh!

lukashino May 11, 2026

Uh oh!

lukashino May 11, 2026

Uh oh!

codecov Bot commented May 11, 2026 •

edited

Loading

Uh oh!

suricata-qa commented May 12, 2026

Uh oh!

catenacyber commented May 17, 2026

Uh oh!

catenacyber left a comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

lukashino commented May 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

lukashino May 11, 2026

Choose a reason for hiding this comment

Uh oh!

lukashino May 11, 2026

Choose a reason for hiding this comment

Uh oh!

lukashino May 11, 2026

Choose a reason for hiding this comment

Uh oh!

lukashino May 11, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

suricata-qa commented May 12, 2026

Uh oh!

catenacyber commented May 17, 2026

Uh oh!

catenacyber left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

codecov Bot commented May 11, 2026 •

edited

Loading