Skip to content

New ACL-based flow-filter implementation (with DPDK)#1638

Open
qmonnet wants to merge 16 commits into
mainfrom
pr/qmonnet/flow-filter-v2
Open

New ACL-based flow-filter implementation (with DPDK)#1638
qmonnet wants to merge 16 commits into
mainfrom
pr/qmonnet/flow-filter-v2

Conversation

@qmonnet

@qmonnet qmonnet commented Jul 16, 2026

Copy link
Copy Markdown
Member

This builds up from #1611 and #1636, with the following differences:

  • It's been rebased on main
  • It received a few minor fixes
  • Commits have been re-ordered
  • Daniel's changes to the flow-filter to bring DPDK have been squashed into the flow-filter creation commit
  • flofi has been renamed as flow-filter, legacy flow-filter is temporarily renamed then removed

@qmonnet
qmonnet requested a review from a team as a code owner July 16, 2026 22:26
@qmonnet qmonnet added the area/acl Related to ACLs (Access Control Lists) label Jul 16, 2026
Copilot AI review requested due to automatic review settings July 16, 2026 22:26
@qmonnet qmonnet added the area/dpdk Related to DPDK (interface with or usage of the library) label Jul 16, 2026

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 replaces the legacy flow-filter lookup structure with a new ACL-based implementation backed by DPDK rte_acl, introducing a new FlowFilterContext that is built on config application and used by the dataplane in a burst-batched lookup path.

Changes:

  • Reworked flow-filter into an ACL-table-backed FlowFilterContext (DPDK rte_acl) with burst batching and a new writer/reader API.
  • Added supporting match-action derive features (#[phantom]) and conversion utilities (prefix/port specs, FixedSize impls) to express flow-filter ACL keys/rules.
  • Wired the new context through mgmt/dataplane and introduced EAL initialization requirements for code paths that build rte_acl classifiers.

Reviewed changes

Copilot reviewed 45 out of 46 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
net/src/tcp/port.rs Adds TcpPort::{MIN,MAX} constants.
net/src/ip/mod.rs Makes NextHeader::as_u8 a const fn.
net/src/icmp4/mod.rs Adds #[repr(u8)] to Icmp4Type.
net/src/fixed_size.rs Implements FixedSize for unicast IP wrapper types via std IP types.
nat/src/test.rs Updates NAT tests to use FlowFilterContext writer/reader.
nat/src/static_nat/setup/mod.rs Adjusts static-NAT test data (IPv4 ranges).
nat/src/masquerade/test.rs Updates masquerade tests to use FlowFilterContext.
nat/Cargo.toml Updates dev-deps after config/testing changes and flow-filter refactor.
mgmt/src/tests/mgmt.rs Uses FlowFilterContextWriter and initializes EAL in tests that build rte_acl tables.
mgmt/src/processor/proc.rs Switches config application to build/store FlowFilterContext.
mgmt/Cargo.toml Adds dpdk test dependency for EAL in tests.
match-action/tests/derive_roundtrip.rs Adds tests for phantom-field handling in MatchKey derive.
match-action/src/rule.rs Adds From conversions for ExactSpec/MaskSpec.
match-action-derive/src/lib.rs Adds #[phantom] support and narrows FixedSize bounds to match fields only.
lpm/src/prefix/with_ports.rs Adds port-range conversions and helpers to build RangeSpec<u16> for ACLs.
lpm/src/prefix/ip.rs Adds conversions from IP prefixes to PrefixSpec.
lpm/Cargo.toml Adds dependency on match-action.
flow-filter/src/test_utils.rs Adds shared test utilities for building overlays and headers.
flow-filter/src/tables.rs Removes legacy nested-table flow-filter implementation.
flow-filter/src/setup.rs Removes legacy table build logic.
flow-filter/src/lib.rs Replaces stage implementation with batched ACL lookups and FlowFilterContext usage.
flow-filter/src/filter_rw.rs Removes left-right table RW in favor of slot-based context swap.
flow-filter/src/display.rs Removes legacy table display logic.
flow-filter/src/context/tests.rs Adds semantic + differential tests (reference vs dpdk backend).
flow-filter/src/context/tables.rs Implements ACL key/rule lowering and batched two-stage lookup.
flow-filter/src/context/mod.rs Defines FlowFilterContext and slot-based writer/reader handles.
flow-filter/src/context/display.rs Adds production/test rendering for context tables.
flow-filter/README.md Adds documentation for the new flow-filter design and pipeline role.
flow-filter/Cargo.toml Updates dependencies for ACL/DPDK backend and test-only reference backend.
dataplane/src/runtime.rs Initializes a minimal EAL early so config application can build rte_acl contexts.
dataplane/src/packet_processor/mod.rs Wires FlowFilterContextWriter into router setup and CLI sources.
dataplane/Cargo.toml Adds dpdk dependency for EAL init.
config/src/utils/overlap.rs Fixes L4 protocol intersection call with updated return type.
config/src/external/overlay/vpcpeering.rs Adds can_init_connection / can_receive_connection; changes nat_proto() to return by value.
config/src/external/overlay/vpc.rs Adds validation forbidding peerings with mismatched IP-version manifests; removes unsafe test-only fakes.
config/src/external/overlay/mod.rs Removes unsafe test-only fake validation helpers.
config/src/external/mod.rs Removes unsafe test-only fake validation helpers.
config/Cargo.toml Removes testing feature.
Cargo.lock Updates lockfile for new/changed dependencies.
acl/src/reference/table.rs Exposes rules() accessor for reference backend display/testing.
acl/src/reference/mod.rs Gates reference backend behind a feature (now impacts tests).
acl/src/lib.rs Gates reference backend behind a feature (now impacts tests/docs).
acl/src/dpdk/lookup.rs Makes DpdkAclLookup clonable (Arc classifier) and fixes batch result initialization.
acl/src/dpdk/dyn_table.rs Switches dyn classifier ownership to Arc.
acl/Cargo.toml Enables dpdk by default; introduces reference feature.

Comment thread nat/src/test.rs
Comment thread nat/src/masquerade/test.rs
Comment thread nat/Cargo.toml
Comment thread flow-filter/README.md
Comment thread flow-filter/README.md Outdated
Comment thread flow-filter/README.md Outdated
Comment thread mgmt/src/processor/proc.rs Outdated
Comment thread acl/src/lib.rs
Comment thread acl/src/reference/mod.rs
@qmonnet
qmonnet force-pushed the pr/qmonnet/flow-filter-v2 branch 2 times, most recently from d2d1a5b to bbbfde3 Compare July 16, 2026 22:44
@qmonnet qmonnet added ci:+release Enable VLAB release tests ci:+vlab Enable VLAB tests labels Jul 16, 2026
@qmonnet
qmonnet force-pushed the pr/qmonnet/flow-filter-v2 branch from bbbfde3 to 81e311d Compare July 17, 2026 11:43
@coderabbitai

This comment was marked as low quality.

@qmonnet
qmonnet force-pushed the pr/qmonnet/flow-filter-v2 branch from 81e311d to b184b20 Compare July 17, 2026 12:01

@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: 3

🧹 Nitpick comments (2)
acl/src/dpdk/dyn_table.rs (1)

773-877: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover batched lookup for every key shape.

lookup_batch is only exercised on RemoteKey<Ipv4Addr>. Add batch assertions for LocalKey<Ipv4Addr> and both IPv6 tables so their distinct field counts and strides are covered.

🤖 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 773 - 877, Extend the existing
lookup_batch test after the local_v4, remote_v6, and local_v6 table setup to
cover each remaining key shape. For LocalKey<Ipv4Addr>, RemoteKey<Ipv6Addr>, and
LocalKey<Ipv6Addr>, build representative key arrays including matching and
non-matching entries, call lookup_batch, and assert the outputs match the
corresponding single-lookup expectations.
flow-filter/README.md (1)

21-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Rewrite this README for the new wide-table, batched architecture.

These sections still describe per-source-VPC reference tables, VpcContext, protocol buckets, process_packet, lookup_route, and per-packet snapshot loads. The implementation now uses four global ACL tables, key fields for VPC/protocol, lookup_route_batch, and one snapshot per burst.

Also applies to: 33-34, 53-180

🤖 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 `@flow-filter/README.md` around lines 21 - 23, Rewrite the flow-filter README
to document the wide-table, batched architecture: four global ACL tables keyed
by VPC and protocol, batch lookups through lookup_route_batch, and a single
snapshot load per packet burst. Remove or replace descriptions of per-source-VPC
tables, VpcContext, protocol buckets, process_packet, lookup_route, and
per-packet snapshot loads while preserving the control-plane/data-plane and
ArcSwap context where applicable.
🤖 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 `@flow-filter/src/context/mod.rs`:
- Around line 91-94: Update the reconfiguration flow around
FlowFilterContext::store and pipeline_data.set_genid so the context publication
and generation update occur atomically or under a single synchronized operation.
Ensure packets cannot observe the new ACL tables with the old generation during
the update gap, while preserving the existing context and generation values.

In `@flow-filter/src/lib.rs`:
- Around line 262-273: Update the stale-flow validation logic near the existing
requires_masquerade and requires_port_forwarding comparisons to also compare
flow_summary.requires_static_nat_src and flow_summary.requires_static_nat_dst
against the corresponding meta requirements, invalidating on either direction
change. Add a regression test covering transition from stateful plus static-NAT
routing to stateful-only routing and verify the old flow is invalidated.

In `@match-action-derive/src/lib.rs`:
- Around line 126-143: The generic-bound logic currently scans match-field
tokens and incorrectly constrains inner type parameters such as T in Wrapper<T>.
Replace the collect_idents-based updates in the generics setup with a FixedSize
predicate for each complete match field type, using the field type symbol from
the match-field generation path, and remove the token-scan approach. Preserve
author-defined bounds and ensure wrapper types are accepted based on the
wrapper’s FixedSize implementation.

---

Nitpick comments:
In `@acl/src/dpdk/dyn_table.rs`:
- Around line 773-877: Extend the existing lookup_batch test after the local_v4,
remote_v6, and local_v6 table setup to cover each remaining key shape. For
LocalKey<Ipv4Addr>, RemoteKey<Ipv6Addr>, and LocalKey<Ipv6Addr>, build
representative key arrays including matching and non-matching entries, call
lookup_batch, and assert the outputs match the corresponding single-lookup
expectations.

In `@flow-filter/README.md`:
- Around line 21-23: Rewrite the flow-filter README to document the wide-table,
batched architecture: four global ACL tables keyed by VPC and protocol, batch
lookups through lookup_route_batch, and a single snapshot load per packet burst.
Remove or replace descriptions of per-source-VPC tables, VpcContext, protocol
buckets, process_packet, lookup_route, and per-packet snapshot loads while
preserving the control-plane/data-plane and ArcSwap context where applicable.
🪄 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

Run ID: 3a4dec45-530e-475a-9346-3eaa4f86c8b4

📥 Commits

Reviewing files that changed from the base of the PR and between fcb9dd8 and b184b20.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (49)
  • Cargo.toml
  • acl/Cargo.toml
  • acl/benches/table_build.rs
  • acl/src/dpdk/dyn_table.rs
  • acl/src/dpdk/lookup.rs
  • acl/src/lib.rs
  • acl/src/reference/mod.rs
  • acl/src/reference/table.rs
  • acl/tests/property_dyn_shape.rs
  • acl/tests/property_predicate.rs
  • config/Cargo.toml
  • config/src/external/mod.rs
  • config/src/external/overlay/mod.rs
  • config/src/external/overlay/vpc.rs
  • config/src/external/overlay/vpcpeering.rs
  • config/src/utils/overlap.rs
  • dataplane/Cargo.toml
  • dataplane/src/packet_processor/mod.rs
  • dataplane/src/runtime.rs
  • flow-filter/Cargo.toml
  • flow-filter/README.md
  • flow-filter/src/context/display.rs
  • flow-filter/src/context/mod.rs
  • flow-filter/src/context/tables.rs
  • flow-filter/src/context/tests.rs
  • flow-filter/src/display.rs
  • flow-filter/src/filter_rw.rs
  • flow-filter/src/lib.rs
  • flow-filter/src/setup.rs
  • flow-filter/src/tables.rs
  • flow-filter/src/test_utils.rs
  • flow-filter/src/tests.rs
  • lpm/Cargo.toml
  • lpm/src/prefix/ip.rs
  • lpm/src/prefix/with_ports.rs
  • match-action-derive/src/lib.rs
  • match-action/src/rule.rs
  • match-action/tests/derive_roundtrip.rs
  • mgmt/Cargo.toml
  • mgmt/src/processor/proc.rs
  • mgmt/src/tests/mgmt.rs
  • nat/Cargo.toml
  • nat/src/masquerade/test.rs
  • nat/src/static_nat/setup/mod.rs
  • nat/src/test.rs
  • net/src/fixed_size.rs
  • net/src/icmp4/mod.rs
  • net/src/ip/mod.rs
  • net/src/tcp/port.rs
💤 Files with no reviewable changes (7)
  • flow-filter/src/setup.rs
  • flow-filter/src/filter_rw.rs
  • config/Cargo.toml
  • flow-filter/src/display.rs
  • config/src/external/mod.rs
  • config/src/external/overlay/mod.rs
  • flow-filter/src/tables.rs

Comment thread flow-filter/src/context/mod.rs
Comment thread flow-filter/src/lib.rs
Comment thread match-action-derive/src/lib.rs Outdated
@qmonnet
qmonnet force-pushed the pr/qmonnet/flow-filter-v2 branch from b184b20 to 7f26ea9 Compare July 17, 2026 16:55
@qmonnet
qmonnet force-pushed the pr/qmonnet/flow-filter-v2 branch from 7f26ea9 to b9ef7b0 Compare July 17, 2026 16:58
Copilot AI review requested due to automatic review settings July 21, 2026 13:41
@qmonnet
qmonnet force-pushed the pr/qmonnet/flow-filter-v2 branch from b9ef7b0 to 58b3876 Compare July 21, 2026 13:41

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

Copilot reviewed 41 out of 42 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

flow-filter/README.md:83

confidence: 8
tags: [docs]

This section describes protocol bucketing into separate `tcp`/`udp`/`other` tables, but the new implementation encodes protocol as a (masked) key field in the ACL tables instead. This is misleading for readers trying to map the docs to `context/tables.rs`.

Inside each end, rules are bucketed by L4 protocol so lookups are protocol-aware:

  • tcp / udp tables: keyed on prefix + port range (TwoTuple).
  • other table: keyed on prefix only (Singleton) -- used for ICMP etc.
  • An expose with no protocol restriction (L4Protocol::Any) is fanned out into all three.
**flow-filter/README.md:136**
* ```yaml
confidence: 9
tags: [docs]

The code no longer has a FlowFilter::process_packet method (the data-path is now burst-oriented via process_burst). The heading should be updated so the symbol reference is accurate.

## 4. The per-packet path (`FlowFilter::process_packet`)

Comment thread config/src/external/overlay/vpc.rs Outdated
Comment thread flow-filter/README.md Outdated
qmonnet and others added 4 commits July 21, 2026 15:15
We don't support peerings with two manifests using different IP versions
for the prefixes in their expose blocks. We should reject it at
validation time.

We already validate that all expose blocks in a given manifest use the
same IP version for their prefixes, so there's no additional
verification required on that side.

Update a test in nat crate where we use both IP versions in a peering.

Also remove helpers is_v4_or_default() and is_v6_or_default(): they are
unused, we kept them for a previous version of this commit but we don't
use them, in the end.

Signed-off-by: Quentin Monnet <qmo@qmon.net>
Add helpers to see if the NAT mode for an expose block allows it to
initiate and/or be at the receiving end of a connection. We'll use this
later to rework the flow-filter implementation.

Signed-off-by: Quentin Monnet <qmo@qmon.net>
This will help us make the code simpler later when reusing this method.

Signed-off-by: Quentin Monnet <qmo@qmon.net>
Add a bitmask field kind (value + mask) to the MatchKey derive and its
companion *Rule type, alongside exact/prefix/range. A field matches when
input & mask == value & mask, so a full mask is an exact match and a
zero mask is a wildcard - letting a single key field express "any"
without fanning rules across separate tables.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Signed-off-by: Quentin Monnet <qmo@qmon.net>
daniel-noland and others added 11 commits July 21, 2026 15:16
Add some trait implementations and some constants so that header field
types can be packed in to ACL key bytes. This includes:

- for net: #[repr(u8)] on Icmp4Type, TcpPort::MIN/MAX, and FixedSize
- for lpm: From prefix/port for RangeSpec<u16>

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Signed-off-by: Quentin Monnet <qmo@qmon.net>
We recently added a freature for the reference backend in the "acl"
crate. We had the commit doing that in two different branches, one for
user ACLs (which got merged first) and one for the new flow-filter
implementation. The "less clean" version, regarding the comments, got
merged; in particular, one comment in acl/src/dpdk/dyn_table.rs is not
where it should be (not strictly above the import directive it comments
on). Let's address it.

We also have some comments mentioning "flofi", which was a working name
for the new flow-filter implementation, but the comments made it through
cherry-picking into the user ACLs branch. Let's clean them up, too.

Fixes: 43e5930 ("feat(acl): Add feature for reference backend, Arc-share DpdkAclLookup")
Fixes: 9415a2b ("feat(dataplane,mgmt): Init EAL early for the ACL filter rte_acl backend")
Signed-off-by: Quentin Monnet <qmo@qmon.net>
Add two tests for the dyn_table module.

Test flow_filter_two_table_key_shapes_build_and_classify() proves an
rte_acl key can carry a masked proto byte, one or two exact u32 VNI
fields, and v4/v6 prefix + port range, and that single and batched
lookups classify correctly.

Test flow_filter_build_capacity_probe() (skipped by default)
characterizes build cost for disjoint vs overlapping rule sets, to
inform a config-layer rule-count cap.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Signed-off-by: Quentin Monnet <qmo@qmon.net>
In preparation for a new flow-filter implementation, make room for the
new version of the crate by renaming the existing flow-filter crate as
flow-filter-legacy. No functional change.

Signed-off-by: Quentin Monnet <qmo@qmon.net>
This is a new, simpler version of the flow-filter.

One of the main changes is that the logic has been simplified, because
we can get rid of the logic regarding the "multiple matches" used by the
legacy implementation.

Another essential change is the use of ACL tables for the lookups,
relying on DPDK as a backend, with support for batch processing. Routing
is a two-stage lookup (destination -> peer + dst NAT, then source -> src
NAT); a default expose lowers to a lowest-priority /0 rule so
longest-prefix-match handles it. Function process() collects each rx
burst and runs the lookups in batched rte_acl calls, partitioning by IP
version without reordering the output. The rte_acl backend is used in
production, while a linear-scan reference backend is used for tests.

The new stage is not plugged into the datapath pipeline, yet.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Daniel Noland <daniel@githedgehog.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Signed-off-by: Quentin Monnet <qmo@qmon.net>
Under the loom backend, concurrency::sync::atomic::{AtomicU32,
AtomicU64}::new is not const: each instance registers with the loom
executor, so it cannot initialize a "static". This makes "cargo check
--features loom --workspace --tests --benches" fail with E0015 ("cannot
call non-const associated function ... in statics").

This is a consequence of a recent commit where we flipped acl's default
features from [] to ["dpdk"]. Every atomic static involved lives behind
'feature = "dpdk"' -- the integration tests and bench module, and the
lib's #[cfg(all(test, feature = "dpdk"))] modules. With dpdk off by
default they were compiled out and never seen by the loom backend;
turning dpdk on by default brings them into every build, including the
loom one.

Wrap each in LazyLock<Atomic..> so the initializer runs on first use
instead of at compile time. The value is still the backend atomic, so
"fetch_add" stays loom-instrumented; only the const requirement on
construction is removed. These are process-unique name/sequence counters
on cold paths, so the one-time lazy init is negligible, and on every
non-loom backend LazyLock merely wraps an otherwise-const atomic.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Quentin Monnet <qmo@qmon.net>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Daniel Noland <daniel@githedgehog.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Signed-off-by: Quentin Monnet <qmo@qmon.net>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Daniel Noland <daniel@githedgehog.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Signed-off-by: Quentin Monnet <qmo@qmon.net>
Swap the legacy flow-filter stage for its new, ACL-based implementation
in dataplane, in the mgmt crate, and when used in nat tests.

Remove one NAT (masquerade) test that now fails: it relied on a
non-valid configuration anyway, and is no longer relevant with the new
flow-filter implementation.

We do not delete the legacy implementation just yet.

Signed-off-by: Quentin Monnet <qmo@qmon.net>
Now that we've changed the flow-filter implementation and that it relies
on DPDK (rte_acl), pulling dpdk-sys in the build, we can no longer run
the NAT tests relying on it with Miri. Where a simpler mock version is
used instead (TestFlowFilter), we can keep using Miri.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Quentin Monnet <qmo@qmon.net>
Remove the now deprecated legacy flow-filter implementation. Along with
it, we get rid of the "testing" feature for the config crate, and of the
related "fake_validated...()" helpers from the same crate.

Signed-off-by: Quentin Monnet <qmo@qmon.net>
Copilot AI review requested due to automatic review settings July 21, 2026 14:17
@qmonnet
qmonnet force-pushed the pr/qmonnet/flow-filter-v2 branch from 58b3876 to 8bc10eb Compare July 21, 2026 14:17

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

Copilot reviewed 41 out of 42 changed files in this pull request and generated 1 comment.

Comment thread match-action/src/rule.rs
@qmonnet qmonnet self-assigned this Jul 21, 2026
The MatchKey derive scanned the tokens of each match field's type and
added a "FixedSize" bound to every generic type parameter whose name
appeared in them. But the generated code never requires the parameter to
be "FixedSize": it requires the field's type to be, since that is what
it calls "SIZE" and "write_be" on.

The two only coincide when the field is spelled as a bare parameter. A
field of type "Wrapper<T>", with "Wrapper<T>: FixedSize" holding for
every T, gets rejected: the derive demands "T: FixedSize", a constraint
the generated code has no use for, so the key cannot be instantiated
with a tag type that is not itself "FixedSize". This is the usual
"perfect derive" pitfall, the same reason "#[derive(Clone)]" over-bounds
its parameters.

Bound each match field's type directly, with one where-predicate per
field, and drop the token scan. This is exactly what the expansion
needs, so it accepts wrappers while staying no more permissive than the
generated code demands. It also keeps phantom-only parameters unbounded
by construction rather than by name matching, since a parameter that no
match field mentions appears in no predicate. The predicates reach the
rule struct as well, whose spec fields bound their own parameter.

Raised by CodeRabbit.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Quentin Monnet <qmo@qmon.net>
Copilot AI review requested due to automatic review settings July 21, 2026 16:34

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

Copilot reviewed 41 out of 42 changed files in this pull request and generated 1 comment.

Comment thread match-action-derive/src/lib.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/acl Related to ACLs (Access Control Lists) area/dpdk Related to DPDK (interface with or usage of the library) ci:+cross/full ci:+release Enable VLAB release tests ci:+vlab Enable VLAB tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants