diff --git a/.cargo/mutants.toml b/.cargo/mutants.toml new file mode 100644 index 0000000000..02124cc20c --- /dev/null +++ b/.cargo/mutants.toml @@ -0,0 +1,13 @@ + +exclude_re = [ + " vpc2). The manifest names // ("vpc1"/"vpc2") double as the ACL rule `from`/`to` endpoints. @@ -670,7 +670,7 @@ fn ipv6_allow_and_default_deny() { // reply's weak `related` reference can still be upgraded. fn attach_related_flow(reply: &mut Packet, fwd_key: FlowKey) -> Arc { let reply_key = FlowKey::try_from(&*reply).unwrap(); - let expiry = Instant::now() + Duration::from_secs(60); + let expiry = clock::now() + Duration::from_secs(60); let (fwd_flow, reply_flow) = FlowInfo::related_pair( expiry, fwd_key, diff --git a/acl/Cargo.toml b/acl/Cargo.toml index 41eee1cafe..f9d01cb8a9 100644 --- a/acl/Cargo.toml +++ b/acl/Cargo.toml @@ -12,6 +12,7 @@ reference = [] [dependencies] arrayvec = { workspace = true, default-features = true } +clock = { workspace = true } concurrency = { workspace = true, features = [] } dpdk = { workspace = true, optional = true } lookup = { workspace = true, features = [] } @@ -20,6 +21,7 @@ net = { workspace = true, features = [] } thiserror = { workspace = true } [dev-dependencies] +clock = { workspace = true, features = ["virtual"] } # Enable the "reference" backend for this crate's own tests and benches. It is a non-default feature # (production links only the rte_acl backend), but the integration tests and benches # differential-test against it, so make it available whenever test/bench targets are built. diff --git a/acl/src/dpdk/dyn_table.rs b/acl/src/dpdk/dyn_table.rs index 744aa9e4e0..0635215bea 100644 --- a/acl/src/dpdk/dyn_table.rs +++ b/acl/src/dpdk/dyn_table.rs @@ -494,7 +494,6 @@ mod tests { use lookup::Lookup; use match_action::{Erased, ExactSpec, MaskSpec, MatchKey, PrefixSpec, RangeSpec}; use std::hint::black_box; - use std::time::Instant; #[derive(MatchKey, Debug, Clone, Copy)] struct FiveTuple { @@ -969,7 +968,7 @@ mod tests { let rules = make(n); let max = NonZero::new(u32::try_from(n).unwrap()).unwrap(); let rss_before = rss_kb(); - let t = Instant::now(); + let t = clock::now(); let res: Result, u32>, _> = install_table(&unique_name("cap"), max, rules); let dt = t.elapsed().as_secs_f64() * 1e3; diff --git a/clock/Cargo.toml b/clock/Cargo.toml new file mode 100644 index 0000000000..17644aa53f --- /dev/null +++ b/clock/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "dataplane-clock" +edition.workspace = true +license.workspace = true +publish.workspace = true +version.workspace = true + +[features] +default = [] +virtual = ["dep:tokio"] + +[dependencies] +tokio = { workspace = true, optional = true, features = ["test-util", "time"] } diff --git a/clock/src/lib.rs b/clock/src/lib.rs new file mode 100644 index 0000000000..9493f60e41 --- /dev/null +++ b/clock/src/lib.rs @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +#![deny(clippy::all, clippy::pedantic)] +#![deny(rustdoc::all)] +#![deny(unsafe_code)] + +pub use std::time::{Duration, Instant, SystemTime, SystemTimeError, TryFromFloatSecsError}; + +#[must_use] +pub fn now() -> Instant { + #[cfg(feature = "virtual")] + { + tokio::time::Instant::now().into_std() + } + #[cfg(not(feature = "virtual"))] + { + Instant::now() + } +} + +#[must_use] +pub fn system_now() -> SystemTime { + SystemTime::now() +} + +#[cfg(test)] +mod tests { + use super::{Duration, now, system_now}; + + #[test] + fn now_is_monotonic() { + let first = now(); + let second = now(); + assert!(second >= first, "the monotonic clock went backwards"); + } + + #[test] + fn now_works_with_no_runtime() { + let _ = now(); + let _ = system_now(); + } + + #[test] + fn durations_are_plain_values() { + assert_eq!(Duration::from_secs(1).as_millis(), 1000); + } +} diff --git a/config/Cargo.toml b/config/Cargo.toml index ffe89a893a..b14a43a35c 100644 --- a/config/Cargo.toml +++ b/config/Cargo.toml @@ -10,6 +10,7 @@ bolero = ["dep:bolero", "lpm/bolero"] [dependencies] # internal +clock = { workspace = true } common = { workspace = true } concurrency = { workspace = true } k8s-intf = { workspace = true } @@ -33,6 +34,7 @@ linkme = { workspace = true } tracectl = { workspace = true } [dev-dependencies] +clock = { workspace = true, features = ["virtual"] } # internal pipeline = { workspace = true } # should be removed w/ NAT lpm = { workspace = true, features = ["bolero", "testing"] } diff --git a/config/src/external/overlay/vpcpeering.rs b/config/src/external/overlay/vpcpeering.rs index da855cd2f7..b616a8a32c 100644 --- a/config/src/external/overlay/vpcpeering.rs +++ b/config/src/external/overlay/vpcpeering.rs @@ -1078,6 +1078,95 @@ pub mod contract { } } + #[derive(Debug, Clone, Copy)] + pub struct PortForwardingExposes(pub u8); + + impl Default for PortForwardingExposes { + fn default() -> Self { + Self(2) + } + } + + pub const MAX_PORT_FORWARDING_EXPOSES: u8 = 2; + + impl ValueGenerator for PortForwardingExposes { + type Output = Vec; + + fn generate(&self, driver: &mut D) -> Option> { + let v4 = driver.produce::()?; + let count = driver.gen_u8( + Included(&1), + Included(&self.0.clamp(1, MAX_PORT_FORWARDING_EXPOSES)), + )?; + (0..count) + .map(|slot| { + let proto = if slot == 0 { + L4Protocol::Tcp + } else { + L4Protocol::Udp + }; + port_forwarding_expose(driver, v4, slot, proto) + }) + .collect() + } + } + + fn port_forwarding_expose( + driver: &mut D, + v4: bool, + block: u8, + proto: L4Protocol, + ) -> Option { + let host_bits = driver.gen_u8(Included(&0), Included(&MAX_HOST_BITS))?; + let (internal, external) = if v4 { + v4_pair_in_block(driver, host_bits, block)? + } else { + v6_pair_in_block(driver, host_bits, block)? + }; + + let count = driver.gen_u16(Included(&1), Included(&MAX_PORTS))?; + let internal_ports = port_range(driver, count)?; + let external_ports = port_range(driver, count)?; + let idle_timeout = match driver.gen_u8(Included(&0), Included(&2))? { + 0 => None, + 1 => Some(Duration::from_secs(5)), + _ => Some(Duration::from_mins(5)), + }; + + VpcExpose::empty() + .make_port_forwarding(idle_timeout, Some(proto)) + .ok()? + .ip(PrefixWithOptionalPorts::new(internal, Some(internal_ports))) + .as_range(PrefixWithOptionalPorts::new(external, Some(external_ports))) + .ok() + } + + fn v4_pair_in_block( + driver: &mut D, + host_bits: u8, + block: u8, + ) -> Option<(Prefix, Prefix)> { + let len = 32 - host_bits; + let mask = u32::MAX.checked_shl(u32::from(host_bits)).unwrap_or(0); + let slot = u32::from(block) << 16; + let internal = (0x0A00_0000 | slot | (driver.produce::()? & 0x0000_FFFF)) & mask; + let external = (0xAC10_0000 | slot | (driver.produce::()? & 0x0000_FFFF)) & mask; + Some((prefix_v4(internal, len)?, prefix_v4(external, len)?)) + } + + fn v6_pair_in_block( + driver: &mut D, + host_bits: u8, + block: u8, + ) -> Option<(Prefix, Prefix)> { + let len = 128 - host_bits; + let mask = u128::MAX.checked_shl(u32::from(host_bits)).unwrap_or(0); + let slot = u128::from(block) << 64; + let internal = (INTERNAL_BASE | slot | u128::from(driver.produce::()?)) & mask; + let external = (EXTERNAL_BASE | slot | u128::from(driver.produce::()?)) & mask; + Some((prefix_v6(internal, len)?, prefix_v6(external, len)?)) + } + #[derive(Debug, Clone, Copy, Default)] pub struct MasqueradeExpose; diff --git a/config/src/gwconfig.rs b/config/src/gwconfig.rs index 5061a4e91e..8a195fa8ad 100644 --- a/config/src/gwconfig.rs +++ b/config/src/gwconfig.rs @@ -36,7 +36,7 @@ impl GwConfigMeta { fn new(genid: GenId) -> Self { Self { genid, - create_t: SystemTime::now(), + create_t: clock::system_now(), apply_t: None, error: None, is_rollback: false, @@ -46,7 +46,7 @@ impl GwConfigMeta { /// Set the time when attempting to apply a configuration finished. //////////////////////////////////////////////////////////////////////////////// pub fn apply_time(&mut self) { - self.apply_t = Some(SystemTime::now()); + self.apply_t = Some(clock::system_now()); } //////////////////////////////////////////////////////////////////////////////// diff --git a/dataplane/Cargo.toml b/dataplane/Cargo.toml index c258c38e95..78f13d5cd6 100644 --- a/dataplane/Cargo.toml +++ b/dataplane/Cargo.toml @@ -18,6 +18,7 @@ args = { workspace = true } arrayvec = { workspace = true } axum = { workspace = true, features = ["http1", "tokio"] } axum-server = { workspace = true } +clock = { workspace = true } common = { workspace = true } concurrency = { workspace = true } config = { workspace = true } @@ -55,6 +56,7 @@ tracing-subscriber = { workspace = true, default-features = true } vpcmap = { workspace = true } [dev-dependencies] +clock = { workspace = true, features = ["virtual"] } # internal net = { workspace = true, features = ["test_buffer"] } routing = { workspace = true, features = ["testing"] } diff --git a/dataplane/src/drivers/kernel/mod.rs b/dataplane/src/drivers/kernel/mod.rs index 51a0072b32..3e6ca11ead 100644 --- a/dataplane/src/drivers/kernel/mod.rs +++ b/dataplane/src/drivers/kernel/mod.rs @@ -18,7 +18,7 @@ mod sockstats; mod worker; use std::ops::Add; -use std::time::{Duration, Instant}; +use std::time::Duration; use concurrency::sync::Arc; use concurrency::thread; @@ -252,7 +252,7 @@ impl DriverKernel { .collect(); // the next instant when the rx tasks watchdogs should be checked. - let mut next_watchdog_check = Instant::now().add(check_period); + let mut next_watchdog_check = clock::now().add(check_period); loop { // check if we must run. Otherwise (got cancelled) join all workers @@ -262,7 +262,7 @@ impl DriverKernel { // check the current time and decide if we should check whether the rx tasks patted the watchdogs. // If so, compute the next time we should check them again in the future. - let now = Instant::now(); + let now = clock::now(); let check_watchdog = now >= next_watchdog_check; if check_watchdog { while next_watchdog_check <= now { diff --git a/flow-entry/Cargo.toml b/flow-entry/Cargo.toml index c0bd08e621..e76663374b 100644 --- a/flow-entry/Cargo.toml +++ b/flow-entry/Cargo.toml @@ -16,6 +16,7 @@ bolero = ["dep:bolero", "net/bolero"] [dependencies] ahash = { workspace = true, features = ["no-rng"] } bolero = { workspace = true, optional = true } +clock = { workspace = true } common = { workspace = true } concurrency = { workspace = true } dashmap = { workspace = true, features = ["raw-api"] } @@ -29,6 +30,7 @@ tracectl = { workspace = true } tracing = { workspace = true } [dev-dependencies] +clock = { workspace = true, features = ["virtual"] } bolero = { workspace = true, default-features = false } net = { workspace = true, features = ["bolero"] } tokio = { workspace = true, features = ["macros", "rt", "test-util", "time"] } diff --git a/flow-entry/src/flow_table/concurrent_fuzz.rs b/flow-entry/src/flow_table/concurrent_fuzz.rs index 9a757cd6b7..78924a974e 100644 --- a/flow-entry/src/flow_table/concurrent_fuzz.rs +++ b/flow-entry/src/flow_table/concurrent_fuzz.rs @@ -49,12 +49,12 @@ use concurrency::sync::atomic::{AtomicU8, Ordering}; use concurrency::sync::{Arc, Weak}; use concurrency::thread; // `spawn_scoped` is inherent on std's `Builder`, but supplied by `BuilderExt` under shuttle +use clock::Duration; #[cfg_attr(not(feature = "shuttle"), allow(unused_imports))] use concurrency::thread::BuilderExt; use net::FlowKey; use net::flows::{ExtractRef, FlowInfo, FlowInfoFlags}; use std::fmt; -use std::time::{Duration, Instant}; /// Stub [`FlowInfoItem`] payload: a single `Arc` that all flows /// share within one scenario. The blanket @@ -160,7 +160,7 @@ fn insert_flows(table: &FlowTable, keys: &[FlowKey], stub_status: &Arc // Far-future expiry so the per-flow timer never fires inside the test // window — we race the insert path, not the expiry path. (The timer task // is also cfg'd out entirely under shuttle.) - let expires_at = Instant::now() + Duration::from_hours(1); + let expires_at = clock::now() + Duration::from_hours(1); let flows: Vec> = match keys { [fwd_key, rev_key] => { let (fwd, rev) = FlowInfo::related_pair( diff --git a/flow-entry/src/flow_table/nf_lookup.rs b/flow-entry/src/flow_table/nf_lookup.rs index e741a05a42..d67a489628 100644 --- a/flow-entry/src/flow_table/nf_lookup.rs +++ b/flow-entry/src/flow_table/nf_lookup.rs @@ -57,6 +57,7 @@ impl NetworkFunction for FlowLookup { #[cfg(test)] mod test { + use clock::Duration; use concurrency::sync::Arc; use net::FlowKey; use net::buffer::PacketBufferMut; @@ -74,7 +75,6 @@ mod test { use pipeline::DynPipeline; use pipeline::NetworkFunction; use std::net::IpAddr; - use std::time::{Duration, Instant}; use tracing_test::traced_test; use crate::flow_table::FlowTable; @@ -101,7 +101,7 @@ mod test { // Insert matching flow entry let flow_key = FlowKey::try_from(&packet).unwrap(); - let flow_info = FlowInfo::new(flow_key, Instant::now() + Duration::from_secs(10)); + let flow_info = FlowInfo::new(flow_key, clock::now() + Duration::from_secs(10)); flow_table.insert(flow_info).unwrap(); // Ensure packet is tagged @@ -130,7 +130,7 @@ mod test { ) -> impl Iterator> + 'a { input.filter_map(move |packet| { let flow_key = FlowKey::try_from(&packet).unwrap(); - let flow_info = FlowInfo::new(flow_key, Instant::now() + self.timeout); + let flow_info = FlowInfo::new(flow_key, clock::now() + self.timeout); self.flow_table .insert(flow_info) .expect("insert in FlowInfoCreator should not fail"); @@ -196,7 +196,7 @@ mod test { let key_2 = FlowKey::try_from(&packet_2).unwrap(); // create a pair of related flow entries; flow_2 will get a longer timeout - let expires_at = tokio::time::Instant::now().into_std() + Duration::from_secs(2); + let expires_at = clock::now() + Duration::from_secs(2); let (flow_1, flow_2) = FlowInfo::related_pair( expires_at, key_1, diff --git a/flow-entry/src/flow_table/table.rs b/flow-entry/src/flow_table/table.rs index 9972072937..757c605e67 100644 --- a/flow-entry/src/flow_table/table.rs +++ b/flow-entry/src/flow_table/table.rs @@ -472,14 +472,13 @@ mod tests { #[concurrency_mode(std)] mod std_tests { use net::flows::FlowInfoFlags; - use std::time::Instant; use tracing_test::traced_test; use super::*; #[tokio::test] async fn test_flow_table_insert_and_remove() { - let now = Instant::now(); + let now = clock::now(); let five_seconds = Duration::new(5, 0); let five_seconds_from_now = now + five_seconds; @@ -503,13 +502,11 @@ mod tests { // start_paused so the timer task's sleep_until and the test's sleeps share tokio's // virtual clock; otherwise miri's slow interpretation can drift the wall clock far - // enough between Instant::now() and the first sleep that the deadline elapses early. - // Anchor `now` on the virtual clock too -- a std::Instant::now() here would be many // real-time seconds past the paused baseline under miri, putting the deadline beyond // any virtual-time advance the test performs. #[tokio::test(start_paused = true)] async fn test_flow_table_timeout() { - let now = tokio::time::Instant::now().into_std(); + let now = clock::now(); let two_seconds = Duration::from_secs(2); let one_second = Duration::from_secs(1); @@ -541,7 +538,7 @@ mod tests { #[tokio::test] async fn test_flow_table_entry_replaced_on_insert() { - let now = Instant::now(); + let now = clock::now(); let first_expiry_time = now + Duration::from_secs(5); let second_expiry_time = now + Duration::from_secs(10); @@ -596,7 +593,7 @@ mod tests { flow_table .insert(FlowInfo::new( flow_key, - Instant::now() + Duration::from_mins(1), + clock::now() + Duration::from_mins(1), )) .unwrap(); let flow_info = flow_table.lookup(&flow_key).unwrap(); @@ -626,7 +623,7 @@ mod tests { async fn test_flow_table_flow_invalidation() { const NUM_FLOWS: u16 = 10; let flow_table = FlowTable::default(); - let now = Instant::now(); + let now = clock::now(); let deadline = now + Duration::from_secs(3); let mut flow_keys = vec![]; @@ -669,7 +666,7 @@ mod tests { /// Test that invalidating flows causes timer to expire and flows to be removed async fn test_flow_table_flow_reinsertion() { let flow_table = FlowTable::default(); - let now = Instant::now(); + let now = clock::now(); let deadline = now + Duration::from_secs(2); let flow_key = FlowKey::new( @@ -710,7 +707,7 @@ mod tests { async fn an_active_flow_holds_its_key_against_a_second_insertion() { let flow_table = FlowTable::default(); let key = key_for(1025); - let far_future = Instant::now() + Duration::from_hours(1); + let far_future = clock::now() + Duration::from_hours(1); let first = Arc::new(FlowInfo::new(key, far_future)); assert!(matches!( @@ -734,7 +731,7 @@ mod tests { async fn a_flow_that_is_not_live_is_displaced() { let flow_table = FlowTable::default(); let key = key_for(1026); - let far_future = Instant::now() + Duration::from_hours(1); + let far_future = clock::now() + Duration::from_hours(1); let first = Arc::new(FlowInfo::new(key, far_future)); flow_table.insert_if_absent(&first).unwrap(); @@ -756,7 +753,7 @@ mod tests { async fn displacing_a_flow_invalidates_its_partner() { let flow_table = FlowTable::default(); let (forward_key, reverse_key) = (key_for(1027), key_for(1028)); - let far_future = Instant::now() + Duration::from_hours(1); + let far_future = clock::now() + Duration::from_hours(1); let (forward, reverse) = FlowInfo::related_pair( far_future, @@ -787,7 +784,7 @@ mod tests { let src_vpcd = VpcDiscriminant::VNI(Vni::new_checked(100).unwrap()); let src_ip: IpAddr = "1.2.3.4".parse().unwrap(); let dst_ip: IpAddr = "5.6.7.8".parse().unwrap(); - let far_future = Instant::now() + Duration::from_hours(1); + let far_future = clock::now() + Duration::from_hours(1); // Insert up to the capacity limit — both should succeed. for i in 1u16..=2 { @@ -828,7 +825,6 @@ mod tests { use crate::flow_table::FlowInfo; use concurrency::sync::Arc; use concurrency::thread; - use std::time::Instant; #[allow(clippy::too_many_lines)] #[concurrency::test] @@ -855,7 +851,7 @@ mod tests { let flow_table = Arc::new(FlowTable::default()); - let now = Instant::now(); + let now = clock::now(); // Insert the first flow let orig_flow_info = FlowInfo::new(flow_keys[0], now + two_seconds); @@ -954,7 +950,7 @@ mod tests { fn test_flow_table_reshard() { let flow_table = Arc::new(FlowTable::default()); - let five_seconds_from_now = Instant::now() + Duration::from_secs(5); + let five_seconds_from_now = clock::now() + Duration::from_secs(5); let flow_key1 = FlowKey::new( Some(VpcDiscriminant::VNI(Vni::new_checked(1).unwrap())), "1.2.3.4".parse::().unwrap(), diff --git a/flow-filter/Cargo.toml b/flow-filter/Cargo.toml index 8ebae286b9..156ca093d1 100644 --- a/flow-filter/Cargo.toml +++ b/flow-filter/Cargo.toml @@ -8,6 +8,7 @@ version.workspace = true [dependencies] # internal acl = { workspace = true } +clock = { workspace = true } common = { workspace = true } concurrency = { workspace = true } config = { workspace = true } @@ -25,6 +26,7 @@ linkme = { workspace = true } tracing = { workspace = true } [dev-dependencies] +clock = { workspace = true, features = ["virtual"] } # The reference (linear-scan) ACL backend is the differential-test oracle and drives the fast, # EAL-free semantic suite. It is `cfg(test)`-gated in the source, so it is never part of a # production build; this dev-dep just makes `acl::reference` available to test builds. diff --git a/flow-filter/src/tests.rs b/flow-filter/src/tests.rs index e4181c1558..4fc62869a1 100644 --- a/flow-filter/src/tests.rs +++ b/flow-filter/src/tests.rs @@ -14,6 +14,7 @@ use crate::test_utils::{ vpcd, }; use crate::{FlowFilter, LookupResult, NatRequirement}; +use clock::Duration; use concurrency::sync::Arc; use lpm::prefix::L4Protocol; use net::FlowKey; @@ -23,7 +24,6 @@ use net::headers::Headers; use net::packet::{DoneReason, Packet, VpcDiscriminant}; use net::parse::DeParse; use pipeline::{NetworkFunction, PipelineData}; -use std::time::{Duration, Instant}; // ------------------------------------------------------------------------------------------------- // Helpers @@ -51,7 +51,7 @@ fn create_flow_pair( nat_state: bool, port_fw_state: bool, ) -> (Arc, Arc) { - let expires_at = Instant::now() + Duration::from_secs(60); + let expires_at = clock::now() + Duration::from_secs(60); let (flow_info_fwd, flow_info_reply) = FlowInfo::related_pair(expires_at, flow_key, flags, reply_flow_key, reply_flags).unwrap(); diff --git a/nat/Cargo.toml b/nat/Cargo.toml index b2e69b14e7..59cf71c42c 100644 --- a/nat/Cargo.toml +++ b/nat/Cargo.toml @@ -14,6 +14,7 @@ shuttle_dfs = ["concurrency/shuttle_dfs", "shuttle"] ahash = { workspace = true, features = ["std"] } arc-swap = { workspace = true } bnum = { workspace = true } +clock = { workspace = true } common = { workspace = true } concurrency = { workspace = true, features = [] } config = { workspace = true } @@ -35,13 +36,14 @@ tracing = { workspace = true } shuttle = { workspace = true, optional = true } [dev-dependencies] +clock = { workspace = true, features = ["virtual"] } # internal config = { workspace = true, features = ["bolero"] } fixin = { workspace = true } test-utils = { workspace = true } lpm = { workspace = true, features = ["testing"] } net = { workspace = true, features = ["bolero"] } -tokio = { workspace = true, features = ["macros", "rt", "time"] } +tokio = { workspace = true, features = ["macros", "rt", "test-util", "time"] } tracectl = { workspace = true } # external diff --git a/nat/src/masquerade/expiry.rs b/nat/src/masquerade/expiry.rs new file mode 100644 index 0000000000..ccdd90ffdc --- /dev/null +++ b/nat/src/masquerade/expiry.rs @@ -0,0 +1,258 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +#![cfg(test)] + +use crate::Masquerade; +use crate::masquerade::probe::{Arrival, Fabric, run}; +use crate::static_nat::probe::build; +use clock::Duration; +use config::external::overlay::vpcpeering::VpcExpose; +use config::external::overlay::vpcpeering::contract::{LOCAL_VNI, REMOTE_VNI}; +use flow_entry::flow_table::FlowLookup; +use net::buffer::TestBuffer; +use net::packet::Packet; +use net::vxlan::Vni; +use std::net::IpAddr; + +const PAST_EXPIRY: Duration = Duration::from_secs(30); + +const WITHIN_LIFETIME: Duration = Duration::from_secs(1); + +fn vni(raw: u32) -> Vni { + Vni::new_checked(raw).unwrap_or_else(|_| unreachable!()) +} + +fn with_paused_clock>(body: impl FnOnce() -> F) { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_time() + .start_paused(true) + .build() + .unwrap_or_else(|e| unreachable!("{e}")); + runtime.block_on(body()); +} + +async fn advance(by: Duration) { + tokio::time::advance(by).await; + for _ in 0..4 { + tokio::task::yield_now().await; + } +} + +fn fabric() -> (Fabric, Vec) { + let exposes = vec![ + VpcExpose::empty() + .make_masquerade(None) + .unwrap_or_else(|e| unreachable!("{e}")) + .ip(lpm::prefix::PrefixWithOptionalPorts::new( + "10.0.0.0/24".parse().unwrap_or_else(|_| unreachable!()), + None, + )) + .as_range(lpm::prefix::PrefixWithOptionalPorts::new( + "172.16.0.0/24".parse().unwrap_or_else(|_| unreachable!()), + None, + )) + .unwrap_or_else(|e| unreachable!("{e}")), + ]; + let fabric = Fabric::build(&exposes).unwrap_or_else(|| unreachable!("a fixed expose builds")); + (fabric, exposes) +} + +fn open_flow( + lookup: &mut FlowLookup, + masq: &mut Masquerade, + source: IpAddr, + peer: IpAddr, + sport: u16, +) -> Option<(IpAddr, u16)> { + let mut packet = build(source, peer, false, sport, 80); + Arrival::outbound().stamp(&mut packet); + let out = run(lookup, masq, vec![packet], Some(vni(REMOTE_VNI))); + if out[0].is_done() { + return None; + } + let addr = out[0].ip_source()?; + let port = out[0].transport_src_port()?.get(); + (addr != source).then_some((addr, port)) +} + +fn reply_to( + lookup: &mut FlowLookup, + masq: &mut Masquerade, + peer: IpAddr, + translated: (IpAddr, u16), +) -> Option { + let mut packet = build(peer, translated.0, false, 80, translated.1); + Arrival::inbound().stamp(&mut packet); + let out: Vec> = run(lookup, masq, vec![packet], Some(vni(LOCAL_VNI))); + (!out[0].is_done()) + .then(|| out[0].ip_destination()) + .flatten() +} + +#[test] +fn a_flow_inside_its_lifetime_survives() { + with_paused_clock(|| async { + let (fabric, _) = fabric(); + let (mut lookup, mut masq) = fabric.stages(); + let peer = fabric.peer[0]; + let source: IpAddr = "10.0.0.7".parse().unwrap_or_else(|_| unreachable!()); + + let translated = open_flow(&mut lookup, &mut masq, source, peer, 1234) + .unwrap_or_else(|| unreachable!("a fixed private source is masqueraded")); + + advance(WITHIN_LIFETIME).await; + + assert_eq!( + reply_to(&mut lookup, &mut masq, peer, translated), + Some(source), + "a flow one second into a five second lifetime stopped answering" + ); + }); +} + +#[test] +fn a_flow_past_its_lifetime_stops_answering() { + with_paused_clock(|| async { + let (fabric, _) = fabric(); + let (mut lookup, mut masq) = fabric.stages(); + let peer = fabric.peer[0]; + let source: IpAddr = "10.0.0.7".parse().unwrap_or_else(|_| unreachable!()); + + let translated = open_flow(&mut lookup, &mut masq, source, peer, 1234) + .unwrap_or_else(|| unreachable!("a fixed private source is masqueraded")); + + advance(PAST_EXPIRY).await; + + let delivered = reply_to(&mut lookup, &mut masq, peer, translated); + assert_ne!( + delivered, + Some(source), + "an expired flow still delivered its reply, so the timeout is not enforced" + ); + assert!( + delivered.is_none(), + "the reply to an expired flow was forwarded to {delivered:?} instead of being dropped" + ); + }); +} + +#[test] +fn traffic_extends_a_flow_past_its_first_deadline() { + with_paused_clock(|| async { + let (fabric, _) = fabric(); + let (mut lookup, mut masq) = fabric.stages(); + let peer = fabric.peer[0]; + let source: IpAddr = "10.0.0.7".parse().unwrap_or_else(|_| unreachable!()); + + let translated = open_flow(&mut lookup, &mut masq, source, peer, 1234) + .unwrap_or_else(|| unreachable!("a fixed private source is masqueraded")); + + for _ in 0..4 { + advance(WITHIN_LIFETIME).await; + assert_eq!( + reply_to(&mut lookup, &mut masq, peer, translated), + Some(source), + "a refreshed flow stopped answering while still inside its extended lifetime" + ); + } + }); +} + +#[test] +fn an_expired_flow_is_never_resurrected() { + with_paused_clock(|| async { + let (fabric, _) = fabric(); + let (mut lookup, mut masq) = fabric.stages(); + let peer = fabric.peer[0]; + let first: IpAddr = "10.0.0.7".parse().unwrap_or_else(|_| unreachable!()); + let second: IpAddr = "10.0.0.9".parse().unwrap_or_else(|_| unreachable!()); + + let dead = open_flow(&mut lookup, &mut masq, first, peer, 1234) + .unwrap_or_else(|| unreachable!("a fixed private source is masqueraded")); + + advance(PAST_EXPIRY).await; + + let live = open_flow(&mut lookup, &mut masq, second, peer, 4321).unwrap_or_else(|| { + unreachable!( + "a flow opened after the clock advanced was refused; the deadline and the timer \ + are on different clocks again" + ) + }); + + assert_eq!( + reply_to(&mut lookup, &mut masq, peer, live), + Some(second), + "a flow opened after the clock advanced could not receive its reply" + ); + assert_ne!( + reply_to(&mut lookup, &mut masq, peer, dead), + Some(first), + "an expired flow answered again after a later flow had been created" + ); + }); +} + +#[test] +fn both_halves_of_a_pair_outlive_one_sided_traffic() { + with_paused_clock(|| async { + let (fabric, _) = fabric(); + let (mut lookup, mut masq) = fabric.stages(); + let peer = fabric.peer[0]; + let source: IpAddr = "10.0.0.7".parse().unwrap_or_else(|_| unreachable!()); + + let translated = open_flow(&mut lookup, &mut masq, source, peer, 1234) + .unwrap_or_else(|| unreachable!("a fixed private source is masqueraded")); + assert_eq!( + fabric.live_flows(), + 2, + "a masqueraded flow should install a forward and a reverse entry" + ); + + for elapsed in 1..=8 { + advance(WITHIN_LIFETIME).await; + assert_eq!( + reply_to(&mut lookup, &mut masq, peer, translated), + Some(source), + "the flow stopped answering at t={elapsed}s" + ); + assert_eq!( + fabric.live_flows(), + 2, + "at t={elapsed}s one half of the pair had expired under a live connection; the \ + forward half owns the allocation, so its tuple is now free to be reissued" + ); + } + }); +} + +#[test] +fn a_live_flows_tuple_is_never_reissued() { + with_paused_clock(|| async { + let (fabric, _) = fabric(); + let (mut lookup, mut masq) = fabric.stages(); + let peer = fabric.peer[0]; + let first: IpAddr = "10.0.0.10".parse().unwrap_or_else(|_| unreachable!()); + let second: IpAddr = "10.0.0.99".parse().unwrap_or_else(|_| unreachable!()); + + let held = open_flow(&mut lookup, &mut masq, first, peer, 2000) + .unwrap_or_else(|| unreachable!("a fixed private source is masqueraded")); + + for _ in 0..6 { + advance(WITHIN_LIFETIME).await; + assert_eq!( + reply_to(&mut lookup, &mut masq, peer, held), + Some(first), + "the flow being held open stopped answering" + ); + } + + let other = open_flow(&mut lookup, &mut masq, second, peer, 3000) + .unwrap_or_else(|| unreachable!("a second private source is masqueraded")); + assert_ne!( + other, held, + "{held:?} is held by a live flow from {first} and was reissued to {second}; replies \ + for {first} will be delivered to {second}" + ); + }); +} diff --git a/nat/src/masquerade/mod.rs b/nat/src/masquerade/mod.rs index 5e149bb06f..1e708dd93d 100644 --- a/nat/src/masquerade/mod.rs +++ b/nat/src/masquerade/mod.rs @@ -4,6 +4,7 @@ pub(crate) mod allocation; mod allocator_writer; pub mod apalloc; +mod expiry; pub(crate) mod flows; mod fuzz; pub(crate) mod icmp_handling; @@ -13,6 +14,7 @@ mod packet; mod probe; mod protocol; mod state; +mod state_machine; mod test; // re exports diff --git a/nat/src/masquerade/nf.rs b/nat/src/masquerade/nf.rs index 3edc15fb2f..a39f10a6b2 100644 --- a/nat/src/masquerade/nf.rs +++ b/nat/src/masquerade/nf.rs @@ -13,6 +13,7 @@ use crate::masquerade::flows::check_masquerading_flow; use crate::masquerade::packet::{NatPacketError, NatTranslate, masquerade}; use crate::masquerade::protocol::next_flow_status; use crate::masquerade::state::MasqueradeState; +use clock::Duration; use concurrency::sync::{Arc, Weak}; use config::GenId; use flow_entry::flow_table::table::{FlowTable, FlowTableError}; @@ -26,7 +27,6 @@ use net::{FlowKey, IpProtoKey}; use pipeline::{NetworkFunction, PipelineData}; use std::fmt::Debug; use std::net::IpAddr; -use std::time::{Duration, Instant}; #[allow(unused)] use tracing::{debug, error, warn}; @@ -180,14 +180,9 @@ impl Masquerade { } }; - // extend the duration of the flow according to the new status if let Some(extend_by) = extend_by { let _ = flow_info.reset_expiry_unchecked(extend_by); - // if we transition to established, let the related flow get the configured timeout too - if current != new_status - && new_status == NatFlowStatus::Established - && let Some(related) = flow_info.related.as_ref().and_then(Weak::upgrade) - { + if let Some(related) = flow_info.related.as_ref().and_then(Weak::upgrade) { let _ = related.reset_expiry_unchecked(extend_by); } } @@ -289,7 +284,7 @@ impl Masquerade { MasqueradeState::new_pair(alloc.allocation, src_ip, src_port, idle_timeout); // build a flow pair from the keys (without NAT state) - let expires_at = Instant::now() + Self::MASQUERADE_ONEWAY_TIMEOUT; + let expires_at = clock::now() + Self::MASQUERADE_ONEWAY_TIMEOUT; let (forward, reverse) = FlowInfo::related_pair( expires_at, *initial_flow_key, diff --git a/nat/src/masquerade/probe.rs b/nat/src/masquerade/probe.rs index b689e831df..49a9863438 100644 --- a/nat/src/masquerade/probe.rs +++ b/nat/src/masquerade/probe.rs @@ -92,6 +92,14 @@ impl Fabric { ) } + pub(crate) fn live_flows(&self) -> usize { + let count = concurrency::sync::atomic::AtomicUsize::new(0); + self.flow_table.for_each_flow_sharded(|_, _| { + count.fetch_add(1, concurrency::sync::atomic::Ordering::Relaxed); + }); + count.load(concurrency::sync::atomic::Ordering::Relaxed) + } + pub(crate) fn is_probeable(&self) -> bool { !self.private.is_empty() && !self.public.is_empty() } diff --git a/nat/src/masquerade/state_machine.rs b/nat/src/masquerade/state_machine.rs new file mode 100644 index 0000000000..d7616493b9 --- /dev/null +++ b/nat/src/masquerade/state_machine.rs @@ -0,0 +1,228 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +#![cfg(test)] + +use crate::common::{NatAction, NatFlowStatus}; +use crate::masquerade::protocol::next_flow_status; +use net::buffer::TestBuffer; +use net::headers::TryTcpMut; +use net::packet::Packet; +use net::packet::test_utils::{ + IcmpEchoDirection, build_test_icmp4_echo, build_test_tcp_ipv4_packet, + build_test_udp_ipv4_packet, +}; + +const STATUSES: [NatFlowStatus; 10] = [ + NatFlowStatus::OneWay, + NatFlowStatus::TwoWay, + NatFlowStatus::Established, + NatFlowStatus::Reset, + NatFlowStatus::CClosing, + NatFlowStatus::SClosing, + NatFlowStatus::CHalfClose, + NatFlowStatus::SHalfClose, + NatFlowStatus::LastAck, + NatFlowStatus::Closed, +]; + +#[allow(clippy::struct_excessive_bools)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct Flags { + syn: bool, + ack: bool, + fin: bool, + rst: bool, +} + +impl Flags { + fn from_bits(bits: u8) -> Self { + Self { + syn: bits & 0b1000 != 0, + ack: bits & 0b0100 != 0, + fin: bits & 0b0010 != 0, + rst: bits & 0b0001 != 0, + } + } +} + +fn tcp_packet(flags: Flags) -> Packet { + let mut packet = build_test_tcp_ipv4_packet("1.1.1.1", "2.2.2.2", 1024, 80); + { + let tcp = packet.try_tcp_mut().unwrap_or_else(|| unreachable!()); + tcp.set_syn(flags.syn); + tcp.set_ack(flags.ack); + tcp.set_fin(flags.fin); + tcp.set_rst(flags.rst); + } + packet +} + +fn udp_packet(source_port: u16) -> Packet { + build_test_udp_ipv4_packet("1.1.1.1", "2.2.2.2", source_port, 80) +} + +#[allow(clippy::match_same_arms)] +fn expected_tcp(action: NatAction, status: NatFlowStatus, f: Flags) -> NatFlowStatus { + use NatFlowStatus as S; + let progressed = match (action, status) { + (NatAction::SrcNat, S::TwoWay) if !f.syn && f.ack => Some(S::Established), + (NatAction::DstNat, S::OneWay) if f.syn && f.ack => Some(S::TwoWay), + + (NatAction::SrcNat, S::Established) if f.fin => Some(S::CClosing), + (NatAction::DstNat, S::Established) if f.fin => Some(S::SClosing), + + (NatAction::SrcNat, S::SClosing) if !f.fin && f.ack => Some(S::SHalfClose), + (NatAction::DstNat, S::CClosing) if !f.fin && f.ack => Some(S::CHalfClose), + + (NatAction::SrcNat, S::SClosing) if f.fin && f.ack => Some(S::LastAck), + (NatAction::DstNat, S::CClosing) if f.fin && f.ack => Some(S::LastAck), + + (NatAction::SrcNat, S::SHalfClose) if f.fin => Some(S::LastAck), + (NatAction::DstNat, S::CHalfClose) if f.fin => Some(S::LastAck), + + (_, S::LastAck) if f.ack => Some(S::Closed), + + _ => None, + }; + + match progressed { + Some(next) => next, + None if f.rst => S::Reset, + None => status, + } +} + +#[test] +fn the_tcp_state_machine_follows_the_close_sequence() { + for action in [NatAction::SrcNat, NatAction::DstNat] { + for status in STATUSES { + for bits in 0..16u8 { + let flags = Flags::from_bits(bits); + let packet = tcp_packet(flags); + let got = next_flow_status(&packet, action, status); + let want = expected_tcp(action, status, flags); + assert_eq!( + got, want, + "{action} from {status:?} with {flags:?}: expected {want:?}, got {got:?}" + ); + } + } + } +} + +#[test] +fn a_segment_with_no_flags_moves_nothing() { + let bare = Flags { + syn: false, + ack: false, + fin: false, + rst: false, + }; + for action in [NatAction::SrcNat, NatAction::DstNat] { + for status in STATUSES { + let packet = tcp_packet(bare); + assert_eq!( + next_flow_status(&packet, action, status), + status, + "{action} from {status:?} moved on a segment with no flags set" + ); + } + } +} + +#[test] +fn reset_and_closed_absorb() { + let rst = Flags { + syn: false, + ack: false, + fin: false, + rst: true, + }; + for action in [NatAction::SrcNat, NatAction::DstNat] { + for bits in 0..16u8 { + let packet = tcp_packet(Flags::from_bits(bits)); + assert_eq!( + next_flow_status(&packet, action, NatFlowStatus::Reset), + NatFlowStatus::Reset, + "a reset connection was revived" + ); + } + let packet = tcp_packet(rst); + assert_eq!( + next_flow_status(&packet, action, NatFlowStatus::Closed), + NatFlowStatus::Reset + ); + } +} + +#[test] +fn a_reply_from_a_resolver_closes_the_flow_at_once() { + for source_port in [53u16, 853, 8853] { + let packet = udp_packet(source_port); + assert_eq!( + next_flow_status(&packet, NatAction::DstNat, NatFlowStatus::OneWay), + NatFlowStatus::Closed, + "a reply from port {source_port} should close the flow" + ); + assert_eq!( + next_flow_status(&packet, NatAction::SrcNat, NatFlowStatus::TwoWay), + NatFlowStatus::Established, + "an outbound packet must not be closed by its own source port" + ); + } +} + +#[test] +fn ordinary_udp_opens_and_settles() { + let packet = udp_packet(12345); + assert_eq!( + next_flow_status(&packet, NatAction::DstNat, NatFlowStatus::OneWay), + NatFlowStatus::TwoWay, + "a reply makes a one-way flow two-way" + ); + assert_eq!( + next_flow_status(&packet, NatAction::SrcNat, NatFlowStatus::TwoWay), + NatFlowStatus::Established, + "and the next outbound packet establishes it" + ); + for status in [NatFlowStatus::Established, NatFlowStatus::Closed] { + assert_eq!( + next_flow_status(&packet, NatAction::SrcNat, status), + status, + "an established or closed udp flow does not move outbound" + ); + } +} + +#[test] +fn an_icmp_reply_makes_a_flow_two_way_and_nothing_more() { + let packet = build_test_icmp4_echo( + "1.1.1.1".parse().unwrap_or_else(|_| unreachable!()), + "2.2.2.2".parse().unwrap_or_else(|_| unreachable!()), + 1, + IcmpEchoDirection::Reply, + ) + .unwrap_or_else(|_| unreachable!()); + + assert_eq!( + next_flow_status(&packet, NatAction::DstNat, NatFlowStatus::OneWay), + NatFlowStatus::TwoWay, + "a reply must answer the request" + ); + + for status in STATUSES { + assert_eq!( + next_flow_status(&packet, NatAction::SrcNat, status), + status, + "an outbound icmp packet moved a flow in {status:?}" + ); + if status != NatFlowStatus::OneWay { + assert_eq!( + next_flow_status(&packet, NatAction::DstNat, status), + status, + "an inbound icmp packet moved a flow in {status:?}" + ); + } + } +} diff --git a/nat/src/masquerade/test.rs b/nat/src/masquerade/test.rs index 6dc22440c5..aa975a24d7 100644 --- a/nat/src/masquerade/test.rs +++ b/nat/src/masquerade/test.rs @@ -44,7 +44,7 @@ use pipeline::DynPipeline; use pipeline::NetworkFunction; use std::net::{IpAddr, Ipv4Addr}; use std::str::FromStr; -use std::time::{Duration, Instant}; +use std::time::Duration; use tracectl::get_trace_ctl; use tracing::debug; use tracing_test::traced_test; @@ -1679,7 +1679,7 @@ fn establish_tcp_connection(pipeline: &mut DynPipeline) { // check that flow timeouts "match" the ones configured, allowing for 5 second error (for the test) let flow_info_ack = output.meta().flow_info.as_ref().unwrap(); let related = flow_info_ack.related.as_ref().unwrap().upgrade().unwrap(); - let valid_until = (Instant::now() + timeout) + let valid_until = (clock::now() + timeout) .checked_sub(Duration::from_secs(5)) .unwrap(); assert!(flow_info_ack.expires_at() >= valid_until); diff --git a/nat/src/portfw/expiry.rs b/nat/src/portfw/expiry.rs new file mode 100644 index 0000000000..0c7275e9c6 --- /dev/null +++ b/nat/src/portfw/expiry.rs @@ -0,0 +1,163 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +#![cfg(test)] + +use crate::portfw::PortForwarder; +use crate::portfw::probe::{Arrival, Fabric, PAST_ANY_TIMEOUT, run}; +use crate::static_nat::probe::build; +use clock::Duration; +use config::external::overlay::vpcpeering::VpcExpose; +use flow_entry::flow_table::FlowLookup; +use lpm::prefix::{L4Protocol, PrefixWithOptionalPorts}; +use std::net::IpAddr; + +const WITHIN_LIFETIME: Duration = Duration::from_secs(1); + +fn with_paused_clock>(body: impl FnOnce() -> F) { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_time() + .start_paused(true) + .build() + .unwrap_or_else(|e| unreachable!("{e}")); + runtime.block_on(body()); +} + +async fn advance(by: Duration) { + tokio::time::advance(by).await; + for _ in 0..4 { + tokio::task::yield_now().await; + } +} + +fn fabric() -> Fabric { + let expose = VpcExpose::empty() + .make_port_forwarding(Some(Duration::from_secs(5)), Some(L4Protocol::Tcp)) + .unwrap_or_else(|e| unreachable!("{e}")) + .ip(PrefixWithOptionalPorts::new( + "10.0.0.0/30".parse().unwrap_or_else(|_| unreachable!()), + Some(lpm::prefix::PortRange::new(9000, 9003).unwrap_or_else(|_| unreachable!())), + )) + .as_range(PrefixWithOptionalPorts::new( + "172.16.0.0/30".parse().unwrap_or_else(|_| unreachable!()), + Some(lpm::prefix::PortRange::new(8000, 8003).unwrap_or_else(|_| unreachable!())), + )) + .unwrap_or_else(|e| unreachable!("{e}")); + Fabric::build(&[expose]).unwrap_or_else(|| unreachable!("a fixed expose builds")) +} + +fn forward( + lookup: &mut FlowLookup, + pfw: &mut PortForwarder, + peer: IpAddr, + published: (IpAddr, u16), +) -> Option<(IpAddr, u16)> { + let mut packet = build(peer, published.0, true, 1234, published.1); + Arrival::inbound().stamp(&mut packet); + let out = run(lookup, pfw, vec![packet], Arrival::inbound().dst_vpcd); + if out[0].is_done() { + return None; + } + let addr = out[0].ip_destination()?; + let port = out[0].transport_dst_port()?.get(); + ((addr, port) != published).then_some((addr, port)) +} + +#[test] +fn a_published_service_answers_while_time_passes() { + with_paused_clock(|| async { + let fabric = fabric(); + let (mut lookup, mut pfw) = fabric.stages(); + let peer = fabric.peer[0]; + let published = ( + "172.16.0.1" + .parse::() + .unwrap_or_else(|_| unreachable!()), + 8001, + ); + + for step in 0..6 { + assert!( + forward(&mut lookup, &mut pfw, peer, published).is_some(), + "the published service stopped answering at step {step}" + ); + advance(WITHIN_LIFETIME).await; + } + }); +} + +#[test] +fn a_service_re_established_after_expiry_reaches_the_same_backend() { + with_paused_clock(|| async { + let fabric = fabric(); + let (mut lookup, mut pfw) = fabric.stages(); + let peer = fabric.peer[0]; + let published = ( + "172.16.0.2" + .parse::() + .unwrap_or_else(|_| unreachable!()), + 8002, + ); + + let first = forward(&mut lookup, &mut pfw, peer, published) + .unwrap_or_else(|| unreachable!("a published tuple is forwarded")); + + advance(PAST_ANY_TIMEOUT).await; + + let again = forward(&mut lookup, &mut pfw, peer, published).unwrap_or_else(|| { + unreachable!( + "a published service could not be reached after its flow expired; the rule is \ + configuration and does not expire with the flow" + ) + }); + + assert_eq!( + first, again, + "{published:?} reached {first:?} and then, after its flow expired, {again:?}; a \ + published address moved between connections" + ); + }); +} + +#[test] +fn published_tuples_stay_distinct_across_an_expiry() { + with_paused_clock(|| async { + let fabric = fabric(); + let (mut lookup, mut pfw) = fabric.stages(); + let peer = fabric.peer[0]; + + let published: Vec<(IpAddr, u16)> = (0..4u8) + .map(|i| { + ( + format!("172.16.0.{i}") + .parse::() + .unwrap_or_else(|_| unreachable!()), + 8000 + u16::from(i), + ) + }) + .collect(); + + let before: Vec<_> = published + .iter() + .map(|p| forward(&mut lookup, &mut pfw, peer, *p)) + .collect(); + + advance(PAST_ANY_TIMEOUT).await; + + let after: Vec<_> = published + .iter() + .map(|p| forward(&mut lookup, &mut pfw, peer, *p)) + .collect(); + + assert_eq!( + before, after, + "the mapping from published tuples to backends changed across an expiry" + ); + let distinct: std::collections::BTreeSet<_> = after.iter().flatten().collect(); + assert_eq!( + distinct.len(), + after.iter().flatten().count(), + "two published tuples share a backend after an expiry: {after:?}" + ); + }); +} diff --git a/nat/src/portfw/flow_state.rs b/nat/src/portfw/flow_state.rs index 0b5c8c5677..dc6fd93757 100644 --- a/nat/src/portfw/flow_state.rs +++ b/nat/src/portfw/flow_state.rs @@ -240,23 +240,19 @@ pub(crate) fn refresh_port_fw_entry( let seconds = extend_by.as_secs(); - // refresh the flow. In general, we only refresh the flow in one direction ... if let Some(flow) = packet.meta_mut().flow_info.as_ref() { if flow.reset_expiry_unchecked(extend_by).is_ok() { debug!("Extended flow lifetime by {seconds}s"); } - // .. except if we transition to established, as that is a sound indication of legit traffic - if new_status == NatFlowStatus::Established && new_status != current_status { - flow.related - .as_ref() - .and_then(Weak::upgrade) - .inspect(|reverse| { - if reverse.reset_expiry_unchecked(extend_by).is_ok() { - debug!("Extended reverse-flow lifetime by {seconds}s"); - } - }); - } + flow.related + .as_ref() + .and_then(Weak::upgrade) + .inspect(|reverse| { + if reverse.reset_expiry_unchecked(extend_by).is_ok() { + debug!("Extended reverse-flow lifetime by {seconds}s"); + } + }); // update flow info generation flow.set_genid(genid); diff --git a/nat/src/portfw/fuzz.rs b/nat/src/portfw/fuzz.rs new file mode 100644 index 0000000000..a58d6390bb --- /dev/null +++ b/nat/src/portfw/fuzz.rs @@ -0,0 +1,384 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +#![cfg(test)] + +use crate::portfw::probe::{Arrival, Fabric, ProbeSpec, run}; +use bolero::{Driver, TypeGenerator, ValueGenerator}; +use concurrency::sync::atomic::{AtomicUsize, Ordering}; +use config::external::overlay::vpcpeering::VpcExpose; +use config::external::overlay::vpcpeering::contract::PortForwardingExposes; +use net::buffer::TestBuffer; +use net::packet::Packet; +use std::collections::BTreeMap; +use std::net::IpAddr; +use std::num::NonZero; + +const MAX_EXPOSES: u8 = 2; + +const PROBES: usize = 8; + +#[derive(Debug, Clone, Copy)] +struct Scenario { + strays: bool, +} + +impl ValueGenerator for Scenario { + type Output = (Vec, Vec); + + fn generate(&self, driver: &mut D) -> Option { + let exposes = PortForwardingExposes(MAX_EXPOSES).generate(driver)?; + let mut probes = Vec::with_capacity(PROBES); + for _ in 0..PROBES { + let mut probe = ProbeSpec::generate(driver)?; + if !self.strays { + probe.clear_stray(); + } + probes.push(probe); + } + Some((exposes, probes)) + } +} + +fn with_runtime(body: impl FnOnce()) { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_time() + .build() + .unwrap_or_else(|e| unreachable!("{e}")); + let _guard = runtime.enter(); + body(); +} + +fn fabric(exposes: &[VpcExpose]) -> Option { + let fabric = Fabric::build(exposes)?; + fabric.is_probeable().then_some(fabric) +} + +fn destination_of(packet: &Packet) -> (IpAddr, u16) { + ( + packet + .ip_destination() + .unwrap_or_else(|| unreachable!("a probe is always an ip packet")), + packet.transport_dst_port().map_or(0, NonZero::get), + ) +} + +fn source_of(packet: &Packet) -> (IpAddr, u16) { + ( + packet + .ip_source() + .unwrap_or_else(|| unreachable!("a probe is always an ip packet")), + packet.transport_src_port().map_or(0, NonZero::get), + ) +} + +#[derive(Default)] +struct Tally { + seen: AtomicUsize, + built: AtomicUsize, + reached: AtomicUsize, +} + +impl Tally { + fn report(&self, what: &str) { + let (seen, built, reached) = ( + self.seen.load(Ordering::Relaxed), + self.built.load(Ordering::Relaxed), + self.reached.load(Ordering::Relaxed), + ); + if seen == 0 { + return; + } + println!("{what}: {built}/{seen} configurations built, {reached} packets reached it"); + assert!( + built * 2 >= seen, + "only {built} of {seen} configurations built, so this checked much less than it looks \ + like it did" + ); + assert!( + reached > 0 && reached * 2 >= built, + "{reached} packets reached the {what} assertion across {built} configurations; this \ + property has gone vacuous" + ); + } +} + +fn forward( + fabric: &Fabric, + lookup: &mut flow_entry::flow_table::FlowLookup, + pfw: &mut crate::portfw::PortForwarder, + probe: &crate::portfw::probe::Probe, +) -> Option<(IpAddr, u16)> { + let before = probe.destination; + let out = run(lookup, pfw, vec![probe.packet()], probe.arrival.dst_vpcd); + let _ = fabric; + if out[0].is_done() { + return None; + } + let after = destination_of(&out[0]); + (after != before).then_some(after) +} + +#[test] +fn a_forwarded_packet_answers_as_the_published_tuple() { + let tally = Tally::default(); + + with_runtime(|| { + bolero::check!() + .with_generator(Scenario { strays: false }) + .cloned() + .for_each(|(exposes, probes): (Vec, Vec)| { + tally.seen.fetch_add(1, Ordering::Relaxed); + let Some(fabric) = fabric(&exposes) else { + return; + }; + tally.built.fetch_add(1, Ordering::Relaxed); + let (mut lookup, mut pfw) = fabric.stages(); + + for spec in &probes { + let probe = (*spec).resolve(&fabric); + let published = probe.destination; + let Some(translated) = forward(&fabric, &mut lookup, &mut pfw, &probe) else { + continue; + }; + + let back = run( + &mut lookup, + &mut pfw, + vec![probe.reply(translated)], + Arrival::outbound().dst_vpcd, + ); + assert_eq!( + source_of(&back[0]), + published, + "{published:?} was forwarded to {translated:?}, and the reply came back \ + as {:?} instead of the tuple the client used", + source_of(&back[0]) + ); + tally.reached.fetch_add(1, Ordering::Relaxed); + } + }); + }); + + tally.report("reversibility"); +} + +#[test] +fn a_forwarded_packet_lands_inside_the_published_target() { + let tally = Tally::default(); + + with_runtime(|| { + bolero::check!() + .with_generator(Scenario { strays: false }) + .cloned() + .for_each(|(exposes, probes): (Vec, Vec)| { + tally.seen.fetch_add(1, Ordering::Relaxed); + let Some(fabric) = fabric(&exposes) else { + return; + }; + tally.built.fetch_add(1, Ordering::Relaxed); + let (mut lookup, mut pfw) = fabric.stages(); + + for spec in &probes { + let probe = (*spec).resolve(&fabric); + let published = probe.destination; + let Some((addr, port)) = forward(&fabric, &mut lookup, &mut pfw, &probe) else { + continue; + }; + + assert!( + fabric.is_private(addr, port), + "{published:?} was forwarded to {addr}:{port}, which no rule names as a \ + target; that address never published this service" + ); + tally.reached.fetch_add(1, Ordering::Relaxed); + } + }); + }); + + tally.report("containment"); +} + +#[test] +fn distinct_published_tuples_reach_distinct_targets() { + let tally = Tally::default(); + + with_runtime(|| { + bolero::check!() + .with_generator(Scenario { strays: false }) + .cloned() + .for_each(|(exposes, _probes): (Vec, Vec)| { + tally.seen.fetch_add(1, Ordering::Relaxed); + let Some(fabric) = fabric(&exposes) else { + return; + }; + tally.built.fetch_add(1, Ordering::Relaxed); + let (mut lookup, mut pfw) = fabric.stages(); + + let mut taken: BTreeMap<(IpAddr, u16), (IpAddr, u16)> = BTreeMap::new(); + for (public, _private, tcp) in &fabric.rules { + let tcp = *tcp; + for (addr, port) in public.every() { + let mut packet = + crate::static_nat::probe::build(fabric.peer[0], addr, tcp, 1024, port); + Arrival::inbound().stamp(&mut packet); + let out = run( + &mut lookup, + &mut pfw, + vec![packet], + Arrival::inbound().dst_vpcd, + ); + if out[0].is_done() { + continue; + } + let after = destination_of(&out[0]); + if after == (addr, port) { + continue; + } + if let Some(previous) = taken.insert(after, (addr, port)) { + assert_eq!( + previous, + (addr, port), + "published tuples {previous:?} and {:?} both reach {after:?}, so \ + two services share one target", + (addr, port) + ); + } + tally.reached.fetch_add(1, Ordering::Relaxed); + } + } + }); + }); + + tally.report("injectivity"); +} + +#[test] +fn nothing_is_forwarded_that_was_not_published() { + let tally = Tally::default(); + + with_runtime(|| { + bolero::check!() + .with_generator(Scenario { strays: true }) + .cloned() + .for_each(|(exposes, probes): (Vec, Vec)| { + tally.seen.fetch_add(1, Ordering::Relaxed); + let Some(fabric) = fabric(&exposes) else { + return; + }; + tally.built.fetch_add(1, Ordering::Relaxed); + let (mut lookup, mut pfw) = fabric.stages(); + + for spec in &probes { + let probe = (*spec).resolve(&fabric); + if probe.asks_for_forwarding() && probe.published { + continue; + } + let (before, stray) = (probe.destination, probe.stray); + let out = run( + &mut lookup, + &mut pfw, + vec![probe.packet()], + probe.arrival.dst_vpcd, + ); + + if !out[0].is_done() { + assert_eq!( + destination_of(&out[0]), + before, + "a packet to {before:?} was forwarded although {stray:?} meant no rule \ + published it" + ); + } + tally.reached.fetch_add(1, Ordering::Relaxed); + } + }); + }); + + tally.report("permission"); +} + +#[test] +fn forwarding_touches_only_the_destination() { + let tally = Tally::default(); + + with_runtime(|| { + bolero::check!() + .with_generator(Scenario { strays: false }) + .cloned() + .for_each(|(exposes, probes): (Vec, Vec)| { + tally.seen.fetch_add(1, Ordering::Relaxed); + let Some(fabric) = fabric(&exposes) else { + return; + }; + tally.built.fetch_add(1, Ordering::Relaxed); + let (mut lookup, mut pfw) = fabric.stages(); + + for spec in &probes { + let probe = (*spec).resolve(&fabric); + let expected = (probe.source, probe.sport); + let out = run( + &mut lookup, + &mut pfw, + vec![probe.packet()], + probe.arrival.dst_vpcd, + ); + if out[0].is_done() { + continue; + } + + assert_eq!( + source_of(&out[0]), + expected, + "destination forwarding rewrote the source, so the reply has nowhere to go" + ); + tally.reached.fetch_add(1, Ordering::Relaxed); + } + }); + }); + + tally.report("frame"); +} + +#[test] +fn a_forwarded_flow_keeps_its_target() { + let tally = Tally::default(); + + with_runtime(|| { + bolero::check!() + .with_generator(Scenario { strays: false }) + .cloned() + .for_each(|(exposes, probes): (Vec, Vec)| { + tally.seen.fetch_add(1, Ordering::Relaxed); + let Some(fabric) = fabric(&exposes) else { + return; + }; + tally.built.fetch_add(1, Ordering::Relaxed); + let (mut lookup, mut pfw) = fabric.stages(); + + for spec in &probes { + let probe = (*spec).resolve(&fabric); + let Some(first) = forward(&fabric, &mut lookup, &mut pfw, &probe) else { + continue; + }; + let Some(second) = forward(&fabric, &mut lookup, &mut pfw, &probe) else { + panic!( + "the second packet of a flow to {:?} was not forwarded at all, though \ + the first reached {first:?}", + probe.destination + ) + }; + + assert_eq!( + first, second, + "a flow to {:?} reached {first:?} and then {second:?}, so its packets are \ + split across two backends", + probe.destination + ); + tally.reached.fetch_add(1, Ordering::Relaxed); + } + }); + }); + + tally.report("stability"); +} diff --git a/nat/src/portfw/mod.rs b/nat/src/portfw/mod.rs index 20d367e319..2b6bd43b66 100644 --- a/nat/src/portfw/mod.rs +++ b/nat/src/portfw/mod.rs @@ -3,11 +3,14 @@ //! Port forwarding +mod expiry; mod flow_state; +mod fuzz; pub(crate) mod icmp_handling; mod nf; mod packet; mod portfwtable; +mod probe; mod protocol; mod test; diff --git a/nat/src/portfw/nf.rs b/nat/src/portfw/nf.rs index af5262236f..dda2962df2 100644 --- a/nat/src/portfw/nf.rs +++ b/nat/src/portfw/nf.rs @@ -14,7 +14,6 @@ use net::ip::UnicastIpAddr; use net::packet::{DoneReason, Packet, VpcDiscriminant}; use pipeline::{NetworkFunction, PipelineData}; use std::num::NonZero; -use std::time::Instant; use crate::common::NatAction; use crate::portfw::flow_state::build_portfw_flow_keys; @@ -114,7 +113,7 @@ impl PortForwarder { }; // create a pair of related flow entries (outside the flow table). Timeout is set according to the rule matched - let timeout = Instant::now() + entry.init_timeout(); + let timeout = clock::now() + entry.init_timeout(); let Ok((fw_flow, rev_flow)) = FlowInfo::related_pair( timeout, fw_key, diff --git a/nat/src/portfw/probe.rs b/nat/src/portfw/probe.rs new file mode 100644 index 0000000000..60f88c9565 --- /dev/null +++ b/nat/src/portfw/probe.rs @@ -0,0 +1,315 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +#![cfg(test)] + +use crate::portfw::{PortForwarder, PortFwTableWriter, build_port_forwarding_configuration}; +use crate::static_nat::probe::{build, vni}; +use bolero::TypeGenerator; +use clock::Duration; +use concurrency::sync::Arc; +use config::external::overlay::vpcpeering::VpcExpose; +use config::external::overlay::vpcpeering::contract::{ + LOCAL_VNI, REMOTE_VNI, overlay_with_exposes, +}; +use flow_entry::flow_table::{FlowLookup, FlowTable}; +use lpm::prefix::Prefix; +use net::buffer::TestBuffer; +use net::packet::{Packet, VpcDiscriminant}; +use net::vxlan::Vni; +use pipeline::NetworkFunction; +use std::net::IpAddr; + +const FLOW_CAPACITY: usize = 4096; + +const ABSENT_VNI: u32 = 4_000; + +#[derive(Debug, Clone, Copy)] +pub(crate) struct Side { + pub(crate) prefix: Prefix, + pub(crate) first_port: u16, + pub(crate) last_port: u16, +} + +impl Side { + pub(crate) fn covers(&self, addr: IpAddr, port: u16) -> bool { + self.prefix.covers_addr(&addr) && port >= self.first_port && port <= self.last_port + } + + pub(crate) fn endpoint(&self, host: u16, port: u16) -> (IpAddr, u16) { + let span = self.span(); + let offset = u128::from(host) % span.max(1); + let addr = match self.prefix.as_address() { + IpAddr::V4(base) => IpAddr::V4( + u32::try_from(u128::from(base.to_bits()) + offset) + .unwrap_or_else(|_| unreachable!()) + .into(), + ), + IpAddr::V6(base) => IpAddr::V6((base.to_bits() + offset).into()), + }; + let ports = u32::from(self.last_port - self.first_port) + 1; + let port = self.first_port + u16::try_from(u32::from(port) % ports).unwrap_or(0); + (addr, port) + } + + fn span(&self) -> u128 { + let host_bits = match self.prefix.as_address() { + IpAddr::V4(_) => 32 - u32::from(self.prefix.length()), + IpAddr::V6(_) => 128 - u32::from(self.prefix.length()), + }; + 1u128 << host_bits.min(64) + } + + pub(crate) fn every(&self) -> Vec<(IpAddr, u16)> { + const CAP: usize = 256; + let hosts = u32::try_from(self.span()) + .unwrap_or(u32::from(u16::MAX)) + .max(1); + let ports = u32::from(self.last_port - self.first_port) + 1; + let total = u64::from(hosts) * u64::from(ports); + let stride = (total / CAP as u64).max(1); + + let mut out = Vec::new(); + let mut index = 0u64; + while index < total { + let host = u16::try_from(index / u64::from(ports)).unwrap_or(u16::MAX); + let port = u16::try_from(index % u64::from(ports)).unwrap_or(0); + out.push(self.endpoint(host, port)); + index += stride; + } + out + } +} + +pub(crate) struct Fabric { + flow_table: Arc, + writer: PortFwTableWriter, + pub(crate) rules: Vec<(Side, Side, bool)>, + pub(crate) peer: Vec, +} + +impl Fabric { + pub(crate) fn build(exposes: &[VpcExpose]) -> Option { + let overlay = overlay_with_exposes(exposes.to_vec()).ok()?; + let validated = overlay.validate().ok()?; + let ruleset = build_port_forwarding_configuration(validated.vpc_table()).ok()?; + + let mut writer = PortFwTableWriter::new(); + writer.update_table(&ruleset).ok()?; + + let rules: Vec<(Side, Side, bool)> = exposes + .iter() + .filter_map(|expose| { + let public = expose.nat.as_ref()?.as_range.first()?; + let private = expose.ips.first()?; + let (pub_ports, priv_ports) = (public.ports()?, private.ports()?); + let tcp = expose.nat.as_ref()?.proto != lpm::prefix::L4Protocol::Udp; + Some(( + Side { + prefix: public.prefix(), + first_port: pub_ports.start(), + last_port: pub_ports.end(), + }, + Side { + prefix: private.prefix(), + first_port: priv_ports.start(), + last_port: priv_ports.end(), + }, + tcp, + )) + }) + .collect(); + + let peer = match rules + .first() + .map(|(public, _, _)| public.prefix.as_address()) + { + Some(IpAddr::V6(_)) => vec![ + "2001:db8:ffff::1" + .parse() + .unwrap_or_else(|_| unreachable!()), + "2001:db8:ffff::2" + .parse() + .unwrap_or_else(|_| unreachable!()), + ], + _ => vec![ + "3.3.3.1".parse().unwrap_or_else(|_| unreachable!()), + "3.3.3.2".parse().unwrap_or_else(|_| unreachable!()), + ], + }; + + Some(Self { + flow_table: Arc::new(FlowTable::new(FLOW_CAPACITY)), + writer, + rules, + peer, + }) + } + + pub(crate) fn stages(&self) -> (FlowLookup, PortForwarder) { + ( + FlowLookup::new("flow-lookup", self.flow_table.clone()), + PortForwarder::new( + "port-forwarder", + self.writer.reader(), + self.flow_table.clone(), + ), + ) + } + + pub(crate) fn is_probeable(&self) -> bool { + !self.rules.is_empty() + } + + pub(crate) fn is_private(&self, addr: IpAddr, port: u16) -> bool { + self.rules + .iter() + .any(|(_, private, _)| private.covers(addr, port)) + } +} + +pub(crate) fn run( + lookup: &mut FlowLookup, + pfw: &mut PortForwarder, + packets: Vec>, + dst_vpcd: Option, +) -> Vec> { + let mut looked: Vec<_> = lookup.process(packets.into_iter()).collect(); + for packet in &mut looked { + packet.meta_mut().dst_vpcd = dst_vpcd.map(VpcDiscriminant::from_vni); + } + pfw.process(looked.into_iter()).collect() +} + +#[derive(Debug, Clone, Copy)] +pub(crate) struct Arrival { + pub(crate) src_vpcd: Option, + pub(crate) dst_vpcd: Option, + pub(crate) wants_port_forwarding: bool, +} + +impl Arrival { + pub(crate) fn inbound() -> Self { + Self { + src_vpcd: Some(vni(REMOTE_VNI)), + dst_vpcd: Some(vni(LOCAL_VNI)), + wants_port_forwarding: true, + } + } + + pub(crate) fn outbound() -> Self { + Self { + src_vpcd: Some(vni(LOCAL_VNI)), + dst_vpcd: Some(vni(REMOTE_VNI)), + wants_port_forwarding: true, + } + } + + pub(crate) fn stamp(self, packet: &mut Packet) { + let meta = packet.meta_mut(); + meta.src_vpcd = self.src_vpcd.map(VpcDiscriminant::from_vni); + meta.set_overlay(true); + meta.set_keep(true); + meta.set_port_forwarding(self.wants_port_forwarding); + } +} + +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) enum Stray { + DestinationNotPublished, + PortOutsideRange, + UnknownSourceVni, + NotAskedFor, +} + +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) struct ProbeSpec { + rule: u8, + host: u16, + port: u16, + peer: u8, + sport: u16, + stray: Option, +} + +pub(crate) struct Probe { + pub(crate) destination: (IpAddr, u16), + pub(crate) source: IpAddr, + pub(crate) sport: u16, + pub(crate) tcp: bool, + pub(crate) published: bool, + pub(crate) arrival: Arrival, + pub(crate) stray: Option, +} + +impl Probe { + pub(crate) fn asks_for_forwarding(&self) -> bool { + self.arrival.wants_port_forwarding && self.arrival.src_vpcd == Some(vni(REMOTE_VNI)) + } + + pub(crate) fn packet(&self) -> Packet { + let mut packet = build( + self.source, + self.destination.0, + self.tcp, + self.sport, + self.destination.1, + ); + self.arrival.stamp(&mut packet); + packet + } + + pub(crate) fn reply(&self, translated: (IpAddr, u16)) -> Packet { + let mut packet = build( + translated.0, + self.source, + self.tcp, + translated.1, + self.sport, + ); + Arrival::outbound().stamp(&mut packet); + packet + } +} + +impl ProbeSpec { + pub(crate) fn clear_stray(&mut self) { + self.stray = None; + } + + pub(crate) fn resolve(self, fabric: &Fabric) -> Probe { + let (public, _private, tcp) = fabric.rules[self.rule as usize % fabric.rules.len()]; + let mut arrival = Arrival::inbound(); + let mut destination = public.endpoint(self.host, self.port); + let source = fabric.peer[self.peer as usize % fabric.peer.len()]; + let mut published = true; + + match self.stray { + None => {} + Some(Stray::DestinationNotPublished) => { + destination = (fabric.peer[0], destination.1); + published = false; + } + Some(Stray::PortOutsideRange) => { + if public.last_port < u16::MAX { + destination = (destination.0, public.last_port + 1); + published = false; + } + } + Some(Stray::UnknownSourceVni) => arrival.src_vpcd = Some(vni(ABSENT_VNI)), + Some(Stray::NotAskedFor) => arrival.wants_port_forwarding = false, + } + + Probe { + destination, + source, + sport: self.sport.max(1), + tcp, + published, + arrival, + stray: self.stray, + } + } +} + +pub(crate) const PAST_ANY_TIMEOUT: Duration = Duration::from_mins(30); diff --git a/nat/src/portfw/test.rs b/nat/src/portfw/test.rs index 95c60451b1..d44e2e8736 100644 --- a/nat/src/portfw/test.rs +++ b/nat/src/portfw/test.rs @@ -240,7 +240,7 @@ mod nf_test { // strict lower bound (`expires_at >= before + timeout`) that is // independent of how long the rest of the test takes -- otherwise // slow test execution (e.g. under miri) eats into the tolerance. - let before_reply = std::time::Instant::now(); + let before_reply = clock::now(); let output = process_packet(&mut pipeline, reply); assert_eq!(output.ip_source().unwrap().to_string(), "70.71.72.73"); assert_eq!(output.ip_destination().unwrap().to_string(), "10.0.0.1"); @@ -255,7 +255,7 @@ mod nf_test { // process original packet again. It should be fast-natted let repeated = udp_packet_to_port_forward(); - let before_repeated = std::time::Instant::now(); + let before_repeated = clock::now(); let output = process_packet(&mut pipeline, repeated); assert_eq!(output.ip_source().unwrap().to_string(), "10.0.0.1"); assert_eq!(output.ip_destination().unwrap().to_string(), "192.168.1.2"); diff --git a/net/Cargo.toml b/net/Cargo.toml index bbda3162aa..5059c423fd 100644 --- a/net/Cargo.toml +++ b/net/Cargo.toml @@ -22,6 +22,7 @@ arrayvec = { workspace = true, features = ["serde", "std"] } bitflags = { workspace = true } bolero = { workspace = true, features = ["std"], optional = true } bytecheck = { workspace = true } +clock = { workspace = true } common = { workspace = true } concurrency = { workspace = true } derive_builder = { workspace = true, features = ["alloc"] } @@ -41,5 +42,7 @@ tokio-util = { workspace = true } tracing = { workspace = true } [dev-dependencies] +clock = { workspace = true, features = ["virtual"] } ahash = { workspace = true, features = ["no-rng"] } bolero = { workspace = true, features = ["std"] } +tokio = { workspace = true, features = ["macros", "rt", "test-util", "time"] } diff --git a/net/src/flows/display.rs b/net/src/flows/display.rs index 4617cbef16..84f0ee20ca 100644 --- a/net/src/flows/display.rs +++ b/net/src/flows/display.rs @@ -8,7 +8,6 @@ use super::flow_key::FlowKey; use concurrency::sync::Weak; use std::fmt::Display; -use std::time::Instant; impl Display for FlowKey { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -49,7 +48,7 @@ impl Display for FlowInfoLocked { impl Display for FlowInfo { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let expires_at = self.expires_at(); - let expires_in = expires_at.saturating_duration_since(Instant::now()); + let expires_in = expires_at.saturating_duration_since(clock::now()); let genid = self.genid(); let info = self.locked.read(); let has_related = self diff --git a/net/src/flows/flow_info.rs b/net/src/flows/flow_info.rs index b23e52b838..0f1a356c6e 100644 --- a/net/src/flows/flow_info.rs +++ b/net/src/flows/flow_info.rs @@ -397,12 +397,11 @@ impl FlowInfo { /// Returns `FlowInfoError::TimeoutUnchanged` if the new timeout is smaller than the current. /// pub fn reset_expiry_unchecked(&self, duration: Duration) -> Result<(), FlowInfoError> { - let current = self.expires_at(); - let new = Instant::now() + duration; - if new < current { + let new = clock::now() + duration; + let previous = self.expires_at.fetch_max(new, Ordering::Relaxed); + if previous > new { return Err(FlowInfoError::TimeoutUnchanged); } - self.expires_at.store(new, Ordering::Relaxed); Ok(()) } diff --git a/net/src/flows/flow_info_fuzz.rs b/net/src/flows/flow_info_fuzz.rs new file mode 100644 index 0000000000..5885f60e2e --- /dev/null +++ b/net/src/flows/flow_info_fuzz.rs @@ -0,0 +1,494 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +#![cfg(test)] + +use crate::FlowKey; +use crate::flows::FlowInfoFlags; +use crate::flows::flow_info::{FlowInfo, FlowInfoError, FlowStatus}; +use bolero::TypeGenerator; +use clock::Duration; +use std::net::IpAddr; + +#[derive(Debug, Clone, Copy, TypeGenerator)] +struct Millis(u16); + +impl Millis { + fn duration(self) -> Duration { + Duration::from_millis(u64::from(self.0)) + } +} + +#[derive(Debug, Clone, Copy, TypeGenerator)] +enum Status { + Active, + Cancelled, + Expired, + Detached, +} + +impl From for FlowStatus { + fn from(status: Status) -> Self { + match status { + Status::Active => FlowStatus::Active, + Status::Cancelled => FlowStatus::Cancelled, + Status::Expired => FlowStatus::Expired, + Status::Detached => FlowStatus::Detached, + } + } +} + +#[derive(Debug, Clone, Copy, TypeGenerator)] +enum Op { + ExtendChecked(Millis), + ExtendUnchecked(Millis), + ResetChecked(Millis), + ResetUnchecked(Millis), + SetStatus(Status), + Invalidate, + Advance(Millis), +} + +fn key(port: u16) -> FlowKey { + FlowKey::new( + None, + "10.0.0.1" + .parse::() + .unwrap_or_else(|_| unreachable!()), + "10.0.0.2" + .parse::() + .unwrap_or_else(|_| unreachable!()), + crate::IpProtoKey::Udp(crate::UdpProtoKey { + src_port: crate::udp::UdpPort::new_checked(port.max(1)) + .unwrap_or_else(|_| unreachable!()), + dst_port: crate::udp::UdpPort::new_checked(80).unwrap_or_else(|_| unreachable!()), + }), + ) +} + +fn flow() -> FlowInfo { + let info = FlowInfo::new(key(1024), clock::now() + Duration::from_secs(1)); + info.update_status(FlowStatus::Active); + info +} + +fn with_paused_clock>(body: impl FnOnce() -> F) { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_time() + .start_paused(true) + .build() + .unwrap_or_else(|e| unreachable!("{e}")); + runtime.block_on(body()); +} + +#[test] +fn expiry_never_moves_backwards() { + with_paused_clock(|| async { + bolero::check!() + .with_type::>() + .for_each(|ops: &Vec| { + let entry = flow(); + let mut high_water = entry.expires_at(); + + for op in ops.iter().take(32) { + apply(&entry, *op); + let now = entry.expires_at(); + assert!( + now >= high_water, + "{op:?} moved the expiry backwards, from {high_water:?} to {now:?}" + ); + high_water = now; + } + }); + }); +} + +fn apply(flow: &FlowInfo, op: Op) { + match op { + Op::ExtendChecked(d) => drop(flow.extend_expiry(d.duration())), + Op::ExtendUnchecked(d) => flow.extend_expiry_unchecked(d.duration()), + Op::ResetChecked(d) => drop(flow.reset_expiry(d.duration())), + Op::ResetUnchecked(d) => drop(flow.reset_expiry_unchecked(d.duration())), + Op::SetStatus(s) => drop(flow.update_status(s.into())), + Op::Invalidate => flow.invalidate(), + Op::Advance(_) => {} + } +} + +#[test] +fn a_refused_refresh_leaves_the_deadline_alone() { + with_paused_clock(|| async { + bolero::check!().with_type::<(Status, Millis)>().for_each( + |(status, millis): &(Status, Millis)| { + let entry = flow(); + entry.update_status((*status).into()); + let before = entry.expires_at(); + + if entry.reset_expiry(millis.duration()).is_err() { + assert_eq!( + entry.expires_at(), + before, + "reset_expiry refused for status {status:?} but moved the deadline anyway" + ); + } + if entry.extend_expiry(millis.duration()).is_err() { + assert_eq!( + entry.expires_at(), + before, + "extend_expiry refused for status {status:?} but moved the deadline anyway" + ); + } + }, + ); + }); +} + +#[test] +fn a_refresh_is_permitted_exactly_when_the_status_allows() { + with_paused_clock(|| async { + bolero::check!().with_type::<(Status, Millis)>().for_each( + |(status, millis): &(Status, Millis)| { + let status = FlowStatus::from(*status); + + let entry = flow(); + entry.update_status(status); + let reset = entry.reset_expiry(millis.duration()); + assert_eq!( + reset.is_ok() || matches!(reset, Err(FlowInfoError::TimeoutUnchanged)), + status == FlowStatus::Active, + "reset_expiry on a {status} flow returned {reset:?}" + ); + + let entry = flow(); + entry.update_status(status); + let extend = entry.extend_expiry(millis.duration()); + assert_eq!( + extend.is_ok(), + status != FlowStatus::Expired, + "extend_expiry on a {status} flow returned {extend:?}" + ); + }, + ); + }); +} + +#[test] +fn invalidating_is_idempotent_and_cancels_the_timer() { + with_paused_clock(|| async { + bolero::check!() + .with_type::() + .for_each(|status: &Status| { + let entry = flow(); + let started_active = FlowStatus::from(*status) == FlowStatus::Active; + entry.update_status((*status).into()); + + entry.invalidate(); + assert_eq!( + entry.status(), + FlowStatus::Cancelled, + "invalidating a {status:?} flow left it in {:?}", + entry.status() + ); + if started_active { + assert!( + entry.token.is_cancelled(), + "an active flow was invalidated without cancelling its timer, so its entry \ + lingers until the original deadline" + ); + } + + entry.invalidate(); + assert_eq!( + entry.status(), + FlowStatus::Cancelled, + "invalidating twice did not leave the flow cancelled" + ); + }); + }); +} + +#[test] +fn a_related_pair_refers_to_its_partner() { + with_paused_clock(|| async { + bolero::check!() + .with_type::<(u16, u16)>() + .for_each(|(a, b): &(u16, u16)| { + let (one, two) = (key(*a), key(b.wrapping_add(1))); + let built = FlowInfo::related_pair( + clock::now() + Duration::from_secs(1), + one, + FlowInfoFlags::INITIATOR, + two, + FlowInfoFlags::default(), + ); + + let Ok((first, second)) = built else { + assert_eq!(one, two, "a pair of distinct keys was refused"); + return; + }; + assert_ne!(one, two, "a pair of identical keys was accepted"); + + let first_partner = first + .related + .as_ref() + .and_then(concurrency::sync::Weak::upgrade) + .unwrap_or_else(|| panic!("a flow's partner did not upgrade")); + let second_partner = second + .related + .as_ref() + .and_then(concurrency::sync::Weak::upgrade) + .unwrap_or_else(|| panic!("a flow's partner did not upgrade")); + assert_eq!( + first_partner.flowkey(), + second.flowkey(), + "a flow's partner is not the other half of its pair" + ); + assert_eq!( + second_partner.flowkey(), + first.flowkey(), + "the pairing is not symmetric" + ); + + first.invalidate_pair(); + assert_eq!(first.status(), FlowStatus::Cancelled); + assert_eq!( + second.status(), + FlowStatus::Cancelled, + "invalidating one half of a pair left the other live, so half a translation \ + survives with no reverse" + ); + }); + }); +} + +#[test] +fn a_pair_needs_exactly_one_initiator() { + let both = |a: FlowInfoFlags, b: FlowInfoFlags| { + FlowInfo::related_pair(clock::now() + Duration::from_secs(1), key(1), a, key(2), b).is_err() + }; + assert!( + both(FlowInfoFlags::INITIATOR, FlowInfoFlags::INITIATOR), + "a pair with two initiators was accepted" + ); + assert!( + both(FlowInfoFlags::default(), FlowInfoFlags::default()), + "a pair with no initiator was accepted" + ); + assert!( + !both(FlowInfoFlags::INITIATOR, FlowInfoFlags::default()), + "a well-formed pair was refused" + ); +} + +#[test] +fn every_status_survives_its_byte() { + for status in [ + FlowStatus::Active, + FlowStatus::Cancelled, + FlowStatus::Expired, + FlowStatus::Detached, + ] { + let byte = u8::from(status); + assert_eq!( + FlowStatus::try_from(byte) + .unwrap_or_else(|e| panic!("{status} did not round trip: {e}")), + status + ); + } + bolero::check!().with_type::().for_each(|byte: &u8| { + assert_eq!( + FlowStatus::try_from(*byte).is_ok(), + *byte <= 3, + "byte {byte} parsed as a status it should not have" + ); + }); +} + +#[test] +fn the_unchecked_refreshes_move_the_deadline_exactly() { + with_paused_clock(|| async { + bolero::check!() + .with_type::<(Millis, Millis)>() + .for_each(|(a, b): &(Millis, Millis)| { + let (extend, reset) = (a.duration(), b.duration()); + + let subject = flow(); + let before = subject.expires_at(); + subject.extend_expiry_unchecked(extend); + assert_eq!( + subject.expires_at(), + before + extend, + "an unchecked extension must add exactly its duration" + ); + + let subject = flow(); + let target = clock::now() + reset; + if target < subject.expires_at() { + assert!( + matches!( + subject.reset_expiry_unchecked(reset), + Err(FlowInfoError::TimeoutUnchanged) + ), + "a reset that moves the deadline earlier must be refused" + ); + } else { + assert!(subject.reset_expiry_unchecked(reset).is_ok()); + assert_eq!( + subject.expires_at(), + target, + "an accepted reset must land on now + duration, exactly" + ); + } + + let subject = flow(); + let long = reset + Duration::from_secs(1); + assert!(subject.reset_expiry_unchecked(long).is_ok()); + let held = subject.expires_at(); + assert!( + subject.reset_expiry_unchecked(long).is_ok(), + "resetting to the deadline already held must be accepted, not refused" + ); + assert_eq!(subject.expires_at(), held, "and must leave it where it was"); + }); + }); +} + +#[test] +fn a_flow_is_active_exactly_when_its_status_says_so() { + with_paused_clock(|| async { + bolero::check!() + .with_type::() + .for_each(|status: &Status| { + let want = FlowStatus::from(*status); + let flow = flow(); + flow.update_status(want); + assert_eq!( + flow.is_active(), + want == FlowStatus::Active, + "is_active disagreed with the status it was asked about" + ); + }); + }); +} + +#[test] +fn a_flow_built_with_a_status_has_it() { + with_paused_clock(|| async { + bolero::check!() + .with_type::<(u16, Status)>() + .for_each(|(port, status): &(u16, Status)| { + let want = FlowStatus::from(*status); + let flow = FlowInfo::new_with_status( + key(*port), + clock::now() + Duration::from_secs(1), + want, + ); + assert_eq!( + flow.status(), + want, + "the status asked for was not the one built" + ); + }); + }); +} + +#[test] +fn a_genid_is_remembered_and_reaches_the_partner() { + with_paused_clock(|| async { + bolero::check!().with_type::<(u16, u16, i64)>().for_each( + |(a, b, genid): &(u16, u16, i64)| { + let (one, two) = (key(*a), key(b.wrapping_add(1))); + let Ok((first, second)) = FlowInfo::related_pair( + clock::now() + Duration::from_secs(1), + one, + FlowInfoFlags::INITIATOR, + two, + FlowInfoFlags::default(), + ) else { + return; + }; + + first.set_genid(*genid); + assert_eq!( + first.genid(), + *genid, + "a genid must read back as it was set" + ); + assert_ne!( + second.genid(), + *genid, + "setting one half's genid must not reach the other" + ); + + let paired = genid.wrapping_add(1); + first.set_genid_pair(paired); + assert_eq!(first.genid(), paired); + assert_eq!( + second.genid(), + paired, + "set_genid_pair must reach the partner, or the halves disagree about which \ + configuration they belong to" + ); + }, + ); + }); +} + +#[test] +fn each_flag_predicate_answers_for_its_own_bit() { + with_paused_clock(|| async { + bolero::check!() + .with_type::<(u16, u16, u8)>() + .for_each(|(a, b, bits): &(u16, u16, u8)| { + let flags = FlowInfoFlags::from_bits_truncate(*bits); + let (one, two) = (key(*a), key(b.wrapping_add(1))); + let Ok((first, _second)) = FlowInfo::related_pair( + clock::now() + Duration::from_secs(1), + one, + flags, + two, + FlowInfoFlags::default(), + ) else { + return; + }; + + let got = first.get_flags(); + assert_eq!(got, flags, "the flags read back must be the ones set"); + assert_eq!( + got.requires_static_nat_src(), + flags.contains(FlowInfoFlags::REQ_STATIC_NAT_SRC), + "the source predicate answered for the wrong bit" + ); + assert_eq!( + got.requires_static_nat_dst(), + flags.contains(FlowInfoFlags::REQ_STATIC_NAT_DST), + "the destination predicate answered for the wrong bit" + ); + assert_eq!( + got.is_initiator(), + flags.contains(FlowInfoFlags::INITIATOR), + "the initiator predicate answered for the wrong bit" + ); + }); + }); +} + +#[test] +fn the_destination_vpc_is_remembered() { + with_paused_clock(|| async { + bolero::check!() + .with_type::>() + .for_each(|vni: &Option| { + let want = vni + .and_then(|v| crate::vxlan::Vni::new_checked(v % 0x00FF_FFFF).ok()) + .map(crate::packet::VpcDiscriminant::from_vni); + let flow = flow(); + flow.locked.write().dst_vpcd = want; + assert_eq!( + flow.get_dst_vpcd(), + want, + "the destination vpc read back must be the one stamped" + ); + }); + }); +} diff --git a/net/src/flows/mod.rs b/net/src/flows/mod.rs index 32f8d6e426..ff56d41bb7 100644 --- a/net/src/flows/mod.rs +++ b/net/src/flows/mod.rs @@ -7,6 +7,7 @@ pub mod atomic_instant; pub mod flow_info; +pub mod flow_info_fuzz; pub mod flow_info_item; pub mod display; diff --git a/routing/Cargo.toml b/routing/Cargo.toml index b24f21a943..90e72b2c48 100644 --- a/routing/Cargo.toml +++ b/routing/Cargo.toml @@ -15,6 +15,7 @@ testing = [] # internal args = { workspace = true } cli = { workspace = true } +clock = { workspace = true } common = { workspace = true } config = { workspace = true } concurrency = { workspace = true } @@ -55,6 +56,7 @@ procfs = { workspace = true } netdev = { workspace = true } [dev-dependencies] +clock = { workspace = true, features = ["virtual"] } bolero = { workspace = true, default-features = false } concurrency = { workspace = true } lpm = { workspace = true, features = ["testing"] } diff --git a/routing/src/bmp/bmp_render.rs b/routing/src/bmp/bmp_render.rs index 1aa81e582a..10cadd0c64 100644 --- a/routing/src/bmp/bmp_render.rs +++ b/routing/src/bmp/bmp_render.rs @@ -308,7 +308,7 @@ fn on_peer_down( if prev_state == BgpNeighborSessionState::Established { neigh.connections_dropped = neigh.connections_dropped.saturating_add(1); neigh.last_reset_reason = Some(pretty(pd.reason().get_type())); - neigh.last_reset_time = Some(std::time::Instant::now()); + neigh.last_reset_time = Some(clock::now()); } else { // we should not get to see this message with current FRR and RFC 7854 // Peer down events are only produced when leaving Established state diff --git a/routing/src/cli/display.rs b/routing/src/cli/display.rs index a5c54cb9c8..1c853b827e 100644 --- a/routing/src/cli/display.rs +++ b/routing/src/cli/display.rs @@ -34,6 +34,7 @@ use crate::evpn::{RmacEntry, RmacStore, Vtep}; use chrono::DateTime; use common::cliprovider::{Heading, line}; +use clock::Instant; use lpm::prefix::{IpPrefix, Ipv4Prefix, Ipv6Prefix}; use lpm::trie::{PrefixMapTrie, TrieMap}; use net::vxlan::Vni; @@ -42,7 +43,6 @@ use std::fmt::Write; use std::os::unix::net::SocketAddr; use std::rc::{Rc, Weak}; use std::time::Duration; -use std::time::Instant; use tracing::{error, warn}; diff --git a/routing/src/evpn/rmac.rs b/routing/src/evpn/rmac.rs index 19c055f777..23f33efd08 100644 --- a/routing/src/evpn/rmac.rs +++ b/routing/src/evpn/rmac.rs @@ -133,7 +133,7 @@ impl RmacStore { entry.mac, entry.vni, entry.address, ); // recall time when it became stale - current.stale_t = Some(Instant::now()); + current.stale_t = Some(clock::now()); self.stale = self.stale.saturating_add(1); } } diff --git a/routing/src/fib/test.rs b/routing/src/fib/test.rs index e2e9ec8ca1..347a90ce79 100644 --- a/routing/src/fib/test.rs +++ b/routing/src/fib/test.rs @@ -30,7 +30,6 @@ mod tests { use rand::RngExt; use rand::rngs::ThreadRng; use std::str::FromStr; - use std::time::Instant; use std::{collections::HashMap, collections::HashSet, sync::atomic::Ordering}; use crate::fib::fibgroupstore::tests::build_fib_entry_egress; @@ -348,7 +347,7 @@ mod tests { fibw.register_fibgroup(&nhkey, fibgroup, true); fibw.add_fibroute(prefix, vec![nhkey.clone()], true); } - let start = Instant::now(); + let start = clock::now(); loop { if fibw.is_none() { fibw = Some(fibtw.add_fib(vrfid, None)); diff --git a/routing/src/frr/frrmi.rs b/routing/src/frr/frrmi.rs index a359e6cf5a..c1165a47f6 100644 --- a/routing/src/frr/frrmi.rs +++ b/routing/src/frr/frrmi.rs @@ -154,7 +154,7 @@ impl Frrmi { revent!(RouterEvent::FrrmiDisconnected); } pub(crate) fn timeout(&mut self) { - if self.timeout.take_if(|t| *t < Instant::now()).is_some() { + if self.timeout.take_if(|t| *t < clock::now()).is_some() { warn!("Request sent to frr-agent timed out! Will reconnect..."); self.disconnect(); } @@ -272,7 +272,7 @@ impl Frrmi { debug!("Sending config request to frr-agent for gen {genid}..."); Self::send(sock, &mut self.writeb)?; debug!("FRR config request for gen {genid} successfully sent"); - self.timeout = Instant::now().checked_add(Self::TIMEOUT); + self.timeout = clock::now().checked_add(Self::TIMEOUT); Ok(()) } diff --git a/routing/src/rib/vrf.rs b/routing/src/rib/vrf.rs index ee7561b47c..92c2f2777e 100644 --- a/routing/src/rib/vrf.rs +++ b/routing/src/rib/vrf.rs @@ -77,7 +77,7 @@ impl Default for Route { distance: 0, metric: 0, s_nhops: Vec::with_capacity(1), - tstamp: Instant::now(), + tstamp: clock::now(), } } } @@ -817,7 +817,7 @@ pub mod tests { distance, metric, s_nhops: vec![], - tstamp: Instant::now(), + tstamp: clock::now(), } } diff --git a/routing/src/router/cpi.rs b/routing/src/router/cpi.rs index 08f0fc1bbe..96f1063411 100644 --- a/routing/src/router/cpi.rs +++ b/routing/src/router/cpi.rs @@ -25,7 +25,7 @@ use net::interface::InterfaceIndex; use net::interface::address::IfAddr; use std::os::unix::net::SocketAddr; use std::process; -use std::time::{SystemTime, UNIX_EPOCH}; +use std::time::UNIX_EPOCH; #[allow(unused)] use tracing::{debug, error, info, trace, warn}; @@ -104,7 +104,7 @@ pub(crate) struct CpiStats { impl CpiStats { pub(crate) fn new() -> CpiStats { Self { - synt: SystemTime::now() + synt: clock::system_now() .duration_since(UNIX_EPOCH) .expect("System time is wrong!") .as_secs(), diff --git a/routing/src/router/rio.rs b/routing/src/router/rio.rs index e57b4fdbfd..a661ef7ff4 100644 --- a/routing/src/router/rio.rs +++ b/routing/src/router/rio.rs @@ -379,14 +379,10 @@ impl Rio { let duration = 60; debug!("Set stale timeout ({duration} seconds)"); let duration = Duration::from_secs(duration); - self.stale_timeout = Instant::now().checked_add(duration); + self.stale_timeout = clock::now().checked_add(duration); } fn check_stale_timeout(&mut self, db: &mut RoutingDb) { - if self - .stale_timeout - .take_if(|t| *t < Instant::now()) - .is_some() - { + if self.stale_timeout.take_if(|t| *t < clock::now()).is_some() { info!("Stale timeout expired"); db.vrftable.remove_stale_routes(&db.rmac_store); db.vrftable.remove_deleted_vrfs(&mut db.iftw); @@ -572,14 +568,35 @@ pub(crate) fn start_rio( #[cfg(test)] mod tests { use crate::atable::atablerw::AtableWriter; + use crate::config::{FrrConfig, RouterConfig}; use crate::errors::RouterError; use crate::fib::fibtable::FibTableWriter; + use crate::fib::fibtype::FibKey; use crate::interfaces::iftablerw::IfTableWriter; - use crate::router::rio::{RioConf, start_rio}; + use crate::rib::vrf::{RouterVrfConfig, VrfStatus}; + use crate::router::cpi::CpiStatus; + use crate::router::rio::{Rio, RioConf, RioHandle, start_rio}; + use crate::routingdb::RoutingDb; + use cli::cliproto::{CliAction, CliRequest, CliResponse, RequestArgs}; + use concurrency::sync::atomic::{AtomicUsize, Ordering}; use concurrency::thread; + use config::GenId; + use dplane_rpc::msg::{ + ConnectInfo, IpRoute, RouteType, RpcMsg, RpcObject, RpcOp, RpcRequest, RpcResultCode, + VerInfo, + }; + use dplane_rpc::wire::Wire; use lifecycle::{CancellationToken, Subsystem}; + use std::io::{Read, Write}; + use std::os::unix::net::{UnixDatagram, UnixListener, UnixStream}; + use std::path::Path; use std::time::Duration; + const PATIENCE: Duration = Duration::from_secs(cfg_select! { + instrumented => 120, + _ => 10, + }); + fn test_router_subsystem() -> Subsystem { Subsystem::new("router", CancellationToken::new()) } @@ -587,18 +604,8 @@ mod tests { #[test] #[cfg_attr(emulated, ignore = "binds Unix domain sockets at /tmp/hh_*.sock")] fn test_rio_ctl() { - let cpi_bind_addr = "/tmp/hh_dataplane.sock".to_string(); - let cli_bind_addr = "/tmp/hh_cli.sock".to_string(); - let frra_path = "/tmp/frr-agent.sock".to_string(); - let _ = std::fs::remove_file(&cpi_bind_addr); - - /* Build cpi configuration */ - let conf = RioConf { - name: "test-routter".to_string(), - cpi_sock_path: Some(cpi_bind_addr), - cli_sock_path: Some(cli_bind_addr), - frrmi_sock_path: Some(frra_path), - }; + let dir = SockDir::new(); + let conf = dir.conf(); /* create interface table */ let (iftw, _iftr) = IfTableWriter::new(); @@ -641,4 +648,870 @@ mod tests { let rio = start_rio(&router, &conf, fibtw, iftw, atabler, None); assert!(rio.is_err_and(|e| matches!(e, RouterError::InvalidPath(_)))); } + + struct SockDir(std::path::PathBuf); + impl SockDir { + fn new() -> Self { + static NEXT: AtomicUsize = AtomicUsize::new(0); + let dir = std::env::temp_dir().join(format!( + "hh-rio-{}-{}", + std::process::id(), + NEXT.fetch_add(1, Ordering::Relaxed) + )); + std::fs::create_dir_all(&dir).expect("temp dir for test sockets"); + Self(dir) + } + fn path(&self, name: &str) -> String { + self.0.join(name).to_string_lossy().into_owned() + } + fn conf(&self) -> RioConf { + RioConf { + name: "rio-under-test".to_string(), + cpi_sock_path: Some(self.path("cpi.sock")), + cli_sock_path: Some(self.path("cli.sock")), + frrmi_sock_path: Some(self.path("frr-agent.sock")), + } + } + } + impl Drop for SockDir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + fn rio_for_test() -> (Rio, SockDir) { + let dir = SockDir::new(); + let rio = Rio::new(&dir.conf()).expect("rio should build on fresh socket paths"); + (rio, dir) + } + + struct TestDb { + db: RoutingDb, + #[allow(dead_code)] + held: ( + crate::interfaces::iftablerw::IfTableReader, + crate::fib::fibtable::FibTableReader, + crate::atable::atablerw::AtableWriter, + ), + } + fn db_for_test() -> TestDb { + let (iftw, iftr) = IfTableWriter::new(); + let (fibtw, fibtr) = FibTableWriter::new(); + let (atablew, atabler) = AtableWriter::new(); + TestDb { + db: RoutingDb::new(fibtw, iftw, atabler), + held: (iftr, fibtr, atablew), + } + } + + const STALE_WINDOW: Duration = Duration::from_mins(1); + + #[tokio::test(start_paused = true)] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + async fn arming_the_stale_timeout_sweeps_nothing() { + let (mut rio, _dir) = rio_for_test(); + let mut t = db_for_test(); + + assert!(rio.stale_timeout.is_none(), "starts unarmed"); + rio.set_stale_timeout(); + assert!(rio.stale_timeout.is_some(), "arming records a deadline"); + + rio.check_stale_timeout(&mut t.db); + assert!( + rio.stale_timeout.is_some(), + "a check at arm time must not consume the deadline" + ); + } + + #[tokio::test(start_paused = true)] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + async fn the_stale_timeout_survives_its_own_deadline() { + let (mut rio, _dir) = rio_for_test(); + let mut t = db_for_test(); + + rio.set_stale_timeout(); + tokio::time::advance(STALE_WINDOW).await; + + rio.check_stale_timeout(&mut t.db); + assert!( + rio.stale_timeout.is_some(), + "at exactly the deadline the window is still open" + ); + + tokio::time::advance(Duration::from_nanos(1)).await; + rio.check_stale_timeout(&mut t.db); + assert!( + rio.stale_timeout.is_none(), + "one nanosecond later it must close" + ); + } + + #[tokio::test(start_paused = true)] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + async fn the_stale_timeout_fires_once_and_then_disarms() { + let (mut rio, _dir) = rio_for_test(); + let mut t = db_for_test(); + + rio.set_stale_timeout(); + tokio::time::advance(STALE_WINDOW + Duration::from_secs(1)).await; + + rio.check_stale_timeout(&mut t.db); + assert!(rio.stale_timeout.is_none(), "expiry consumes the deadline"); + + for _ in 0..3 { + tokio::time::advance(STALE_WINDOW).await; + rio.check_stale_timeout(&mut t.db); + assert!(rio.stale_timeout.is_none(), "it must not re-arm itself"); + } + } + + #[tokio::test(start_paused = true)] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + async fn an_unarmed_stale_timeout_never_fires() { + let (mut rio, _dir) = rio_for_test(); + let mut t = db_for_test(); + + for _ in 0..4 { + tokio::time::advance(STALE_WINDOW * 10).await; + rio.check_stale_timeout(&mut t.db); + assert!(rio.stale_timeout.is_none()); + } + } + + #[tokio::test(start_paused = true)] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + async fn a_deleted_vrf_outlives_the_window_and_no_longer() { + let (mut rio, _dir) = rio_for_test(); + let mut t = db_for_test(); + + let cfg = RouterVrfConfig::new(909, "doomed"); + t.db.vrftable.add_vrf(&cfg).expect("vrf should be created"); + assert_eq!(t.db.vrftable.len(), 2, "the default vrf plus ours"); + t.db.vrftable + .get_vrf_mut(909) + .expect("just created") + .set_status(VrfStatus::Deleted); + + rio.set_stale_timeout(); + tokio::time::advance(STALE_WINDOW).await; + rio.check_stale_timeout(&mut t.db); + assert_eq!( + t.db.vrftable.len(), + 2, + "a deleted vrf is held for the whole window" + ); + + tokio::time::advance(Duration::from_secs(1)).await; + rio.check_stale_timeout(&mut t.db); + assert_eq!( + t.db.vrftable.len(), + 1, + "and is swept when the window closes" + ); + } + + #[tokio::test(start_paused = true)] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + async fn an_frr_restart_opens_the_stale_window() { + let (mut rio, _dir) = rio_for_test(); + let mut t = db_for_test(); + + let cfg = RouterVrfConfig::new(910, "half-deleted"); + t.db.vrftable.add_vrf(&cfg).expect("vrf should be created"); + t.db.vrftable + .get_vrf_mut(910) + .expect("just created") + .set_status(VrfStatus::Deleting); + + rio.cpistats.status = CpiStatus::FrrRestarted; + rio.cpi_status_check(&mut t.db); + + assert!( + rio.stale_timeout.is_some(), + "the restart must open the stale window" + ); + assert!( + rio.cpistats.status == CpiStatus::Connected, + "and leave the link healthy again" + ); + assert_eq!( + t.db.vrftable.len(), + 1, + "a vrf that was mid-deletion goes immediately, not on the timeout" + ); + } + + #[tokio::test(start_paused = true)] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + async fn a_refresh_with_no_peer_to_ask_is_not_marked_done() { + let (mut rio, _dir) = rio_for_test(); + let mut t = db_for_test(); + + assert!(rio.cpistats.peer.is_none(), "no peer has connected"); + rio.cpistats.status = CpiStatus::NeedRefresh; + rio.cpi_status_check(&mut t.db); + + assert!( + rio.cpistats.status == CpiStatus::NeedRefresh, + "with no peer to ask, the refresh stays outstanding" + ); + assert!( + rio.stale_timeout.is_none(), + "and no stale window is opened: our own restart leaves nothing stale" + ); + } + + #[tokio::test(start_paused = true)] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + async fn the_settled_cpi_states_do_nothing() { + for status in [ + CpiStatus::NotConnected, + CpiStatus::Connected, + CpiStatus::Incompatible, + ] { + let (mut rio, _dir) = rio_for_test(); + let mut t = db_for_test(); + let cfg = RouterVrfConfig::new(911, "bystander"); + t.db.vrftable.add_vrf(&cfg).expect("vrf should be created"); + + rio.cpistats.status = status; + for _ in 0..3 { + rio.cpi_status_check(&mut t.db); + } + + assert!( + rio.stale_timeout.is_none(), + "a settled state must not open the stale window" + ); + assert!(rio.cpistats.status == status, "nor change the status"); + assert_eq!(t.db.vrftable.len(), 2, "nor touch the vrf table"); + } + } + + struct CpiPeer { + sock: UnixDatagram, + rio: std::os::unix::net::SocketAddr, + } + impl CpiPeer { + fn attach(dir: &SockDir) -> Self { + let sock = UnixDatagram::bind(dir.path("peer.sock")).expect("peer sock should bind"); + sock.set_read_timeout(Some(PATIENCE)) + .expect("read timeout should be settable"); + let _ = &sock; + let rio = std::os::unix::net::SocketAddr::from_pathname(dir.path("cpi.sock")) + .expect("rio's cpi path should be addressable"); + Self { sock, rio } + } + fn send(&self, msg: &RpcMsg) { + dplane_rpc::socks::send_msg(&self.sock, msg, &self.rio).expect("send to rio"); + } + fn send_raw(&self, bytes: &[u8]) { + self.sock + .send_to_addr(bytes, &self.rio) + .expect("raw send to rio"); + } + fn expect_silence(&self, patience: Duration) { + self.sock + .set_read_timeout(Some(patience)) + .expect("read timeout should be settable"); + let mut buf = [0u8; 4096]; + let outcome = self.sock.recv_from(&mut buf); + self.sock + .set_read_timeout(Some(PATIENCE)) + .expect("read timeout should be settable"); + assert!( + outcome.is_err(), + "expected no answer at all, got {} bytes", + outcome.map_or(0, |(len, _)| len) + ); + } + fn recv(&self) -> RpcMsg { + let mut buf = [0u8; 4096]; + let (len, _) = self + .sock + .recv_from(&mut buf) + .expect("rio should answer within the read timeout"); + let mut data = bytes::Bytes::copy_from_slice(&buf[..len]); + RpcMsg::decode(&mut data).expect("rio should answer with a well-formed message") + } + fn connect_request(seqn: u64, pid: u32) -> RpcMsg { + RpcMsg::Request(RpcRequest::new(RpcOp::Connect, seqn).set_object( + RpcObject::ConnectInfo(ConnectInfo { + pid, + name: "test-plugin".to_string(), + verinfo: VerInfo::default(), + synt: 0, + }), + )) + } + fn route_request(op: RpcOp, seqn: u64) -> RpcMsg { + RpcMsg::Request( + RpcRequest::new(op, seqn).set_object(RpcObject::IpRoute(IpRoute { + prefix: "10.0.0.0".parse().expect("literal"), + prefix_len: 24, + vrfid: 0, + tableid: 254, + rtype: RouteType::Bgp, + distance: 20, + metric: 100, + nhops: vec![], + })), + ) + } + } + + struct RunningRio { + handle: RioHandle, + dir: SockDir, + #[allow(dead_code)] + held: ( + crate::interfaces::iftablerw::IfTableReader, + crate::fib::fibtable::FibTableReader, + crate::atable::atablerw::AtableWriter, + ), + } + impl RunningRio { + fn start() -> Self { + let dir = SockDir::new(); + let (iftw, iftr) = IfTableWriter::new(); + let (fibtw, fibtr) = FibTableWriter::new(); + let (atablew, atabler) = AtableWriter::new(); + let handle = start_rio( + &test_router_subsystem(), + &dir.conf(), + fibtw, + iftw, + atabler, + None, + ) + .expect("rio should start on fresh socket paths"); + Self { + handle, + dir, + held: (iftr, fibtr, atablew), + } + } + } + impl RunningRio { + fn attend_cpi(&self) { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("a runtime to drive the ctl channel") + .block_on(self.handle.get_ctl_tx().unlock()) + .expect("the cpi should unlock"); + } + } + impl Drop for RunningRio { + fn drop(&mut self) { + let _ = self.handle.finish(); + } + } + + #[test] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + fn the_cpi_is_not_attended_until_it_is_unlocked() { + let rio = RunningRio::start(); + let peer = CpiPeer::attach(&rio.dir); + + peer.send(&CpiPeer::connect_request(1, 4242)); + peer.expect_silence(Duration::from_secs(2)); + + rio.attend_cpi(); + let RpcMsg::Response(deferred) = peer.recv() else { + panic!("an attended cpi should answer what it deferred"); + }; + assert_eq!( + deferred.seqn, 1, + "the request sent while deaf is served, not discarded" + ); + assert_eq!(deferred.rescode, RpcResultCode::Ok); + + peer.send(&CpiPeer::connect_request(2, 4242)); + let RpcMsg::Response(resp) = peer.recv() else { + panic!("an attended cpi should keep answering"); + }; + assert_eq!(resp.seqn, 2); + assert_eq!(resp.rescode, RpcResultCode::Ok); + } + + #[test] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + fn a_connect_over_the_cpi_socket_is_answered() { + let rio = RunningRio::start(); + let peer = CpiPeer::attach(&rio.dir); + rio.attend_cpi(); + + peer.send(&CpiPeer::connect_request(1, 4242)); + + let RpcMsg::Response(resp) = peer.recv() else { + panic!("a request should draw a response"); + }; + assert_eq!(resp.op, RpcOp::Connect); + assert_eq!(resp.seqn, 1, "the response is matched to the request"); + assert_eq!(resp.rescode, RpcResultCode::Ok); + assert!( + matches!(resp.objs.first(), Some(RpcObject::ConnectInfo(_))), + "a connect is answered with the sync token, not bare" + ); + } + + #[test] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + fn a_request_before_any_connect_is_ignored() { + let rio = RunningRio::start(); + let peer = CpiPeer::attach(&rio.dir); + rio.attend_cpi(); + + peer.send(&CpiPeer::route_request(RpcOp::Del, 7)); + + let RpcMsg::Response(resp) = peer.recv() else { + panic!("a request should draw a response"); + }; + assert_eq!(resp.seqn, 7); + assert_eq!( + resp.rescode, + RpcResultCode::Ignored, + "a route withdrawal offered before a connect must not be acted on" + ); + } + + #[test] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + fn without_a_config_additions_are_refused_and_deletions_are_not() { + let rio = RunningRio::start(); + let peer = CpiPeer::attach(&rio.dir); + rio.attend_cpi(); + + peer.send(&CpiPeer::connect_request(1, 4242)); + let RpcMsg::Response(connected) = peer.recv() else { + panic!("connect should be answered"); + }; + assert_eq!(connected.rescode, RpcResultCode::Ok); + + peer.send(&CpiPeer::route_request(RpcOp::Add, 2)); + let RpcMsg::Response(added) = peer.recv() else { + panic!("add should be answered"); + }; + assert_eq!( + added.rescode, + RpcResultCode::Ignored, + "an addition with no config is refused" + ); + + peer.send(&CpiPeer::route_request(RpcOp::Del, 3)); + let RpcMsg::Response(deleted) = peer.recv() else { + panic!("del should be answered"); + }; + assert_ne!( + deleted.rescode, + RpcResultCode::Ignored, + "a deletion with no config is allowed through, to wipe stale state" + ); + } + + #[test] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + fn a_malformed_datagram_draws_a_notification_and_does_not_wedge_the_loop() { + let rio = RunningRio::start(); + let peer = CpiPeer::attach(&rio.dir); + rio.attend_cpi(); + + peer.send_raw(&[0xff; 32]); + assert!( + matches!(peer.recv(), RpcMsg::Notification(_)), + "garbage should be answered with a notification" + ); + + peer.send(&CpiPeer::connect_request(9, 4242)); + let RpcMsg::Response(resp) = peer.recv() else { + panic!("the loop should still answer after a decode failure"); + }; + assert_eq!(resp.seqn, 9); + assert_eq!(resp.rescode, RpcResultCode::Ok); + } + + fn ask_the_cli(dir: &SockDir, tag: &str) -> CliResponse { + let sock = UnixDatagram::bind(dir.path(&format!("cli-client-{tag}.sock"))) + .expect("cli client sock should bind"); + sock.connect(dir.path("cli.sock")) + .expect("rio's cli socket should be connectable"); + sock.set_read_timeout(Some(PATIENCE)) + .expect("read timeout should be settable"); + CliRequest::new(CliAction::ShowCpiStats, RequestArgs::default()) + .send(&sock) + .expect("cli request should send"); + CliResponse::recv_sync(&sock).expect("rio should answer the cli within the timeout") + } + + #[test] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + fn the_cli_survives_having_its_socket_path_removed() { + let rio = RunningRio::start(); + let cli = rio.dir.path("cli.sock"); + + let before = ask_the_cli(&rio.dir, "before"); + assert_eq!( + before.request.action, + CliAction::ShowCpiStats, + "the cli answers before the path is disturbed" + ); + + std::fs::remove_file(&cli).expect("the path should be removable"); + + let deadline = clock::now() + PATIENCE; + while clock::now() < deadline && !Path::new(&cli).exists() { + thread::sleep(Duration::from_millis(20)); + } + assert!( + Path::new(&cli).exists(), + "the loop should notice the unlink and rebind" + ); + + let after = ask_the_cli(&rio.dir, "after"); + assert_eq!( + after.request.action, + CliAction::ShowCpiStats, + "and the rebound socket must actually be served, not merely exist" + ); + } + + struct FakeAgent { + listener: UnixListener, + } + impl FakeAgent { + fn listening_at(dir: &SockDir) -> Self { + let listener = + UnixListener::bind(dir.path("frr-agent.sock")).expect("the agent should bind"); + listener + .set_nonblocking(true) + .expect("the agent should be pollable"); + Self { listener } + } + fn accept(&self, expectation: &str) -> UnixStream { + let deadline = clock::now() + PATIENCE; + loop { + match self.listener.accept() { + Ok((stream, _)) => return stream, + Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => { + assert!(clock::now() < deadline, "rio never {expectation}"); + thread::sleep(Duration::from_millis(20)); + } + Err(e) => panic!("the agent could not accept: {e}"), + } + } + } + } + + #[test] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + fn the_loop_connects_to_the_agent_whenever_it_appears() { + let rio = RunningRio::start(); + let agent = FakeAgent::listening_at(&rio.dir); + let _conn = agent.accept("connected to an agent that appeared after it started"); + } + + #[test] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + fn the_loop_reconnects_when_the_agent_goes_away() { + let rio = RunningRio::start(); + let agent = FakeAgent::listening_at(&rio.dir); + + let first = agent.accept("connected to the agent"); + drop(first); + + let _second = agent.accept("reconnected after the agent left"); + } + + #[test] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + fn nonsense_from_the_agent_restarts_the_link() { + let rio = RunningRio::start(); + let agent = FakeAgent::listening_at(&rio.dir); + + let mut first = agent.accept("connected to the agent"); + first + .write_all(&[0xff; 64]) + .expect("the agent should be able to write nonsense"); + + let _second = agent.accept("rebuilt the link after a message it could not read"); + drop(first); + } + + impl RunningRio { + fn configure(&self, genid: GenId, frr: Option) { + let mut cfg = RouterConfig::new(genid); + cfg.add_vrf(RouterVrfConfig::new(0, "default")); + if let Some(frr) = frr { + cfg.set_frr_config(frr); + } + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("a runtime to drive the ctl channel") + .block_on(self.handle.get_ctl_tx().configure(cfg)) + .expect("the router should accept a minimal config"); + } + + fn fib_v4(&self) -> Vec { + let Some(table) = self.held.1.enter() else { + return vec![]; + }; + let Some(fibr) = table.get_fib(FibKey::from_vrfid(0)) else { + return vec![]; + }; + let Some(fib) = fibr.enter() else { + return vec![]; + }; + let mut out: Vec = fib + .iter_v4() + .map(|(p, _)| p.to_string()) + .filter(|p| p != "0.0.0.0/0") + .collect(); + out.sort(); + out + } + + fn await_fib_v4(&self, want: usize) -> Vec { + let deadline = clock::now() + PATIENCE; + loop { + let got = self.fib_v4(); + if got.len() == want { + return got; + } + assert!( + clock::now() < deadline, + "the fib settled on {got:?}, wanted {want} v4 route(s)" + ); + thread::sleep(Duration::from_millis(20)); + } + } + } + + impl CpiPeer { + fn route_for(op: RpcOp, seqn: u64, prefix: &str, metric: u32) -> RpcMsg { + RpcMsg::Request( + RpcRequest::new(op, seqn).set_object(RpcObject::IpRoute(IpRoute { + prefix: prefix.parse().expect("a literal prefix"), + prefix_len: 24, + vrfid: 0, + tableid: 254, + rtype: RouteType::Bgp, + distance: 20, + metric, + nhops: vec![], + })), + ) + } + fn send_accepted(&self, msg: &RpcMsg, what: &str) { + self.send(msg); + let RpcMsg::Response(resp) = self.recv() else { + panic!("{what} drew no response"); + }; + assert_eq!(resp.rescode, RpcResultCode::Ok, "{what} was refused"); + } + fn announce_routes(&self, count: usize) { + const BATCH: usize = 64; + let mut seqn = 100u64; + for chunk in (0..count).collect::>().chunks(BATCH) { + for i in chunk { + self.send(&Self::route_for( + RpcOp::Add, + seqn, + &format!("10.{}.{}.0", i / 256, i % 256), + 100, + )); + seqn += 1; + } + for _ in chunk { + let RpcMsg::Response(resp) = self.recv() else { + panic!("a route drew no response"); + }; + assert_eq!(resp.rescode, RpcResultCode::Ok, "a route was refused"); + } + } + } + fn say_hello(&self) { + self.send_accepted(&Self::connect_request(1, 4242), "the connect"); + } + } + + #[test] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + fn a_route_the_control_plane_announces_reaches_the_fib() { + let rio = RunningRio::start(); + let peer = CpiPeer::attach(&rio.dir); + rio.attend_cpi(); + rio.configure(1, None); + peer.say_hello(); + + peer.send_accepted( + &CpiPeer::route_for(RpcOp::Add, 2, "10.0.0.0", 100), + "the route", + ); + + assert_eq!(rio.await_fib_v4(1), vec!["10.0.0.0/24".to_string()]); + } + + #[test] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + fn a_route_the_control_plane_withdraws_leaves_the_fib() { + let rio = RunningRio::start(); + let peer = CpiPeer::attach(&rio.dir); + rio.attend_cpi(); + rio.configure(1, None); + peer.say_hello(); + + peer.send_accepted( + &CpiPeer::route_for(RpcOp::Add, 2, "10.0.0.0", 100), + "the route", + ); + assert_eq!(rio.await_fib_v4(1), vec!["10.0.0.0/24".to_string()]); + + peer.send_accepted( + &CpiPeer::route_for(RpcOp::Del, 3, "10.0.0.0", 100), + "the withdrawal", + ); + assert_eq!(rio.await_fib_v4(0), Vec::::new()); + } + + #[test] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + fn announcing_a_prefix_twice_leaves_one_route() { + let rio = RunningRio::start(); + let peer = CpiPeer::attach(&rio.dir); + rio.attend_cpi(); + rio.configure(1, None); + peer.say_hello(); + + peer.send_accepted( + &CpiPeer::route_for(RpcOp::Add, 2, "10.0.0.0", 100), + "the route", + ); + assert_eq!(rio.await_fib_v4(1), vec!["10.0.0.0/24".to_string()]); + + peer.send_accepted( + &CpiPeer::route_for(RpcOp::Add, 3, "10.0.0.0", 200), + "the re-announcement", + ); + peer.send_accepted( + &CpiPeer::route_for(RpcOp::Update, 4, "10.0.0.0", 300), + "the update", + ); + + assert_eq!(rio.await_fib_v4(1), vec!["10.0.0.0/24".to_string()]); + } + + impl FakeAgent { + fn read_request(stream: &mut UnixStream) -> (GenId, String) { + stream + .set_read_timeout(Some(PATIENCE)) + .expect("read timeout should be settable"); + let mut header = [0u8; 16]; + stream + .read_exact(&mut header) + .expect("the agent should receive a header"); + let len = u64::from_ne_bytes(header[0..8].try_into().expect("8 octets")); + let genid = GenId::from_ne_bytes(header[8..16].try_into().expect("8 octets")); + let mut body = vec![0u8; usize::try_from(len).expect("a sane length")]; + stream + .read_exact(&mut body) + .expect("the agent should receive the announced body"); + ( + genid, + String::from_utf8(body).expect("the config should be text"), + ) + } + fn answer(stream: &mut UnixStream, genid: GenId, data: &str) { + let body = data.as_bytes(); + let mut msg = Vec::with_capacity(16 + body.len()); + msg.extend_from_slice(&(body.len() as u64).to_ne_bytes()); + msg.extend_from_slice(&genid.to_ne_bytes()); + msg.extend_from_slice(body); + stream.write_all(&msg).expect("the agent should answer"); + } + } + + #[test] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + fn a_configuration_reaches_the_agent_and_its_answer_comes_back() { + const GENID: GenId = 7; + const CONFIG: &str = "router bgp 65000\n neighbor 10.0.0.1 remote-as 65001\n"; + + let rio = RunningRio::start(); + let agent = FakeAgent::listening_at(&rio.dir); + let mut link = agent.accept("connected to the agent"); + + rio.configure(GENID, Some(CONFIG.to_string())); + + let (genid, body) = FakeAgent::read_request(&mut link); + assert_eq!(genid, GENID, "the request carries the generation it is for"); + assert_eq!(body, CONFIG, "and the configuration verbatim"); + + FakeAgent::answer(&mut link, GENID, "Ok"); + + let deadline = clock::now() + PATIENCE; + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("a runtime to drive the ctl channel"); + loop { + let applied = runtime + .block_on(rio.handle.get_ctl_tx().get_frr_applied_config()) + .expect("the router should answer"); + if let Some(applied) = applied { + assert_eq!(applied.genid, GENID); + assert_eq!(applied.cfg, CONFIG); + return; + } + assert!( + clock::now() < deadline, + "the acknowledged configuration was never recorded as applied" + ); + thread::sleep(Duration::from_millis(20)); + } + } + + #[test] + #[cfg_attr(emulated, ignore = "binds Unix domain sockets")] + fn a_large_answer_arrives_whole() { + const ROUTES: usize = 8192; + + let rio = RunningRio::start(); + let peer = CpiPeer::attach(&rio.dir); + rio.attend_cpi(); + rio.configure(1, None); + peer.say_hello(); + peer.announce_routes(ROUTES); + assert_eq!(rio.await_fib_v4(ROUTES).len(), ROUTES); + + let sock = UnixDatagram::bind(rio.dir.path("cli-big.sock")).expect("client should bind"); + sock.connect(rio.dir.path("cli.sock")) + .expect("rio's cli socket should be connectable"); + sock.set_read_timeout(Some(PATIENCE)) + .expect("read timeout should be settable"); + + CliRequest::new(CliAction::ShowRouterIpv4FibEntries, RequestArgs::default()) + .send(&sock) + .expect("cli request should send"); + + let answer = CliResponse::recv_sync(&sock) + .expect("the whole answer should arrive, across as many chunks as it takes"); + let body = answer.result.expect("the listing should succeed"); + + assert!( + body.len() > 100 * 2048, + "the answer must span many chunks for this to test reassembly, got {} octets", + body.len() + ); + assert!(body.contains("10.0.0.0/24"), "the first route is missing"); + assert!( + body.contains(&format!( + "10.{}.{}.0/24", + (ROUTES - 1) / 256, + (ROUTES - 1) % 256 + )), + "the last route is missing, so the answer was cut short" + ); + } } diff --git a/routing/src/router/rpc_adapt.rs b/routing/src/router/rpc_adapt.rs index b39f66a479..ed018a2f3f 100644 --- a/routing/src/router/rpc_adapt.rs +++ b/routing/src/router/rpc_adapt.rs @@ -23,7 +23,6 @@ use net::eth::mac::Mac; use net::interface::InterfaceIndex; use net::vxlan::Vni; use std::net::{IpAddr, Ipv4Addr}; -use std::time::Instant; use tracing::{error, warn}; impl From for RouteOrigin { @@ -160,7 +159,7 @@ impl Route { distance: iproute.distance, metric: iproute.metric, s_nhops: vec![], /* shim nhops are empty here */ - tstamp: Instant::now(), + tstamp: clock::now(), } } } diff --git a/stats/Cargo.toml b/stats/Cargo.toml index 50fe1da0da..ffbada8f22 100644 --- a/stats/Cargo.toml +++ b/stats/Cargo.toml @@ -11,6 +11,7 @@ bolero = ["dep:bolero", "vpcmap/bolero", "net/bolero"] [dependencies] # internal +clock = { workspace = true } concurrency = { workspace = true } net = { workspace = true } pipeline = { workspace = true } @@ -34,6 +35,8 @@ tokio = { workspace = true, features = ["macros", "time", "sync"] } tracing = { workspace = true, features = ["attributes"] } [dev-dependencies] +clock = { workspace = true, features = ["virtual"] } +tokio = { workspace = true, features = ["macros", "rt", "sync", "time"] } bolero = { workspace = true } net = { workspace = true, features = ["bolero"] } vpcmap = { workspace = true, features = ["bolero"] } diff --git a/stats/src/dpstats.rs b/stats/src/dpstats.rs index 19f1b01733..10f9264183 100644 --- a/stats/src/dpstats.rs +++ b/stats/src/dpstats.rs @@ -244,7 +244,7 @@ impl StatsCollector { let updates = PacketStatsReader(r); let outstanding: VecDeque<_> = (0..10) .scan( - BatchSummary::::new(Instant::now() + Self::TIME_TICK), + BatchSummary::::new(clock::now() + Self::TIME_TICK), |prior, _| Some(BatchSummary::new(prior.planned_end + Self::TIME_TICK)), ) .collect(); @@ -522,7 +522,7 @@ impl StatsCollector { }); } - let current_time = Instant::now(); + let current_time = clock::now(); let mut expired = self .outstanding .iter() @@ -833,7 +833,7 @@ impl BatchSummary { #[inline] pub fn with_capacity(planned_end: Instant, capacity: usize) -> Self { Self { - start: Instant::now(), + start: clock::now(), planned_end, vpc: hashbrown::HashMap::with_capacity(capacity), } @@ -900,7 +900,7 @@ impl Stats { stats: PacketStatsWriter, delivery_schedule: Duration, ) -> Self { - let planned_end = Instant::now() + delivery_schedule; + let planned_end = clock::now() + delivery_schedule; Self { name: name.to_string(), update: Box::new(BatchSummary::new(planned_end)), @@ -920,7 +920,7 @@ impl NetworkFunction for Stats { // amount of spare room in hash table. Padding a little bit will hopefully save us some // reallocations const CAPACITY_PAD: usize = 16; - let time = Instant::now(); + let time = clock::now(); if time > self.update.planned_end { trace!("sending stats update"); let batch = Box::new(BatchSummary::with_capacity( @@ -1088,8 +1088,8 @@ pub struct SplitCount { mod contract { use crate::{BatchSummary, PacketAndByte, TransmitSummary}; use bolero::{Driver, TypeGenerator, ValueGenerator}; + use clock::Duration; use small_map::SmallMap; - use std::time::{Duration, Instant}; use vpcmap::VpcDiscriminant; impl TypeGenerator for PacketAndByte @@ -1153,7 +1153,7 @@ mod contract { T: TypeGenerator, { fn generate(driver: &mut D) -> Option { - let start = Instant::now() + Duration::from_millis(driver.produce()?); + let start = clock::now() + Duration::from_millis(driver.produce()?); let duration: Duration = driver.produce()?; let vpc_gen = VpcDiscMap::> { _marker: std::marker::PhantomData, @@ -1328,7 +1328,7 @@ mod drop_stats_tests { } fn batch(offset_secs: u64) -> BatchSummary { - BatchSummary::::new(Instant::now() + Duration::from_secs(offset_secs)) + BatchSummary::::new(clock::now() + Duration::from_secs(offset_secs)) } #[test] diff --git a/stats/src/dpstats_fuzz.rs b/stats/src/dpstats_fuzz.rs new file mode 100644 index 0000000000..d9162eaf02 --- /dev/null +++ b/stats/src/dpstats_fuzz.rs @@ -0,0 +1,236 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +#![cfg(test)] + +use crate::{SplitCount, TimeSlice}; +use bolero::TypeGenerator; +use clock::{Duration, Instant}; + +#[derive(Debug, Clone, Copy)] +struct Slice { + start: Instant, + end: Instant, +} + +impl TimeSlice for Slice { + fn start(&self) -> Instant { + self.start + } + fn end(&self) -> Instant { + self.end + } +} + +#[derive(Debug, Clone, Copy, TypeGenerator)] +struct Pair { + window_start: u16, + window_len: u16, + sample_start: u16, + sample_len: u16, +} + +impl Pair { + fn slices(self) -> (Slice, Slice) { + let origin = clock::now(); + let ms = |v: u16| Duration::from_millis(u64::from(v)); + ( + Slice { + start: origin + ms(self.window_start), + end: origin + ms(self.window_start) + ms(self.window_len), + }, + Slice { + start: origin + ms(self.sample_start), + end: origin + ms(self.sample_start) + ms(self.sample_len), + }, + ) + } +} + +#[test] +fn a_split_conserves_the_count() { + bolero::check!() + .with_type::<(Pair, u64)>() + .for_each(|(pair, count): &(Pair, u64)| { + let (window, sample) = pair.slices(); + let SplitCount { inside, outside } = window.split_count(&sample, *count); + assert_eq!( + inside.checked_add(outside), + Some(*count), + "splitting {count} across {window:?} and {sample:?} produced {inside} + {outside}" + ); + }); +} + +#[test] +fn a_contained_sample_is_wholly_inside() { + bolero::check!() + .with_type::<(u16, u16, u16, u64)>() + .for_each(|(start, len, inset, count): &(u16, u16, u16, u64)| { + let origin = clock::now(); + let ms = |v: u16| Duration::from_millis(u64::from(v)); + let window = Slice { + start: origin + ms(*start), + end: origin + ms(*start) + ms(*len), + }; + let inset = inset % len.saturating_add(1).max(1); + let sample = Slice { + start: window.start + ms(inset), + end: window.end, + }; + if sample.end <= sample.start { + return; + } + + let split = window.split_count(&sample, *count); + assert_eq!( + split.inside, *count, + "a sample inside the window was apportioned outside it: {split:?}" + ); + }); +} + +#[test] +fn a_sample_after_the_window_carries_over() { + bolero::check!() + .with_type::<(u16, u16, u16, u16, u64)>() + .for_each(|(start, len, gap, sample_len, count): &(u16, u16, u16, u16, u64)| { + let origin = clock::now(); + let ms = |v: u16| Duration::from_millis(u64::from(v)); + let window = Slice { + start: origin + ms(*start), + end: origin + ms(*start) + ms(*len), + }; + let sample = Slice { + start: window.end + ms(*gap), + end: window.end + ms(*gap) + ms(*sample_len), + }; + + let split = window.split_count(&sample, *count); + assert_eq!( + split.outside, *count, + "a sample beginning at or after the window's end was apportioned into it: {split:?}" + ); + }); +} + +#[test] +fn a_sample_before_the_window_is_salvaged_into_it() { + bolero::check!() + .with_type::<(u16, u16, u16, u16, u64)>() + .for_each( + |(start, len, gap, sample_len, count): &(u16, u16, u16, u16, u64)| { + let origin = clock::now(); + let ms = |v: u16| Duration::from_millis(u64::from(v)); + let window = Slice { + start: origin + ms(u16::MAX) + ms(*start), + end: origin + ms(u16::MAX) + ms(*start) + ms(*len), + }; + let end = window.start - ms(*gap); + let sample = Slice { + start: end - ms(*sample_len), + end, + }; + if sample.end <= sample.start { + return; + } + + let split = window.split_count(&sample, *count); + assert_eq!( + split.inside, *count, + "a sample entirely before the window was not salvaged into it: {split:?}" + ); + }, + ); +} + +#[test] +fn a_longer_sample_never_gains_inside_share() { + bolero::check!() + .with_type::<(u16, u16, u16, u16, u16, u64)>() + .for_each( + |(start, len, sample_start, sample_len, extra, count): &( + u16, + u16, + u16, + u16, + u16, + u64, + )| { + let origin = clock::now(); + let ms = |v: u16| Duration::from_millis(u64::from(v)); + let window = Slice { + start: origin + ms(*start), + end: origin + ms(*start) + ms(*len), + }; + let begin = window.start + ms(*sample_start); + if *sample_len == 0 { + return; + } + let short = Slice { + start: begin, + end: begin + ms(*sample_len), + }; + let long = Slice { + start: begin, + end: begin + ms(*sample_len) + ms(*extra), + }; + + let short_split = window.split_count(&short, *count); + let long_split = window.split_count(&long, *count); + assert!( + long_split.inside <= short_split.inside, + "extending a sample past the window increased the share attributed to the \ + window: {short_split:?} became {long_split:?}" + ); + }, + ); +} + +#[test] +fn a_zero_length_sample_is_wholly_outside() { + bolero::check!() + .with_type::<(u16, u16, u16, u64)>() + .for_each(|(start, len, at, count): &(u16, u16, u16, u64)| { + let origin = clock::now(); + let ms = |v: u16| Duration::from_millis(u64::from(v)); + let window = Slice { + start: origin + ms(*start), + end: origin + ms(*start) + ms(*len), + }; + let instant = origin + ms(*at); + let sample = Slice { + start: instant, + end: instant, + }; + + let split = window.split_count(&sample, *count); + assert_eq!( + split, + SplitCount { + inside: 0, + outside: *count + }, + "a zero-length sample was apportioned into the window" + ); + }); +} + +#[test] +fn swapping_the_intervals_swaps_the_halves() { + bolero::check!() + .with_type::<(Pair, u64)>() + .for_each(|(pair, count): &(Pair, u64)| { + let (window, sample) = pair.slices(); + if window.duration() == Duration::ZERO || sample.duration() == Duration::ZERO { + return; + } + let forward = window.split_count(&sample, *count); + let reverse = sample.split_count(&window, *count); + assert_eq!( + forward.inside + forward.outside, + reverse.inside + reverse.outside, + "the two directions conserve different totals" + ); + }); +} diff --git a/stats/src/lib.rs b/stats/src/lib.rs index 1d1eb999c1..7e9bb5ea82 100644 --- a/stats/src/lib.rs +++ b/stats/src/lib.rs @@ -4,11 +4,14 @@ // SCRATCH mod dpstats; +mod dpstats_fuzz; mod rate; +mod rate_fuzz; mod register; mod spec; mod vpc; mod vpc_stats; +mod vpc_stats_fuzz; pub use dpstats::*; pub use rate::*; diff --git a/stats/src/rate_fuzz.rs b/stats/src/rate_fuzz.rs new file mode 100644 index 0000000000..9959a48a05 --- /dev/null +++ b/stats/src/rate_fuzz.rs @@ -0,0 +1,175 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +#![cfg(test)] + +use crate::rate::ExponentiallyWeightedMovingAverage; +use clock::{Duration, Instant}; + +fn timeline(steps: &[u16]) -> Vec { + let start = clock::now(); + let mut out = Vec::with_capacity(steps.len()); + let mut elapsed = Duration::from_millis(0); + for step in steps { + elapsed += Duration::from_millis(u64::from(*step) + 1); + out.push(start + elapsed); + } + out +} + +fn ewma() -> ExponentiallyWeightedMovingAverage { + ExponentiallyWeightedMovingAverage::new(Duration::from_secs(1)) +} + +#[test] +fn the_first_sample_is_the_average() { + bolero::check!().with_type::().for_each(|value: &f64| { + if !value.is_finite() { + return; + } + let mut avg = ewma(); + let at = timeline(&[0]); + assert_eq!( + avg.update((at[0], *value)), + *value, + "the first sample was averaged against something" + ); + assert_eq!( + avg.get(), + *value, + "get() disagreed with what update() returned" + ); + }); +} + +#[test] +fn the_average_stays_within_the_samples() { + bolero::check!() + .with_type::>() + .for_each(|steps: &Vec<(u16, u16)>| { + if steps.is_empty() { + return; + } + let times = timeline(&steps.iter().map(|(t, _)| *t).collect::>()); + let mut avg = ewma(); + let (mut low, mut high) = (f64::INFINITY, f64::NEG_INFINITY); + + for (at, (_, sample)) in times.iter().zip(steps.iter()) { + let sample = f64::from(*sample); + low = low.min(sample); + high = high.max(sample); + let out = avg.update((*at, sample)); + let tolerance = 1e-9 * high.abs().max(low.abs()).max(1.0); + assert!( + out >= low - tolerance && out <= high + tolerance, + "the average came out at {out}, outside the samples seen so far [{low}, {high}]" + ); + } + }); +} + +#[test] +fn a_constant_input_stays_constant() { + bolero::check!() + .with_type::<(u16, Vec)>() + .for_each(|(value, steps): &(u16, Vec)| { + if steps.is_empty() { + return; + } + let value = f64::from(*value); + let mut avg = ewma(); + for at in timeline(steps) { + let out = avg.update((at, value)); + assert!( + (out - value).abs() < 1e-9, + "a constant {value} averaged to {out}" + ); + } + }); +} + +#[test] +fn a_step_is_approached_monotonically() { + bolero::check!() + .with_type::<(u16, u16, Vec)>() + .for_each(|(from, to, steps): &(u16, u16, Vec)| { + if steps.is_empty() { + return; + } + let (from, to) = (f64::from(*from), f64::from(*to)); + let times = timeline(steps); + let mut avg = ewma(); + let mut previous = avg.update((times[0], from)); + + for at in ×[1..] { + let out = avg.update((*at, to)); + let closer = (out - to).abs() <= (previous - to).abs() + 1e-9; + assert!( + closer, + "the average moved away from the new level {to}: {previous} -> {out}" + ); + let overshot = if to >= from { + out > to + 1e-9 + } else { + out < to - 1e-9 + }; + assert!(!overshot, "the average overshot {to}, reaching {out}"); + previous = out; + } + }); +} + +#[test] +fn a_longer_gap_weights_the_new_sample_strictly_more() { + bolero::check!() + .with_type::<(u16, u16, u16, u16)>() + .for_each(|(first, second, short, extra): &(u16, u16, u16, u16)| { + let (first, second) = (f64::from(*first), f64::from(*second)); + if (first - second).abs() < 1.0 { + return; + } + let short = Duration::from_millis(u64::from(*short % 1000) + 1); + let long = short + Duration::from_millis(u64::from(*extra % 900) + 100); + let start = clock::now(); + + let mut near = ewma(); + near.update((start, first)); + let near = near.update((start + short, second)); + + let mut far = ewma(); + far.update((start, first)); + let far = far.update((start + long, second)); + + assert!( + (far - second).abs() < (near - second).abs(), + "a sample after {long:?} landed at {far} and the same sample after {short:?} landed \ + at {near}; the longer gap did not weight the new sample more, so elapsed time is \ + not affecting the average" + ); + }); +} + +#[test] +fn reading_the_average_does_not_change_it() { + bolero::check!() + .with_type::>() + .for_each(|steps: &Vec| { + if steps.is_empty() { + return; + } + let mut avg = ewma(); + for (at, sample) in timeline(steps).iter().zip(steps.iter()) { + let out = avg.update((*at, f64::from(*sample))); + for _ in 0..3 { + assert_eq!(avg.get(), out, "get() changed the average it reported"); + } + } + }); +} + +#[test] +fn an_untouched_average_reports_the_default() { + let avg: ExponentiallyWeightedMovingAverage = + ExponentiallyWeightedMovingAverage::new(Duration::from_secs(1)); + assert_eq!(avg.get(), 0.0); +} diff --git a/stats/src/vpc_stats_fuzz.rs b/stats/src/vpc_stats_fuzz.rs new file mode 100644 index 0000000000..82a0cea50d --- /dev/null +++ b/stats/src/vpc_stats_fuzz.rs @@ -0,0 +1,332 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +#![cfg(test)] + +use crate::vpc_stats::{VpcId, VpcStatsStore}; +use bolero::TypeGenerator; +use net::vxlan::Vni; +use std::collections::HashSet; +use vpcmap::VpcDiscriminant; + +#[derive(Debug, Clone, Copy, TypeGenerator)] +struct VpcRef(u8); + +impl VpcRef { + fn id(self) -> VpcId { + let raw = u32::from(self.0 % 6) + 100; + VpcDiscriminant::from_vni(Vni::new_checked(raw).unwrap_or_else(|_| unreachable!())) + } +} + +#[derive(Debug, Clone, Copy, TypeGenerator)] +enum Op { + AddPair(VpcRef, VpcRef, u32, u32), + AddPairDrops(VpcRef, VpcRef, u32, u32), + SetPairRates(VpcRef, VpcRef, u16, u16), + RecordPair(VpcRef, VpcRef, u32, u32, u16, u16), + AddVpc(VpcRef, u32, u32), + AddVpcDrops(VpcRef, u32, u32), + SetVpcRates(VpcRef, u16, u16), + RecordVpc(VpcRef, u32, u32, u16, u16), + SetName(VpcRef, u8), +} + +async fn apply(store: &VpcStatsStore, op: Op) { + match op { + Op::AddPair(a, b, p, y) => { + store + .add_pair_counts(a.id(), b.id(), u64::from(p), u64::from(y)) + .await; + } + Op::AddPairDrops(a, b, p, y) => { + store + .add_pair_drops(a.id(), b.id(), u64::from(p), u64::from(y)) + .await; + } + Op::SetPairRates(a, b, p, y) => { + store + .set_pair_rates(a.id(), b.id(), f64::from(p), f64::from(y)) + .await; + } + Op::RecordPair(a, b, p, y, pps, bps) => { + store + .record_pair( + a.id(), + b.id(), + u64::from(p), + u64::from(y), + f64::from(pps), + f64::from(bps), + ) + .await; + } + Op::AddVpc(a, p, y) => { + store + .add_vpc_counts(a.id(), u64::from(p), u64::from(y)) + .await + } + Op::AddVpcDrops(a, p, y) => { + store + .add_vpc_drops(a.id(), u64::from(p), u64::from(y)) + .await + } + Op::SetVpcRates(a, p, y) => { + store + .set_vpc_rates(a.id(), f64::from(p), f64::from(y)) + .await + } + Op::RecordVpc(a, p, y, pps, bps) => { + store + .record_vpc( + a.id(), + u64::from(p), + u64::from(y), + f64::from(pps), + f64::from(bps), + ) + .await; + } + Op::SetName(a, n) => store.set_vpc_name_sync(a.id(), format!("vpc-{n}")), + } +} + +fn runtime() -> tokio::runtime::Runtime { + tokio::runtime::Builder::new_current_thread() + .build() + .unwrap_or_else(|e| unreachable!("{e}")) +} + +#[test] +fn counters_only_ever_increase() { + let rt = runtime(); + bolero::check!() + .with_type::>() + .cloned() + .for_each(|ops: Vec| { + rt.block_on(async { + let store = VpcStatsStore::new(); + let mut high: std::collections::HashMap<(VpcId, VpcId), (u64, u64, u64)> = + std::collections::HashMap::new(); + + for op in ops.iter().take(24) { + apply(&store, *op).await; + for (key, stats) in store.snapshot_pairs().await { + let seen = high.entry(key).or_default(); + assert!( + stats.ctr.packets >= seen.0 + && stats.ctr.bytes >= seen.1 + && stats.drops.packets >= seen.2, + "a counter for {key:?} went backwards after {op:?}: \ + {stats:?} against a high water mark of {seen:?}" + ); + *seen = (stats.ctr.packets, stats.ctr.bytes, stats.drops.packets); + } + } + }); + }); +} + +#[test] +fn recording_a_pair_equals_counting_then_rating() { + let rt = runtime(); + bolero::check!() + .with_type::>() + .cloned() + .for_each(|steps: Vec<(VpcRef, VpcRef, u32, u32, u16, u16)>| { + rt.block_on(async { + let compound = VpcStatsStore::new(); + let parts = VpcStatsStore::new(); + + for (a, b, p, y, pps, bps) in steps.iter().take(24) { + let (a, b) = (a.id(), b.id()); + let (p, y) = (u64::from(*p), u64::from(*y)); + let (pps, bps) = (f64::from(*pps), f64::from(*bps)); + compound.record_pair(a, b, p, y, pps, bps).await; + parts.add_pair_counts(a, b, p, y).await; + parts.set_pair_rates(a, b, pps, bps).await; + } + + let mut left = compound.snapshot_pairs().await; + let mut right = parts.snapshot_pairs().await; + left.sort_by_key(|(k, _)| *k); + right.sort_by_key(|(k, _)| *k); + assert_eq!( + left.len(), + right.len(), + "the two stores hold different pairs" + ); + for ((lk, lv), (rk, rv)) in left.iter().zip(right.iter()) { + assert_eq!(lk, rk, "the two stores disagree on which pairs exist"); + assert_eq!( + (lv.ctr.packets, lv.ctr.bytes, lv.rate.pps, lv.rate.bps), + (rv.ctr.packets, rv.ctr.bytes, rv.rate.pps, rv.rate.bps), + "record_pair and add-then-set disagree for {lk:?}" + ); + } + }); + }); +} + +#[test] +fn the_two_tables_are_independent() { + let rt = runtime(); + bolero::check!() + .with_type::>() + .cloned() + .for_each(|ops: Vec| { + rt.block_on(async { + let store = VpcStatsStore::new(); + for op in ops.iter().take(24) { + if matches!( + op, + Op::AddPair(..) + | Op::AddPairDrops(..) + | Op::SetPairRates(..) + | Op::RecordPair(..) + ) { + apply(&store, *op).await; + } + } + assert!( + store.snapshot_vpcs().await.is_empty(), + "recording pair statistics populated the per-vpc table, so per-vpc totals \ + would double count once a caller maintains both" + ); + }); + }); +} + +#[test] +fn pruning_keeps_exactly_the_live_set() { + let rt = runtime(); + bolero::check!() + .with_type::<(Vec, Vec)>() + .cloned() + .for_each(|(ops, alive): (Vec, Vec)| { + rt.block_on(async { + let store = VpcStatsStore::new(); + for op in ops.iter().take(24) { + apply(&store, *op).await; + } + let alive: HashSet = alive.iter().map(|v| v.id()).collect(); + store.prune_to_vpcs(&alive).await; + + for (key @ (src, dst), _) in store.snapshot_pairs().await { + assert!( + alive.contains(&src) && alive.contains(&dst), + "pruning kept the pair {key:?} although one of its ends is gone" + ); + } + for (vpc, _) in store.snapshot_vpcs().await { + assert!( + alive.contains(&vpc), + "pruning kept per-vpc statistics for {vpc:?}, which is gone" + ); + } + for vpc in store.snapshot_names().await.keys() { + assert!( + alive.contains(vpc), + "pruning kept the name of {vpc:?}, which is gone; it will be read \ + against whichever vpc is allocated that discriminant next" + ); + } + }); + }); +} + +#[test] +fn pruning_removes_nothing_that_is_alive() { + let rt = runtime(); + bolero::check!() + .with_type::>() + .cloned() + .for_each(|ops: Vec| { + rt.block_on(async { + let store = VpcStatsStore::new(); + for op in ops.iter().take(24) { + apply(&store, *op).await; + } + let before_pairs = store.snapshot_pairs().await; + let before_vpcs = store.snapshot_vpcs().await; + let before_names = store.snapshot_names().await; + + let mut alive: HashSet = HashSet::new(); + for ((src, dst), _) in &before_pairs { + alive.insert(*src); + alive.insert(*dst); + } + for (vpc, _) in &before_vpcs { + alive.insert(*vpc); + } + alive.extend(before_names.keys().copied()); + + store.prune_to_vpcs(&alive).await; + + assert_eq!( + store.snapshot_pairs().await.len(), + before_pairs.len(), + "pruning dropped a pair whose ends are both alive" + ); + assert_eq!( + store.snapshot_vpcs().await.len(), + before_vpcs.len(), + "pruning dropped per-vpc statistics for a live vpc" + ); + assert_eq!( + store.snapshot_names().await.len(), + before_names.len(), + "pruning dropped the name of a live vpc" + ); + }); + }); +} + +#[test] +fn counters_saturate_rather_than_wrap() { + runtime().block_on(async { + let store = VpcStatsStore::new(); + let (a, b) = (VpcRef(0).id(), VpcRef(1).id()); + store.add_pair_counts(a, b, u64::MAX, u64::MAX).await; + store.add_pair_counts(a, b, 1_000, 1_000).await; + store.add_pair_drops(a, b, u64::MAX, u64::MAX).await; + store.add_pair_drops(a, b, 1_000, 1_000).await; + + let pairs = store.snapshot_pairs().await; + let (_, stats) = pairs.first().unwrap_or_else(|| unreachable!()); + assert_eq!(stats.ctr.packets, u64::MAX, "a packet counter wrapped"); + assert_eq!(stats.ctr.bytes, u64::MAX, "a byte counter wrapped"); + assert_eq!(stats.drops.packets, u64::MAX, "a drop counter wrapped"); + }); +} + +#[test] +fn a_name_belongs_to_exactly_one_vpc() { + let rt = runtime(); + bolero::check!() + .with_type::>() + .cloned() + .for_each(|pairs: Vec<(VpcRef, u8)>| { + rt.block_on(async { + let store = VpcStatsStore::new(); + let mut expected = std::collections::HashMap::new(); + for (vpc, n) in pairs.iter().take(24) { + let name = format!("vpc-{n}"); + store.set_vpc_name_sync(vpc.id(), name.clone()); + expected.insert(vpc.id(), name); + } + for (vpc, name) in &expected { + assert_eq!( + store.name_of(*vpc).as_ref(), + Some(name), + "the name read back for {vpc:?} is not the one set" + ); + } + assert_eq!( + store.snapshot_names().await.len(), + expected.len(), + "the store holds names for vpcs that were never named" + ); + }); + }); +} diff --git a/tracectl/Cargo.toml b/tracectl/Cargo.toml index 381c503928..08e33946b6 100644 --- a/tracectl/Cargo.toml +++ b/tracectl/Cargo.toml @@ -6,6 +6,7 @@ publish.workspace = true version.workspace = true [dependencies] +clock = { workspace = true } color-eyre = { workspace = true , features = [ "capture-spantrace", "color-spantrace", "tracing-error", "track-caller" ] } common = { workspace = true } concurrency = { workspace = true } @@ -17,5 +18,6 @@ tracing-error = { workspace = true, features = ["traced-error"] } tracing-subscriber = { workspace = true, features = ["registry", "std", "env-filter", "fmt"] } [dev-dependencies] +clock = { workspace = true, features = ["virtual"] } serial_test = { workspace = true } tracing-test = { workspace = true } diff --git a/tracectl/src/throttle.rs b/tracectl/src/throttle.rs index 2b09a3832f..5d7b4db795 100644 --- a/tracectl/src/throttle.rs +++ b/tracectl/src/throttle.rs @@ -70,7 +70,7 @@ impl RateLimitFilter { buckets: Box::new(std::array::from_fn(|_| AtomicU64::new(initial))), capacity_milli, refill_milli_per_ms: u64::from(config.replenish_per_second), - baseline: Instant::now(), + baseline: clock::now(), } }