(8) feat(dpdk): test EAL harness (with_eal macro + const fn)#1572
Open
daniel-noland wants to merge 6 commits into
Open
(8) feat(dpdk): test EAL harness (with_eal macro + const fn)#1572daniel-noland wants to merge 6 commits into
daniel-noland wants to merge 6 commits into
Conversation
This was referenced May 31, 2026
daniel-noland
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Adds shared DPDK EAL initialization support for tests (via a start_eal() helper and a #[with_eal] proc-macro), and refactors an ACL helper to be usable in const contexts.
Changes:
- Introduces
dpdk::test_support::start_eal()(OnceLock-backed) to initialize EAL once per process with test-friendly flags. - Adds
dataplane-dpdk-test-macrosproviding#[with_eal], and wires it intodataplane-dpdkbehind atestfeature. - Refactors
AclBuildConfig::compute_min_input_sizeto aconst fnand adds a const-context unit test; updates ACL tests to use the macro and simplifies some call sites.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| dpdk/src/test_support.rs | New shared EAL init helper for tests. |
| dpdk/src/lib.rs | Exposes test_support in tests / test feature; re-exports #[with_eal] under feature = "test". |
| dpdk/src/acl/mod.rs | Updates ACL unit tests to use #[with_eal]; minor doc/example tweak. |
| dpdk/src/acl/config.rs | Makes compute_min_input_size const-friendly and adds a const-context test. |
| dpdk/Cargo.toml | Adds test feature and optional deps/macro wiring for test harness support. |
| dpdk-test-macros/src/lib.rs | Implements the #[with_eal] attribute macro. |
| dpdk-test-macros/Cargo.toml | New proc-macro crate manifest. |
| Cargo.toml | Adds new workspace member + workspace dependency entry. |
| Cargo.lock | Locks new proc-macro crate and updated dependency graph. |
Comment on lines
+908
to
913
| /// Compute DPDK's minimum input buffer size. | ||
| /// | ||
| /// See [`min_input_size`][AclBuildConfig::min_input_size] for the | ||
| /// formula and rationale. Factored out so that `new` can call it | ||
| /// once and cache the result; the public accessor returns the cached | ||
| /// value. | ||
| /// | ||
| /// Precondition: all fields' `offset + 4` fit in `u32`. This is | ||
| /// guaranteed by the `FieldExtentOverflow` check in | ||
| /// [`new`][AclBuildConfig::new], so the plain `+` below cannot | ||
| /// overflow. | ||
| fn compute_min_input_size(field_defs: &[FieldDef; N]) -> usize { | ||
| /// `offset + 4` must fit in `u32`; [`AclBuildConfig::new`] validates this. | ||
| #[must_use] | ||
| pub const fn compute_min_input_size(field_defs: &[FieldDef; N]) -> usize { | ||
| let mut max_load_end: u32 = 0; |
mvachhar
approved these changes
Jun 1, 2026
Member
|
Sign-off tag missing |
Add a `dataplane-lifecycle` crate with `Shutdown` and `Subsystem` primitives, signal-handler installation, and a process-wide shutdown watchdog. `Shutdown` bundles a root `CancellationToken` and one `Subsystem` per long-lived component (workers, router, mgmt, metrics). Each subsystem exposes a per-subsystem cancel token, a `TaskTracker`, and a shared fatal flag. Subsystems drain in topological order with per-subsystem deadlines; the detached watchdog enforces an absolute upper bound on total shutdown duration. No consumers yet -- wired up in follow-on commits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Plumb lifecycle Subsystems into the routing crate as the first step of the threading rewrite. The rest of `main`'s shutdown signaling (ctrlc + mpsc<i32> + start_mgmt + MetricsServer + old DriverKernel::start) stays in place; follow-on commits migrate each. - `Router::new` takes `(mgmt, mgmt_handle, router)` Subsystems + runtime handle. Plumbed through `packet_processor::start_router`. - `start_rio` takes `&Subsystem`; the IO loop observes its cancel between poll cycles (worst-case exit latency = 1s poll). Adds an ExitGuard so panic-unwind or unexpected loop exit reports fatal. - `RouterCtlMsg::Finish` removed; `RioHandle::finish` becomes idempotent. - `bmp::spawn_background` spawns onto the caller-provided runtime handle tracked under `mgmt`; no more leaked runtime. - `runtime.rs` builds a multi-thread mgmt runtime (only BMP tenants it in this commit) and a `Shutdown` for plumbing into Router::new. - `dataplane` and `mgmt` Cargo.toml gain `lifecycle` dep. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Move mgmt + metrics from dedicated OS threads with private runtimes to tenants of the multi-thread mgmt runtime introduced in the prior commit. The kernel driver still uses the legacy ctrlc + mpsc<i32> path; the final unification commit migrates that. - `run_mgmt` (renamed from `start_mgmt`): synchronous init on the caller-provided handle, then spawns the three long-lived tasks (config processor, status updater, config watcher) via `Subsystem::spawn_fatal_on_exit` so their unexpected exit flips fatal. Init observes `mgmt.root_token()` so SIGINT during k8s retries returns `LaunchError::Cancelled` within cancel latency. - `LaunchError::Cancelled` is a clean-shutdown signal; the call site in `runtime.rs` forwards the existing mpsc stop channel with code 0 so the legacy shutdown path stays consistent. - `spawn_metrics` replaces `MetricsServer`: HTTP endpoint, upkeep ticker, and stats collector all spawn onto `mgmt_handle` tracked under `metrics`. Uses plain `spawn_on` (not `spawn_fatal_on_exit`) — a dead metrics endpoint should not take down the dataplane. - Drop stale `LaunchError` variants no longer constructed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Replace the last legacy shutdown signaling (ctrlc handler + mpsc<i32> exit code + dedicated controller thread) with the single `lifecycle::Shutdown` path, and migrate the kernel driver to scoped threads with cancellation observation. After this commit there is one signaling path: SIGINT/SIGTERM -> shutdown.root, or any subsystem's report_fatal -> shutdown.root, with the watchdog as the absolute upper bound. - `main`: install `spawn_signal_handler` and `spawn_shutdown_watchdog`, run everything inside `concurrency::thread::scope`, block on `root.cancelled()`, then drain subsystems in canonical order (workers -> router -> metrics -> mgmt). Exit code from `is_fatal()`. - `DriverKernel::start`: takes `&Scope` and `&Subsystem`; workers spawn via `spawn_scoped` with an `ExitGuard` Drop pattern that reports fatal on panic-unwind, early `?`-return, and unexpected normal exit. Reader loops observe cancel between reads. Supervisor joins-and-logs. - Drop `dataplane/src/drivers/tokio_util.rs` and its `run_in_local_tokio_runtime` helper (inlined where needed). - Drop `ctrlc` and `mio` from `dataplane` dependencies; drop `ctrlc` from the workspace. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Lets callers derive the rte_acl input-buffer size requirement from a const FIELD_DEFS array at compile time -- useful for feeding into a const generic (e.g. the STRIDE on a DPDK-backed Lookup type alias) rather than rediscovering it at runtime. The runtime path through AclBuildConfig::new still calls into the same helper and caches the result; only the surface broadens from fn to const fn (with the for-loop rewritten as a while-let-i index walk that const-fn-eval can chew on). Existing FieldExtentOverflow validation is preserved; in a const context overflow becomes a compile error instead of UB. Adds one test exercising the const path: const MIN_INPUT_SIZE = AclBuildConfig::compute_min_input_size(&DEFS); plus the assertion that the runtime path returns the same value. just fmt; cargo check -p dataplane-dpdk --all-targets passes.
Introduces shared EAL test scaffolding for downstream crates that
need to exercise rte_acl, mempools, or any other DPDK runtime under
`#[test]`. Two pieces, both behind the new dpdk `test` feature so
production builds remain unchanged:
- dpdk-test-macros: proc-macro crate exposing #[with_eal], which
injects `let _eal = <dpdk_crate>::test_support::start_eal();` at
the top of a #[test] function. Resolves the dpdk crate's path via
proc-macro-crate so the macro works in-tree (`crate`), under the
workspace alias (`::dpdk`), or with the canonical name
(`::dataplane_dpdk`).
- dpdk::test_support: hosts a shared OnceLock<Eal> initialized with
--no-huge / --no-pci / --in-memory and the cpu-affinity-aware
--lcores derivation that was previously inline in acl/mod.rs.
Once-per-process by construction; safe under nextest's per-test
forking and single-process runners alike.
dpdk/src/acl/mod.rs picks up the new macro: every test in the module
loses its `let _eal = start_eal();` prologue and gains a #[with_eal]
attribute above #[test]. The inline start_eal helper and the
module-scoped OnceLock<Eal> static go away.
dpdk/Cargo.toml grows the `test` feature (turns on dpdk-test-macros
plus the id and nix runtime helpers that test_support needs) and a
self-referencing dev-dep `dataplane-dpdk = { path = ".", features =
["test"] }` so dpdk's own tests see the macro surface.
just fmt; cargo check --workspace --all-targets passes.
daniel-noland
force-pushed
the
pr/daniel-noland/threading-rewrite
branch
from
2021414 to
b7b5ece
Compare
daniel-noland
force-pushed
the
pr/daniel-noland/dpdk-test-eal
branch
from
9da6fad to
14adb95
Compare
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.
Stack (8). Base:
pr/daniel-noland/threading-rewrite.DPDK test plumbing that the rest of the ACL stack relies on, kept separate so it
reviews on its own:
refactor(dpdk/acl): makecompute_min_input_sizeaconst fn.feat(dpdk):test_support::start_eal+#[with_eal]attribute macro(
dpdk-test-macros), so EAL-dependent tests can opt in declaratively.No new abstractions, no behavior change to shipping code.
Review stack (merge bottom -> top):