From 1f4df447bece446fc13d9da610743d8340cd553a Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Wed, 15 Jul 2026 14:01:47 +0100 Subject: [PATCH 01/15] feat(config): Implement From for RangeSpec This is so we can use one protocol, or "any protocol" (by ranging on all existing values) in ACL tables to apply filtering rules to packets. Signed-off-by: Quentin Monnet --- Cargo.lock | 1 + config/Cargo.toml | 3 ++- config/src/external/overlay/acl.rs | 16 ++++++++++++++++ net/src/ip/mod.rs | 2 +- 4 files changed, 20 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..a1ca3e742a 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::RangeSpec; +use net::ip::NextHeader; use tracing::debug; #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] @@ -26,6 +28,20 @@ pub enum AclProtoMatch { Any, } +const TCP: u8 = NextHeader::TCP.as_u8(); +const UDP: u8 = NextHeader::UDP.as_u8(); + +impl From for RangeSpec { + fn from(proto: AclProtoMatch) -> Self { + match proto { + AclProtoMatch::Tcp => RangeSpec::new(TCP, TCP), + AclProtoMatch::Udp => RangeSpec::new(UDP, UDP), + AclProtoMatch::Other(p) => RangeSpec::new(p, p), + AclProtoMatch::Any => RangeSpec::new(0, u8::MAX), + } + } +} + #[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 0280f8850bf3e212fdb5599f58c4a95d5eed02a3 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Mon, 13 Jul 2026 10:53:54 +0100 Subject: [PATCH 02/15] 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 a1ca3e742a..ed4b2bec04 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, @@ -127,8 +127,8 @@ impl ValidatedAclPattern { } #[must_use] - pub fn proto(&self) -> &AclProtoMatch { - &self.proto + pub fn proto(&self) -> AclProtoMatch { + self.proto } } From b1f594a78bd844a778bc5bd6c87819fb83aaffb4 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Mon, 13 Jul 2026 10:54:36 +0100 Subject: [PATCH 03/15] 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 61c2b81c3bf2fef9e18377ca7a04f1a1aa7f392f Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Mon, 13 Jul 2026 15:51:23 +0100 Subject: [PATCH 04/15] 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 ed4b2bec04..85320302cd 100644 --- a/config/src/external/overlay/acl.rs +++ b/config/src/external/overlay/acl.rs @@ -421,6 +421,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 @@ -633,6 +638,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 8d32d9dfdb1f712650b93e47a9c392c845682e60 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Tue, 14 Jul 2026 15:18:36 +0100 Subject: [PATCH 05/15] 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 85320302cd..2210188b28 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, @@ -76,7 +75,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() @@ -722,22 +721,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() { @@ -770,18 +753,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() { @@ -963,7 +934,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 aab05d9be055f4a9d4e3329fa41735443fd71a26 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Wed, 15 Jul 2026 14:04:41 +0100 Subject: [PATCH 06/15] 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 bd5b3438f10e202e2bb768039af64489e3353fe7 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Mon, 13 Jul 2026 11:00:15 +0100 Subject: [PATCH 07/15] 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. Signed-off-by: Quentin Monnet --- Cargo.lock | 16 + Cargo.toml | 7 + acl-filter/Cargo.toml | 23 ++ acl-filter/src/access.rs | 81 +++++ acl-filter/src/context.rs | 487 +++++++++++++++++++++++++++++ acl-filter/src/lib.rs | 206 ++++++++++++ config/src/external/overlay/acl.rs | 12 + lpm/src/prefix/with_ports.rs | 2 +- 8 files changed, 833 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 93775c47c3..1da6cb6327 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1234,6 +1234,22 @@ dependencies = [ "thiserror", ] +[[package]] +name = "dataplane-acl-filter" +version = "0.22.0" +dependencies = [ + "dataplane-acl", + "dataplane-concurrency", + "dataplane-config", + "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..7621b4b59e 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 = true +wasm = false # miss + [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..cf69362062 --- /dev/null +++ b/acl-filter/Cargo.toml @@ -0,0 +1,23 @@ +[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 } +lookup = { workspace = true } +lpm = { workspace = true } +match-action = { workspace = true, features = ["derive"] } +net = { workspace = true } +pipeline = { workspace = true } +tracing = { workspace = true } + +[dev-dependencies] +config = { workspace = true, features = ["testing"] } +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..fe7dc0369b --- /dev/null +++ b/acl-filter/src/access.rs @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Read and write handles for the ACL filter context. + +use crate::context::AclTables; +use concurrency::slot::Slot; +use concurrency::sync::Arc; +use config::ConfigError; +use config::external::overlay::ValidatedOverlay; + +#[derive(Debug, Default, Clone)] +pub struct AclFilterContext { + pub(super) acls: AclTables, +} + +impl TryFrom<&ValidatedOverlay> for AclFilterContext { + type Error = ConfigError; + + fn try_from(overlay: &ValidatedOverlay) -> Result { + let acls = AclTables::from(overlay); + 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 { + 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..b7abc8d500 --- /dev/null +++ b/acl-filter/src/context.rs @@ -0,0 +1,487 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Context table build (ACLs) + +use super::PacketSummary; +use acl::reference::table::{RefRule, ReferenceTable}; +use config::external::overlay::ValidatedOverlay; +use config::external::overlay::acl::{AclAction, AclProtoMatch, AclScope, ValidatedAclRule}; +use lookup::Lookup; +use lpm::prefix::{Prefix, PrefixPortsSet, PrefixWithOptionalPorts}; +use match_action::{Erased, ExactSpec, FieldPredicate, FixedSize, MatchKey, RangeSpec}; +use net::ip::NextHeader; +use net::vxlan::Vni; +use std::collections::HashMap; +use std::fmt::Debug; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; +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 { + tcp_v4: Vec, + udp_v4: Vec, + other_v4: Vec, + + tcp_v6: Vec, + udp_v6: Vec, + other_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())); + match (template_rule.proto, template_rule.is_ipv4) { + (AclProtoMatch::Tcp, true) => self.tcp_v4.push(template_rule), + (AclProtoMatch::Udp, true) => self.udp_v4.push(template_rule), + (AclProtoMatch::Other(_), true) => self.other_v4.push(template_rule), + (AclProtoMatch::Any, true) => { + self.udp_v4.push(template_rule.clone()); + self.tcp_v4.push(template_rule.clone()); + self.other_v4.push(template_rule); + } + + (AclProtoMatch::Tcp, false) => self.tcp_v6.push(template_rule), + (AclProtoMatch::Udp, false) => self.udp_v6.push(template_rule), + (AclProtoMatch::Other(_), false) => self.other_v6.push(template_rule), + (AclProtoMatch::Any, false) => { + self.udp_v6.push(template_rule.clone()); + self.tcp_v6.push(template_rule.clone()); + self.other_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 + } +} + +// ------------------------------------------------------------------------------------------------- +// Backend lowering layer. +// +// This is where source rules are lowered into ACL field predicates +// (`into_backend_fields::()`). Switching the reference backend for a hardware backend +// means changing `RuleBackend` here (and the table types in `PeeringEndsTables`). + +type RuleBackend = Erased; + +#[derive(Debug, MatchKey, Clone, PartialEq, Eq)] +struct IpsPortsTuple { + #[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 IpsPortsTuple { + fn new( + src_vni: Vni, + dst_vni: Vni, + src_ip: I, + dst_ip: I, + src_port: Option, + dst_port: Option, + ) -> Self { + Self { + 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), + } + } + + fn predicates( + src_vni: Vni, + dst_vni: Vni, + src_ip_range: Prefix, + dst_ip_range: Prefix, + src_port_range: RangeSpec, + dst_port_range: RangeSpec, + ) -> Option> { + match (src_ip_range, dst_ip_range) { + (Prefix::IPV4(src_ip_range), Prefix::IPV4(dst_ip_range)) => Some( + IpsPortsTupleRule { + 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( + IpsPortsTupleRule { + 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, + } + } +} + +#[derive(Debug, MatchKey, Clone, PartialEq, Eq)] +struct IpsProtoTuple { + #[exact] + src_vni: u32, + #[exact] + dst_vni: u32, + // A range so a single rule can match one protocol (when user specifies a numerical protocol), + // or any protocol. For the lookup, the packet obviously supplies its single protocol number. + #[range] + proto: u8, + #[prefix] + src_ip_range: I, + #[prefix] + dst_ip_range: I, +} + +impl IpsProtoTuple { + fn predicates( + src_vni: Vni, + dst_vni: Vni, + proto: AclProtoMatch, + src_ip_range: Prefix, + dst_ip_range: Prefix, + ) -> Option> { + match (src_ip_range, dst_ip_range) { + (Prefix::IPV4(src_ip_range), Prefix::IPV4(dst_ip_range)) => Some( + IpsProtoTupleRule { + src_vni: ExactSpec::new(src_vni.as_u32()), + dst_vni: ExactSpec::new(dst_vni.as_u32()), + proto: proto.into(), + src_ip_range: src_ip_range.into(), + dst_ip_range: dst_ip_range.into(), + } + .into_backend_fields::(), + ), + (Prefix::IPV6(src_ip_range), Prefix::IPV6(dst_ip_range)) => Some( + IpsProtoTupleRule { + src_vni: ExactSpec::new(src_vni.as_u32()), + dst_vni: ExactSpec::new(dst_vni.as_u32()), + proto: proto.into(), + src_ip_range: src_ip_range.into(), + dst_ip_range: dst_ip_range.into(), + } + .into_backend_fields::(), + ), + _ => None, + } + } +} + +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 rules into a tuple (prefixes + ports) table. +fn build_ips_ports_tuple( + rules: &[PeeringAclRule], +) -> ReferenceTable, LookupResult> { + let rules = rules + .iter() + .filter_map(|rule| { + Some(RefRule::new( + IpsPortsTuple::::predicates( + 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(); + ReferenceTable::new(rules) +} + +// Lower rules into a tuple (prefixes + proto) table. +fn build_ips_proto_tuple( + rules: &[PeeringAclRule], +) -> ReferenceTable, LookupResult> { + let rules = rules + .iter() + .filter_map(|rule| { + Some(RefRule::new( + IpsProtoTuple::::predicates( + rule.src_vni, + rule.dst_vni, + rule.proto, + rule.src_ip_range.unwrap_or_else(T::wildcard), + rule.dst_ip_range.unwrap_or_else(T::wildcard), + )?, + LookupResult { + action: rule.action, + log: rule.log, + scope: rule.scope, + }, + )) + }) + .collect(); + ReferenceTable::new(rules) +} + +pub(super) trait TupleProto { + fn from_tuple_and_proto(tuple: &T, proto: u8) -> Self; +} + +impl TupleProto> for IpsProtoTuple +where + I: Clone, +{ + fn from_tuple_and_proto(tuple: &IpsPortsTuple, proto: u8) -> Self { + Self { + src_vni: tuple.src_vni, + dst_vni: tuple.dst_vni, + proto, + src_ip_range: tuple.src_ip_range.clone(), + dst_ip_range: tuple.dst_ip_range.clone(), + } + } +} + +#[derive(Debug, Clone)] +pub(super) struct LookupResult { + pub(super) action: AclAction, + pub(super) log: bool, + pub(super) scope: AclScope, +} + +#[derive(Debug, Clone)] +struct PeeringAclTables { + tcp: ReferenceTable, + udp: ReferenceTable, + other: ReferenceTable, +} + +impl Default for PeeringAclTables +where + T: MatchKey, + U: MatchKey, +{ + fn default() -> Self { + Self { + tcp: ReferenceTable::empty(), + udp: ReferenceTable::empty(), + other: ReferenceTable::empty(), + } + } +} + +impl PeeringAclTables, IpsProtoTuple> +where + I: FixedSize + Wildcardable, +{ + fn from_tables( + tcp: &[PeeringAclRule], + udp: &[PeeringAclRule], + other: &[PeeringAclRule], + ) -> Self { + PeeringAclTables { + tcp: build_ips_ports_tuple(tcp), + udp: build_ips_ports_tuple(udp), + other: build_ips_proto_tuple(other), + } + } +} + +#[derive(Debug, Default, Clone)] +pub(super) struct AclTables { + v4: PeeringAclTables, IpsProtoTuple>, + v6: PeeringAclTables, IpsProtoTuple>, + default_actions: HashMap<(Vni, Vni), AclAction>, +} + +impl From for AclTables { + fn from(context: PeeringAclRuleSet) -> Self { + Self { + v4: PeeringAclTables::from_tables(&context.tcp_v4, &context.udp_v4, &context.other_v4), + v6: PeeringAclTables::from_tables(&context.tcp_v6, &context.udp_v6, &context.other_v6), + default_actions: context.default_actions, + } + } +} + +impl From<&ValidatedOverlay> for AclTables { + fn from(overlay: &ValidatedOverlay) -> Self { + PeeringAclRuleSet::from(overlay).into() + } +} + +// ------------------------------------------------------------------------------------------------- +// Lookup logic + +impl PeeringAclTables +where + T: MatchKey + Clone, + U: MatchKey + TupleProto, +{ + fn lookup(&self, proto: NextHeader, tuple: &T) -> Option<&LookupResult> { + match proto { + NextHeader::TCP => self.tcp.lookup(tuple), + NextHeader::UDP => self.udp.lookup(tuple), + _ => { + let proto = proto.as_u8(); + let proto_tuple = U::from_tuple_and_proto(tuple, proto); + self.other.lookup(&proto_tuple) + } + } + } +} + +impl AclTables { + pub(super) fn lookup(&self, p: &PacketSummary) -> Option<&LookupResult> { + let (src_ports, dst_ports) = p.ports.unzip(); + match (p.src_ip, p.dst_ip) { + (IpAddr::V4(src_ip), IpAddr::V4(dst_ip)) => { + let tuple = + IpsPortsTuple::new(p.src_vni, p.dst_vni, src_ip, dst_ip, src_ports, dst_ports); + self.v4.lookup(p.proto, &tuple) + } + (IpAddr::V6(src_ip), IpAddr::V6(dst_ip)) => { + let tuple = + IpsPortsTuple::new(p.src_vni, p.dst_vni, src_ip, dst_ip, src_ports, dst_ports); + self.v6.lookup(p.proto, &tuple) + } + _ => { + error!("Found packet with different IP versions for source and destination!"); + None + } + } + } + + 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..5b2a451ce3 --- /dev/null +++ b/acl-filter/src/lib.rs @@ -0,0 +1,206 @@ +// 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 { + 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/config/src/external/overlay/acl.rs b/config/src/external/overlay/acl.rs index 2210188b28..cb25dfc632 100644 --- a/config/src/external/overlay/acl.rs +++ b/config/src/external/overlay/acl.rs @@ -237,6 +237,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 2ff3acd1da811d314fd50cd5c11c92e8bca06123 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Mon, 13 Jul 2026 13:02:47 +0100 Subject: [PATCH 08/15] feat(acl-filter): Implement Display trait for acl-filter's context Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Quentin Monnet --- Cargo.lock | 2 + acl-filter/Cargo.toml | 2 + acl-filter/src/context.rs | 20 ++-- acl-filter/src/display.rs | 236 +++++++++++++++++++++++++++++++++++++ acl-filter/src/lib.rs | 1 + acl/src/reference/table.rs | 6 + 6 files changed, 257 insertions(+), 10 deletions(-) create mode 100644 acl-filter/src/display.rs diff --git a/Cargo.lock b/Cargo.lock index 1da6cb6327..17b11b1ab3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1239,6 +1239,7 @@ name = "dataplane-acl-filter" version = "0.22.0" dependencies = [ "dataplane-acl", + "dataplane-common", "dataplane-concurrency", "dataplane-config", "dataplane-lookup", @@ -1246,6 +1247,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 cf69362062..3e65200cdc 100644 --- a/acl-filter/Cargo.toml +++ b/acl-filter/Cargo.toml @@ -7,8 +7,10 @@ version.workspace = true [dependencies] acl = { workspace = true } +common = { workspace = true } concurrency = { workspace = true } config = { workspace = true } +indenter = { workspace = true } lookup = { workspace = true } lpm = { workspace = true } match-action = { workspace = true, features = ["derive"] } diff --git a/acl-filter/src/context.rs b/acl-filter/src/context.rs index b7abc8d500..aebf5bbbe7 100644 --- a/acl-filter/src/context.rs +++ b/acl-filter/src/context.rs @@ -161,7 +161,7 @@ impl From<&ValidatedOverlay> for PeeringAclRuleSet { type RuleBackend = Erased; #[derive(Debug, MatchKey, Clone, PartialEq, Eq)] -struct IpsPortsTuple { +pub(super) struct IpsPortsTuple { #[exact] src_vni: u32, #[exact] @@ -232,7 +232,7 @@ impl IpsPortsTuple { } #[derive(Debug, MatchKey, Clone, PartialEq, Eq)] -struct IpsProtoTuple { +pub(super) struct IpsProtoTuple { #[exact] src_vni: u32, #[exact] @@ -281,7 +281,7 @@ impl IpsProtoTuple { } } -trait Wildcardable { +pub(super) trait Wildcardable { fn wildcard() -> Prefix; } @@ -379,10 +379,10 @@ pub(super) struct LookupResult { } #[derive(Debug, Clone)] -struct PeeringAclTables { - tcp: ReferenceTable, - udp: ReferenceTable, - other: ReferenceTable, +pub(super) struct PeeringAclTables { + pub(super) tcp: ReferenceTable, + pub(super) udp: ReferenceTable, + pub(super) other: ReferenceTable, } impl Default for PeeringAclTables @@ -418,9 +418,9 @@ where #[derive(Debug, Default, Clone)] pub(super) struct AclTables { - v4: PeeringAclTables, IpsProtoTuple>, - v6: PeeringAclTables, IpsProtoTuple>, - default_actions: HashMap<(Vni, Vni), AclAction>, + pub(super) v4: PeeringAclTables, IpsProtoTuple>, + pub(super) v6: PeeringAclTables, IpsProtoTuple>, + pub(super) default_actions: HashMap<(Vni, Vni), AclAction>, } impl From for AclTables { diff --git a/acl-filter/src/display.rs b/acl-filter/src/display.rs new file mode 100644 index 0000000000..e22a215211 --- /dev/null +++ b/acl-filter/src/display.rs @@ -0,0 +1,236 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Display implementations for the ACL filter context. +//! +//! The ACL rules are stored in the context after being lowered into positional field predicates +//! (see [`crate::context`]), so the human-readable field values are reconstructed here from the raw +//! bytes of each predicate. This relies on the field layout of the match keys defined in +//! `context.rs`: +//! +//! - `IpsPortsTuple`: src_vni, dst_vni, src_ip, dst_ip, src_port, dst_port (TCP/UDP tables) +//! - `IpsProtoTuple`: src_vni, dst_vni, proto, src_ip, dst_ip (the "other" table) +//! +//! If those layouts change, the decoding below must be updated accordingly. + +use common::cliprovider::{CliSource, Heading}; +use indenter::indented; + +use std::fmt::{self, Display, Write}; +use std::net::{Ipv4Addr, Ipv6Addr}; + +use acl::reference::table::ReferenceTable; +use match_action::{FieldPredicate, MatchKey}; + +use crate::AclFilterContext; +use crate::context::{AclTables, LookupResult}; + +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) + } +} + +impl Display for AclTables { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + writeln!(f, "IPv4:")?; + { + let mut w = indented(f).with_str(" "); + fmt_ref_table(&mut w, "TCP", &self.v4.tcp, Layout::Ports)?; + fmt_ref_table(&mut w, "UDP", &self.v4.udp, Layout::Ports)?; + fmt_ref_table(&mut w, "other", &self.v4.other, Layout::Proto)?; + } + + writeln!(f, "IPv6:")?; + { + let mut w = indented(f).with_str(" "); + fmt_ref_table(&mut w, "TCP", &self.v6.tcp, Layout::Ports)?; + fmt_ref_table(&mut w, "UDP", &self.v6.udp, Layout::Ports)?; + fmt_ref_table(&mut w, "other", &self.v6.other, Layout::Proto)?; + } + + writeln!(f, "default actions:")?; + let mut w = indented(f).with_str(" "); + if self.default_actions.is_empty() { + writeln!(w, "(none)")?; + } else { + // Sort for a deterministic dump. + let mut defaults: Vec<_> = self.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(()) + } +} + +/// The positional field layout of the reference table being dumped, so the decoding below reads +/// the right predicate for each column. +#[derive(Clone, Copy)] +enum Layout { + /// `IpsPortsTuple` (TCP/UDP tables): src_vni, dst_vni, src_ip, dst_ip, src_port, dst_port. + Ports, + /// `IpsProtoTuple` (the "other" table): src_vni, dst_vni, proto, src_ip, dst_ip. + Proto, +} + +/// Format a single reference table as a labelled, numbered list of rules indented under that label. +fn fmt_ref_table( + w: &mut W, + label: &str, + table: &ReferenceTable, + layout: Layout, +) -> fmt::Result { + writeln!(w, "{label}:")?; + let mut w = indented(w).with_str(" "); + if table.is_empty() { + return writeln!(w, "(none)"); + } + for (idx, rule) in table.rules().iter().enumerate() { + let fields = rule.fields(); + let result = rule.action(); + let src_vni = decode_vni(fields.first()); + let dst_vni = decode_vni(fields.get(1)); + let log = if result.log { ", log" } else { "" }; + match layout { + Layout::Ports => { + let src_ip = decode_prefix(fields.get(2)); + let dst_ip = decode_prefix(fields.get(3)); + let src_ports = decode_ports(fields.get(4)); + let dst_ports = decode_ports(fields.get(5)); + writeln!( + w, + "[{idx}] VPC {src_vni} -> VPC {dst_vni} | src {src_ip}:{src_ports} | dst {dst_ip}:{dst_ports} | {:?} ({:?}{log})", + result.action, result.scope + )?; + } + Layout::Proto => { + let proto = decode_proto(fields.get(2)); + let src_ip = decode_prefix(fields.get(3)); + let dst_ip = decode_prefix(fields.get(4)); + writeln!( + w, + "[{idx}] VPC {src_vni} -> VPC {dst_vni} | proto {proto} | src {src_ip} | dst {dst_ip} | {:?} ({:?}{log})", + result.action, result.scope + )?; + } + } + } + Ok(()) +} + +/// Decode a VNI stored as a 4-byte big-endian exact-match predicate. +fn decode_vni(predicate: Option<&FieldPredicate>) -> String { + match predicate.and_then(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). +fn decode_prefix(predicate: Option<&FieldPredicate>) -> String { + match predicate.and_then(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 number stored as a 1-byte range. A full range renders as `any`, an exact +/// match as the single protocol number. +fn decode_proto(predicate: Option<&FieldPredicate>) -> String { + match predicate.and_then(FieldPredicate::as_range) { + Some(([lo], [hi])) => { + if *lo == 0 && *hi == u8::MAX { + "any".to_string() + } else if lo == hi { + lo.to_string() + } else { + format!("{lo}-{hi}") + } + } + _ => "?".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. +fn decode_ports(predicate: Option<&FieldPredicate>) -> String { + match predicate.and_then(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, 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_exact_and_range() { + let any = IntoBackendField::::into_backend_field(RangeSpec::new(0u8, u8::MAX)); + assert_eq!(decode_proto(Some(&any)), "any"); + + let exact = IntoBackendField::::into_backend_field(RangeSpec::new(1u8, 1u8)); + assert_eq!(decode_proto(Some(&exact)), "1"); + + let range = IntoBackendField::::into_backend_field(RangeSpec::new(1u8, 132u8)); + assert_eq!(decode_proto(Some(&range)), "1-132"); + } +} diff --git a/acl-filter/src/lib.rs b/acl-filter/src/lib.rs index 5b2a451ce3..cb4e0ac21b 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, 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; From c9eed619cf69f9d9fdfc80913107b0e8e5c832fc Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Mon, 13 Jul 2026 16:19:22 +0100 Subject: [PATCH 09/15] 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) Signed-off-by: Quentin Monnet --- Cargo.lock | 4 + acl-filter/Cargo.toml | 4 + acl-filter/src/lib.rs | 3 + acl-filter/src/tests.rs | 993 +++++++++++++++++++++++++++++ config/src/external/overlay/acl.rs | 2 +- 5 files changed, 1005 insertions(+), 1 deletion(-) create mode 100644 acl-filter/src/tests.rs diff --git a/Cargo.lock b/Cargo.lock index 17b11b1ab3..f0c27292ac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1242,12 +1242,16 @@ dependencies = [ "dataplane-common", "dataplane-concurrency", "dataplane-config", + "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 3e65200cdc..b6b9b9ff59 100644 --- a/acl-filter/Cargo.toml +++ b/acl-filter/Cargo.toml @@ -20,6 +20,10 @@ tracing = { workspace = true } [dev-dependencies] config = { workspace = true, features = ["testing"] } +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 cb4e0ac21b..402528e268 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..4354dc277d --- /dev/null +++ b/acl-filter/src/tests.rs @@ -0,0 +1,993 @@ +// 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(); + writer.store(AclFilterContext::try_from(overlay).unwrap()); + 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: false, + } +} + +// 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}; + + // 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::try_from(overlay).unwrap()); + 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) + } + + #[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())); + } +} diff --git a/config/src/external/overlay/acl.rs b/config/src/external/overlay/acl.rs index cb25dfc632..51c96901d9 100644 --- a/config/src/external/overlay/acl.rs +++ b/config/src/external/overlay/acl.rs @@ -392,7 +392,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 b63f9f305fdfd96c39a11b9a03e5164e22561e49 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Mon, 13 Jul 2026 11:01:21 +0100 Subject: [PATCH 10/15] 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 f0c27292ac..44e5f1f0f7 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", @@ -1632,6 +1633,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 83d86fcba4..31edf5b62f 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 7610261432..c09bf758e4 100644 --- a/dataplane/src/runtime.rs +++ b/dataplane/src/runtime.rs @@ -278,6 +278,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 b8e9d9fa41..4c7f0aff72 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 778c2ffdca..d47e5a6f92 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; @@ -455,6 +456,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(); @@ -476,6 +480,7 @@ pub mod test { nattablesw, natallocatorw, flowfilterw, + aclfilterw, portfw_w, vpc_stats_store, dp_status_r, From e4615d1226e7e927e8ca57e4c89f6fa18c39c9b7 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Tue, 14 Jul 2026 01:05:33 -0600 Subject: [PATCH 11/15] feat(acl): gate reference backend behind a feature; 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 --- Cargo.lock | 277 +++++++++++++++++++------------------- acl-filter/Cargo.toml | 5 +- 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 + 7 files changed, 172 insertions(+), 149 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 44e5f1f0f7..3bda793456 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -239,7 +239,7 @@ checksum = "c7c24de15d275a1ecfd47a380fb4d5ec9bfe0933f309ed5e705b775596a3574d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -250,7 +250,7 @@ checksum = "9035ad2d096bed7955a320ee7e2230574d28fd3c3a0f186cbea1ff3c7eed5dbb" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -405,7 +405,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "993776b509cfb49c750f11b8f07a46fa23e0a1386ffc01fb1e7d343efc387895" dependencies = [ "annotate-snippets 0.11.5", - "bitflags 2.13.0", + "bitflags 2.13.1", "cexpr", "clang-sys", "itertools 0.13.0", @@ -414,7 +414,7 @@ dependencies = [ "regex", "rustc-hash", "shlex 1.3.0", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -425,9 +425,9 @@ checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" [[package]] name = "bitflags" -version = "2.13.0" +version = "2.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b4388bee8683e3d04af747c73422af53102d2bd24d9eadb6cbc100baef4b43f8" +checksum = "b588b76d00fde79687d7646a9b5bdf3cc0f655e0bbd080335a95d7e96f3587da" dependencies = [ "serde_core", ] @@ -530,7 +530,7 @@ dependencies = [ "proc-macro-crate 2.0.2", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -636,7 +636,7 @@ checksum = "89385e82b5d1821d2219e0b095efa2cc1f246cbf99080f3be46a1a85c0d392d9" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -693,7 +693,7 @@ dependencies = [ "quote", "serde", "serde_json", - "syn 2.0.118", + "syn 2.0.119", "tempfile", "toml", ] @@ -727,9 +727,9 @@ checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" [[package]] name = "cfg_aliases" -version = "0.2.1" +version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" +checksum = "f079e83a288787bcd14a6aea84cee5c87a67c5a3e660c30f557a3d24761b3527" [[package]] name = "chacha20" @@ -794,9 +794,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.6.1" +version = "4.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1ddb117e43bbf7dacf0a4190fef4d345b9bad68dfc649cb349e7d17d28428e51" +checksum = "dd059f9da4f5c36b3787f65d38ccaab1cc315f07b01f89abc8359ee6a8205011" dependencies = [ "clap_builder", "clap_derive", @@ -804,9 +804,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.6.0" +version = "4.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "714a53001bf66416adb0e2ef5ac857140e7dc3a0c48fb28b2f10762fc4b5069f" +checksum = "f09628afdcc538b57f3c6341e9c8e9970f18e4a481690a64974d7023bd33548b" dependencies = [ "anstream", "anstyle", @@ -823,7 +823,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -900,7 +900,7 @@ version = "3.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -1048,7 +1048,7 @@ version = "0.29.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d8b9f2e4c67f833b660cdb0a3523065869fb35570177239812ed4c905aeff87b" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "crossterm_winapi", "derive_more", "document-features", @@ -1116,7 +1116,7 @@ dependencies = [ "proc-macro2", "quote", "strsim", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -1129,7 +1129,7 @@ dependencies = [ "proc-macro2", "quote", "strsim", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -1140,7 +1140,7 @@ checksum = "fc34b93ccb385b40dc71c6fceac4b2ad23662c7eeb248cf10d529b7e055b6ead" dependencies = [ "darling_core 0.20.11", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -1151,7 +1151,7 @@ checksum = "ac3984ec7bd6cfa798e62b4a642426a5be0e68f9401cfc2a01e3fa9ea2fcdb8d" dependencies = [ "darling_core 0.23.0", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -1227,6 +1227,7 @@ dependencies = [ "arrayvec", "bolero", "criterion", + "dataplane-acl", "dataplane-concurrency", "dataplane-dpdk", "dataplane-lookup", @@ -1324,7 +1325,7 @@ dependencies = [ "proc-macro-crate 3.5.0", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -1392,7 +1393,7 @@ dependencies = [ "proc-macro-crate 3.5.0", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -1431,7 +1432,7 @@ dependencies = [ name = "dataplane-flow-filter" version = "0.22.0" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "dataplane-common", "dataplane-concurrency", "dataplane-config", @@ -1622,7 +1623,7 @@ dependencies = [ "proc-macro-crate 3.5.0", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -1712,7 +1713,7 @@ dependencies = [ "ahash", "arrayvec", "atomic-instant-full", - "bitflags 2.13.0", + "bitflags 2.13.1", "bolero", "bytecheck", "dataplane-common", @@ -1761,7 +1762,7 @@ dependencies = [ "ahash", "anyhow", "async-trait", - "bitflags 2.13.0", + "bitflags 2.13.1", "bolero", "bytes", "chrono", @@ -1914,7 +1915,7 @@ dependencies = [ "defmt-parser", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -1950,7 +1951,7 @@ dependencies = [ "darling 0.20.11", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -1960,7 +1961,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ab63b0e2bf4d5928aff72e83a7dace85d7bba5fe12dcc3c5a572d78caffd3f3c" dependencies = [ "derive_builder_core", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -1982,7 +1983,7 @@ dependencies = [ "proc-macro2", "quote", "rustc_version", - "syn 2.0.118", + "syn 2.0.119", "unicode-xid", ] @@ -2008,7 +2009,7 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1e0e367e4e7da84520dedcac1901e4da967309406d1e51017ae1abfb97adbd38" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "block2", "libc", "objc2", @@ -2022,7 +2023,7 @@ checksum = "1ac70aa55017e108007fbaf5aa0f54b021c98f92ff8af59d42eda9da96e3dd4f" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -2103,7 +2104,7 @@ dependencies = [ "enum-ordinalize", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -2115,7 +2116,7 @@ dependencies = [ "enum-ordinalize", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -2159,7 +2160,7 @@ checksum = "42e528e2d34ba8a67a1a650b86beae8ef69fc5fdb638016f386b973226590432" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -2179,7 +2180,7 @@ checksum = "44f23cf4b44bfce11a86ace86f8a73ffdec849c9fd00a386a53d278bd9e81fb3" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -2195,7 +2196,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -2306,7 +2307,7 @@ source = "git+https://github.com/githedgehog/fixin?branch=main#5e0de31606466b173 dependencies = [ "proc-macro-error2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -2447,7 +2448,7 @@ checksum = "e835b70203e41293343137df5c0664546da5745f82ec9b84d40be8336958447b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -2680,9 +2681,9 @@ dependencies = [ [[package]] name = "http-body" -version = "1.0.1" +version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1efedce1fb8e6913f23e0c92de8e62cd5b772a67e7b3946df930a62566c93184" +checksum = "ca2a8f2913ee65f60facd6a5905613afaa448497a0230cc41ce022d93290bc2c" dependencies = [ "bytes", "http", @@ -2690,9 +2691,9 @@ dependencies = [ [[package]] name = "http-body-util" -version = "0.1.3" +version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b021d93e26becf5dc7e1b75b1bed1fd93124b374ceb73f43d4d4eafec896a64a" +checksum = "e9f41fd6a08e4d4ec69df65976da761afd5ad5e58a9d4acb46bd1c953a9e3ff2" dependencies = [ "bytes", "futures-core", @@ -2720,7 +2721,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4c2e65a48d3b300843ac84a2fe8e166bb5a5b00f30054593bcee8157e4b465fd" dependencies = [ "arrayvec", - "bitflags 2.13.0", + "bitflags 2.13.1", "derive_more", "errno", "hwlocality-sys", @@ -3010,7 +3011,7 @@ version = "0.11.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "153be1941a183ec9ccd095ddbe17a8b8d435ef6c76e9e02451b933c3999af2c8" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "futures-util", "inotify-sys", "libc", @@ -3102,7 +3103,7 @@ checksum = "d0879bd39df99c4c5e2c6615ccc026391a423dde10532c573e6086eb94a802cc" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -3140,9 +3141,9 @@ dependencies = [ [[package]] name = "jsonpath-rust" -version = "1.0.4" +version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "633a7320c4bb672863a3782e89b9094ad70285e097ff6832cddd0ec615beadfa" +checksum = "4222e00941bfe18bf81b79fa23ad933e233bfa18f3ee27254c5dd9b1543b5d5f" dependencies = [ "pest", "pest_derive", @@ -3262,7 +3263,7 @@ dependencies = [ "quote", "serde", "serde_json", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -3376,7 +3377,7 @@ checksum = "32d59e20403c7d08fe62b4376edfe5c7fb2ef1e6b1465379686d0f21c8df444b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -3560,7 +3561,7 @@ checksum = "db5b29714e950dbb20d5e6f74f9dcec4edbcc1067bb7f8ed198c097b8c1a818b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -3587,9 +3588,9 @@ dependencies = [ [[package]] name = "mio" -version = "1.2.1" +version = "1.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "02bd0af71c67b473010cbbc60715ee815645a4dc942899111f494b4b737d6fda" +checksum = "30d65c71f1ce40ab09135ce117d742b9f8a19ff91a41a8b57ed50bc2de59c427" dependencies = [ "libc", "log", @@ -3637,7 +3638,7 @@ checksum = "4568f25ccbd45ab5d5603dc34318c1ec56b117531781260002151b8530a9f931" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -3669,7 +3670,7 @@ source = "git+https://github.com/githedgehog/testn.git?tag=v0.0.10#e49aba8400beb dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -3720,7 +3721,7 @@ version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d3cd0bf490a6766e6a21bb64232f38afbacfb5dc2c3ef585c98cd368b921e381" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "byteorder", "bytes", "chrono", @@ -3774,7 +3775,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f6d903f5b7a616c70b4d80ccc274a853edd8e5db6ebe7433f2c326eb7bb675c1" dependencies = [ "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -3791,7 +3792,7 @@ name = "netlink-packet-route" version = "0.30.0" source = "git+https://github.com/githedgehog/netlink-packet-route.git?branch=pr%2Fdaniel-noland%2Fswing6#9a257c60e25bc5db50a1cd14aa493d6ec294c23d" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "byteorder", "libc", "log", @@ -3804,7 +3805,7 @@ version = "0.31.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e2288fcb784eb3defd5fb16f4c4160d5f477de192eac730f43e1d11c24d9a007" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "libc", "log", "netlink-packet-core", @@ -3854,7 +3855,7 @@ version = "0.29.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "71e2746dc3a24dd78b3cfcb7be93368c6de9963d30f43a6a73998a9cf4b17b46" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "cfg-if", "cfg_aliases", "libc", @@ -3867,7 +3868,7 @@ version = "0.31.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf20d2fde8ff38632c426f1165ed7436270b44f199fc55284c38276f9db47c3d" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "cfg-if", "cfg_aliases", "libc", @@ -3905,7 +3906,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -3922,7 +3923,7 @@ checksum = "ed3955f1a9c7c0c15e092f9c887db08b1fc683305fdf6eb6684f22555355e202" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -3949,7 +3950,7 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2a180dd8642fa45cdb7dd721cd4c11b1cadd4929ce112ebd8b9f5803cc79d536" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "block2", "dispatch2", "libc", @@ -3962,7 +3963,7 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c71e34919aba0d701380d911702455038a8a3587467fe0141d6a71501e7ffe48" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "objc2", "objc2-core-foundation", "objc2-foundation", @@ -3982,7 +3983,7 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e3e0adef53c21f888deb4fa59fc59f7eb17404926ee8a6f59f5df0fd7f9f3272" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "block2", "libc", "objc2", @@ -4163,7 +4164,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "97f6fccfd2d9d2df765ca23ff85fe5cc437fb0e6d3e164e4d3cbe09d14780c93" dependencies = [ "arrayvec", - "bitflags 2.13.0", + "bitflags 2.13.1", "thiserror", "zerocopy", "zerocopy-derive", @@ -4215,7 +4216,7 @@ dependencies = [ "pest_meta", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -4282,7 +4283,7 @@ checksum = "c96395f0a926bc13b1c17622aaddda1ecb55d49c8f1bf9777e4d877800a43f8b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -4393,7 +4394,7 @@ version = "3.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e67ba7e9b2b56446f1d419b1d807906278ffa1a658a8a5d8a39dcb1f5a78614f" dependencies = [ - "toml_edit 0.25.12+spec-1.1.0", + "toml_edit 0.25.13+spec-1.1.0", ] [[package]] @@ -4415,7 +4416,7 @@ dependencies = [ "proc-macro-error-attr2", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -4433,7 +4434,7 @@ version = "0.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "25485360a54d6861439d60facef26de713b1e126bf015ec8f98239467a2b82f7" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "procfs-core", "rustix", ] @@ -4444,7 +4445,7 @@ version = "0.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6401bf7b6af22f78b563665d15a22e9aef27775b79b149a66ca022468a4e405" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "hex", ] @@ -4468,7 +4469,7 @@ dependencies = [ "itertools 0.14.0", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -4488,7 +4489,7 @@ checksum = "7347867d0a7e1208d93b46767be83e2b8f978c3dad35f775ac8d8847551d6fe1" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -4689,7 +4690,7 @@ version = "11.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "498cd0dc59d73224351ee52a95fee0f1a617a2eae0e7d9d720cc622c73a54186" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", ] [[package]] @@ -4698,7 +4699,7 @@ version = "0.5.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed2bf2547551a7053d6fdfafda3f938979645c44812fbfcda098faae3f1a362d" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", ] [[package]] @@ -4738,14 +4739,14 @@ checksum = "b7186006dcb21920990093f30e3dea63b7d6e977bf1256be20c3563a5db070da" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] name = "regex" -version = "1.13.0" +version = "1.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a0e75113e14dc5acb068cd0786884f214f1312650a3d36d269f5c4f3cdee8a2" +checksum = "f020237b6c8eed93db2e2cb53c00c60a8e1bc73da7d073199a1180401450218d" dependencies = [ "aho-corasick", "memchr", @@ -4755,9 +4756,9 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.4.15" +version = "0.4.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1f388202e4b80542a0921078cc23b6333bcf1409c1e3f86404cae4766a6131db" +checksum = "8fcfdb36bda0c880c5931cdc7a2bcdc8ba4556847b9d912bca70bc94708711ad" dependencies = [ "aho-corasick", "memchr", @@ -4853,7 +4854,7 @@ checksum = "c0ed1a78a1b19d184b0daa629dd9a024573173ec7d485b287cb369fb3607cc1c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -4913,18 +4914,18 @@ version = "1.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6fe4565b9518b83ef4f91bb47ce29620ca828bd32cb7e408f0062e9930ba190" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "errno", "libc", "linux-raw-sys", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] name = "rustls" -version = "0.23.41" +version = "0.23.42" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6b92b125634d9b795e7beca796cc790df15a7fb38323bf3196fda83292d06b1f" +checksum = "3c54fcab019b409d04215d3a17cb438fd7fbf192ee61461f20f4fe18704bc138" dependencies = [ "aws-lc-rs", "log", @@ -5029,7 +5030,7 @@ dependencies = [ "proc-macro2", "quote", "serde_derive_internals", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -5059,7 +5060,7 @@ version = "3.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b7f4bc775c73d9a02cde8bf7b2ec4c9d12743edf609006c7facc23998404cd1d" dependencies = [ - "bitflags 2.13.0", + "bitflags 2.13.1", "core-foundation", "core-foundation-sys", "libc", @@ -5138,7 +5139,7 @@ checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -5149,7 +5150,7 @@ checksum = "18d26a20a969b9e3fdf2fc2d9f21eda6c40e2de84c9408bb5d3b05d499aae711" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -5173,7 +5174,7 @@ checksum = "175ee3e80ae9982737ca543e96133087cbd9a485eecc3bc4de9c1a37b47ea59c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -5229,7 +5230,7 @@ checksum = "94e153fc76e1c6a068703d6d29c508a0b15c061c4b7e43da59cc097bc342673c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -5324,9 +5325,9 @@ dependencies = [ [[package]] name = "simd-adler32" -version = "0.3.9" +version = "0.3.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "703d5c7ef118737c72f1af64ad2f6f8c5e1921f818cdcb97b8fe6fc69bf66214" +checksum = "3a219298ac11a56ea9a6d2120044824d6f01aeb034955e7af7bc16858527deea" [[package]] name = "simdutf8" @@ -5372,9 +5373,9 @@ checksum = "8ed6a63f02c8539c91a8685a86f4099661ba3da017932f6ebbea6de3f0fa7c90" [[package]] name = "socket2" -version = "0.6.4" +version = "0.6.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "52d1cfed4120b4d927bf7c0f86d2087a4a7d6027c906d9f9d525a80573b9be51" +checksum = "c3d1e2c7f27f8d4cb10542a02c49005dbd6e93095799d6f3be745fae9f8fedd4" dependencies = [ "libc", "windows-sys 0.61.2", @@ -5382,9 +5383,9 @@ dependencies = [ [[package]] name = "spin" -version = "0.12.1" +version = "0.12.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bd5231412d905519dca6a5deb0327d407be68d6c941feec004533401d3a0a715" +checksum = "8abadc99fd9c7bbb7d0ca2b31d72a067d0c0dcd7aad25ab8cac71ba91417694b" dependencies = [ "lock_api", ] @@ -5443,7 +5444,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -5455,7 +5456,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -5520,9 +5521,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.118" +version = "2.0.119" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b9ae57f904213ebb649ce6895b8a66c66f0203b9319718f69a5612a065b1422" +checksum = "872831b642d1a07999a962a351ed35b955ea2cfc8f3862091e2a240a84f17297" dependencies = [ "proc-macro2", "quote", @@ -5546,7 +5547,7 @@ checksum = "728a70f3dbaf5bab7f0c4b1ac8d7ae5ea60a4b5549c8a5914361c99147a709d2" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -5562,10 +5563,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand 2.4.1", - "getrandom 0.3.4", + "getrandom 0.4.3", "once_cell", "rustix", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -5575,7 +5576,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "230a1b821ccbd75b185820a1f1ff7b14d21da1e442e22c0863ea5f08771a8874" dependencies = [ "rustix", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -5605,7 +5606,7 @@ checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -5684,9 +5685,9 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" -version = "1.52.3" +version = "1.53.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8fc7f01b389ac15039e4dc9531aa973a135d7a4135281b12d7c1bc79fd57fffe" +checksum = "d988bcd52dbe076d3d46903332f58c912b87a2c49b1428419a5845154762ffee" dependencies = [ "bytes", "libc", @@ -5701,13 +5702,13 @@ dependencies = [ [[package]] name = "tokio-macros" -version = "2.7.0" +version = "2.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "385a6cb71ab9ab790c5fe8d67f1645e6c450a7ce006a33de03daa956cf70a496" +checksum = "6328af13490e73a9b4694030fafd93f8c8c6a9dede33e821c3fc63eddf8042ba" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -5826,14 +5827,14 @@ dependencies = [ [[package]] name = "toml_edit" -version = "0.25.12+spec-1.1.0" +version = "0.25.13+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d2153edc6955a6c354fad8f5efd38b6a8769bdccf9fe50f8e1329f81b0baa5d7" +checksum = "6975367e4d2ef766d86af01ffad14b622fecc8d4357a998fbc4deb6e9bacaf9b" dependencies = [ "indexmap", "toml_datetime 1.1.1+spec-1.1.0", "toml_parser", - "winnow 1.0.3", + "winnow 1.0.4", ] [[package]] @@ -5842,14 +5843,14 @@ version = "1.1.2+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2abe9b86193656635d2411dc43050282ca48aa31c2451210f4202550afb7526" dependencies = [ - "winnow 1.0.3", + "winnow 1.0.4", ] [[package]] name = "toml_writer" -version = "1.1.1+spec-1.1.0" +version = "1.1.2+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "756daf9b1013ebe47a8776667b466417e2d4c5679d441c26230efd9ef78692db" +checksum = "7d56353a2a665ad0f41a421187180aab746c8c325620617ad883a99a1cbe66d2" [[package]] name = "tower" @@ -5875,7 +5876,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4cfcf7e2740e6fc6d4d688b4ef00650406bb94adf4731e43c096c3a19fe40840" dependencies = [ "base64", - "bitflags 2.13.0", + "bitflags 2.13.1", "bytes", "futures-util", "http", @@ -5921,7 +5922,7 @@ checksum = "7490cfa5ec963746568740651ac6781f701c9c5ea257c58e057f3ba8cf69e8da" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -5991,7 +5992,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ad06847b7afb65c7866a36664b75c40b895e318cea4f71299f013fb22965329d" dependencies = [ "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -6099,9 +6100,9 @@ checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" [[package]] name = "uuid" -version = "1.23.5" +version = "1.24.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ea5fab0d6c3c01ae70085a09cb03d4c7a1d6314e2b3e075392783396d724ca0a" +checksum = "bf3923a6f5c4c6382e0b653c4117f48d631ea17f38ed86e2a828e6f7412f5239" dependencies = [ "getrandom 0.4.3", "js-sys", @@ -6223,7 +6224,7 @@ dependencies = [ "bumpalo", "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", "wasm-bindgen-shared", ] @@ -6268,7 +6269,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -6298,7 +6299,7 @@ checksum = "053e2e040ab57b9dc951b72c264860db7eb3b0200ba345b4e4c3b14f67855ddf" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -6309,7 +6310,7 @@ checksum = "3f316c4a2570ba26bbec722032c4099d8c8bc095efccdc15688708623367e358" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -6444,9 +6445,9 @@ checksum = "df79d97927682d2fd8adb29682d1140b343be4ac0f08fd68b7765d9c059d3945" [[package]] name = "winnow" -version = "1.0.3" +version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0592e1c9d151f854e6fd382574c3a0855250e1d9b2f99d9281c6e6391af352f1" +checksum = "23b97319f7b8343df12cc98938e5c3eb436064524c8d2b4e30a1d3a36eecdf81" dependencies = [ "memchr", ] @@ -6503,7 +6504,7 @@ checksum = "de844c262c8848816172cef550288e7dc6c7b7814b4ee56b3e1553f275f1858e" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", "synstructure", ] @@ -6524,7 +6525,7 @@ checksum = "e2e817b7b52d0c7358d3246da9d69935ebb18116b2b102b4230dac079b4862f5" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] @@ -6544,7 +6545,7 @@ checksum = "11532158c46691caf0f2593ea8358fed6bbf68a0315e80aae9bd41fbade684a1" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", "synstructure", ] @@ -6584,11 +6585,11 @@ checksum = "625dc425cab0dca6dc3c3319506e6593dcb08a9f387ea3b284dbd52a92c40555" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 2.0.119", ] [[package]] name = "zmij" -version = "1.0.22" +version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bd2f034a4bebf216c9e4b7083603e024cf930873fd67830cfb083c9fa33129d9" +checksum = "29666d0abbfad1e3dc4dcf6144730dd3a3ab225bbbdac83319345b1b44ccfc1b" diff --git a/acl-filter/Cargo.toml b/acl-filter/Cargo.toml index b6b9b9ff59..4bacd1f613 100644 --- a/acl-filter/Cargo.toml +++ b/acl-filter/Cargo.toml @@ -6,7 +6,10 @@ publish.workspace = true version.workspace = true [dependencies] -acl = { workspace = true } +# `reference` is now a non-default feature of `acl` (the crate defaults to the +# dpdk backend). acl-filter's tables are still built on the reference backend +# during the switch to dpdk, so enable it explicitly alongside the default dpdk. +acl = { workspace = true, features = ["reference"] } common = { workspace = true } concurrency = { workspace = true } config = { workspace = true } 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 d281b19730a71667b8b045cdc3d382f948fb6acd Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Tue, 14 Jul 2026 01:06:28 -0600 Subject: [PATCH 12/15] 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 --- 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 3bda793456..6e754ae2ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1186,6 +1186,7 @@ dependencies = [ "dataplane-args", "dataplane-concurrency", "dataplane-config", + "dataplane-dpdk", "dataplane-flow-entry", "dataplane-flow-filter", "dataplane-id", @@ -1638,6 +1639,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 31edf5b62f..44cf70278b 100644 --- a/dataplane/Cargo.toml +++ b/dataplane/Cargo.toml @@ -20,6 +20,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 c09bf758e4..0c0bc2e356 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 4c7f0aff72..f0fb215b5b 100644 --- a/mgmt/Cargo.toml +++ b/mgmt/Cargo.toml @@ -57,6 +57,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 d47e5a6f92..e8fa828c62 100644 --- a/mgmt/src/tests/mgmt.rs +++ b/mgmt/src/tests/mgmt.rs @@ -410,6 +410,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 ff532cfc17da71d2dbb5a33764408311c4552748 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 17 Jul 2026 01:16:51 -0600 Subject: [PATCH 13/15] refactor(acl-filter): fold protocol into a masked key field, one table per IP version rte_acl requires the first key field to be a single byte, but the tuple keys led with a u32 VNI. Restructure to a single `AclKey` per IP version whose first field is a `#[mask]` protocol byte: it satisfies the one-byte-first-field rule and carries the protocol match in the key (exact via mask 0xff, any via mask 0x00). This collapses the six per-protocol tables (tcp/udp/other x v4/v6) into two (v4/v6) and removes the duplication of every `Any` rule across protocols. All rules for an IP version now live in one ordered table, so first-match precedence holds across protocol kinds, not just within a protocol's table. Config validation forbids ports on non-TCP/UDP protocols, so those rules always carry a wildcard port range. Lower `AclProtoMatch` to `MaskSpec` (replacing the now-unused RangeSpec conversion). Rules are still lowered via the backend-neutral `Erased` path; this only changes the key shape so it is rte_acl-installable. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Daniel Noland --- acl-filter/src/context.rs | 318 +++++++++-------------------- acl-filter/src/display.rs | 100 ++++----- config/src/external/overlay/acl.rs | 17 +- 3 files changed, 140 insertions(+), 295 deletions(-) diff --git a/acl-filter/src/context.rs b/acl-filter/src/context.rs index aebf5bbbe7..db7c7a88f2 100644 --- a/acl-filter/src/context.rs +++ b/acl-filter/src/context.rs @@ -9,11 +9,9 @@ use config::external::overlay::ValidatedOverlay; use config::external::overlay::acl::{AclAction, AclProtoMatch, AclScope, ValidatedAclRule}; use lookup::Lookup; use lpm::prefix::{Prefix, PrefixPortsSet, PrefixWithOptionalPorts}; -use match_action::{Erased, ExactSpec, FieldPredicate, FixedSize, MatchKey, RangeSpec}; -use net::ip::NextHeader; +use match_action::{Erased, ExactSpec, FieldPredicate, FixedSize, MaskSpec, MatchKey, RangeSpec}; use net::vxlan::Vni; use std::collections::HashMap; -use std::fmt::Debug; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use tracing::error; @@ -34,13 +32,12 @@ struct PeeringAclRule { #[derive(Debug, Default, Clone, PartialEq, Eq)] struct PeeringAclRuleSet { - tcp_v4: Vec, - udp_v4: Vec, - other_v4: Vec, - - tcp_v6: Vec, - udp_v6: Vec, - other_v6: Vec, + // 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>, } @@ -96,24 +93,11 @@ impl PeeringAclRuleSet { 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())); - match (template_rule.proto, template_rule.is_ipv4) { - (AclProtoMatch::Tcp, true) => self.tcp_v4.push(template_rule), - (AclProtoMatch::Udp, true) => self.udp_v4.push(template_rule), - (AclProtoMatch::Other(_), true) => self.other_v4.push(template_rule), - (AclProtoMatch::Any, true) => { - self.udp_v4.push(template_rule.clone()); - self.tcp_v4.push(template_rule.clone()); - self.other_v4.push(template_rule); - } - - (AclProtoMatch::Tcp, false) => self.tcp_v6.push(template_rule), - (AclProtoMatch::Udp, false) => self.udp_v6.push(template_rule), - (AclProtoMatch::Other(_), false) => self.other_v6.push(template_rule), - (AclProtoMatch::Any, false) => { - self.udp_v6.push(template_rule.clone()); - self.tcp_v6.push(template_rule.clone()); - self.other_v6.push(template_rule); - } + // 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); } } } @@ -156,12 +140,25 @@ impl From<&ValidatedOverlay> for PeeringAclRuleSet { // // This is where source rules are lowered into ACL field predicates // (`into_backend_fields::()`). Switching the reference backend for a hardware backend -// means changing `RuleBackend` here (and the table types in `PeeringEndsTables`). +// means changing `RuleBackend` here (and the table types in `AclTables`). type RuleBackend = Erased; +/// 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 IpsPortsTuple { +pub(super) struct AclKey { + #[mask] + proto: u8, #[exact] src_vni: u32, #[exact] @@ -176,8 +173,9 @@ pub(super) struct IpsPortsTuple { dst_port_range: u16, } -impl IpsPortsTuple { +impl AclKey { fn new( + proto: u8, src_vni: Vni, dst_vni: Vni, src_ip: I, @@ -186,6 +184,7 @@ impl IpsPortsTuple { dst_port: Option, ) -> Self { Self { + proto, src_vni: src_vni.as_u32(), dst_vni: dst_vni.as_u32(), src_ip_range: src_ip, @@ -194,90 +193,47 @@ impl IpsPortsTuple { dst_port_range: dst_port.unwrap_or(0), } } - - fn predicates( - src_vni: Vni, - dst_vni: Vni, - src_ip_range: Prefix, - dst_ip_range: Prefix, - src_port_range: RangeSpec, - dst_port_range: RangeSpec, - ) -> Option> { - match (src_ip_range, dst_ip_range) { - (Prefix::IPV4(src_ip_range), Prefix::IPV4(dst_ip_range)) => Some( - IpsPortsTupleRule { - 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( - IpsPortsTupleRule { - 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, - } - } } -#[derive(Debug, MatchKey, Clone, PartialEq, Eq)] -pub(super) struct IpsProtoTuple { - #[exact] - src_vni: u32, - #[exact] - dst_vni: u32, - // A range so a single rule can match one protocol (when user specifies a numerical protocol), - // or any protocol. For the lookup, the packet obviously supplies its single protocol number. - #[range] - proto: u8, - #[prefix] - src_ip_range: I, - #[prefix] - dst_ip_range: I, -} - -impl IpsProtoTuple { - fn predicates( - src_vni: Vni, - dst_vni: Vni, - proto: AclProtoMatch, - src_ip_range: Prefix, - dst_ip_range: Prefix, - ) -> Option> { - match (src_ip_range, dst_ip_range) { - (Prefix::IPV4(src_ip_range), Prefix::IPV4(dst_ip_range)) => Some( - IpsProtoTupleRule { - src_vni: ExactSpec::new(src_vni.as_u32()), - dst_vni: ExactSpec::new(dst_vni.as_u32()), - proto: proto.into(), - src_ip_range: src_ip_range.into(), - dst_ip_range: dst_ip_range.into(), - } - .into_backend_fields::(), - ), - (Prefix::IPV6(src_ip_range), Prefix::IPV6(dst_ip_range)) => Some( - IpsProtoTupleRule { - src_vni: ExactSpec::new(src_vni.as_u32()), - dst_vni: ExactSpec::new(dst_vni.as_u32()), - proto: proto.into(), - src_ip_range: src_ip_range.into(), - dst_ip_range: dst_ip_range.into(), - } - .into_backend_fields::(), - ), - _ => None, - } +/// Lower a single rule to backend 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, } } @@ -297,15 +253,17 @@ impl Wildcardable for Ipv6Addr { } } -// Lower rules into a tuple (prefixes + ports) table. -fn build_ips_ports_tuple( +// Lower a set of rules (already grouped by IP version) into a single table. A missing prefix or +// port range becomes the wildcard for that field. +fn build_acl_table( rules: &[PeeringAclRule], -) -> ReferenceTable, LookupResult> { +) -> ReferenceTable, LookupResult> { let rules = rules .iter() .filter_map(|rule| { Some(RefRule::new( - IpsPortsTuple::::predicates( + rule_predicates( + rule.proto, rule.src_vni, rule.dst_vni, rule.src_ip_range.unwrap_or_else(T::wildcard), @@ -326,51 +284,6 @@ fn build_ips_ports_tuple( ReferenceTable::new(rules) } -// Lower rules into a tuple (prefixes + proto) table. -fn build_ips_proto_tuple( - rules: &[PeeringAclRule], -) -> ReferenceTable, LookupResult> { - let rules = rules - .iter() - .filter_map(|rule| { - Some(RefRule::new( - IpsProtoTuple::::predicates( - rule.src_vni, - rule.dst_vni, - rule.proto, - rule.src_ip_range.unwrap_or_else(T::wildcard), - rule.dst_ip_range.unwrap_or_else(T::wildcard), - )?, - LookupResult { - action: rule.action, - log: rule.log, - scope: rule.scope, - }, - )) - }) - .collect(); - ReferenceTable::new(rules) -} - -pub(super) trait TupleProto { - fn from_tuple_and_proto(tuple: &T, proto: u8) -> Self; -} - -impl TupleProto> for IpsProtoTuple -where - I: Clone, -{ - fn from_tuple_and_proto(tuple: &IpsPortsTuple, proto: u8) -> Self { - Self { - src_vni: tuple.src_vni, - dst_vni: tuple.dst_vni, - proto, - src_ip_range: tuple.src_ip_range.clone(), - dst_ip_range: tuple.dst_ip_range.clone(), - } - } -} - #[derive(Debug, Clone)] pub(super) struct LookupResult { pub(super) action: AclAction, @@ -379,55 +292,27 @@ pub(super) struct LookupResult { } #[derive(Debug, Clone)] -pub(super) struct PeeringAclTables { - pub(super) tcp: ReferenceTable, - pub(super) udp: ReferenceTable, - pub(super) other: ReferenceTable, +pub(super) struct AclTables { + pub(super) v4: ReferenceTable, LookupResult>, + pub(super) v6: ReferenceTable, LookupResult>, + pub(super) default_actions: HashMap<(Vni, Vni), AclAction>, } -impl Default for PeeringAclTables -where - T: MatchKey, - U: MatchKey, -{ +impl Default for AclTables { fn default() -> Self { Self { - tcp: ReferenceTable::empty(), - udp: ReferenceTable::empty(), - other: ReferenceTable::empty(), - } - } -} - -impl PeeringAclTables, IpsProtoTuple> -where - I: FixedSize + Wildcardable, -{ - fn from_tables( - tcp: &[PeeringAclRule], - udp: &[PeeringAclRule], - other: &[PeeringAclRule], - ) -> Self { - PeeringAclTables { - tcp: build_ips_ports_tuple(tcp), - udp: build_ips_ports_tuple(udp), - other: build_ips_proto_tuple(other), + v4: ReferenceTable::empty(), + v6: ReferenceTable::empty(), + default_actions: HashMap::new(), } } } -#[derive(Debug, Default, Clone)] -pub(super) struct AclTables { - pub(super) v4: PeeringAclTables, IpsProtoTuple>, - pub(super) v6: PeeringAclTables, IpsProtoTuple>, - pub(super) default_actions: HashMap<(Vni, Vni), AclAction>, -} - impl From for AclTables { fn from(context: PeeringAclRuleSet) -> Self { Self { - v4: PeeringAclTables::from_tables(&context.tcp_v4, &context.udp_v4, &context.other_v4), - v6: PeeringAclTables::from_tables(&context.tcp_v6, &context.udp_v6, &context.other_v6), + v4: build_acl_table(&context.v4), + v6: build_acl_table(&context.v6), default_actions: context.default_actions, } } @@ -442,37 +327,22 @@ impl From<&ValidatedOverlay> for AclTables { // ------------------------------------------------------------------------------------------------- // Lookup logic -impl PeeringAclTables -where - T: MatchKey + Clone, - U: MatchKey + TupleProto, -{ - fn lookup(&self, proto: NextHeader, tuple: &T) -> Option<&LookupResult> { - match proto { - NextHeader::TCP => self.tcp.lookup(tuple), - NextHeader::UDP => self.udp.lookup(tuple), - _ => { - let proto = proto.as_u8(); - let proto_tuple = U::from_tuple_and_proto(tuple, proto); - self.other.lookup(&proto_tuple) - } - } - } -} - impl AclTables { 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 tuple = - IpsPortsTuple::new(p.src_vni, p.dst_vni, src_ip, dst_ip, src_ports, dst_ports); - self.v4.lookup(p.proto, &tuple) + 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 tuple = - IpsPortsTuple::new(p.src_vni, p.dst_vni, src_ip, dst_ip, src_ports, dst_ports); - self.v6.lookup(p.proto, &tuple) + 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!"); diff --git a/acl-filter/src/display.rs b/acl-filter/src/display.rs index e22a215211..3c199ae1d6 100644 --- a/acl-filter/src/display.rs +++ b/acl-filter/src/display.rs @@ -5,13 +5,12 @@ //! //! The ACL rules are stored in the context after being lowered into positional field predicates //! (see [`crate::context`]), so the human-readable field values are reconstructed here from the raw -//! bytes of each predicate. This relies on the field layout of the match keys defined in +//! bytes of each predicate. This relies on the field layout of the `AclKey` match key defined in //! `context.rs`: //! -//! - `IpsPortsTuple`: src_vni, dst_vni, src_ip, dst_ip, src_port, dst_port (TCP/UDP tables) -//! - `IpsProtoTuple`: src_vni, dst_vni, proto, src_ip, dst_ip (the "other" table) +//! - `AclKey`: proto, src_vni, dst_vni, src_ip, dst_ip, src_port, dst_port //! -//! If those layouts change, the decoding below must be updated accordingly. +//! If that layout changes, the decoding below must be updated accordingly. use common::cliprovider::{CliSource, Heading}; use indenter::indented; @@ -39,17 +38,13 @@ impl Display for AclTables { writeln!(f, "IPv4:")?; { let mut w = indented(f).with_str(" "); - fmt_ref_table(&mut w, "TCP", &self.v4.tcp, Layout::Ports)?; - fmt_ref_table(&mut w, "UDP", &self.v4.udp, Layout::Ports)?; - fmt_ref_table(&mut w, "other", &self.v4.other, Layout::Proto)?; + fmt_ref_table(&mut w, &self.v4)?; } writeln!(f, "IPv6:")?; { let mut w = indented(f).with_str(" "); - fmt_ref_table(&mut w, "TCP", &self.v6.tcp, Layout::Ports)?; - fmt_ref_table(&mut w, "UDP", &self.v6.udp, Layout::Ports)?; - fmt_ref_table(&mut w, "other", &self.v6.other, Layout::Proto)?; + fmt_ref_table(&mut w, &self.v6)?; } writeln!(f, "default actions:")?; @@ -68,57 +63,32 @@ impl Display for AclTables { } } -/// The positional field layout of the reference table being dumped, so the decoding below reads -/// the right predicate for each column. -#[derive(Clone, Copy)] -enum Layout { - /// `IpsPortsTuple` (TCP/UDP tables): src_vni, dst_vni, src_ip, dst_ip, src_port, dst_port. - Ports, - /// `IpsProtoTuple` (the "other" table): src_vni, dst_vni, proto, src_ip, dst_ip. - Proto, -} - -/// Format a single reference table as a labelled, numbered list of rules indented under that label. +/// Format a single reference table as a numbered list of rules. The decoding reads each rule's +/// predicates by position, following the `AclKey` layout in `context.rs`: +/// proto, src_vni, dst_vni, src_ip, dst_ip, src_port, dst_port. fn fmt_ref_table( w: &mut W, - label: &str, table: &ReferenceTable, - layout: Layout, ) -> fmt::Result { - writeln!(w, "{label}:")?; - let mut w = indented(w).with_str(" "); if table.is_empty() { return writeln!(w, "(none)"); } for (idx, rule) in table.rules().iter().enumerate() { let fields = rule.fields(); let result = rule.action(); - let src_vni = decode_vni(fields.first()); - let dst_vni = decode_vni(fields.get(1)); + 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 { "" }; - match layout { - Layout::Ports => { - let src_ip = decode_prefix(fields.get(2)); - let dst_ip = decode_prefix(fields.get(3)); - let src_ports = decode_ports(fields.get(4)); - let dst_ports = decode_ports(fields.get(5)); - writeln!( - w, - "[{idx}] VPC {src_vni} -> VPC {dst_vni} | src {src_ip}:{src_ports} | dst {dst_ip}:{dst_ports} | {:?} ({:?}{log})", - result.action, result.scope - )?; - } - Layout::Proto => { - let proto = decode_proto(fields.get(2)); - let src_ip = decode_prefix(fields.get(3)); - let dst_ip = decode_prefix(fields.get(4)); - writeln!( - w, - "[{idx}] VPC {src_vni} -> VPC {dst_vni} | proto {proto} | src {src_ip} | dst {dst_ip} | {:?} ({:?}{log})", - result.action, result.scope - )?; - } - } + 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(()) } @@ -145,17 +115,17 @@ fn decode_prefix(predicate: Option<&FieldPredicate>) -> String { } } -/// Decode an IP protocol number stored as a 1-byte range. A full range renders as `any`, an exact -/// match as the single protocol number. +/// 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. fn decode_proto(predicate: Option<&FieldPredicate>) -> String { - match predicate.and_then(FieldPredicate::as_range) { - Some(([lo], [hi])) => { - if *lo == 0 && *hi == u8::MAX { + match predicate.and_then(FieldPredicate::as_mask) { + Some(([value], [mask])) => { + if *mask == 0 { "any".to_string() - } else if lo == hi { - lo.to_string() + } else if *mask == u8::MAX { + value.to_string() } else { - format!("{lo}-{hi}") + format!("{value}&{mask:#04x}") } } _ => "?".to_string(), @@ -184,7 +154,7 @@ fn decode_ports(predicate: Option<&FieldPredicate>) -> String { #[cfg(test)] mod tests { use super::{decode_ports, decode_prefix, decode_proto, decode_vni}; - use match_action::{Erased, ExactSpec, IntoBackendField, PrefixSpec, RangeSpec}; + use match_action::{Erased, ExactSpec, IntoBackendField, MaskSpec, PrefixSpec, RangeSpec}; use std::net::{Ipv4Addr, Ipv6Addr}; #[test] @@ -223,14 +193,14 @@ mod tests { } #[test] - fn decode_proto_handles_any_exact_and_range() { - let any = IntoBackendField::::into_backend_field(RangeSpec::new(0u8, u8::MAX)); + 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 exact = IntoBackendField::::into_backend_field(RangeSpec::new(1u8, 1u8)); - assert_eq!(decode_proto(Some(&exact)), "1"); + let tcp = IntoBackendField::::into_backend_field(MaskSpec::new(6u8, u8::MAX)); + assert_eq!(decode_proto(Some(&tcp)), "6"); - let range = IntoBackendField::::into_backend_field(RangeSpec::new(1u8, 132u8)); - assert_eq!(decode_proto(Some(&range)), "1-132"); + let udp = IntoBackendField::::into_backend_field(MaskSpec::new(17u8, u8::MAX)); + assert_eq!(decode_proto(Some(&udp)), "17"); } } diff --git a/config/src/external/overlay/acl.rs b/config/src/external/overlay/acl.rs index 51c96901d9..253bceeefb 100644 --- a/config/src/external/overlay/acl.rs +++ b/config/src/external/overlay/acl.rs @@ -7,7 +7,7 @@ use super::vpcpeering::ValidatedManifest; use crate::ConfigError; use crate::utils::normalize; use lpm::prefix::{PortRange, Prefix, PrefixPortsSet, PrefixWithOptionalPorts}; -use match_action::RangeSpec; +use match_action::MaskSpec; use net::ip::NextHeader; use tracing::debug; @@ -30,13 +30,18 @@ pub enum AclProtoMatch { const TCP: u8 = NextHeader::TCP.as_u8(); const UDP: u8 = NextHeader::UDP.as_u8(); -impl From for RangeSpec { +/// 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 => RangeSpec::new(TCP, TCP), - AclProtoMatch::Udp => RangeSpec::new(UDP, UDP), - AclProtoMatch::Other(p) => RangeSpec::new(p, p), - AclProtoMatch::Any => RangeSpec::new(0, u8::MAX), + 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), } } } From b69b634a7a99709507ba87304ad8c00496b02255 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 17 Jul 2026 01:16:51 -0600 Subject: [PATCH 14/15] feat(acl-filter): switch production classifier to the rte_acl backend Replace the reference (linear-scan) tables with rte_acl `DpdkAclLookup` tables in production, keeping the reference backend as a cfg(test) differential oracle. - Introduce `AnyTable { Empty, Dpdk, Reference }` per IP version: `Empty` avoids building a zero-rule rte_acl context, `Dpdk` is production, `Reference` is the test/opt-in oracle. Rules are lowered once to backend-neutral predicates and the selected `Backend` consumes them. - Positional first-match: rte_acl returns the highest-priority match, so rules are installed with priority descending by insertion index (rule 0 wins). This reproduces the reference backend's first-match-on-insertion-order precedence. - Fallible build threads through `ConfigError`; `TryFrom` uses the dpdk backend (EAL initialised early in dataplane::main), and `for_test`/`for_test_dpdk` select a backend for tests. - Each rte_acl context gets a process-unique name (a hot-swap briefly keeps the old and new contexts alive). - Display shows per-table rule counts in production (opaque classifier) and the full per-rule dump in test builds (reference backend retains the rules). The semantic suite runs on the reference backend (fast, EAL-free); a new `#[dpdk::with_eal]` differential test builds the same overlay on both backends and asserts identical verdicts, proving the rte_acl path. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Daniel Noland --- Cargo.lock | 1 + Cargo.toml | 4 +- acl-filter/Cargo.toml | 11 +- acl-filter/src/access.rs | 26 ++++- acl-filter/src/context.rs | 217 +++++++++++++++++++++++++++++++------- acl-filter/src/display.rs | 115 +++++++++++--------- acl-filter/src/tests.rs | 131 ++++++++++++++++++++++- 7 files changed, 407 insertions(+), 98 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6e754ae2ed..ce664f21f1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1245,6 +1245,7 @@ dependencies = [ "dataplane-common", "dataplane-concurrency", "dataplane-config", + "dataplane-dpdk", "dataplane-flow-entry", "dataplane-flow-filter", "dataplane-lookup", diff --git a/Cargo.toml b/Cargo.toml index 7621b4b59e..930464ee9b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -277,8 +277,8 @@ wasm = false # hopeless + pointless [workspace.metadata.package.acl-filter] package = "dataplane-acl-filter" -miri = true -wasm = false # miss +miri = false # hopeless + pointless +wasm = false # hopeless + pointless [workspace.metadata.package.args] package = "dataplane-args" diff --git a/acl-filter/Cargo.toml b/acl-filter/Cargo.toml index 4bacd1f613..15a6ff9350 100644 --- a/acl-filter/Cargo.toml +++ b/acl-filter/Cargo.toml @@ -6,13 +6,11 @@ publish.workspace = true version.workspace = true [dependencies] -# `reference` is now a non-default feature of `acl` (the crate defaults to the -# dpdk backend). acl-filter's tables are still built on the reference backend -# during the switch to dpdk, so enable it explicitly alongside the default dpdk. -acl = { workspace = true, features = ["reference"] } +acl = { workspace = true } common = { workspace = true } concurrency = { workspace = true } config = { workspace = true } +dpdk = { workspace = true } indenter = { workspace = true } lookup = { workspace = true } lpm = { workspace = true } @@ -22,7 +20,12 @@ 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"] } flow-entry = { workspace = true } flow-filter = { workspace = true } lpm = { workspace = true, features = ["testing"] } diff --git a/acl-filter/src/access.rs b/acl-filter/src/access.rs index fe7dc0369b..0667dda796 100644 --- a/acl-filter/src/access.rs +++ b/acl-filter/src/access.rs @@ -3,13 +3,13 @@ //! Read and write handles for the ACL filter context. -use crate::context::AclTables; +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, Clone)] +#[derive(Debug, Default)] pub struct AclFilterContext { pub(super) acls: AclTables, } @@ -18,7 +18,27 @@ impl TryFrom<&ValidatedOverlay> for AclFilterContext { type Error = ConfigError; fn try_from(overlay: &ValidatedOverlay) -> Result { - let acls = AclTables::from(overlay); + 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 }) } } diff --git a/acl-filter/src/context.rs b/acl-filter/src/context.rs index db7c7a88f2..1d7728ead7 100644 --- a/acl-filter/src/context.rs +++ b/acl-filter/src/context.rs @@ -4,15 +4,25 @@ //! 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)] @@ -136,13 +146,11 @@ impl From<&ValidatedOverlay> for PeeringAclRuleSet { } // ------------------------------------------------------------------------------------------------- -// Backend lowering layer. +// Key and rule lowering. // -// This is where source rules are lowered into ACL field predicates -// (`into_backend_fields::()`). Switching the reference backend for a hardware backend -// means changing `RuleBackend` here (and the table types in `AclTables`). - -type RuleBackend = Erased; +// 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. /// @@ -195,9 +203,9 @@ impl AclKey { } } -/// Lower a single rule to backend 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. +/// 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, @@ -219,7 +227,7 @@ fn rule_predicates( src_port_range, dst_port_range, } - .into_backend_fields::(), + .into_backend_fields::(), ), (Prefix::IPV6(src_ip_range), Prefix::IPV6(dst_ip_range)) => Some( AclKeyRule { @@ -231,7 +239,7 @@ fn rule_predicates( src_port_range, dst_port_range, } - .into_backend_fields::(), + .into_backend_fields::(), ), _ => None, } @@ -253,15 +261,15 @@ impl Wildcardable for Ipv6Addr { } } -// Lower a set of rules (already grouped by IP version) into a single table. A missing prefix or -// port range becomes the wildcard for that field. -fn build_acl_table( +/// 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], -) -> ReferenceTable, LookupResult> { - let rules = rules +) -> Vec<(Vec, LookupResult)> { + rules .iter() .filter_map(|rule| { - Some(RefRule::new( + Some(( rule_predicates( rule.proto, rule.src_vni, @@ -280,8 +288,142 @@ fn build_acl_table( }, )) }) - .collect(); - ReferenceTable::new(rules) + .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)] @@ -291,36 +433,37 @@ pub(super) struct LookupResult { pub(super) scope: AclScope, } -#[derive(Debug, Clone)] +#[derive(Debug)] pub(super) struct AclTables { - pub(super) v4: ReferenceTable, LookupResult>, - pub(super) v6: ReferenceTable, LookupResult>, + 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: ReferenceTable::empty(), - v6: ReferenceTable::empty(), + v4: AnyTable::Empty, + v6: AnyTable::Empty, default_actions: HashMap::new(), } } } -impl From for AclTables { - fn from(context: PeeringAclRuleSet) -> Self { - Self { - v4: build_acl_table(&context.v4), - v6: build_acl_table(&context.v6), - default_actions: context.default_actions, - } - } -} - -impl From<&ValidatedOverlay> for AclTables { - fn from(overlay: &ValidatedOverlay) -> Self { - PeeringAclRuleSet::from(overlay).into() +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, + }) } } diff --git a/acl-filter/src/display.rs b/acl-filter/src/display.rs index 3c199ae1d6..986226ab40 100644 --- a/acl-filter/src/display.rs +++ b/acl-filter/src/display.rs @@ -3,26 +3,19 @@ //! Display implementations for the ACL filter context. //! -//! The ACL rules are stored in the context after being lowered into positional field predicates -//! (see [`crate::context`]), so the human-readable field values are reconstructed here from the raw -//! bytes of each predicate. This relies on the field layout of the `AclKey` match key defined in -//! `context.rs`: +//! 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 -//! -//! If that layout changes, the decoding below must be updated accordingly. use common::cliprovider::{CliSource, Heading}; -use indenter::indented; use std::fmt::{self, Display, Write}; -use std::net::{Ipv4Addr, Ipv6Addr}; - -use acl::reference::table::ReferenceTable; -use match_action::{FieldPredicate, MatchKey}; use crate::AclFilterContext; -use crate::context::{AclTables, LookupResult}; +use crate::context::AclTables; impl CliSource for AclFilterContext {} @@ -33,47 +26,64 @@ impl Display for AclFilterContext { } } +/// 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:")?; - { - let mut w = indented(f).with_str(" "); - fmt_ref_table(&mut w, &self.v4)?; - } + writeln!(f, "IPv4: {} rules", self.v4.len())?; + writeln!(f, "IPv6: {} rules", self.v6.len())?; + fmt_default_actions(f, self) + } +} - writeln!(f, "IPv6:")?; - { - let mut w = indented(f).with_str(" "); - fmt_ref_table(&mut w, &self.v6)?; - } +// ------------------------------------------------------------------------------------------------- +// Test / `reference` builds: full per-rule rendering from the reference backend. - writeln!(f, "default actions:")?; - let mut w = indented(f).with_str(" "); - if self.default_actions.is_empty() { - writeln!(w, "(none)")?; - } else { - // Sort for a deterministic dump. - let mut defaults: Vec<_> = self.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(()) +#[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 reference table as a numbered list of rules. The decoding reads each rule's -/// predicates by position, following the `AclKey` layout in `context.rs`: -/// proto, src_vni, dst_vni, src_ip, dst_ip, src_port, dst_port. -fn fmt_ref_table( +/// 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: &ReferenceTable, + table: &crate::context::AnyTable, ) -> fmt::Result { - if table.is_empty() { + 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 table.rules().iter().enumerate() { + for (idx, rule) in rules.iter().enumerate() { let fields = rule.fields(); let result = rule.action(); let proto = decode_proto(fields.first()); @@ -94,8 +104,9 @@ fn fmt_ref_table( } /// Decode a VNI stored as a 4-byte big-endian exact-match predicate. -fn decode_vni(predicate: Option<&FieldPredicate>) -> String { - match predicate.and_then(FieldPredicate::as_exact) { +#[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(), } @@ -103,8 +114,10 @@ fn decode_vni(predicate: Option<&FieldPredicate>) -> String { /// Decode an IP prefix stored as a prefix-match predicate (4 bytes for IPv4, 16 bytes for IPv6, /// plus a prefix length). -fn decode_prefix(predicate: Option<&FieldPredicate>) -> String { - match predicate.and_then(FieldPredicate::as_prefix) { +#[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]; @@ -117,8 +130,9 @@ fn decode_prefix(predicate: Option<&FieldPredicate>) -> 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. -fn decode_proto(predicate: Option<&FieldPredicate>) -> String { - match predicate.and_then(FieldPredicate::as_mask) { +#[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() @@ -134,8 +148,9 @@ fn decode_proto(predicate: Option<&FieldPredicate>) -> 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. -fn decode_ports(predicate: Option<&FieldPredicate>) -> String { - match predicate.and_then(FieldPredicate::as_range) { +#[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]); diff --git a/acl-filter/src/tests.rs b/acl-filter/src/tests.rs index 4354dc277d..0aeef471e4 100644 --- a/acl-filter/src/tests.rs +++ b/acl-filter/src/tests.rs @@ -135,7 +135,9 @@ fn overlay(vpcs: &[(&str, u32)], peerings: Vec) -> ValidatedOverlay /// Build an `AclFilter` network function from a validated overlay. fn acl_filter(overlay: &ValidatedOverlay) -> AclFilter { let writer = AclFilterContextWriter::new(); - writer.store(AclFilterContext::try_from(overlay).unwrap()); + // 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()) } @@ -910,7 +912,7 @@ mod end_to_end { // ACL filter: placed here so packets still carry VPC-internal addresses let acl_writer = AclFilterContextWriter::new(); - acl_writer.store(AclFilterContext::try_from(overlay).unwrap()); + acl_writer.store(AclFilterContext::for_test(overlay)); pipeline = pipeline.add_stage(AclFilter::new("acl-filter", acl_writer.get_reader())); // Static NAT @@ -991,3 +993,128 @@ mod end_to_end { 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), + ) + }); + } +} From fba60ac3d164759a832a1dee890de55fb7ff730f Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Fri, 17 Jul 2026 12:18:36 +0100 Subject: [PATCH 15/15] 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 1d7728ead7..27fa7ab29b 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; @@ -363,7 +364,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))