(ignore for now) acl design test#1567
Draft
daniel-noland wants to merge 28 commits into
Draft
Conversation
daniel-noland
changed the base branch from
main
to
pr/daniel-noland/threading-rewrite
daniel-noland
force-pushed
the
pr/daniel-noland/acl-pat-design
branch
from
d241ddf to
40c7472
Compare
Sketch of the field-tracking design: HList accumulates per-field type info as `.add_field` advances; the resulting `Table<L, A>` is parameterised by the FieldList instead of by the underlying `AclContext`'s const-generic N. N is held behind a trait-object-erased `dyn ErasedAclContext`, with a closed match arm-table dispatching `1..=12` to the right `AclContext<N>` instantiation at construction time. Status: experimental. No rule installation yet -- `Table::lookup` always returns None until a typed-rule path lands. The three unit tests exercise the type machinery only (no EAL required) and validate that: - each `add_field` advances the type parameter as expected - `FieldList::N` counts fields correctly - `write_field_defs` assigns `field_index` in HList order Drive-by: drop a pre-existing unused `use std::borrow::Borrow` in `cascade/src/lookup.rs`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the typestate match-action sketch (deleted in the same
commit) with a derive-driven design. A user writes a struct of
predicate-wrapped fields:
#[derive(MatchKey)]
struct FiveTuple {
proto: Exact<u8>,
src_ip: Prefix<Ipv4Addr>,
dst_ip: Prefix<Ipv4Addr>,
src_port: Range<u16>,
dst_port: Range<u16>,
}
and the derive emits:
- const N: usize -- field count
- const KEY_SIZE: usize -- summed field widths (chained from
`<FieldType as MatchField>::SIZE`)
- field_specs() -- static layout: kind / size / offset /
name per field, in declaration order
- as_key_into(out) -- big-endian-pack each field at its
offset
The key win over the typestate version: backends never see an
HList, never need const-generic erasure, never N-dispatch. The
struct definition is the universal description; per-backend
translation lives in the backend's crate. Same source struct
can program a DPDK rte_acl context, a future rte_flow rule, a
future tc-flower netlink message, or a HashMap-keyed test stub.
DPDK-specific layout rules (first field 1 byte at offset 0,
input_index grouping, padding synthesis) belong in the DPDK ACL
backend's translation layer -- not here. Other backends won't
have those constraints.
Two new crates:
- dataplane-match-action: types + traits. No DPDK dep; no
backend coupling. `derive` feature re-exports the proc macro
(default-on).
- dataplane-match-action-derive: the proc macro. Uses
proc-macro-crate to resolve the consumer's import name so
workspace consumers (alias `match-action`) and external
consumers (canonical `dataplane-match-action`) both work.
Roundtrip test exercises N, KEY_SIZE, field_specs, and packing
for a 5-tuple-shaped struct plus a 1-field degenerate case.
Drive-by: `just fmt` collapsed two-line `let mut ctx = ...`
bindings in dpdk/src/acl/mod.rs after the prior turbofish cleanup
shortened the lines.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Translates a backend-agnostic `&[match_action::FieldSpec]` into a
DPDK-shaped `DpdkLayout`: a `Vec<FieldDef>` (user fields plus any
synthesized padding fields), the total `stride`, and a
`user_to_dpdk` permutation for the eventual lookup-time packer.
Encodes three DPDK quirks that the `match-action` vocabulary
deliberately doesn't know about:
- `input_index 0` is reserved for the 1-byte first field alone;
DPDK reads its full 4-byte window but only matches byte 0.
No padding belongs in group 0 -- the rte_acl validator
rejects extra fields there.
- Subsequent `input_index >= 1` groups must each cover exactly
4 bytes. Short groups get padded with 1-byte `Bitmask`
fields; rule installation (TBD) splices wildcard `(0, 0)`
into those slots so they always match.
- Field sizes are 1/2/4 bytes only; we reject 3 explicitly.
Tests validate each planned layout against `AclBuildConfig::new()`
itself -- the same validator the production rule-install path will
hit -- so layout bugs surface at unit-test time rather than at
runtime.
Coverage: 5-tuple, all-4B layout, three 2-byte fields (forces a
final group with padding), single 1-byte field, short final
group, the three error paths (empty, non-1-byte first field,
3-byte field).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Drops the four wrapper types (Prefix<T>, Mask<T>, Range<T>,
Exact<T>) and the MatchField trait. Field kinds move to per-field
attributes; field values are bare value types.
#[derive(MatchKey)]
struct FiveTuple {
#[exact] proto: IpProto,
#[prefix] src_ip: Ipv4Addr,
#[prefix] dst_ip: Ipv4Addr,
#[range] src_port: u16,
#[range] dst_port: u16,
}
The previous design conflated lookup-time data (a single packet's
field value) with rule-time data (the per-field bounds: prefix
length, mask, range bounds). That conflation broke `Range<T>`:
a packet has one value, not a range, so I had stapled a `value:
T` onto `Range<T> { value, min, max }` and offered a `Range::at(v)`
constructor that abused it. The TODO in field.rs called this out.
The new design separates concerns cleanly:
- The struct holds lookup-time values only. No `value/min/max`
or `addr/len` pairing -- just the bare field value.
- The kind (prefix / mask / range / exact) is metadata on the
type, encoded via the attribute -- read by the derive,
available to backends via FieldSpec::kind.
- Rule construction (which still needs prefix lengths, masks,
range bounds) becomes a separate API consuming the same
MatchKey shape. Not built yet -- but we can design it from
a clean slate without the wrapper-type artifacts.
Also adds an inherent `as_key()` returning `[u8; KEY_SIZE]`
directly. The trait method's `[u8; Self::KEY_SIZE]` form would
require `generic_const_exprs`, but in the derive's inherent impl
block `Self` is concrete, so `<Self as MatchKey>::KEY_SIZE`
resolves to a literal and the array return works on stable.
Generic code still uses `MatchKey::as_key_into(&mut [u8])`;
concrete user code gets the ergonomic array return.
Roundtrip test updated:
- Uses bare value types (IpProto newtype + Ipv4Addr + u16) with
`#[exact]`/`#[prefix]`/`#[range]` attributes
- Exercises both `as_key_into` (trait) and `as_key` (inherent)
- Demonstrates a user-defined newtype (IpProto) plugging in via
just impl FixedSize
The DPDK layout planner (`acl::dpdk_layout`) is unchanged --
it consumes `&[FieldSpec]` whose shape didn't change.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
End-to-end path from a `#[derive(MatchKey)]` struct to a concrete
`DpdkAclLookup<N, STRIDE, A>` type, on stable Rust, with no
`generic_const_exprs` and no trait-object erasure:
#[derive(MatchKey)]
struct FiveTuple {
#[exact] proto: IpProto,
#[prefix] src_ip: Ipv4Addr,
#[prefix] dst_ip: Ipv4Addr,
#[range] src_port: u16,
#[range] dst_port: u16,
}
acl::dpdk_table_alias!(pub type FiveTupleTable<Verdict> = FiveTuple);
// -> type alias for DpdkAclLookup<5, 16, Verdict>
Three pieces compose:
1. The MatchKey derive emits an inherent associated const
`FIELD_SPECS: &'static [FieldSpec]` (the trait method
`field_specs()` is implemented as `Self::FIELD_SPECS`). Trait
methods can't be const-fn on stable; an inherent const is the
escape hatch that lets const-fn callers reach the specs.
2. `acl::dpdk_layout::const_extents(specs)` is a const fn that
returns `(N, STRIDE)` for the DPDK-shaped layout (post-padding).
Mirrors `plan_layout`'s sizing decisions without allocating;
three tests assert it agrees with `plan_layout`'s `(n, stride)`
for every shape `plan_layout` is tested against.
3. `acl::dpdk_table_alias!` is a function-style macro that wraps
the const-fn call inline at the type-alias site. The two
`{ const_extents(...).<i> }` const expressions in const-generic
position are concrete (no generic parameters), so stable Rust
accepts them.
Test coverage:
- const_extents matches plan_layout for the planner's existing
test shapes
- const_extents works in a const item (`const EXTENTS: (usize,
usize) = const_extents(...)`)
- the inherent const is observably the same slice as
`MatchKey::field_specs()`
- the manually-typed alias and the macro-emitted alias produce
identical `type_name`s, both `DpdkAclLookup<5, 16, Verdict>`
Drive-by: the MatchKey trait's docstring referenced a vanished
`MatchField` trait; updated to describe the attribute-based shape.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
User code provides one AclField per MatchKey field (in declaration order); the DPDK layout may include synthesised padding fields the user shouldn't have to think about. `splice_user_fields_to_dpdk` takes the user-ordered Vec<AclField> + the planner's DpdkLayout and emits a DPDK-ordered `[AclField; N]` with wildcard `(value=0, mask=0)` 1-byte Bitmask `AclField`s in every padding slot. `RuleSpec<K, A>` is the user-side rule type: priority, category mask, user-ordered AclFields (length pinned to K::N at construction), and the action. `userdata` is intentionally not exposed -- the install path assigns it from the rule's position in the action table. Five tests against the splice logic, no EAL required: - new() rejects user_fields.len() != K::N - 5-tuple identity-permutes (no padding -> user_to_dpdk = [0..5]) - padded final-group layout puts wildcards at indices 4, 5 - splice rejects wrong N const generic - splice rejects wrong user-field count This is the load-bearing logic for rule installation; the EAL-bound install function that actually calls `add_rules` + `build` on a real AclContext can land next once we plumb in EAL test setup. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
End-to-end install path that ties together the previous slices:
let rules: Vec<RuleSpec<FiveTuple, Verdict>> = ...;
let table: FiveTupleTable<Verdict> = install_table(
"my_acl",
NonZero::new(1024).unwrap(),
1, // num_categories
rules,
)?;
The function:
1. plan_layout(K::field_specs()) -- DPDK-shaped FieldDefs
2. validates N / STRIDE const generics agree with the planner
3. AclBuildConfig<N>::new + AclContext<N>::new
4. per rule: splice user fields + assign 1-based userdata + Rule<N>::new
5. ctx.add_rules + ctx.build -> AclContext<N, Built<N>>
6. DpdkAclLookup::<N, STRIDE, A>::new
N and STRIDE come from `dpdk_table_alias!` in normal use; callers
writing them by hand get a runtime InstallError::WrongN /
WrongStride on mismatch.
InstallError unifies every error source on the path (LayoutError,
SpliceError, InvalidAclBuildConfig, InvalidAclName, AclCreateError,
AclAddRulesError, AclBuildFailure-as-string, StrideTooSmall) with
`From` impls so the function body is `?`-driven.
The function compiles without EAL but its runtime call requires
EAL because `AclContext::new` reaches into DPDK. No integration
test in this commit -- EAL test plumbing is its own concern. The
type machinery is exercised by the existing
`match_key_table_alias` test (which constructs the type but
doesn't build it).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
12 small helpers wrapping `rte_acl`'s reuse of the (value, mask) slot for each match flavor: exact_uN(value) -> Bitmask with full-ones mask prefix_uN(value, len) -> Mask with prefix length in mask slot mask_uN(value, mask) -> Mask, slots pass through range_uN(min, max) -> Range with bounds in (value, mask) (Three sizes -- u8, u16, u32 -- by four kinds.) User code building a RuleSpec doesn't need to remember which DPDK slot means what for each kind. For custom value types (Ipv4Addr, IpProto newtypes, ...) the caller does the obvious width conversion at the call site -- `prefix_u32(u32::from(addr), 24)` etc. A typed rule-builder per K that uses the field's `FieldKind` to pick the right helper automatically is the natural next step. Each helper has a one-liner round-trip test against the raw `AclField::from_uN` it wraps. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
End-to-end usage snippet at the crate root: declare a MatchKey struct, alias the table type via dpdk_table_alias!, build a RuleSpec via the per-kind helpers, install. Marked ignore because install_table needs a live EAL, but the snippet does compile through the same crate paths a real consumer would use, so changes that break the API surface will show up as typecheck errors on the doctest harvest (when we enable it later under an EAL test feature). Also reframes the crate-level doc from "ACL-shaped building blocks" (vague) to "DPDK rte_acl backend for match_action::MatchKey tables" (what this crate actually is now). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ruct
Two pieces:
1. `match-action/src/rule.rs` adds the four rule-construction
wrapper types -- ExactSpec<T>, PrefixSpec<T>, MaskSpec<T>,
RangeSpec<T> -- each carrying the predicate data for its kind
(value / value+len / value+mask / min+max). Each implements
a `RuleField` trait exposing its `KIND` and `Value` so
backends can dispatch generically. Backend-agnostic; lives
in match-action.
2. The `MatchKey` derive now emits a parallel `<KeyName>Rule`
struct alongside the MatchKey impl. Each `#[exact]` /
`#[prefix]` / `#[mask]` / `#[range]` field becomes a field
of type `ExactSpec<T>` / `PrefixSpec<T>` / `MaskSpec<T>` /
`RangeSpec<T>` in the parallel struct, in declaration order.
Inherits the user struct's visibility; derives Copy + Clone +
Debug. Lives outside the `const _ = { ... }` wrapper so it's
a real top-level type.
Test coverage:
- constructs a `FiveTupleRule` via struct literal and reads back
each wrapper's fields
- checks each wrapper's `KIND` const lines up with the original
`#[attr]`
Next iteration: an acl-side adapter that converts a `<K>Rule` into
a `Vec<AclField>` suitable for `RuleSpec`. That's the
DPDK-coupled half; this commit deliberately stops at the
backend-agnostic shape.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
End-to-end implementation of option (c) from the design discussion:
typed rule construction with backend-extensibility, zero per-rule-struct
boilerplate.
Three pieces compose:
1. `match-action::Backend` (marker trait with associated `Field`) +
`match-action::IntoBackendField<B>` (per-spec convert-to-wire).
Backend-agnostic; both live in match-action.
2. The MatchKey derive now emits, on the parallel `<Name>Rule`
struct, a generic method
`into_backend_fields::<B: Backend>(self) -> Vec<B::Field>`
with per-field where-clauses pinning each wrapped spec type to
the backend's `IntoBackendField` impl. No per-struct user
boilerplate.
3. The acl crate adds `Dpdk` (backend marker), `AclWord` (per-T
trait: as_u32 + ACL_SIZE), and four generic
`IntoBackendField<Dpdk>` impls (one per spec kind) that
size-dispatch on `T::ACL_SIZE` to call the right per-kind
`AclField` constructor. `AclWord` impls cover u8/u16/u32/Ipv4Addr;
user newtypes need a single one-line impl per type.
User-facing surface end-to-end:
#[derive(MatchKey)]
struct FiveTuple { #[exact] proto: IpProto, #[prefix] src_ip: Ipv4Addr, ... }
impl AclWord for IpProto { ... } // one-line per newtype
let rule = FiveTupleRule {
proto: ExactSpec::new(IpProto(6)),
src_ip: PrefixSpec::new(addr, 24),
...
};
let acl_rule = RuleSpec::new(
priority, mask,
rule.into_backend_fields::<Dpdk>(), // <- the new method
action,
)?;
Tests:
- match-action: rule struct construction + kind invariants
- acl: `rule_to_acl_fields` integration test verifies each field
converts to the expected AclField via the per-kind helpers, and
the converted Vec flows straight into RuleSpec::new
The `as u8` / `as u16` truncation casts in the dispatch impls are
intentional (slot width comes from `ACL_SIZE`, upper bits are
zero per the `AclWord` contract); annotated with
`#[allow(clippy::cast_possible_truncation)]` per impl.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Updates the crate-level doctest to reflect the new backend-extensibility surface: the user builds a `FiveTupleRule` struct literal and calls `.into_backend_fields::<Dpdk>()` instead of constructing a `Vec<AclField>` with per-kind helpers. The per-kind helpers (`exact_u8`, `prefix_u32`, etc.) still exist for callers who want to skip the rule struct, but the typed path is the preferred surface. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three pieces wire up an end-to-end EAL-integrated test for the
ACL framework:
1. `dpdk::test_support::start_eal()` -- a `OnceLock`-backed lazy
initialiser for a `--no-huge --no-pci --in-memory --no-telemetry`
EAL with lcore 0 floated across the process's allowed CPUs.
Moved out of dpdk's private `acl::tests` module into the crate
root behind a new `test` feature; the dpdk crate's own acl
tests now use it.
2. `dpdk-test-macros` (new proc-macro crate) provides
`#[with_eal]`, an attribute macro that inserts
`let _eal = ::dpdk::test_support::start_eal();` at the top of
a `#[test]` function body. Uses `proc-macro-crate` to resolve
the consumer's import name for `dataplane-dpdk` (works as
`dpdk::with_eal` under the workspace alias, or
`dataplane_dpdk::with_eal` for external consumers).
3. `acl::dpdk_lookup::dpdk_key_bytes::<K, STRIDE>(&key)` -- bridges
the user-layout bytes from `MatchKey::as_key_into` to the
DPDK-layout bytes the underlying context reads. Heap-allocates
a scratch buffer because `K::KEY_SIZE` can't reach a
const-generic position without `generic_const_exprs`; acceptable
for tests / one-shot lookups, a hot-path version is future work.
End-to-end EAL test (acl/tests/eal_install_classify.rs):
#[dpdk::with_eal]
#[test]
fn install_one_rule_and_classify() {
// Build a FiveTupleRule, install via install_table, classify
// hit + miss + miss-port packets against a real rte_acl
// context. This is the first test that actually goes through
// DPDK rather than just exercising type/layout machinery.
}
Cargo plumbing:
- dpdk: `test` feature gates `dep:id`, `dep:nix`, `dep:dpdk-test-macros`
- dpdk dev-deps: self-reference with `features = ["test"]` so dpdk's
own tests get the test-support surface
- acl dev-deps: override the regular `dpdk` dep to enable `test`
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two ergonomic wins on the lookup side:
1. `DpdkAclLookup<N, STRIDE, A>` now implements `Lookup<K, A>` for
any `K: MatchKey`. Users call `table.lookup(&packet)` with their
`MatchKey`-derived struct directly; the table packs the user
fields into DPDK-layout bytes via a stored `DpdkLayout` and
issues `rte_acl_classify`. No heap allocations on the hot path.
2. The byte-array path moves from a `Lookup<[u8; STRIDE], A>` impl
to an inherent `lookup_via_bytes(&[u8; STRIDE])` method. Rust's
coherence checker can't rule out a future `MatchKey` impl for
`[u8; STRIDE]`, so the two `Lookup<...>` impls conflict; the
inherent method keeps the escape hatch without the conflict.
Mechanics:
- `DpdkAclLookup` now stores `layout: DpdkLayout`. `new()` takes
it as a third arg; `install_table` passes it. Adds a `layout()`
getter for callers who want to do their own packing.
- `MAX_USER_KEY_BYTES = 256` workspace constant: stack-allocated
scratch buffer for the user-layout intermediate, sized to cover
`RTE_ACL_MAX_FIELDS * sizeof(u32)`. Eliminates the per-call
`vec![0u8; K::KEY_SIZE]` allocation in `dpdk_key_bytes` and the
new `Lookup<K, A>` path.
- `pack_user_to_dpdk_stack` factored out so both code paths
(free function `dpdk_key_bytes` and the trait impl) share the
packing logic.
EAL integration test cleaned up: `table.lookup(&FiveTuple { ... })`
instead of the previous `dpdk_key_bytes` round-trip.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`DpdkLayout::field_defs` and `user_to_dpdk` switch from `Vec<T>` to `ArrayVec<T, MAX_FIELDS>` (where `MAX_FIELDS = 64` is rte_acl's hard cap on field count). Both fields are sized by the field count, so the cap is the natural bound. Net effect on the hot path: `DpdkAclLookup` stores its `DpdkLayout` inline -- no heap-allocated `Vec` indirections. `plan_layout` itself still runs at construction time and is allocation-free for the layout result (`ArrayVec` is stack-stored). `plan_layout` validates `specs.len() <= MAX_FIELDS` up front; `try_push` errors convert to `LayoutError::TooManyFieldDefs` for the synthesised padding case. Internal helper `push_def` wraps the try_push + map_err pattern so the main planning loop stays readable. `install_table`'s `[FieldDef; N]` construction switches from `Vec::try_into` to `core::array::from_fn(|i| layout.field_defs[i])` -- the `N == layout.field_defs.len()` check above the `from_fn` guarantees in-bounds indexing, and the from_fn doesn't need a heap roundtrip. Test assertions updated: `layout.user_to_dpdk.as_slice()` instead of equating to `vec![...]` since ArrayVec and Vec don't share PartialEq. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`DpdkAclLookup::lookup_batch` packs a slice of `K: MatchKey` values
into per-key `[u8; STRIDE]` buffers and feeds them to a single
`rte_acl_classify` call -- the SIMD-friendly path rte_acl is
designed for.
All scratch (per-key buffers, pointer array, results array) lives
on the stack via `ArrayVec` bounded by `MAX_BATCH = 32` -- a
typical NIC `rx_burst` size. Bigger batches must be sliced.
Worst-case stack: `MAX_BATCH * STRIDE` bytes for buffers plus
small pointer/result vectors; well under any reasonable thread
stack.
API:
pub fn lookup_batch<'a, K: MatchKey>(
&'a self,
keys: &[K],
out: &mut [Option<&'a A>],
) -> Result<(), BatchError>
`out.len() == keys.len()` is enforced; mismatch returns
`BatchError::OutputLenMismatch`. Each `out[i]` ends up either
`Some(&action)` or `None`, matching what `lookup(&keys[i])` would
produce per-item.
EAL integration test extended with a 3-element batch covering the
hit + two misses; the batch results agree with the single-shot
classifications already verified above.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The full composition story: a real `HeadersView<(&Eth, &Ipv4, &Tcp)>`
constructed via `HeaderStack`, wrapped in a `V4TcpSource` newtype
that implements `Projection<FiveTuple>`, passed to
`Lookup::classify` on a real DPDK-backed `FiveTupleTable<Verdict>`.
`classify` runs the projection (`V4TcpSource -> FiveTuple`) and the
lookup (`FiveTuple -> Option<&Verdict>`) as a single call. No
byte-shuffling at the call site, no manual rte_acl bookkeeping --
the cascade traits drive everything.
Two tests:
- `classify_real_packet_via_projection` -- builds a TCP packet,
runs the cascade-style classify, asserts hit/miss against the
installed `10.0.0.0/8 + dport 22 -> Drop` rule.
- `non_tcp_packet_cannot_be_projected_to_five_tuple` -- a UDP
packet's `Headers::as_view::<(&Eth, &Ipv4, &Tcp)>` returns
`None`, so the `V4TcpSource` can't even be constructed.
Type-level shape guard, documented at the source.
This rounds out the architecture:
Headers (raw bytes)
-> as_view<S> (typed packet shape, net crate)
-> Projection<K> (user picks the key from the view)
-> Lookup<K, A> (DpdkAclLookup wraps rte_acl + stored layout)
-> rte_acl_classify (DPDK)
-> Option<&A> (the user's action)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- match-action: document why MatchKey::field_specs / as_key_into and FixedSize::write_be take slices rather than Self::N / Self::SIZE arrays (generic_const_exprs wall; sized forms live on inherent impls where Self is concrete). For write_be the slice is also the better shape regardless -- callers pack into offset windows of a larger buffer. - eal_install_classify: drop the inner FiveTuple/Verdict that shadowed the module-level types (leftover from a hand-crafted illustration); the test now uses the same types the dpdk_table_alias! references. Two-rule priority-precedence demonstration, single-shot + batch. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two derive enhancements: 1. A field with no match-flavor attribute defaults to #[exact] (was: hard error). The most common / most restrictive kind, and the mistake of forgetting #[prefix] surfaces at rule construction (the rule struct gets an ExactSpec field with nowhere to put a prefix length), not silently at classify time. 2. `#[derive(MatchKey)]` now works on generic structs -- e.g. `FiveTuple<Addr, Port>` instantiated as v4/v6 or tcp/udp. This is the path to a growing variant set (inner/outer encap addrs, stacked VLANs, SRv6, ...). The enabling refactor: replace the free `const OFF_i` offset items (which couldn't see the impl's type params) with inline size / boundary expressions used in associated-const and per-field positions. `KEY_SIZE` is a sum expression; `FIELD_SPECS` is a generic-dependent associated const (rustc promotes a per-monomorphization static); `as_key_into` writes each field into `out[boundary[i]..boundary[i+1]]`. None of this needs generic_const_exprs. The derive adds a `FixedSize` bound to every type param (like `#[derive(Clone)]` bounds `T: Clone`), and the rule struct is declared with those bounded generics so its std Copy/Clone/Debug derives inherit the bound and satisfy the `*Spec<Addr>` fields. The sized inherent `as_key() -> [u8; KEY_SIZE]` is emitted only for non-generic structs -- its return type would need generic_const_exprs once KEY_SIZE depends on a param. Generic keys use `as_key_into(&mut [u8])` (and the table's `Lookup<K, A>`, which packs internally). Tests: default-exact two-field struct; `TwoTuple<Addr>` at Ipv4Addr (KEY_SIZE 8) and Ipv6Addr (KEY_SIZE 32) with as_key_into packing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Lets callers write `(80..=8080).into()` instead of
`RangeSpec::new(80, 8080)`. Only the inclusive `..=` form is
supported -- `RangeSpec` is inclusive `[min, max]` (matching
rte_acl), so `RangeInclusive` maps 1:1, while the half-open
`Range` (`a..b`) would need an off-by-one that underflows at 0 and
turns the empty `a..a` into an invalid `{a, a-1}`.
Keeps the two-field struct rather than adopting RangeInclusive
itself: the std type isn't Copy and has awkward accessors.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
These fields are read only within the acl crate and are built via public constructors (plan_layout, RuleSpec::new). Narrowing keeps the RuleSpec user_fields.len() == K::N invariant from being mutated out from under the constructor and removes the open "should this be pub?" TODOs on DpdkLayout. Cross-crate-consumed types (the *Spec wrappers, FieldSpec) and diagnostic error payloads stay pub. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
LayoutError, SpliceError, BatchError, StrideTooSmall, DpdkKeyError and InstallError all hand-rolled their Display/Error/From mechanics (or lacked Display/Error entirely). Switch to #[derive(thiserror::Error)] with #[error(...)] messages and #[from] on the wrapping variants, matching the dpdk crate's convention and dropping the manual impls. thiserror is gated behind the dpdk feature since every error-bearing module is. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The 11 acl lifecycle tests each opened with `let _eal = start_eal();`, the exact boilerplate the #[with_eal] attribute macro was built to inject. Replace each with #[with_eal] above #[test] (resolves inside the crate via the self dev-dependency that enables the `test` feature), drop the now-unused start_eal import, and refresh the module docs to describe the macro-driven EAL setup. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The Itself branch emitted `::dataplane_dpdk`, which only resolves inside the crate's own tests because the self dev-dependency injects that name into the extern prelude. Emit `crate` instead, the conventional handling: `crate::test_support` resolves natively from within the crate, so the expansion no longer leans on the crate being reachable by its own name. Verified Itself is the branch taken for the crate's in-tree unit tests; external consumers still hit the unchanged Name branch (`::dpdk`). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The crate is currently nothing but a wrapper/extension of dpdk's acl
mod, with install/layout/lookup/rule as flat `dpdk_`-prefixed
top-level modules. Move them under a single `dpdk` module so the
public paths read `dpdk::{install,layout,lookup,rule}` and the crate
has room for a future non-DPDK backend alongside it.
Mechanical: `git mv` the four files (history preserved), drop the
`dpdk_` prefix, repoint intra-crate paths to `crate::dpdk::*`, and
update the `dpdk_table_alias!` macro body, the worked example, and the
consumers in tests. One real consequence: the new crate-root `dpdk`
module shadows the extern `dpdk` crate in path resolution, so the
crate-level doc link now reads `::dpdk::acl::AclContext`.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add `acl::reference`: a linear-scan software classifier that shares the rule vocabulary (the `*Spec` wrappers + Backend/IntoBackendField) via a new `Reference` backend marker, and implements `cascade::Lookup` so it is a drop-in alongside the DPDK backend. It serves goal 1 of the reference-backend plan -- a differential oracle for `rte_acl` -- and is shaped to later serve as the fast mutable front of a cascade over a slow backend (empty table => cheap miss). Priority/overlap mechanics are deliberately trivial and non-prejudicial to the pending framework-wide priority model: precedence is positional (first match wins), there is no numeric-priority convention, and the decision-free `matches()` primitive returns every match. The differential harness (tests/reference_vs_dpdk.rs, bolero + EAL) asserts reference == DPDK only when at most one rule matches, so it validates per-field predicate semantics without depending on any priority convention. Rule-derived boundary probes hit each predicate edge (range endpoints, prefix edges, exact near-misses) -- verified to catch an inclusive/exclusive range bug that independent random packets miss. A coverage guard fails the run if it ever goes vacuous. Adding a second IntoBackendField impl makes the bare `.into_backend_fields()` calls ambiguous, so the existing EAL test now spells `::<Dpdk>`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
daniel-noland
force-pushed
the
pr/daniel-noland/acl-pat-design
branch
from
40c7472 to
8a65a29
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
1 participant
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.
No description provided.