Skip to content

(12) feat(acl): rte_acl backend + differential-vs-reference + benches#1576

Merged
qmonnet merged 10 commits into
mainfrom
pr/daniel-noland/acl-dpdk
Jun 30, 2026
Merged

(12) feat(acl): rte_acl backend + differential-vs-reference + benches#1576
qmonnet merged 10 commits into
mainfrom
pr/daniel-noland/acl-dpdk

Conversation

@daniel-noland

@daniel-noland daniel-noland commented May 31, 2026

Copy link
Copy Markdown
Collaborator

Stack (12). Base: pr/daniel-noland/acl-reference.

The rte_acl backend and everything proving it matches the reference oracle.
This is the largest PR -- the irreducible DPDK lowering complexity:

  • feat(acl/dpdk): layout planner + rule lowering.
  • feat(acl/dpdk): install + DpdkAclLookup + EAL smoke.
  • feat(acl/dpdk): DynDpdkLookup + shape-fuzz property test.
  • test(acl): differential property (reference vs DPDK) + Headers/metadata
    projection demos.
  • feat(acl): criterion benchmarks + nix bench-builder.

Review stack (merge bottom -> top):

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 adds a DPDK rte_acl-powered backend for the dataplane-acl crate, plus differential/property tests and criterion benchmarks to validate correctness against the existing software reference backend and measure performance.

Changes:

  • Introduces acl::dpdk backend (layout planning, rule lowering/splicing, install/build, typed + dynamic lookup APIs).
  • Adds DPDK-gated integration/property tests (reference-vs-DPDK differential checks, dynamic-shape fuzzing, header/metadata projection examples).
  • Adds criterion benchmarks and Nix/Just plumbing to build and run bench binaries.

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
justfile Adds bench recipe to build and execute bench binaries.
default.nix Adds bench-builder derivation and exports benches.
Cargo.toml Adds workspace dependency on criterion.
Cargo.lock Locks new benchmark dependency graph (criterion + transitive deps).
acl/tests/property_predicate.rs Differential property test for typed 5-tuple (v4/v6) against reference.
acl/tests/property_dyn_shape.rs Differential property test for dynamic shapes (DPDK vs reference).
acl/tests/net_field_types.rs Demonstrates net newtypes classification through DPDK and reference.
acl/tests/metadata_projection.rs Demonstrates Projection-based classification using headers + metadata.
acl/tests/eal_install_classify.rs EAL smoke test for install + classify + batch classify.
acl/tests/eal_classify_via_projection.rs EAL smoke test classifying a real packet via projection.
acl/src/lib.rs Exposes dpdk module and adds dpdk_table_alias! helper macro under feature gate.
acl/src/dpdk/mod.rs Defines DPDK backend module surface.
acl/src/dpdk/rule.rs Implements DPDK rule lowering + field splicing + related errors/tests.
acl/src/dpdk/lookup.rs Implements typed DPDK lookup and batch classify helpers.
acl/src/dpdk/layout.rs Implements layout planning + const extents for compile-time type aliases.
acl/src/dpdk/install.rs Implements table installation/building into an AclContext.
acl/src/dpdk/dyn_table.rs Implements dynamic-shape install + byte-key lookup path.
acl/Cargo.toml Adds dpdk feature, DPDK/criterion dev-deps, and bench targets.
acl/benches/table_build.rs Benchmarks table build cost (reference vs DPDK, v4/v6).
acl/benches/reference_five_tuple.rs Benchmarks reference backend lookup patterns (v4/v6).
acl/benches/dpdk_five_tuple.rs Benchmarks DPDK backend lookups (single + batch, v4/v6).

Comment thread Cargo.toml Outdated
Comment thread acl/Cargo.toml Outdated
Comment thread acl/src/lib.rs
Comment thread acl/src/dpdk/lookup.rs
Comment thread acl/src/dpdk/lookup.rs Outdated
Comment thread acl/src/dpdk/dyn_table.rs
Comment thread justfile Outdated
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/acl-reference branch from 3220e66 to 710b08b Compare June 3, 2026 19:50
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/acl-dpdk branch from 16e2188 to 01d98b2 Compare June 3, 2026 19:50
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/acl-dpdk branch from 01d98b2 to ddbc607 Compare June 15, 2026 20:18
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/acl-reference branch from 710b08b to 598ba60 Compare June 15, 2026 20:18
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@daniel-noland, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 27b2d018-2962-4872-ab11-42fff4b7e0ac

📥 Commits

Reviewing files that changed from the base of the PR and between f0f8dc0 and a445674.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (23)
  • Cargo.toml
  • acl/Cargo.toml
  • acl/benches/dpdk_five_tuple.rs
  • acl/benches/reference_five_tuple.rs
  • acl/benches/table_build.rs
  • acl/src/dpdk/dyn_table.rs
  • acl/src/dpdk/install.rs
  • acl/src/dpdk/layout.rs
  • acl/src/dpdk/lookup.rs
  • acl/src/dpdk/mod.rs
  • acl/src/dpdk/rule.rs
  • acl/src/lib.rs
  • acl/tests/eal_classify_via_projection.rs
  • acl/tests/eal_install_classify.rs
  • acl/tests/metadata_projection.rs
  • acl/tests/net_field_types.rs
  • acl/tests/property_dyn_shape.rs
  • acl/tests/property_predicate.rs
  • default.nix
  • justfile
  • nix/overlays/dataplane.nix
  • npins/sources.json
  • scripts/doc/custom-header.html
📝 Walkthrough

Walkthrough

Adds a DPDK-backed ACL pipeline behind a new feature flag, with typed and dynamic install/lookup APIs, layout and rule lowering, tests, benchmarks, and related build/dependency updates.

Changes

DPDK ACL Backend

Layer / File(s) Summary
Cargo manifest, feature gate, and crate-level exports
Cargo.toml, acl/Cargo.toml, acl/src/lib.rs
Adds criterion as a workspace dependency, introduces the optional dpdk feature and dev-dependencies in acl, and exports the dpdk module plus dpdk_table_alias!.
DPDK field layout planner
acl/src/dpdk/layout.rs
Implements DPDK field layout planning, const extents, layout validation, and layout tests.
Rule lowering and field splicing
acl/src/dpdk/rule.rs
Adds DPDK field lowering, rule specs, splicing helpers, and unit tests for lowering and placement.
Dynamic DPDK classifier and lookup
acl/src/dpdk/dyn_table.rs
Adds dynamic rule specs, classifier dispatch, dynamic install, dynamic lookup, and validation tests.
Typed lookup and install API
acl/src/dpdk/lookup.rs, acl/src/dpdk/install.rs
Adds typed ACL install and lookup APIs, batch lookup, packing, and error types.
EAL integration tests
acl/tests/eal_install_classify.rs, acl/tests/eal_classify_via_projection.rs, acl/tests/metadata_projection.rs, acl/tests/net_field_types.rs
Verifies DPDK ACL classification through direct install, projection-based views, metadata, and net field types.
Property-based tests for predicates and dynamic shapes
acl/tests/property_predicate.rs, acl/tests/property_dyn_shape.rs
Adds Bolero-based agreement tests for typed predicates and dynamic table shapes.
Criterion benchmarks for lookup and table build
acl/benches/reference_five_tuple.rs, acl/benches/dpdk_five_tuple.rs, acl/benches/table_build.rs
Adds reference, DPDK, and table-build benchmarks for IPv4 and IPv6.

Build and dependency updates

Layer / File(s) Summary
Bench derivation and recipe wiring
default.nix, justfile
Adds a Nix bench derivation, exposes benches, and adds a just bench recipe with version handling updates.
Pinned sources and Mermaid script
npins/sources.json, scripts/doc/custom-header.html
Updates pinned upstream source revisions and the Mermaid CDN asset version/hash.
Rdma-core patch filtering
nix/overlays/dataplane.nix
Filters one patch out of the rdma-core overlay patch list.

Possibly related PRs

Suggested reviewers

  • sergeymatov
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding the rte_acl backend, reference comparison, and benches.
Description check ✅ Passed The description is directly aligned with the changeset and accurately summarizes the backend, tests, and benchmarks.
Docstring Coverage ✅ Passed Docstring coverage is 82.10% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@daniel-noland daniel-noland force-pushed the pr/daniel-noland/acl-dpdk branch 2 times, most recently from 19ca1a1 to 29232af Compare June 15, 2026 22:08
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/acl-reference branch from 598ba60 to 08b4c60 Compare June 15, 2026 22:08
Comment thread acl/src/dpdk/layout.rs Outdated
Comment thread acl/src/dpdk/lookup.rs Outdated
Comment thread acl/src/dpdk/layout.rs Outdated
Comment thread acl/src/dpdk/layout.rs
Comment thread acl/src/dpdk/layout.rs
Comment thread acl/src/dpdk/lookup.rs Outdated
Comment thread acl/src/dpdk/dyn_table.rs
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/acl-dpdk branch from 29232af to fbcf630 Compare June 18, 2026 19:34
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/acl-reference branch from 08b4c60 to 7a7d109 Compare June 18, 2026 19:34
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/acl-dpdk branch from c795ed9 to 19de724 Compare June 18, 2026 20:16
Base automatically changed from pr/daniel-noland/acl-reference to main June 18, 2026 20:50
@mvachhar mvachhar self-requested a review June 18, 2026 21:46
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/acl-dpdk branch 2 times, most recently from 5293190 to 1665ce5 Compare June 30, 2026 02:10

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 7

🧹 Nitpick comments (1)
acl/benches/table_build.rs (1)

70-78: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Stop reparsing the IPv6 prefix in the timed path.

rule_v6 parses the same literal on every rule build, so table_build_v6 measures string parsing n times per iteration in addition to table construction. Hoist the address once or use Ipv6Addr::new(...) so the benchmark stays focused on build cost.

Suggested change
     fn rule_v6(i: usize) -> FiveTuple6Rule {
         FiveTuple6Rule {
             proto: ExactSpec::new(6),
-            src: PrefixSpec::new("2001:db8::".parse().expect("v6 literal"), 32),
+            src: PrefixSpec::new(Ipv6Addr::new(0x2001, 0x0db8, 0, 0, 0, 0, 0, 0), 32),
             dst: PrefixSpec::new(Ipv6Addr::UNSPECIFIED, 0),
             sport: RangeSpec::new(0, u16::MAX),
             dport: RangeSpec::exact(u16::try_from(i).unwrap_or(u16::MAX)),
         }
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@acl/benches/table_build.rs` around lines 70 - 78, The benchmark helper
rule_v6 is reparsing the same IPv6 literal on every call, which adds parsing
overhead to table_build_v6 instead of measuring table construction. Fix this by
hoisting the parsed IPv6 address out of rule_v6 or replacing the string parse
with a direct Ipv6Addr construction, then reuse that value when building the
PrefixSpec for the src field.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@acl/benches/dpdk_five_tuple.rs`:
- Around line 141-149: The benchmark batch in dpdk_five_tuple currently varies
from mostly misses to all hits as n grows because the shared batch uses a fixed
dport range, so make the workload consistent across table sizes. Move batch
construction into the n loop in the benchmark setup and derive dport from j % n
(or otherwise separate hit/miss cases explicitly) so each rule count is measured
apples-to-apples. Apply the same adjustment to the bench_v6 path and the related
benchmark blocks that reuse the shared batch pattern.

In `@acl/src/dpdk/dyn_table.rs`:
- Around line 392-397: The match in dyn_table’s classification path is
incorrectly converting AclClassifyError into None, making backend failures look
like normal misses. Update the logic around self.classifier.classify_one in the
dyn_table lookup path so only genuine no-match cases return None; either
propagate the error to the caller or change the API to return Result<Option<_>,
AclClassifyError> so failures are not hidden. Ensure the behavior in the
classifier-facing methods remains consistent with the documented meaning of
None.
- Around line 51-54: The chunk-shape validation in `predicate_to_chunks` is only
enforced by `debug_assert!`, so invalid widths can slip through in release
builds and reach `pack_chunks`/`chunks_exact` incorrectly. Replace the
debug-only check with a real runtime validation in `predicate_to_chunks` (and
the corresponding later check around the other affected block), rejecting
unsupported `size_bytes` values before lowering continues. Keep the existing
shape rule explicit using the same `size_bytes` conditions so bad input is
handled consistently in all builds.

In `@acl/src/dpdk/layout.rs`:
- Around line 68-74: The `const_extents` helper can still produce a field count
that later exceeds `MAX_FIELDS` in `plan_layout`, so add a final compile-time
assertion in `const_extents` to guarantee the returned planned layout stays
bounded. Use the existing `plan_layout`/`MAX_FIELDS` constraint as the source of
truth and ensure compile-time callers fail consistently before any invalid
layout can be returned.

In `@acl/src/dpdk/lookup.rs`:
- Around line 146-149: `Lookup::lookup` is currently collapsing
`AclClassifyError` into a normal miss by using `ok()?`, which hides backend
failures inside the `Classifier::classify_one` path. Update the `Lookup` impl
for `AclLookup` so classify failure is treated as invariant breakage (for
example via `expect`/panic with a clear message), or add a separate fallible
typed lookup API and keep this trait method reserved for real ACL misses only.

In `@acl/src/dpdk/rule.rs`:
- Around line 56-62: `predicate_to_chunks` can still receive unsupported widths
even though `install_table_dynamic` validates layouts, and `acl_size_for`
currently maps them to `AclSize::Four` by default. Update `predicate_to_chunks`
(and any helper path it relies on, including `acl_size_for`) to explicitly
reject non-supported sizes like 3, 5, or 6 in release mode, either by returning
a checked error result or by adding a validation guard before chunk packing. Use
the existing symbols `predicate_to_chunks` and `acl_size_for` so the fix is
applied at the point where widths are converted into ACL chunks.

In `@acl/tests/property_dyn_shape.rs`:
- Around line 156-179: The install_both helper is swallowing DPDK install
failures for shapes that should already be valid, which can hide regressions in
dynamic-table installation. In install_both, and the other matching call
site(s), stop using .ok()?/early return to skip failed installs; instead, make
the failure visible by asserting or otherwise failing the test when
install_table_dynamic::<u32> (and the paired DynReferenceTable::new setup)
cannot be created for a generated valid shape. Keep the existing install path
around install_both, predicate_to_chunks, and build_shape intact, but ensure
generated valid shapes always attempt installation and surface any failure.

---

Nitpick comments:
In `@acl/benches/table_build.rs`:
- Around line 70-78: The benchmark helper rule_v6 is reparsing the same IPv6
literal on every call, which adds parsing overhead to table_build_v6 instead of
measuring table construction. Fix this by hoisting the parsed IPv6 address out
of rule_v6 or replacing the string parse with a direct Ipv6Addr construction,
then reuse that value when building the PrefixSpec for the src field.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 53b80bb3-e98e-49aa-afbd-fdc26675befb

📥 Commits

Reviewing files that changed from the base of the PR and between 2572239 and 5293190.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (20)
  • Cargo.toml
  • acl/Cargo.toml
  • acl/benches/dpdk_five_tuple.rs
  • acl/benches/reference_five_tuple.rs
  • acl/benches/table_build.rs
  • acl/src/dpdk/dyn_table.rs
  • acl/src/dpdk/install.rs
  • acl/src/dpdk/layout.rs
  • acl/src/dpdk/lookup.rs
  • acl/src/dpdk/mod.rs
  • acl/src/dpdk/rule.rs
  • acl/src/lib.rs
  • acl/tests/eal_classify_via_projection.rs
  • acl/tests/eal_install_classify.rs
  • acl/tests/metadata_projection.rs
  • acl/tests/net_field_types.rs
  • acl/tests/property_dyn_shape.rs
  • acl/tests/property_predicate.rs
  • default.nix
  • justfile

Comment on lines +141 to +149
let batch: Vec<FiveTuple> = (0..BATCH)
.map(|j| FiveTuple {
proto: 6,
src: Ipv4Addr::new(10, 0, 0, 1),
dst: Ipv4Addr::new(192, 0, 2, 1),
sport: 1234,
dport: u16::try_from(j).unwrap_or(0),
})
.collect();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Keep the batch workload constant across n.

The shared batch uses dport = 0..31, so for n < 32 most entries are guaranteed misses, while for n >= 32 they are all hits. That makes the batch series change workload as the table grows, so the reported throughput is not apples-to-apples across rule counts. Build the batch inside the n loop with dport = j % n, or split hit and miss batch benchmarks explicitly.

As per coding guidelines, logic errors in the code under review should be fixed when the behavior is clearly incorrect.

Suggested change
-        let batch: Vec<FiveTuple> = (0..BATCH)
-            .map(|j| FiveTuple {
-                proto: 6,
-                src: Ipv4Addr::new(10, 0, 0, 1),
-                dst: Ipv4Addr::new(192, 0, 2, 1),
-                sport: 1234,
-                dport: u16::try_from(j).unwrap_or(0),
-            })
-            .collect();
-
         let mut group = c.benchmark_group("dpdk_five_tuple_v4");
         for n in RULE_COUNTS {
+            let batch: Vec<FiveTuple> = (0..BATCH)
+                .map(|j| FiveTuple {
+                    proto: 6,
+                    src: Ipv4Addr::new(10, 0, 0, 1),
+                    dst: Ipv4Addr::new(192, 0, 2, 1),
+                    sport: 1234,
+                    dport: u16::try_from(j % n).unwrap_or(0),
+                })
+                .collect();
             let table = build_table_v4(n);
             let miss = FiveTuple {
                 proto: 6,
@@
             let hit = FiveTuple { dport: 0, ..miss };
             run_lookups(&mut group, n, &table, &miss, &hit, &batch);
         }

Apply the same pattern in bench_v6.

Also applies to: 151-163, 170-178, 180-192

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@acl/benches/dpdk_five_tuple.rs` around lines 141 - 149, The benchmark batch
in dpdk_five_tuple currently varies from mostly misses to all hits as n grows
because the shared batch uses a fixed dport range, so make the workload
consistent across table sizes. Move batch construction into the n loop in the
benchmark setup and derive dport from j % n (or otherwise separate hit/miss
cases explicitly) so each rule count is measured apples-to-apples. Apply the
same adjustment to the bench_v6 path and the related benchmark blocks that reuse
the shared batch pattern.

Source: Coding guidelines

Comment thread acl/src/dpdk/dyn_table.rs
Comment thread acl/src/dpdk/dyn_table.rs
Comment on lines +392 to +397
// SAFETY: stride >= min_input_size (checked in `do_install_n`).
let user_data = unsafe {
match self.classifier.classify_one(&dpdk_buf[..stride]) {
Ok(ud) => ud,
Err(_) => return None,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not turn classifier errors into table misses.

The docs say None means no rule matched, but AclClassifyError is also returned as None here. Fail loudly or expose a Result API so backend failures are not indistinguishable from misses.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@acl/src/dpdk/dyn_table.rs` around lines 392 - 397, The match in dyn_table’s
classification path is incorrectly converting AclClassifyError into None, making
backend failures look like normal misses. Update the logic around
self.classifier.classify_one in the dyn_table lookup path so only genuine
no-match cases return None; either propagate the error to the caller or change
the API to return Result<Option<_>, AclClassifyError> so failures are not
hidden. Ensure the behavior in the classifier-facing methods remains consistent
with the documented meaning of None.

Comment thread acl/src/dpdk/layout.rs
Comment on lines +68 to +74
while group_used > 0 && group_used < 4 {
n += 1;
offset += 1;
group_used += 1;
}

(n, offset)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep const_extents bounded by MAX_FIELDS.

const_extents validates field sizes but can still return a planned field count that plan_layout rejects once chunking/padding exceeds DPDK’s MAX_FIELDS. Add a final const assertion so compile-time layout users fail consistently.

Proposed fix
     while group_used > 0 && group_used < 4 {
         n += 1;
         offset += 1;
         group_used += 1;
     }
 
+    assert!(
+        n <= MAX_FIELDS,
+        "too many field defs; planned layout exceeds rte_acl's field limit",
+    );
+
     (n, offset)
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
while group_used > 0 && group_used < 4 {
n += 1;
offset += 1;
group_used += 1;
}
(n, offset)
while group_used > 0 && group_used < 4 {
n += 1;
offset += 1;
group_used += 1;
}
assert!(
n <= MAX_FIELDS,
"too many field defs; planned layout exceeds rte_acl's field limit",
);
(n, offset)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@acl/src/dpdk/layout.rs` around lines 68 - 74, The `const_extents` helper can
still produce a field count that later exceeds `MAX_FIELDS` in `plan_layout`, so
add a final compile-time assertion in `const_extents` to guarantee the returned
planned layout stays bounded. Use the existing `plan_layout`/`MAX_FIELDS`
constraint as the source of truth and ensure compile-time callers fail
consistently before any invalid layout can be returned.

Comment thread acl/src/dpdk/lookup.rs
Comment on lines +146 to +149
// SAFETY: `buf` holds `stride >= min_input_size` valid bytes (invariant
// established in `new`).
let user_data = unsafe { self.classifier.classify_one(&buf).ok()? };
action_for(&self.actions, user_data)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Keep classify failures distinct from misses.

Lookup::lookup has no error channel, but returning None for AclClassifyError makes backend failures look like ACL misses. Prefer expect/panic on invariant breakage here, or add a fallible typed lookup alongside this trait impl.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@acl/src/dpdk/lookup.rs` around lines 146 - 149, `Lookup::lookup` is currently
collapsing `AclClassifyError` into a normal miss by using `ok()?`, which hides
backend failures inside the `Classifier::classify_one` path. Update the `Lookup`
impl for `AclLookup` so classify failure is treated as invariant breakage (for
example via `expect`/panic with a clear message), or add a separate fallible
typed lookup API and keep this trait method reserved for real ACL misses only.

Comment thread acl/src/dpdk/rule.rs
Comment thread acl/tests/property_dyn_shape.rs
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/acl-dpdk branch 4 times, most recently from f0f8dc0 to ca1cc66 Compare June 30, 2026 04:11
daniel-noland and others added 10 commits June 29, 2026 22:14
`cargo miri nextest` builds proc-macro test binaries on the host
toolchain (not the miri cross target), so they are real ELF executables
rather than the JSON stubs miri emits for target binaries. nextest is
invoked with a `target.cfg(all()).runner` that routes every test binary,
host platform included, through the cargo-miri runner wrapper. The
wrapper tries to parse the real ELF as its JSON stub and fails with a
misleading "contains outdated or invalid JSON; try \`cargo clean\`" --
which is why cleaning never helped.

match-action-derive (the third proc-macro crate) was already excluded
for this reason, but concurrency-macros and dpdk-test-macros had no
workspace metadata entry, so --workspace pulled them in. Add the missing
 miri = false / wasm = false exclusions, mirroring match-action-derive.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
The just language changed to make our previous || hack illegal.

The fix is very simple.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
anyhow had a downcast bug which is now resolved.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
nixpkgs added the cmake-allow-overriding-sysusers.d-install-directory
patch, which turns SYSUSERS_DIR into a CMake cache variable. Our pinned
githedgehog/rdma-core fork already cherry-picked that exact upstream
commit, so applying the nixpkgs patch on top fails patchPhase with
"Reversed (or previously applied) patch detected" and aborts the build.

The breakage is profile-independent but only surfaces on uncached
derivations; common profiles were served from the binary cache and never
re-ran patchPhase, so it first showed up under
'just profile=fuzz sanitize=thread test'.

Filter that one patch out by name (rather than clearing the whole list)
so any future nixpkgs patches we do want still apply.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Lands the static type machinery for the DPDK `rte_acl` backend
behind the new `dpdk` feature gate: how a MatchKey's FIELD_SPECS maps
into rte_acl's per-field FieldDef array, and how the four
match-action *Spec predicate kinds lower into IntoBackendField for
the `Dpdk` backend marker.  The runtime install / classify path
(install.rs, lookup.rs) and the dpdk_table_alias! macro land next.

acl/src/dpdk/:

- mod.rs declares the two submodules; carries a temporary
  #![allow(dead_code)] because the layout's `stride` field and the
  rule.rs RuleSpec fields are consumed only once install / lookup
  arrive in the next PR.  The allow goes away then.
- layout.rs has the rte_acl field planner: group fields by
  input_index (rte_acl requires the first field to be one byte,
  remaining fields grouped into <= 4-byte buckets), insert padding
  for gaps, and yield a DpdkLayout { field_defs: [FieldDef; N],
  stride, user_to_dpdk }.  const_extents() is const fn so a const
  alias can derive N / STRIDE from K::FIELD_SPECS without unstable
  generic_const_exprs.  Wide fields (Ipv6Addr, u128) decompose into
  four u32 sub-fields the way l3fwd-acl does.
- rule.rs holds the Dpdk backend marker, the AclWord trait (blanket
  impl over FixedSize via chunks()), the IntoBackendField impls
  carrying each *Spec into a backend-typed AclField group, the
  RuleSpec rule-field envelope, and splice_user_fields_to_dpdk for
  reordering user-declared fields into rte_acl's layout-driven
  ordering.

acl/src/lib.rs picks up the #[cfg(feature = "dpdk")] gate on
pub mod dpdk; (no macro yet -- the dpdk_table_alias! macro lands with
its lookup-side referent next PR).

acl/Cargo.toml grows the dpdk feature and the optional dpdk
workspace dep.  No dev-deps yet.

just fmt; cargo check --workspace --all-targets and
cargo clippy -p dataplane-acl --features dpdk -- -D warnings pass.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Wires the layout planner and rule lowering from the previous PR into
a working DPDK backend: build an AclContext from a MatchKey plus its
rules, wrap it in a DpdkAclLookup, and classify packets through it.
First EAL-touching PR in the acl stack.

src/dpdk/:

- install.rs is the from-K-plus-rules constructor: take a MatchKey,
  call plan_layout to get the rte_acl FieldDefs, build an
  AclContext, splice each user RuleSpec through layout's
  user_to_dpdk map into rte_acl's column order, hand the rules to
  the context, build, and wrap the built context in a
  DpdkAclLookup<K, N, STRIDE, A>.
- lookup.rs is DpdkAclLookup itself: stack-packed key bytes
  (MAX_USER_KEY_BYTES sentinel feeds the compile-time guard in
  dpdk_table_alias!), the impl Lookup<K, A> single-shot path, and a
  batched classify_batch over a slice of K returning aligned actions.
- mod.rs picks up pub mod install / pub mod lookup and drops the
  temporary #![allow(dead_code)] from the previous PR -- RuleSpec
  fields and DpdkLayout.stride now have readers.

src/lib.rs gains the dpdk_table_alias! macro: dpdk_table_alias!(pub
type FiveTupleTable<Verdict> = FiveTuple); yields a DpdkAclLookup<K,
N, STRIDE, A> with N / STRIDE derived from K::FIELD_SPECS via
const_extents.  A const _: () = assert!(KEY_SIZE <=
MAX_USER_KEY_BYTES) guards against keys that wouldn't fit the
stack scratch buffer.  The hidden __match_action module re-exports
MatchKey so the macro resolves without a caller-side import.

tests/eal_install_classify.rs is the smoke: derive a MatchKey, install
two rules with priority precedence, classify via the single-shot path
and the batch path, assert userdata.

acl/Cargo.toml grows a single dev-dep -- self-overriding dpdk with the
`test` feature on so #[with_eal] from dpdk-test-macros works.

just fmt; cargo check --workspace --all-targets and
cargo clippy -p dataplane-acl --features dpdk -- -D warnings pass.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Adds the runtime-shape twin of DpdkAclLookup -- DynDpdkLookup carries
its FieldSpec layout at runtime instead of in const generics -- and
the shape-fuzz oracle that proves the byte-level pipeline agrees
between the reference oracle and rte_acl over an unconstrained
schema.

src/dpdk/dyn_table.rs is DynDpdkLookup<A>:

- new(name, max_rule_num, field_specs) plans the rte_acl layout from
  a Vec<FieldSpec> at runtime, builds an empty AclContext, and
  returns a typed lookup keyed by an Erased FieldPredicate vector.
- add_rules takes Vec<DynRuleSpec> -- the runtime-shape rule
  carrier (priority, category_mask, lowered fields, action) -- and
  splices each rule's field bytes through the user_to_dpdk map into
  rte_acl's column order, then builds.
- impl Lookup<Vec<FieldBytes>, A>: pack the probe bytes onto the
  stack scratch buffer in the layout's column order, hand them to
  rte_acl_classify, and translate the userdata hit back to &A.

src/dpdk/mod.rs picks up pub mod dyn_table; alongside the typed path.

tests/property_dyn_shape.rs is the schema fuzz:

- bolero TypeGenerator yields a random Vec<FieldSpec>, a single rule
  matching that shape, and packet seeds.
- For each shape: install the same rule into a DynReferenceTable
  (oracle) and a DynDpdkLookup, then probe both with both a
  hit-byte seed and a miss-byte seed.  Assert agreement on every
  probe.
- No MatchKey types involved -- exercises the byte-level pipeline
  end-to-end and catches drift in layout planning, the splice map,
  and rte_acl's per-predicate semantics simultaneously.

acl/Cargo.toml gains bolero + match-action[bolero] dev-deps the
test needs.

just fmt; cargo check --workspace --all-targets and
cargo clippy -p dataplane-acl --features dpdk -- -D warnings pass.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Pure test broadening; no src changes.  Adds the single-rule v4/v6
differential against the reference oracle and three Headers /
metadata projection demos that exercise classify / classify_opt
against real net::HeadersView packets.

tests/property_predicate.rs is the differential.  For a random
5-tuple rule + random hit/miss byte seeds drawn via match-action's
FieldHit / FieldMiss generators, both the reference oracle and the
DPDK backend must accept every hits() draw and reject every misses()
draw.  Parameterised over the address width via a sealed IpAddress
trait so a single body covers v4 (Ipv4Addr) and v6 (Ipv6Addr) -- the
DPDK wide-field split (one 16-byte address -> four 4-byte
sub-fields) is exercised end-to-end by the v6 invocation.  Single
rule only; multi-rule differential is deferred (positional
precedence vs numeric Priority).

tests/eal_classify_via_projection.rs is the end-to-end projection
demo: a real packet -> HeadersView -> Projection<FiveTuple> -> DPDK
Lookup<FiveTuple, _> -> action.  Shows Lookup::classify runs the
projection and the lookup as a single call -- the call site reads
table.classify(\&headers) and doesn't see the intermediate key
construction.

tests/metadata_projection.rs is the partial-projection demo.  Header
fields live in Headers; VRF / VNI live in PacketMeta.  A projection
source bundles &HeadersView with &PacketMeta and projects to
Option<K>: the header part is total (shape proves presence), the
metadata part narrows from its Option with ?.  Missing metadata
projects to None and Lookup::classify_opt turns that into a table
miss with no explicit branch in user code.

tests/net_field_types.rs uses net wire newtypes (TcpPort, UdpPort,
Vni, UnicastIpv4Addr) directly as MatchKey fields with no acl-side
AclWord impl, leaning on net's FixedSize impls (PR 2a) and the DPDK
backend's blanket AclWord-over-FixedSize impl.

acl/Cargo.toml grows the net[test_buffer, builder] dev-dep these
projection demos need.

just fmt; cargo check --workspace --all-targets and
cargo clippy -p dataplane-acl --features dpdk -- -D warnings pass.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Adds criterion benchmarks for both backends at v4 and v6 widths,
plus the nix / just plumbing to produce bench binaries from a
sandboxed build.

acl/benches/:

- reference_five_tuple.rs sweeps a deep miss (full per-rule scan)
  and an early hit through the reference's O(rules * fields) linear
  scan.  Both widths.
- dpdk_five_tuple.rs is the rte_acl companion: trie walk cost (close
  to flat in rule count), miss vs hit, single-shot vs SIMD batch.
  v6 exercises the wide-field split (one 16-byte address -> four
  4-byte sub-fields).  Requires a live EAL.
- table_build.rs measures construction cost vs rule count: reference
  (lower + Vec wrap) and DPDK (rte_acl_build, the update-latency
  cost).  Both widths.  iter_batched so teardown is excluded.

acl/Cargo.toml gets the criterion dev-dep and three harness = false
[[bench]] entries.  Workspace Cargo.toml gets the criterion = 0.5.1
shared dep entry.

default.nix adds a bench-builder derivation: cargo bench --no-run
under the profile-appropriate DPDK sysroot, then copies each
compiled benchmark into $out/bin (stripping cargo's -<hash>
suffix).  Linked against the optimized DPDK when profile = release.

justfile adds a bench recipe that builds the benches package and
runs every binary under results/benches/bin/ in turn.

just fmt; cargo check --workspace --all-targets and
cargo clippy -p dataplane-acl --features dpdk -- -D warnings pass.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
DpdkAclLookup<K,A> drops N_FIELDS/STRIDE and holds Box<dyn DynClassifier>; install_table<K,A> dispatches the field count at runtime via build_classifier_n shared with the dyn install path; adds DynClassifier::classify_batch so the typed batch path keeps real rte_acl batching through the type erasure; dispatch ceiling raised to MAX_FIELDS (64); keys packed into a single runtime-layout.stride arena (batch path drops the redundant per-key copy); removes lookup_via_bytes/dpdk_key_bytes and simplifies the dpdk_table_alias! macro (no const_extents).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/acl-dpdk branch from ca1cc66 to a445674 Compare June 30, 2026 04:15
@daniel-noland daniel-noland added this pull request to the merge queue Jun 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 30, 2026
@qmonnet qmonnet added this pull request to the merge queue Jun 30, 2026
Merged via the queue into main with commit 70f9521 Jun 30, 2026
35 checks passed
@qmonnet qmonnet deleted the pr/daniel-noland/acl-dpdk branch June 30, 2026 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants