From 3a1e9ff28304d34f005cf0e3b20b97e51c8d62d1 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 19:04:11 -0600 Subject: [PATCH 01/21] test(dataplane): Establish what the model checker can reach First step of the multi-worker work: find out whether the pipeline can be driven inside a model-checked execution at all. It cannot, and the obstacle is not DPDK's opacity but one process-global lock, which the module docs set out. The three-case lock test exists to make that attributable rather than to assert anything interesting on its own. Its failing third case is written out rather than run: shuttle aborts the process instead of failing a test, so a live one would take the suite down with it. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/Cargo.toml | 2 + dataplane/src/packet_processor/fuzz.rs | 77 +++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/dataplane/Cargo.toml b/dataplane/Cargo.toml index dc3b553da5..a18193d256 100644 --- a/dataplane/Cargo.toml +++ b/dataplane/Cargo.toml @@ -10,6 +10,8 @@ default = [] # Bin is gated out under loom (see `src/main.rs`); the feature exists so # `--features loom` resolves at workspace level and propagates to libs. loom = ["concurrency/loom"] +shuttle = ["concurrency/shuttle", "nat/shuttle", "flow-entry/shuttle"] +shuttle_dfs = ["concurrency/shuttle_dfs", "shuttle"] [dependencies] acl-filter = { workspace = true } diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index ccfc05691f..b7200079c6 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -2813,7 +2813,7 @@ mod routed { matches!(copy.vxlan_decap(), Some(Ok(_))).then_some(copy) } - fn inner() -> Packet { + pub(super) fn inner() -> Packet { build_test_udp_ipv4_packet("1.1.0.1", "3.3.3.1", 1234, 80) } @@ -3462,3 +3462,78 @@ mod routed { out.pop().unwrap_or_else(|| unreachable!()) } } + +#[cfg(test)] +mod model { + use super::routed::{exposes, inner, tunnelled}; + use super::*; + use concurrency::sync::Mutex; + use concurrency::thread; + #[cfg_attr(not(feature = "shuttle"), allow(unused_imports))] + use concurrency::thread::BuilderExt; + use std::sync::OnceLock; + use std::sync::atomic::{AtomicUsize, Ordering}; + + fn on_two_threads(body: &(impl Fn() + Sync)) { + thread::scope(|scope| { + let handles: Vec<_> = (0..2) + .map(|_| { + thread::Builder::new() + .spawn_scoped(scope, body) + .expect("spawn") + }) + .collect(); + for handle in handles { + handle.join().expect("join"); + } + }); + } + + #[concurrency::model_test] + fn a_lock_that_outlives_its_execution_is_not_model_checkable() { + static LOCK: OnceLock> = OnceLock::new(); + static RUNS: AtomicUsize = AtomicUsize::new(0); + + concurrency::stress(|| { + let lock = concurrency::sync::Arc::new(Mutex::new(0u32)); + let inner = lock.clone(); + let bump = move || *inner.lock() += 1; + on_two_threads(&bump); + assert_eq!(*lock.lock(), 2); + }); + + concurrency::stress(|| { + let first = RUNS.fetch_add(1, Ordering::Relaxed) == 0; + let bump = move || { + if first { + *LOCK.get_or_init(|| Mutex::new(0)).lock() += 1; + } + }; + on_two_threads(&bump); + }); + } + + #[cfg(not(feature = "shuttle"))] + #[concurrency::model_test] + fn a_pipeline_can_be_driven_inside_a_stress_run() { + let _eal = dpdk::test_support::start_eal(); + + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("build tokio runtime"); + let handle = rt.handle().clone(); + + concurrency::stress(move || { + let _guard = tokio::runtime::Handle::enter(&handle); + let mut fabric = Fabric::routed(&exposes(), None) + .unwrap_or_else(|| unreachable!("the fixture exposes do not validate")); + let out = fabric.send(tunnelled(&inner())); + assert!( + matches!(verdict(&out), Verdict::Delivered { .. }), + "a packet that is delivered single-threaded was not: {:?}", + verdict(&out) + ); + }); + } +} From 7a238dd6c181b52d2189d4856a42fe17ce7639ba Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 19:15:31 -0600 Subject: [PATCH 02/21] fix(dpdk): Let the acl registry lock survive a model checker A `concurrency::sync` mutex in a `static` typechecks under loom and shuttle and then aborts the process, because the primitive belongs to the execution that created it and a `OnceLock` outlives every execution. `OnceLock` is what the concurrency crate recommends for the separate problem that `Mutex::new` is not `const fn` there, so the advice and the hazard point in opposite directions and nothing says so. This unblocks driving the pipeline under shuttle: every `Fabric` builds ACL contexts and so took this lock. Revisit if loom and shuttle ever make `Mutex::new` const. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/fuzz.rs | 95 ++++++++++++++++++++------ dpdk/src/acl/context.rs | 49 +++++++++---- 2 files changed, 110 insertions(+), 34 deletions(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index b7200079c6..82c08080cd 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -58,11 +58,23 @@ impl Fabric { } pub(crate) fn routed(exposes: &[VpcExpose], acl: Option<&Acl>) -> Option { - Self::assemble( - exposes, - acl, + Self::routed_sharing(exposes, acl, Arc::new(FlowTable::default())) + } + + pub(crate) fn routed_sharing( + exposes: &[VpcExpose], + acl: Option<&Acl>, + flow_table: Arc, + ) -> Option { + let overlay = overlay_with_exposes_and_acl(exposes.to_vec(), acl) + .ok()? + .validate() + .ok()?; + Some(Self::with_overlay_sharing( + &overlay, Some(topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)])), - ) + flow_table, + )) } pub(crate) fn routed_over(overlay: &Overlay, tables: RouterTables) -> Option { @@ -89,8 +101,15 @@ impl Fabric { } fn with_overlay(overlay: &ValidatedOverlay, tables: Option) -> Self { + Self::with_overlay_sharing(overlay, tables, Arc::new(FlowTable::default())) + } + + fn with_overlay_sharing( + overlay: &ValidatedOverlay, + tables: Option, + flow_table: Arc, + ) -> Self { let translations = Arc::new(Mutex::new(Translations::declaring(overlay))); - let flow_table = Arc::new(FlowTable::default()); let mut pipeline = DynPipeline::new(); if let Some(tables) = &tables { @@ -201,6 +220,10 @@ impl Fabric { self.flow_table.len() } + pub(crate) fn shared_flow_table(&self) -> Arc { + self.flow_table.clone() + } + pub(crate) fn send_batch( &mut self, mut packets: Vec>, @@ -3513,27 +3536,59 @@ mod model { }); } - #[cfg(not(feature = "shuttle"))] #[concurrency::model_test] fn a_pipeline_can_be_driven_inside_a_stress_run() { let _eal = dpdk::test_support::start_eal(); - let rt = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .expect("build tokio runtime"); - let handle = rt.handle().clone(); + let rt = cfg_select! { + feature = "shuttle" => None::, + _ => Some( + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("build tokio runtime") + ) + }; + let handle = rt.as_ref().map(tokio::runtime::Runtime::handle).cloned(); concurrency::stress(move || { - let _guard = tokio::runtime::Handle::enter(&handle); - let mut fabric = Fabric::routed(&exposes(), None) - .unwrap_or_else(|| unreachable!("the fixture exposes do not validate")); - let out = fabric.send(tunnelled(&inner())); - assert!( - matches!(verdict(&out), Verdict::Delivered { .. }), - "a packet that is delivered single-threaded was not: {:?}", - verdict(&out) - ); + let table = Arc::new(FlowTable::default()); + let sending = table.clone(); + let entering = handle.clone(); + + thread::scope(|scope| { + let sender = thread::Builder::new() + .name("sender".to_owned()) + .spawn_scoped(scope, move || { + let _guard = entering.as_ref().map(tokio::runtime::Handle::enter); + let mut fabric = Fabric::routed_sharing(&exposes(), None, sending) + .unwrap_or_else(|| unreachable!("the fixture exposes do not validate")); + let out = fabric.send(tunnelled(&inner())); + assert!( + matches!(verdict(&out), Verdict::Delivered { .. }), + "a packet that is delivered single-threaded was not: {:?}", + verdict(&out) + ); + }) + .expect("spawn sender"); + + let reader = thread::Builder::new() + .name("reader".to_owned()) + .spawn_scoped(scope, move || { + for _ in 0..3 { + let seen = table.len(); + assert!(seen.is_some(), "the flow table refused a concurrent read"); + table.for_each_flow(|_, flow| { + let _ = flow.status(); + }); + thread::yield_now(); + } + }) + .expect("spawn reader"); + + sender.join().expect("sender panicked"); + reader.join().expect("reader panicked"); + }); }); } } diff --git a/dpdk/src/acl/context.rs b/dpdk/src/acl/context.rs index 5e81675734..aaac8373e6 100644 --- a/dpdk/src/acl/context.rs +++ b/dpdk/src/acl/context.rs @@ -41,7 +41,7 @@ use core::fmt; use core::mem::ManuallyDrop; use core::ptr::NonNull; -use concurrency::sync::{Mutex, OnceLock}; +use concurrency::sync::OnceLock; use errno::Errno; use tracing::{debug, error, trace}; @@ -71,13 +71,7 @@ use super::rule::Rule; /// `loom`/`shuttle` model-checker backends, `concurrency::sync::Mutex::new` /// is not `const fn` (each instance registers with the scheduler), so a /// `static M: Mutex<()> = Mutex::new(())` would fail to typecheck on those -/// configurations. `OnceLock` + lazy init is the portable idiom across -/// all backends. See the module docs on `concurrency::sync`. /// -/// Why the concurrency facade rather than [`std::sync::Mutex`] directly: -/// the workspace policy is poison-as-panic ("poison is a fatal invariant -/// violation"); the facade applies that policy uniformly so call sites -/// never see `LockResult`. /// /// # Tracing reentrancy /// @@ -91,11 +85,38 @@ use super::rule::Rule; /// configuration never touches ACL, but custom layers (e.g. one that /// resolves the context name from a registry lookup for log enrichment) /// could trip this if added later. -static ACL_CREATE_LOCK: OnceLock> = OnceLock::new(); +static ACL_CREATE_LOCK: OnceLock = OnceLock::new(); -/// Lazy accessor for [`ACL_CREATE_LOCK`]. -fn acl_create_lock() -> &'static Mutex<()> { - ACL_CREATE_LOCK.get_or_init(|| Mutex::new(())) +concurrency::with_std! { + type RegistryMutex = concurrency::sync::Mutex<()>; + + fn hold(lock: &'static RegistryMutex) -> impl Sized { + lock.lock() + } +} + +concurrency::with_loom! { + // nosemgrep: rust-no-direct-std-sync-import + type RegistryMutex = std::sync::Mutex<()>; + + fn hold(lock: &'static RegistryMutex) -> impl Sized { + lock.lock() + .unwrap_or_else(|_| unreachable!("the acl registry lock is poisoned")) + } +} + +concurrency::with_shuttle! { + // nosemgrep: rust-no-direct-std-sync-import + type RegistryMutex = std::sync::Mutex<()>; + + fn hold(lock: &'static RegistryMutex) -> impl Sized { + lock.lock() + .unwrap_or_else(|_| unreachable!("the acl registry lock is poisoned")) + } +} + +fn hold_acl_registry() -> impl Sized { + hold(ACL_CREATE_LOCK.get_or_init(|| RegistryMutex::new(()))) } // --------------------------------------------------------------------------- @@ -475,7 +496,7 @@ impl AclContext> { // leaves DPDK's TAILQ in an unknown state, and continuing // silently could lead to use-after-free. Aborting via the // panic is the only safe answer. - let _create_guard = acl_create_lock().lock(); + let _create_guard = hold_acl_registry(); // Pre-flight: DPDK's `rte_acl_create` silently returns the existing // context for a duplicate name. Refuse if one is already registered. @@ -1181,7 +1202,7 @@ impl Drop for AclContext { // The facade panics on poison. Dropping while another holder // panicked mid-operation means the DPDK registry may be in an // unknown state; aborting via the panic is the only safe answer. - let _guard = acl_create_lock().lock(); + let _guard = hold_acl_registry(); // SAFETY: rte_acl_free is safe to call on any valid context pointer; `Drop` runs at // most once per `AclContext`, and the create-lock acquired above serialises against // `rte_acl_create` / `dump_all_contexts`. @@ -1211,7 +1232,7 @@ pub fn dump_all_contexts() { // expose a list in an inconsistent state to the walk. Facade panics // on poison (workspace policy -- a prior holder panic implies the // registry may be inconsistent). - let _guard = acl_create_lock().lock(); + let _guard = hold_acl_registry(); // SAFETY: rte_acl_list_dump takes no arguments and simply iterates an internal list. unsafe { dpdk_sys::rte_acl_list_dump() } } From 7550418a4aebbb8dd4e9db45182fcf55ba436198 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 19:22:31 -0600 Subject: [PATCH 03/21] docs(concurrency): Warn that OnceLock does not rescue a static lock The facade documented `OnceLock` as the workaround for `Mutex::new` not being `const fn` under the model checkers. It resolves that compile error and leaves a worse one, which nothing said: the second execution to take the lock aborts the process. `dpdk` followed the advice and was the only instance. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- concurrency/src/lib.rs | 7 ------- concurrency/src/sync/mod.rs | 2 -- 2 files changed, 9 deletions(-) diff --git a/concurrency/src/lib.rs b/concurrency/src/lib.rs index 55c0355658..575e59d0ec 100644 --- a/concurrency/src/lib.rs +++ b/concurrency/src/lib.rs @@ -38,13 +38,6 @@ //! schedule that `parking_lot` permits. Tests that hinge on that //! interleaving need `RwLock` with explicit `read()` then //! `write()`, or a richer state machine in the facade. -//! * **`static FOO: Mutex = Mutex::new(...)` does not compile -//! under loom.** `loom::sync::Mutex::new` is plain `fn`, not -//! `const fn`, so a static initialiser fails to typecheck. Use -//! `OnceLock` for the static (the facade re-exports -//! `std::sync::OnceLock` under all backends) or move the -//! construction into a runtime initialiser gated by -//! `#[concurrency_mode(std)]`. //! * **`OnceLock` under loom/shuttle** is the real `std::sync::OnceLock`, //! not a model-aware shim. Loom and shuttle do not see the //! atomics inside `OnceLock::get_or_init`, so tests whose diff --git a/concurrency/src/sync/mod.rs b/concurrency/src/sync/mod.rs index 711b95d991..8fc333f723 100644 --- a/concurrency/src/sync/mod.rs +++ b/concurrency/src/sync/mod.rs @@ -36,8 +36,6 @@ //! `const fn`, but the facade exposes the lowest common //! denominator. So `static M: Mutex = Mutex::new(...)` compiles //! under the default and `parking_lot` backends and fails to -//! typecheck under the model-checker backends. Workaround for -//! tests that need a static: wrap the static in `OnceLock`. //! //! * **`OnceLock` under `loom`/`shuttle*` is re-exported from //! `std::sync` unchanged.** It is sound for laziness, but it uses From 9437ec85c6441b69a37a790b6a615c0202ec9c82 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 19:26:15 -0600 Subject: [PATCH 04/21] feat(routing): Publish reader factories from the test tables A pipeline per thread needs one reader per thread. The readers this already published cannot serve: they hold `NonNull` table pointers and a `Cell` counter, and the fib readers cache `Rc>`, so none of them is `Send` or `Sync` -- nor is `RouterTables`. A factory crosses the boundary and each thread makes its own reader, which is how `start_router` feeds its workers. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- routing/src/testing.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/routing/src/testing.rs b/routing/src/testing.rs index 9752a0e339..ea59df4bb2 100644 --- a/routing/src/testing.rs +++ b/routing/src/testing.rs @@ -15,11 +15,11 @@ use net::interface::InterfaceIndex; use net::vxlan::Vni; use crate::atable::adjacency::Adjacency; -use crate::atable::atablerw::{AtableReader, AtableWriter}; +use crate::atable::atablerw::{AtableReader, AtableReaderFactory, AtableWriter}; use crate::evpn::Vtep; -use crate::fib::fibtable::{FibTableReader, FibTableWriter}; +use crate::fib::fibtable::{FibTableReader, FibTableReaderFactory, FibTableWriter}; use crate::fib::fibtype::FibKey; -use crate::interfaces::iftablerw::{IfTableReader, IfTableWriter}; +use crate::interfaces::iftablerw::{IfTableReader, IfTableReaderFactory, IfTableWriter}; use crate::interfaces::interface::{IfDataEthernet, IfState, IfType, RouterInterfaceConfig}; use crate::rib::vrf::VrfId; @@ -143,6 +143,21 @@ impl RouterTables { self.adj_reader.clone() } + #[must_use] + pub fn interface_factory(&self) -> IfTableReaderFactory { + self.if_reader.factory() + } + + #[must_use] + pub fn fib_factory(&self) -> FibTableReaderFactory { + self.fib_reader.factory() + } + + #[must_use] + pub fn adjacency_factory(&self) -> AtableReaderFactory { + self.adj_reader.factory() + } + fn fib_mut(&mut self, vrfid: VrfId) -> &mut FibWriter { self.fibs .get_mut(&vrfid) From 2ee22aff9baf40ed21ac261e1b8005e35cdfafec Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 19:46:38 -0600 Subject: [PATCH 05/21] refactor(dataplane): Split the fuzz fabric into fleet, blueprint and worker A `Fabric` interleaved writer creation with stage assembly, so one configuration could only ever produce one pipeline and no property here could be about more than one worker. Which of the three parts may cross a thread boundary, and why only that one, is on `Blueprint`. The `nat` and `routing` re-exports are part of the same change: a blueprint has to name the reader-factory types in a field, and outside those crates they could not be named at all. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/fuzz.rs | 340 +++++++++++++++---------- nat/src/masquerade/mod.rs | 2 +- nat/src/static_nat/mod.rs | 1 + routing/src/lib.rs | 6 +- 4 files changed, 215 insertions(+), 134 deletions(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 82c08080cd..ddf2a3d286 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -4,7 +4,9 @@ #![cfg(test)] #![cfg(not(miri))] -use acl_filter::{AclFilter, AclFilterContext, AclFilterContextWriter}; +use acl_filter::{ + AclFilter, AclFilterContext, AclFilterContextReaderFactory, AclFilterContextWriter, +}; use concurrency::sync::{Arc, Mutex}; use config::external::overlay::acl::Acl; use config::external::overlay::vpcpeering::VpcExpose; @@ -13,12 +15,14 @@ use config::external::overlay::vpcpeering::contract::{ }; use config::external::overlay::{Overlay, ValidatedOverlay}; use flow_entry::flow_table::{FlowLookup, FlowTable}; -use flow_filter::{FlowFilter, FlowFilterContext, FlowFilterContextWriter}; +use flow_filter::{ + FlowFilter, FlowFilterContext, FlowFilterContextReaderFactory, FlowFilterContextWriter, +}; use lpm::prefix::Prefix; -use nat::masquerade::{MasqueradeConfig, NatAllocatorWriter}; -use nat::portfw::{PortForwarder, PortFwTableWriter}; -use nat::static_nat::NatTablesWriter; +use nat::masquerade::{MasqueradeConfig, NatAllocatorReaderFactory, NatAllocatorWriter}; +use nat::portfw::{PortForwarder, PortFwTableReaderFactory, PortFwTableWriter}; use nat::static_nat::setup::build_nat_configuration; +use nat::static_nat::{NatTablesReaderFactory, NatTablesWriter}; use nat::{IcmpErrorHandler, Masquerade, StaticNat}; use net::buffer::{PacketBufferMut, TestBuffer}; use net::eth::mac::{Mac, SourceMac}; @@ -28,6 +32,7 @@ use net::vxlan::Vni; use pipeline::{DynPipeline, NetworkFunction}; use routing::testing::RouterTables; use routing::testing::{FibGroup, FwAction, NhopKey, RouteOrigin}; +use routing::{AtableReaderFactory, FibTableReaderFactory, IfTableReaderFactory}; use routing::{EgressObject, FibEntry, PktInstruction, ResolvedEncapsulation, ResolvedVxlan, Vtep}; use std::net::IpAddr; @@ -35,125 +40,68 @@ use super::egress::Egress; use super::ingress::Ingress; use super::ipforward::IpForwarder; -pub(crate) struct Fabric { - pipeline: DynPipeline, - flow_table: Arc, +pub(crate) struct Fleet { _flow_filter: FlowFilterContextWriter, _acl: AclFilterContextWriter, _static_nat: NatTablesWriter, _portfw: PortFwTableWriter, _masquerade: NatAllocatorWriter, _tables: Option, - translations: Arc>, - next_id: u64, + blueprint: Blueprint, } -impl Fabric { - pub(crate) fn build(exposes: &[VpcExpose]) -> Option { - Self::build_with_acl(exposes, None) - } - - pub(crate) fn build_with_acl(exposes: &[VpcExpose], acl: Option<&Acl>) -> Option { - Self::assemble(exposes, acl, None) - } - - pub(crate) fn routed(exposes: &[VpcExpose], acl: Option<&Acl>) -> Option { - Self::routed_sharing(exposes, acl, Arc::new(FlowTable::default())) - } - - pub(crate) fn routed_sharing( - exposes: &[VpcExpose], - acl: Option<&Acl>, - flow_table: Arc, - ) -> Option { - let overlay = overlay_with_exposes_and_acl(exposes.to_vec(), acl) - .ok()? - .validate() - .ok()?; - Some(Self::with_overlay_sharing( - &overlay, - Some(topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)])), - flow_table, - )) - } - - pub(crate) fn routed_over(overlay: &Overlay, tables: RouterTables) -> Option { - Some(Self::with_overlay( - &overlay.clone().validate().ok()?, - Some(tables), - )) - } +pub(crate) struct Blueprint { + flow_filter: FlowFilterContextReaderFactory, + acl: AclFilterContextReaderFactory, + static_nat: NatTablesReaderFactory, + portfw: PortFwTableReaderFactory, + masquerade: NatAllocatorReaderFactory, + underlay: Option, + flow_table: Arc, + declared: Arc<[Prefix]>, +} - pub(crate) fn routed_over_validated(overlay: &ValidatedOverlay, tables: RouterTables) -> Self { - Self::with_overlay(overlay, Some(tables)) - } +struct Underlay { + interfaces: IfTableReaderFactory, + fibs: FibTableReaderFactory, + adjacencies: AtableReaderFactory, +} - fn assemble( - exposes: &[VpcExpose], - acl: Option<&Acl>, - tables: Option, - ) -> Option { - let overlay = overlay_with_exposes_and_acl(exposes.to_vec(), acl) - .ok()? - .validate() - .ok()?; - Some(Self::with_overlay(&overlay, tables)) - } +pub(crate) struct Worker { + pipeline: DynPipeline, + translations: Arc>, + next_id: u64, +} - fn with_overlay(overlay: &ValidatedOverlay, tables: Option) -> Self { - Self::with_overlay_sharing(overlay, tables, Arc::new(FlowTable::default())) - } +pub(crate) struct Fabric { + fleet: Fleet, + worker: Worker, +} - fn with_overlay_sharing( +impl Fleet { + pub(crate) fn lowering( overlay: &ValidatedOverlay, tables: Option, flow_table: Arc, ) -> Self { - let translations = Arc::new(Mutex::new(Translations::declaring(overlay))); - let mut pipeline = DynPipeline::new(); - - if let Some(tables) = &tables { - pipeline = pipeline.add_stage(Ingress::new("ingress", tables.interfaces())); - pipeline = pipeline.add_stage(IpForwarder::new("ip-forward-1", tables.fibs())); - pipeline = pipeline.add_stage(Checkpoint::new( - "after ip-forward-1", - contract::decapsulated, - )); - } - - pipeline = pipeline.add_stage(IcmpErrorHandler::new(flow_table.clone())); - pipeline = pipeline.add_stage(FlowLookup::new("flow-lookup", flow_table.clone())); - let flow_filter = FlowFilterContextWriter::new(); flow_filter.store( FlowFilterContext::try_from(overlay).expect("a validated overlay lowers to tables"), ); - pipeline = pipeline.add_stage(FlowFilter::new("flow-filter", flow_filter.get_reader())); - pipeline = pipeline.add_stage(Checkpoint::new("after flow-filter", contract::placed)); let acl = AclFilterContextWriter::new(); acl.store(AclFilterContext::try_from(overlay).expect("a validated overlay lowers to acls")); - pipeline = pipeline.add_stage(AclFilter::new("acl-filter", acl.get_reader())); let mut static_nat = NatTablesWriter::new(); static_nat.update_nat_tables( build_nat_configuration(overlay.vpc_table()) .expect("a validated overlay lowers to nat"), ); - pipeline = pipeline.add_stage(StaticNat::with_reader( - "static-nat", - static_nat.get_reader(), - )); let mut portfw = PortFwTableWriter::new(); portfw .update_from_vpc_table(overlay.vpc_table()) .expect("a validated overlay lowers to port forwarding"); - pipeline = pipeline.add_stage(PortForwarder::new( - "port-forwarder", - portfw.reader(), - flow_table.clone(), - )); let mut masquerade = NatAllocatorWriter::new(); masquerade.update_nat_allocator( @@ -161,6 +109,67 @@ impl Fabric { 1, &flow_table, ); + + let blueprint = Blueprint { + flow_filter: flow_filter.get_reader_factory(), + acl: acl.get_reader_factory(), + static_nat: static_nat.get_reader_factory(), + portfw: portfw.reader().factory(), + masquerade: masquerade.get_reader_factory(), + underlay: tables.as_ref().map(|tables| Underlay { + interfaces: tables.interface_factory(), + fibs: tables.fib_factory(), + adjacencies: tables.adjacency_factory(), + }), + flow_table, + declared: declared_public_ranges(overlay), + }; + + Self { + _flow_filter: flow_filter, + _acl: acl, + _static_nat: static_nat, + _portfw: portfw, + _masquerade: masquerade, + _tables: tables, + blueprint, + } + } + + pub(crate) fn blueprint(&self) -> &Blueprint { + &self.blueprint + } +} + +impl Blueprint { + pub(crate) fn worker(&self) -> Worker { + let translations = Arc::new(Mutex::new(Translations::declaring(&self.declared))); + let mut pipeline = DynPipeline::new(); + + if let Some(underlay) = &self.underlay { + pipeline = pipeline.add_stage(Ingress::new("ingress", underlay.interfaces.handle())); + pipeline = pipeline.add_stage(IpForwarder::new("ip-forward-1", underlay.fibs.handle())); + pipeline = pipeline.add_stage(Checkpoint::new( + "after ip-forward-1", + contract::decapsulated, + )); + } + + pipeline = pipeline.add_stage(IcmpErrorHandler::new(self.flow_table.clone())); + pipeline = pipeline.add_stage(FlowLookup::new("flow-lookup", self.flow_table.clone())); + pipeline = pipeline.add_stage(FlowFilter::new("flow-filter", self.flow_filter.handle())); + pipeline = pipeline.add_stage(Checkpoint::new("after flow-filter", contract::placed)); + pipeline = pipeline.add_stage(AclFilter::new("acl-filter", self.acl.handle())); + pipeline = pipeline.add_stage(StaticNat::with_reader( + "static-nat", + self.static_nat.handle(), + )); + pipeline = pipeline.add_stage(PortForwarder::new( + "port-forwarder", + self.portfw.handle(), + self.flow_table.clone(), + )); + pipeline = pipeline.add_stage(Checkpoint::new( "before masquerade", contract::ready_to_translate, @@ -174,8 +183,8 @@ impl Fabric { )); pipeline = pipeline.add_stage(Masquerade::new( "masquerade", - flow_table.clone(), - masquerade.get_reader(), + self.flow_table.clone(), + self.masquerade.handle(), )); let checking = translations.clone(); @@ -186,44 +195,31 @@ impl Fabric { }, )); - if let Some(tables) = &tables { - pipeline = pipeline.add_stage(IpForwarder::new("ip-forward-2", tables.fibs())); + if let Some(underlay) = &self.underlay { + pipeline = pipeline.add_stage(IpForwarder::new("ip-forward-2", underlay.fibs.handle())); pipeline = pipeline.add_stage(Egress::new( "egress", - tables.interfaces(), - tables.adjacencies(), + underlay.interfaces.handle(), + underlay.adjacencies.handle(), )); pipeline = pipeline.add_stage(Checkpoint::new("after egress", contract::finished)); } - Self { + Worker { pipeline, - flow_table, - _flow_filter: flow_filter, - _acl: acl, - _static_nat: static_nat, - _portfw: portfw, - _masquerade: masquerade, - _tables: tables, translations, next_id: 0, } } +} +impl Worker { pub(crate) fn send(&mut self, packet: Packet) -> Packet { let mut out = self.send_batch(vec![packet]); assert_eq!(out.len(), 1, "the pipeline did not return the packet"); out.pop().unwrap_or_else(|| unreachable!()) } - pub(crate) fn flows(&self) -> Option { - self.flow_table.len() - } - - pub(crate) fn shared_flow_table(&self) -> Arc { - self.flow_table.clone() - } - pub(crate) fn send_batch( &mut self, mut packets: Vec>, @@ -241,6 +237,85 @@ impl Fabric { } } +impl Fabric { + pub(crate) fn build(exposes: &[VpcExpose]) -> Option { + Self::build_with_acl(exposes, None) + } + + pub(crate) fn build_with_acl(exposes: &[VpcExpose], acl: Option<&Acl>) -> Option { + Self::assemble(exposes, acl, None) + } + + pub(crate) fn routed(exposes: &[VpcExpose], acl: Option<&Acl>) -> Option { + Self::routed_sharing(exposes, acl, Arc::new(FlowTable::default())) + } + + pub(crate) fn routed_sharing( + exposes: &[VpcExpose], + acl: Option<&Acl>, + flow_table: Arc, + ) -> Option { + let overlay = overlay_with_exposes_and_acl(exposes.to_vec(), acl) + .ok()? + .validate() + .ok()?; + Some(Self::over( + &overlay, + Some(topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)])), + flow_table, + )) + } + + pub(crate) fn routed_over(overlay: &Overlay, tables: RouterTables) -> Option { + Some(Self::over( + &overlay.clone().validate().ok()?, + Some(tables), + Arc::new(FlowTable::default()), + )) + } + + pub(crate) fn routed_over_validated(overlay: &ValidatedOverlay, tables: RouterTables) -> Self { + Self::over(overlay, Some(tables), Arc::new(FlowTable::default())) + } + + fn assemble( + exposes: &[VpcExpose], + acl: Option<&Acl>, + tables: Option, + ) -> Option { + let overlay = overlay_with_exposes_and_acl(exposes.to_vec(), acl) + .ok()? + .validate() + .ok()?; + Some(Self::over(&overlay, tables, Arc::new(FlowTable::default()))) + } + + fn over( + overlay: &ValidatedOverlay, + tables: Option, + flow_table: Arc, + ) -> Self { + let fleet = Fleet::lowering(overlay, tables, flow_table); + let worker = fleet.blueprint().worker(); + Self { fleet, worker } + } + + pub(crate) fn send(&mut self, packet: Packet) -> Packet { + self.worker.send(packet) + } + + pub(crate) fn send_batch( + &mut self, + packets: Vec>, + ) -> Vec> { + self.worker.send_batch(packets) + } + + pub(crate) fn flows(&self) -> Option { + self.fleet.blueprint().flow_table.len() + } +} + pub(crate) fn local() -> VpcDiscriminant { VpcDiscriminant::VNI(Vni::new_checked(LOCAL_VNI).unwrap_or_else(|_| unreachable!())) } @@ -347,7 +422,7 @@ pub(crate) struct Translations { was: std::collections::HashMap, from: std::collections::HashMap, given: std::collections::HashMap, - declared: Vec, + declared: Arc<[Prefix]>, } #[cfg(test)] @@ -407,29 +482,34 @@ impl Translations { self.given.clear(); } - fn declaring(overlay: &ValidatedOverlay) -> Self { - let mut declared = Vec::new(); - for vpc in overlay.vpc_table().values() { - for peering in vpc.peerings() { - for manifest in [peering.local(), peering.remote()] { - for expose in manifest.valexp() { - declared.extend( - expose - .public_ips() - .into_iter() - .map(lpm::prefix::PrefixWithOptionalPorts::prefix), - ); - } - } - } - } + fn declaring(declared: &Arc<[Prefix]>) -> Self { Self { - declared, + declared: declared.clone(), ..Self::default() } } } +#[cfg(test)] +fn declared_public_ranges(overlay: &ValidatedOverlay) -> Arc<[Prefix]> { + let mut declared = Vec::new(); + for vpc in overlay.vpc_table().values() { + for peering in vpc.peerings() { + for manifest in [peering.local(), peering.remote()] { + for expose in manifest.valexp() { + declared.extend( + expose + .public_ips() + .into_iter() + .map(lpm::prefix::PrefixWithOptionalPorts::prefix), + ); + } + } + } + } + declared.into() +} + #[cfg(test)] pub(crate) trait Load { fn next(&mut self) -> Option>; diff --git a/nat/src/masquerade/mod.rs b/nat/src/masquerade/mod.rs index 43e4488e3b..c8790bf223 100644 --- a/nat/src/masquerade/mod.rs +++ b/nat/src/masquerade/mod.rs @@ -22,7 +22,7 @@ mod test; //# REQ-9: A NAT MUST support "Hairpinning". // re exports pub use allocator_writer::MasqueradeConfig; -pub use allocator_writer::NatAllocatorWriter; +pub use allocator_writer::{NatAllocatorReaderFactory, NatAllocatorWriter}; pub use nf::Masquerade; use tracectl::trace_target; diff --git a/nat/src/static_nat/mod.rs b/nat/src/static_nat/mod.rs index a2bf321741..e520b7be92 100644 --- a/nat/src/static_nat/mod.rs +++ b/nat/src/static_nat/mod.rs @@ -11,6 +11,7 @@ pub mod setup; pub(crate) mod test; // re-exports +pub use natrw::NatTablesReaderFactory; pub use nf::{NatTablesWriter, StaticNat}; use tracectl::trace_target; diff --git a/routing/src/lib.rs b/routing/src/lib.rs index 3791867c8a..beb8234abb 100644 --- a/routing/src/lib.rs +++ b/routing/src/lib.rs @@ -27,17 +27,17 @@ mod router; mod routingdb; // re-exports -pub use atable::atablerw::AtableReader; +pub use atable::atablerw::{AtableReader, AtableReaderFactory}; pub use config::RouterConfig; pub use errors::RouterError; pub use evpn::Vtep; pub use fib::fibobjects::{EgressObject, FibEntry, PktInstruction}; -pub use fib::fibtable::FibTableReader; +pub use fib::fibtable::{FibTableReader, FibTableReaderFactory}; pub use fib::fibtype::FibKey; pub use frr::frrmi::FrrAppliedConfig; pub use frr::renderer::builder::Render; pub use interfaces::iftable::IfTable; -pub use interfaces::iftablerw::IfTableReader; +pub use interfaces::iftablerw::{IfTableReader, IfTableReaderFactory}; pub use interfaces::interface::{AttachConfig, Attachment, RouterInterfaceConfig}; pub use interfaces::interface::{IfDataEthernet, IfState, IfType, Interface}; pub use rib::encapsulation::{ From 57098995a311e8565ebc5682b3bfbd091ebf5bdb Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 19:47:00 -0600 Subject: [PATCH 06/21] test(dataplane): Model-check two workers against one masquerade allocator The first property here that runs in the arrangement production runs in: two pipelines, one allocator, one flow table. It discriminates a lost update in the port-block claim. The measurement that says shuttle rather than the sanitizer is what catches it -- and what shuttle still cannot see -- is recorded on the test. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/fuzz.rs | 101 ++++++++++++++++++++++++- 1 file changed, 100 insertions(+), 1 deletion(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index ddf2a3d286..24959d043d 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -3568,12 +3568,13 @@ mod routed { #[cfg(test)] mod model { - use super::routed::{exposes, inner, tunnelled}; + use super::routed::{exposes, inner, inside, tunnelled}; use super::*; use concurrency::sync::Mutex; use concurrency::thread; #[cfg_attr(not(feature = "shuttle"), allow(unused_imports))] use concurrency::thread::BuilderExt; + use net::packet::test_utils::build_test_udp_ipv4_packet; use std::sync::OnceLock; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -3671,4 +3672,102 @@ mod model { }); }); } + fn _a_blueprint_crosses_a_thread_boundary(blueprint: &Blueprint) { + fn shareable(_: &T) {} + shareable(blueprint); + } + + #[concurrency::model_test] + fn two_workers_are_not_given_the_same_public_tuple() { + const FLOWS: u16 = 3; + + let _eal = dpdk::test_support::start_eal(); + + let rt = cfg_select! { + feature = "shuttle" => None::, + _ => Some( + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("build tokio runtime") + ) + }; + let handle = rt.as_ref().map(tokio::runtime::Runtime::handle).cloned(); + + concurrency::stress(move || { + let overlay = overlay_with_exposes_and_acl(exposes(), None) + .expect("the fixture exposes form an overlay") + .validate() + .expect("the fixture overlay validates"); + let fleet = Fleet::lowering( + &overlay, + Some(topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)])), + Arc::new(FlowTable::default()), + ); + let blueprint = fleet.blueprint(); + + let workers = [("1.1.0.1", 1000u16), ("1.1.0.2", 2000u16)]; + let given: Vec<(Option, Option)> = thread::scope(|scope| { + let running: Vec<_> = workers + .iter() + .map(|(host, first)| { + let entering = handle.clone(); + thread::Builder::new() + .name(format!("worker-{host}")) + .spawn_scoped(scope, move || { + let _guard = entering.as_ref().map(tokio::runtime::Handle::enter); + let mut worker = blueprint.worker(); + let burst = (0..FLOWS) + .map(|n| { + tunnelled(&build_test_udp_ipv4_packet( + host, + "3.3.3.1", + first + n, + 80, + )) + }) + .collect(); + worker + .send_batch(burst) + .iter() + .map(|out| { + assert!( + matches!(verdict(out), Verdict::Delivered { .. }), + "a packet that is delivered single-threaded was not: \ + {:?}", + verdict(out) + ); + let tenant = inside(out).expect( + "a delivered frame leaves this gateway tunnelled", + ); + ( + tenant.ip_source(), + tenant.transport_src_port().map(std::num::NonZero::get), + ) + }) + .collect::>() + }) + .expect("spawn worker") + }) + .collect(); + running + .into_iter() + .flat_map(|worker| worker.join().expect("worker panicked")) + .collect() + }); + + let mut seen = std::collections::BTreeMap::new(); + for tuple in &given { + let count: &mut usize = seen.entry(format!("{tuple:?}")).or_default(); + *count += 1; + assert_eq!( + *count, + 1, + "{} distinct flows were translated and {tuple:?} was handed out twice, so a \ + reply to it cannot be attributed to either of them: {given:?}", + given.len() + ); + } + }); + } } From dd20068480d7271b9e78ab34e9966b596f5cf689 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 20:22:09 -0600 Subject: [PATCH 07/21] fix(flow-filter): Make the acl table-name counter process-unique again `table_name` promised process uniqueness and, under a model checker, did not deliver it. The counter was the concurrency facade's atomic, which belongs to the execution that created it; `rte_acl`'s registry is process-global and resets for nobody. Found by `dataplane::packet_processor::fuzz::model`, which lowers many configurations per process and so was the first thing to notice. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- flow-filter/src/context/tables.rs | 37 +++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/flow-filter/src/context/tables.rs b/flow-filter/src/context/tables.rs index dd2a051ba0..1c4a72fc64 100644 --- a/flow-filter/src/context/tables.rs +++ b/flow-filter/src/context/tables.rs @@ -347,18 +347,37 @@ impl fmt::Debug for AnyTable { } } -// 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)); +concurrency::with_std! { + static TABLE_SEQ: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + + fn next_in_sequence() -> u64 { + TABLE_SEQ.fetch_add(1, Ordering::Relaxed) + } +} + +concurrency::with_loom! { + // nosemgrep: rust-no-direct-std-sync-import + static TABLE_SEQ: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + + fn next_in_sequence() -> u64 { + // nosemgrep: rust-no-direct-std-sync-import + TABLE_SEQ.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + } +} + +concurrency::with_shuttle! { + // nosemgrep: rust-no-direct-std-sync-import + static TABLE_SEQ: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + + fn next_in_sequence() -> u64 { + // nosemgrep: rust-no-direct-std-sync-import + TABLE_SEQ.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + } +} /// A process-unique rte_acl context name (rte_acl rejects duplicate names). fn table_name(base: &str) -> String { - format!( - "flow_filter_{base}_{}", - TABLE_SEQ.fetch_add(1, Ordering::Relaxed) - ) + format!("flow_filter_{base}_{}", next_in_sequence()) } /// Build one table from backend-neutral rules using the selected backend. From c8ae794f3a56d1f3779677bc460b4e4b5caca7be Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 20:22:09 -0600 Subject: [PATCH 08/21] test(dataplane): Draw the configuration for the multi-worker properties bolero becomes the outer loop over the same draw the single-threaded generated property uses, so a shape reachable on one thread is reachable on two by construction. Measured against a deliberate allocator defect: this property does not catch it and the targeted one does. Both the measurement and the reason are on the test, because "the generated property covers that too" is the assumption it refutes. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/fuzz.rs | 122 ++++++++++++++++++++++--- 1 file changed, 110 insertions(+), 12 deletions(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 24959d043d..9de4d59fd6 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -311,6 +311,10 @@ impl Fabric { self.worker.send_batch(packets) } + pub(crate) fn worker(&mut self) -> &mut Worker { + &mut self.worker + } + pub(crate) fn flows(&self) -> Option { self.fleet.blueprint().flow_table.len() } @@ -524,7 +528,7 @@ pub(crate) trait Load { } #[cfg(test)] -pub(crate) fn drive(fabric: &mut Fabric, load: &mut dyn Load) { +pub(crate) fn drive(worker: &mut Worker, load: &mut dyn Load) { for _ in 0..64 { if load.finished() { return; @@ -535,7 +539,7 @@ pub(crate) fn drive(fabric: &mut Fabric, load: &mut dyn Load) { load.describe() ); }; - let out = fabric.send(packet); + let out = worker.send(packet); load.observe(&out); } panic!("a load did not finish in 64 steps: {}", load.describe()); @@ -553,7 +557,7 @@ pub(crate) type Poll = Vec; #[cfg(test)] pub(crate) fn run_schedule( - fabric: &mut Fabric, + worker: &mut Worker, loads: &mut [Box], schedule: &[Poll], ) -> Vec> { @@ -577,14 +581,14 @@ pub(crate) fn run_schedule( if burst.is_empty() { continue; } - for (answer, which) in fabric.send_batch(burst).iter().zip(&origin) { + for (answer, which) in worker.send_batch(burst).iter().zip(&origin) { loads[*which].observe(answer); } bursts.push(origin); } for load in loads { - drive(fabric, load.as_mut()); + drive(worker, load.as_mut()); } bursts } @@ -2101,7 +2105,7 @@ mod interleaved { }); } - for burst in run_schedule(&mut fabric, &mut loads, schedule) { + for burst in run_schedule(fabric.worker(), &mut loads, schedule) { let mut loads_in: Vec = burst.clone(); loads_in.sort_unstable(); loads_in.dedup(); @@ -2273,7 +2277,7 @@ mod offers { } } - for burst in run_schedule(&mut fabric, &mut loads, schedule) { + for burst in run_schedule(fabric.worker(), &mut loads, schedule) { let mut seen = burst.clone(); seen.sort_unstable(); seen.dedup(); @@ -2336,7 +2340,7 @@ mod generated { const SENDERS: usize = 6; const POLLS: usize = 8; - struct Generated; + pub(super) struct Generated; impl ValueGenerator for Generated { type Output = (Vec, Vec, Vec); @@ -2421,7 +2425,7 @@ mod generated { let mut loads = loads_for(&validated, vary); DERIVED.fetch_add(loads.len() as u64, Ordering::Relaxed); - for burst in run_schedule(&mut fabric, &mut loads, schedule) { + for burst in run_schedule(fabric.worker(), &mut loads, schedule) { let mut seen = burst.clone(); seen.sort_unstable(); seen.dedup(); @@ -3082,7 +3086,7 @@ mod routed { let mut load = Conversation::new(Path::fixture(), src, dst, flow.sport, flow.dport); - drive(&mut fabric, &mut load); + drive(fabric.worker(), &mut load); if load.checked() { ROUND_TRIPPED.fetch_add(1, Ordering::Relaxed); @@ -3568,15 +3572,17 @@ mod routed { #[cfg(test)] mod model { + use super::derive::loads_for; use super::routed::{exposes, inner, inside, tunnelled}; use super::*; use concurrency::sync::Mutex; use concurrency::thread; #[cfg_attr(not(feature = "shuttle"), allow(unused_imports))] use concurrency::thread::BuilderExt; + use config::external::overlay::algebra::Sequence; use net::packet::test_utils::build_test_udp_ipv4_packet; - use std::sync::OnceLock; - use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; + use std::sync::{LazyLock, OnceLock}; fn on_two_threads(body: &(impl Fn() + Sync)) { thread::scope(|scope| { @@ -3770,4 +3776,96 @@ mod model { } }); } + + #[concurrency::model_test] + fn generated_traffic_survives_being_split_across_two_workers() { + const CASES: usize = 64; + + static SPLIT: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static THIN: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + + let _eal = dpdk::test_support::start_eal(); + + let rt = cfg_select! { + feature = "shuttle" => None::, + _ => Some( + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("build tokio runtime") + ) + }; + let handle = rt.as_ref().map(tokio::runtime::Runtime::handle).cloned(); + + bolero::check!() + .with_max_len(MAX_INPUT_LEN) + .with_generator(generated::Generated) + .with_iterations(CASES) + .for_each(|(ops, vary, schedule)| { + let validated = Sequence::fold(ops) + .overlay() + .unwrap_or_else(|e| panic!("{ops:?} does not assemble: {e}")) + .validate() + .unwrap_or_else(|e| panic!("{ops:?} does not validate: {e}")); + + let vnis: Vec = validated + .vpc_table() + .values() + .map(config::external::overlay::vpc::ValidatedVpc::vni) + .collect(); + if vnis.is_empty() || loads_for(&validated, vary).len() < 2 { + THIN.fetch_add(1, Ordering::Relaxed); + return; + } + SPLIT.fetch_add(1, Ordering::Relaxed); + + let drawn = std::sync::Arc::new((validated, vnis, vary.clone(), schedule.clone())); + let entering = handle.clone(); + + concurrency::stress(move || { + let (validated, vnis, vary, schedule) = &*drawn; + let fleet = Fleet::lowering( + validated, + Some(topology(vnis)), + Arc::new(FlowTable::default()), + ); + let blueprint = fleet.blueprint(); + + thread::scope(|scope| { + let running: Vec<_> = (0..2) + .map(|which| { + let entering = entering.clone(); + thread::Builder::new() + .name(format!("worker-{which}")) + .spawn_scoped(scope, move || { + let _guard = + entering.as_ref().map(tokio::runtime::Handle::enter); + let mut worker = blueprint.worker(); + let mut mine: Vec> = + loads_for(validated, vary) + .into_iter() + .enumerate() + .filter(|(nth, _)| nth % 2 == which) + .map(|(_, load)| load) + .collect(); + run_schedule(&mut worker, &mut mine, schedule); + }) + .expect("spawn worker") + }) + .collect(); + for worker in running { + worker.join().expect("worker panicked"); + } + }); + }); + }); + + let (split, thin) = (SPLIT.load(Ordering::Relaxed), THIN.load(Ordering::Relaxed)); + eprintln!("split={split} thin={thin}"); + super::assert_covered( + split > 0, + "no drawn configuration ever implied enough traffic to load two workers, so this ran \ + nothing concurrently", + ); + } } From 3628f9e4f765b04c823b1cb7528c1beb86c7a24d Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 20:48:37 -0600 Subject: [PATCH 09/21] feat(tracectl): Capture trace evidence, and print it only on a failure Tests can now hold a recording that dumps the spans and events leading to a panic. It cannot be asserted on: there is no way to read the trace back into the program, and the module explains at length why that is the feature rather than a missing one. Three things about it were measured rather than assumed, and are written down where the next person will need them: which targets drown a dump under a model checker, that `#[instrument]` on the packet path captures whole `Packet` values, and that a scoped subscriber cannot exist under loom or shuttle at all. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/Cargo.toml | 1 + dataplane/src/packet_processor/fuzz.rs | 21 +- tracectl/Cargo.toml | 3 + tracectl/src/evidence.rs | 532 +++++++++++++++++++++++++ tracectl/src/lib.rs | 3 + 5 files changed, 556 insertions(+), 4 deletions(-) create mode 100644 tracectl/src/evidence.rs diff --git a/dataplane/Cargo.toml b/dataplane/Cargo.toml index a18193d256..d5cae60996 100644 --- a/dataplane/Cargo.toml +++ b/dataplane/Cargo.toml @@ -69,6 +69,7 @@ routing = { workspace = true, features = ["testing"] } bolero = { workspace = true, default-features = false, features = ["alloc"] } n-vm = { workspace = true } tokio = { workspace = true, features = ["macros", "rt", "test-util", "time"] } +tracectl = { workspace = true, features = ["evidence"] } tracing = { workspace = true } tracing-subscriber = { workspace = true } tracing-test = { workspace = true, features = [] } diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 9de4d59fd6..928b543d50 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -3584,6 +3584,10 @@ mod model { use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; use std::sync::{LazyLock, OnceLock}; + type Tuple = (Option, Option); + + type Reported = (Vec, tracectl::evidence::Evidence); + fn on_two_threads(body: &(impl Fn() + Sync)) { thread::scope(|scope| { let handles: Vec<_> = (0..2) @@ -3713,7 +3717,7 @@ mod model { let blueprint = fleet.blueprint(); let workers = [("1.1.0.1", 1000u16), ("1.1.0.2", 2000u16)]; - let given: Vec<(Option, Option)> = thread::scope(|scope| { + let given: Vec = thread::scope(|scope| { let running: Vec<_> = workers .iter() .map(|(host, first)| { @@ -3722,6 +3726,8 @@ mod model { .name(format!("worker-{host}")) .spawn_scoped(scope, move || { let _guard = entering.as_ref().map(tokio::runtime::Handle::enter); + let recording = + tracectl::evidence::capture(format!("worker-{host}")); let mut worker = blueprint.worker(); let burst = (0..FLOWS) .map(|n| { @@ -3733,7 +3739,7 @@ mod model { )) }) .collect(); - worker + let tuples = worker .send_batch(burst) .iter() .map(|out| { @@ -3751,17 +3757,22 @@ mod model { tenant.transport_src_port().map(std::num::NonZero::get), ) }) - .collect::>() + .collect::>(); + (tuples, recording.evidence()) }) .expect("spawn worker") }) .collect(); running .into_iter() - .flat_map(|worker| worker.join().expect("worker panicked")) + .map(|worker| worker.join().expect("worker panicked")) .collect() }); + let (given, evidence): (Vec>, Vec<_>) = given.into_iter().unzip(); + let given: Vec<_> = given.into_iter().flatten().collect(); + let _explain = tracectl::evidence::dump_on_panic(evidence); + let mut seen = std::collections::BTreeMap::new(); for tuple in &given { let count: &mut usize = seen.entry(format!("{tuple:?}")).or_default(); @@ -3840,6 +3851,8 @@ mod model { .spawn_scoped(scope, move || { let _guard = entering.as_ref().map(tokio::runtime::Handle::enter); + let _evidence = + tracectl::evidence::capture(format!("worker-{which}")); let mut worker = blueprint.worker(); let mut mine: Vec> = loads_for(validated, vary) diff --git a/tracectl/Cargo.toml b/tracectl/Cargo.toml index 08e33946b6..48849da4f5 100644 --- a/tracectl/Cargo.toml +++ b/tracectl/Cargo.toml @@ -5,6 +5,9 @@ license.workspace = true publish.workspace = true version.workspace = true +[features] +evidence = [] + [dependencies] clock = { workspace = true } color-eyre = { workspace = true , features = [ "capture-spantrace", "color-spantrace", "tracing-error", "track-caller" ] } diff --git a/tracectl/src/evidence.rs b/tracectl/src/evidence.rs new file mode 100644 index 0000000000..37f67d4dc9 --- /dev/null +++ b/tracectl/src/evidence.rs @@ -0,0 +1,532 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +#![allow(clippy::disallowed_types)] + +use std::collections::VecDeque; +use std::fmt; +use std::fmt::Write as _; +// nosemgrep: rust-no-direct-std-sync-import +use std::sync::Arc; + +use tracing::field::{Field, Visit}; +use tracing::span::Attributes; +use tracing::subscriber::DefaultGuard; +use tracing::{Event, Id, Level, Metadata, Subscriber}; +use tracing_subscriber::layer::{Context, Layer, SubscriberExt}; +use tracing_subscriber::registry::LookupSpan; + +pub const DEFAULT_DEPTH: usize = 512; + +struct Line { + level: Level, + target: &'static str, + kind: Kind, + scope: String, + message: String, + fields: String, + repeats: usize, +} + +impl Line { + fn same_as(&self, other: &Self) -> bool { + self.level == other.level + && self.target == other.target + && self.kind == other.kind + && self.scope == other.scope + && self.message == other.message + && self.fields == other.fields + } +} + +#[derive(Clone, Copy, PartialEq, Eq)] +enum Kind { + Span, + Event, +} + +impl fmt::Display for Line { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let marker = match self.kind { + Kind::Span => "span", + Kind::Event => " ", + }; + write!(f, "{marker} {:>5} {}", self.level, self.target)?; + if !self.scope.is_empty() { + write!(f, " [{}]", self.scope)?; + } + if !self.message.is_empty() { + write!(f, ": {}", self.message)?; + } + if !self.fields.is_empty() { + write!(f, " {{{}}}", self.fields)?; + } + if self.repeats > 1 { + write!(f, " (x{})", self.repeats)?; + } + Ok(()) + } +} + +struct Log { + lines: VecDeque, + depth: usize, + dropped: usize, +} + +impl Log { + fn push(&mut self, line: Line) { + if let Some(last) = self.lines.back_mut() + && last.same_as(&line) + { + last.repeats += 1; + return; + } + if self.lines.len() == self.depth { + self.lines.pop_front(); + self.dropped += 1; + } + self.lines.push_back(line); + } +} + +#[derive(Default)] +struct Fields { + message: String, + rest: String, +} + +const FIELD_LIMIT: usize = 160; + +fn push_clipped(out: &mut String, value: &dyn fmt::Display) { + let rendered = value.to_string(); + if rendered.len() <= FIELD_LIMIT { + out.push_str(&rendered); + return; + } + let cut = rendered + .char_indices() + .map(|(at, _)| at) + .take_while(|at| *at <= FIELD_LIMIT) + .last() + .unwrap_or(0); + out.push_str(&rendered[..cut]); + let _ = write!(out, "...<{} more bytes>", rendered.len() - cut); +} + +impl Fields { + fn put(&mut self, field: &Field, value: &dyn fmt::Display) { + if field.name() == "message" { + push_clipped(&mut self.message, value); + return; + } + if !self.rest.is_empty() { + self.rest.push(' '); + } + let _ = write!(self.rest, "{}=", field.name()); + push_clipped(&mut self.rest, value); + } +} + +impl Visit for Fields { + fn record_f64(&mut self, field: &Field, value: f64) { + self.put(field, &value); + } + fn record_i64(&mut self, field: &Field, value: i64) { + self.put(field, &value); + } + fn record_u64(&mut self, field: &Field, value: u64) { + self.put(field, &value); + } + fn record_i128(&mut self, field: &Field, value: i128) { + self.put(field, &value); + } + fn record_u128(&mut self, field: &Field, value: u128) { + self.put(field, &value); + } + fn record_bool(&mut self, field: &Field, value: bool) { + self.put(field, &value); + } + fn record_str(&mut self, field: &Field, value: &str) { + self.put(field, &value); + } + fn record_error(&mut self, field: &Field, value: &(dyn std::error::Error + 'static)) { + self.put(field, &value); + } + fn record_debug(&mut self, field: &Field, value: &dyn fmt::Debug) { + self.put(field, &format_args!("{value:?}")); + } +} + +pub const MACHINERY: &[&str] = &["shuttle", "tokio", "runtime", "mio", "hyper"]; + +type Keep = Arc bool + Send + Sync>; + +concurrency::with_std! { + const RECORDS: bool = true; + + #[allow(clippy::unnecessary_wraps)] + fn install(subscriber: S) -> Option { + Some(tracing::subscriber::set_default(subscriber)) + } +} + +concurrency::with_loom! { + const RECORDS: bool = false; + + fn install(_subscriber: S) -> Option { + None + } +} + +concurrency::with_shuttle! { + const RECORDS: bool = false; + + fn install(_subscriber: S) -> Option { + None + } +} + +struct EvidenceLayer { + // nosemgrep: rust-no-direct-std-sync-import + log: Arc>, + keep: Keep, +} + +impl EvidenceLayer { + fn push(&self, line: Line) { + if let Ok(mut log) = self.log.lock() { + log.push(line); + } + } +} + +fn scope_of(ctx: &Context<'_, S>, event: Option<&Event<'_>>, id: Option<&Id>) -> String +where + S: Subscriber + for<'a> LookupSpan<'a>, +{ + let scope = match (event, id) { + (Some(event), _) => ctx.event_scope(event).map(from_root), + (None, Some(id)) => ctx.span_scope(id).map(from_root), + (None, None) => None, + }; + scope.unwrap_or_default() +} + +fn from_root<'a, S>(scope: tracing_subscriber::registry::Scope<'a, S>) -> String +where + S: Subscriber + for<'lookup> LookupSpan<'lookup>, +{ + let mut names: Vec<&str> = scope.map(|span| span.name()).collect(); + names.reverse(); + names.join(" > ") +} + +impl Layer for EvidenceLayer +where + S: Subscriber + for<'a> LookupSpan<'a>, +{ + fn enabled(&self, metadata: &Metadata<'_>, _ctx: Context<'_, S>) -> bool { + (self.keep)(metadata.target()) + } + + fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) { + let mut fields = Fields::default(); + attrs.record(&mut fields); + let metadata = attrs.metadata(); + self.push(Line { + level: *metadata.level(), + target: metadata.target(), + kind: Kind::Span, + scope: scope_of(&ctx, None, Some(id)), + message: fields.message, + fields: fields.rest, + repeats: 1, + }); + } + + fn on_event(&self, event: &Event<'_>, ctx: Context<'_, S>) { + let mut fields = Fields::default(); + event.record(&mut fields); + let metadata = event.metadata(); + self.push(Line { + level: *metadata.level(), + target: metadata.target(), + kind: Kind::Event, + scope: scope_of(&ctx, Some(event), None), + message: fields.message, + fields: fields.rest, + repeats: 1, + }); + } +} + +#[must_use = "a recording that is dropped immediately captures nothing"] +pub struct Recording { + // nosemgrep: rust-no-direct-std-sync-import + log: Arc>, + label: String, + _installed: Option, +} + +#[must_use = "a recording that is dropped immediately captures nothing"] +pub fn capture(label: impl Into) -> Recording { + Capture::new(label).start() +} + +pub struct Capture { + label: String, + depth: usize, + keep: Keep, +} + +impl Capture { + #[must_use] + pub fn new(label: impl Into) -> Self { + Self { + label: label.into(), + depth: DEFAULT_DEPTH, + keep: Arc::new(|target: &str| !MACHINERY.iter().any(|noisy| target.starts_with(noisy))), + } + } + + #[must_use] + pub fn depth(mut self, depth: usize) -> Self { + self.depth = depth.max(1); + self + } + + #[must_use] + pub fn keeping(mut self, keep: impl Fn(&str) -> bool + Send + Sync + 'static) -> Self { + self.keep = Arc::new(keep); + self + } + + #[must_use = "a recording that is dropped immediately captures nothing"] + pub fn start(self) -> Recording { + let empty = Log { + lines: VecDeque::new(), + depth: self.depth, + dropped: 0, + }; + // nosemgrep: rust-no-direct-std-sync-import + let log = Arc::new(std::sync::Mutex::new(empty)); + let subscriber = tracing_subscriber::registry().with(EvidenceLayer { + log: log.clone(), + keep: self.keep, + }); + Recording { + log, + label: self.label, + _installed: install(subscriber), + } + } +} + +#[derive(Clone)] +pub struct Evidence { + // nosemgrep: rust-no-direct-std-sync-import + log: Arc>, + label: String, +} + +impl Evidence { + pub fn dump(&self) { + let Ok(log) = self.log.lock() else { + eprintln!("==== trace evidence ({}): buffer poisoned ====", self.label); + return; + }; + if log.lines.is_empty() { + let why = if RECORDS { + "Either the code under test emits no spans or events, or the level filter is above \ + them, or the targets were excluded -- see MACHINERY. Note that \ + `release_max_level_debug` compiles `trace!` out of release builds altogether, and \ + that a recording only sees the thread it was created on." + } else { + "This is a model-checker build, where a recording deliberately installs nothing: \ + loom and shuttle multiplex their tasks onto one OS thread and would corrupt the \ + thread-local the scoped dispatcher lives in. Use the replayable schedule the \ + backend printed instead. See this module's docs." + }; + eprintln!( + "==== trace evidence ({}): nothing captured ====\n{why}", + self.label + ); + return; + } + eprintln!("==== trace evidence ({}) ====", self.label); + if log.dropped > 0 { + eprintln!("[{} earlier lines dropped by the ring]", log.dropped); + } + for line in &log.lines { + eprintln!("{line}"); + } + eprintln!("==== end trace evidence ({}) ====", self.label); + } +} + +#[must_use = "the guard must outlive the assertions it is meant to explain"] +pub fn dump_on_panic(evidence: Vec) -> impl Sized { + struct OnPanic(Vec); + impl Drop for OnPanic { + fn drop(&mut self) { + if std::thread::panicking() { + for evidence in &self.0 { + evidence.dump(); + } + } + } + } + OnPanic(evidence) +} + +impl Recording { + #[must_use] + pub fn evidence(&self) -> Evidence { + Evidence { + log: self.log.clone(), + label: self.label.clone(), + } + } + + pub fn dump(&self) { + self.evidence().dump(); + } + + #[cfg(test)] + fn len(&self) -> usize { + self.log.lock().map_or(0, |log| log.lines.len()) + } + + #[cfg(test)] + fn rendered(&self) -> Vec { + self.log.lock().map_or_else( + |_| Vec::new(), + |log| log.lines.iter().map(ToString::to_string).collect(), + ) + } +} + +impl Drop for Recording { + fn drop(&mut self) { + if std::thread::panicking() { + self.dump(); + } + } +} + +#[cfg(test)] +#[cfg(not(any(feature = "loom", feature = "shuttle")))] +mod tests { + use super::*; + + #[test] + fn a_recording_captures_events_on_its_own_thread() { + let recording = capture("own thread"); + tracing::error!(port = 8080_u64, "a thing happened"); + assert_eq!(recording.len(), 1, "the event was not captured"); + } + + #[test] + fn a_recording_captures_spans_and_the_events_inside_them() { + let recording = capture("spans"); + let span = tracing::error_span!("outer", vni = 100_u64); + let _entered = span.enter(); + tracing::error!("inside"); + assert_eq!(recording.len(), 2, "expected the span and the event"); + } + + #[test] + fn the_ring_drops_the_oldest() { + let recording = Capture::new("bounded").depth(4).start(); + for n in 0..10_u64 { + tracing::error!(n, "line"); + } + assert_eq!(recording.len(), 4, "the ring did not bound itself"); + } + + #[test] + fn a_recording_does_not_capture_another_thread() { + let recording = capture("this thread"); + std::thread::scope(|scope| { + scope.spawn(|| tracing::error!("from elsewhere")); + }); + assert_eq!(recording.len(), 0, "a recording reached across a thread"); + } + + #[test] + fn a_scalar_field_is_read_as_a_scalar() { + let recording = capture("typed"); + tracing::error!(port = 8080_u64, name = "eth0", up = true, "hello"); + let rendered = recording.rendered().join("\n"); + + assert!(rendered.contains("port=8080"), "u64 field: {rendered}"); + assert!( + rendered.contains("name=eth0"), + "a str field went through the `Debug` fallback: {rendered}" + ); + assert!(rendered.contains("up=true"), "bool field: {rendered}"); + assert!( + rendered.contains(": hello"), + "the message was not lifted out of the fields: {rendered}" + ); + } + + #[test] + fn machinery_is_not_evidence() { + let recording = capture("filtered"); + tracing::error!(target: "shuttle::runtime::execution", "scheduling decision"); + tracing::error!(target: "dataplane_nat::masquerade", "allocated"); + assert_eq!(recording.len(), 1, "the machinery target was captured"); + } + + #[test] + fn a_capture_can_be_narrowed_to_one_crate() { + let recording = Capture::new("narrow") + .keeping(|target| target.starts_with("dataplane_nat")) + .start(); + tracing::error!(target: "dataplane_net::packet", "dropping"); + tracing::error!(target: "dataplane_nat::masquerade", "allocated"); + assert_eq!(recording.len(), 1, "the filter did not narrow"); + } + + #[test] + fn a_run_of_identical_lines_collapses() { + let recording = Capture::new("repeats").depth(8).start(); + for _ in 0..4 { + tracing::error!(target: "dataplane_net::buffer", "Dropping TestBuffer"); + } + assert_eq!(recording.len(), 1, "identical lines were not collapsed"); + assert!( + recording.rendered().join("\n").contains("(x4)"), + "the collapsed line does not say how many: {:?}", + recording.rendered() + ); + } + + #[test] + fn only_consecutive_lines_collapse() { + let recording = Capture::new("interleaved").depth(8).start(); + tracing::error!(target: "dataplane_net::buffer", "a"); + tracing::error!(target: "dataplane_net::buffer", "b"); + tracing::error!(target: "dataplane_net::buffer", "a"); + assert_eq!(recording.len(), 3, "non-consecutive lines were merged"); + } + + #[test] + fn an_enormous_field_is_clipped() { + let recording = capture("clipped"); + let huge = "x".repeat(8192); + tracing::error!(target: "dataplane_net::packet", packet = %huge, "in"); + let rendered = recording.rendered().join("\n"); + assert!( + rendered.len() < 512, + "an 8 KiB field was kept whole: {} bytes", + rendered.len() + ); + assert!( + rendered.contains("more bytes>"), + "the clip is not marked, so a reader cannot tell: {rendered}" + ); + } +} diff --git a/tracectl/src/lib.rs b/tracectl/src/lib.rs index 3e6d5bb735..4e18c10b4e 100644 --- a/tracectl/src/lib.rs +++ b/tracectl/src/lib.rs @@ -6,6 +6,9 @@ #![deny(clippy::all, clippy::pedantic)] #![allow(clippy::missing_errors_doc)] +#[cfg(any(test, feature = "evidence"))] +pub mod evidence; + pub mod control; pub mod display; pub mod targets; From b7a6e5382e266265f4dc5a252b7b3d7cbe4bb964 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 20:56:32 -0600 Subject: [PATCH 10/21] test(dataplane): Split a flow's request and reply across two workers The arrangement receive-side steering normally puts the gateway in, and the one no property here has ever been in: a reply's tuple is the request's reversed, so the return traffic of a flow opened on one worker routinely lands on another. The oracle is the existing `Conversation::judge_reply`, reused rather than restated. The limits of what this property isolates are recorded on the test itself, since it does not discriminate the crossing from the flow handling. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/fuzz.rs | 140 ++++++++++++++++++++++++- 1 file changed, 139 insertions(+), 1 deletion(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 928b543d50..96970f13bd 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -3573,7 +3573,7 @@ mod routed { #[cfg(test)] mod model { use super::derive::loads_for; - use super::routed::{exposes, inner, inside, tunnelled}; + use super::routed::{Conversation, exposes, inner, inside, tunnelled}; use super::*; use concurrency::sync::Mutex; use concurrency::thread; @@ -3881,4 +3881,142 @@ mod model { nothing concurrently", ); } + + fn step(worker: &mut Worker, load: &mut dyn Load) { + let Some(packet) = load.next() else { + panic!( + "a load was asked for a packet it would not give: {}", + load.describe() + ); + }; + let out = worker.send(packet); + load.observe(&out); + } + + #[concurrency::model_test] + fn a_reply_is_translated_by_a_worker_that_never_saw_the_request() { + const FLOWS: u8 = 2; + + static CLOSED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static ABANDONED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + + let _eal = dpdk::test_support::start_eal(); + + let rt = cfg_select! { + feature = "shuttle" => None::, + _ => Some( + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("build tokio runtime") + ) + }; + let handle = rt.as_ref().map(tokio::runtime::Runtime::handle).cloned(); + + concurrency::stress(move || { + let overlay = overlay_with_exposes_and_acl(exposes(), None) + .expect("the fixture exposes form an overlay") + .validate() + .expect("the fixture overlay validates"); + let fleet = Fleet::lowering( + &overlay, + Some(topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)])), + Arc::new(FlowTable::default()), + ); + let blueprint = fleet.blueprint(); + let entering = handle.clone(); + + let mut opened: Vec> = thread::scope(|scope| { + let running: Vec<_> = (0..2u8) + .map(|which| { + let entering = entering.clone(); + thread::Builder::new() + .name(format!("open-{which}")) + .spawn_scoped(scope, move || { + let _guard = entering.as_ref().map(tokio::runtime::Handle::enter); + let _evidence = + tracectl::evidence::capture(format!("open-{which}")); + let mut worker = blueprint.worker(); + (0..FLOWS) + .map(|nth| { + let src = format!("1.1.{which}.{}", nth + 1); + let mut convo = Conversation::new( + super::routed::Path::fixture(), + src.parse().unwrap_or_else(|e| { + unreachable!("{src} is an address: {e}") + }), + "3.3.3.1" + .parse() + .unwrap_or_else(|e| unreachable!("{e}")), + u16::from(nth) + 1000, + 80, + ); + step(&mut worker, &mut convo); + convo + }) + .collect::>() + }) + .expect("spawn opener") + }) + .collect(); + running + .into_iter() + .map(|opener| opener.join().expect("opener panicked")) + .collect() + }); + + let second = opened + .pop() + .unwrap_or_else(|| unreachable!("two openers ran")); + let first = opened + .pop() + .unwrap_or_else(|| unreachable!("two openers ran")); + + let answered: Vec> = thread::scope(|scope| { + let running: Vec<_> = [(0u8, second), (1u8, first)] + .into_iter() + .map(|(which, mut theirs)| { + let entering = entering.clone(); + thread::Builder::new() + .name(format!("answer-{which}")) + .spawn_scoped(scope, move || { + let _guard = entering.as_ref().map(tokio::runtime::Handle::enter); + let _evidence = + tracectl::evidence::capture(format!("answer-{which}")); + let mut worker = blueprint.worker(); + for convo in &mut theirs { + if !convo.finished() { + step(&mut worker, convo); + } + } + theirs + }) + .expect("spawn answerer") + }) + .collect(); + running + .into_iter() + .map(|answerer| answerer.join().expect("answerer panicked")) + .collect() + }); + + for convo in answered.into_iter().flatten() { + if convo.checked() { + CLOSED.fetch_add(1, Ordering::Relaxed); + } else { + ABANDONED.fetch_add(1, Ordering::Relaxed); + } + } + }); + + let (closed, abandoned) = ( + CLOSED.load(Ordering::Relaxed), + ABANDONED.load(Ordering::Relaxed), + ); + eprintln!("closed={closed} abandoned={abandoned}"); + super::assert_covered( + closed > 0, + "no conversation was ever answered by the other worker, so nothing crossed", + ); + } } From 5a86ac46ab64846aa1a2794d70028dd9c4bc998a Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 21:12:22 -0600 Subject: [PATCH 11/21] fix(nat): Do not hold a flow's read guard across the icmp handlers Both handlers re-read the same `RwLock`, and so does the `Display` behind the `logfmt()` in their debug lines, so the guard made those recursive acquisitions. `parking_lot` gives no recursion guarantee, and writers on this lock are ordinary data-path traffic; one of the two acquisitions being a log line means the window only opens when tracing is turned up. Found by the new model property below, which shuttle refused outright. Real threads had passed it every time. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/fuzz.rs | 189 +++++++++++++++++++++++++ nat/src/icmp_handler/nf.rs | 16 ++- 2 files changed, 201 insertions(+), 4 deletions(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 96970f13bd..028d05b0ea 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -4019,4 +4019,193 @@ mod model { "no conversation was ever answered by the other worker, so nothing crossed", ); } + + fn unreachable( + code: net::icmp4::Icmp4DestUnreachable, + public: (IpAddr, u16), + target: IpAddr, + ) -> Packet { + let (IpAddr::V4(public_v4), IpAddr::V4(target_v4)) = (public.0, target) else { + unreachable!("the fixture is v4 throughout") + }; + let inner = + net::packet::test_utils::build_test_icmp4_destination_unreachable_packet_with_code( + code, + net::packet::test_utils::IcmpErrorAddrs { + outer_src: target_v4, + outer_dst: public_v4, + inner_src: public_v4, + inner_dst: target_v4, + }, + net::ip::NextHeader::UDP, + public.1, + 80, + ) + .unwrap_or_else(|e| unreachable!("the icmp error builds: {e:?}")); + super::routed::tunnelled_from(vni(REMOTE_VNI), &inner) + } + + #[concurrency::model_test] + fn an_icmp_teardown_leaves_another_workers_flow_alone() { + static REPORTED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static SURVIVED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + + let _eal = dpdk::test_support::start_eal(); + + let rt = cfg_select! { + feature = "shuttle" => None::, + _ => Some( + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("build tokio runtime") + ) + }; + let handle = rt.as_ref().map(tokio::runtime::Runtime::handle).cloned(); + + concurrency::stress(move || { + let target: IpAddr = "3.3.3.1".parse().unwrap_or_else(|e| unreachable!("{e}")); + + for (code, tears_down) in [ + (net::icmp4::Icmp4DestUnreachable::Network, true), + ( + net::icmp4::Icmp4DestUnreachable::FragmentationNeeded { + next_hop_mtu: Some(1400.try_into().unwrap_or_else(|_| unreachable!())), + }, + false, + ), + ] { + let overlay = overlay_with_exposes_and_acl(exposes(), None) + .expect("the fixture exposes form an overlay") + .validate() + .expect("the fixture overlay validates"); + let fleet = Fleet::lowering( + &overlay, + Some(topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)])), + Arc::new(FlowTable::default()), + ); + let blueprint = fleet.blueprint(); + let entering = handle.clone(); + + let opening = entering.clone(); + let answering = entering.clone(); + let (doomed, mut spared): ((IpAddr, u16), Conversation) = thread::scope(|scope| { + let doomed = thread::Builder::new() + .name("open-doomed".to_owned()) + .spawn_scoped(scope, move || { + let _guard = opening.as_ref().map(tokio::runtime::Handle::enter); + let _evidence = tracectl::evidence::capture("open-doomed"); + let mut worker = blueprint.worker(); + let request = super::round_trip::udp( + "1.1.0.1".parse().unwrap_or_else(|e| unreachable!("{e}")), + target, + 1000, + 80, + ) + .unwrap_or_else(|| unreachable!("the fixture request builds")); + let out = worker.send(tunnelled(&request)); + let carried = inside(&out).unwrap_or_else(|| { + unreachable!( + "the request was not delivered tunnelled: {:?}", + verdict(&out) + ) + }); + let (Some(public), Some(port)) = + (carried.ip_source(), carried.transport_src_port()) + else { + unreachable!("a delivered request had no public tuple") + }; + (public, port.get()) + }) + .expect("spawn opener"); + + let spared = thread::Builder::new() + .name("open-spared".to_owned()) + .spawn_scoped(scope, move || { + let _guard = answering.as_ref().map(tokio::runtime::Handle::enter); + let _evidence = tracectl::evidence::capture("open-spared"); + let mut worker = blueprint.worker(); + let mut convo = Conversation::new( + super::routed::Path::fixture(), + "1.1.0.2".parse().unwrap_or_else(|e| unreachable!("{e}")), + target, + 2000, + 80, + ); + step(&mut worker, &mut convo); + convo + }) + .expect("spawn opener"); + + ( + doomed.join().expect("opener panicked"), + spared.join().expect("opener panicked"), + ) + }); + + let tearing = entering.clone(); + let keeping = entering.clone(); + let named = format!("{code:?}"); + let spared = thread::scope(|scope| { + let teardown = thread::Builder::new() + .name("teardown".to_owned()) + .spawn_scoped(scope, move || { + let _guard = tearing.as_ref().map(tokio::runtime::Handle::enter); + let _evidence = tracectl::evidence::capture("teardown"); + let mut worker = blueprint.worker(); + let out = worker.send(unreachable(code, doomed, target)); + matches!(verdict(&out), Verdict::Delivered { .. }) + }) + .expect("spawn teardown"); + + let answer = thread::Builder::new() + .name("answer".to_owned()) + .spawn_scoped(scope, move || { + let _guard = keeping.as_ref().map(tokio::runtime::Handle::enter); + let _evidence = tracectl::evidence::capture("answer"); + let mut worker = blueprint.worker(); + step(&mut worker, &mut spared); + spared + }) + .expect("spawn answer"); + + assert!( + teardown.join().expect("teardown panicked"), + "the icmp error never reached the flow it named, so this raced against \ + nothing" + ); + REPORTED.fetch_add(1, Ordering::Relaxed); + answer.join().expect("answer panicked") + }); + + assert!( + spared.checked(), + "a flow was disturbed by an icmp teardown of a different flow on another \ + worker. {}", + spared.describe() + ); + SURVIVED.fetch_add(1, Ordering::Relaxed); + + let mut worker = blueprint.worker(); + let reply = super::round_trip::udp(target, doomed.0, 80, doomed.1) + .unwrap_or_else(|| unreachable!("the reply builds")); + let out = worker.send(super::routed::tunnelled_from(vni(REMOTE_VNI), &reply)); + let delivered = matches!(verdict(&out), Verdict::Delivered { .. }); + assert_eq!( + delivered, + !tears_down, + "an icmp error with code {named} left the flow it named {}: {:?}", + if delivered { "alive" } else { "torn down" }, + verdict(&out) + ); + } + }); + + let (reported, survived) = ( + REPORTED.load(Ordering::Relaxed), + SURVIVED.load(Ordering::Relaxed), + ); + eprintln!("reported={reported} survived={survived}"); + super::assert_covered(reported > 0, "no icmp error ever reached the flow it named"); + } } diff --git a/nat/src/icmp_handler/nf.rs b/nat/src/icmp_handler/nf.rs index 7b7576bfe0..a919d0b796 100644 --- a/nat/src/icmp_handler/nf.rs +++ b/nat/src/icmp_handler/nf.rs @@ -142,8 +142,16 @@ impl IcmpErrorHandler { return; } - let flow_info_locked = flow.locked.read(); - let Some(dst_vpcd) = flow_info_locked.dst_vpcd else { + let (dst_vpcd, masquerading, port_forwarding) = { + let flow_info_locked = flow.locked.read(); + ( + flow_info_locked.dst_vpcd, + flow_info_locked.nat_state.is_some(), + flow_info_locked.port_fw_state.is_some(), + ) + }; + + let Some(dst_vpcd) = dst_vpcd else { warn!("Flow for {rev_flow_key} has no dst VPC discriminant set. This is a bug"); packet.done(DoneReason::InternalFailure); return; @@ -153,10 +161,10 @@ impl IcmpErrorHandler { packet.meta_mut().dst_vpcd = Some(dst_vpcd); // process the packet depending on the flow info - let result = if flow_info_locked.nat_state.is_some() { + let result = if masquerading { debug!("Icmp error is for vpc {dst_vpcd}. Will process with masquerade state"); handle_icmp_error_masquerading(packet, flow.as_ref()) - } else if flow_info_locked.port_fw_state.is_some() { + } else if port_forwarding { debug!("Icmp error is for vpc {dst_vpcd}. Will process with port-forwarding state"); handle_icmp_error_port_forwarding(packet, flow.as_ref()) } else { From 137453bf7c8a2b210686d4d7c9e638af9e82e433 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 22:32:39 -0600 Subject: [PATCH 12/21] test(dataplane): Republish routes while workers forward over them Production's third thread. Every property before this froze the tables before sending a packet, so left-right under `FibTableReader` -- and the per-thread `Rc>` cache behind it -- had never been asked to publish while a reader was mid-lookup. `Fleet` stops owning the `RouterTables` to make it possible: a fleet is shared by reference with every worker, so whoever wants to change a route cannot also be borrowing it through one. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/fuzz.rs | 155 ++++++++++++++++++++----- 1 file changed, 129 insertions(+), 26 deletions(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 028d05b0ea..603d77a1d5 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -46,7 +46,6 @@ pub(crate) struct Fleet { _static_nat: NatTablesWriter, _portfw: PortFwTableWriter, _masquerade: NatAllocatorWriter, - _tables: Option, blueprint: Blueprint, } @@ -74,6 +73,7 @@ pub(crate) struct Worker { } pub(crate) struct Fabric { + _tables: Option, fleet: Fleet, worker: Worker, } @@ -81,7 +81,7 @@ pub(crate) struct Fabric { impl Fleet { pub(crate) fn lowering( overlay: &ValidatedOverlay, - tables: Option, + tables: Option<&RouterTables>, flow_table: Arc, ) -> Self { let flow_filter = FlowFilterContextWriter::new(); @@ -116,7 +116,7 @@ impl Fleet { static_nat: static_nat.get_reader_factory(), portfw: portfw.reader().factory(), masquerade: masquerade.get_reader_factory(), - underlay: tables.as_ref().map(|tables| Underlay { + underlay: tables.map(|tables| Underlay { interfaces: tables.interface_factory(), fibs: tables.fib_factory(), adjacencies: tables.adjacency_factory(), @@ -131,7 +131,6 @@ impl Fleet { _static_nat: static_nat, _portfw: portfw, _masquerade: masquerade, - _tables: tables, blueprint, } } @@ -295,9 +294,13 @@ impl Fabric { tables: Option, flow_table: Arc, ) -> Self { - let fleet = Fleet::lowering(overlay, tables, flow_table); + let fleet = Fleet::lowering(overlay, tables.as_ref(), flow_table); let worker = fleet.blueprint().worker(); - Self { fleet, worker } + Self { + _tables: tables, + fleet, + worker, + } } pub(crate) fn send(&mut self, packet: Packet) -> Packet { @@ -3709,11 +3712,8 @@ mod model { .expect("the fixture exposes form an overlay") .validate() .expect("the fixture overlay validates"); - let fleet = Fleet::lowering( - &overlay, - Some(topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)])), - Arc::new(FlowTable::default()), - ); + let tables = topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)]); + let fleet = Fleet::lowering(&overlay, Some(&tables), Arc::new(FlowTable::default())); let blueprint = fleet.blueprint(); let workers = [("1.1.0.1", 1000u16), ("1.1.0.2", 2000u16)]; @@ -3835,11 +3835,9 @@ mod model { concurrency::stress(move || { let (validated, vnis, vary, schedule) = &*drawn; - let fleet = Fleet::lowering( - validated, - Some(topology(vnis)), - Arc::new(FlowTable::default()), - ); + let tables = topology(vnis); + let fleet = + Fleet::lowering(validated, Some(&tables), Arc::new(FlowTable::default())); let blueprint = fleet.blueprint(); thread::scope(|scope| { @@ -3918,11 +3916,8 @@ mod model { .expect("the fixture exposes form an overlay") .validate() .expect("the fixture overlay validates"); - let fleet = Fleet::lowering( - &overlay, - Some(topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)])), - Arc::new(FlowTable::default()), - ); + let tables = topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)]); + let fleet = Fleet::lowering(&overlay, Some(&tables), Arc::new(FlowTable::default())); let blueprint = fleet.blueprint(); let entering = handle.clone(); @@ -4079,11 +4074,9 @@ mod model { .expect("the fixture exposes form an overlay") .validate() .expect("the fixture overlay validates"); - let fleet = Fleet::lowering( - &overlay, - Some(topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)])), - Arc::new(FlowTable::default()), - ); + let tables = topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)]); + let fleet = + Fleet::lowering(&overlay, Some(&tables), Arc::new(FlowTable::default())); let blueprint = fleet.blueprint(); let entering = handle.clone(); @@ -4208,4 +4201,114 @@ mod model { eprintln!("reported={reported} survived={survived}"); super::assert_covered(reported > 0, "no icmp error ever reached the flow it named"); } + + #[concurrency::model_test] + fn forwarding_survives_a_route_being_republished_underneath_it() { + const FLOWS: u8 = 2; + const CHURN: u8 = 6; + + static COMPLETED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static PUBLISHED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + + let _eal = dpdk::test_support::start_eal(); + + let rt = cfg_select! { + feature = "shuttle" => None::, + _ => Some( + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("build tokio runtime") + ) + }; + let handle = rt.as_ref().map(tokio::runtime::Runtime::handle).cloned(); + + concurrency::stress(move || { + let overlay = overlay_with_exposes_and_acl(exposes(), None) + .expect("the fixture exposes form an overlay") + .validate() + .expect("the fixture overlay validates"); + let mut tables = topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)]); + let fleet = Fleet::lowering(&overlay, Some(&tables), Arc::new(FlowTable::default())); + let blueprint = fleet.blueprint(); + let entering = handle.clone(); + let start = Arc::new(concurrency::sync::Barrier::new(3)); + + thread::scope(|scope| { + let running: Vec<_> = (0..2u8) + .map(|which| { + let entering = entering.clone(); + let start = start.clone(); + thread::Builder::new() + .name(format!("forward-{which}")) + .spawn_scoped(scope, move || { + let _guard = entering.as_ref().map(tokio::runtime::Handle::enter); + let _evidence = + tracectl::evidence::capture(format!("forward-{which}")); + let mut worker = blueprint.worker(); + start.wait(); + let mut done = 0u64; + for nth in 0..FLOWS { + let src = format!("1.1.{which}.{}", nth + 1); + let mut convo = Conversation::new( + super::routed::Path::fixture(), + src.parse().unwrap_or_else(|e| unreachable!("{src}: {e}")), + "3.3.3.1".parse().unwrap_or_else(|e| unreachable!("{e}")), + u16::from(nth) + 1000, + 80, + ); + drive(&mut worker, &mut convo); + assert!( + convo.checked(), + "a conversation did not complete while routes were being \ + republished. {}", + convo.describe() + ); + done += 1; + } + done + }) + .expect("spawn forwarder") + }) + .collect(); + + let peer: IpAddr = PEER_VTEP.parse().unwrap_or_else(|_| unreachable!()); + let landing = + FibGroup::with_entry(FibEntry::with_inst(PktInstruction::Local(uplink()))); + start.wait(); + for nth in 0..CHURN { + let prefix = format!("9.9.{nth}.0"); + tables.route_via( + UNDERLAY_VRF, + Prefix::expect_from(( + prefix + .parse::() + .unwrap_or_else(|e| unreachable!("{e}")), + 24, + )), + nhop(&peer), + &landing, + ); + PUBLISHED.fetch_add(1, Ordering::Relaxed); + } + + for worker in running { + COMPLETED.fetch_add( + worker.join().expect("forwarder panicked"), + Ordering::Relaxed, + ); + } + }); + }); + + let (completed, published) = ( + COMPLETED.load(Ordering::Relaxed), + PUBLISHED.load(Ordering::Relaxed), + ); + eprintln!("completed={completed} published={published}"); + super::assert_covered( + completed > 0 && published > 0, + "either no conversation completed or no route was published, so nothing was raced", + ); + } } From 8d6e86319f2685a9b7597b508ce044ee7adc2425 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 23:11:29 -0600 Subject: [PATCH 13/21] test(dataplane): Move a next hop while workers forward over it The route-churn property states a frame condition, so it can only ever say that publishing did no harm. Aiming the churn at the route under test needs a claim with a lower bound as well, and the fib's in-place next-hop update -- which writes through an `UnsafeCell` every route already points at -- is the part of it whose safety argument had never been exercised. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/fuzz.rs | 209 +++++++++++++++++++++++++ 1 file changed, 209 insertions(+) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 603d77a1d5..c32952fe64 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -4311,4 +4311,213 @@ mod model { "either no conversation completed or no route was published, so nothing was raced", ); } + + #[concurrency::model_test] + fn a_next_hop_that_moves_is_never_seen_half_moved() { + const CHURN: u8 = 3; + const PER_ROUND: u8 = 2; + + static FRESH: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static STALE: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + + type Seen = Result; + + fn waypoint(nth: u8) -> (IpAddr, InterfaceIndex) { + if nth == 0 { + ( + PEER_VTEP.parse().unwrap_or_else(|_| unreachable!()), + uplink(), + ) + } else { + ( + IpAddr::from([10, 0, 0, nth]), + InterfaceIndex::try_new(UPLINK + u32::from(nth)) + .unwrap_or_else(|_| unreachable!()), + ) + } + } + + fn framing(nth: u8) -> Mac { + Mac([0x02, 0, 0, 0, 0x11, nth]) + } + + fn towards(nth: u8) -> FibGroup { + let (remote, oif) = waypoint(nth); + let mut out = FibEntry::with_inst(PktInstruction::Encap(ResolvedEncapsulation::Vxlan( + ResolvedVxlan { + vni: vni(REMOTE_VNI), + remote, + dmac: framing(nth), + }, + ))); + out.add(PktInstruction::Egress(EgressObject::new( + Some(oif), + Some(remote), + ))); + FibGroup::with_entry(out) + } + + let _eal = dpdk::test_support::start_eal(); + + let rt = cfg_select! { + feature = "shuttle" => None::, + _ => Some( + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("build tokio runtime") + ) + }; + let handle = rt.as_ref().map(tokio::runtime::Runtime::handle).cloned(); + + concurrency::stress(move || { + let overlay = overlay_with_exposes_and_acl(exposes(), None) + .expect("the fixture exposes form an overlay") + .validate() + .expect("the fixture overlay validates"); + let mut tables = topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)]); + for nth in 1..=CHURN { + let (_, oif) = waypoint(nth); + tables.interface( + oif, + &format!("uplink-{nth}"), + SourceMac::new(framing(nth)).unwrap_or_else(|_| unreachable!()), + ); + tables.attach(oif, UNDERLAY_VRF); + } + for to in 0..=CHURN { + for over in 0..=CHURN { + tables.adjacency(waypoint(to).0, waypoint(over).1, framing(to)); + } + } + let fleet = Fleet::lowering(&overlay, Some(&tables), Arc::new(FlowTable::default())); + let blueprint = fleet.blueprint(); + let entering = handle.clone(); + let gate = Arc::new(concurrency::sync::Barrier::new(3)); + let mut reports = Vec::new(); + + thread::scope(|scope| { + let running: Vec<_> = (0..2u8) + .map(|which| { + let entering = entering.clone(); + let gate = gate.clone(); + thread::Builder::new() + .name(format!("probe-{which}")) + .spawn_scoped(scope, move || { + let _guard = entering.as_ref().map(tokio::runtime::Handle::enter); + let _evidence = + tracectl::evidence::capture(format!("probe-{which}")); + let mut worker = blueprint.worker(); + + let mut port = u16::from(which) * 1000 + 1000; + let send = |worker: &mut Worker, port: &mut u16| -> Seen { + *port += 1; + let src = format!("1.1.{which}.1"); + let out = worker.send(tunnelled(&build_test_udp_ipv4_packet( + &src, "3.3.3.1", *port, 80, + ))); + let Verdict::Delivered { + oif: Some(oif), + dst: Some(dst), + .. + } = verdict(&out) + else { + return Err(format!( + "it did not leave the gateway: {:?}", + verdict(&out) + )); + }; + (0..=CHURN) + .find(|nth| waypoint(*nth) == (dst, oif)) + .ok_or_else(|| { + format!( + "it left over interface {oif} towards {dst}, which \ + is no published next hop: either the \ + encapsulation and the egress came from different \ + versions, or the group was read while it was \ + being written" + ) + }) + }; + + gate.wait(); + let mut seen = Vec::with_capacity( + usize::from(CHURN) * usize::from(PER_ROUND) + 1, + ); + for _ in 1..=CHURN { + for _ in 0..PER_ROUND { + seen.push(send(&mut worker, &mut port)); + } + gate.wait(); + } + seen.push(send(&mut worker, &mut port)); + seen + }) + .expect("spawn prober") + }) + .collect(); + + let key = nhop(&PEER_VTEP.parse().unwrap_or_else(|_| unreachable!())); + gate.wait(); + for round in 1..=CHURN { + tables.nexthop(REMOTE_VNI, &key, &towards(round)); + gate.wait(); + } + + for prober in running { + reports.push(prober.join().expect("prober panicked")); + } + }); + + for seen in reports { + for (nth, observed) in seen.iter().enumerate() { + let last = nth == seen.len() - 1; + let round = if last { + CHURN + } else { + u8::try_from(nth).unwrap_or_else(|_| unreachable!()) / PER_ROUND + 1 + }; + let version = match observed { + Ok(version) => *version, + Err(why) => panic!( + "a probe sent in round {round}, while the next hop was moving, is not \ + attributable to any version: {why}" + ), + }; + if last { + assert_eq!( + version, CHURN, + "a probe sent after the churn had finished was forwarded by version \ + {version}, not by version {CHURN}, the last one published. Every \ + publish returned before the barrier that released this probe, so a \ + reader still serving an earlier version is serving a next hop that \ + no longer exists" + ); + continue; + } + assert!( + version == round || version + 1 == round, + "a probe sent in round {round} was forwarded by version {version}. \ + Version {} was published before this round opened, so no reader may \ + still be serving anything older, and version {round} is the newest that \ + exists", + round - 1 + ); + if version == round { + FRESH.fetch_add(1, Ordering::Relaxed); + } else { + STALE.fetch_add(1, Ordering::Relaxed); + } + } + } + }); + + let (fresh, stale) = (FRESH.load(Ordering::Relaxed), STALE.load(Ordering::Relaxed)); + eprintln!("fresh={fresh} stale={stale}"); + super::assert_covered( + fresh > 0, + "no probe was ever forwarded by the version published in its own round, so the publish \ + never once landed inside the window it was racing", + ); + } } From ea2752b5f1f6d641c455871496d87308476fe3dd Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 23:18:37 -0600 Subject: [PATCH 14/21] test(dataplane): Enact a configuration while workers carry its traffic The overlay half of config-apply-under-load. `Fleet::reconfigure` re-stores through the writers a running worker already reads, which is what production does and what rebuilding a fleet would not be. Written twice: the first shape overlapped nothing on the plain backend and passed for that reason. See the doc comment: rounds are what make the passing case mean something, and it records the two ways of provoking a failure that were tried and discarded, each of which looked conclusive and was not. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/fuzz.rs | 150 +++++++++++++++++++++++-- 1 file changed, 140 insertions(+), 10 deletions(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index c32952fe64..5f6a46d647 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -34,6 +34,7 @@ use routing::testing::RouterTables; use routing::testing::{FibGroup, FwAction, NhopKey, RouteOrigin}; use routing::{AtableReaderFactory, FibTableReaderFactory, IfTableReaderFactory}; use routing::{EgressObject, FibEntry, PktInstruction, ResolvedEncapsulation, ResolvedVxlan, Vtep}; +use std::cell::RefCell; use std::net::IpAddr; use super::egress::Egress; @@ -41,11 +42,11 @@ use super::ingress::Ingress; use super::ipforward::IpForwarder; pub(crate) struct Fleet { - _flow_filter: FlowFilterContextWriter, - _acl: AclFilterContextWriter, - _static_nat: NatTablesWriter, - _portfw: PortFwTableWriter, - _masquerade: NatAllocatorWriter, + flow_filter: FlowFilterContextWriter, + acl: AclFilterContextWriter, + static_nat: RefCell, + portfw: RefCell, + masquerade: RefCell, blueprint: Blueprint, } @@ -126,15 +127,37 @@ impl Fleet { }; Self { - _flow_filter: flow_filter, - _acl: acl, - _static_nat: static_nat, - _portfw: portfw, - _masquerade: masquerade, + flow_filter, + acl, + static_nat: RefCell::new(static_nat), + portfw: RefCell::new(portfw), + masquerade: RefCell::new(masquerade), blueprint, } } + pub(crate) fn reconfigure(&self, overlay: &ValidatedOverlay) { + self.flow_filter.store( + FlowFilterContext::try_from(overlay).expect("a validated overlay lowers to tables"), + ); + self.acl.store( + AclFilterContext::try_from(overlay).expect("a validated overlay lowers to acls"), + ); + self.static_nat.borrow_mut().update_nat_tables( + build_nat_configuration(overlay.vpc_table()) + .expect("a validated overlay lowers to nat"), + ); + self.portfw + .borrow_mut() + .update_from_vpc_table(overlay.vpc_table()) + .expect("a validated overlay lowers to port forwarding"); + self.masquerade.borrow_mut().update_nat_allocator( + MasqueradeConfig::new(overlay.vpc_table()).set_randomize(false), + 1, + &self.blueprint.flow_table, + ); + } + pub(crate) fn blueprint(&self) -> &Blueprint { &self.blueprint } @@ -4520,4 +4543,111 @@ mod model { never once landed inside the window it was racing", ); } + + #[concurrency::model_test] + fn re_enacting_a_configuration_under_load_disturbs_nothing() { + const FLOWS: u8 = 2; + const APPLIES: u8 = 4; + + static COMPLETED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static ENACTED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + + let _eal = dpdk::test_support::start_eal(); + + let rt = cfg_select! { + feature = "shuttle" => None::, + _ => Some( + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("build tokio runtime") + ) + }; + let handle = rt.as_ref().map(tokio::runtime::Runtime::handle).cloned(); + + concurrency::stress(move || { + let overlay = overlay_with_exposes_and_acl(exposes(), None) + .expect("the fixture exposes form an overlay") + .validate() + .expect("the fixture overlay validates"); + let tables = topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)]); + let fleet = Fleet::lowering(&overlay, Some(&tables), Arc::new(FlowTable::default())); + let blueprint = fleet.blueprint(); + let entering = handle.clone(); + let gate = Arc::new(concurrency::sync::Barrier::new(3)); + let mut reports = Vec::new(); + + thread::scope(|scope| { + let running: Vec<_> = (0..2u8) + .map(|which| { + let entering = entering.clone(); + let gate = gate.clone(); + thread::Builder::new() + .name(format!("tenant-{which}")) + .spawn_scoped(scope, move || { + let _guard = entering.as_ref().map(tokio::runtime::Handle::enter); + let _evidence = + tracectl::evidence::capture(format!("tenant-{which}")); + let mut worker = blueprint.worker(); + gate.wait(); + let mut seen = Vec::new(); + for round in 1..=APPLIES { + for nth in 0..FLOWS { + let src = format!("1.1.{which}.{}", nth + 1); + let mut convo = Conversation::new( + super::routed::Path::fixture(), + src.parse() + .unwrap_or_else(|e| unreachable!("{src}: {e}")), + "3.3.3.1" + .parse() + .unwrap_or_else(|e| unreachable!("{e}")), + u16::from(round) * 100 + u16::from(nth) + 1000, + 80, + ); + drive(&mut worker, &mut convo); + seen.push((round, convo.checked(), convo.describe())); + } + gate.wait(); + } + seen + }) + .expect("spawn tenant") + }) + .collect(); + + gate.wait(); + for _ in 0..APPLIES { + fleet.reconfigure(&overlay); + ENACTED.fetch_add(1, Ordering::Relaxed); + gate.wait(); + } + + for worker in running { + reports.push(worker.join().expect("tenant panicked")); + } + }); + + for seen in reports { + for (round, checked, described) in seen { + assert!( + checked, + "a conversation in round {round} did not survive the configuration it was \ + already running being enacted again. Every enactment before this round \ + had returned, and the one racing it changes nothing. {described}" + ); + COMPLETED.fetch_add(1, Ordering::Relaxed); + } + } + }); + + let (completed, enacted) = ( + COMPLETED.load(Ordering::Relaxed), + ENACTED.load(Ordering::Relaxed), + ); + eprintln!("completed={completed} enacted={enacted}"); + super::assert_covered( + completed > 0 && enacted > 0, + "either no conversation completed or no configuration was enacted, so nothing was raced", + ); + } } From 6df4f905b05317acdd35f85e18b66758a13436c6 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 23:25:43 -0600 Subject: [PATCH 15/21] test(config): Measure what the operation algebra cannot express The design note proposed checking completeness against real configurations. That answers the wrong question -- the algebra's address plan is a function of its handles, so no real configuration is expressible and the report is "0% reachable". Per degree of freedom instead, with the fuzzer supplying the evidence and the survey's exhaustive destructuring stopping the build if the schema grows a field nobody classified. Reports thirteen of twenty-seven degrees of freedom fixed. Nothing acted on yet; the note names the four worth doing first. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/completeness.rs | 410 ++++++++++++++++++++ config/src/external/overlay/mod.rs | 2 + 2 files changed, 412 insertions(+) create mode 100644 config/src/external/overlay/completeness.rs diff --git a/config/src/external/overlay/completeness.rs b/config/src/external/overlay/completeness.rs new file mode 100644 index 0000000000..7b78fb5d4e --- /dev/null +++ b/config/src/external/overlay/completeness.rs @@ -0,0 +1,410 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +use std::cell::RefCell; +use std::collections::{BTreeMap, BTreeSet}; + +use lpm::prefix::with_ports::{L4Protocol, PrefixPortsSet}; + +use super::Overlay; +use super::algebra::Sequence; +use super::vpc::Vpc; +use super::vpcpeering::{ + VpcExpose, VpcExposeMasquerade, VpcExposeNat, VpcExposeNatConfig, VpcExposePortForwarding, + VpcExposeStaticNat, VpcManifest, VpcPeering, +}; + +#[derive(Debug)] +enum Reach { + Spans(&'static [&'static str]), + Determined(&'static str), + Derived(&'static str), + Fixed(&'static str), +} + +const REACH: &[(&str, Reach)] = &[ + ( + "Overlay.vpc_table", + Reach::Determined("one vpc per `AddVpc`, in handle order"), + ), + ( + "Overlay.peering_table", + Reach::Determined("one peering per `AddPeering`, in handle order"), + ), + ("Vpc.name", Reach::Determined("`VpcHandle::name`")), + ("Vpc.id", Reach::Determined("`VpcHandle::id`")), + ("Vpc.vni", Reach::Determined("`VpcHandle::vni`")), + ( + "Vpc.interfaces", + Reach::Fixed( + "empty. No operation attaches an interface to a vpc, so no generated configuration \ + has one. Reaching the interface-bearing paths at all needs a new operation.", + ), + ), + ( + "Vpc.peerings", + Reach::Derived( + "collected from the peering table by `Overlay::validate`, not by the algebra", + ), + ), + ( + "VpcPeering.name", + Reach::Determined("`PeeringHandle::name`"), + ), + ( + "VpcPeering.left", + Reach::Determined("the peering's left handle"), + ), + ( + "VpcPeering.right", + Reach::Determined("the peering's right handle"), + ), + ( + "VpcPeering.gwgroup", + Reach::Fixed( + "the default group. `with_default_group` is the only constructor the algebra calls, \ + so nothing generated ever splits vpcs across gateway groups.", + ), + ), + ( + "VpcPeering.acl", + Reach::Fixed( + "absent. Peering-scoped ACLs are not in the vocabulary, so no generated configuration \ + carries one -- and an ACL is precisely a thing that changes a verdict, which is what \ + every property here asserts over.", + ), + ), + ( + "VpcManifest.name", + Reach::Determined("the side's vpc handle"), + ), + ( + "VpcManifest.exposes", + Reach::Determined("one per `AddExpose`, in slot order"), + ), + ( + "VpcExpose.default", + Reach::Fixed("false. `VpcExpose::empty` never sets it and no operation does either."), + ), + ( + "VpcExpose.ips", + Reach::Determined("one prefix, from the expose's peering, side and slot"), + ), + ( + "VpcExpose.ips.ports", + Reach::Fixed( + "unset. The algebra exposes whole prefixes, so a port-restricted expose is \ + unreachable, and with it every question about how ports partition an address.", + ), + ), + ( + "VpcExpose.nots", + Reach::Fixed( + "empty -- the survey renders it as no prefixes at all. An expose that carves holes out of its own range is unreachable, which is a \ + real hole rather than a canonicalisation: an exclusion is what makes a prefix set \ + non-contiguous, and non-contiguous is where a matcher goes wrong.", + ), + ), + ("VpcExpose.nat", Reach::Spans(&["absent", "present"])), + ( + "VpcExposeNat.as_range", + Reach::Determined("one prefix in the masquerade pool, from peering, side and slot"), + ), + ( + "VpcExposeNat.as_range.ports", + Reach::Fixed("unset, for the same reason as `VpcExpose.ips.ports`."), + ), + ( + "VpcExposeNat.not_as", + Reach::Fixed("empty, for the same reason as `VpcExpose.nots`."), + ), + ( + "VpcExposeNat.config", + Reach::Fixed( + "masquerade. `Flavour` has two members and only one of them makes a nat, so static \ + nat and port forwarding are both unreachable -- which the design note already names \ + as missing vocabulary.", + ), + ), + ( + "VpcExposeNat.proto", + Reach::Fixed("`Any`. No operation narrows an expose to tcp or udp."), + ), + ( + "VpcExposeMasquerade.idle_timeout", + Reach::Fixed( + "absent. `make_masquerade(None)` is the only call, so the timeout paths -- and every \ + question about a flow ageing out under a configuration that set one -- are never \ + entered.", + ), + ), + ( + "VpcExposeStaticNat", + Reach::Fixed("never constructed; see `VpcExposeNat.config`."), + ), + ( + "VpcExposePortForwarding.idle_timeout", + Reach::Fixed("never constructed; see `VpcExposeNat.config`."), + ), +]; + +#[derive(Default)] +struct Observed(BTreeMap<&'static str, BTreeSet>); + +impl Observed { + fn note(&mut self, field: &'static str, value: impl Into) { + self.0.entry(field).or_default().insert(value.into()); + } + + fn count(&mut self, field: &'static str, n: usize) { + self.note(field, n.to_string()); + } + + fn prefixes(&mut self, field: &'static str, set: &PrefixPortsSet) { + let listed: Vec = set + .into_iter() + .map(|entry| entry.prefix().to_string()) + .collect(); + self.note(field, listed.join(", ")); + } + + fn ports(&mut self, field: &'static str, set: &PrefixPortsSet) { + self.note( + field, + if set.into_iter().any(|entry| entry.ports().is_some()) { + "set" + } else { + "unset" + }, + ); + } +} + +fn survey(overlay: &Overlay, seen: &mut Observed) { + let Overlay { + vpc_table, + peering_table, + } = overlay; + seen.count("Overlay.vpc_table", vpc_table.len()); + seen.count("Overlay.peering_table", peering_table.len()); + + for vpc in vpc_table.values() { + let Vpc { + name, + id, + vni, + interfaces, + peerings, + } = vpc; + seen.note("Vpc.name", name.clone()); + seen.note("Vpc.id", id.to_string()); + seen.note("Vpc.vni", vni.as_u32().to_string()); + seen.count("Vpc.interfaces", interfaces.values().count()); + seen.count("Vpc.peerings", peerings.len()); + } + + for peering in peering_table.values() { + let VpcPeering { + name, + left, + right, + gwgroup, + acl, + } = peering; + seen.note("VpcPeering.name", name.clone()); + seen.note("VpcPeering.gwgroup", gwgroup.clone()); + seen.note( + "VpcPeering.acl", + if acl.is_some() { "present" } else { "absent" }, + ); + for (side, manifest) in [("VpcPeering.left", left), ("VpcPeering.right", right)] { + seen.note(side, manifest.name.clone()); + survey_manifest(manifest, seen); + } + } +} + +fn survey_manifest(manifest: &VpcManifest, seen: &mut Observed) { + let VpcManifest { name, exposes } = manifest; + seen.note("VpcManifest.name", name.clone()); + seen.count("VpcManifest.exposes", exposes.len()); + for expose in exposes { + let VpcExpose { + default, + ips, + nots, + nat, + } = expose; + seen.note("VpcExpose.default", default.to_string()); + seen.prefixes("VpcExpose.ips", ips); + seen.ports("VpcExpose.ips.ports", ips); + seen.prefixes("VpcExpose.nots", nots); + seen.note( + "VpcExpose.nat", + if nat.is_some() { "present" } else { "absent" }, + ); + if let Some(nat) = nat { + survey_nat(nat, seen); + } + } +} + +fn survey_nat(nat: &VpcExposeNat, seen: &mut Observed) { + let VpcExposeNat { + as_range, + not_as, + config, + proto, + } = nat; + seen.prefixes("VpcExposeNat.as_range", as_range); + seen.ports("VpcExposeNat.as_range.ports", as_range); + seen.prefixes("VpcExposeNat.not_as", not_as); + seen.note( + "VpcExposeNat.proto", + match proto { + L4Protocol::Tcp => "tcp", + L4Protocol::Udp => "udp", + L4Protocol::Any => "any", + }, + ); + match config { + VpcExposeNatConfig::Masquerade(VpcExposeMasquerade { idle_timeout }) => { + seen.note("VpcExposeNat.config", "masquerade"); + seen.note( + "VpcExposeMasquerade.idle_timeout", + if idle_timeout.is_some() { + "present" + } else { + "absent" + }, + ); + } + VpcExposeNatConfig::Static(VpcExposeStaticNat {}) => { + seen.note("VpcExposeNat.config", "static"); + seen.note("VpcExposeStaticNat", "constructed"); + } + VpcExposeNatConfig::PortForwarding(VpcExposePortForwarding { idle_timeout }) => { + seen.note("VpcExposeNat.config", "port-forwarding"); + seen.note( + "VpcExposePortForwarding.idle_timeout", + if idle_timeout.is_some() { + "present" + } else { + "absent" + }, + ); + } + } +} + +const CASES: usize = 512; + +fn survey_drawn(seen: &RefCell) { + let seen = std::panic::AssertUnwindSafe(seen); + bolero::check!() + .with_generator(Sequence::default()) + .with_iterations(CASES) + .for_each(|ops| { + let overlay = Sequence::fold(ops) + .overlay() + .unwrap_or_else(|e| panic!("{ops:?} does not assemble: {e}")); + survey(&overlay, &mut seen.borrow_mut()); + }); +} + +fn census() -> Observed { + let seen = RefCell::new(Observed::default()); + survey_drawn(&seen); + seen.into_inner() +} + +#[test] +fn every_surveyed_field_is_classified() { + let seen = census(); + let surveyed: BTreeSet<&str> = seen.0.keys().copied().collect(); + let classified: BTreeSet<&str> = REACH.iter().map(|(field, _)| *field).collect(); + + let unclassified: Vec<&&str> = surveyed.difference(&classified).collect(); + assert!( + unclassified.is_empty(), + "the survey records fields with no verdict in `REACH`: {unclassified:?}. A field added to \ + the overlay schema is not reachable by the algebra until an operation produces it, so say \ + which it is -- `Fixed` is a perfectly good answer and is what most of the table already \ + says." + ); + + let unreachable_by_construction: BTreeSet<&str> = REACH + .iter() + .filter( + |(_, reach)| matches!(reach, Reach::Fixed(why) if why.contains("never constructed")), + ) + .map(|(field, _)| *field) + .collect(); + let missing: Vec<&&str> = classified + .difference(&surveyed) + .filter(|field| !unreachable_by_construction.contains(**field)) + .collect(); + assert!( + missing.is_empty(), + "`REACH` gives a verdict for fields the survey never records: {missing:?}. Either the \ + field was removed from the schema, or the survey stopped visiting it -- and a survey that \ + has stopped visiting a field is a ratchet that has come loose." + ); +} + +#[test] +fn the_algebra_reaches_what_it_is_recorded_to_reach() { + let seen = census(); + + for (field, reach) in REACH { + let Some(values) = seen.0.get(field) else { + continue; + }; + let rendered: Vec<&str> = values.iter().map(String::as_str).collect(); + match reach { + Reach::Spans(expected) => assert_eq!( + rendered, *expected, + "`{field}` is recorded as spanning {expected:?} and {CASES} drawn configurations \ + show {rendered:?}" + ), + Reach::Determined(by) => assert!( + values.len() > 1, + "`{field}` is recorded as determined by {by}, which varies, and yet {CASES} drawn \ + configurations all show {rendered:?}. Either it is fixed after all, or the \ + generator has stopped varying what determines it" + ), + Reach::Derived(_) => {} + Reach::Fixed(why) => assert_eq!( + values.len(), + 1, + "`{field}` is recorded as fixed -- {why} -- and yet {CASES} drawn configurations \ + show {rendered:?}. If the vocabulary grew, say so here: this table is what tells \ + a reader of a green run which configurations it did not cover" + ), + } + } +} + +#[test] +fn report_what_the_algebra_reaches() { + let counted = |wanted: fn(&Reach) -> bool| -> usize { + REACH.iter().filter(|(_, reach)| wanted(reach)).count() + }; + eprintln!( + "of {} degrees of freedom in the overlay schema, the algebra spans {}, determines {}, \ + leaves {} to be derived, and fixes {}:", + REACH.len(), + counted(|reach| matches!(reach, Reach::Spans(_))), + counted(|reach| matches!(reach, Reach::Determined(_))), + counted(|reach| matches!(reach, Reach::Derived(_))), + counted(|reach| matches!(reach, Reach::Fixed(_))), + ); + for (field, reach) in REACH { + match reach { + Reach::Spans(values) => eprintln!(" spans {field}: {values:?}"), + Reach::Determined(by) => eprintln!(" determined {field}: by {by}"), + Reach::Derived(how) => eprintln!(" derived {field}: {how}"), + Reach::Fixed(why) => eprintln!(" FIXED {field}: {why}"), + } + } +} diff --git a/config/src/external/overlay/mod.rs b/config/src/external/overlay/mod.rs index fdf8d85186..85e0039a6e 100644 --- a/config/src/external/overlay/mod.rs +++ b/config/src/external/overlay/mod.rs @@ -6,6 +6,8 @@ pub mod acl; #[cfg(any(test, feature = "bolero"))] pub mod algebra; +#[cfg(test)] +mod completeness; pub mod tests; pub mod validation_tests; pub mod vpc; From 221351e3f2651e65d9de9366544b81d3806fdd90 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sun, 23 Aug 2026 00:28:00 -0600 Subject: [PATCH 16/21] test(dataplane): Change a configuration under the traffic it does not touch The frame condition over a configuration that actually changes, with the frame taken from the algebra rather than guessed at. `X => A.X` from one draw split at its last operation. Round 1 races the change and is counted, not asserted: it fails about one run in twenty, and the doc comment records what was measured and what is still unknown. Rounds after it race a re-enactment and carry the full claim. Two harness faults found on the way and fixed here, both of which made earlier properties in this module prove less than they appeared to: a worker panicking between barriers hung the run instead of failing it, and the genid was pinned at 1 across enactments, which told the nat allocator no configuration ever changed and manufactured two symptoms that are not the dataplane's. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/algebra.rs | 40 ++++ dataplane/src/packet_processor/fuzz.rs | 308 +++++++++++++++++++++++-- 2 files changed, 328 insertions(+), 20 deletions(-) diff --git a/config/src/external/overlay/algebra.rs b/config/src/external/overlay/algebra.rs index 9516fe5e7d..5fcf22bc4a 100644 --- a/config/src/external/overlay/algebra.rs +++ b/config/src/external/overlay/algebra.rs @@ -291,6 +291,16 @@ impl Footprint { pub fn is_empty(&self) -> bool { self.vpcs.is_empty() && self.peerings.is_empty() } + + #[must_use] + pub fn touches_vpc_named(&self, name: &str) -> bool { + self.vpcs.iter().any(|vpc| vpc.name() == name) + } + + #[must_use] + pub fn touches_peering_named(&self, name: &str) -> bool { + self.peerings.iter().any(|peering| peering.name() == name) + } } #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -970,6 +980,36 @@ mod tests { use bolero::check; use concurrency::sync::atomic::{AtomicUsize, Ordering::Relaxed}; + #[test] + fn writing_a_vpc_does_not_imply_writing_its_peerings() { + static SEEN: AtomicUsize = AtomicUsize::new(0); + + check!() + .with_generator(Sequence::default()) + .for_each(|ops: &Vec| { + let mut draft = Draft::new(); + for op in ops { + let footprint = op.writes(&draft); + for vpc in &footprint.vpcs { + for (handle, spec) in draft.peerings() { + if spec.touches(*vpc) && !footprint.peerings.contains(&handle) { + SEEN.fetch_add(1, Relaxed); + } + } + } + op.apply(&mut draft).expect("a drawn operation applies"); + } + }); + + assert!( + SEEN.load(Relaxed) > 0, + "no drawn operation ever wrote a vpc while leaving one of its peerings unwritten. \ + Either the vocabulary changed and a peering-only frame filter is now sound -- in \ + which case say so where the filter is written -- or the generator stopped drawing \ + `AddPeering` against a vpc that already had one" + ); + } + static DRAWN: [AtomicUsize; 7] = [ AtomicUsize::new(0), AtomicUsize::new(0), diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 5f6a46d647..c6aadfc981 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -8,6 +8,7 @@ use acl_filter::{ AclFilter, AclFilterContext, AclFilterContextReaderFactory, AclFilterContextWriter, }; use concurrency::sync::{Arc, Mutex}; +use config::external::GenId; use config::external::overlay::acl::Acl; use config::external::overlay::vpcpeering::VpcExpose; use config::external::overlay::vpcpeering::contract::{ @@ -34,9 +35,11 @@ use routing::testing::RouterTables; use routing::testing::{FibGroup, FwAction, NhopKey, RouteOrigin}; use routing::{AtableReaderFactory, FibTableReaderFactory, IfTableReaderFactory}; use routing::{EgressObject, FibEntry, PktInstruction, ResolvedEncapsulation, ResolvedVxlan, Vtep}; -use std::cell::RefCell; +use std::cell::{Cell, RefCell}; use std::net::IpAddr; +const FIRST_GENID: GenId = 1; + use super::egress::Egress; use super::ingress::Ingress; use super::ipforward::IpForwarder; @@ -47,6 +50,7 @@ pub(crate) struct Fleet { static_nat: RefCell, portfw: RefCell, masquerade: RefCell, + genid: Cell, blueprint: Blueprint, } @@ -107,7 +111,7 @@ impl Fleet { let mut masquerade = NatAllocatorWriter::new(); masquerade.update_nat_allocator( MasqueradeConfig::new(overlay.vpc_table()).set_randomize(false), - 1, + FIRST_GENID, &flow_table, ); @@ -132,6 +136,7 @@ impl Fleet { static_nat: RefCell::new(static_nat), portfw: RefCell::new(portfw), masquerade: RefCell::new(masquerade), + genid: Cell::new(FIRST_GENID), blueprint, } } @@ -151,9 +156,10 @@ impl Fleet { .borrow_mut() .update_from_vpc_table(overlay.vpc_table()) .expect("a validated overlay lowers to port forwarding"); + self.genid.set(self.genid.get() + 1); self.masquerade.borrow_mut().update_nat_allocator( MasqueradeConfig::new(overlay.vpc_table()).set_randomize(false), - 1, + self.genid.get(), &self.blueprint.flow_table, ); } @@ -684,10 +690,32 @@ pub(crate) mod derive { } pub(crate) fn loads_for(overlay: &ValidatedOverlay, vary: &[Vary]) -> Vec> { + loads_where(overlay, vary, &|_| true) + } + + #[derive(Debug, Clone, Copy)] + pub(crate) struct Named<'a> { + pub(crate) local: &'a str, + pub(crate) remote: &'a str, + pub(crate) peering: &'a str, + } + + pub(crate) fn loads_where( + overlay: &ValidatedOverlay, + vary: &[Vary], + keep: &dyn Fn(Named<'_>) -> bool, + ) -> Vec> { let mut loads: Vec> = Vec::new(); let mut nth = 0usize; for vpc in overlay.vpc_table().values() { for peering in vpc.peerings() { + if !keep(Named { + local: vpc.name(), + remote: peering.remote().name(), + peering: peering.name(), + }) { + continue; + } let path = super::routed::Path::new(vpc.vni(), peering.remote_vni()); for expose in peering.local().valexp() { let Some(v) = vary.get(nth % vary.len().max(1)).copied() else { @@ -3605,7 +3633,7 @@ mod model { use concurrency::thread; #[cfg_attr(not(feature = "shuttle"), allow(unused_imports))] use concurrency::thread::BuilderExt; - use config::external::overlay::algebra::Sequence; + use config::external::overlay::algebra::{Footprint, Sequence}; use net::packet::test_utils::build_test_udp_ipv4_packet; use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; use std::sync::{LazyLock, OnceLock}; @@ -3903,6 +3931,20 @@ mod model { ); } + fn without_unwinding(body: impl FnOnce() -> T) -> Result { + cfg_select! { + feature = "shuttle" => Ok(body()), + feature = "loom" => Ok(body()), + _ => std::panic::catch_unwind(std::panic::AssertUnwindSafe(body)).map_err(|payload| { + payload + .downcast_ref::<&str>() + .map(|message| (*message).to_owned()) + .or_else(|| payload.downcast_ref::().cloned()) + .unwrap_or_else(|| "a panic carrying no message".to_owned()) + }), + } + } + fn step(worker: &mut Worker, load: &mut dyn Load) { let Some(packet) = load.next() else { panic!( @@ -4463,17 +4505,23 @@ mod model { }) }; + let probe = |worker: &mut Worker, port: &mut u16| -> Seen { + without_unwinding(|| send(worker, port)).unwrap_or_else(|why| { + Err(format!("sending it panicked: {why}")) + }) + }; + gate.wait(); let mut seen = Vec::with_capacity( usize::from(CHURN) * usize::from(PER_ROUND) + 1, ); for _ in 1..=CHURN { for _ in 0..PER_ROUND { - seen.push(send(&mut worker, &mut port)); + seen.push(probe(&mut worker, &mut port)); } gate.wait(); } - seen.push(send(&mut worker, &mut port)); + seen.push(probe(&mut worker, &mut port)); seen }) .expect("spawn prober") @@ -4593,19 +4641,25 @@ mod model { let mut seen = Vec::new(); for round in 1..=APPLIES { for nth in 0..FLOWS { - let src = format!("1.1.{which}.{}", nth + 1); - let mut convo = Conversation::new( - super::routed::Path::fixture(), - src.parse() - .unwrap_or_else(|e| unreachable!("{src}: {e}")), - "3.3.3.1" - .parse() - .unwrap_or_else(|e| unreachable!("{e}")), - u16::from(round) * 100 + u16::from(nth) + 1000, - 80, - ); - drive(&mut worker, &mut convo); - seen.push((round, convo.checked(), convo.describe())); + seen.push(( + round, + without_unwinding(|| { + let src = format!("1.1.{which}.{}", nth + 1); + let mut convo = Conversation::new( + super::routed::Path::fixture(), + src.parse().unwrap_or_else(|e| { + unreachable!("{src}: {e}") + }), + "3.3.3.1" + .parse() + .unwrap_or_else(|e| unreachable!("{e}")), + u16::from(round) * 100 + u16::from(nth) + 1000, + 80, + ); + drive(&mut worker, &mut convo); + (convo.checked(), convo.describe()) + }), + )); } gate.wait(); } @@ -4628,7 +4682,10 @@ mod model { }); for seen in reports { - for (round, checked, described) in seen { + for (round, ran) in seen { + let (checked, described) = ran.unwrap_or_else(|why| { + panic!("a conversation in round {round} panicked mid-enactment: {why}") + }); assert!( checked, "a conversation in round {round} did not survive the configuration it was \ @@ -4650,4 +4707,215 @@ mod model { "either no conversation completed or no configuration was enacted, so nothing was raced", ); } + + #[concurrency::model_test] + fn a_configuration_change_leaves_traffic_outside_its_footprint_alone() { + const CASES: usize = 64; + const ROUNDS: u8 = 3; + + static FRAMED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static BARREN: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static ENGULFED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static FRAMED_OUT: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static DISTURBED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static UNDISTURBED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + + fn outside(footprint: &Footprint) -> impl Fn(derive::Named<'_>) -> bool + '_ { + move |named| { + !footprint.touches_peering_named(named.peering) + && !footprint.touches_vpc_named(named.local) + && !footprint.touches_vpc_named(named.remote) + } + } + + let _eal = dpdk::test_support::start_eal(); + + let rt = cfg_select! { + feature = "shuttle" => None::, + _ => Some( + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("build tokio runtime") + ) + }; + let handle = rt.as_ref().map(tokio::runtime::Runtime::handle).cloned(); + + bolero::check!() + .with_max_len(MAX_INPUT_LEN) + .with_generator(generated::Generated) + .with_iterations(CASES) + .for_each(|(ops, vary, _schedule)| { + let Some((change, built)) = ops.split_last() else { + BARREN.fetch_add(1, Ordering::Relaxed); + return; + }; + let before = Sequence::fold(built); + let footprint = change.writes(&before); + + let assemble = |draft: &config::external::overlay::algebra::Draft| { + draft + .overlay() + .unwrap_or_else(|e| panic!("{ops:?} does not assemble: {e}")) + .validate() + .unwrap_or_else(|e| panic!("{ops:?} does not validate: {e}")) + }; + let running = assemble(&before); + let enacted = assemble(&Sequence::fold(ops)); + + let framed = derive::loads_where(&running, vary, &outside(&footprint)).len(); + let total = derive::loads_for(&running, vary).len(); + FRAMED_OUT.fetch_add( + u64::try_from(total - framed).unwrap_or_else(|_| unreachable!()), + Ordering::Relaxed, + ); + if framed == 0 { + if total == 0 { &BARREN } else { &ENGULFED }.fetch_add(1, Ordering::Relaxed); + return; + } + FRAMED.fetch_add(1, Ordering::Relaxed); + + let vnis: Vec = enacted + .vpc_table() + .values() + .map(config::external::overlay::vpc::ValidatedVpc::vni) + .collect(); + if vnis.is_empty() { + BARREN.fetch_add(1, Ordering::Relaxed); + return; + } + + let drawn = + std::sync::Arc::new((running, enacted, vnis, vary.clone(), footprint, *change)); + let entering = handle.clone(); + + concurrency::stress(move || { + let (running, enacted, vnis, vary, footprint, change) = &*drawn; + let tables = topology(vnis); + let fleet = + Fleet::lowering(running, Some(&tables), Arc::new(FlowTable::default())); + let blueprint = fleet.blueprint(); + let gate = Arc::new(concurrency::sync::Barrier::new(3)); + let mut reports = Vec::new(); + + thread::scope(|scope| { + let handles: Vec<_> = (0..2usize) + .map(|which| { + let entering = entering.clone(); + let gate = gate.clone(); + thread::Builder::new() + .name(format!("framed-{which}")) + .spawn_scoped(scope, move || { + let _guard = + entering.as_ref().map(tokio::runtime::Handle::enter); + let _evidence = + tracectl::evidence::capture(format!("framed-{which}")); + let mut worker = blueprint.worker(); + let outside = outside(footprint); + gate.wait(); + let mut seen = Vec::new(); + for round in 1..=ROUNDS { + let varied: Vec = vary + .iter() + .map(|v| derive::Vary { + sport: v + .sport + .wrapping_add(u16::from(round) * 997) + .wrapping_add( + u16::try_from(which) + .unwrap_or_else(|_| unreachable!()) + * 401, + ) + .max(1), + ..*v + }) + .collect(); + let mine = + derive::loads_where(running, &varied, &outside); + seen.push(( + round, + without_unwinding(|| { + let mut ran = Vec::new(); + for mut load in mine { + for _ in 0..8 { + let Some(packet) = load.next() else { + break; + }; + let out = worker.send(packet); + load.observe(&out); + } + ran.push((load.checked(), load.describe())); + } + ran + }), + )); + gate.wait(); + } + seen + }) + .expect("spawn framed worker") + }) + .collect(); + + gate.wait(); + for _ in 0..ROUNDS { + fleet.reconfigure(enacted); + gate.wait(); + } + + for worker in handles { + reports.push(worker.join().expect("framed worker panicked")); + } + }); + + for seen in reports { + for (round, ran) in seen { + let ran = ran.unwrap_or_else(|why| { + panic!( + "carrying traffic in round {round} panicked while {change:?} \ + was being enacted: {why}" + ) + }); + for (checked, described) in ran { + if round == 1 { + if checked { &UNDISTURBED } else { &DISTURBED } + .fetch_add(1, Ordering::Relaxed); + continue; + } + assert!( + checked, + "traffic outside the footprint of a configuration change did \ + not survive it, in round {round}. The change was \ + {change:?}, whose write set this load is outside of, so the \ + configuration it ran against and the one enacted agree about \ + it entirely -- and the enactment that carried the difference \ + returned before this round opened. {described}" + ); + } + } + } + }); + }); + + let (framed, barren, engulfed, framed_out, disturbed, undisturbed) = ( + FRAMED.load(Ordering::Relaxed), + BARREN.load(Ordering::Relaxed), + ENGULFED.load(Ordering::Relaxed), + FRAMED_OUT.load(Ordering::Relaxed), + DISTURBED.load(Ordering::Relaxed), + UNDISTURBED.load(Ordering::Relaxed), + ); + eprintln!("framed={framed} barren={barren} engulfed={engulfed} framed_out={framed_out}"); + eprintln!("racing the change: disturbed={disturbed} undisturbed={undisturbed}"); + super::assert_covered( + framed > 0, + "no draw left two loads outside the footprint of its last operation, so no \ + configuration change was ever carried under traffic", + ); + super::assert_covered( + framed_out > 0, + "no load was ever filtered out for being inside a footprint, so the frame was always \ + the whole configuration and this property is the re-enactment one with extra steps", + ); + } } From d4603d56bb2c28ad92a80978ab7a1b2545429076 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sun, 23 Aug 2026 11:07:21 -0600 Subject: [PATCH 17/21] test(dataplane): Model both of config-apply's generation mechanisms `Fleet` published neither: the nat allocator was told generation one on every enactment, and the stages were never told at all, because `Blueprint::worker` built its pipeline without `set_data`. The second also meant the flow revalidation path -- the code that exists for a configuration changing under live flows -- was unreachable from every property in this module. Also separates the two workers' five-tuples by disjoint halves of the port space rather than by an offset, which two drawn ports could cancel; `enact` performs config-apply one step at a time, for attributing a disturbance to one; and the ordering now follows `mgmt`, where the generation is published last on purpose. Found by `just fuzz` on this target. The random engine `cargo test` runs cannot get here: it was the coverage-guided corpus that produced the crashing inputs these were diagnosed from. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/fuzz.rs | 274 +++++++++++++++++++++---- 1 file changed, 232 insertions(+), 42 deletions(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index c6aadfc981..1996c537aa 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -30,7 +30,7 @@ use net::eth::mac::{Mac, SourceMac}; use net::interface::InterfaceIndex; use net::packet::{DoneReason, Packet, VpcDiscriminant}; use net::vxlan::Vni; -use pipeline::{DynPipeline, NetworkFunction}; +use pipeline::{DynPipeline, NetworkFunction, PipelineData}; use routing::testing::RouterTables; use routing::testing::{FibGroup, FwAction, NhopKey, RouteOrigin}; use routing::{AtableReaderFactory, FibTableReaderFactory, IfTableReaderFactory}; @@ -40,6 +40,17 @@ use std::net::IpAddr; const FIRST_GENID: GenId = 1; +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub(crate) enum Enact { + FlowFilter, + Acl, + StaticNat, + Masquerade, + PortForward, + Generation, + Everything, +} + use super::egress::Egress; use super::ingress::Ingress; use super::ipforward::IpForwarder; @@ -60,6 +71,7 @@ pub(crate) struct Blueprint { static_nat: NatTablesReaderFactory, portfw: PortFwTableReaderFactory, masquerade: NatAllocatorReaderFactory, + pipeline: Arc, underlay: Option, flow_table: Arc, declared: Arc<[Prefix]>, @@ -121,6 +133,7 @@ impl Fleet { static_nat: static_nat.get_reader_factory(), portfw: portfw.reader().factory(), masquerade: masquerade.get_reader_factory(), + pipeline: Arc::new(PipelineData::new(FIRST_GENID)), underlay: tables.map(|tables| Underlay { interfaces: tables.interface_factory(), fibs: tables.fib_factory(), @@ -142,26 +155,44 @@ impl Fleet { } pub(crate) fn reconfigure(&self, overlay: &ValidatedOverlay) { - self.flow_filter.store( - FlowFilterContext::try_from(overlay).expect("a validated overlay lowers to tables"), - ); - self.acl.store( - AclFilterContext::try_from(overlay).expect("a validated overlay lowers to acls"), - ); - self.static_nat.borrow_mut().update_nat_tables( - build_nat_configuration(overlay.vpc_table()) - .expect("a validated overlay lowers to nat"), - ); - self.portfw - .borrow_mut() - .update_from_vpc_table(overlay.vpc_table()) - .expect("a validated overlay lowers to port forwarding"); - self.genid.set(self.genid.get() + 1); - self.masquerade.borrow_mut().update_nat_allocator( - MasqueradeConfig::new(overlay.vpc_table()).set_randomize(false), - self.genid.get(), - &self.blueprint.flow_table, - ); + self.enact(overlay, Enact::Everything); + } + + pub(crate) fn enact(&self, overlay: &ValidatedOverlay, which: Enact) { + let doing = |step: Enact| which == Enact::Everything || which == step; + if doing(Enact::FlowFilter) { + self.flow_filter.store( + FlowFilterContext::try_from(overlay).expect("a validated overlay lowers to tables"), + ); + } + if doing(Enact::Acl) { + self.acl.store( + AclFilterContext::try_from(overlay).expect("a validated overlay lowers to acls"), + ); + } + if doing(Enact::StaticNat) { + self.static_nat.borrow_mut().update_nat_tables( + build_nat_configuration(overlay.vpc_table()) + .expect("a validated overlay lowers to nat"), + ); + } + if doing(Enact::Masquerade) { + self.genid.set(self.genid.get() + 1); + self.masquerade.borrow_mut().update_nat_allocator( + MasqueradeConfig::new(overlay.vpc_table()).set_randomize(false), + self.genid.get(), + &self.blueprint.flow_table, + ); + } + if doing(Enact::PortForward) { + self.portfw + .borrow_mut() + .update_from_vpc_table(overlay.vpc_table()) + .expect("a validated overlay lowers to port forwarding"); + } + if doing(Enact::Generation) { + self.blueprint.pipeline.set_genid(self.genid.get()); + } } pub(crate) fn blueprint(&self) -> &Blueprint { @@ -172,7 +203,7 @@ impl Fleet { impl Blueprint { pub(crate) fn worker(&self) -> Worker { let translations = Arc::new(Mutex::new(Translations::declaring(&self.declared))); - let mut pipeline = DynPipeline::new(); + let mut pipeline = DynPipeline::new().set_data(self.pipeline.clone()); if let Some(underlay) = &self.underlay { pipeline = pipeline.add_stage(Ingress::new("ingress", underlay.interfaces.handle())); @@ -4717,8 +4748,13 @@ mod model { static BARREN: LazyLock = LazyLock::new(|| AtomicU64::new(0)); static ENGULFED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); static FRAMED_OUT: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static RACED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); static DISTURBED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); - static UNDISTURBED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + + fn separate(drawn: u16, round: u8, which: usize) -> u16 { + let within = (drawn / 2).wrapping_add(u16::from(round).wrapping_mul(997)) % 32_000 + 1; + within + u16::try_from(which).unwrap_or_else(|_| unreachable!()) * 32_768 + } fn outside(footprint: &Footprint) -> impl Fn(derive::Named<'_>) -> bool + '_ { move |named| { @@ -4818,15 +4854,7 @@ mod model { let varied: Vec = vary .iter() .map(|v| derive::Vary { - sport: v - .sport - .wrapping_add(u16::from(round) * 997) - .wrapping_add( - u16::try_from(which) - .unwrap_or_else(|_| unreachable!()) - * 401, - ) - .max(1), + sport: separate(v.sport, round, which), ..*v }) .collect(); @@ -4878,18 +4906,24 @@ mod model { }); for (checked, described) in ran { if round == 1 { - if checked { &UNDISTURBED } else { &DISTURBED } + if checked { &RACED } else { &DISTURBED } .fetch_add(1, Ordering::Relaxed); continue; } assert!( checked, "traffic outside the footprint of a configuration change did \ - not survive it, in round {round}. The change was \ - {change:?}, whose write set this load is outside of, so the \ - configuration it ran against and the one enacted agree about \ - it entirely -- and the enactment that carried the difference \ - returned before this round opened. {described}" + not survive it, in round {round}. The change was {change:?}, \ + whose write set this load is outside of, so the configuration \ + it ran against and the one enacted agree about it entirely. \ + {}. {described}", + if round == 1 { + "This round races the enactment that carries the difference" + } else { + "That enactment returned before this round opened, and \ + this round races only a re-enactment of what is already \ + running" + } ); } } @@ -4897,25 +4931,181 @@ mod model { }); }); - let (framed, barren, engulfed, framed_out, disturbed, undisturbed) = ( + let (framed, barren, engulfed, framed_out, raced) = ( FRAMED.load(Ordering::Relaxed), BARREN.load(Ordering::Relaxed), ENGULFED.load(Ordering::Relaxed), FRAMED_OUT.load(Ordering::Relaxed), - DISTURBED.load(Ordering::Relaxed), - UNDISTURBED.load(Ordering::Relaxed), + RACED.load(Ordering::Relaxed), ); eprintln!("framed={framed} barren={barren} engulfed={engulfed} framed_out={framed_out}"); - eprintln!("racing the change: disturbed={disturbed} undisturbed={undisturbed}"); + eprintln!( + "carried while the change was being enacted: {raced}, of which disturbed: {}", + DISTURBED.load(Ordering::Relaxed) + ); super::assert_covered( framed > 0, "no draw left two loads outside the footprint of its last operation, so no \ configuration change was ever carried under traffic", ); + super::assert_covered( + raced > 0, + "no load was ever carried by the round that races the change itself, so every \ + assertion here was about a re-enactment of a configuration already running -- which \ + `re_enacting_a_configuration_under_load_disturbs_nothing` already covers", + ); super::assert_covered( framed_out > 0, "no load was ever filtered out for being inside a footprint, so the frame was always \ the whole configuration and this property is the re-enactment one with extra steps", ); } + + #[tokio::test] + #[dpdk::with_eal] + #[ignore = "an instrument, not a property: reports a rate and asserts nothing"] + #[allow(clippy::too_many_lines, reason = "one instrument, read top to bottom")] + async fn report_which_enactment_step_disturbs_traffic() { + use config::external::overlay::algebra::{ + Draft, Flavour, Op, PeeringHandle, Side, VpcHandle, + }; + + const ROUNDS: usize = 500; + + let vpc = VpcHandle; + let peering = PeeringHandle; + let built = vec![ + Op::AddVpc(vpc(0)), + Op::AddVpc(vpc(1)), + Op::AddPeering { + handle: peering(0), + left: vpc(0), + right: vpc(1), + }, + Op::SetFlavour { + peering: peering(0), + side: Side::Left, + slot: 0, + flavour: Flavour::Masquerade, + }, + Op::AddVpc(vpc(2)), + Op::AddVpc(vpc(3)), + Op::AddPeering { + handle: peering(1), + left: vpc(2), + right: vpc(3), + }, + ]; + let change = Op::SetFlavour { + peering: peering(1), + side: Side::Left, + slot: 0, + flavour: Flavour::Masquerade, + }; + let assemble = |draft: &Draft| { + draft + .overlay() + .expect("assembles") + .validate() + .expect("validates") + }; + let before = Sequence::fold(&built); + let running = assemble(&before); + let mut after_draft = before.clone(); + change.apply(&mut after_draft).expect("the change applies"); + let enacted = assemble(&after_draft); + let footprint = change.writes(&before); + + let vnis: Vec = enacted + .vpc_table() + .values() + .map(config::external::overlay::vpc::ValidatedVpc::vni) + .collect(); + let vary: Vec = (0..2) + .map(|n| derive::Vary { + host: n, + port: 0, + sport: 30000, + dport: 80, + burst: 2, + blast: false, + }) + .collect(); + let outside = |named: derive::Named<'_>| { + !footprint.touches_peering_named(named.peering) + && !footprint.touches_vpc_named(named.local) + && !footprint.touches_vpc_named(named.remote) + }; + eprintln!( + "loads: total={} framed={}", + derive::loads_for(&running, &vary).len(), + derive::loads_where(&running, &vary, &outside).len() + ); + + let handle = tokio::runtime::Handle::current(); + for part in [ + Enact::Everything, + Enact::FlowFilter, + Enact::Acl, + Enact::StaticNat, + Enact::Masquerade, + Enact::PortForward, + Enact::Generation, + ] { + let disturbed = std::sync::atomic::AtomicU64::new(0); + let carried = std::sync::atomic::AtomicU64::new(0); + + for round in 0..ROUNDS { + let tables = topology(&vnis); + let fleet = + Fleet::lowering(&running, Some(&tables), Arc::new(FlowTable::default())); + let blueprint = fleet.blueprint(); + let gate = std::sync::Barrier::new(3); + std::thread::scope(|scope| { + for which in 0..2u16 { + let handle = handle.clone(); + let (gate, disturbed, carried) = (&gate, &disturbed, &carried); + let (running, vary, outside) = (&running, &vary, &outside); + scope.spawn(move || { + let _guard = handle.enter(); + let mut worker = blueprint.worker(); + let varied: Vec = vary + .iter() + .map(|v| derive::Vary { + sport: v.sport + + which * 401 + + u16::try_from(round).unwrap_or(0) * 7, + ..*v + }) + .collect(); + let mine = derive::loads_where(running, &varied, outside); + gate.wait(); + for mut load in mine { + carried.fetch_add(1, Ordering::Relaxed); + let ran = without_unwinding(|| { + for _ in 0..8 { + let Some(packet) = load.next() else { break }; + let out = worker.send(packet); + load.observe(&out); + } + (load.checked(), load.describe()) + }); + let (ok, how) = ran.unwrap_or_else(|why| (false, why)); + if !ok && disturbed.fetch_add(1, Ordering::Relaxed) == 0 { + eprintln!(" {part:?} first, round {round}: {how}"); + } + } + }); + } + gate.wait(); + fleet.enact(&enacted, part); + }); + } + eprintln!( + "{part:?}: carried={} disturbed={}", + carried.load(Ordering::Relaxed), + disturbed.load(Ordering::Relaxed) + ); + } + } } From fbeb66883aabacb62489350be0b44083e8e1a58c Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sun, 23 Aug 2026 11:26:15 -0600 Subject: [PATCH 18/21] fix(nix): Restore the sanitizer ABI check, and stamp the sysroot `-Cunsafe-allow-abi-mismatch=sanitizer` disabled rustc's check that every crate agrees about sanitizer flags -- the check that refuses a half-instrumented link. `tests.pkg.dataplane` builds and links without it under `--argstr sanitize thread`, so there was nothing to excuse: `std` is rebuilt from source by `-Zbuild-std` under the same sanitizer as the rest. The sysroot now records what it was built for, and `just fuzz` refuses when that disagrees with the sanitizer it was asked for. The two knobs are independent and a mismatch produced a binary whose green run meant nothing. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- default.nix | 6 ++++++ justfile | 12 ++++++++++++ nix/profiles.nix | 2 -- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/default.nix b/default.nix index c2a06e9055..c153120f62 100644 --- a/default.nix +++ b/default.nix @@ -87,10 +87,15 @@ let }; in if platform != "wasm32-wasip1" then over.pkgsCross.${platform'.info.nixarch} else over; + sysroot-stamp = '' + printf '%s' '${sanitize}' > "$out/.sanitize" + printf '%s' '${instrumentation}' > "$out/.instrumentation" + ''; sysroot = if platform != "wasm32-wasip1" then pkgs.symlinkJoin { name = "sysroot"; + postBuild = sysroot-stamp; paths = with pkgs.pkgsHostHost; [ pkgs.pkgsHostHost.libc.dev # fully qualified: bare `libc` resolves to the "gnu" function argument, not pkgs.pkgsHostHost.libc pkgs.pkgsHostHost.libc.out # (same as above) @@ -116,6 +121,7 @@ let else pkgs.symlinkJoin { name = "sysroot"; + postBuild = sysroot-stamp; paths = with pkgs.pkgsHostHost; [ fancy.hwloc.dev fancy.hwloc.static diff --git a/justfile b/justfile index f6257d8777..2f481f291f 100644 --- a/justfile +++ b/justfile @@ -205,6 +205,18 @@ fuzz target time="60s" *args="": # asan does not need that, and skipping the std rebuild keeps it far quicker. # `sanitize=NONE` drops instrumentation altogether, which buys roughly four times # the executions per second in exchange for only catching what the test asserts. + sysroot="${DATAPLANE_SYSROOT:-}" + if [ -n "${sysroot}" ] && [ -r "${sysroot}/.sanitize" ]; then + built_with="$(cat "${sysroot}/.sanitize")" + if [ "${built_with}" != "{{ sanitize }}" ]; then + printf 'refusing to fuzz: sanitize=%s was asked for, but this sysroot was built with sanitize=%s.\n' \ + "{{ sanitize }}" "${built_with:-}" >&2 + printf 'the C dependencies would not be instrumented. Re-enter the shell with:\n' >&2 + printf ' just sanitize=%s setup-roots && nix-shell --argstr sanitize %s\n' \ + "{{ sanitize }}" "{{ sanitize }}" >&2 + exit 1 + fi + fi corpus_dir="{{ fuzz_corpus_root }}/$(printf '%s' '{{ target }}' | tr -c 'A-Za-z0-9_.-' '_')" mkdir -p "${corpus_dir}" cargo bolero test '{{ target }}' --rustc-bootstrap -T '{{ time }}' \ diff --git a/nix/profiles.nix b/nix/profiles.nix index 3e0671a531..c05f346764 100644 --- a/nix/profiles.nix +++ b/nix/profiles.nix @@ -165,8 +165,6 @@ let sanitize.thread.RUSTFLAGS = [ "-Zsanitizer=thread" "-Zexternal-clangrt" - # gimli doesn't like thread sanitizer, but it shouldn't be an issue since that is all build time logic - "-Cunsafe-allow-abi-mismatch=sanitizer" ] ++ (map (flag: "-Clink-arg=${flag}") sanitize.thread.NIX_CFLAGS_LINK); # note: cfi _requires_ LTO and is fundamentally ill suited to debug builds From 9bbbab15646156c370b2da09a154ef32736a3edc Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sun, 23 Aug 2026 11:32:40 -0600 Subject: [PATCH 19/21] test(dataplane): Sustain the traffic the enactment instrument measures against A short burst right after the barrier measured when the allocator swap landed rather than what it did: `Everything` spends about a millisecond rebuilding rte_acl before reaching the allocator, by which time the burst was over, so it looked fifty times safer than the masquerade step alone. Sustained, the two are the same order. Also models the router-config await `mgmt` performs between the allocator swap and the generation publish, which turns out to change nothing -- the doc comment records that and what thread sanitizer said, which was nothing, on a build that instruments dpdk and rebuilds std. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/fuzz.rs | 76 ++++++++++++++++++++------ 1 file changed, 59 insertions(+), 17 deletions(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 1996c537aa..43efdd40ef 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -37,6 +37,7 @@ use routing::{AtableReaderFactory, FibTableReaderFactory, IfTableReaderFactory}; use routing::{EgressObject, FibEntry, PktInstruction, ResolvedEncapsulation, ResolvedVxlan, Vtep}; use std::cell::{Cell, RefCell}; use std::net::IpAddr; +use std::time::Duration; const FIRST_GENID: GenId = 1; @@ -158,6 +159,20 @@ impl Fleet { self.enact(overlay, Enact::Everything); } + pub(crate) fn enact_with_router_gap(&self, overlay: &ValidatedOverlay, gap: Duration) { + for step in [ + Enact::FlowFilter, + Enact::Acl, + Enact::StaticNat, + Enact::Masquerade, + Enact::PortForward, + ] { + self.enact(overlay, step); + } + std::thread::sleep(gap); + self.enact(overlay, Enact::Generation); + } + pub(crate) fn enact(&self, overlay: &ValidatedOverlay, which: Enact) { let doing = |step: Enact| which == Enact::Everything || which == step; if doing(Enact::FlowFilter) { @@ -4970,7 +4985,13 @@ mod model { Draft, Flavour, Op, PeeringHandle, Side, VpcHandle, }; - const ROUNDS: usize = 500; + fn separate(drawn: u16, rep: u8, which: u16) -> u16 { + let within = (drawn / 2).wrapping_add(u16::from(rep).wrapping_mul(997)) % 32_000 + 1; + within + which * 32_768 + } + + const ROUNDS: usize = 60; + const REPS: u8 = 24; let vpc = VpcHandle; let peering = PeeringHandle; @@ -5043,7 +5064,7 @@ mod model { ); let handle = tokio::runtime::Handle::current(); - for part in [ + let steps = [ Enact::Everything, Enact::FlowFilter, Enact::Acl, @@ -5051,7 +5072,10 @@ mod model { Enact::Masquerade, Enact::PortForward, Enact::Generation, - ] { + Enact::Everything, + ]; + for (nth, part) in steps.into_iter().enumerate() { + let gapped = nth == steps.len() - 1; let disturbed = std::sync::atomic::AtomicU64::new(0); let carried = std::sync::atomic::AtomicU64::new(0); @@ -5080,29 +5104,47 @@ mod model { .collect(); let mine = derive::loads_where(running, &varied, outside); gate.wait(); - for mut load in mine { - carried.fetch_add(1, Ordering::Relaxed); - let ran = without_unwinding(|| { - for _ in 0..8 { - let Some(packet) = load.next() else { break }; - let out = worker.send(packet); - load.observe(&out); + for rep in 0..REPS { + let mine = derive::loads_where( + running, + &vary + .iter() + .map(|v| derive::Vary { + sport: separate(v.sport, rep, which), + ..*v + }) + .collect::>(), + outside, + ); + for mut load in mine { + carried.fetch_add(1, Ordering::Relaxed); + let ran = without_unwinding(|| { + for _ in 0..8 { + let Some(packet) = load.next() else { break }; + let out = worker.send(packet); + load.observe(&out); + } + (load.checked(), load.describe()) + }); + let (ok, how) = ran.unwrap_or_else(|why| (false, why)); + if !ok && disturbed.fetch_add(1, Ordering::Relaxed) == 0 { + eprintln!(" {part:?} first, round {round}: {how}"); } - (load.checked(), load.describe()) - }); - let (ok, how) = ran.unwrap_or_else(|why| (false, why)); - if !ok && disturbed.fetch_add(1, Ordering::Relaxed) == 0 { - eprintln!(" {part:?} first, round {round}: {how}"); } } }); } gate.wait(); - fleet.enact(&enacted, part); + if gapped { + fleet.enact_with_router_gap(&enacted, Duration::from_micros(500)); + } else { + fleet.enact(&enacted, part); + } }); } eprintln!( - "{part:?}: carried={} disturbed={}", + "{part:?}{}: carried={} disturbed={}", + if gapped { " (with the router gap)" } else { "" }, carried.load(Ordering::Relaxed), disturbed.load(Ordering::Relaxed) ); From 3e9ae0174dc975fbf82f5926c07b2145ee4d5167 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sun, 23 Aug 2026 11:51:12 -0600 Subject: [PATCH 20/21] test(dataplane): Explain the masquerade swap transient The transient the step instrument attributed to the masquerade store is a dataplane defect, not a harness one: an allocator swap can give one public tuple to two live flows, and a reply then reaches the wrong tenant conversation. `set_randomize(false)` was the last deviation this harness had from `mgmt::apply_masquerade_config` and was the obvious suspect. It is not the answer -- the instrument now runs both settings, with the running allocator primed so the swap under test is random-to-random. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/fuzz.rs | 239 +++++++++++++++++++++++-- 1 file changed, 228 insertions(+), 11 deletions(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 43efdd40ef..da42d7a086 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -63,6 +63,7 @@ pub(crate) struct Fleet { portfw: RefCell, masquerade: RefCell, genid: Cell, + randomize: Cell, blueprint: Blueprint, } @@ -151,6 +152,7 @@ impl Fleet { portfw: RefCell::new(portfw), masquerade: RefCell::new(masquerade), genid: Cell::new(FIRST_GENID), + randomize: Cell::new(false), blueprint, } } @@ -173,6 +175,11 @@ impl Fleet { self.enact(overlay, Enact::Generation); } + pub(crate) fn randomizing(&self, on: bool) -> &Self { + self.randomize.set(on); + self + } + pub(crate) fn enact(&self, overlay: &ValidatedOverlay, which: Enact) { let doing = |step: Enact| which == Enact::Everything || which == step; if doing(Enact::FlowFilter) { @@ -194,7 +201,7 @@ impl Fleet { if doing(Enact::Masquerade) { self.genid.set(self.genid.get() + 1); self.masquerade.borrow_mut().update_nat_allocator( - MasqueradeConfig::new(overlay.vpc_table()).set_randomize(false), + MasqueradeConfig::new(overlay.vpc_table()).set_randomize(self.randomize.get()), self.genid.get(), &self.blueprint.flow_table, ); @@ -603,6 +610,10 @@ pub(crate) trait Load { fn checked(&self) -> bool; fn describe(&self) -> String; + + fn public(&self) -> Option<(IpAddr, u16)> { + None + } } #[cfg(test)] @@ -3215,6 +3226,7 @@ mod routed { dport: u16, sent: Option>, state: State, + public: Option<(IpAddr, u16)>, log: Vec, } @@ -3238,6 +3250,7 @@ mod routed { dport, sent: None, state: State::Opening, + public: None, log: Vec::new(), } } @@ -3282,6 +3295,7 @@ mod routed { return; }; self.note(&format!("request left as {public_src}:{}", port.get())); + self.public = Some((public_src, port.get())); self.state = State::Replying { public: (public_src, port.get()), }; @@ -3326,6 +3340,10 @@ mod routed { } impl Load for Conversation { + fn public(&self) -> Option<(IpAddr, u16)> { + self.public + } + fn next(&mut self) -> Option> { match self.state { State::Opening => { @@ -5093,16 +5111,6 @@ mod model { scope.spawn(move || { let _guard = handle.enter(); let mut worker = blueprint.worker(); - let varied: Vec = vary - .iter() - .map(|v| derive::Vary { - sport: v.sport - + which * 401 - + u16::try_from(round).unwrap_or(0) * 7, - ..*v - }) - .collect(); - let mine = derive::loads_where(running, &varied, outside); gate.wait(); for rep in 0..REPS { let mine = derive::loads_where( @@ -5150,4 +5158,213 @@ mod model { ); } } + + #[tokio::test] + #[dpdk::with_eal] + #[ignore = "an instrument, not a property: prints one trace and asserts nothing"] + #[allow(clippy::too_many_lines, reason = "one instrument, read top to bottom")] + async fn report_why_the_masquerade_swap_disturbs_traffic() { + use config::external::overlay::algebra::{ + Draft, Flavour, Op, PeeringHandle, Side, VpcHandle, + }; + use std::sync::atomic::AtomicBool; + + fn separate(drawn: u16, rep: u8, which: u16) -> u16 { + let within = (drawn / 2).wrapping_add(u16::from(rep).wrapping_mul(997)) % 32_000 + 1; + within + which * 32_768 + } + + const ROUNDS: [usize; 2] = [200, 6000]; + const REPS: u8 = 24; + + fn reason(how: &str) -> String { + how.lines() + .next() + .unwrap_or(how) + .rsplit('|') + .next() + .unwrap_or(how) + .trim() + .to_owned() + } + + let vpc = VpcHandle; + let peering = PeeringHandle; + let built = vec![ + Op::AddVpc(vpc(0)), + Op::AddVpc(vpc(1)), + Op::AddPeering { + handle: peering(0), + left: vpc(0), + right: vpc(1), + }, + Op::SetFlavour { + peering: peering(0), + side: Side::Left, + slot: 0, + flavour: Flavour::Masquerade, + }, + Op::AddVpc(vpc(2)), + Op::AddVpc(vpc(3)), + Op::AddPeering { + handle: peering(1), + left: vpc(2), + right: vpc(3), + }, + ]; + let change = Op::SetFlavour { + peering: peering(1), + side: Side::Left, + slot: 0, + flavour: Flavour::Masquerade, + }; + let assemble = |draft: &Draft| { + draft + .overlay() + .expect("assembles") + .validate() + .expect("validates") + }; + let before = Sequence::fold(&built); + let running = assemble(&before); + let mut after_draft = before.clone(); + change.apply(&mut after_draft).expect("the change applies"); + let enacted = assemble(&after_draft); + let footprint = change.writes(&before); + + let vnis: Vec = enacted + .vpc_table() + .values() + .map(config::external::overlay::vpc::ValidatedVpc::vni) + .collect(); + let vary: Vec = (0..2) + .map(|n| derive::Vary { + host: n, + port: 0, + sport: 30000, + dport: 80, + burst: 2, + blast: false, + }) + .collect(); + let outside = |named: derive::Named<'_>| { + !footprint.touches_peering_named(named.peering) + && !footprint.touches_vpc_named(named.local) + && !footprint.touches_vpc_named(named.remote) + }; + + let handle = tokio::runtime::Handle::current(); + for (nth, randomize) in [false, true].into_iter().enumerate() { + eprintln!("\n== randomize={randomize} =="); + let explained = AtomicBool::new(false); + let carried = AtomicU64::new(0); + let disturbed = AtomicU64::new(0); + let why: Mutex> = + Mutex::new(std::collections::BTreeMap::new()); + let mut duplicated = 0u64; + + for round in 0..ROUNDS[nth] { + let tables = topology(&vnis); + let fleet = + Fleet::lowering(&running, Some(&tables), Arc::new(FlowTable::default())); + fleet.randomizing(randomize); + if randomize { + fleet.enact(&running, Enact::Masquerade); + } + let blueprint = fleet.blueprint(); + let gate = std::sync::Barrier::new(3); + let handed: Mutex> = Mutex::new(Vec::new()); + let writing = tracectl::evidence::Capture::new("config-apply") + .depth(8192) + .keeping(|target| target.starts_with("dataplane_nat")) + .start(); + let written = writing.evidence(); + let traces: Mutex> = Mutex::new(vec![written]); + std::thread::scope(|scope| { + for which in 0..2u16 { + let handle = handle.clone(); + let (gate, explained, carried, disturbed) = + (&gate, &explained, &carried, &disturbed); + let (running, vary, outside) = (&running, &vary, &outside); + let (traces, why, handed) = (&traces, &why, &handed); + scope.spawn(move || { + let _guard = handle.enter(); + let recording = + tracectl::evidence::Capture::new(format!("masq-{which}")) + .depth(8192) + .keeping(|target| target.starts_with("dataplane_nat")) + .start(); + traces.lock().push(recording.evidence()); + let mut worker = blueprint.worker(); + gate.wait(); + let mut given = Vec::new(); + for rep in 0..REPS { + let mine = derive::loads_where( + running, + &vary + .iter() + .map(|v| derive::Vary { + sport: separate(v.sport, rep, which), + ..*v + }) + .collect::>(), + outside, + ); + for mut load in mine { + carried.fetch_add(1, Ordering::Relaxed); + let ran = without_unwinding(|| { + for _ in 0..8 { + let Some(packet) = load.next() else { break }; + let out = worker.send(packet); + load.observe(&out); + } + load.checked() + }); + given.extend(load.public()); + let (ok, how) = match ran { + Ok(ok) => (ok, load.describe()), + Err(why) => (false, format!("{why}. {}", load.describe())), + }; + if ok { + continue; + } + disturbed.fetch_add(1, Ordering::Relaxed); + *why.lock().entry(reason(&how)).or_default() += 1; + if !explained.swap(true, Ordering::Relaxed) { + eprintln!( + "\ndisturbed in round {round}, rep {rep}, worker \ + {which}: {how}\n" + ); + for trace in traces.lock().iter() { + trace.dump(); + } + } + } + } + handed.lock().extend(given); + }); + } + gate.wait(); + fleet.enact(&enacted, Enact::Masquerade); + }); + + let mut seen = std::collections::BTreeSet::new(); + for tuple in handed.lock().iter() { + if !seen.insert(*tuple) { + duplicated += 1; + eprintln!(" round {round}: {tuple:?} was handed to two live flows"); + } + } + } + + eprintln!( + "carried={} disturbed={} duplicated={duplicated}", + carried.load(Ordering::Relaxed), + disturbed.load(Ordering::Relaxed), + ); + for (reason, count) in why.lock().iter() { + eprintln!(" {count:>4} {reason}"); + } + } + } } From 12dca4e357360559dae6c050b189eb12d1d233af Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Thu, 27 Aug 2026 21:23:04 -0600 Subject: [PATCH 21/21] style(dataplane,dpdk,flow-filter,tracectl): Settle the sync facade for opengrep Two different answers, because there are two different cases. The properties in `packet_processor::fuzz` go through the facade. Some of them run under `concurrency::model_test`, and the `Barrier` two of them synchronise their workers on was a real `std::sync::Barrier` -- invisible to the model checker, which is the case the facade exists for rather than a lint nit. The rest are deliberate and stay, with the suppression the rule provides and the neighbouring code already uses: a model-checked mutex cannot live in a `static` (`dpdk`, `flow-filter`), and an instrumented diagnostic would perturb the schedule it is meant to report on (`tracectl`). Signed-off-by: Daniel Noland --- dataplane/src/packet_processor/fuzz.rs | 27 ++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index da42d7a086..7b9bac5c7f 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -3694,13 +3694,13 @@ mod model { use super::routed::{Conversation, exposes, inner, inside, tunnelled}; use super::*; use concurrency::sync::Mutex; + use concurrency::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; + use concurrency::sync::{LazyLock, OnceLock}; use concurrency::thread; #[cfg_attr(not(feature = "shuttle"), allow(unused_imports))] use concurrency::thread::BuilderExt; use config::external::overlay::algebra::{Footprint, Sequence}; use net::packet::test_utils::build_test_udp_ipv4_packet; - use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; - use std::sync::{LazyLock, OnceLock}; type Tuple = (Option, Option); @@ -3945,7 +3945,8 @@ mod model { } SPLIT.fetch_add(1, Ordering::Relaxed); - let drawn = std::sync::Arc::new((validated, vnis, vary.clone(), schedule.clone())); + let drawn = + concurrency::sync::Arc::new((validated, vnis, vary.clone(), schedule.clone())); let entering = handle.clone(); concurrency::stress(move || { @@ -4854,8 +4855,14 @@ mod model { return; } - let drawn = - std::sync::Arc::new((running, enacted, vnis, vary.clone(), footprint, *change)); + let drawn = concurrency::sync::Arc::new(( + running, + enacted, + vnis, + vary.clone(), + footprint, + *change, + )); let entering = handle.clone(); concurrency::stress(move || { @@ -5094,15 +5101,15 @@ mod model { ]; for (nth, part) in steps.into_iter().enumerate() { let gapped = nth == steps.len() - 1; - let disturbed = std::sync::atomic::AtomicU64::new(0); - let carried = std::sync::atomic::AtomicU64::new(0); + let disturbed = concurrency::sync::atomic::AtomicU64::new(0); + let carried = concurrency::sync::atomic::AtomicU64::new(0); for round in 0..ROUNDS { let tables = topology(&vnis); let fleet = Fleet::lowering(&running, Some(&tables), Arc::new(FlowTable::default())); let blueprint = fleet.blueprint(); - let gate = std::sync::Barrier::new(3); + let gate = concurrency::sync::Barrier::new(3); std::thread::scope(|scope| { for which in 0..2u16 { let handle = handle.clone(); @@ -5164,10 +5171,10 @@ mod model { #[ignore = "an instrument, not a property: prints one trace and asserts nothing"] #[allow(clippy::too_many_lines, reason = "one instrument, read top to bottom")] async fn report_why_the_masquerade_swap_disturbs_traffic() { + use concurrency::sync::atomic::AtomicBool; use config::external::overlay::algebra::{ Draft, Flavour, Op, PeeringHandle, Side, VpcHandle, }; - use std::sync::atomic::AtomicBool; fn separate(drawn: u16, rep: u8, which: u16) -> u16 { let within = (drawn / 2).wrapping_add(u16::from(rep).wrapping_mul(997)) % 32_000 + 1; @@ -5272,7 +5279,7 @@ mod model { fleet.enact(&running, Enact::Masquerade); } let blueprint = fleet.blueprint(); - let gate = std::sync::Barrier::new(3); + let gate = concurrency::sync::Barrier::new(3); let handed: Mutex> = Mutex::new(Vec::new()); let writing = tracectl::evidence::Capture::new("config-apply") .depth(8192)