From 06e8d4e7931f60ec686fd87f370cc4f2fb139f56 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Tue, 30 Jun 2026 17:37:16 +0100 Subject: [PATCH 01/17] feat(lpm): Implement From<...> for RangeSpec for types with ports In preparation for using port ranges with ACL tables, implement trait From to convert types with ports into RangeSpec elements. Signed-off-by: Quentin Monnet --- Cargo.lock | 1 + lpm/Cargo.toml | 1 + lpm/src/prefix/ip.rs | 13 +++++++++++++ lpm/src/prefix/with_ports.rs | 29 +++++++++++++++++++++++++++++ 4 files changed, 44 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 423e33294f..815e921930 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1571,6 +1571,7 @@ version = "0.22.0" dependencies = [ "bnum", "bolero", + "dataplane-match-action", "ipnet", "num-traits", "prefix-trie", diff --git a/lpm/Cargo.toml b/lpm/Cargo.toml index 9ac771981e..07ba003fed 100644 --- a/lpm/Cargo.toml +++ b/lpm/Cargo.toml @@ -13,6 +13,7 @@ testing = [] bnum = { workspace = true } bolero = { workspace = true, optional = true } ipnet = { workspace = true, features = ["serde"] } +match-action = { workspace = true } num-traits = { workspace = true } prefix-trie = { workspace = true } serde = { workspace = true, features = ["derive"] } diff --git a/lpm/src/prefix/ip.rs b/lpm/src/prefix/ip.rs index ff709a7ab7..1e31ecdc76 100644 --- a/lpm/src/prefix/ip.rs +++ b/lpm/src/prefix/ip.rs @@ -7,6 +7,7 @@ use std::str::FromStr; use crate::prefix::{Prefix, PrefixError, PrefixSize}; use ipnet::{Ipv4Net, Ipv6Net}; +use match_action::rule::PrefixSpec; use num_traits::{CheckedShr, PrimInt, Unsigned, Zero}; pub trait Representable { @@ -235,6 +236,12 @@ impl TryFrom for Ipv4Prefix { } } +impl From for PrefixSpec { + fn from(prefix: Ipv4Prefix) -> Self { + PrefixSpec::new(prefix.network(), prefix.len()) + } +} + //////////////////////////////////////////////////////////// // IPv6 Prefix //////////////////////////////////////////////////////////// @@ -371,6 +378,12 @@ impl TryFrom for Ipv6Prefix { } } +impl From for PrefixSpec { + fn from(prefix: Ipv6Prefix) -> Self { + PrefixSpec::new(prefix.network(), prefix.len()) + } +} + #[cfg(any(test, feature = "bolero"))] mod contract { use crate::prefix::{IpPrefix, Ipv4Prefix, Ipv6Prefix}; diff --git a/lpm/src/prefix/with_ports.rs b/lpm/src/prefix/with_ports.rs index e4efee4d78..44179d3ba1 100644 --- a/lpm/src/prefix/with_ports.rs +++ b/lpm/src/prefix/with_ports.rs @@ -5,6 +5,7 @@ use crate::prefix::range_map::UpperBoundFrom; use crate::prefix::{Prefix, PrefixSize}; use bnum::cast::CastFrom; use bnum::{n, t}; +use match_action::RangeSpec; use std::collections::BTreeSet; use std::fmt::Display; use std::ops::{Bound, RangeBounds}; @@ -571,6 +572,34 @@ impl RangeBounds for PortRange { } } +// Build RangeSpec from relevant types - This is used with ACLs + +const PORT_RANGE_WILDCARD: RangeSpec = RangeSpec::new(0, u16::MAX); + +impl From for RangeSpec { + fn from(range: PortRange) -> Self { + RangeSpec::new(range.start, range.end) + } +} + +impl From<&Prefix> for RangeSpec { + fn from(_: &Prefix) -> Self { + PORT_RANGE_WILDCARD + } +} + +impl From<&PrefixWithPorts> for RangeSpec { + fn from(prefix_with_ports: &PrefixWithPorts) -> Self { + prefix_with_ports.ports().into() + } +} + +impl From<&PrefixWithOptionalPorts> for RangeSpec { + fn from(pwop: &PrefixWithOptionalPorts) -> Self { + (&PrefixWithPorts::from(*pwop)).into() + } +} + impl FromStr for PortRange { type Err = PortRangeError; From 190f051c95b2868b1390820be7ab4d5b277dc228 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Wed, 15 Jul 2026 14:01:47 +0100 Subject: [PATCH 02/17] feat(config): Implement From for MaskSpec This is so we can use one protocol, or "any protocol" (by using a mask) in ACL tables to apply filtering rules to packets. Co-authored-by: Daniel Noland Signed-off-by: Quentin Monnet --- Cargo.lock | 1 + config/Cargo.toml | 3 ++- config/src/external/overlay/acl.rs | 21 +++++++++++++++++++++ net/src/ip/mod.rs | 2 +- 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 815e921930..93775c47c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1316,6 +1316,7 @@ dependencies = [ "dataplane-concurrency", "dataplane-k8s-intf", "dataplane-lpm", + "dataplane-match-action", "dataplane-net", "dataplane-pipeline", "dataplane-tracectl", diff --git a/config/Cargo.toml b/config/Cargo.toml index 4a64689ba8..44563653b4 100644 --- a/config/Cargo.toml +++ b/config/Cargo.toml @@ -15,8 +15,9 @@ testing = [] common = { workspace = true } concurrency = { workspace = true } k8s-intf = { workspace = true } -net = { workspace = true } lpm = { workspace = true } +match-action = { workspace = true } +net = { workspace = true } # external arc-swap = { workspace = true } diff --git a/config/src/external/overlay/acl.rs b/config/src/external/overlay/acl.rs index 14ff4a36ef..59aff05756 100644 --- a/config/src/external/overlay/acl.rs +++ b/config/src/external/overlay/acl.rs @@ -7,6 +7,8 @@ use super::vpcpeering::ValidatedManifest; use crate::ConfigError; use crate::utils::normalize; use lpm::prefix::{PortRange, Prefix, PrefixPortsSet, PrefixWithOptionalPorts}; +use match_action::MaskSpec; +use net::ip::NextHeader; use tracing::debug; #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] @@ -26,6 +28,25 @@ pub enum AclProtoMatch { Any, } +const TCP: u8 = NextHeader::TCP.as_u8(); +const UDP: u8 = NextHeader::UDP.as_u8(); + +/// Lower a protocol match to a `(value, mask)` bitmask predicate on the 1-byte protocol key field. +/// A specific protocol matches exactly (`mask 0xff`); `Any` wildcards the field (`mask 0x00`), so a +/// single key field expresses "any protocol" without fanning rules across per-protocol tables. This +/// is also what lets the protocol byte be the rte_acl-mandated 1-byte first field (a `#[mask]` byte +/// lowers to the same `Bitmask` field type as `#[exact]`). +impl From for MaskSpec { + fn from(proto: AclProtoMatch) -> Self { + match proto { + AclProtoMatch::Tcp => MaskSpec::new(TCP, u8::MAX), + AclProtoMatch::Udp => MaskSpec::new(UDP, u8::MAX), + AclProtoMatch::Other(p) => MaskSpec::new(p, u8::MAX), + AclProtoMatch::Any => MaskSpec::new(0, 0), + } + } +} + #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct AclPattern { pub src: PrefixPortsSet, diff --git a/net/src/ip/mod.rs b/net/src/ip/mod.rs index 4fa4afd548..0eb40b03b4 100644 --- a/net/src/ip/mod.rs +++ b/net/src/ip/mod.rs @@ -68,7 +68,7 @@ impl NextHeader { /// Return the [`NextHeader`] represented as a `u8` #[must_use] - pub fn as_u8(&self) -> u8 { + pub const fn as_u8(&self) -> u8 { self.0.0 } From 01fd377db5df20b406fd7eda77fbd0b1e1768acf Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Mon, 13 Jul 2026 10:53:54 +0100 Subject: [PATCH 03/17] chore(config): Derive Copy for AclProtoMatch No need to clone the enum variants. Signed-off-by: Quentin Monnet --- config/src/external/overlay/acl.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config/src/external/overlay/acl.rs b/config/src/external/overlay/acl.rs index 59aff05756..28d5d76b9c 100644 --- a/config/src/external/overlay/acl.rs +++ b/config/src/external/overlay/acl.rs @@ -18,7 +18,7 @@ pub enum AclAction { Deny, } -#[derive(Debug, Clone, PartialEq, Eq, Default)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum AclProtoMatch { Tcp, Udp, @@ -132,8 +132,8 @@ impl ValidatedAclPattern { } #[must_use] - pub fn proto(&self) -> &AclProtoMatch { - &self.proto + pub fn proto(&self) -> AclProtoMatch { + self.proto } } From cbff5b7c8087a98679b501da40fcc5d3c6422d81 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Mon, 13 Jul 2026 10:54:36 +0100 Subject: [PATCH 04/17] feat(config): Add acl() method to ValidatedPeering 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 --- config/src/external/overlay/vpc.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/config/src/external/overlay/vpc.rs b/config/src/external/overlay/vpc.rs index 41685753fb..7f5bae8baf 100644 --- a/config/src/external/overlay/vpc.rs +++ b/config/src/external/overlay/vpc.rs @@ -141,6 +141,11 @@ impl ValidatedPeering { &self.gwgroup } + #[must_use] + pub fn acl(&self) -> &Option { + &self.acl + } + #[must_use] pub fn is_v4(&self) -> bool { // This is a validated object, we checked at validation time that both manifests use the From 07b12b3cf70175644485a98840b070d24b013333 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Mon, 13 Jul 2026 15:51:23 +0100 Subject: [PATCH 05/17] feat(config): Reject empty ACL lists 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 --- config/src/external/overlay/acl.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/config/src/external/overlay/acl.rs b/config/src/external/overlay/acl.rs index 28d5d76b9c..49dcfdd12a 100644 --- a/config/src/external/overlay/acl.rs +++ b/config/src/external/overlay/acl.rs @@ -426,6 +426,11 @@ impl Acl { manifest_left: &ValidatedManifest, manifest_right: &ValidatedManifest, ) -> Result { + if self.rules.is_empty() { + return Err(ConfigError::InvalidAcl( + "ACL list must contain at least one rule".to_string(), + )); + } self.validate_rules_names()?; let rules = self .rules @@ -638,6 +643,25 @@ mod validation_tests { ); } + // ============================================================================================= + // Empty ACL is rejected + // ============================================================================================= + + // Empty ACL is rejected + #[test] + fn test_acl_empty_list_rejected() { + let (left, right) = manifests(); + let acl = Acl { + default: AclAction::Deny, + rules: vec![], + }; + let result = acl.validate(&left, &right); + assert!( + matches!(result, Err(ConfigError::InvalidAcl(_))), + "{result:?}" + ); + } + // ============================================================================================= // Rule name validation (at the ACL level) // ============================================================================================= From a698e027b0e70775a20e173925b4589af033153b Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Tue, 14 Jul 2026 15:18:36 +0100 Subject: [PATCH 06/17] chore(config): Remove AclProtoMatch::Icmp variant ICMP doesn't really deserve any specific treatment internally, as we can trivially implement it with AclProtoMatch::Other(1). Signed-off-by: Quentin Monnet --- config/src/external/overlay/acl.rs | 33 ++---------------------------- 1 file changed, 2 insertions(+), 31 deletions(-) diff --git a/config/src/external/overlay/acl.rs b/config/src/external/overlay/acl.rs index 49dcfdd12a..2c733777ce 100644 --- a/config/src/external/overlay/acl.rs +++ b/config/src/external/overlay/acl.rs @@ -22,7 +22,6 @@ pub enum AclAction { pub enum AclProtoMatch { Tcp, Udp, - Icmp, Other(u8), #[default] Any, @@ -81,7 +80,7 @@ impl AclPattern { fn validate_ports(&self) -> bool { match self.proto { AclProtoMatch::Tcp | AclProtoMatch::Udp => true, - AclProtoMatch::Other(_) | AclProtoMatch::Icmp | AclProtoMatch::Any => { + AclProtoMatch::Other(_) | AclProtoMatch::Any => { !self.src.uses_ports() && !self.dst.uses_ports() && self.src_any_ports.is_empty() @@ -727,22 +726,6 @@ mod validation_tests { assert!(validate_rule(rule).is_ok()); } - // ICMP with port matching is rejected (ICMP has no ports) - #[test] - fn test_acl_icmp_with_ports_rejected() { - let p = pattern( - prefixes(&["10.0.0.0/24"]), - [prefix_with_ports("10.1.0.0/24", 443, 443)].into(), - AclProtoMatch::Icmp, - ); - let rule = rule("r", "VPC-1", "VPC-2", AclAction::Allow, p); - let result = validate_rule(rule); - assert!( - matches!(result, Err(ConfigError::InvalidAcl(_))), - "{result:?}" - ); - } - // A numeric protocol with port matching is rejected #[test] fn test_acl_other_proto_with_ports_rejected() { @@ -775,18 +758,6 @@ mod validation_tests { ); } - // ICMP without ports passes - #[test] - fn test_acl_icmp_without_ports_passes() { - let p = pattern( - prefixes(&["10.0.0.0/24"]), - prefixes(&["10.1.0.0/24"]), - AclProtoMatch::Icmp, - ); - let rule = rule("r", "VPC-1", "VPC-2", AclAction::Allow, p); - assert!(validate_rule(rule).is_ok()); - } - // Unknown protocol without ports passes #[test] fn test_acl_other_proto_without_ports_passes() { @@ -968,7 +939,7 @@ mod validation_tests { let mut p = pattern( PrefixPortsSet::new(), PrefixPortsSet::new(), - AclProtoMatch::Icmp, + AclProtoMatch::Other(1), ); p.src_any_ports = vec![PortRange::new(443, 443).unwrap()]; let icmp_rule = rule("r", "VPC-1", "VPC-2", AclAction::Allow, p.clone()); From 7bfbac82da1de565bf57e2b4d549b57179c5d768 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Wed, 15 Jul 2026 14:04:41 +0100 Subject: [PATCH 07/17] feat(config): For ACLs, normalise AclProtoMatch(6|17) as TCP/UDP 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 --- config/src/converters/k8s/config/acl.rs | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/config/src/converters/k8s/config/acl.rs b/config/src/converters/k8s/config/acl.rs index 5d140411d0..05d658c488 100644 --- a/config/src/converters/k8s/config/acl.rs +++ b/config/src/converters/k8s/config/acl.rs @@ -8,6 +8,7 @@ use k8s_intf::gateway_agent_crd::{ GatewayAgentPeeringsAclRulesScope, }; use lpm::prefix::{PortRange, Prefix, PrefixPortsSet, PrefixWithOptionalPorts}; +use net::ip::NextHeader; use crate::converters::k8s::FromK8sConversionError; use crate::converters::k8s::config::expose::parse_port_ranges; @@ -45,11 +46,17 @@ fn parse_proto(proto: Option<&str>) -> Result Ok(AclProtoMatch::Tcp), "udp" => Ok(AclProtoMatch::Udp), - other => other.parse::().map(AclProtoMatch::Other).map_err(|_| { - FromK8sConversionError::InvalidData(format!( - "ACL protocol '{other}': expected \"tcp\", \"udp\", or a numeric protocol" - )) - }), + other => { + let number = other.parse::(); + match number { + Ok(n) if n == NextHeader::TCP.as_u8() => Ok(AclProtoMatch::Tcp), + Ok(n) if n == NextHeader::UDP.as_u8() => Ok(AclProtoMatch::Udp), + Ok(n) => Ok(AclProtoMatch::Other(n)), + Err(_) => Err(FromK8sConversionError::InvalidData(format!( + "ACL protocol '{other}': expected \"tcp\", \"udp\", or a numeric protocol" + ))), + } + } } } @@ -367,6 +374,10 @@ mod test { assert_eq!(parse_proto(Some("tcp")).unwrap(), AclProtoMatch::Tcp); assert_eq!(parse_proto(Some("udp")).unwrap(), AclProtoMatch::Udp); assert_eq!(parse_proto(Some("47")).unwrap(), AclProtoMatch::Other(47)); + // Numeric TCP/UDP protocol numbers normalize to the dedicated variants (not Other), so + // they route to the TCP/UDP tables in the dataplane rather than the generic proto table. + assert_eq!(parse_proto(Some("6")).unwrap(), AclProtoMatch::Tcp); + assert_eq!(parse_proto(Some("17")).unwrap(), AclProtoMatch::Udp); assert!(parse_proto(Some("bogus")).is_err()); // "icmp" is not supported yet assert!(parse_proto(Some("icmp")).is_err()); From f2b44eac918dc7322ff8f1a27560357a68a5fd0c Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Tue, 14 Jul 2026 01:05:33 -0600 Subject: [PATCH 08/17] feat(acl): Add feature for reference backend, Arc-share DpdkAclLookup 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 and derive Clone so a built classifier can be cheaply shared across pipeline workers. Co-authored-by: Claude Opus 4.8 (1M context) Signed-off-by: Daniel Noland Signed-off-by: Quentin Monnet --- Cargo.lock | 1 + acl/Cargo.toml | 7 ++++++- acl/src/dpdk/dyn_table.rs | 13 +++++++++---- acl/src/dpdk/lookup.rs | 16 +++++++++++----- acl/src/lib.rs | 1 + acl/src/reference/mod.rs | 2 ++ 6 files changed, 30 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 93775c47c3..ec95b9bc94 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1226,6 +1226,7 @@ dependencies = [ "arrayvec", "bolero", "criterion", + "dataplane-acl", "dataplane-concurrency", "dataplane-dpdk", "dataplane-lookup", diff --git a/acl/Cargo.toml b/acl/Cargo.toml index 65fd4a54a7..acef7a0dd6 100644 --- a/acl/Cargo.toml +++ b/acl/Cargo.toml @@ -6,8 +6,9 @@ publish.workspace = true version.workspace = true [features] -default = [] +default = ["dpdk"] dpdk = ["dep:dpdk"] +reference = [] [dependencies] arrayvec = { workspace = true, default-features = true } @@ -19,6 +20,10 @@ net = { workspace = true, features = [] } thiserror = { workspace = true } [dev-dependencies] +# Enable the `reference` backend for this crate's own tests and benches. It is a non-default +# feature (production links only the rte_acl backend), but the integration tests and benches +# differential-test against it, so make it available whenever test/bench targets are built. +dataplane-acl = { path = ".", features = ["reference"] } bolero = { workspace = true, features = ["std"] } criterion = { workspace = true, features = ["cargo_bench_support"] } dpdk = { workspace = true, features = ["test"] } diff --git a/acl/src/dpdk/dyn_table.rs b/acl/src/dpdk/dyn_table.rs index 1353872c9c..3b2c56f844 100644 --- a/acl/src/dpdk/dyn_table.rs +++ b/acl/src/dpdk/dyn_table.rs @@ -2,7 +2,12 @@ // Copyright Open Network Fabric Authors #![allow(unsafe_code)] +// `std::sync::Arc` (not `concurrency::sync::Arc`) is required: the built classifier is coerced to +// `Arc` below, and loom/shuttle's `Arc` does not implement `CoerceUnsized`. The +// classifier is immutable and shared read-only, so it is not a model-checked synchronization point. +// The suppression is a trailing comment so it stays on the `std::sync` line if rustfmt reorders. use core::num::NonZero; +use std::sync::Arc; // nosemgrep: rust-no-direct-std-sync-import use dpdk::acl::{ AclAddRulesError, AclBuildConfig, AclBuildFailure, AclContext, AclCreateError, AclCreateParams, @@ -259,7 +264,7 @@ pub(crate) fn dispatch_build_classifier( layout: &DpdkLayout, rules: Vec>, max_rules: NonZero, -) -> Result<(Box, Vec), DynInstallError> { +) -> Result<(Arc, Vec), DynInstallError> { const _: () = assert!( MAX_FIELDS == 64, "MAX_FIELDS changed; regenerate the dispatch_match literal list", @@ -291,7 +296,7 @@ fn build_classifier_n( layout: &DpdkLayout, rules: Vec>, max_rules: NonZero, -) -> Result<(Box, Vec), DynInstallError> { +) -> Result<(Arc, Vec), DynInstallError> { debug_assert_eq!(N, layout.field_defs.len()); let field_defs: [FieldDef; N] = core::array::from_fn(|i| layout.field_defs[i]); let build_cfg = AclBuildConfig::::new(1, field_defs, 0)?; @@ -327,12 +332,12 @@ fn build_classifier_n( }); } - Ok((Box::new(built), actions)) + Ok((Arc::new(built), actions)) } #[allow(dead_code)] const _PAD: fn() -> AclField = padding_field; pub struct DynDpdkLookup { - classifier: Box, + classifier: Arc, actions: Vec, layout: DpdkLayout, user_field_sizes: Vec, diff --git a/acl/src/dpdk/lookup.rs b/acl/src/dpdk/lookup.rs index 42f54d3a57..ea24475c37 100644 --- a/acl/src/dpdk/lookup.rs +++ b/acl/src/dpdk/lookup.rs @@ -4,6 +4,13 @@ use core::marker::PhantomData; +// The classifier is immutable after build and shared read-only across pipeline workers; it is not +// a synchronization point that the concurrency model checkers need to observe. It must be +// `std::sync::Arc` (not `concurrency::sync::Arc`) because loom/shuttle's `Arc` does not implement +// `CoerceUnsized`, so `Arc::new(concrete)` cannot coerce to `Arc` under those +// backends. See `acl/src/dpdk/dyn_table.rs` for the coercion site. +use std::sync::Arc; // nosemgrep: rust-no-direct-std-sync-import + use arrayvec::ArrayVec; use lookup::Lookup; use match_action::MatchKey; @@ -26,11 +33,12 @@ const ZEROS: [u8; MAX_USER_KEY_BYTES] = [0u8; MAX_USER_KEY_BYTES]; /// packed at the runtime `layout.stride`, so a single `DpdkAclLookup` /// covers any key shape -- including generic keys such as /// `DpdkAclLookup, A>`. +#[derive(Clone)] pub struct DpdkAclLookup where K: MatchKey, { - classifier: Box, + classifier: Arc, actions: Vec, layout: DpdkLayout, _key: PhantomData, @@ -50,7 +58,7 @@ where /// calls below sound (see `development/code/unsafe-code.md` -- `unsafe` used /// only to build a safe abstraction with local reasoning). pub fn new( - classifier: Box, + classifier: Arc, actions: Vec, layout: DpdkLayout, ) -> Result { @@ -114,9 +122,7 @@ where for i in 0..keys.len() { let _ = ptrs.try_push(arena[i * stride..].as_ptr()); } - let mut results: ArrayVec = ArrayVec::new(); - let mut results: ArrayVec = - std::std::iter::repeat_n(0, keys.len()).collect(); + let mut results: ArrayVec = std::iter::repeat_n(0, keys.len()).collect(); // SAFETY: every pointer addresses `stride >= min_input_size` valid // bytes (invariant established in `new`), packed contiguously above. unsafe { diff --git a/acl/src/lib.rs b/acl/src/lib.rs index f82d821948..789d2ae7e7 100644 --- a/acl/src/lib.rs +++ b/acl/src/lib.rs @@ -26,6 +26,7 @@ #[cfg(feature = "dpdk")] pub mod dpdk; +#[cfg(feature = "reference")] pub mod reference; #[cfg(feature = "dpdk")] #[macro_export] diff --git a/acl/src/reference/mod.rs b/acl/src/reference/mod.rs index 0fd27fea59..b67e5dc9ec 100644 --- a/acl/src/reference/mod.rs +++ b/acl/src/reference/mod.rs @@ -1,6 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright Open Network Fabric Authors +#![cfg(feature = "reference")] + pub mod dyn_table; pub mod table; From d44dae30e36c0b186176eb93194410b583b23275 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Mon, 13 Jul 2026 11:00:15 +0100 Subject: [PATCH 09/17] feat(acl-filter): Add and implement new ACL filter crate 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. Internally, the new stage uses ACL structures (match-action) backed by DPDK (rte_acl). Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Daniel Noland Signed-off-by: Daniel Noland Signed-off-by: Quentin Monnet --- Cargo.lock | 17 + Cargo.toml | 7 + acl-filter/Cargo.toml | 29 ++ acl-filter/src/access.rs | 106 ++++++ acl-filter/src/context.rs | 503 +++++++++++++++++++++++++++++ acl-filter/src/lib.rs | 207 ++++++++++++ acl/src/reference/table.rs | 6 + config/src/external/overlay/acl.rs | 12 + lpm/src/prefix/with_ports.rs | 2 +- 9 files changed, 888 insertions(+), 1 deletion(-) create mode 100644 acl-filter/Cargo.toml create mode 100644 acl-filter/src/access.rs create mode 100644 acl-filter/src/context.rs create mode 100644 acl-filter/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index ec95b9bc94..4ba6a9b32b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1235,6 +1235,23 @@ dependencies = [ "thiserror", ] +[[package]] +name = "dataplane-acl-filter" +version = "0.22.0" +dependencies = [ + "dataplane-acl", + "dataplane-concurrency", + "dataplane-config", + "dataplane-dpdk", + "dataplane-lookup", + "dataplane-lpm", + "dataplane-match-action", + "dataplane-net", + "dataplane-pipeline", + "tracing", + "tracing-test", +] + [[package]] name = "dataplane-args" version = "0.22.0" diff --git a/Cargo.toml b/Cargo.toml index fcbe00a30d..930464ee9b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,6 +2,7 @@ members = [ "acl", + "acl-filter", "args", "cli", "common", @@ -66,6 +67,7 @@ repository = "https://github.com/githedgehog/dataplane/" # Internal acl = { path = "./acl", package = "dataplane-acl", features = [] } +acl-filter = { path = "./acl-filter", package = "dataplane-acl-filter", features = [] } args = { path = "./args", package = "dataplane-args", features = [] } cli = { path = "./cli", package = "dataplane-cli", features = [] } common = { path = "./common", package = "dataplane-common", features = [] } @@ -273,6 +275,11 @@ package = "dataplane-acl" miri = false # hopeless + pointless wasm = false # hopeless + pointless +[workspace.metadata.package.acl-filter] +package = "dataplane-acl-filter" +miri = false # hopeless + pointless +wasm = false # hopeless + pointless + [workspace.metadata.package.args] package = "dataplane-args" miri = true diff --git a/acl-filter/Cargo.toml b/acl-filter/Cargo.toml new file mode 100644 index 0000000000..2f95cf0ac6 --- /dev/null +++ b/acl-filter/Cargo.toml @@ -0,0 +1,29 @@ +[package] +name = "dataplane-acl-filter" +edition.workspace = true +license.workspace = true +publish.workspace = true +version.workspace = true + +[dependencies] +acl = { workspace = true } +concurrency = { workspace = true } +config = { workspace = true } +dpdk = { workspace = true } +lookup = { workspace = true } +lpm = { workspace = true } +match-action = { workspace = true, features = ["derive"] } +net = { workspace = true } +pipeline = { workspace = true } +tracing = { workspace = true } + +[dev-dependencies] +# The reference (linear-scan) ACL backend is the differential-test oracle and drives the fast, +# EAL-free semantic suite. It is `cfg(test)`-gated in the source, so it is never part of a +# production build; this dev-dep just makes `acl::reference` available to test builds. +acl = { workspace = true, features = ["reference"] } +config = { workspace = true, features = ["testing"] } +dpdk = { workspace = true, features = ["test"] } +lpm = { workspace = true, features = ["testing"] } +net = { workspace = true, features = ["builder"] } +tracing-test = { workspace = true, features = [] } diff --git a/acl-filter/src/access.rs b/acl-filter/src/access.rs new file mode 100644 index 0000000000..72554bf7d9 --- /dev/null +++ b/acl-filter/src/access.rs @@ -0,0 +1,106 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Read and write handles for the ACL filter context. + +use crate::context::{AclTables, PRODUCTION_BACKEND}; +use concurrency::slot::Slot; +use concurrency::sync::Arc; +use config::ConfigError; +use config::external::overlay::ValidatedOverlay; + +#[derive(Debug, Default)] +pub struct AclFilterContext { + pub(super) acls: AclTables, +} + +impl TryFrom<&ValidatedOverlay> for AclFilterContext { + type Error = ConfigError; + + fn try_from(overlay: &ValidatedOverlay) -> Result { + let acls = AclTables::build(overlay, PRODUCTION_BACKEND)?; + Ok(Self { acls }) + } +} + +#[cfg(test)] +impl AclFilterContext { + /// Build a context using the reference backend, for tests that want the fast, EAL-free oracle. + /// Production goes through [`TryFrom`], which uses the rte_acl backend. + pub(crate) fn for_test(overlay: &ValidatedOverlay) -> Self { + use crate::context::Backend; + let acls = AclTables::build(overlay, Backend::Reference) + .expect("reference backend build is infallible"); + Self { acls } + } + + /// Build a context using the rte_acl backend, for the differential test that exercises the + /// production classifier. Requires the EAL to be initialized (`#[dpdk::with_eal]`). + pub(crate) fn for_test_dpdk(overlay: &ValidatedOverlay) -> Result { + use crate::context::Backend; + let acls = AclTables::build(overlay, Backend::Dpdk)?; + Ok(Self { acls }) + } +} + +/// Control-plane handle used to hot-swap the context. +#[derive(Debug, Clone)] +pub struct AclFilterContextWriter(Arc>); + +impl Default for AclFilterContextWriter { + fn default() -> Self { + Self(Arc::new(Slot::from_pointee(AclFilterContext::default()))) + } +} + +impl AclFilterContextWriter { + /// Create a new handle with a default context. + #[must_use] + pub fn new() -> Self { + Self::default() + } + + /// Atomically publish a new context on reconfiguration. + pub fn store(&self, context: AclFilterContext) { + self.0.store(Arc::new(context)); + } + + /// Obtain a reader for the context. + #[must_use] + pub fn get_reader(&self) -> AclFilterContextReader { + AclFilterContextReader(Arc::clone(&self.0)) + } + + /// Obtain a reader factory for the context. + #[must_use] + pub fn get_reader_factory(&self) -> AclFilterContextReaderFactory { + AclFilterContextReaderFactory(self.get_reader()) + } +} + +#[derive(Debug, Clone)] +pub struct AclFilterContextReader(Arc>); + +impl AclFilterContextReader { + /// Load the current context for read-only access. + #[must_use] + pub fn load(&self) -> Arc { + // FIXME: We'd like to use self.0.load() instead of .load_full() (saves the cost of the + // atomic counter update, although x86_64 shouldn't be affected), but if we do, then the + // loom and shuttle back-ends expect a different return type for the method + // (arc_swap::Guard>). We need to create a wrapper around that type in + // the concurrency crate. + self.0.load_full() + } +} + +#[derive(Debug, Clone)] +pub struct AclFilterContextReaderFactory(AclFilterContextReader); + +impl AclFilterContextReaderFactory { + /// Obtain a reader from the factory. + #[must_use] + pub fn handle(&self) -> AclFilterContextReader { + self.0.clone() + } +} diff --git a/acl-filter/src/context.rs b/acl-filter/src/context.rs new file mode 100644 index 0000000000..df140f452d --- /dev/null +++ b/acl-filter/src/context.rs @@ -0,0 +1,503 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Context table build (ACLs) + +use super::PacketSummary; +use acl::dpdk::dyn_table::predicate_to_chunks; +use acl::dpdk::install::install_table; +use acl::dpdk::lookup::DpdkAclLookup; +use acl::dpdk::rule::{AclFieldChunks, RuleSpec}; +#[cfg(test)] +use acl::reference::table::{RefRule, ReferenceTable}; +use concurrency::sync::atomic::{AtomicU64, Ordering}; +use config::ConfigError; +use config::external::overlay::ValidatedOverlay; +use config::external::overlay::acl::{AclAction, AclProtoMatch, AclScope, ValidatedAclRule}; +use dpdk::acl::{CategoryMask, Priority}; +use lookup::Lookup; +use lpm::prefix::{Prefix, PrefixPortsSet, PrefixWithOptionalPorts}; +use match_action::{Erased, ExactSpec, FieldPredicate, FixedSize, MaskSpec, MatchKey, RangeSpec}; +use net::vxlan::Vni; +use std::collections::HashMap; +use std::fmt; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; +use std::num::NonZero; +use tracing::error; + +#[derive(Debug, Clone, PartialEq, Eq)] +struct PeeringAclRule { + src_vni: Vni, + dst_vni: Vni, + src_ip_range: Option, + dst_ip_range: Option, + src_port_range: Option>, + dst_port_range: Option>, + log: bool, + scope: AclScope, + action: AclAction, + proto: AclProtoMatch, + is_ipv4: bool, +} + +#[derive(Debug, Default, Clone, PartialEq, Eq)] +struct PeeringAclRuleSet { + // All rules for a given IP version live in one ordered list. The protocol is carried in the key + // (a `#[mask]` byte), so there is no per-protocol table split and no need to duplicate an + // `Any`-protocol rule across protocols. Insertion order is preserved, which is what gives + // first-match precedence at lookup time. + v4: Vec, + v6: Vec, + + default_actions: HashMap<(Vni, Vni), AclAction>, +} + +impl PeeringAclRuleSet { + fn insert(&mut self, src_vni: Vni, dst_vni: Vni, rule: &ValidatedAclRule, is_ipv4: bool) { + let pattern = rule.pattern(); + let (src_prefixes, dst_prefixes) = (pattern.src(), pattern.dst()); + let template = PeeringAclRule { + src_vni, + dst_vni, + src_ip_range: None, + dst_ip_range: None, + src_port_range: None, + dst_port_range: None, + log: rule.log(), + scope: rule.scope(), + action: rule.action(), + proto: pattern.proto(), + is_ipv4, + }; + if src_prefixes.is_empty() { + self.insert_for_src_prefix(template, None, dst_prefixes); + } else { + for src_prefix in src_prefixes.iter() { + self.insert_for_src_prefix(template.clone(), Some(src_prefix), dst_prefixes); + } + } + } + + fn insert_for_src_prefix( + &mut self, + template: PeeringAclRule, + src: Option<&PrefixWithOptionalPorts>, + dst_prefixes: &PrefixPortsSet, + ) { + if dst_prefixes.is_empty() { + self.insert_for_src_and_dst_prefix(template, src, None); + } else { + for dst_prefix in dst_prefixes.iter() { + self.insert_for_src_and_dst_prefix(template.clone(), src, Some(dst_prefix)); + } + } + } + + fn insert_for_src_and_dst_prefix( + &mut self, + mut template_rule: PeeringAclRule, + src: Option<&PrefixWithOptionalPorts>, + dst: Option<&PrefixWithOptionalPorts>, + ) { + template_rule.src_ip_range = src.map(|pwop| pwop.prefix()); + template_rule.dst_ip_range = dst.map(|pwop| pwop.prefix()); + template_rule.src_port_range = src.and_then(|pwop| pwop.ports().map(|p| p.into())); + template_rule.dst_port_range = dst.and_then(|pwop| pwop.ports().map(|p| p.into())); + // The protocol is a key field (not a table selector), so a rule only splits by IP version. + if template_rule.is_ipv4 { + self.v4.push(template_rule); + } else { + self.v6.push(template_rule); + } + } +} + +impl From<&ValidatedOverlay> for PeeringAclRuleSet { + fn from(overlay: &ValidatedOverlay) -> Self { + let mut ruleset = Self::default(); + for vpc in overlay.vpc_table().values() { + let local_vni = vpc.vni(); + for peering in vpc.peerings() { + let Some(acl) = peering.acl() else { + continue; + }; + let remote_vni = overlay.vpc_table().get_remote_vni(peering); + + // The default action is a property of the peering and applies to both directions. + // Each peering is visited once per VPC (with local/remote swapped), so registering + // the local->remote pair on every visit covers both directed pairs. + ruleset + .default_actions + .insert((local_vni, remote_vni), acl.default_action()); + + for acl_rule in acl.rules() { + // A rule is directional: its validated pattern's src/dst prefixes are bound to + // the rule's `from`/`to` VPCs. Each peering is visited once per end VPC (with + // local/remote swapped), so insert the rule only from its `from` VPC's visit. + if acl_rule.from() != peering.local().name() { + continue; + } + ruleset.insert(local_vni, remote_vni, acl_rule, peering.is_v4()); + } + } + } + ruleset + } +} + +// ------------------------------------------------------------------------------------------------- +// Key and rule lowering. +// +// Every rule is lowered once to backend-neutral `FieldPredicate`s (via the `Erased` backend). The +// selected backend (see `Backend`) then consumes those: `Dpdk` converts them to rte_acl fields at +// build time; `Reference` (test only) matches them directly. + +/// A single ACL match key: protocol, VPC pair, address pair, and port pair. +/// +/// `proto` is a `#[mask]` byte on purpose. rte_acl requires the first field to be a single byte, +/// and a bitmask byte both satisfies that and lets one field express either an exact protocol +/// (`mask 0xff`) or "any protocol" (`mask 0x00`). Carrying the protocol in the key -- rather than +/// in the table identity -- collapses what used to be six per-protocol tables into one table per +/// IP version, and removes the need to duplicate an `Any` rule across protocols. +/// +/// Ports are only meaningful for TCP/UDP; config validation forbids port matching on other +/// protocols, so non-TCP/UDP rules always carry a wildcard port range and a portless packet looks +/// up with port `0`. +#[derive(Debug, MatchKey, Clone, PartialEq, Eq)] +pub(super) struct AclKey { + #[mask] + proto: u8, + #[exact] + src_vni: u32, + #[exact] + dst_vni: u32, + #[prefix] + src_ip_range: I, + #[prefix] + dst_ip_range: I, + #[range] + src_port_range: u16, + #[range] + dst_port_range: u16, +} + +impl AclKey { + #[must_use] + fn new( + proto: u8, + src_vni: Vni, + dst_vni: Vni, + src_ip: I, + dst_ip: I, + src_port: Option, + dst_port: Option, + ) -> Self { + Self { + proto, + src_vni: src_vni.as_u32(), + dst_vni: dst_vni.as_u32(), + src_ip_range: src_ip, + dst_ip_range: dst_ip, + src_port_range: src_port.unwrap_or(0), + dst_port_range: dst_port.unwrap_or(0), + } + } +} + +/// Lower a single rule to backend-neutral field predicates for the concrete IP version of its +/// prefixes. Returns `None` if the source and destination prefixes disagree on IP version, which +/// the config validation already rules out for a well-formed peering. +fn rule_predicates( + proto: AclProtoMatch, + src_vni: Vni, + dst_vni: Vni, + src_ip_range: Prefix, + dst_ip_range: Prefix, + src_port_range: RangeSpec, + dst_port_range: RangeSpec, +) -> Option> { + let proto: MaskSpec = proto.into(); + match (src_ip_range, dst_ip_range) { + (Prefix::IPV4(src_ip_range), Prefix::IPV4(dst_ip_range)) => Some( + AclKeyRule { + proto, + src_vni: ExactSpec::new(src_vni.as_u32()), + dst_vni: ExactSpec::new(dst_vni.as_u32()), + src_ip_range: src_ip_range.into(), + dst_ip_range: dst_ip_range.into(), + src_port_range, + dst_port_range, + } + .into_backend_fields::(), + ), + (Prefix::IPV6(src_ip_range), Prefix::IPV6(dst_ip_range)) => Some( + AclKeyRule { + proto, + src_vni: ExactSpec::new(src_vni.as_u32()), + dst_vni: ExactSpec::new(dst_vni.as_u32()), + src_ip_range: src_ip_range.into(), + dst_ip_range: dst_ip_range.into(), + src_port_range, + dst_port_range, + } + .into_backend_fields::(), + ), + _ => None, + } +} + +pub(super) trait Wildcardable { + fn wildcard() -> Prefix; +} + +impl Wildcardable for Ipv4Addr { + fn wildcard() -> Prefix { + Prefix::root_v4() + } +} + +impl Wildcardable for Ipv6Addr { + fn wildcard() -> Prefix { + Prefix::root_v6() + } +} + +/// Lower all rules for one IP version into `(predicates, action)` pairs, preserving order (which is +/// the precedence). A missing prefix or port range becomes the wildcard for that field. +fn lower_rules( + rules: &[PeeringAclRule], +) -> Vec<(Vec, LookupResult)> { + rules + .iter() + .filter_map(|rule| { + Some(( + rule_predicates( + rule.proto, + rule.src_vni, + rule.dst_vni, + rule.src_ip_range.unwrap_or_else(T::wildcard), + rule.dst_ip_range.unwrap_or_else(T::wildcard), + rule.src_port_range + .unwrap_or(lpm::prefix::with_ports::PORT_RANGE_WILDCARD), + rule.dst_port_range + .unwrap_or(lpm::prefix::with_ports::PORT_RANGE_WILDCARD), + )?, + LookupResult { + action: rule.action, + log: rule.log, + scope: rule.scope, + }, + )) + }) + .collect() +} + +// ------------------------------------------------------------------------------------------------- +// Backend selection. + +/// Backend used to build the production context: the rte_acl classifier. It requires the EAL to be +/// initialized (done once in `dataplane::main`). The reference backend is `cfg(test)`-gated and is +/// never compiled into production builds. +pub(super) const PRODUCTION_BACKEND: Backend = Backend::Dpdk; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum Backend { + Dpdk, + #[cfg(test)] + Reference, +} + +/// A built ACL table for one IP version. +/// +/// `Empty` matches nothing -- used for the default context and any zero-rule table, so we never +/// ask rte_acl to build an empty context. `Dpdk` is the production rte_acl classifier. `Reference` +/// is the `cfg(test)` linear-scan oracle that drives the fast, EAL-free semantic suite. +#[allow(clippy::large_enum_variant)] // test only +pub(super) enum AnyTable { + Empty, + Dpdk(DpdkAclLookup), + #[cfg(test)] + Reference(ReferenceTable), +} + +impl AnyTable { + /// Single-key lookup -- the per-packet production path (acl-filter classifies one packet at a + /// time rather than in batches). + fn lookup(&self, key: &K) -> Option<&A> { + match self { + AnyTable::Empty => None, + AnyTable::Dpdk(table) => table.lookup(key), + #[cfg(test)] + AnyTable::Reference(table) => table.lookup(key), + } + } + + pub(super) fn len(&self) -> usize { + match self { + AnyTable::Empty => 0, + AnyTable::Dpdk(table) => table.actions().len(), + #[cfg(test)] + AnyTable::Reference(table) => table.len(), + } + } + + /// The reference-backend rules, for the full CLI dump; `None` for the (opaque) rte_acl and + /// empty tables. + #[cfg(test)] + pub(super) fn reference_rules(&self) -> Option<&[RefRule]> { + match self { + AnyTable::Reference(table) => Some(table.rules()), + _ => None, + } + } +} + +impl fmt::Debug for AnyTable { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let kind = match self { + AnyTable::Empty => "empty", + AnyTable::Dpdk(_) => "dpdk", + #[cfg(test)] + AnyTable::Reference(_) => "reference", + }; + write!(f, "AnyTable::{kind}({} rules)", self.len()) + } +} + +static TABLE_SEQ: AtomicU64 = AtomicU64::new(0); + +/// A process-unique rte_acl context name. rte_acl rejects duplicate names, and a hot-swap briefly +/// keeps the old and new contexts alive at once, so the name must be unique across the process. +fn table_name(base: &str) -> String { + format!("acl_{base}_{}", TABLE_SEQ.fetch_add(1, Ordering::Relaxed)) +} + +/// Build one table for the selected backend from rules in precedence (insertion) order. +fn build_table( + backend: Backend, + base_name: &str, + rules: Vec<(Vec, A)>, +) -> Result, String> { + match backend { + Backend::Dpdk => { + // A zero-rule table matches nothing; represent it as `Empty` rather than asking + // rte_acl to build an empty context. + if rules.is_empty() { + return Ok(AnyTable::Empty); + } + let n = rules.len(); + let max = NonZero::new(u32::try_from(n).unwrap_or(u32::MAX)) + .ok_or_else(|| "zero-rule table reached the dpdk builder".to_string())?; + let specs = K::field_specs(); + let mut rule_specs = Vec::with_capacity(n); + for (i, (fields, action)) in rules.into_iter().enumerate() { + // Positional first-match: the first matching rule wins. rte_acl returns the + // highest-priority match, so priority descends with insertion index (rule 0 gets + // the highest priority). Distinct priorities keep the outcome deterministic. + let priority_value = i32::try_from(n - i).map_err(|e| e.to_string())?; + let priority = Priority::new(priority_value).map_err(|e| e.to_string())?; + let chunks: Vec = fields + .iter() + .zip(specs) + .map(|(pred, spec)| predicate_to_chunks(pred, spec.size)) + .collect(); + let spec = RuleSpec::::new( + priority, + CategoryMask::new(1).map_err(|e| e.to_string())?, + chunks, + action, + ) + .map_err(|e| e.to_string())?; + rule_specs.push(spec); + } + install_table::(&table_name(base_name), max, rule_specs) + .map(AnyTable::Dpdk) + .map_err(|e| e.to_string()) + } + #[cfg(test)] + Backend::Reference => { + // The reference backend is first-match on insertion order, which is exactly the + // precedence we want -- no priority sort needed. + let rules = rules + .into_iter() + .map(|(fields, action)| RefRule::new(fields, action)) + .collect(); + Ok(AnyTable::Reference(ReferenceTable::new(rules))) + } + } +} + +#[derive(Debug, Clone)] +pub(super) struct LookupResult { + pub(super) action: AclAction, + pub(super) log: bool, + pub(super) scope: AclScope, +} + +#[derive(Debug)] +pub(super) struct AclTables { + pub(super) v4: AnyTable, LookupResult>, + pub(super) v6: AnyTable, LookupResult>, + pub(super) default_actions: HashMap<(Vni, Vni), AclAction>, +} + +impl Default for AclTables { + fn default() -> Self { + Self { + v4: AnyTable::Empty, + v6: AnyTable::Empty, + default_actions: HashMap::new(), + } + } +} + +impl AclTables { + pub(super) fn build(overlay: &ValidatedOverlay, backend: Backend) -> Result { + let ruleset = PeeringAclRuleSet::from(overlay); + let v4 = + build_table::, _>(backend, "v4", lower_rules::(&ruleset.v4)) + .map_err(ConfigError::FailureApply)?; + let v6 = + build_table::, _>(backend, "v6", lower_rules::(&ruleset.v6)) + .map_err(ConfigError::FailureApply)?; + Ok(Self { + v4, + v6, + default_actions: ruleset.default_actions, + }) + } +} + +// ------------------------------------------------------------------------------------------------- +// Lookup logic + +impl AclTables { + #[must_use] + pub(super) fn lookup(&self, p: &PacketSummary) -> Option<&LookupResult> { + let proto = p.proto.as_u8(); + let (src_ports, dst_ports) = p.ports.unzip(); + match (p.src_ip, p.dst_ip) { + (IpAddr::V4(src_ip), IpAddr::V4(dst_ip)) => { + let key = AclKey::new( + proto, p.src_vni, p.dst_vni, src_ip, dst_ip, src_ports, dst_ports, + ); + self.v4.lookup(&key) + } + (IpAddr::V6(src_ip), IpAddr::V6(dst_ip)) => { + let key = AclKey::new( + proto, p.src_vni, p.dst_vni, src_ip, dst_ip, src_ports, dst_ports, + ); + self.v6.lookup(&key) + } + _ => { + error!("Found packet with different IP versions for source and destination!"); + None + } + } + } + + #[must_use] + pub(super) fn find_default_action(&self, src_vni: Vni, dst_vni: Vni) -> Option { + self.default_actions.get(&(src_vni, dst_vni)).copied() + } +} diff --git a/acl-filter/src/lib.rs b/acl-filter/src/lib.rs new file mode 100644 index 0000000000..50f55f29ba --- /dev/null +++ b/acl-filter/src/lib.rs @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +use concurrency::sync::Arc; +use config::external::overlay::acl::{AclAction, AclScope}; +use net::buffer::PacketBufferMut; +use net::flows::FlowInfo; +use net::flows::FlowStatus; +use net::headers::{TryIp, TryTransport}; +use net::ip::NextHeader; +use net::packet::{DoneReason, Packet, PacketMeta, VpcDiscriminant}; +use net::vxlan::Vni; +use pipeline::{NetworkFunction, PipelineData}; +use std::num::NonZero; +use tracing::{debug, info}; + +mod access; +mod context; + +pub use access::{ + AclFilterContext, AclFilterContextReader, AclFilterContextReaderFactory, AclFilterContextWriter, +}; + +pub struct AclFilter { + name: String, + tablesr: AclFilterContextReader, + pipeline_data: Arc, +} + +impl AclFilter { + #[must_use] + pub fn new(name: &str, tablesr: AclFilterContextReader) -> Self { + Self { + name: name.to_string(), + tablesr, + pipeline_data: Arc::new(PipelineData::default()), + } + } + + fn process_packet(&mut self, packet: &mut Packet) { + let nfi = &self.name; + let genid = self.pipeline_data.genid(); + + let summary = match PacketSummary::try_from(&*packet) { + Ok(summary) => summary, + Err(reason) => { + packet.done(reason); + return; + } + }; + let valid_flow = self.packet_has_valid_flow(packet.meta(), genid); + + if self.lookup(&summary, valid_flow) == AclAction::Deny { + debug!("{nfi}: Packet rejected by ACLs, dropping packet"); + packet.invalidate_flows(); + packet.done(DoneReason::Filtered); + } + } + + fn packet_has_valid_flow<'a>( + &self, + meta: &'a PacketMeta, + genid: i64, + ) -> Option<&'a Arc> { + let nfi = &self.name; + let Some(flow) = &meta.flow_info else { + debug!("{nfi}: Packet has no flow information"); + return None; + }; + + if flow.status() != FlowStatus::Active { + debug!("{nfi}: Packet has inactive flow information"); + return None; + } + let flow_genid = flow.genid(); + if flow_genid < genid { + debug!( + "{nfi}: Packet has outdated flow information from a prior configuration ({flow_genid} < {genid})" + ); + return None; + } + Some(flow) + } + + fn lookup(&self, summary: &PacketSummary, flow_info: Option<&Arc>) -> AclAction { + let guard = self.tablesr.load(); + let tables = &guard.acls; + + // Look up for an ACL directly matching the packet + let lookup_result = tables.lookup(summary); + if let Some(result) = lookup_result { + let verdict = result.action; + if result.log { + info!("ACL filtering: {summary:?} -> {verdict:?}") + } + return verdict; + } + + // If we have flow information, we may be dealing with a reply for an authorized flow. But + // we need to check whether the corresponding ACL rule has a 'flow' or 'packet' scope. To do + // that, we need to reverse-NAT the packet and do another lookup, to find the rule and its + // scope, if any. + if let Some(flow) = flow_info + && let Some(reverse_summary) = summary.reverse_summary(flow) + { + let reverse_lookup_result = tables.lookup(&reverse_summary); + if let Some(result) = reverse_lookup_result + && result.action == AclAction::Allow + && result.scope == AclScope::Flow + { + let verdict = result.action; + if result.log { + info!("ACL filtering: {summary:?} -> {verdict:?} (reply from allowed flow)") + } + return verdict; + } + } + + // Look for a fallback default action for the peering + tables + .find_default_action(summary.src_vni, summary.dst_vni) + .unwrap_or( + // No default action was found for this peering, this means no ACL list was + // configured for the peering. Allow packet to go through. + AclAction::Allow, + ) + } +} + +impl NetworkFunction for AclFilter { + fn process<'a, Input: Iterator> + 'a>( + &'a mut self, + input: Input, + ) -> impl Iterator> + 'a { + input.filter_map(|mut packet| { + if !packet.is_done() && packet.meta().is_overlay() { + self.process_packet(&mut packet); + } + packet.enforce() + }) + } + + fn set_data(&mut self, data: Arc) { + self.pipeline_data = data; + } +} + +#[derive(Debug, Clone)] +pub(crate) struct PacketSummary { + pub(crate) src_vni: Vni, + pub(crate) dst_vni: Vni, + pub(crate) src_ip: std::net::IpAddr, + pub(crate) dst_ip: std::net::IpAddr, + pub(crate) proto: NextHeader, + pub(crate) ports: Option<(u16, u16)>, +} + +impl TryFrom<&Packet> for PacketSummary { + type Error = DoneReason; + + fn try_from(packet: &Packet) -> Result { + let Some((src_vpcd, dst_vpcd)) = packet.meta().src_vpcd.zip(packet.meta().dst_vpcd) else { + debug!("Missing source or destination VPC discriminant, dropping packet"); + return Err(DoneReason::Unroutable); + }; + let VpcDiscriminant::VNI(src_vni) = src_vpcd; + let VpcDiscriminant::VNI(dst_vni) = dst_vpcd; + + let Some(net) = packet.try_ip() else { + debug!("No IP headers found, dropping packet"); + return Err(DoneReason::NotIp); + }; + + let src_ip = net.src_addr(); + let dst_ip = net.dst_addr(); + let proto = net.next_header(); + let ports = packet.try_transport().and_then(|t| { + t.src_port() + .map(NonZero::get) + .zip(t.dst_port().map(NonZero::get)) + }); + + Ok(Self { + src_vni, + dst_vni, + src_ip, + dst_ip, + proto, + ports, + }) + } +} + +impl PacketSummary { + fn reverse_summary(&self, flow_info: &Arc) -> Option { + let related = flow_info.related.as_ref()?.upgrade()?; + let flow_key = related.flowkey(); + Some(PacketSummary { + src_vni: self.dst_vni, + dst_vni: self.src_vni, + src_ip: *flow_key.src_ip(), + dst_ip: *flow_key.dst_ip(), + proto: flow_key.proto(), + ports: flow_key.ports().map(|(src, dst)| (src.get(), dst.get())), + }) + } +} diff --git a/acl/src/reference/table.rs b/acl/src/reference/table.rs index 80463f3480..a05c6b491e 100644 --- a/acl/src/reference/table.rs +++ b/acl/src/reference/table.rs @@ -63,6 +63,12 @@ impl ReferenceTable { pub fn is_empty(&self) -> bool { self.rules.is_empty() } + + #[must_use] + pub fn rules(&self) -> &[RefRule] { + &self.rules + } + fn pack(key: &K) -> Option<[u8; MAX_KEY_BYTES]> { if K::KEY_SIZE > MAX_KEY_BYTES { return None; diff --git a/config/src/external/overlay/acl.rs b/config/src/external/overlay/acl.rs index 2c733777ce..5fcfbc2671 100644 --- a/config/src/external/overlay/acl.rs +++ b/config/src/external/overlay/acl.rs @@ -242,6 +242,18 @@ impl ValidatedAclRule { self.action } + /// Name of the VPC the rule applies traffic *from* (matches one of the peering's two VPCs). + #[must_use] + pub fn from(&self) -> &str { + &self.from + } + + /// Name of the VPC the rule applies traffic *to* (matches one of the peering's two VPCs). + #[must_use] + pub fn to(&self) -> &str { + &self.to + } + #[must_use] pub fn pattern(&self) -> &ValidatedAclPattern { &self.pattern diff --git a/lpm/src/prefix/with_ports.rs b/lpm/src/prefix/with_ports.rs index 44179d3ba1..7c8f60d126 100644 --- a/lpm/src/prefix/with_ports.rs +++ b/lpm/src/prefix/with_ports.rs @@ -574,7 +574,7 @@ impl RangeBounds for PortRange { // Build RangeSpec from relevant types - This is used with ACLs -const PORT_RANGE_WILDCARD: RangeSpec = RangeSpec::new(0, u16::MAX); +pub const PORT_RANGE_WILDCARD: RangeSpec = RangeSpec::new(0, u16::MAX); impl From for RangeSpec { fn from(range: PortRange) -> Self { From 3b8f381ea986fa98a301ca4ec20b3c0be805d7aa Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Mon, 13 Jul 2026 13:02:47 +0100 Subject: [PATCH 10/17] feat(acl-filter): Implement Display trait for acl-filter's context Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Daniel Noland Signed-off-by: Daniel Noland Signed-off-by: Quentin Monnet --- Cargo.lock | 2 + acl-filter/Cargo.toml | 2 + acl-filter/src/display.rs | 221 ++++++++++++++++++++++++++++++++++++++ acl-filter/src/lib.rs | 1 + 4 files changed, 226 insertions(+) create mode 100644 acl-filter/src/display.rs diff --git a/Cargo.lock b/Cargo.lock index 4ba6a9b32b..f89a747c17 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1240,6 +1240,7 @@ name = "dataplane-acl-filter" version = "0.22.0" dependencies = [ "dataplane-acl", + "dataplane-common", "dataplane-concurrency", "dataplane-config", "dataplane-dpdk", @@ -1248,6 +1249,7 @@ dependencies = [ "dataplane-match-action", "dataplane-net", "dataplane-pipeline", + "indenter", "tracing", "tracing-test", ] diff --git a/acl-filter/Cargo.toml b/acl-filter/Cargo.toml index 2f95cf0ac6..c5d1d58911 100644 --- a/acl-filter/Cargo.toml +++ b/acl-filter/Cargo.toml @@ -7,9 +7,11 @@ version.workspace = true [dependencies] acl = { workspace = true } +common = { workspace = true } concurrency = { workspace = true } config = { workspace = true } dpdk = { workspace = true } +indenter = { workspace = true } lookup = { workspace = true } lpm = { workspace = true } match-action = { workspace = true, features = ["derive"] } diff --git a/acl-filter/src/display.rs b/acl-filter/src/display.rs new file mode 100644 index 0000000000..986226ab40 --- /dev/null +++ b/acl-filter/src/display.rs @@ -0,0 +1,221 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Display implementations for the ACL filter context. +//! +//! In production (rte_acl backend) the rules are baked into an opaque classifier, so only a rule +//! count is shown per table. In test / `reference` builds the reference backend keeps the rules, so +//! each rule's field predicates and action are rendered in full by decoding the positional field +//! layout of the `AclKey` match key defined in `context.rs`: +//! +//! - `AclKey`: proto, src_vni, dst_vni, src_ip, dst_ip, src_port, dst_port + +use common::cliprovider::{CliSource, Heading}; + +use std::fmt::{self, Display, Write}; + +use crate::AclFilterContext; +use crate::context::AclTables; + +impl CliSource for AclFilterContext {} + +impl Display for AclFilterContext { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + Heading("ACL filter").fmt(f)?; + write!(f, "{}", self.acls) + } +} + +/// Dump the peering default actions, sorted for a deterministic rendering. Shared by both the +/// production (count-only) and test (full) table renderings. +fn fmt_default_actions(w: &mut W, tables: &AclTables) -> fmt::Result { + writeln!(w, "default actions:")?; + let mut w = indenter::indented(w).with_str(" "); + if tables.default_actions.is_empty() { + return writeln!(w, "(none)"); + } + let mut defaults: Vec<_> = tables.default_actions.iter().collect(); + defaults.sort_by_key(|((src, dst), _)| (src.as_u32(), dst.as_u32())); + for ((src, dst), action) in defaults { + writeln!(w, "VPC {src} -> VPC {dst}: {action:?}")?; + } + Ok(()) +} + +// ------------------------------------------------------------------------------------------------- +// Production (rte_acl / opaque): a rule count per table. + +#[cfg(not(test))] +impl Display for AclTables { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + writeln!(f, "IPv4: {} rules", self.v4.len())?; + writeln!(f, "IPv6: {} rules", self.v6.len())?; + fmt_default_actions(f, self) + } +} + +// ------------------------------------------------------------------------------------------------- +// Test / `reference` builds: full per-rule rendering from the reference backend. + +#[cfg(test)] +impl Display for AclTables { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + use indenter::indented; + writeln!(f, "IPv4:")?; + fmt_ref_table(&mut indented(f).with_str(" "), &self.v4)?; + writeln!(f, "IPv6:")?; + fmt_ref_table(&mut indented(f).with_str(" "), &self.v6)?; + fmt_default_actions(f, self) + } +} + +/// Format a single table as a numbered list of rules, decoding each rule's predicates by position +/// (`AclKey` layout: proto, src_vni, dst_vni, src_ip, dst_ip, src_port, dst_port). +#[cfg(test)] +fn fmt_ref_table( + w: &mut W, + table: &crate::context::AnyTable, +) -> fmt::Result { + let Some(rules) = table.reference_rules() else { + // rte_acl / empty tables are opaque; fall back to a count. + return writeln!(w, "({} rules)", table.len()); + }; + if rules.is_empty() { + return writeln!(w, "(none)"); + } + for (idx, rule) in rules.iter().enumerate() { + let fields = rule.fields(); + let result = rule.action(); + let proto = decode_proto(fields.first()); + let src_vni = decode_vni(fields.get(1)); + let dst_vni = decode_vni(fields.get(2)); + let src_ip = decode_prefix(fields.get(3)); + let dst_ip = decode_prefix(fields.get(4)); + let src_ports = decode_ports(fields.get(5)); + let dst_ports = decode_ports(fields.get(6)); + let log = if result.log { ", log" } else { "" }; + writeln!( + w, + "[{idx}] VPC {src_vni} -> VPC {dst_vni} | proto {proto} | src {src_ip}:{src_ports} | dst {dst_ip}:{dst_ports} | {:?} ({:?}{log})", + result.action, result.scope + )?; + } + Ok(()) +} + +/// Decode a VNI stored as a 4-byte big-endian exact-match predicate. +#[cfg(test)] +fn decode_vni(predicate: Option<&match_action::FieldPredicate>) -> String { + match predicate.and_then(match_action::FieldPredicate::as_exact) { + Some([a, b, c, d]) => u32::from_be_bytes([*a, *b, *c, *d]).to_string(), + _ => "?".to_string(), + } +} + +/// Decode an IP prefix stored as a prefix-match predicate (4 bytes for IPv4, 16 bytes for IPv6, +/// plus a prefix length). +#[cfg(test)] +fn decode_prefix(predicate: Option<&match_action::FieldPredicate>) -> String { + use std::net::{Ipv4Addr, Ipv6Addr}; + match predicate.and_then(match_action::FieldPredicate::as_prefix) { + Some(([a, b, c, d], len)) => format!("{}/{len}", Ipv4Addr::new(*a, *b, *c, *d)), + Some((bytes, len)) if bytes.len() == 16 => { + let mut octets = [0u8; 16]; + octets.copy_from_slice(bytes); + format!("{}/{len}", Ipv6Addr::from(octets)) + } + _ => "?".to_string(), + } +} + +/// Decode an IP protocol stored as a 1-byte bitmask predicate. A zero mask (wildcard) renders as +/// `any`, a full mask as the single protocol number. +#[cfg(test)] +fn decode_proto(predicate: Option<&match_action::FieldPredicate>) -> String { + match predicate.and_then(match_action::FieldPredicate::as_mask) { + Some(([value], [mask])) => { + if *mask == 0 { + "any".to_string() + } else if *mask == u8::MAX { + value.to_string() + } else { + format!("{value}&{mask:#04x}") + } + } + _ => "?".to_string(), + } +} + +/// Decode a port range stored as a pair of 2-byte big-endian range bounds. A full range renders as +/// `*`, an exact match as the single port. +#[cfg(test)] +fn decode_ports(predicate: Option<&match_action::FieldPredicate>) -> String { + match predicate.and_then(match_action::FieldPredicate::as_range) { + Some(([lo_hi, lo_lo], [hi_hi, hi_lo])) => { + let lo = u16::from_be_bytes([*lo_hi, *lo_lo]); + let hi = u16::from_be_bytes([*hi_hi, *hi_lo]); + if lo == 0 && hi == u16::MAX { + "*".to_string() + } else if lo == hi { + lo.to_string() + } else { + format!("{lo}-{hi}") + } + } + _ => "?".to_string(), + } +} + +#[cfg(test)] +mod tests { + use super::{decode_ports, decode_prefix, decode_proto, decode_vni}; + use match_action::{Erased, ExactSpec, IntoBackendField, MaskSpec, PrefixSpec, RangeSpec}; + use std::net::{Ipv4Addr, Ipv6Addr}; + + #[test] + fn decode_vni_reads_u32() { + let p = IntoBackendField::::into_backend_field(ExactSpec::new(4242u32)); + assert_eq!(decode_vni(Some(&p)), "4242"); + assert_eq!(decode_vni(None), "?"); + } + + #[test] + fn decode_prefix_reads_v4_and_v6() { + let v4 = IntoBackendField::::into_backend_field(PrefixSpec::new( + Ipv4Addr::new(10, 0, 0, 0), + 8, + )); + assert_eq!(decode_prefix(Some(&v4)), "10.0.0.0/8"); + + let v6 = IntoBackendField::::into_backend_field(PrefixSpec::new( + Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 0), + 32, + )); + assert_eq!(decode_prefix(Some(&v6)), "2001:db8::/32"); + } + + #[test] + fn decode_ports_handles_wildcard_exact_and_range() { + let wildcard = + IntoBackendField::::into_backend_field(RangeSpec::new(0u16, u16::MAX)); + assert_eq!(decode_ports(Some(&wildcard)), "*"); + + let exact = IntoBackendField::::into_backend_field(RangeSpec::new(80u16, 80u16)); + assert_eq!(decode_ports(Some(&exact)), "80"); + + let range = IntoBackendField::::into_backend_field(RangeSpec::new(80u16, 8080u16)); + assert_eq!(decode_ports(Some(&range)), "80-8080"); + } + + #[test] + fn decode_proto_handles_any_and_exact() { + let any = IntoBackendField::::into_backend_field(MaskSpec::new(0u8, 0u8)); + assert_eq!(decode_proto(Some(&any)), "any"); + + let tcp = IntoBackendField::::into_backend_field(MaskSpec::new(6u8, u8::MAX)); + assert_eq!(decode_proto(Some(&tcp)), "6"); + + let udp = IntoBackendField::::into_backend_field(MaskSpec::new(17u8, u8::MAX)); + assert_eq!(decode_proto(Some(&udp)), "17"); + } +} diff --git a/acl-filter/src/lib.rs b/acl-filter/src/lib.rs index 50f55f29ba..12a51db87a 100644 --- a/acl-filter/src/lib.rs +++ b/acl-filter/src/lib.rs @@ -16,6 +16,7 @@ use tracing::{debug, info}; mod access; mod context; +mod display; pub use access::{ AclFilterContext, AclFilterContextReader, AclFilterContextReaderFactory, AclFilterContextWriter, From 51e8ae9a29a854f0ad600a14349981bdc3ec6d37 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Sun, 19 Jul 2026 21:15:32 +0200 Subject: [PATCH 11/17] feat(acl-filter): impl Display for PacketSummary Signed-off-by: Fredi Raspall --- acl-filter/src/display.rs | 19 +++++++++++++++++++ acl-filter/src/lib.rs | 4 ++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/acl-filter/src/display.rs b/acl-filter/src/display.rs index 986226ab40..b4b20b3e70 100644 --- a/acl-filter/src/display.rs +++ b/acl-filter/src/display.rs @@ -15,6 +15,7 @@ use common::cliprovider::{CliSource, Heading}; use std::fmt::{self, Display, Write}; use crate::AclFilterContext; +use crate::PacketSummary; use crate::context::AclTables; impl CliSource for AclFilterContext {} @@ -103,6 +104,24 @@ fn fmt_ref_table( Ok(()) } +impl Display for PacketSummary { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let (Some(sport), Some(dport)) = self.ports.unzip() { + write!( + f, + "vni:{} -> vni:{}, {}:{} -> {}:{} ({})", + self.src_vni, self.dst_vni, self.src_ip, sport, self.dst_ip, dport, self.proto + ) + } else { + write!( + f, + "vni:{} -> vni:{}, {} -> {} ({})", + self.src_vni, self.dst_vni, self.src_ip, self.dst_ip, self.proto + ) + } + } +} + /// Decode a VNI stored as a 4-byte big-endian exact-match predicate. #[cfg(test)] fn decode_vni(predicate: Option<&match_action::FieldPredicate>) -> String { diff --git a/acl-filter/src/lib.rs b/acl-filter/src/lib.rs index 12a51db87a..d8ab3a87d8 100644 --- a/acl-filter/src/lib.rs +++ b/acl-filter/src/lib.rs @@ -92,7 +92,7 @@ impl AclFilter { if let Some(result) = lookup_result { let verdict = result.action; if result.log { - info!("ACL filtering: {summary:?} -> {verdict:?}") + info!("ACL filtering: {summary} -> {verdict:?}") } return verdict; } @@ -111,7 +111,7 @@ impl AclFilter { { let verdict = result.action; if result.log { - info!("ACL filtering: {summary:?} -> {verdict:?} (reply from allowed flow)") + info!("ACL filtering: {summary} -> {verdict:?} (reply from allowed flow)") } return verdict; } From df4c100384cd8a648a68080f29a36f5dd2fa7f29 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Mon, 13 Jul 2026 16:19:22 +0100 Subject: [PATCH 12/17] test(acl-filter): Add unit tests for ACL filter Add unit tests to validate the behaviour of the new ACL filter pipeline stage. Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Daniel Noland Signed-off-by: Daniel Noland Co-authored-by: Fredi Raspall Signed-off-by: Fredi Raspall Signed-off-by: Quentin Monnet --- Cargo.lock | 4 + acl-filter/Cargo.toml | 4 + acl-filter/src/lib.rs | 3 + acl-filter/src/tests.rs | 1122 ++++++++++++++++++++++++++++ config/src/external/overlay/acl.rs | 2 +- 5 files changed, 1134 insertions(+), 1 deletion(-) create mode 100644 acl-filter/src/tests.rs diff --git a/Cargo.lock b/Cargo.lock index f89a747c17..9001e90abf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1244,12 +1244,16 @@ dependencies = [ "dataplane-concurrency", "dataplane-config", "dataplane-dpdk", + "dataplane-flow-entry", + "dataplane-flow-filter", "dataplane-lookup", "dataplane-lpm", "dataplane-match-action", + "dataplane-nat", "dataplane-net", "dataplane-pipeline", "indenter", + "tokio", "tracing", "tracing-test", ] diff --git a/acl-filter/Cargo.toml b/acl-filter/Cargo.toml index c5d1d58911..15a6ff9350 100644 --- a/acl-filter/Cargo.toml +++ b/acl-filter/Cargo.toml @@ -26,6 +26,10 @@ tracing = { workspace = true } acl = { workspace = true, features = ["reference"] } config = { workspace = true, features = ["testing"] } dpdk = { workspace = true, features = ["test"] } +flow-entry = { workspace = true } +flow-filter = { workspace = true } lpm = { workspace = true, features = ["testing"] } +nat = { workspace = true } net = { workspace = true, features = ["builder"] } +tokio = { workspace = true, features = ["macros", "rt", "time"] } tracing-test = { workspace = true, features = [] } diff --git a/acl-filter/src/lib.rs b/acl-filter/src/lib.rs index d8ab3a87d8..6d254b8e2d 100644 --- a/acl-filter/src/lib.rs +++ b/acl-filter/src/lib.rs @@ -18,6 +18,9 @@ mod access; mod context; mod display; +#[cfg(test)] +mod tests; + pub use access::{ AclFilterContext, AclFilterContextReader, AclFilterContextReaderFactory, AclFilterContextWriter, }; diff --git a/acl-filter/src/tests.rs b/acl-filter/src/tests.rs new file mode 100644 index 0000000000..0e97d58a66 --- /dev/null +++ b/acl-filter/src/tests.rs @@ -0,0 +1,1122 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Tests for the ACL filter. +//! +//! ACLs are attached to a peering and evaluated in the order the user provides them: the first +//! matching rule decides the action, and if no rule matches, the peering's default action is used +//! (if no ACL is configured at all, traffic is allowed). A rule's scope ('flow' or 'packet') +//! decides whether reply traffic is allowed once the corresponding request is allowed. + +use crate::{AclFilter, AclFilterContext, AclFilterContextWriter}; + +use config::external::overlay::acl::{ + Acl, AclAction, AclPattern, AclProtoMatch, AclRule, AclScope, +}; +use config::external::overlay::vpc::{Vpc, VpcTable}; +use config::external::overlay::vpcpeering::{VpcExpose, VpcManifest, VpcPeering, VpcPeeringTable}; +use config::external::overlay::{Overlay, ValidatedOverlay}; + +use lpm::prefix::{Prefix, PrefixPortsSet, PrefixWithOptionalPorts}; + +use net::FlowKey; +use net::buffer::TestBuffer; +use net::flows::{FlowInfo, FlowInfoFlags, FlowStatus}; +use net::headers::Headers; +use net::headers::builder::HeaderStack; +use net::ipv4::UnicastIpv4Addr; +use net::ipv6::UnicastIpv6Addr; +use net::packet::{DoneReason, Packet, VpcDiscriminant}; +use net::parse::DeParse; +use net::tcp::TcpPort; +use net::udp::UdpPort; +use net::vxlan::Vni; + +use concurrency::sync::Arc; +use pipeline::NetworkFunction; + +use std::net::{Ipv4Addr, Ipv6Addr}; +use std::time::{Duration, Instant}; + +// VNIs and IP ranges used by the standard two-VPC peering (vpc1 <-> vpc2). The manifest names +// ("vpc1"/"vpc2") double as the ACL rule `from`/`to` endpoints. +const VNI1: u32 = 100; +const VNI2: u32 = 200; +const V1_IPS: &str = "10.0.0.0/24"; +const V2_IPS: &str = "20.0.0.0/24"; +const V1_IPS_V6: &str = "2001:db8::/64"; +const V2_IPS_V6: &str = "2001:db9::/64"; + +// ------------------------------------------------------------------------------------------------- +// Identifiers and address parsing + +fn vni(id: u32) -> Vni { + Vni::new_checked(id).unwrap() +} + +fn vpcd(id: u32) -> VpcDiscriminant { + VpcDiscriminant::from_vni(vni(id)) +} + +fn v4(s: &str) -> Ipv4Addr { + s.parse().unwrap() +} + +fn v6(s: &str) -> Ipv6Addr { + s.parse().unwrap() +} + +// ------------------------------------------------------------------------------------------------- +// Expose / peering / overlay builders + +/// Plain expose (no NAT): the listed prefix is both private and public. +fn expose(ip: &str) -> VpcExpose { + VpcExpose::empty().ip(ip.into()) +} + +/// Static-NAT expose: `private` addresses are translated 1:1 to `public`. +fn expose_static(private: &str, public: &str) -> VpcExpose { + VpcExpose::empty() + .make_static_nat() + .unwrap() + .ip(private.into()) + .as_range(public.into()) + .unwrap() +} + +/// Masquerade expose: `private` addresses are masqueraded behind `public`. +fn expose_masquerade(private: &str, public: &str) -> VpcExpose { + VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip(private.into()) + .as_range(public.into()) + .unwrap() +} + +/// Build a peering between `local` and `remote`, each given as `(vpc_name, exposes)`, with an +/// optional ACL attached. +fn peering( + name: &str, + local: (&str, Vec), + remote: (&str, Vec), + acl: Option, +) -> VpcPeering { + let mut peering = VpcPeering::with_default_group( + name, + VpcManifest::with_exposes(local.0, local.1), + VpcManifest::with_exposes(remote.0, remote.1), + ); + peering.acl = acl; + peering +} + +/// Assemble a `VpcTable` from `(name, vni)` pairs, generating valid 5-char ids. +fn vpc_table(vpcs: &[(&str, u32)]) -> VpcTable { + let mut table = VpcTable::new(); + for (i, (name, vni_id)) in vpcs.iter().enumerate() { + let id = format!("VPC{:02}", i + 1); // VpcId must be exactly 5 chars + table.add(Vpc::new(name, &id, *vni_id).unwrap()).unwrap(); + } + table +} + +/// Build and validate an overlay from a set of VPCs and peerings. +fn overlay(vpcs: &[(&str, u32)], peerings: Vec) -> ValidatedOverlay { + let mut peering_table = VpcPeeringTable::new(); + for p in peerings { + peering_table.add(p).unwrap(); + } + Overlay::new(vpc_table(vpcs), peering_table) + .validate() + .unwrap() +} + +/// Build an `AclFilter` network function from a validated overlay. +fn acl_filter(overlay: &ValidatedOverlay) -> AclFilter { + let writer = AclFilterContextWriter::new(); + // Use the reference backend so the semantic suite stays fast and EAL-free; the rte_acl backend + // is exercised separately (see the `dpdk_backend` differential module). + writer.store(AclFilterContext::for_test(overlay)); + AclFilter::new("test-acl-filter", writer.get_reader()) +} + +// Build an `AclFilter` for the standard vpc1 <-> vpc2 peering with plain exposes and the given ACL +fn build_filter(local_ips: &str, remote_ips: &str, acl: Option) -> AclFilter { + acl_filter(&overlay( + &[("vpc1", VNI1), ("vpc2", VNI2)], + vec![peering( + "vpc1-to-vpc2", + ("vpc1", vec![expose(local_ips)]), + ("vpc2", vec![expose(remote_ips)]), + acl, + )], + )) +} + +fn build_filter_v4(acl: Option) -> AclFilter { + build_filter(V1_IPS, V2_IPS, acl) +} + +// ------------------------------------------------------------------------------------------------- +// ACL rule builders + +fn prefixes(entries: &[&str]) -> PrefixPortsSet { + entries + .iter() + .map(|p| PrefixWithOptionalPorts::new(Prefix::from(*p), None)) + .collect() +} + +fn pattern(src: &[&str], dst: &[&str], proto: AclProtoMatch) -> AclPattern { + AclPattern { + src: prefixes(src), + dst: prefixes(dst), + src_any_ports: Vec::new(), + dst_any_ports: Vec::new(), + proto, + } +} + +// A rule in the given direction (`from`/`to` are the peering's VPC manifest names) +fn directional_rule( + name: &str, + from: &str, + to: &str, + action: AclAction, + scope: AclScope, + pattern: AclPattern, +) -> AclRule { + AclRule { + name: name.to_owned(), + from: from.to_owned(), + to: to.to_owned(), + action, + pattern, + scope, + log: true, + } +} + +// A rule in the request direction, from "vpc1" to "vpc2" (the standard peering's manifest names) +fn rule(name: &str, action: AclAction, scope: AclScope, pattern: AclPattern) -> AclRule { + directional_rule(name, "vpc1", "vpc2", action, scope, pattern) +} + +// ------------------------------------------------------------------------------------------------- +// Packet-header builders (via the HeaderStack builder) and packet assembly + +fn build_tcp_packet(src: Ipv4Addr, dst: Ipv4Addr, sport: u16, dport: u16) -> Headers { + HeaderStack::new() + .eth(|_| {}) + .ipv4(|ip| { + ip.set_source(UnicastIpv4Addr::new(src).unwrap()); + ip.set_destination(dst); + }) + .tcp(|tcp| { + tcp.set_source(TcpPort::try_from(sport).unwrap()); + tcp.set_destination(TcpPort::try_from(dport).unwrap()); + }) + .build_headers() + .unwrap() +} + +fn build_udp_packet(src: Ipv4Addr, dst: Ipv4Addr, sport: u16, dport: u16) -> Headers { + HeaderStack::new() + .eth(|_| {}) + .ipv4(|ip| { + ip.set_source(UnicastIpv4Addr::new(src).unwrap()); + ip.set_destination(dst); + }) + .udp(|udp| { + udp.set_source(UdpPort::try_from(sport).unwrap()); + udp.set_destination(UdpPort::try_from(dport).unwrap()); + }) + .build_headers() + .unwrap() +} + +fn build_tcp_packet_v6(src: Ipv6Addr, dst: Ipv6Addr, sport: u16, dport: u16) -> Headers { + HeaderStack::new() + .eth(|_| {}) + .ipv6(|ip| { + ip.set_source(UnicastIpv6Addr::new(src).unwrap()); + ip.set_destination(dst); + }) + .tcp(|tcp| { + tcp.set_source(TcpPort::try_from(sport).unwrap()); + tcp.set_destination(TcpPort::try_from(dport).unwrap()); + }) + .build_headers() + .unwrap() +} + +// ICMP (IP protocol 1): a non-TCP/UDP protocol, used to exercise the `Other(n)` and `Any` tables. +fn build_icmp_packet(src: Ipv4Addr, dst: Ipv4Addr) -> Headers { + HeaderStack::new() + .eth(|_| {}) + .ipv4(|ip| { + ip.set_source(UnicastIpv4Addr::new(src).unwrap()); + ip.set_destination(dst); + }) + .icmp4(|_| {}) + .build_headers() + .unwrap() +} + +// Serialize built headers into a parseable overlay test packet with the given source and (optional) +// destination VPC. By the time a packet reaches the ACL filter both discriminants are set, so unit +// tests provide `dst_vpcd`; in the end-to-end pipeline the flow filter sets it, so `None` is used. +fn packet( + src_vpcd: VpcDiscriminant, + dst_vpcd: Option, + headers: Headers, +) -> Packet { + let mut buffer = TestBuffer::new(); + headers.deparse(buffer.as_mut()).unwrap(); + let mut packet = Packet::new(buffer).unwrap(); + packet.meta_mut().set_overlay(true); + packet.meta_mut().src_vpcd = Some(src_vpcd); + packet.meta_mut().dst_vpcd = dst_vpcd; + packet +} + +fn run(filter: &mut AclFilter, packet: Packet) -> Packet { + filter.process(std::iter::once(packet)).next().unwrap() +} + +fn is_allowed(packet: &Packet) -> bool { + packet.get_done().is_none() +} + +fn is_denied(packet: &Packet) -> bool { + packet.get_done() == Some(DoneReason::Filtered) +} + +// ------------------------------------------------------------------------------------------------- +// Packet-scope tests: ordering, default action, protocol matching + +#[test] +fn no_acl_allows_all() { + let mut filter = build_filter_v4(None); + let pkt = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + assert!(is_allowed(&run(&mut filter, pkt))); +} + +// A rule that only covers the upper half of vpc2 (20.0.0.128/25), so a packet destined to the +// lower half matches no rule and hits the peering's default action +fn acl_with_nonmatching_rule(default: AclAction) -> Acl { + Acl::new( + default, + vec![rule( + "narrow-allow", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS], &["20.0.0.128/25"], AclProtoMatch::Tcp), + )], + ) +} + +#[test] +fn default_deny_when_no_rule_matches() { + let mut filter = build_filter_v4(Some(acl_with_nonmatching_rule(AclAction::Deny))); + // Destination 20.0.0.5 is in the lower half, not covered by the rule -> default Deny + let pkt = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + assert!(is_denied(&run(&mut filter, pkt))); +} + +#[test] +fn default_allow_when_no_rule_matches() { + let mut filter = build_filter_v4(Some(acl_with_nonmatching_rule(AclAction::Allow))); + // Destination 20.0.0.5 is not covered by the rule -> default Allow + let pkt = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + assert!(is_allowed(&run(&mut filter, pkt))); +} + +#[test] +fn explicit_allow_over_default_deny() { + let acl = Acl::new( + AclAction::Deny, + vec![rule( + "allow-tcp", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Tcp), + )], + ); + let mut filter = build_filter_v4(Some(acl)); + + // Matches the allow rule + let allowed = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + assert!(is_allowed(&run(&mut filter, allowed))); + + // Destination outside the rule's dst prefix -> falls through to the default Deny + let denied = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.1.5"), 1234, 80), + ); + assert!(is_denied(&run(&mut filter, denied))); +} + +// The first matching rule wins: a narrow Allow placed before a broad Deny lets matching traffic +// through, while traffic that only hits the later Deny is dropped +#[test] +fn first_matching_rule_wins() { + let acl = Acl::new( + AclAction::Deny, + vec![ + rule( + "allow-lower-half", + AclAction::Allow, + AclScope::Packet, + pattern(&["10.0.0.0/25"], &[V2_IPS], AclProtoMatch::Tcp), + ), + rule( + "deny-whole-range", + AclAction::Deny, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Tcp), + ), + ], + ); + let mut filter = build_filter_v4(Some(acl)); + + // 10.0.0.5 is in the /25 -> hits the Allow rule first + let allowed = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + assert!(is_allowed(&run(&mut filter, allowed))); + + // 10.0.0.200 is outside the /25 but inside the /24 -> only the Deny rule matches + let denied = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.200"), v4("20.0.0.5"), 1234, 80), + ); + assert!(is_denied(&run(&mut filter, denied))); +} + +// A rule matching only TCP must not affect UDP traffic, which falls through to the default +#[test] +fn protocol_specific_rule_does_not_match_other_protocol() { + let acl = Acl::new( + AclAction::Deny, + vec![rule( + "allow-tcp-only", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Tcp), + )], + ); + let mut filter = build_filter_v4(Some(acl)); + + let tcp = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + assert!(is_allowed(&run(&mut filter, tcp))); + + // Same addresses, UDP -> the TCP-only rule doesn't apply, default Deny drops it + let udp = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_udp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + assert!(is_denied(&run(&mut filter, udp))); +} + +// A rule matching any protocol must match both TCP and UDP traffic +#[test] +fn protocol_any_rule_matches_both_tcp_and_udp() { + let acl = Acl::new( + AclAction::Deny, + vec![rule( + "allow-any-proto", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Any), + )], + ); + let mut filter = build_filter_v4(Some(acl)); + + let tcp = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + assert!(is_allowed(&run(&mut filter, tcp))); + + // Same addresses, UDP -> the TCP-only rule doesn't apply, default Deny drops it + let udp = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_udp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + assert!(is_allowed(&run(&mut filter, udp))); +} + +// An 'any protocol' rule also covers non-TCP/UDP traffic (matched via the proto-agnostic table) +#[test] +fn protocol_any_rule_matches_icmp() { + let acl = Acl::new( + AclAction::Deny, + vec![rule( + "allow-any-proto", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Any), + )], + ); + let mut filter = build_filter_v4(Some(acl)); + + let icmp = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_icmp_packet(v4("10.0.0.5"), v4("20.0.0.5")), + ); + assert!(is_allowed(&run(&mut filter, icmp))); +} + +// A numeric-protocol rule matches only packets carrying that exact IP protocol number +#[test] +fn protocol_other_rule_matches_only_its_protocol_number() { + // ICMP is IP protocol 1: an Other(1) rule must match ICMP traffic. + let acl = Acl::new( + AclAction::Deny, + vec![rule( + "allow-icmp", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Other(1)), + )], + ); + let mut filter = build_filter_v4(Some(acl)); + let icmp = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_icmp_packet(v4("10.0.0.5"), v4("20.0.0.5")), + ); + assert!(is_allowed(&run(&mut filter, icmp))); + + // A rule for a different protocol number (2 = IGMP) must not match ICMP traffic; it falls + // through to the default Deny. + let acl = Acl::new( + AclAction::Deny, + vec![rule( + "allow-igmp", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Other(2)), + )], + ); + let mut filter = build_filter_v4(Some(acl)); + let icmp = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_icmp_packet(v4("10.0.0.5"), v4("20.0.0.5")), + ); + assert!(is_denied(&run(&mut filter, icmp))); +} + +// A numeric-protocol rule for a non-TCP/UDP protocol must not leak onto TCP traffic +#[test] +fn protocol_other_rule_does_not_match_tcp() { + let acl = Acl::new( + AclAction::Deny, + vec![rule( + "allow-icmp-only", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Other(1)), + )], + ); + let mut filter = build_filter_v4(Some(acl)); + + let tcp = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + assert!(is_denied(&run(&mut filter, tcp))); +} + +// For a non-TCP/UDP packet, a numeric-protocol rule and an 'any protocol' rule can both match, so +// first-match ordering must still decide (they share a single proto-range table). Both rules below +// match the ICMP packet, so the peering default never applies; only their relative order matters. +#[test] +fn protocol_any_and_other_rules_respect_order() { + // Other(1) (ICMP) Allow placed before Any Deny -> ICMP is allowed. + let acl = Acl::new( + AclAction::Deny, + vec![ + rule( + "allow-icmp", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Other(1)), + ), + rule( + "deny-any", + AclAction::Deny, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Any), + ), + ], + ); + let mut filter = build_filter_v4(Some(acl)); + let icmp = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_icmp_packet(v4("10.0.0.5"), v4("20.0.0.5")), + ); + assert!(is_allowed(&run(&mut filter, icmp))); + + // Any Deny placed before Other(1) Allow -> ICMP is denied (the earlier rule wins). + let acl = Acl::new( + AclAction::Allow, + vec![ + rule( + "deny-any", + AclAction::Deny, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Any), + ), + rule( + "allow-icmp", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Other(1)), + ), + ], + ); + let mut filter = build_filter_v4(Some(acl)); + let icmp = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_icmp_packet(v4("10.0.0.5"), v4("20.0.0.5")), + ); + assert!(is_denied(&run(&mut filter, icmp))); +} + +#[test] +fn ipv6_allow_and_default_deny() { + let acl = Acl::new( + AclAction::Deny, + vec![rule( + "allow-v6", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS_V6], &[V2_IPS_V6], AclProtoMatch::Tcp), + )], + ); + let mut filter = build_filter(V1_IPS_V6, V2_IPS_V6, Some(acl)); + + let allowed = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet_v6(v6("2001:db8::5"), v6("2001:db9::5"), 1234, 80), + ); + assert!(is_allowed(&run(&mut filter, allowed))); + + let denied = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet_v6(v6("2001:db8::5"), v6("2001:dbf::5"), 1234, 80), + ); + assert!(is_denied(&run(&mut filter, denied))); +} + +// ------------------------------------------------------------------------------------------------- +// Flow-scope tests +// +// These exercise the reply logic in isolation by faking the flow relation that masquerade or port +// forwarding would otherwise establish: a reply packet carries flow info whose `related` flow is +// the original request. See `end_to_end` below for the same behavior through a full NAT pipeline. + +// Attach flow info to `reply` linking it to the request described by `fwd_key`, mimicking an +// established session. Returns the request-side flow, which the caller must keep alive so the +// reply's weak `related` reference can still be upgraded. +fn attach_related_flow(reply: &mut Packet, fwd_key: FlowKey) -> Arc { + let reply_key = FlowKey::try_from(&*reply).unwrap(); + let expiry = Instant::now() + Duration::from_secs(60); + let (fwd_flow, reply_flow) = FlowInfo::related_pair( + expiry, + fwd_key, + FlowInfoFlags::default(), + reply_key, + FlowInfoFlags::default(), + ); + reply_flow.update_status(FlowStatus::Active); + reply.meta_mut().flow_info = Some(reply_flow); + fwd_flow +} + +#[test] +fn flow_scope_allows_reply_for_allowed_request() { + let acl = Acl::new( + AclAction::Deny, + vec![rule( + "allow-flow", + AclAction::Allow, + AclScope::Flow, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Tcp), + )], + ); + let mut filter = build_filter_v4(Some(acl)); + + // The request is allowed by the rule directly + let request = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + let fwd_key = FlowKey::try_from(&request).unwrap(); + assert!(is_allowed(&run(&mut filter, request))); + + // The reply doesn't match any rule directly, but is allowed because it belongs to an allowed + // flow (scope == Flow) + let mut reply = packet( + vpcd(VNI2), + Some(vpcd(VNI1)), + build_tcp_packet(v4("20.0.0.5"), v4("10.0.0.5"), 80, 1234), + ); + let _request_flow = attach_related_flow(&mut reply, fwd_key); + assert!(is_allowed(&run(&mut filter, reply))); +} + +#[test] +fn packet_scope_denies_reply_for_allowed_request() { + let acl = Acl::new( + AclAction::Deny, + vec![rule( + "allow-packet", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Tcp), + )], + ); + let mut filter = build_filter_v4(Some(acl)); + + let request = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + let fwd_key = FlowKey::try_from(&request).unwrap(); + assert!(is_allowed(&run(&mut filter, request))); + + // With a 'packet' scope rule, the reply is not covered by the request's authorization: it + // matches no rule directly and falls through to the default Deny + let mut reply = packet( + vpcd(VNI2), + Some(vpcd(VNI1)), + build_tcp_packet(v4("20.0.0.5"), v4("10.0.0.5"), 80, 1234), + ); + let _request_flow = attach_related_flow(&mut reply, fwd_key); + assert!(is_denied(&run(&mut filter, reply))); +} + +// An explicit Deny rule matching the reply direction takes precedence over any flow authorization: +// the reply is dropped even though it belongs to an allowed flow. (A direct rule match is decided +// before the reply's flow relation is ever consulted.) +#[test] +fn explicit_deny_rule_drops_reply_despite_matching_flow() { + let acl = Acl::new( + // Default Allow, so the drop can only come from the explicit Deny rule, not the default + AclAction::Allow, + vec![ + // Request direction (vpc1 -> vpc2): allowed, with 'flow' scope + rule( + "allow-request", + AclAction::Allow, + AclScope::Flow, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Tcp), + ), + // Reply direction (vpc2 -> vpc1): explicitly denied + directional_rule( + "deny-reply", + "vpc2", + "vpc1", + AclAction::Deny, + AclScope::Packet, + pattern(&[V2_IPS], &[V1_IPS], AclProtoMatch::Tcp), + ), + ], + ); + let mut filter = build_filter_v4(Some(acl)); + + // The request is allowed by the request-direction rule + let request = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + let fwd_key = FlowKey::try_from(&request).unwrap(); + assert!(is_allowed(&run(&mut filter, request))); + + // The reply belongs to the established (allowed) flow, but matches the reply-direction Deny + // rule directly, so it is dropped regardless of the flow + let mut reply = packet( + vpcd(VNI2), + Some(vpcd(VNI1)), + build_tcp_packet(v4("20.0.0.5"), v4("10.0.0.5"), 80, 1234), + ); + let _request_flow = attach_related_flow(&mut reply, fwd_key); + assert!(is_denied(&run(&mut filter, reply))); +} + +// A rule's validated src/dst prefixes are bound to its `from`/`to` VPCs. Make sure we process the +// source and destination manifests correctly when building the rule, and that the rule only applies +// to the correct direction. +// +// Here the rule is `vpc1 -> vpc2 Deny` over a default-Allow ACL. A reverse (vpc2 -> vpc1) packet +// carrying addresses that land in the forward rule's prefixes must NOT be denied by that rule; it +// must fall through to the peering default (Allow). +#[test] +fn directional_rule_only_applies_to_correct_direction() { + let acl = Acl::new( + // Default Allow: only the explicit Deny rule can drop a packet, so a denied reverse packet + // could only come from the forward rule "leaking" into the reverse direction. + AclAction::Allow, + vec![rule( + "deny-forward", + AclAction::Deny, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Tcp), + )], + ); + let mut filter = build_filter_v4(Some(acl)); + + // Forward direction (vpc1 -> vpc2) matches the Deny rule. + let forward = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + assert!(is_denied(&run(&mut filter, forward))); + + // Reverse direction (vpc2 -> vpc1) with addresses that fall in the forward rule's prefixes + // (the overlapping-address-space case). The forward Deny rule must not apply; the peering + // default (Allow) does. + let reverse = packet( + vpcd(VNI2), + Some(vpcd(VNI1)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 80, 1234), + ); + assert!(is_allowed(&run(&mut filter, reverse))); +} + +// ------------------------------------------------------------------------------------------------- +// End-to-end flow-scope test +// +// This runs a real NAT pipeline so the `related` flow link is established by masquerade (rather +// than faked). The ACL filter sits right after the flow filter, i.e. where the packet still +// carries pre-NAT addresses that ACL rules are written against. A 'flow'-scoped Allow rule +// authorizes the request; the reply matches no rule directly but must be allowed because it belongs +// to the established flow. + +mod end_to_end { + use super::{ + Acl, AclAction, AclFilterContext, AclFilterContextWriter, AclProtoMatch, AclScope, + build_udp_packet, expose_masquerade, expose_static, is_allowed, overlay, packet, pattern, + peering, rule, v4, vpcd, + }; + use crate::AclFilter; + + use config::external::overlay::ValidatedOverlay; + + use flow_entry::flow_table::{FlowLookup, FlowTable}; + use flow_filter::{FlowFilter, FlowFilterTable, FlowFilterTableWriter}; + use nat::masquerade::{MasqueradeConfig, NatAllocatorWriter}; + use nat::portfw::{PortForwarder, PortFwTableWriter}; + use nat::static_nat::NatTablesWriter; + use nat::static_nat::setup::build_nat_configuration; + use nat::{IcmpErrorHandler, Masquerade, StaticNat}; + + use net::buffer::TestBuffer; + + use concurrency::sync::Arc; + use pipeline::{DynPipeline, NetworkFunction}; + use tracing_test::traced_test; + + // Keep every writer handle alive for the lifetime of the pipeline: dropping one would tear down + // the data it published + struct PipelineHandles { + _flow_filter: FlowFilterTableWriter, + _static_nat: NatTablesWriter, + _portfw: PortFwTableWriter, + _masquerade: NatAllocatorWriter, + _acl: AclFilterContextWriter, + } + + // vpc1 masquerades (1.2.3.0/24 -> 5.5.5.5/32); vpc2 uses static NAT (192.168.0.0/24 <-> + // 5.6.7.0/24). An ACL allows vpc1 -> vpc2 UDP with 'flow' scope, and denies by default. + fn build_overlay() -> ValidatedOverlay { + let acl = Acl::new( + AclAction::Deny, + vec![rule( + "allow-flow-udp", + AclAction::Allow, + AclScope::Flow, + // Empty src/dst: match all traffic of the peering in this direction + pattern(&[], &[], AclProtoMatch::Udp), + )], + ); + overlay( + &[("vpc1", 100), ("vpc2", 200)], + vec![peering( + "vpc1-to-vpc2", + ("vpc1", vec![expose_masquerade("1.2.3.0/24", "5.5.5.5/32")]), + ("vpc2", vec![expose_static("192.168.0.0/24", "5.6.7.0/24")]), + Some(acl), + )], + ) + } + + fn setup_pipeline( + overlay: &ValidatedOverlay, + ) -> (DynPipeline, Arc, PipelineHandles) { + let flow_table = Arc::new(FlowTable::default()); + + let mut pipeline = DynPipeline::new(); + pipeline = pipeline.add_stage(IcmpErrorHandler::new(flow_table.clone())); + pipeline = pipeline.add_stage(FlowLookup::new("flow-lookup", flow_table.clone())); + + // Flow filter (determines destination VPC and NAT requirements) + let mut flow_filter_writer = FlowFilterTableWriter::new(); + flow_filter_writer + .update_flow_filter_table(FlowFilterTable::build_from_overlay(overlay).unwrap()); + pipeline = pipeline.add_stage(FlowFilter::new( + "flow-filter", + flow_filter_writer.get_reader(), + )); + + // ACL filter: placed here so packets still carry VPC-internal addresses + let acl_writer = AclFilterContextWriter::new(); + acl_writer.store(AclFilterContext::for_test(overlay)); + pipeline = pipeline.add_stage(AclFilter::new("acl-filter", acl_writer.get_reader())); + + // Static NAT + let mut static_nat_writer = NatTablesWriter::new(); + static_nat_writer.update_nat_tables(build_nat_configuration(overlay.vpc_table()).unwrap()); + pipeline = pipeline.add_stage(StaticNat::with_reader( + "static-nat", + static_nat_writer.get_reader(), + )); + + // Port forwarding + let mut portfw_writer = PortFwTableWriter::new(); + portfw_writer + .update_from_vpc_table(overlay.vpc_table()) + .unwrap(); + pipeline = pipeline.add_stage(PortForwarder::new( + "port-forwarder", + portfw_writer.reader(), + flow_table.clone(), + )); + + // Masquerade (creates the related flow pair used by 'flow'-scoped replies) + let mut allocator = NatAllocatorWriter::new(); + allocator.update_nat_allocator(MasqueradeConfig::new(overlay.vpc_table(), 1), &flow_table); + pipeline = pipeline.add_stage(Masquerade::new( + "masquerade", + flow_table.clone(), + allocator.get_reader(), + )); + + let handles = PipelineHandles { + _flow_filter: flow_filter_writer, + _static_nat: static_nat_writer, + _portfw: portfw_writer, + _masquerade: allocator, + _acl: acl_writer, + }; + (pipeline, flow_table, handles) + } + + #[traced_test] + #[tokio::test] + async fn flow_scope_end_to_end() { + let overlay = build_overlay(); + let (mut pipeline, _flow_table, _handles) = setup_pipeline(&overlay); + + // Request: 1.2.3.4:1234 -> 5.6.7.8:5678 (vpc1 -> vpc2). Allowed by the flow-scoped rule and + // NAT'd on the way out. Only the source VPC is set: the flow filter fills in the rest. + let request = packet( + vpcd(100), + None, + build_udp_packet(v4("1.2.3.4"), v4("5.6.7.8"), 1234, 5678), + ); + let out: Vec<_> = pipeline.process(std::iter::once(request)).collect(); + let request_out = out.first().unwrap(); + assert!( + is_allowed(request_out), + "request should be allowed and forwarded" + ); + assert_eq!(request_out.ip_source(), Some(v4("5.5.5.5").into())); + assert_eq!(request_out.ip_destination(), Some(v4("192.168.0.8").into())); + let masq_src_port = request_out.transport_src_port().unwrap().get(); + + // Reply: 192.168.0.8:5678 -> 5.5.5.5: (vpc2 -> vpc1). Matches no ACL rule + // directly, but belongs to the established flow, so the 'flow' scope authorizes it. + let reply = packet( + vpcd(200), + None, + build_udp_packet(v4("192.168.0.8"), v4("5.5.5.5"), 5678, masq_src_port), + ); + let out: Vec<_> = pipeline.process(std::iter::once(reply)).collect(); + let reply_out = out.first().unwrap(); + assert!( + is_allowed(reply_out), + "reply of an allowed flow should be allowed" + ); + // De-NAT'd back to the original request's endpoints + assert_eq!(reply_out.ip_source(), Some(v4("5.6.7.8").into())); + assert_eq!(reply_out.ip_destination(), Some(v4("1.2.3.4").into())); + } +} + +// ------------------------------------------------------------------------------------------------- +// rte_acl backend differential test +// +// The semantic suite above runs on the reference backend (fast, no EAL). This module builds the +// same overlay on BOTH backends and asserts they return the same verdict for a spread of packets: +// since the reference backend's verdicts are pinned by the suite above, agreement proves the +// production rte_acl path (rule install, positional priority, per-version tables, mask-proto and +// range fields) is correct. Requires the EAL (`#[dpdk::with_eal]`). + +mod dpdk_backend { + use super::{ + Acl, AclAction, AclFilter, AclFilterContext, AclFilterContextWriter, AclProtoMatch, + AclScope, Packet, TestBuffer, V1_IPS, V2_IPS, VNI1, VNI2, ValidatedOverlay, + build_icmp_packet, build_tcp_packet, build_udp_packet, expose, is_allowed, overlay, packet, + pattern, peering, rule, run, v4, vpcd, + }; + + fn filter_with(overlay: &ValidatedOverlay, dpdk: bool) -> AclFilter { + let writer = AclFilterContextWriter::new(); + let ctx = if dpdk { + AclFilterContext::for_test_dpdk(overlay).expect("rte_acl backend build") + } else { + AclFilterContext::for_test(overlay) + }; + writer.store(ctx); + AclFilter::new("diff-acl-filter", writer.get_reader()) + } + + // Run a freshly built packet through a reference-backed and an rte_acl-backed filter and assert + // both reach the same allow/deny verdict. + fn assert_backends_agree( + overlay: &ValidatedOverlay, + label: &str, + make_packet: impl Fn() -> Packet, + ) { + let mut reference = filter_with(overlay, false); + let mut dpdk = filter_with(overlay, true); + let reference_allowed = is_allowed(&run(&mut reference, make_packet())); + let dpdk_allowed = is_allowed(&run(&mut dpdk, make_packet())); + assert_eq!( + reference_allowed, dpdk_allowed, + "reference and rte_acl backends disagree on {label}" + ); + } + + #[test] + #[dpdk::with_eal] + fn dpdk_agrees_with_reference() { + // A representative ACL exercising first-match ordering, TCP/UDP/ICMP protocol matching, and + // the peering default. The overlapping allow-before-deny pair pins the priority wiring. + let acl = Acl::new( + AclAction::Deny, + vec![ + rule( + "allow-lower-half", + AclAction::Allow, + AclScope::Packet, + pattern(&["10.0.0.0/25"], &[V2_IPS], AclProtoMatch::Tcp), + ), + rule( + "deny-whole-range", + AclAction::Deny, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Tcp), + ), + rule( + "allow-icmp", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Other(1)), + ), + ], + ); + let overlay = overlay( + &[("vpc1", VNI1), ("vpc2", VNI2)], + vec![peering( + "vpc1-to-vpc2", + ("vpc1", vec![expose(V1_IPS)]), + ("vpc2", vec![expose(V2_IPS)]), + Some(acl), + )], + ); + + // First-match: 10.0.0.5 is in the /25 (allow wins); 10.0.0.200 only hits the deny. + assert_backends_agree(&overlay, "tcp in /25 (allow wins)", || { + packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ) + }); + assert_backends_agree(&overlay, "tcp outside /25 (deny)", || { + packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.200"), v4("20.0.0.5"), 1234, 80), + ) + }); + // UDP matches neither the TCP nor the ICMP rule -> peering default (deny). + assert_backends_agree(&overlay, "udp (default deny)", || { + packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_udp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ) + }); + // ICMP matches the Other(1) allow. + assert_backends_agree(&overlay, "icmp (Other(1) allow)", || { + packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_icmp_packet(v4("10.0.0.5"), v4("20.0.0.5")), + ) + }); + // Destination outside the peering's remote range -> default deny for every protocol. + assert_backends_agree(&overlay, "tcp to foreign dst (default deny)", || { + packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("30.0.0.5"), 1234, 80), + ) + }); + } +} diff --git a/config/src/external/overlay/acl.rs b/config/src/external/overlay/acl.rs index 5fcfbc2671..253bceeefb 100644 --- a/config/src/external/overlay/acl.rs +++ b/config/src/external/overlay/acl.rs @@ -397,7 +397,7 @@ pub struct Acl { impl Acl { #[must_use] - pub(crate) fn new(default: AclAction, rules: Vec) -> Self { + pub fn new(default: AclAction, rules: Vec) -> Self { Self { default, rules } } From 63ee4d4dbf163f823f5fbc405ac6602aa5512b10 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Mon, 20 Jul 2026 11:21:45 +0200 Subject: [PATCH 13/17] feat(net,acl-filter): add DoneReason::AclDropped There are several reasons why packets may be dropped: routing, not allowed by peering, etc. Define new drop reason for the case where a packet is dropped due to an ACL denying it as this can help operations. Signed-off-by: Fredi Raspall --- acl-filter/src/lib.rs | 2 +- acl-filter/src/tests.rs | 2 +- net/src/packet/meta.rs | 1 + net/src/packet/stats_display.rs | 1 + 4 files changed, 4 insertions(+), 2 deletions(-) diff --git a/acl-filter/src/lib.rs b/acl-filter/src/lib.rs index 6d254b8e2d..1878ad2e83 100644 --- a/acl-filter/src/lib.rs +++ b/acl-filter/src/lib.rs @@ -57,7 +57,7 @@ impl AclFilter { if self.lookup(&summary, valid_flow) == AclAction::Deny { debug!("{nfi}: Packet rejected by ACLs, dropping packet"); packet.invalidate_flows(); - packet.done(DoneReason::Filtered); + packet.done(DoneReason::AclDropped); } } diff --git a/acl-filter/src/tests.rs b/acl-filter/src/tests.rs index 0e97d58a66..88210d505b 100644 --- a/acl-filter/src/tests.rs +++ b/acl-filter/src/tests.rs @@ -290,7 +290,7 @@ fn is_allowed(packet: &Packet) -> bool { } fn is_denied(packet: &Packet) -> bool { - packet.get_done() == Some(DoneReason::Filtered) + packet.get_done() == Some(DoneReason::AclDropped) } // ------------------------------------------------------------------------------------------------- diff --git a/net/src/packet/meta.rs b/net/src/packet/meta.rs index ee561a2cff..356e72cdc9 100644 --- a/net/src/packet/meta.rs +++ b/net/src/packet/meta.rs @@ -101,6 +101,7 @@ pub enum DoneReason { VxlanDecapFailure, /* Failed to decap a Vxlan packet */ VxlanEncapFailure, /* Failed to encap a packet in vxlan */ Filtered, /* The packet was administratively filtered */ + AclDropped, /* The packet was dropped by an ACL rule */ NatOutOfResources, /* can't do NAT due to lack of resources */ FlowCapacityExceeded, /* could not create flow state: flow table capacity exceeded */ NatUnsupportedProto, /* unsupported transport protocol for NATing */ diff --git a/net/src/packet/stats_display.rs b/net/src/packet/stats_display.rs index f4c832bac6..ab9609e3fe 100644 --- a/net/src/packet/stats_display.rs +++ b/net/src/packet/stats_display.rs @@ -35,6 +35,7 @@ impl Display for DoneReason { Self::VxlanDecapFailure => f.pad("VxLAN: decap failure"), Self::Filtered => f.pad("Filtered"), + Self::AclDropped => f.pad("Dropped by ACL"), Self::FlowCapacityExceeded => f.pad("Flow capacity exceeded"), Self::NatOutOfResources => f.pad("NAT: out of resources"), From b148aab32e2be326d3c08a5fafbfe6a1153ced49 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Tue, 14 Jul 2026 01:06:28 -0600 Subject: [PATCH 14/17] feat(dataplane,mgmt): Init EAL early for the ACL filter rte_acl backend 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) Signed-off-by: Daniel Noland Signed-off-by: Quentin Monnet --- Cargo.lock | 2 ++ dataplane/Cargo.toml | 1 + dataplane/src/runtime.rs | 16 ++++++++++++++++ mgmt/Cargo.toml | 1 + mgmt/src/tests/mgmt.rs | 3 +++ 5 files changed, 23 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 9001e90abf..eea3e60272 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1185,6 +1185,7 @@ dependencies = [ "dataplane-args", "dataplane-concurrency", "dataplane-config", + "dataplane-dpdk", "dataplane-flow-entry", "dataplane-flow-filter", "dataplane-id", @@ -1637,6 +1638,7 @@ dependencies = [ "dataplane-args", "dataplane-concurrency", "dataplane-config", + "dataplane-dpdk", "dataplane-flow-entry", "dataplane-flow-filter", "dataplane-id", diff --git a/dataplane/Cargo.toml b/dataplane/Cargo.toml index 83d86fcba4..73a3e221a1 100644 --- a/dataplane/Cargo.toml +++ b/dataplane/Cargo.toml @@ -19,6 +19,7 @@ axum = { workspace = true, features = ["http1", "tokio"] } axum-server = { workspace = true } concurrency = { workspace = true } config = { workspace = true } +dpdk = { workspace = true } dyn-iter = { workspace = true } flow-entry = { workspace = true } flow-filter = { workspace = true } diff --git a/dataplane/src/runtime.rs b/dataplane/src/runtime.rs index 7610261432..28e9a4ef94 100644 --- a/dataplane/src/runtime.rs +++ b/dataplane/src/runtime.rs @@ -171,6 +171,22 @@ pub fn main() { }; init_logging(&args, &gwname); + // Initialize a minimal EAL as early as possible. The ACL filter builds rte_acl + // classifiers when configuration is applied (which happens before any packet driver starts), + // and rte_acl needs the EAL memory subsystem up. These are the lightweight, classifier-only + // args (no hugepages / no PCI). NOTE: there can be only one `rte_eal_init` per process, so the + // real DPDK datapath driver (currently `todo!()`) must eventually take over EAL ownership with + // device-appropriate args rather than adding a second init. The guard is held for the life of + // the process. + let _eal = dpdk::eal::init([ + "--no-huge", + "--no-pci", + "--in-memory", + "--no-telemetry", + "--no-shconf", + "--iova-mode=va", + ]); + let (bmp_server_params, bmp_client_opts) = parse_bmp_params(&args); let dp_status: Arc> = Arc::new(RwLock::new(DataplaneStatus::new())); diff --git a/mgmt/Cargo.toml b/mgmt/Cargo.toml index b8e9d9fa41..8972975036 100644 --- a/mgmt/Cargo.toml +++ b/mgmt/Cargo.toml @@ -56,6 +56,7 @@ tracing-test = { workspace = true } [dev-dependencies] # internal +dpdk = { workspace = true, features = ["test"] } # EAL for tests that build the rte_acl-backed flofi context fixin = { workspace = true } id = { workspace = true, features = ["bolero"] } interface-manager = { workspace = true, features = ["bolero"] } diff --git a/mgmt/src/tests/mgmt.rs b/mgmt/src/tests/mgmt.rs index 778c2ffdca..55e4178bee 100644 --- a/mgmt/src/tests/mgmt.rs +++ b/mgmt/src/tests/mgmt.rs @@ -409,6 +409,9 @@ pub mod test { #[n_vm::in_vm] #[tokio::test] async fn test_sample_config() { + // Applying the config builds the rte_acl-backed flofi context, which needs the EAL up. + let _eal = dpdk::test_support::start_eal(); + get_trace_ctl() .setup_from_string("cpi=debug,mgmt=debug,routing=debug") .unwrap(); From aa1c1e760ed48664365c06935617043fbadf384f Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Mon, 13 Jul 2026 11:01:21 +0100 Subject: [PATCH 15/17] feat(dataplane,mgmt): Add ACL filter pipeline stage 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 --- Cargo.lock | 2 + dataplane/Cargo.toml | 1 + dataplane/src/packet_processor/mod.rs | 7 ++++ dataplane/src/runtime.rs | 1 + mgmt/Cargo.toml | 1 + mgmt/src/processor/proc.rs | 53 +++++++++++++++++++-------- mgmt/src/tests/mgmt.rs | 5 +++ 7 files changed, 54 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eea3e60272..d68bb4e739 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1182,6 +1182,7 @@ dependencies = [ "arrayvec", "axum", "axum-server", + "dataplane-acl-filter", "dataplane-args", "dataplane-concurrency", "dataplane-config", @@ -1635,6 +1636,7 @@ dependencies = [ "bytes", "caps", "chrono", + "dataplane-acl-filter", "dataplane-args", "dataplane-concurrency", "dataplane-config", diff --git a/dataplane/Cargo.toml b/dataplane/Cargo.toml index 73a3e221a1..44cf70278b 100644 --- a/dataplane/Cargo.toml +++ b/dataplane/Cargo.toml @@ -12,6 +12,7 @@ default = [] loom = ["concurrency/loom"] [dependencies] +acl-filter = { workspace = true } afpacket = { workspace = true, features = ["async-tokio"] } args = { workspace = true } arrayvec = { workspace = true } diff --git a/dataplane/src/packet_processor/mod.rs b/dataplane/src/packet_processor/mod.rs index c7d32a46f5..7424f9bc87 100644 --- a/dataplane/src/packet_processor/mod.rs +++ b/dataplane/src/packet_processor/mod.rs @@ -12,6 +12,7 @@ use super::packet_processor::ipforward::IpForwarder; use concurrency::sync::Arc; +use acl_filter::{AclFilter, AclFilterContextWriter}; use flow_entry::flow_table::{FlowLookup, FlowTable}; use flow_filter::{FlowFilter, FlowFilterTableWriter}; @@ -42,6 +43,7 @@ where pub nattablesw: NatTablesWriter, pub natallocatorw: NatAllocatorWriter, pub flowfiltertablesw: FlowFilterTableWriter, + pub aclfiltertablesw: AclFilterContextWriter, pub stats: StatsCollector, pub vpc_stats_store: Arc, pub portfw_w: PortFwTableWriter, @@ -66,6 +68,8 @@ pub(crate) fn start_router( let flow_table = Arc::new(FlowTable::default()); let flowfiltertablesw = FlowFilterTableWriter::new(); let flowfiltertablesr_factory = flowfiltertablesw.get_reader_factory(); + let aclfiltertablesw = AclFilterContextWriter::new(); + let aclfiltertablesr_factory = aclfiltertablesw.get_reader_factory(); let nattablesw = NatTablesWriter::new(); let natallocatorw = NatAllocatorWriter::new(); let nattabler_factory = nattablesw.get_reader_factory(); @@ -110,6 +114,7 @@ pub(crate) fn start_router( let pktdump = PacketDumper::new("pipeline-end", true, None); let stats_stage = Stats::new("stats", stats_w.clone()); let flow_filter = FlowFilter::new("flow-filter", flowfiltertablesr_factory.handle()); + let acl_filter = AclFilter::new("acl-filter", aclfiltertablesr_factory.handle()); let icmp_error_handler = IcmpErrorHandler::new(flow_table_clone.clone()); let flow_lookup = FlowLookup::new("flow-lookup", flow_table_clone.clone()); let portfw = PortForwarder::new( @@ -128,6 +133,7 @@ pub(crate) fn start_router( .add_stage(icmp_error_handler) .add_stage(flow_lookup) .add_stage(flow_filter) + .add_stage(acl_filter) .add_stage(static_nat) .add_stage(portfw) .add_stage(masquerade) @@ -146,6 +152,7 @@ pub(crate) fn start_router( nattablesw, natallocatorw, flowfiltertablesw, + aclfiltertablesw, stats, vpc_stats_store, portfw_w, diff --git a/dataplane/src/runtime.rs b/dataplane/src/runtime.rs index 28e9a4ef94..0c0bc2e356 100644 --- a/dataplane/src/runtime.rs +++ b/dataplane/src/runtime.rs @@ -294,6 +294,7 @@ pub fn main() { nattablesw: setup.nattablesw, natallocatorw: setup.natallocatorw, flowfilterw: setup.flowfiltertablesw, + aclfilterw: setup.aclfiltertablesw, portfw_w: setup.portfw_w, vpc_stats_store: setup.vpc_stats_store, dp_status_r: dp_status.clone(), diff --git a/mgmt/Cargo.toml b/mgmt/Cargo.toml index 8972975036..f0fb215b5b 100644 --- a/mgmt/Cargo.toml +++ b/mgmt/Cargo.toml @@ -17,6 +17,7 @@ bolero = ["dep:bolero", "interface-manager/bolero", "id/bolero", "net/bolero", " [dependencies] # internal +acl-filter = { workspace = true } args = { workspace = true } config = { workspace = true } concurrency = { workspace = true } diff --git a/mgmt/src/processor/proc.rs b/mgmt/src/processor/proc.rs index 4cbef88b7b..4c4e9931ae 100644 --- a/mgmt/src/processor/proc.rs +++ b/mgmt/src/processor/proc.rs @@ -3,6 +3,8 @@ //! Configuration processor +use acl_filter::AclFilterContext; +use acl_filter::AclFilterContextWriter; use concurrency::sync::Arc; use config::external::overlay::ValidatedOverlay; use flow_entry::flow_table::FlowTable; @@ -95,6 +97,9 @@ pub struct ConfigProcessorParams { // writer for flow filter table pub flowfilterw: FlowFilterTableWriter, + // writer for ACL filter tables + pub aclfilterw: AclFilterContextWriter, + // writer for port forwarding table pub portfw_w: PortFwTableWriter, @@ -498,6 +503,26 @@ fn update_stats_vpc_mappings( pairs } +fn apply_flow_filtering_config( + overlay: &ValidatedOverlay, + flowfilterw: &mut FlowFilterTableWriter, +) -> ConfigResult { + let flow_filter_table = FlowFilterTable::build_from_overlay(overlay)?; + flowfilterw.update_flow_filter_table(flow_filter_table); + debug!("Successfully updated flow-filter table"); + Ok(()) +} + +fn apply_acl_filter_config( + overlay: &ValidatedOverlay, + aclfilterw: &mut AclFilterContextWriter, +) -> ConfigResult { + let acl_filter_context = AclFilterContext::try_from(overlay)?; + aclfilterw.store(acl_filter_context); + debug!("Successfully updated acl-filter tables"); + Ok(()) +} + /// Update the Nat tables for static NAT fn apply_static_nat_config( vpc_table: &ValidatedVpcTable, @@ -522,16 +547,6 @@ fn apply_masquerade_config( debug!("Updated masquerade NAT allocator"); } -fn apply_flow_filtering_config( - overlay: &ValidatedOverlay, - flowfilterw: &mut FlowFilterTableWriter, -) -> ConfigResult { - let flow_filter_table = FlowFilterTable::build_from_overlay(overlay)?; - flowfilterw.update_flow_filter_table(flow_filter_table); - debug!("Successfully updated flow-filter table"); - Ok(()) -} - fn apply_port_forwarding_config( vpc_table: &ValidatedVpcTable, portfw_w: &mut PortFwTableWriter, @@ -583,6 +598,7 @@ impl ConfigProcessor { let nattablesw = &mut self.proc_params.nattablesw; let natallocatorw = &mut self.proc_params.natallocatorw; let flowfilterw = &mut self.proc_params.flowfilterw; + let aclfilterw = &mut self.proc_params.aclfilterw; let portfw_w = &mut self.proc_params.portfw_w; let flow_table = &self.proc_params.flow_table; @@ -619,22 +635,27 @@ impl ConfigProcessor { /* get vrf interfaces from kernel and build a hashmap keyed by name */ let kernel_vrfs = vpc_mgr.get_kernel_vrfs().await?; + let overlay = config.external().overlay(); + + /* apply flow filtering config */ + apply_flow_filtering_config(overlay, flowfilterw)?; + + /* apply ACL filter config */ + apply_acl_filter_config(overlay, aclfilterw)?; + /* apply static NAT config */ - apply_static_nat_config(config.external().overlay().vpc_table(), nattablesw)?; + apply_static_nat_config(overlay.vpc_table(), nattablesw)?; /* apply masquerade config */ apply_masquerade_config( - config.external().overlay().vpc_table(), + overlay.vpc_table(), flow_table.as_ref(), natallocatorw, genid, ); - /* apply flow filtering config */ - apply_flow_filtering_config(config.external().overlay(), flowfilterw)?; - /* apply port-forwarding config */ - apply_port_forwarding_config(config.external().overlay().vpc_table(), portfw_w)?; + apply_port_forwarding_config(overlay.vpc_table(), portfw_w)?; /* update stats mappings and seed names to the stats store */ let _ = update_stats_vpc_mappings(&config, vpcmapw); diff --git a/mgmt/src/tests/mgmt.rs b/mgmt/src/tests/mgmt.rs index 55e4178bee..e8fa828c62 100644 --- a/mgmt/src/tests/mgmt.rs +++ b/mgmt/src/tests/mgmt.rs @@ -4,6 +4,7 @@ #[cfg(test)] #[allow(dead_code)] pub mod test { + use acl_filter::AclFilterContextWriter; use config::external::communities::PriorityCommunityTable; use config::external::gwgroup::GwGroup; use config::external::gwgroup::GwGroupMember; @@ -458,6 +459,9 @@ pub mod test { /* create FlowFilterTable for flow filtering */ let flowfilterw = FlowFilterTableWriter::new(); + /* create AclFilterContext for ACL filtering */ + let aclfilterw = AclFilterContextWriter::new(); + /* create port forwarding table */ let portfw_w = PortFwTableWriter::new(); @@ -479,6 +483,7 @@ pub mod test { nattablesw, natallocatorw, flowfilterw, + aclfilterw, portfw_w, vpc_stats_store, dp_status_r, From 2eb6312df3f5d0a6300bf41d9c3baa88d203468c Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Fri, 17 Jul 2026 12:18:36 +0100 Subject: [PATCH 16/17] fix(acl,acl-filter): Defer atomic statics via LazyLock for loom 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 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) Signed-off-by: Quentin Monnet --- acl-filter/src/context.rs | 7 ++++++- acl/benches/table_build.rs | 4 +++- acl/src/dpdk/dyn_table.rs | 8 ++++++-- acl/tests/property_dyn_shape.rs | 11 +++++++---- acl/tests/property_predicate.rs | 4 +++- 5 files changed, 25 insertions(+), 9 deletions(-) diff --git a/acl-filter/src/context.rs b/acl-filter/src/context.rs index df140f452d..ccb61b663a 100644 --- a/acl-filter/src/context.rs +++ b/acl-filter/src/context.rs @@ -10,6 +10,7 @@ use acl::dpdk::lookup::DpdkAclLookup; use acl::dpdk::rule::{AclFieldChunks, RuleSpec}; #[cfg(test)] use acl::reference::table::{RefRule, ReferenceTable}; +use concurrency::sync::LazyLock; use concurrency::sync::atomic::{AtomicU64, Ordering}; use config::ConfigError; use config::external::overlay::ValidatedOverlay; @@ -364,7 +365,11 @@ impl fmt::Debug for AnyTable { } } -static TABLE_SEQ: AtomicU64 = AtomicU64::new(0); +// Lazily initialized so this compiles under the loom backend, whose AtomicU64::new is not const +// (each instance registers with the loom executor). The atomic itself is still the backend atomic, +// so fetch_add() stays instrumented; only construction is deferred. On every other backend LazyLock +// is a thin wrapper over an otherwise-const atomic. +static TABLE_SEQ: LazyLock = LazyLock::new(|| AtomicU64::new(0)); /// A process-unique rte_acl context name. rte_acl rejects duplicate names, and a hot-swap briefly /// keeps the old and new contexts alive at once, so the name must be unique across the process. diff --git a/acl/benches/table_build.rs b/acl/benches/table_build.rs index e4aa27c2fd..89c8b002f5 100644 --- a/acl/benches/table_build.rs +++ b/acl/benches/table_build.rs @@ -7,6 +7,7 @@ mod bench { use std::hint::black_box; + use concurrency::sync::LazyLock; use concurrency::sync::atomic::{AtomicU32, Ordering}; use core::net::{Ipv4Addr, Ipv6Addr}; use core::num::NonZero; @@ -53,7 +54,8 @@ mod bench { const RULE_COUNTS: [usize; 15] = [ 1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, ]; - static SEQ: AtomicU32 = AtomicU32::new(0); + // Lazily initialized so this compiles under the loom backend, whose AtomicU32::new is not const + static SEQ: LazyLock = LazyLock::new(|| AtomicU32::new(0)); fn unique_name(prefix: &str) -> String { format!("{prefix}_{}", SEQ.fetch_add(1, Ordering::Relaxed)) diff --git a/acl/src/dpdk/dyn_table.rs b/acl/src/dpdk/dyn_table.rs index 3b2c56f844..56fb431373 100644 --- a/acl/src/dpdk/dyn_table.rs +++ b/acl/src/dpdk/dyn_table.rs @@ -432,9 +432,11 @@ mod failing_repros { offset, } } + use concurrency::sync::LazyLock; use concurrency::sync::atomic::{AtomicU32, Ordering}; - static SEQ: AtomicU32 = AtomicU32::new(0); + // Lazily initialized so this compiles under the loom backend, whose AtomicU32::new is not const + static SEQ: LazyLock = LazyLock::new(|| AtomicU32::new(0)); fn uname(p: &str) -> String { format!("{p}_{}", SEQ.fetch_add(1, Ordering::Relaxed)) } @@ -503,9 +505,11 @@ mod tests { dpdk_table_alias!(type FiveTupleTable = FiveTuple); + use concurrency::sync::LazyLock; use concurrency::sync::atomic::{AtomicU32, Ordering}; - static CTX_SEQ: AtomicU32 = AtomicU32::new(0); + // Lazily initialized so this compiles under the loom backend, whose AtomicU32::new is not const + static CTX_SEQ: LazyLock = LazyLock::new(|| AtomicU32::new(0)); fn unique_name(prefix: &str) -> String { format!("{prefix}_{}", CTX_SEQ.fetch_add(1, Ordering::Relaxed)) } diff --git a/acl/tests/property_dyn_shape.rs b/acl/tests/property_dyn_shape.rs index fe61dd3e5b..2c3120d8c8 100644 --- a/acl/tests/property_dyn_shape.rs +++ b/acl/tests/property_dyn_shape.rs @@ -4,6 +4,7 @@ #![cfg(feature = "dpdk")] #![allow(clippy::expect_used, clippy::unwrap_used)] +use concurrency::sync::LazyLock; use concurrency::sync::atomic::{AtomicU32, AtomicU64, Ordering}; use core::num::NonZero; @@ -147,7 +148,8 @@ impl ValueGenerator for ShapeMisses { } } -static CTX_SEQ: AtomicU32 = AtomicU32::new(0); +// Lazily initialized so this compiles under the loom backend, whose AtomicU32::new is not const +static CTX_SEQ: LazyLock = LazyLock::new(|| AtomicU32::new(0)); fn unique_name(prefix: &str) -> String { format!("{prefix}_{}", CTX_SEQ.fetch_add(1, Ordering::Relaxed)) @@ -208,9 +210,10 @@ where #[test] #[dpdk::with_eal] fn dyn_dpdk_and_reference_agree_on_random_shapes() { - static ASSERTED_HITS: AtomicU64 = AtomicU64::new(0); - static ASSERTED_MISSES: AtomicU64 = AtomicU64::new(0); - static SHAPES_RUN: AtomicU64 = AtomicU64::new(0); + // Lazily initialized so this compiles under the loom backend, whose AtomicU64::new is not const + static ASSERTED_HITS: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static ASSERTED_MISSES: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static SHAPES_RUN: LazyLock = LazyLock::new(|| AtomicU64::new(0)); bolero::check!() .with_type::<(RawShape, Box<[u8]>, Box<[u8]>)>() diff --git a/acl/tests/property_predicate.rs b/acl/tests/property_predicate.rs index 2370b8c411..3935a9ff78 100644 --- a/acl/tests/property_predicate.rs +++ b/acl/tests/property_predicate.rs @@ -4,6 +4,7 @@ #![cfg(feature = "dpdk")] #![allow(clippy::expect_used, clippy::unwrap_used)] +use concurrency::sync::LazyLock; use concurrency::sync::atomic::{AtomicU32, AtomicU64, Ordering}; use core::net::{Ipv4Addr, Ipv6Addr}; use core::num::NonZero; @@ -179,7 +180,8 @@ enum Verdict { Drop, } -static CTX_SEQ: AtomicU32 = AtomicU32::new(0); +// Lazily initialized so this compiles under the loom backend, whose AtomicU32::new is not const +static CTX_SEQ: LazyLock = LazyLock::new(|| AtomicU32::new(0)); fn unique_name(prefix: &str) -> String { format!("{prefix}_{}", CTX_SEQ.fetch_add(1, Ordering::Relaxed)) From 7faebaa04321f749b48a820c540280423bd8264b Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Sun, 19 Jul 2026 20:18:48 +0200 Subject: [PATCH 17/17] feat(acl-filter): add tracing target for acl-filter Add tracectl and linkme as dependencies and define a new tracing target "acl-filter". Otherwise the info!()'s used to log the verdict for a packet would be governed by the default loglevel, which may not be desirable. Signed-off-by: Fredi Raspall --- Cargo.lock | 2 ++ acl-filter/Cargo.toml | 2 ++ acl-filter/src/lib.rs | 3 +++ 3 files changed, 7 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index d68bb4e739..66dc60c5e5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1254,7 +1254,9 @@ dependencies = [ "dataplane-nat", "dataplane-net", "dataplane-pipeline", + "dataplane-tracectl", "indenter", + "linkme", "tokio", "tracing", "tracing-test", diff --git a/acl-filter/Cargo.toml b/acl-filter/Cargo.toml index 15a6ff9350..ce11065fc4 100644 --- a/acl-filter/Cargo.toml +++ b/acl-filter/Cargo.toml @@ -12,11 +12,13 @@ concurrency = { workspace = true } config = { workspace = true } dpdk = { workspace = true } indenter = { workspace = true } +linkme = { workspace = true } lookup = { workspace = true } lpm = { workspace = true } match-action = { workspace = true, features = ["derive"] } net = { workspace = true } pipeline = { workspace = true } +tracectl = { workspace = true } tracing = { workspace = true } [dev-dependencies] diff --git a/acl-filter/src/lib.rs b/acl-filter/src/lib.rs index 1878ad2e83..aaafa896ff 100644 --- a/acl-filter/src/lib.rs +++ b/acl-filter/src/lib.rs @@ -14,6 +14,9 @@ use pipeline::{NetworkFunction, PipelineData}; use std::num::NonZero; use tracing::{debug, info}; +use tracectl::trace_target; +trace_target!("acl-filter", LevelFilter::INFO, &["pipeline"]); + mod access; mod context; mod display;