Pr/daniel noland/user acls dpdk rework#1639
Closed
daniel-noland wants to merge 15 commits into
Closed
Conversation
This is so we can use one protocol, or "any protocol" (by ranging on all existing values) in ACL tables to apply filtering rules to packets. Signed-off-by: Quentin Monnet <qmo@qmon.net>
No need to clone the enum variants. Signed-off-by: Quentin Monnet <qmo@qmon.net>
Add a way to return the ValidatedAcl object attached to a ValidatedPeering, if any. We'll use it to process the ACL rules in a future commit. Signed-off-by: Quentin Monnet <qmo@qmon.net>
There's no point in allowing an empty list in the first place: remove the ACL list (to allow all), or the peering altogether (to deny all). This will also avoid the corner case of an empty list when building the context for the ACLs. Signed-off-by: Quentin Monnet <qmo@qmon.net>
ICMP doesn't really deserve any specific treatment internally, as we can trivially implement it with AclProtoMatch::Other(1). Signed-off-by: Quentin Monnet <qmo@qmon.net>
User can specify manual protocol values in the CRD. When they specify 6 or 17, this means TCP or UDP, respectively; but we have dedicated AclProtoMatch enum variants for these protocols. Convert the relevant numerical value into the TCP/UDP variants. Signed-off-by: Quentin Monnet <qmo@qmon.net>
Add a new crate containing the implementation for the user ACLs: list of access control rules that allow or deny specific packets or flow. We recently introduced support for parsing and validating user ACL objects; The new pipeline stage builds up context tables from validated ACLs, and then apply the rules on coming packets. With the "scope: flow" option, reply traffic for authorized flows is automatically allowed as well (unless another explicit rule denies it). This mode should be extended to all NAT modes (including no-NAT) in the future, but is only supported with masquerade and port forwarding at this time. Signed-off-by: Quentin Monnet <qmo@qmon.net>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Quentin Monnet <qmo@qmon.net>
Add unit tests to validate the behaviour of the new ACL filter pipeline stage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Quentin Monnet <qmo@qmon.net>
Add a pipeline stage with the recently-added ACL filter to the main pipeline, so that users can enforce ACLs on the traffic. Signed-off-by: Quentin Monnet <qmo@qmon.net>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
daniel-noland
force-pushed
the
pr/daniel-noland/user-acls-dpdk-rework
branch
from
July 17, 2026 10:02
435cfeb to
36c41f7
Compare
…Lookup Add a non-default `reference` cargo feature and gate the linear-scan reference backend behind it, so production builds link only the rte_acl backend. Share `DpdkAclLookup` via `Arc<dyn DynClassifier>` and derive `Clone` so a built classifier can be cheaply shared across pipeline workers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
The ACL filter builds rte_acl classifiers when configuration is applied, which needs the EAL memory subsystem up. Initialize a minimal EAL (no hugepages / no PCI) early in `dataplane::main`, before `run_mgmt` applies the first config; the guard is held for the process. The real DPDK datapath driver must eventually take over EAL ownership (there is one `rte_eal_init` per process). The mgmt config-processor test starts the EAL for the same reason. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
…e per IP version rte_acl requires the first key field to be a single byte, but the tuple keys led with a u32 VNI. Restructure to a single `AclKey<I>` per IP version whose first field is a `#[mask]` protocol byte: it satisfies the one-byte-first-field rule and carries the protocol match in the key (exact via mask 0xff, any via mask 0x00). This collapses the six per-protocol tables (tcp/udp/other x v4/v6) into two (v4/v6) and removes the duplication of every `Any` rule across protocols. All rules for an IP version now live in one ordered table, so first-match precedence holds across protocol kinds, not just within a protocol's table. Config validation forbids ports on non-TCP/UDP protocols, so those rules always carry a wildcard port range. Lower `AclProtoMatch` to `MaskSpec<u8>` (replacing the now-unused RangeSpec conversion). Rules are still lowered via the backend-neutral `Erased` path; this only changes the key shape so it is rte_acl-installable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Replace the reference (linear-scan) tables with rte_acl `DpdkAclLookup` tables in
production, keeping the reference backend as a cfg(test) differential oracle.
- Introduce `AnyTable { Empty, Dpdk, Reference }` per IP version: `Empty` avoids
building a zero-rule rte_acl context, `Dpdk` is production, `Reference` is the
test/opt-in oracle. Rules are lowered once to backend-neutral predicates and the
selected `Backend` consumes them.
- Positional first-match: rte_acl returns the highest-priority match, so rules are
installed with priority descending by insertion index (rule 0 wins). This
reproduces the reference backend's first-match-on-insertion-order precedence.
- Fallible build threads through `ConfigError`; `TryFrom` uses the dpdk backend
(EAL initialised early in dataplane::main), and `for_test`/`for_test_dpdk` select
a backend for tests.
- Each rte_acl context gets a process-unique name (a hot-swap briefly keeps the old
and new contexts alive).
- Display shows per-table rule counts in production (opaque classifier) and the
full per-rule dump in test builds (reference backend retains the rules).
The semantic suite runs on the reference backend (fast, EAL-free); a new
`#[dpdk::with_eal]` differential test builds the same overlay on both backends and
asserts identical verdicts, proving the rte_acl path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
daniel-noland
force-pushed
the
pr/daniel-noland/user-acls-dpdk-rework
branch
from
July 17, 2026 10:20
36c41f7 to
b69b634
Compare
daniel-noland
marked this pull request as ready for review
July 17, 2026 10:22
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>
qmonnet
force-pushed
the
pr/daniel-noland/user-acls-dpdk-rework
branch
from
July 17, 2026 11:48
5a7b396 to
fba60ac
Compare
Member
|
This passes CI! 🎉 although I'll squash the acl-filter changes into my own commits, in the initial PR. |
qmonnet
force-pushed
the
pr/qmonnet/user-acls
branch
from
July 17, 2026 15:57
b63f9f3 to
ee23168
Compare
qmonnet
force-pushed
the
pr/qmonnet/user-acls
branch
from
July 20, 2026 12:58
6f5a544 to
6bf22e6
Compare
qmonnet
marked this pull request as draft
July 20, 2026 16:33
Member
|
Closing in favour of #1634 |
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
2 participants
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.