diff --git a/Cargo.lock b/Cargo.lock index f7272829..06872bc6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -457,18 +457,22 @@ dependencies = [ [[package]] name = "bit-set" -version = "0.8.0" +version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3" +checksum = "56d87354e4229f54a44f7bf2435906a4656dba36026ab6eaca629a2c436a691c" dependencies = [ "bit-vec", ] [[package]] name = "bit-vec" -version = "0.8.0" +version = "0.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" +checksum = "5727b15fa97d4f4fee0a3b7c3d550ed0269f54329207b86388de918604e31269" +dependencies = [ + "borsh", + "serde", +] [[package]] name = "bitflags" @@ -587,6 +591,30 @@ dependencies = [ "zeroize", ] +[[package]] +name = "borsh" +version = "1.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "553c5d846a6ba5150c65e3b1b8ec073bcf1abc20f9b7220de384a4443ea4e20a" +dependencies = [ + "borsh-derive", + "bytes", + "cfg_aliases", +] + +[[package]] +name = "borsh-derive" +version = "1.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "12cdfe656708a01f89b451a7d36466e6fe6c414de0aa18fc54f864f6f9ca9f56" +dependencies = [ + "once_cell", + "proc-macro-crate 3.5.0", + "proc-macro2", + "quote", + "syn 3.0.9", +] + [[package]] name = "bstr" version = "1.12.0" @@ -723,6 +751,17 @@ dependencies = [ "cpufeatures 0.2.17", ] +[[package]] +name = "chacha20" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "65c35e4b699c7e15ccbe7ee35c005e4fc0a278d22238a2857e6ce2dadeda1b06" +dependencies = [ + "cfg-if", + "cpufeatures 0.3.0", + "rand_core 0.10.1", +] + [[package]] name = "chacha20poly1305" version = "0.10.1" @@ -730,7 +769,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "10cd79432192d1c0f4e1a0fef9527696cc039165d729fb41b3f4f4f354c2dc35" dependencies = [ "aead", - "chacha20", + "chacha20 0.9.1", "cipher", "poly1305", "zeroize", @@ -1131,6 +1170,12 @@ version = "0.8.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" +[[package]] +name = "core_detect" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f8f80099a98041a3d1622845c271458a2d73e688351bf3cb999266764b81d48" + [[package]] name = "cpufeatures" version = "0.2.17" @@ -1773,6 +1818,7 @@ dependencies = [ "slog", "slog-async", "slog-term", + "thiserror 2.0.18", "tokio", "toml 1.1.2+spec-1.1.0", "transceiver-controller", @@ -2565,6 +2611,7 @@ dependencies = [ "cfg-if", "libc", "r-efi 6.0.0", + "rand_core 0.10.1", "wasip2", "wasip3", ] @@ -4433,7 +4480,7 @@ version = "0.5.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dcbff9bc912032c62bf65ef1d5aea88983b420f4f839db1e9b0c281a25c9c799" dependencies = [ - "proc-macro-crate", + "proc-macro-crate 1.3.1", "proc-macro2", "quote", "syn 1.0.109", @@ -4445,7 +4492,7 @@ version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ff32365de1b6743cb203b710788263c44a03de03802daf96092f2da4fe6ba4d7" dependencies = [ - "proc-macro-crate", + "proc-macro-crate 3.5.0", "proc-macro2", "quote", "syn 2.0.118", @@ -4976,9 +5023,9 @@ dependencies = [ [[package]] name = "oxnet" -version = "0.1.6" +version = "0.1.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a1d55676fd6e2fe06971c05d176c7aca3d2b7d6ed1244ae8026beb1e7b89058" +checksum = "2c18d1166306acc00465fd792bab330933eb7587753ff2f15ca3aa0427163dd4" dependencies = [ "ipnetwork", "schemars 0.8.22", @@ -5403,6 +5450,15 @@ dependencies = [ "toml_edit 0.19.15", ] +[[package]] +name = "proc-macro-crate" +version = "3.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e67ba7e9b2b56446f1d419b1d807906278ffa1a658a8a5d8a39dcb1f5a78614f" +dependencies = [ + "toml_edit 0.25.13+spec-1.1.0", +] + [[package]] name = "proc-macro-error" version = "1.0.4" @@ -5740,17 +5796,17 @@ dependencies = [ [[package]] name = "proptest" -version = "1.8.0" +version = "1.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2bb0be07becd10686a0bb407298fb425360a5c44a663774406340c59a22de4ce" +checksum = "b8530004ccb15eae51c7e40009fbe317f341f804db54dc033eec1c50be28cfa0" dependencies = [ "bit-set", "bit-vec", "bitflags 2.11.1", - "lazy_static", + "chacha20 0.10.2", + "core_detect", "num-traits", - "rand 0.9.3", - "rand_chacha 0.9.0", + "rand 0.10.3", "rand_xorshift", "regex-syntax", "rusty-fork", @@ -5931,6 +5987,16 @@ dependencies = [ "rand_core 0.9.3", ] +[[package]] +name = "rand" +version = "0.10.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "65c9fb96cbc91e3478eaae79a69fcd3f1ae4ad052e471fe6732fff548984b4af" +dependencies = [ + "getrandom 0.4.2", + "rand_core 0.10.1", +] + [[package]] name = "rand_chacha" version = "0.3.1" @@ -5969,13 +6035,19 @@ dependencies = [ "getrandom 0.3.3", ] +[[package]] +name = "rand_core" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63b8176103e19a2643978565ca18b50549f6101881c443590420e4dc998a3c69" + [[package]] name = "rand_xorshift" -version = "0.4.0" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "513962919efc330f829edb2535844d1b912b0fbe2ca165d613e4e8788bb05a5a" +checksum = "60aa6af80be32871323012e02e6e65f8a7cc7890931ae421d217ad8fe0df2ccf" dependencies = [ - "rand_core 0.9.3", + "rand_core 0.10.1", ] [[package]] @@ -8040,6 +8112,18 @@ dependencies = [ "winnow 0.7.14", ] +[[package]] +name = "toml_edit" +version = "0.25.13+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6975367e4d2ef766d86af01ffad14b622fecc8d4357a998fbc4deb6e9bacaf9b" +dependencies = [ + "indexmap 2.14.0", + "toml_datetime 1.1.1+spec-1.1.0", + "toml_parser", + "winnow 1.0.3", +] + [[package]] name = "toml_parser" version = "1.1.2+spec-1.1.0" @@ -9471,6 +9555,9 @@ name = "winnow" version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0592e1c9d151f854e6fd382574c3a0855250e1d9b2f99d9281c6e6391af352f1" +dependencies = [ + "memchr", +] [[package]] name = "winreg" diff --git a/Cargo.toml b/Cargo.toml index a7e73841..295e58f6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,7 +53,7 @@ omicron-common = { git = "https://github.com/oxidecomputer/omicron", branch= "ma oximeter = { git = "https://github.com/oxidecomputer/omicron", branch = "main" } oximeter-producer = { git = "https://github.com/oxidecomputer/omicron", branch = "main" } oximeter-instruments = { git = "https://github.com/oxidecomputer/omicron", branch = "main", default-features = false, features = ["kstat"] } -oxnet = { version = "0.1.6", default-features = false, features = ["schemars", "serde"] } +oxnet = { version = "0.1.7", default-features = false, features = ["schemars", "serde"] } propolis = { git = "https://github.com/oxidecomputer/propolis" } smf = { git = "https://github.com/illumos/smf-rs" } slog-error-chain = { git = "https://github.com/oxidecomputer/slog-error-chain", branch = "main" } diff --git a/asic/src/chaos/mod.rs b/asic/src/chaos/mod.rs index e5d58831..36f7906a 100644 --- a/asic/src/chaos/mod.rs +++ b/asic/src/chaos/mod.rs @@ -9,6 +9,7 @@ use serde::{Deserialize, Serialize}; use slog::Logger; use std::collections::HashMap; use std::sync::Mutex; +use std::sync::OnceLock; use tokio::sync::mpsc; #[cfg(feature = "multicast")] @@ -344,6 +345,7 @@ pub struct Handle { ports: Mutex>, config: AsicConfig, log: Logger, + updates: OnceLock>, } impl Handle { @@ -353,6 +355,7 @@ impl Handle { ports: Mutex::new(HashMap::new()), config: config.clone(), log: log.clone(), + updates: OnceLock::new(), }) } /// Chaos ASICs always report as a model. @@ -494,6 +497,22 @@ impl AsicOps for Handle { unfurl!(self, port_enable_set); let mut ports = self.ports.lock().unwrap(); get_port_mut(&mut ports, port_hdl)?.enabled = val; + + let Some(chan) = self.updates.get() else { + return Ok(()); + }; + + let asic_port_id = self.port_to_asic_id(port_hdl)?; + let r1 = chan.send(PortUpdate::Enable { asic_port_id, enabled: val }); + let r2 = chan.send(PortUpdate::LinkUp { asic_port_id, linkup: val }); + + if r1.is_err() || r2.is_err() { + return Err(AsicError::Internal( + "PortUpdate tx failed. That rx chan should never be dropped." + .to_string(), + )); + } + Ok(()) } @@ -579,9 +598,12 @@ impl AsicOps for Handle { fn register_port_update_handler( &self, - _tx_channel: mpsc::UnboundedSender, + tx_channel: mpsc::UnboundedSender, ) -> AsicResult<()> { unfurl!(self, register_port_update_handler); + if self.updates.set(tx_channel).is_err() { + return Err(AsicError::Exists); + } Ok(()) } diff --git a/common/Cargo.toml b/common/Cargo.toml index 7a062bb4..8c4f2533 100644 --- a/common/Cargo.toml +++ b/common/Cargo.toml @@ -6,6 +6,7 @@ edition = "2024" [features] multicast = [] +test-utils = [] [dependencies] anyhow.workspace = true diff --git a/common/src/addr.rs b/common/src/addr.rs new file mode 100644 index 00000000..7fcfc8da --- /dev/null +++ b/common/src/addr.rs @@ -0,0 +1,179 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/ +// +// Copyright 2026 Oxide Computer Company + +use std::fmt::Display; +use std::net::AddrParseError; +use std::net::IpAddr; +use std::net::Ipv4Addr; +use std::net::Ipv6Addr; +use std::str::FromStr; + +use thiserror::Error; + +/// Conversion helpers for [`std::net::IpAddr`] power users. +pub trait IpAddrLike: Sized + Copy + Into { + /// Constructs self from an IP address. + /// + /// This must not map IPv4 addresses to v6. + fn from_ip(addr: IpAddr) -> Option; + + /// Constructs an IP address from self. + fn to_ip(self) -> IpAddr; +} + +impl IpAddrLike for Ipv4Addr { + fn from_ip(addr: IpAddr) -> Option { + match addr { + IpAddr::V4(v4) => Some(v4), + IpAddr::V6(_) => None, + } + } + + fn to_ip(self) -> IpAddr { + IpAddr::V4(self) + } +} + +impl IpAddrLike for Ipv6Addr { + fn from_ip(addr: IpAddr) -> Option { + match addr { + IpAddr::V4(_) => None, + IpAddr::V6(v6) => Some(v6), + } + } + + fn to_ip(self) -> IpAddr { + IpAddr::V6(self) + } +} + +impl IpAddrLike for IpAddr { + fn from_ip(addr: IpAddr) -> Option { + Some(addr) + } + + fn to_ip(self) -> IpAddr { + self + } +} + +impl IpAddrLike for RouterIfaceIpAddr { + fn from_ip(addr: IpAddr) -> Option { + RouterIfaceIpAddr::try_new(addr).ok() + } + + fn to_ip(self) -> IpAddr { + *self.get() + } +} + +/// The reasons a conversion involving [`RouterIfaceIpAddr`] +/// may fail. +#[derive(Debug, Error)] +pub enum RouterIfaceIpAddrErr { + #[error("Forbidden address type 'LocalHost': {0}")] + LocalHost(IpAddr), + + #[error("Forbidden address type 'Unspecified': {0}")] + Unspecified(IpAddr), + + #[error("Forbidden address type 'Multicast': {0}")] + Multicast(IpAddr), + + #[error("Forbidden address type 'Link Local': {0}")] + LinkLocal(IpAddr), + + #[error("Forbidden address type 'Broadcast': {0}")] + Broadcast(IpAddr), + + #[error("Forbidden address type 'IPv4 mapped IPv6': {0}")] + Ipv4MappedV6(IpAddr), + + #[error("IP address could not be parsed: {0}")] + Parse(#[from] AddrParseError), +} + +/// An IP address that may be registered on a loopback +/// or link interface managed by DPD. +/// +/// Maybe be any IP address not among the following: +/// - Localhost +/// - Unspecified +/// - Multicast +/// - IPv4 broadcast +/// - IPv6 link local +/// - IPv6 mapped IPv4 +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct RouterIfaceIpAddr(IpAddr); + +impl RouterIfaceIpAddr { + /// Constructs a new checked address or fails if `addr` does + /// not meet type criteria. + /// + /// This was duplicated from a module in [sled agent](https://github.com/oxidecomputer/omicron/blob/4b9fd125ed6c790ee2c86502c6b481b216c7fc4f/sled-agent/types/versions/src/stronger_bgp_unnumbered_types/early_networking.rs#L255) + /// + /// Ideally sled-agent can someday just inherit this type. + pub fn try_new(addr: IpAddr) -> Result { + let v4 = Ipv4Addr::from_ip(addr); + let v6 = Ipv6Addr::from_ip(addr); + for (forbidden, err) in [ + (addr.is_loopback(), RouterIfaceIpAddrErr::LocalHost(addr)), + (addr.is_unspecified(), RouterIfaceIpAddrErr::Unspecified(addr)), + (addr.is_multicast(), RouterIfaceIpAddrErr::Multicast(addr)), + ( + v4.is_some_and(|a| a.is_broadcast()), + RouterIfaceIpAddrErr::Broadcast(addr), + ), + ( + v6.is_some_and(|a| a.is_unicast_link_local()), + RouterIfaceIpAddrErr::LinkLocal(addr), + ), + ( + v6.is_some_and(|a| a.to_ipv4_mapped().is_some()), + RouterIfaceIpAddrErr::Ipv4MappedV6(addr), + ), + ] { + if forbidden { + return Err(err); + } + } + + Ok(Self(addr)) + } + + /// Retrieves the inner IP address. + pub fn get(&self) -> &IpAddr { + &self.0 + } +} + +impl Display for RouterIfaceIpAddr { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) + } +} + +impl FromStr for RouterIfaceIpAddr { + type Err = RouterIfaceIpAddrErr; + + fn from_str(s: &str) -> Result { + Self::try_new(s.parse()?) + } +} + +impl From for IpAddr { + fn from(value: RouterIfaceIpAddr) -> Self { + value.0 + } +} + +impl TryFrom for RouterIfaceIpAddr { + type Error = RouterIfaceIpAddrErr; + + fn try_from(value: IpAddr) -> Result { + Self::try_new(value) + } +} diff --git a/common/src/lib.rs b/common/src/lib.rs index 25ed16b8..2f8f8f31 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -11,6 +11,7 @@ use std::sync::Once; use std::time::Duration; use std::time::Instant; +pub mod addr; pub mod attached_subnet; pub mod counters; pub mod logging; @@ -19,6 +20,9 @@ pub mod network; pub mod ports; pub mod table; +#[cfg(feature = "test-utils")] +pub mod test_utils; + #[cfg(target_os = "illumos")] pub mod illumos; mod smf; diff --git a/common/src/test_utils/mod.rs b/common/src/test_utils/mod.rs new file mode 100644 index 00000000..5095c0e5 --- /dev/null +++ b/common/src/test_utils/mod.rs @@ -0,0 +1,227 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/ +// +// Copyright 2026 Oxide Computer Company + +//! Helpers that are useful in tests across dendrite crates. +//! These live behind a cargo feature that should probably only +//! be enabled in test contexts. + +use std::collections::HashSet; +use std::net::IpAddr; +use std::net::Ipv4Addr; +use std::net::Ipv6Addr; + +use oxnet::UnicastLinkLocalIpv6Addr; +use rand::Rng; +use rand::RngCore; +use rand::SeedableRng; +use rand::rngs::StdRng; + +use crate::addr::RouterIfaceIpAddr; + +pub mod retry; +pub use crate::fname; +pub use crate::fname_hash; + +/// Returns a string of the calling function's name without +/// any parent path. +#[macro_export] +macro_rules! fname { + () => {{ + fn f() {} + let path = ::std::any::type_name_of_val(&f); + path.rsplit("::") + .filter(|name| *name != "{{closure}}") + .nth(1) + .expect("fname must be called inside a function") + }}; +} + +/// Hashes the name of this function into a u64. +#[macro_export] +macro_rules! fname_hash { + () => {{ + let mut hasher = ::std::hash::DefaultHasher::new(); + let name = $crate::fname!(); + ::std::hash::Hash::hash(&name, &mut hasher); + ::std::hash::Hasher::finish(&hasher) + }}; +} + +/// A random IP address generator. +pub struct IpRng { + rng: StdRng, + claimed: HashSet, +} + +impl IpRng { + /// Creates a new ip address generator from the given seed. + pub fn new(seed: u64) -> Self { + Self { rng: StdRng::seed_from_u64(seed), claimed: HashSet::default() } + } + + /// Returns a random IPv4 address that this instance + /// has never created before. + pub fn unique_ipv4(&mut self) -> Ipv4Addr { + Self::roll_unique(&mut self.claimed, || { + Ipv4Addr::from_bits(self.rng.random()) + }) + } + + /// Returns a random IPv6 address that this instance + /// has never created before. + pub fn unique_ipv6(&mut self) -> Ipv6Addr { + Self::roll_unique(&mut self.claimed, || { + Ipv6Addr::from_bits(self.rng.random()) + }) + } + + /// Returns a random IPv6 address that this instance + /// has never created before. + pub fn link_local_ipv6(&mut self) -> Ipv6Addr { + // https://www.rfc-editor.org/info/rfc4291/#section-2.5.6 + loop { + let v6 = Ipv6Addr::from_bits( + (0xFE80u128 << 112) | self.rng.next_u64() as u128, + ); + UnicastLinkLocalIpv6Addr::new(v6) + .expect("Should be valid link local addr"); + if self.claimed.insert(v6.into()) { + return v6; + } + } + } + + /// Calls [`IpRng::unique_ipv4`] until rolling an address that + /// satisfies [`RouterIfaceIpAddr`]. + pub fn routable_ipv4(&mut self) -> Ipv4Addr { + loop { + let addr = self.unique_ipv4(); + if RouterIfaceIpAddr::try_new(addr.into()).is_ok() { + return addr; + } + } + } + + /// Calls [`IpRng::unique_ipv6`] until rolling an address that + /// satisfies [`RouterIfaceIpAddr`]. + pub fn routable_ipv6(&mut self) -> Ipv6Addr { + loop { + let addr = self.unique_ipv6(); + if RouterIfaceIpAddr::try_new(addr.into()).is_ok() { + return addr; + } + } + } + + /// Creates an Oxide ULA IPv6 address given a bottom 64 ID. + /// + /// All underlay addresses generated with this function belong + /// to the same (arbitrary) rack prefix. + pub fn host_underlay(&mut self) -> Ipv6Addr { + // Example sled from RFD 63: + // https://rfd.shared.oxide.computer/rfd/0063#_ipv6_addressing + const HOST: u128 = + Ipv6Addr::new(0xFD00, 0x1122, 0x3344, 0x0101, 0, 0, 0, 0).to_bits(); + + loop { + let id = self.rng.next_u64(); + let v6 = Ipv6Addr::from_bits(HOST | (id as u128)); + let ip = IpAddr::V6(v6); + if self.claimed.insert(ip) { + RouterIfaceIpAddr::try_new(ip) + .expect("underlay ULA is a valid router Ip"); + return v6; + } + } + } + + fn roll_unique( + tracker: &mut HashSet, + mut random_addr: impl FnMut() -> T, + ) -> T + where + T: Into + Copy, + { + loop { + // Executes infinitely if we've already generated the entire + // IPv4 or IPv6 address space, in which case the offending test + // has earned a more bespoke solution :) + let addr = random_addr(); + if tracker.insert(addr.into()) { + return addr; + } + } + } +} + +#[cfg(test)] +mod util_tests { + use std::collections::HashSet; + use std::net::IpAddr; + + use crate::addr::RouterIfaceIpAddr; + use crate::test_utils::IpRng; + + /// Basic check on unique ipv6 address generation. + #[test] + fn unique_v6() { + let mut rng = IpRng::new(7); + let one = rng.unique_ipv6(); + let two = rng.unique_ipv6(); + assert_ne!(one, two); + } + + /// Basic check on routable IP address generation. + #[test] + fn routable() { + let mut rng = IpRng::new(fname_hash!()); + let ct = 100; + let addrs: HashSet = (0..ct) + .flat_map(|_| { + [ + IpAddr::V4(rng.routable_ipv4()), + IpAddr::V6(rng.routable_ipv6()), + ] + .into_iter() + }) + .collect(); + + assert_eq!(addrs.len(), ct * 2); + for addr in addrs { + assert!(RouterIfaceIpAddr::try_new(addr).is_ok()); + } + } + + /// Verifies [`this_fn`] correctly extracts the calling function's name. + #[test] + fn test_fname() { + assert_eq!(fname!(), "test_fname"); + let test_closure = || { + assert_eq!(fname!(), "test_fname"); + }; + test_closure(); + } + + /// Verifies [`fname_hash`] stably hashes to unique values. + #[test] + fn test_fname_hash() { + fn one() -> u64 { + fname_hash!() + } + + fn two() -> u64 { + fname_hash!() + } + + fn three() -> u64 { + fname_hash!() + } + + assert_ne!(one(), two()); + assert_ne!(one(), three()); + assert_ne!(two(), three()); + } +} diff --git a/common/src/test_utils/retry.rs b/common/src/test_utils/retry.rs new file mode 100644 index 00000000..3e9267e0 --- /dev/null +++ b/common/src/test_utils/retry.rs @@ -0,0 +1,66 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/ +// +// Copyright 2026 Oxide Computer Company + +use std::collections::HashMap; +use std::fmt::Write; +use std::time::Duration; + +use anyhow::bail; +use thiserror::Error; + +/// The status of a retryable operation. +#[derive(Error, Debug)] +pub enum ReturnCode { + #[error("Operation failed but should be retried later: {0}")] + Retry(String), + + #[error("Fatal error: {0}")] + Fatal(String), +} + +/// Attempts `op` on a sleeping interval until success or +/// `poll_max` timeout. +pub async fn retry_op( + poll_interval: Duration, + poll_max: Duration, + mut op: impl AsyncFnMut() -> Result<(), ReturnCode>, +) -> anyhow::Result<()> { + let mut errors: HashMap = HashMap::new(); + + let fut = tokio::time::timeout(poll_max, async { + loop { + let retry_msg = match op().await { + Ok(()) => return Ok(()), + Err(ReturnCode::Fatal(e)) => { + return Err(anyhow::anyhow!(e)); + } + Err(ReturnCode::Retry(msg)) => msg, + }; + *errors.entry(retry_msg).or_default() += 1; + tokio::time::sleep(poll_interval).await; + } + }); + + if let Ok(success_or_fatal) = fut.await { + // Either success or fatal. + return success_or_fatal; + } + + let mut emsg = "Timed out. Errors:".to_string(); + let empty = errors.is_empty(); + for (err, ct) in errors { + writeln!(&mut emsg, "- Count {ct}: {err}")?; + } + + if empty { + writeln!( + &mut emsg, + "- No errors reported, operation never completed." + )?; + } + + bail!(emsg); +} diff --git a/dpd-client/Cargo.toml b/dpd-client/Cargo.toml index 62cba457..8e0c4bf0 100644 --- a/dpd-client/Cargo.toml +++ b/dpd-client/Cargo.toml @@ -34,6 +34,7 @@ packet = { path = "../packet" } pcap = { path = "../pcap" } asic = { path = "../asic" } anyhow.workspace = true +common = { workspace = true, features = [ "test-utils" ] } crc.workspace = true dpd-types.workspace = true lazy_static.workspace = true @@ -42,5 +43,6 @@ rand.workspace = true slog.workspace = true slog-async.workspace = true slog-term.workspace = true +thiserror.workspace = true toml.workspace = true pretty_assertions.workspace = true diff --git a/dpd-client/tests/chaos_tests/port_settings.rs b/dpd-client/tests/chaos_tests/port_settings.rs index bd781779..568aad15 100644 --- a/dpd-client/tests/chaos_tests/port_settings.rs +++ b/dpd-client/tests/chaos_tests/port_settings.rs @@ -9,20 +9,32 @@ use super::harness::{ new_dpd_client, run_dpd, }; use super::util::{link_list_ipv4, link_list_ipv6}; +use crate::chaos_tests::harness; +use crate::chaos_tests::util::HttpResponseCheck; + +use anyhow::Context; use asic::chaos::{AsicConfig, Chaos, TableChaos}; use asic::table_chaos; +use common::addr::IpAddrLike; use common::table::TableType; +use common::test_utils::IpRng; +use common::test_utils::fname; +use common::test_utils::fname_hash; +use common::test_utils::retry; +use common::test_utils::retry::ReturnCode; use dpd_client::types::{ - LinkCreate, LinkId, LinkSettings, PortFec, PortId, PortSettings, PortSpeed, + Ipv4Entry, Ipv6Entry, LinkCreate, LinkId, LinkSettings, LinkState, PortFec, + PortId, PortSettings, PortSpeed, }; use dpd_client::{Client, ROLLBACK_FAILURE_ERROR_CODE}; use http::status::StatusCode; use pretty_assertions::{Comparison, assert_eq}; -use rand::Rng; -use std::collections::HashMap; -use std::net::{Ipv4Addr, Ipv6Addr}; -use std::sync::Arc; +use rand::{Rng, RngCore}; +use std::borrow::Cow; +use std::collections::{BTreeSet, HashMap}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use std::sync::atomic::{AtomicU8, Ordering}; +use std::sync::{Arc, LazyLock}; use tokio::time::Duration; const TESTING_RADIX: usize = 33; @@ -30,44 +42,24 @@ const TESTING_RADIX: usize = 33; // reached a stable state, how frequently should they be retried and when should // we give up? const RETRY_INTERVAL: Duration = Duration::from_millis(200); -const RETRY_MAX: Duration = Duration::from_secs(5); - -#[cfg(test)] -mod retry { - use std::future::Future; - use std::time::Duration; - use std::time::Instant; - - pub enum ReturnCode { - Retry(String), - Fatal(String), - } +const RETRY_MAX: Duration = Duration::from_secs(90); + +/// A `LinkCreate` config with common defaults. +const TEST_LINK_CONF: LinkCreate = LinkCreate { + lane: None, + autoneg: false, + kr: false, + speed: PortSpeed::Speed100G, + fec: Some(PortFec::None), + tx_eq: None, + allow_ddm_traffic: false, +}; - pub async fn retry_op( - poll_interval: Duration, - poll_max: Duration, - mut op: Func, - ) -> anyhow::Result<()> - where - Func: FnMut() -> Fut, - Fut: Future>, - { - let poll_start = Instant::now(); - loop { - let retry_msg = match op().await { - Ok(()) => return Ok(()), - Err(ReturnCode::Fatal(e)) => return Err(anyhow::anyhow!(e)), - Err(ReturnCode::Retry(msg)) => msg, - }; - - let duration = Instant::now().duration_since(poll_start); - if duration > poll_max { - return Err(anyhow::anyhow!("operation failed: {retry_msg}")); - } - tokio::time::sleep(poll_interval).await; - } - } -} +const TAG1: &str = "chaos1"; +const TAG2: &str = "chaos2"; +const LINK0: LinkId = LinkId(0); +static QSFP0: LazyLock = + LazyLock::new(|| "qsfp0".parse().expect("qsfp0 should be a valid port")); // A simple test to ensure that we can observe chaos from these tests. Chaos // ASIC errors come back with HTTP code 418 which makes them easily observable @@ -80,21 +72,10 @@ async fn test_basic_autoneg_chaos() -> anyhow::Result<()> { ..Default::default() }; - let (_guard, client) = init_harness("autoneg", &config); + let (_guard, client) = init_harness(fname!(), &config); let err = client - .link_create( - &"qsfp0".parse().unwrap(), - &LinkCreate { - lane: None, - autoneg: false, - kr: false, - speed: PortSpeed::Speed100G, - fec: Some(PortFec::None), - tx_eq: None, - allow_ddm_traffic: false, - }, - ) + .link_create(&QSFP0, &TEST_LINK_CONF) .await .expect_err("Expected error on create"); @@ -115,32 +96,20 @@ async fn test_port_settings_addr_fail_1() -> anyhow::Result<()> { ..Default::default() }; - let (_guard, client) = init_harness("addr-fail-1", &config); + let (_guard, client) = init_harness(fname!(), &config); let mut settings = PortSettings { links: HashMap::new() }; settings.links.insert( "0".into(), LinkSettings { - params: LinkCreate { - lane: None, - autoneg: false, - kr: false, - fec: Some(PortFec::None), - speed: PortSpeed::Speed100G, - tx_eq: None, - allow_ddm_traffic: false, - }, + params: TEST_LINK_CONF, addrs: vec!["203.0.113.47".parse().unwrap()], }, ); let err = client - .port_settings_apply( - &"qsfp0".parse().unwrap(), - Some("chaos"), - &settings, - ) + .port_settings_apply(&QSFP0, Some("chaos"), &settings) .await .expect_err("Expected error on port settings apply"); @@ -157,33 +126,19 @@ async fn test_port_settings_addr_fail_1() -> anyhow::Result<()> { async fn test_port_settings_addr_success_1() -> anyhow::Result<()> { let config = AsicConfig { radix: TESTING_RADIX, ..Default::default() }; - let (_guard, client) = init_harness("addr-success", &config); + let (_guard, client) = init_harness(fname!(), &config); let mut settings = PortSettings { links: HashMap::new() }; settings.links.insert( "0".into(), LinkSettings { - params: LinkCreate { - lane: None, - autoneg: false, - kr: true, - fec: Some(PortFec::None), - speed: PortSpeed::Speed100G, - tx_eq: None, - allow_ddm_traffic: false, - }, + params: LinkCreate { kr: true, ..TEST_LINK_CONF }, addrs: vec!["203.0.113.47".parse().unwrap()], }, ); - client - .port_settings_apply( - &"qsfp0".parse().unwrap(), - Some("chaos"), - &settings, - ) - .await?; + client.port_settings_apply(&QSFP0, Some("chaos"), &settings).await?; let addrs = link_list_ipv4(&client, "qsfp0", "0").await.unwrap(); @@ -196,7 +151,7 @@ async fn test_port_settings_addr_success_1() -> anyhow::Result<()> { #[tokio::test] async fn test_port_settings_addr_success_multi() -> anyhow::Result<()> { let config = AsicConfig { radix: TESTING_RADIX, ..Default::default() }; - let (_guard, client) = init_harness("addr-success-multi", &config); + let (_guard, client) = init_harness(fname!(), &config); // Start with a link that has one IPv4 address. @@ -204,26 +159,12 @@ async fn test_port_settings_addr_success_multi() -> anyhow::Result<()> { settings.links.insert( "0".into(), LinkSettings { - params: LinkCreate { - lane: None, - autoneg: false, - kr: true, - fec: Some(PortFec::None), - speed: PortSpeed::Speed100G, - tx_eq: None, - allow_ddm_traffic: false, - }, + params: LinkCreate { kr: true, ..TEST_LINK_CONF }, addrs: vec!["203.0.113.47".parse().unwrap()], }, ); - client - .port_settings_apply( - &"qsfp0".parse().unwrap(), - Some("chaos"), - &settings, - ) - .await?; + client.port_settings_apply(&QSFP0, Some("chaos"), &settings).await?; let addrs = link_list_ipv4(&client, "qsfp0", "0").await.unwrap(); assert_eq!(addrs.len(), 1); @@ -234,15 +175,7 @@ async fn test_port_settings_addr_success_multi() -> anyhow::Result<()> { settings.links.insert( "0".into(), LinkSettings { - params: LinkCreate { - lane: None, - autoneg: false, - kr: true, - fec: Some(PortFec::None), - speed: PortSpeed::Speed100G, - tx_eq: None, - allow_ddm_traffic: false, - }, + params: LinkCreate { kr: true, ..TEST_LINK_CONF }, addrs: vec![ "203.0.113.46".parse().unwrap(), "203.0.113.48".parse().unwrap(), @@ -254,13 +187,7 @@ async fn test_port_settings_addr_success_multi() -> anyhow::Result<()> { }, ); - client - .port_settings_apply( - &"qsfp0".parse().unwrap(), - Some("chaos"), - &settings, - ) - .await?; + client.port_settings_apply(&QSFP0, Some("chaos"), &settings).await?; let addrs = link_list_ipv4(&client, "qsfp0", "0").await.unwrap(); assert_eq!(addrs.len(), 2); @@ -275,15 +202,7 @@ async fn test_port_settings_addr_success_multi() -> anyhow::Result<()> { settings.links.insert( "0".into(), LinkSettings { - params: LinkCreate { - lane: None, - autoneg: false, - kr: true, - fec: Some(PortFec::None), - speed: PortSpeed::Speed100G, - tx_eq: None, - allow_ddm_traffic: false, - }, + params: LinkCreate { kr: true, ..TEST_LINK_CONF }, addrs: vec![ "203.0.113.47".parse().unwrap(), "fd00:1701::d".parse().unwrap(), @@ -291,13 +210,7 @@ async fn test_port_settings_addr_success_multi() -> anyhow::Result<()> { }, ); - client - .port_settings_apply( - &"qsfp0".parse().unwrap(), - Some("chaos"), - &settings, - ) - .await?; + client.port_settings_apply(&QSFP0, Some("chaos"), &settings).await?; let addrs = link_list_ipv4(&client, "qsfp0", "0").await.unwrap(); assert_eq!(addrs.len(), 1); @@ -307,15 +220,13 @@ async fn test_port_settings_addr_success_multi() -> anyhow::Result<()> { // Clear all settings - client - .port_settings_clear(&"qsfp0".parse().unwrap(), Some("chaos")) - .await?; + client.port_settings_clear(&QSFP0, Some("chaos")).await?; // The addresses are all cleared synchronously, but the link deletion is // async. We pause briefly to give it a chance to complete. The subsequent // address list should either return an error (if the deletion finished) or // an empty list (if it didn't). - retry::retry_op(RETRY_INTERVAL, RETRY_MAX, || async { + retry::retry_op(RETRY_INTERVAL, RETRY_MAX, async || { match link_list_ipv4(&client, "qsfp0", "0").await { Err(e) if e.status() == Some(StatusCode::NOT_FOUND) => Ok(()), Err(e) => Err(retry::ReturnCode::Fatal(e.to_string())), @@ -358,19 +269,18 @@ const OPERATION_FAILURE_RATE: f64 = 0.1; #[tokio::test] async fn test_port_settings_txn_sweep() -> anyhow::Result<()> { let config = AsicConfig::uniform_set(TESTING_RADIX, OPERATION_FAILURE_RATE); - let (_guard, client) = init_harness("txn-sweep", &config); - let port: PortId = "qsfp0".parse().unwrap(); + let (_guard, client) = init_harness(fname!(), &config); let mut success = 0; let mut fail = 0; let mut rollback_fail = 0; for _ in 0..1000 { - let current = current_port_settings(&client, &port).await?; + let current = current_port_settings(&client, &QSFP0).await?; let target = random_port_settings(); print!("current/target: {}", Comparison::new(¤t, &target)); - match client.port_settings_apply(&port, Some("chaos"), &target).await { + match client.port_settings_apply(&QSFP0, Some("chaos"), &target).await { Ok(mut returned) => { sort_addrs(&mut returned); // Verify that what the server attempted to configure matches @@ -382,7 +292,7 @@ async fn test_port_settings_txn_sweep() -> anyhow::Result<()> { // any errors. We retry the test operation for a few seconds // waiting for that to happen. retry::retry_op(RETRY_INTERVAL, RETRY_MAX, || async { - match current_port_settings(&client, &port).await { + match current_port_settings(&client, &QSFP0).await { Err(e) => Err(retry::ReturnCode::Fatal(e.to_string())), Ok(new) => { if new == target { @@ -405,7 +315,7 @@ async fn test_port_settings_txn_sweep() -> anyhow::Result<()> { continue; } expect_random_chaos!(e); - let new = current_port_settings(&client, &port).await?; + let new = current_port_settings(&client, &QSFP0).await?; assert_eq!(new, current); print!("operation failed, settings remained as expected"); fail += 1; @@ -443,12 +353,11 @@ async fn test_port_settings_txn_par_sweep() -> anyhow::Result<()> { let rollback_fail = rollback_fail.clone(); let j = tokio::spawn(async move { - let port: PortId = "qsfp0".parse().unwrap(); let client = new_dpd_client(4705); let target = random_port_settings(); match client - .port_settings_apply(&port, Some("chaos"), &target) + .port_settings_apply(&QSFP0, Some("chaos"), &target) .await { Ok(mut returned) => { @@ -512,6 +421,7 @@ fn sort_addrs(settings: &mut PortSettings) { fn random_port_settings() -> PortSettings { let mut rng = rand::rng(); + let mut ip_rng = IpRng::new(rng.next_u64()); if rng.random::() < 0.15 { return PortSettings { links: HashMap::new() }; @@ -522,7 +432,7 @@ fn random_port_settings() -> PortSettings { // supported. let params = LinkCreate { - lane: Some(LinkId(0)), + lane: Some(LINK0), autoneg: rng.random(), kr: rng.random(), speed: PortSpeed::Speed100G, @@ -536,10 +446,10 @@ fn random_port_settings() -> PortSettings { let mut addrs = Vec::new(); for _ in 0..rng.random_range(0..15) { - addrs.push(Ipv4Addr::from(rng.random::()).into()); + addrs.push(ip_rng.routable_ipv4().to_ip()); } for _ in 0..rng.random_range(0..15) { - addrs.push(Ipv6Addr::from(rng.random::()).into()); + addrs.push(ip_rng.routable_ipv6().to_ip()); } // Because these routes are in a vector in the API we need to sort them for // comparison. @@ -552,3 +462,1449 @@ fn random_port_settings() -> PortSettings { )]), } } + +/// A simplified version of txn_sweep that ensures `port_settings_*` +/// functions can succeed after partial failures. +#[tokio::test] +async fn settings_eventually_reconcile() -> anyhow::Result<()> { + let config = AsicConfig::uniform_set(TESTING_RADIX, 0.4); + let (_guard, client) = harness::init_harness(fname!(), &config); + let mut rng = IpRng::new(fname_hash!()); + + let settings = PortSettings { + links: [( + LINK0.to_string(), + LinkSettings { + params: TEST_LINK_CONF, + addrs: vec![ + rng.routable_ipv4().into(), + rng.routable_ipv6().into(), + ], + }, + )] + .into_iter() + .collect(), + }; + + let mut apply = 0; + let mut clear = 0; + let mut get = 0; + + let print_retries = |a, c, g| { + println!( + " +Reconciliation retries: + - port_settings_apply: {a} + - port_settings_clear: {c} + - port_settings_get: {g} +" + ); + }; + + // Increase the odds of hitting failure by running + // the sequence a few times. + for _ in 0..3 { + retry::retry_op(RETRY_INTERVAL, RETRY_MAX, async || { + let Err(e) = + client.port_settings_apply(&QSFP0, Some(TAG1), &settings).await + else { + return Ok(()); + }; + apply += 1; + Err(ReturnCode::Retry(e.to_string())) + }) + .await + .inspect_err(|_| print_retries(apply, clear, get)) + .context("Timeout in port_settings_apply")?; + + retry::retry_op(RETRY_INTERVAL, RETRY_MAX, async || { + let Err(e) = client.port_settings_clear(&QSFP0, Some(TAG1)).await + else { + return Ok(()); + }; + clear += 1; + Err(ReturnCode::Retry(e.to_string())) + }) + .await + .inspect_err(|_| print_retries(apply, clear, get)) + .context("Timeout in port_settings_clear")?; + + retry::retry_op(RETRY_INTERVAL, RETRY_MAX, async || { + let err = match client.port_settings_get(&QSFP0, Some(TAG1)).await { + Ok(s) if s.links.is_empty() => return Ok(()), + Ok(s) => Err(ReturnCode::Retry(format!( + "Link settings should be empty. Found {s:?}" + ))), + Err(e) => Err(ReturnCode::Retry(e.to_string())), + }; + get += 1; + err + }) + .await + .inspect_err(|_| print_retries(apply, clear, get)) + .context("Timeout in port_settings_get")?; + } + + Ok(()) +} + +/// Verifies tagged port_settings_apply actions don't affect +/// resources from other tags. +#[tokio::test] +async fn settings_apply_respects_tags() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + + let mut rng = IpRng::new(fname_hash!()); + let link_id = + client.link_create(&QSFP0, &TEST_LINK_CONF).await?.into_inner(); + + let cxt = TestContext::new(&client, QSFP0.clone(), link_id); + let tag1 = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + let tag2 = TestAddrs::new(&cxt, &mut rng, TAG2.to_string()); + + tag1.create_addrs().await?; + tag2.apply_addrs().await?; + + tag1.verify_addrs_exist(Verify::NonExhaustive).await?; + tag2.verify_addrs_exist(Verify::NonExhaustive).await?; + + client + .port_settings_apply( + &QSFP0, + Some(TAG2), + &TestAddrs::empty_settings(link_id), + ) + .await?; + + tag1.verify_addrs_exist(Verify::Exhaustive).await?; + + Ok(()) +} + +/// Verifies that tags correctly isolate address CRUD amid table +/// errors and rollback failures. +#[tokio::test] +async fn settings_rollback_respects_tags() -> anyhow::Result<()> { + let table_fail = table_chaos![ + (TableType::PortAddrIpv4, 0.2), + (TableType::PortAddrIpv6, 0.2) + ]; + + let (_guard, client) = harness::init_harness( + fname!(), + &AsicConfig { + radix: TESTING_RADIX, + table_entry_add: table_fail.clone(), + table_entry_del: table_fail, + ..Default::default() + }, + ); + + let mut rng = IpRng::new(fname_hash!()); + let link_id = + client.link_create(&QSFP0, &TEST_LINK_CONF).await?.into_inner(); + + let cxt = TestContext::new(&client, QSFP0.clone(), link_id); + let mut tag1 = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + let mut tag2 = TestAddrs::new(&cxt, &mut rng, TAG2.to_string()); + + let mut check1 = AddrCheck::default(); + let mut check2 = AddrCheck::default(); + + let mut success1 = 0; + let mut success2 = 0; + let mut ct = 0; + + let status = tokio::time::timeout(Duration::from_secs(90), async { + // Some attempts may fail, but we should expect to see at least + // one success per tag. Otherwise we're probably encountering an + // unexpected failure. + while ct < 1000 || success1 == 0 || success2 == 0 { + ct += 1; + success1 += + check1.apply_and_check(&mut tag1, &mut rng).await? as usize; + success2 += + check2.apply_and_check(&mut tag2, &mut rng).await? as usize; + } + Ok(()) as anyhow::Result<()> + }) + .await; + + println!( + " +Attempts: {ct:?} +Tag 1 successes: {success1:?} +Tag 2 successes: {success2:?}" + ); + + status??; + Ok(()) +} + +/// Helper for validating link state through port_settings_apply +/// over time amid failures. +#[derive(Default)] +enum AddrCheck { + /// We haven't yet successfully applied addresses. + #[default] + Empty, + /// These are the most recent successfully-applied addresses. + Current { v4: Ipv4Addr, v6: Ipv6Addr }, + /// The link returned a rollback error and is an unknown state. + Inconsistent, +} + +impl AddrCheck { + /// Tries applying some addresses on a link. + /// + /// On success, ensures the addresses are present. Returns true. + /// On failure, returns false. For any non-rollback failure, ensures + /// the link was reset to a known-good state. + async fn apply_and_check( + &mut self, + addrs: &mut TestAddrs<'_>, + rng: &mut IpRng, + ) -> anyhow::Result { + if rand::rng().random_bool(0.75) { + addrs.v4_entry.addr = rng.routable_ipv4(); + } + + if rand::rng().random_bool(0.75) { + addrs.v6_entry.addr = rng.routable_ipv6(); + } + + match addrs.apply_addrs().await { + Ok(()) => { + addrs.verify_addrs_exist(Verify::NonExhaustive).await?; + *self = Self::Current { + v4: addrs.v4_entry.addr, + v6: addrs.v6_entry.addr, + }; + return Ok(true); + } + Err(e) if self::is_rollback_error(&e) => { + *self = Self::Inconsistent; + return Ok(false); + } + Err(_) => (), + }; + + // Handle non-rollback error. + match self { + Self::Current { v4, v6 } => { + addrs.v4_entry.addr = *v4; + addrs.v6_entry.addr = *v6; + addrs.verify_addrs_exist(Verify::NonExhaustive).await?; + } + Self::Inconsistent => { + // Can't assume anything until the next successful apply. + } + Self::Empty => { + let link_is_empty = addrs + .cxt + .client + .port_settings_get( + &addrs.cxt.port_id, + Some(&addrs.v4_entry.tag), + ) + .await? + .into_inner() + .links + .get(&addrs.cxt.link_id.to_string()) + .is_none_or(|link| link.addrs.is_empty()); + anyhow::ensure!( + link_is_empty, + "There should be no addrs for a port/link/tag in an empty state" + ); + } + }; + + Ok(false) + } +} + +/// Verifies tagged address_*_create and delete don't affect +/// resources under different tags. +#[tokio::test] +async fn address_cmds_respect_tags() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + + let mut rng = IpRng::new(fname_hash!()); + + let cxt = TestContext::new(&client, QSFP0.clone(), LINK0); + let tag1 = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + let tag2 = TestAddrs::new(&cxt, &mut rng, TAG2.to_string()); + + tag2.apply_addrs().await?; + tag1.apply_addrs().await?; + client + .link_ipv4_create( + &QSFP0, + &LINK0, + &Ipv4Entry { addr: tag2.v4_entry.addr, tag: TAG1.to_string() }, + ) + .await?; + + cxt.ensure_addrs([ + (tag1.tag(), tag1.v4_entry.addr.into()), + (tag1.tag(), tag1.v6_entry.addr.into()), + (tag1.tag(), tag2.v4_entry.addr.into()), + (tag2.tag(), tag2.v4_entry.addr.into()), + (tag2.tag(), tag2.v6_entry.addr.into()), + ]) + .await?; + + tag1.verify_addrs_exist(Verify::NonExhaustive).await?; + tag2.verify_addrs_exist(Verify::NonExhaustive).await?; + + client.link_ipv4_delete(&QSFP0, &LINK0, &tag1.v4_entry.addr).await?; + client.link_ipv6_delete(&QSFP0, &LINK0, &tag1.v6_entry.addr).await?; + + cxt.ensure_addrs([ + (tag2.tag(), tag2.v4_entry.addr.into()), + (tag2.tag(), tag2.v6_entry.addr.into()), + (tag1.tag(), tag2.v4_entry.addr.into()), + ]) + .await?; + + Ok(()) +} + +/// Verifies the current known behavior of port_settings_clear, which +/// is to delete the link and all its config from the port. +/// +/// This test isn't necessarily endorsing the implementation, but +/// it does seek to track API's current behavior. +#[tokio::test] +async fn settings_clear_ignores_tags() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + + let mut rng = IpRng::new(fname_hash!()); + let link_id = + client.link_create(&QSFP0, &TEST_LINK_CONF).await?.into_inner(); + + let cxt = TestContext::new(&client, QSFP0.clone(), link_id); + let tag1 = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + let tag2 = TestAddrs::new(&cxt, &mut rng, TAG2.to_string()); + + tag1.create_addrs().await?; + tag2.apply_addrs().await?; + + tag1.verify_addrs_exist(Verify::NonExhaustive).await?; + tag2.verify_addrs_exist(Verify::NonExhaustive).await?; + + // Tag2 means nothing here + client.port_settings_clear(&cxt.port_id, Some(TAG2)).await?; + + tag1.verify_addrs_exist(Verify::Exhaustive).await.expect_err( + "Addresses do not exist because we cleared the port settings.", + ); + tag2.verify_addrs_exist(Verify::NonExhaustive).await.expect_err( + "Addresses do not exist because we cleared the port settings.", + ); + + Ok(()) +} + +/// Verifies that link address creation fails when that +/// address has already been registered on a different interface. +#[tokio::test] +async fn create_addr_rejects_theft() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + let mut rng = IpRng::new(fname_hash!()); + let link_id = + client.link_create(&QSFP0, &TEST_LINK_CONF).await?.into_inner(); + + let cxt = TestContext::new(&client, QSFP0.clone(), link_id); + + let tag1 = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + let mut tag2 = TestAddrs::new(&cxt, &mut rng, TAG2.to_string()); + tag2.v4_entry.addr = tag1.v4_entry.addr; + tag2.v6_entry.addr = tag1.v6_entry.addr; + + tag1.apply_addrs().await?; + + client + .link_ipv4_create(&QSFP0, &link_id, &tag1.v4_entry) + .await + .expect_err("Duplicate ipv4 registration fails") + .expect_status(StatusCode::CONFLICT); + + client + .link_ipv6_create(&QSFP0, &link_id, &tag1.v6_entry) + .await + .expect_err("Duplicate ipv6 registration fails") + .expect_status(StatusCode::CONFLICT); + + tag2.create_addrs().await?; + tag1.apply_addrs().await?; + + let link4 = LinkId(4); + client + .link_create( + &QSFP0, + &LinkCreate { lane: Some(link4), ..TEST_LINK_CONF }, + ) + .await?; + client + .link_ipv4_create(&QSFP0, &link4, &tag1.v4_entry) + .await + .expect_err("v4 should fail because already registered on another link") + .expect_status(StatusCode::CONFLICT); + client + .link_ipv6_create(&QSFP0, &link4, &tag1.v6_entry) + .await + .expect_err("v6 should fail because already registered on another link") + .expect_status(StatusCode::CONFLICT); + + let cxt_port2 = TestContext::new(&client, "qsfp2".parse()?, link_id); + let tag1_port2 = TestAddrs { + v4_entry: tag1.v4_entry.clone(), + v6_entry: tag1.v6_entry.clone(), + cxt: &cxt_port2, + }; + tag1_port2 + .apply_addrs() + .await + .expect_err("Should fail because already registered on another port") + .expect_status(StatusCode::CONFLICT); + + tag1.verify_addrs_exist(Verify::NonExhaustive).await?; + tag2.verify_addrs_exist(Verify::NonExhaustive).await?; + + Ok(()) +} + +/// Verifies deletion fails when the address does not exist. +#[tokio::test] +async fn deleted_address_must_exist() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + let mut rng = IpRng::new(fname_hash!()); + + let cxt = TestContext::new(&client, QSFP0.clone(), LINK0); + let tag1 = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + + tag1.apply_addrs().await?; + + client + .link_ipv4_delete(&QSFP0, &LINK0, &rng.routable_ipv4()) + .await + .expect_err("Deleting a non-existent IPv4 addr fails") + .expect_status(StatusCode::NOT_FOUND); + + client + .link_ipv6_delete(&QSFP0, &LINK0, &rng.routable_ipv6()) + .await + .expect_err("Deleting a non-existent IPv6 addr fails") + .expect_status(StatusCode::NOT_FOUND); + + // Deletion is not currently tagged, so there's no way/reason to test + // tag conflicts. + + client.link_ipv4_delete(&QSFP0, &LINK0, &tag1.v4_entry.addr).await?; + client.link_ipv6_delete(&QSFP0, &LINK0, &tag1.v6_entry.addr).await?; + tag1.verify_addrs_exist(Verify::NonExhaustive) + .await + .expect_err("Addresses were deleted"); + + Ok(()) +} + +/// One tagged `port_settings_apply` can delete a link created +/// by another tag's `port_settings_apply`. This isn't functionality +/// we necessarily want, but we should also never exercise it in practice. +/// Since it's a subtle footgun, this test at least documents its existence +/// and observes if this ever changes. +#[tokio::test] +async fn apply_implicit_delete() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + let mut rng = IpRng::new(fname_hash!()); + + let cxt1 = TestContext::new(&client, QSFP0.clone(), LinkId(1)); + let cxt2 = TestContext::new(&client, QSFP0.clone(), LinkId(2)); + + let tag1 = TestAddrs::new(&cxt1, &mut rng, TAG1.to_string()); + let tag2 = TestAddrs::new(&cxt2, &mut rng, TAG2.to_string()); + + tag1.apply_addrs().await?; + tag1.verify_addrs_exist(Verify::Exhaustive).await?; + + tag2.apply_addrs().await?; + + tag1.verify_addrs_exist(Verify::NonExhaustive) + .await + .expect_err("Link 1 was deleted"); + tag2.verify_addrs_exist(Verify::Exhaustive).await?; + + Ok(()) +} + +/// Links use tags as a reference count for addresses. An address +/// may accumulate tags and should only be freed when the last +/// tag is released. +#[tokio::test] +async fn apply_allows_multiple_tags() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + let mut rng = IpRng::new(fname_hash!()); + + let cxt = TestContext::new(&client, QSFP0.clone(), LINK0); + let tag1 = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + let mut tag2 = TestAddrs::new(&cxt, &mut rng, TAG2.to_string()); + + tag2.v4_entry.addr = tag1.v4_entry.addr; + tag2.v6_entry.addr = tag1.v6_entry.addr; + + client.link_create(&QSFP0, &TEST_LINK_CONF).await?; + tag2.create_addrs().await?; + tag1.apply_addrs().await?; + + tag1.verify_addrs_exist(Verify::NonExhaustive).await?; + tag2.verify_addrs_exist(Verify::NonExhaustive).await?; + + client.link_ipv4_delete(&QSFP0, &LINK0, &tag1.v4_entry.addr).await?; + + cxt.ensure_addrs([ + (tag1.tag(), tag1.ipv6()), + // (tag2.tag(), tag2.ipv4()), requires tagged deletion + (tag2.tag(), tag2.ipv6()), + ]) + .await?; + + tag1.apply_addrs().await?; + tag1.verify_addrs_exist(Verify::NonExhaustive).await?; + tag2.apply_addrs().await?; + + cxt.client + .port_settings_apply( + &QSFP0, + Some(TAG1), + &TestAddrs::empty_settings(LINK0), + ) + .await?; + + tag1.verify_addrs_exist(Verify::NonExhaustive) + .await + .expect_err("Addrs were cleared"); + tag2.verify_addrs_exist(Verify::Exhaustive).await?; + + cxt.client.link_ipv4_delete(&QSFP0, &LINK0, &tag2.v4_entry.addr).await?; + cxt.client.link_ipv6_delete(&QSFP0, &LINK0, &tag2.v6_entry.addr).await?; + + cxt.ensure_addrs([]).await?; + + Ok(()) +} + +/// A pathological sequence of table errors in dpd must +/// not poison future valid operations. +/// +/// This test port_settings_applies two addresses. The IPv6 +/// table op fails, and then the IPv4 table op in the rollback +/// fails. After that rollback failure, the problematic IPv4 +/// entry still belongs to the link, so it can at least be +/// reused in the next port_settings_apply. +#[tokio::test] +async fn partial_failures_are_recoverable() -> anyhow::Result<()> { + let conf = AsicConfig { + radix: TESTING_RADIX, + table_entry_add: table_chaos!((TableType::PortAddrIpv6, 1.0)), + table_entry_del: table_chaos!((TableType::PortAddrIpv4, 1.0)), + ..Default::default() + }; + + let (_guard, client) = harness::init_harness(fname!(), &conf); + let mut rng = IpRng::new(fname_hash!()); + + let cxt = TestContext::new(&client, QSFP0.clone(), LINK0); + let addrs = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + + let apply_err = addrs + .apply_addrs() + .await + .expect_err("Apply should fail because IPv6 table ops fail"); + assert!( + self::is_rollback_error(&apply_err), + "IPv4 addr couldn't be rolled back from table" + ); + + client + .port_settings_clear(&QSFP0, Some(TAG1)) + .await + .expect_err("Port couldn't be cleared because IPv4 addr is stuck"); + + let mut v4_only = addrs.settings(); + for link in v4_only.links.values_mut() { + link.addrs.retain(|a| a.is_ipv4()); + } + + client + .port_settings_apply(&"qsfp1".parse()?, Some(TAG2), &v4_only) + .await + .expect_err("Stuck entry cannot be stolen by another tag."); + + for link in v4_only.links.values_mut() { + link.addrs.push(rng.routable_ipv4().into()); + } + + let mut applied = client + .port_settings_apply(&QSFP0, Some(TAG1), &v4_only) + .await + .context("Apply should succeed because we can at least overwrite the IPv4 table entry")? + .into_inner(); + + self::sort_addrs(&mut v4_only); + self::sort_addrs(&mut applied); + + assert_eq!( + v4_only, applied, + "Stuck IPv4 addr should still be usable in port_settings_apply" + ); + + Ok(()) +} + +/// A flaky table write must not poison link initialization. +#[tokio::test] +#[cfg(feature = "multicast")] +async fn link_init_recovers() -> anyhow::Result<()> { + let conf = AsicConfig { + radix: TESTING_RADIX, + table_entry_add: table_chaos!((TableType::McastEgressPortMapping, 0.8)), + ..Default::default() + }; + let (_guard, client) = harness::init_harness(fname!(), &conf); + + let link_id = + client.link_create(&QSFP0, &TEST_LINK_CONF).await?.into_inner(); + + retry::retry_op(RETRY_INTERVAL, RETRY_MAX, async || { + let state = client + .link_get(&QSFP0, &link_id) + .await + .map_err(|e| ReturnCode::Fatal(e.to_string()))? + .into_inner() + .link_state; + match state { + LinkState::Up => Ok(()), + e @ (LinkState::Faulted(_) | LinkState::Down) => { + Err(ReturnCode::Fatal(format!("Unexpected link state: {e:?}"))) + } + s @ (LinkState::Unknown | LinkState::ConfigError(_)) => { + // Enabling the link pokes the reconciler, which speeds up the test. + client + .link_enabled_set(&QSFP0, &link_id, true) + .await + .map_err(|e| ReturnCode::Fatal(e.to_string()))?; + Err(ReturnCode::Retry(format!( + "Waiting for LinkState::Up. Currently {s:?}" + ))) + } + } + }) + .await?; + + Ok(()) +} + +/// One does not simply double-register a link address on loopback. +/// +/// This tests the order where link registration wins. +#[tokio::test] +async fn link_addrs_are_isolated() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + let mut rng = IpRng::new(fname_hash!()); + + let link_id = + client.link_create(&QSFP0, &TEST_LINK_CONF).await?.into_inner(); + + let cxt = TestContext::new(&client, QSFP0.clone(), link_id); + let tag1 = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + + tag1.create_addrs().await?; + + client + .loopback_ipv4_create(&tag1.v4_entry) + .await + .expect_err("This link address IPv4 is already registered"); + + client + .loopback_ipv6_create(&tag1.v6_entry) + .await + .expect_err("This link address IPv6 is already registered"); + + tag1.verify_addrs_exist(Verify::Exhaustive).await?; + + Ok(()) +} + +/// Reverse of [`addrs_are_isolated`]. Now loopback registration wins. +#[tokio::test] +async fn loopback_addrs_are_isolated() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + let mut rng = IpRng::new(fname_hash!()); + + let link_id = + client.link_create(&QSFP0, &TEST_LINK_CONF).await?.into_inner(); + + let cxt = TestContext::new(&client, QSFP0.clone(), link_id); + let tag1 = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + + client.loopback_ipv4_create(&tag1.v4_entry).await?; + client.loopback_ipv6_create(&tag1.v6_entry).await?; + + client + .link_ipv4_create(&QSFP0, &link_id, &tag1.v4_entry) + .await + .expect_err("Address already exists on IPv4 loopback"); + client + .link_ipv6_create(&QSFP0, &link_id, &tag1.v6_entry) + .await + .expect_err("Address already exists on IPv6 loopback"); + + tag1.apply_addrs().await.expect_err("Addrs collide and cannot be created"); + + let v4_list = client.loopback_ipv4_list().await?; + assert_eq!( + &v4_list.into_inner(), + std::slice::from_ref(&tag1.v4_entry), + "Loopback should have IPv4 addr" + ); + + let v6_list = client.loopback_ipv6_list().await?; + assert_eq!( + &v6_list.into_inner(), + std::slice::from_ref(&tag1.v6_entry), + "Loopback should have IPv6 addr" + ); + + Ok(()) +} + +/// DPD table state mirrors asic table state. DPD must not drop a +/// resource until it has been successfully cleared from the asic. +/// Doing otherwise would effectively leak asic table memory. +/// +/// This test constructs an "asic" entry that deterministically cannot +/// be cleared, meaning that it should also be impossible to delete +/// from DPD. +#[tokio::test] +async fn deletion_doesnt_leak_table_entries() -> anyhow::Result<()> { + let config = AsicConfig { + radix: TESTING_RADIX, + table_entry_del: table_chaos!((TableType::PortAddrIpv4, 1.0)), + ..Default::default() + }; + let (_guard, client) = harness::init_harness(fname!(), &config); + let mut rng = IpRng::new(fname_hash!()); + + let cxt = TestContext::new(&client, QSFP0.clone(), LINK0); + let tag1 = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + + tag1.apply_addrs().await?; + + let msg = "Link should not be dropped because that would orphan the stuck table entry"; + client.port_settings_clear(&QSFP0, None).await.expect_err(msg); + client + .port_settings_apply( + &QSFP0, + Some(TAG1), + &TestAddrs::empty_settings(LINK0), + ) + .await + .expect_err("Addr cannot be implicitly deleted via apply"); + tag1.verify_addrs_exist(Verify::Exhaustive).await?; + + client.link_delete(&QSFP0, &LINK0).await.expect_err(msg); + // link_delete makes no atomicity guarantees, so the IPv6 address + // might be gone. But the link and stuck address should still exist. + assert_eq!( + &client.link_ipv4_list(&QSFP0, &LINK0, None, None).await?.items, + std::slice::from_ref(&tag1.v4_entry) + ); + + Ok(()) +} + +/// Link deletion can succeed after prior failures. +#[tokio::test] +async fn deletion_prevails() -> anyhow::Result<()> { + let config = AsicConfig { + radix: TESTING_RADIX, + table_entry_del: table_chaos![ + (TableType::PortAddrIpv4, 0.7), + (TableType::PortAddrIpv6, 0.7) + ], + ..Default::default() + }; + let (_guard, client) = harness::init_harness(fname!(), &config); + let mut rng = IpRng::new(fname_hash!()); + + let cxt = TestContext::new(&client, QSFP0.clone(), LINK0); + let tag1 = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + + tag1.apply_addrs().await?; + + let mut retry_ct = 0; + retry::retry_op(Duration::ZERO, RETRY_MAX, async || { + let applied = client + .port_settings_apply( + &QSFP0, + Some(TAG1), + &TestAddrs::empty_settings(LINK0), + ) + .await; + + let Err(e) = applied else { + return Ok(()); + }; + + retry_ct += 1; + Err(ReturnCode::Retry(e.to_string())) + }) + .await + .context("Timeout trying to successfully clear addresses")?; + + let cleared_addrs = client + .port_settings_get(&QSFP0, Some(TAG1)) + .await? + .links + .get(&LINK0.to_string()) + .expect("Link should exist") + .addrs + .is_empty(); + assert!( + cleared_addrs, + "port_settings_apply should only succeed once the addresses are removed" + ); + + println!("Retries: {retry_ct}"); + + Ok(()) +} + +/// Given an IP + Tag belonging to another interface, this calls +/// port_settings_apply from the given conf and tries to steal +/// that address. +/// +/// Returns the result of the apply command, which should probably +/// always be an error. +async fn try_steal( + conf: &mut TestAddrs<'_>, + steal_ip: IpAddr, + current_tag: String, +) -> anyhow::Result<()> { + match steal_ip { + IpAddr::V4(v4) => { + let entry = Ipv4Entry { addr: v4, tag: current_tag }; + let prev = std::mem::replace(&mut conf.v4_entry, entry); + let status = conf.apply_addrs().await; + conf.v4_entry = prev; + status?; + } + IpAddr::V6(v6) => { + let entry = Ipv6Entry { addr: v6, tag: current_tag }; + let prev = std::mem::replace(&mut conf.v6_entry, entry); + let status = conf.apply_addrs().await; + conf.v6_entry = prev; + status?; + } + } + + Ok(()) +} + +/// An address is owned exclusively across the switch, and it must be +/// released to transfer owners. +/// +/// Equivalently, a link/loopback address registration will never +/// succeed if that address is already owned by another +/// link/loopback source. +#[tokio::test] +async fn loopback_cannot_steal_addrs() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + + let port1: PortId = "qsfp0".parse()?; + let port2: PortId = "qsfp1".parse()?; + let mut rng = IpRng::new(fname_hash!()); + + let cxt1 = TestContext::new(&client, port1.clone(), LINK0); + let cxt2 = TestContext::new(&client, port2.clone(), LINK0); + + let p1 = TestAddrs::new(&cxt1, &mut rng, TAG1.to_string()); + let mut p2 = TestAddrs::new(&cxt2, &mut rng, TAG1.to_string()); + + let loopback_v4 = + Ipv4Entry { addr: rng.routable_ipv4(), tag: TAG1.to_string() }; + let loopback_v6 = + Ipv6Entry { addr: rng.routable_ipv6(), tag: TAG1.to_string() }; + + p1.apply_addrs().await?; + p2.apply_addrs().await?; + client.loopback_ipv4_create(&loopback_v4).await?; + client.loopback_ipv6_create(&loopback_v6).await?; + + self::try_steal(&mut p2, p1.v4_entry.addr.into(), TAG1.to_string()) + .await + .expect_err("v4 address is already registered under p1"); + + self::try_steal(&mut p2, p1.v6_entry.addr.into(), TAG1.to_string()) + .await + .expect_err("v6 address is already registered under p1"); + + self::try_steal(&mut p2, loopback_v4.addr.into(), TAG1.to_string()) + .await + .expect_err("v4 address is already registered under loopback"); + + self::try_steal(&mut p2, loopback_v6.addr.into(), TAG1.to_string()) + .await + .expect_err("v6 address is already registered under loopback"); + + client + .loopback_ipv4_create(&p1.v4_entry) + .await + .expect_err("v4 address is already registered under p1"); + client + .loopback_ipv6_create(&p1.v6_entry) + .await + .expect_err("v6 address is already registered under p1"); + + p1.verify_addrs_exist(Verify::Exhaustive).await?; + p2.verify_addrs_exist(Verify::Exhaustive).await?; + + assert_eq!( + &client.loopback_ipv4_list().await?.into_inner(), + std::slice::from_ref(&loopback_v4) + ); + + assert_eq!( + &client.loopback_ipv6_list().await?.into_inner(), + std::slice::from_ref(&loopback_v6) + ); + + Ok(()) +} + +/// Verifies CRUD actions with multiple tags on a single address. +#[tokio::test] +async fn basic_multiple_tag_crud() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + + let mut rng = IpRng::new(fname_hash!()); + let v4 = rng.routable_ipv4(); + let v6 = rng.routable_ipv6(); + let tags: [Cow<'static, str>; 10] = + std::array::from_fn(|idx| format!("chaos{idx}").into()); + + let link_id = client.link_create(&QSFP0, &TEST_LINK_CONF).await?; + let cxt = TestContext::new(&client, QSFP0.clone(), LINK0); + let mut registered = BTreeSet::new(); + + for tag in tags { + let v4_entry = Ipv4Entry { addr: v4, tag: tag.as_ref().to_string() }; + let v6_entry = Ipv6Entry { addr: v6, tag: tag.as_ref().to_string() }; + + client.link_ipv4_create(&QSFP0, &link_id, &v4_entry).await?; + client + .link_ipv4_create(&QSFP0, &link_id, &v4_entry) + .await + .expect_err("duplicate v4 reg should fail") + .expect_status(StatusCode::CONFLICT); + + client.link_ipv6_create(&QSFP0, &link_id, &v6_entry).await?; + client + .link_ipv6_create(&QSFP0, &link_id, &v6_entry) + .await + .expect_err("duplicate v6 reg should fail") + .expect_status(StatusCode::CONFLICT); + + registered.insert((tag.clone(), v4.into())); + registered.insert((tag, v6.into())); + + let fetched = cxt.fetch_all_addrs().await?; + assert_eq!( + registered, fetched, + "fetched addrs should match what was registered" + ); + + // TODO::cory: revive this once dpd api supports tags in page tokens + // Verify pagination works identically. + // let one = NonZeroU32::new(1).unwrap(); + // let mut fetched_stream: BTreeSet<(Cow<'static, str>, IpAddr)> = cxt + // .client + // .link_ipv4_list_stream(&cxt.port_id, &cxt.link_id, Some(one)) + // .map_ok(|entry| (entry.tag.into(), entry.addr.into())) + // .try_collect() + // .await?; + + // let mut v6s = cxt.client.link_ipv6_list_stream( + // &cxt.port_id, + // &cxt.link_id, + // Some(one), + // ); + // while let Some(entry) = v6s.next().await { + // let entry = entry?; + // fetched_stream.insert((entry.tag.into(), entry.addr.into())); + // } + // assert_eq!( + // fetched, fetched_stream, + // "addr fetch must be identical with and without stream" + // ); + } + + // TODO::cory: finish this once we support tagged deletion. + + Ok(()) +} + +/// Verifies that one link with one tag cannot register an +/// address belonging to another link with another tag. +#[tokio::test] +async fn links_cannot_steal_addrs_across_tags() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + + let port1: PortId = "qsfp0".parse()?; + let port2: PortId = "qsfp1".parse()?; + let mut rng = IpRng::new(fname_hash!()); + + let cxt1 = TestContext::new(&client, port1, LINK0); + let cxt2 = TestContext::new(&client, port2, LINK0); + + let p1 = TestAddrs::new(&cxt1, &mut rng, TAG1.to_string()); + let mut p2 = TestAddrs::new(&cxt2, &mut rng, TAG2.to_string()); + p2.v6_entry.addr = p1.v6_entry.addr; + + p1.apply_addrs().await?; + p2.apply_addrs() + .await + .expect_err("v6 addr already belongs to p1") + .expect_status(StatusCode::CONFLICT); + + Ok(()) +} + +/// Link state reports `ConfigErr` when an error is disrupting +/// link creation. +#[tokio::test] +async fn link_state_sanity_check_err() -> anyhow::Result<()> { + let fail_enable = AsicConfig { + radix: TESTING_RADIX, + port_enable_set: Chaos::new(1.0), + ..Default::default() + }; + let (_guard, client) = harness::init_harness(fname!(), &fail_enable); + + self::link_state_sanity_check(&client, |state| match state { + LinkState::ConfigError(_) => Ok(()), + LinkState::Unknown => Err(retry::ReturnCode::Retry( + "Link is in an Unknown state, watching for ConfigError".to_string(), + )), + other => Err(retry::ReturnCode::Fatal(format!( + "Link is in an unexpected state: {other:?}" + ))), + }) + .await +} + +/// Link reaches `LinkState::Up` under a good config with no failures. +#[tokio::test] +async fn link_state_sanity_check_ok() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + + self::link_state_sanity_check(&client, |state| match state { + LinkState::Up => Ok(()), + LinkState::Unknown => Err(retry::ReturnCode::Retry( + "Link is in an Unknown state, watching for Up".to_string(), + )), + other => Err(retry::ReturnCode::Fatal(format!( + "Link is in an unexpected state: {other:?}" + ))), + }) + .await +} + +/// Creates a link via `port_settings_apply`, enables it, and +/// calls `check_link` on the output of `link_get` until Ok or timeout. +async fn link_state_sanity_check( + client: &Client, + check_link: impl Fn(&LinkState) -> Result<(), retry::ReturnCode>, +) -> anyhow::Result<()> { + let mut rng = IpRng::new(fname_hash!()); + + let cxt = TestContext::new(client, QSFP0.clone(), LINK0); + let cmd = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + + cmd.apply_addrs().await?; + + retry::retry_op(RETRY_INTERVAL, RETRY_MAX, async || { + // Setting enabled pokes the reconciler and makes link + // state more interesting. + client + .link_enabled_set(&QSFP0, &LINK0, true) + .await + .map_err(|e| ReturnCode::Fatal(e.to_string()))?; + let link = client + .link_get(&QSFP0, &LINK0) + .await + .map_err(|e| ReturnCode::Fatal(e.to_string()))? + .into_inner(); + check_link(&link.link_state) + }) + .await?; + + Ok(()) +} + +#[tokio::test] +async fn link_local_crud() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + + let mut rng = IpRng::new(fname_hash!()); + + let link_id = client.link_create(&QSFP0, &TEST_LINK_CONF).await?; + let cxt = TestContext::new(&client, QSFP0.clone(), link_id.into_inner()); + let conf = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + + let ll = Ipv6Entry { addr: rng.link_local_ipv6(), tag: TAG1.to_string() }; + + assert_eq!(client.tfport_data().await?[0].link_local, None); + client + .link_ipv6_create(&cxt.port_id, &cxt.link_id, &ll) + .await + .expect("New link local creation should succeed"); + assert_eq!(client.tfport_data().await?[0].link_local, Some(ll.addr)); + + client + .link_ipv6_create( + &cxt.port_id, + &cxt.link_id, + &Ipv6Entry { addr: rng.link_local_ipv6(), tag: TAG1.to_string() }, + ) + .await + .expect_err("Should fail because link already has a link local") + .expect_status(StatusCode::CONFLICT); + + conf.apply_addrs().await?; + conf.verify_addrs_exist(Verify::NonExhaustive).await?; + + assert!( + cxt.fetch_all_addrs().await?.contains(&(TAG1.into(), ll.addr.into())) + ); + assert_eq!(client.tfport_data().await?[0].link_local, Some(ll.addr)); + + client.link_ipv6_delete(&cxt.port_id, &cxt.link_id, &ll.addr).await?; + assert_eq!(client.tfport_data().await?[0].link_local, None); + client + .link_ipv6_delete(&cxt.port_id, &cxt.link_id, &ll.addr) + .await + .expect_err("Addr shouldn't exist") + .expect_status(StatusCode::NOT_FOUND); + + Ok(()) +} + +/// The port settings API only accepts [`RouterIfaceIpAddr`] addresses. +/// Link locals are disallowed for two reasons: +/// +/// 1. An interface may have at most one link local address. If a +/// reconciler tries applying settings to a link that already has +/// its link local address, settings apply will always fail due +/// to the exclusivity constraint. This is a subtle footgun it's +/// best to avoid. +/// 2. Our architecture has no need for it. tfportd sets the link local +/// address and can use the individual `create` endpoint. Whereas +/// sled-agent and friends may actually prefer to ignore link local +/// addresses. +#[tokio::test] +async fn port_settings_disallows_link_local() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); + let (_guard, client) = harness::init_harness(fname!(), &no_failures); + + let mut rng = IpRng::new(fname_hash!()); + + let cxt = TestContext::new(&client, QSFP0.clone(), LINK0); + let mut conf = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + + conf.apply_addrs().await?; + + let mut ll = + Ipv6Entry { addr: rng.link_local_ipv6(), tag: TAG1.to_string() }; + std::mem::swap(&mut ll.addr, &mut conf.v6_entry.addr); + conf.apply_addrs().await.expect_err("Cannot apply a link local address"); + std::mem::swap(&mut ll.addr, &mut conf.v6_entry.addr); + + assert_eq!(client.tfport_data().await?[0].link_local, None); + + client + .link_ipv6_create(&cxt.port_id, &cxt.link_id, &ll) + .await + .expect("New link local creation should succeed"); + client + .link_ipv6_create(&cxt.port_id, &cxt.link_id, &ll) + .await + .expect_err("Address should already exist"); + + let mut link_addrs = client + .port_settings_get(&cxt.port_id, Some(TAG1)) + .await? + .into_inner() + .links + .into_iter() + .next() + .expect("link1 should exist") + .1 + .addrs; + let expect_addrs: &mut [IpAddr] = + &mut [conf.v4_entry.addr.into(), conf.v6_entry.addr.into()]; + link_addrs.sort(); + expect_addrs.sort(); + assert_eq!(&link_addrs, &expect_addrs); + + client.port_settings_clear(&cxt.port_id, None).await?; + assert!(client.tfport_data().await?.is_empty()); + + Ok(()) +} + +/// Contains info for test helpers. +struct TestContext<'a> { + client: &'a Client, + port_id: PortId, + link_id: LinkId, +} + +impl<'a> TestContext<'a> { + /// Constructs a new context instance. + fn new(client: &'a Client, port_id: PortId, link_id: LinkId) -> Self { + Self { client, port_id, link_id } + } + + /// Fetches all tagged addresses on this link and uses [`anyhow::ensure`] + /// to verify that they equal what is expected. + async fn ensure_addrs<'b>( + &self, + expected: impl IntoIterator, IpAddr)>, + ) -> anyhow::Result<()> { + let fetched = self.fetch_all_addrs().await?; + let expected = expected.into_iter().collect(); + + anyhow::ensure!( + fetched == expected, + "Link addresses do not match expectation: +fetched: {fetched:#?} +expected: {expected:#?}" + ); + + Ok(()) + } + + /// Returns all addresses belonging to this link regardless of tag. + async fn fetch_all_addrs( + &self, + ) -> anyhow::Result, IpAddr)>> { + let mut fetched: BTreeSet<_> = self + .client + .link_ipv4_list(&self.port_id, &self.link_id, None, None) + .await? + .into_inner() + .items + .into_iter() + .map(|entry| (entry.tag.into(), entry.addr.into())) + .collect(); + fetched.extend( + self.client + .link_ipv6_list(&self.port_id, &self.link_id, None, None) + .await? + .into_inner() + .items + .into_iter() + .map(|entry| (entry.tag.into(), entry.addr.into())), + ); + + Ok(fetched) + } +} + +/// This struct simplifies repetitive CRUD operations +/// on tagged links with random address registrations. +struct TestAddrs<'a> { + v4_entry: Ipv4Entry, + v6_entry: Ipv6Entry, + cxt: &'a TestContext<'a>, +} + +impl<'a> TestAddrs<'a> { + /// Creates a new instance with a random IPv4 and IPv6 address + /// for this port and link. + fn new(cxt: &'a TestContext<'a>, rng: &mut IpRng, tag: String) -> Self { + Self { + v4_entry: Ipv4Entry { addr: rng.routable_ipv4(), tag: tag.clone() }, + v6_entry: Ipv6Entry { addr: rng.routable_ipv6(), tag }, + cxt, + } + } + + /// Retrieves a cow of self's tag. + fn tag(&self) -> Cow<'_, str> { + Cow::Borrowed(&self.v4_entry.tag) + } + + /// Retrieves the IPv6 address of this instance as an [`IpAddr`]. + fn ipv6(&self) -> IpAddr { + self.v6_entry.addr.into() + } + + /// Adds both tagged addresses to this link using dpd's `link_*_create` endpoints. + async fn create_addrs( + &self, + ) -> Result<(), dpd_client::Error> { + self.cxt + .client + .link_ipv4_create( + &self.cxt.port_id, + &self.cxt.link_id, + &self.v4_entry, + ) + .await?; + self.cxt + .client + .link_ipv6_create( + &self.cxt.port_id, + &self.cxt.link_id, + &self.v6_entry, + ) + .await?; + Ok(()) + } + + /// Adds both tagged addresses to this link using dpd's `port_settings_apply` endpoint. + async fn apply_addrs( + &self, + ) -> Result<(), dpd_client::Error> { + self.cxt + .client + .port_settings_apply( + &self.cxt.port_id, + Some(&self.v4_entry.tag), + &self.settings(), + ) + .await?; + + Ok(()) + } + + /// Creates a [`PortSettings`] instance for these addresses. + fn settings(&self) -> PortSettings { + let mut conf = Self::empty_settings(self.cxt.link_id); + let link = conf + .links + .get_mut(&self.cxt.link_id.to_string()) + .expect("Settings should contain this link"); + + link.params.lane = Some(self.cxt.link_id); + link.addrs.extend_from_slice(&[ + self.v4_entry.addr.into(), + self.v6_entry.addr.into(), + ]); + + conf + } + + /// Creates a new [`PortSettings`] instance for this link + /// with default config and no addrs. + fn empty_settings(link_id: LinkId) -> PortSettings { + PortSettings { + links: HashMap::from([( + link_id.to_string(), + LinkSettings { params: TEST_LINK_CONF, addrs: Vec::new() }, + )]), + } + } + + /// Fetches this tag's addresses using `link_*_list` and `port_settings_get`. + /// Returns Err if both addresses are not found. + /// If `scope == Verify::Exhaustive`, returns Err if other addresses + /// are found on the link besides those in `self`. + async fn verify_addrs_exist(&self, scope: Verify) -> anyhow::Result<()> { + let my_entries = [ + (self.v4_entry.tag.as_str().into(), self.v4_entry.addr.into()), + (self.v6_entry.tag.as_str().into(), self.v6_entry.addr.into()), + ]; + let my_addrs: &mut [IpAddr] = + &mut [self.v4_entry.addr.into(), self.v6_entry.addr.into()]; + + let mut psget_addrs = self + .cxt + .client + .port_settings_get(&self.cxt.port_id, Some(&self.v4_entry.tag)) + .await? + .links + .remove(&self.cxt.link_id.to_string()) + .ok_or_else(|| { + anyhow::anyhow!( + "port {:?} is missing link {:?}", + self.cxt.port_id, + self.cxt.link_id + ) + })? + .addrs; + + if scope == Verify::Exhaustive { + self.cxt.ensure_addrs(my_entries).await?; + + my_addrs.sort(); + psget_addrs.sort(); + anyhow::ensure!( + psget_addrs == my_addrs, + "port_settings_get differs from link_get: +expected: {my_addrs:?} +found: {psget_addrs:?} +" + ); + + return Ok(()); + } + + let all_addrs = self.cxt.fetch_all_addrs().await?; + for pair in my_entries.iter() { + anyhow::ensure!( + all_addrs.contains(pair), + "Link is missing {pair:?}" + ); + } + + for addr in my_addrs { + anyhow::ensure!( + psget_addrs.contains(addr), + "port_settings_get is missing an expected address: {addr:?}" + ); + } + + Ok(()) + } +} + +/// Informs the behavior of address registration verification. +#[derive(Debug, PartialEq, Eq)] +enum Verify { + /// Expect that the target resources are the only of their + /// kind on this link regardless of tag. + Exhaustive, + + /// Expect that the target resources exist on the link, but + /// resources from other tags may also exist. + NonExhaustive, +} diff --git a/dpd-client/tests/chaos_tests/util.rs b/dpd-client/tests/chaos_tests/util.rs index 22bc5c91..55c5d2c5 100644 --- a/dpd-client/tests/chaos_tests/util.rs +++ b/dpd-client/tests/chaos_tests/util.rs @@ -4,10 +4,12 @@ // // Copyright 2025 Oxide Computer Company -use dpd_client::Client; -use dpd_client::types::{Ipv4Entry, Ipv6Entry}; use futures::TryStreamExt; +use dpd_client::Client; +use dpd_client::types::Ipv4Entry; +use dpd_client::types::Ipv6Entry; + pub(crate) async fn link_list_ipv4( client: &Client, port: &str, @@ -37,3 +39,17 @@ pub(crate) async fn link_list_ipv6( .try_collect::>() .await } + +/// Simplifies validating dropshot client output. +pub trait HttpResponseCheck: Sized { + /// Panics if the status code within `self` does not match + /// the given code. + fn expect_status(self, code: reqwest::StatusCode) -> Self; +} + +impl HttpResponseCheck for dpd_client::Error { + fn expect_status(self, code: reqwest::StatusCode) -> Self { + assert_eq!(self.status(), Some(code), "Unexpected HTTP status code"); + self + } +} diff --git a/dpd-client/tests/integration_tests/port_api.rs b/dpd-client/tests/integration_tests/port_api.rs index 838b3796..c580d6d9 100644 --- a/dpd-client/tests/integration_tests/port_api.rs +++ b/dpd-client/tests/integration_tests/port_api.rs @@ -12,8 +12,10 @@ use anyhow::anyhow; use futures::TryStreamExt; use reqwest::StatusCode; -use ::common::ports::Ipv4Entry; -use ::common::ports::Ipv6Entry; +use common::ports::Ipv4Entry; +use common::ports::Ipv6Entry; +use common::test_utils::IpRng; +use common::test_utils::fname_hash; use dpd_client::ClientInfo; use dpd_client::Error; use dpd_client::types; @@ -255,10 +257,11 @@ async fn test_ipv4_delete() -> TestResult { #[ignore] async fn test_ipv6_set() -> TestResult { let switch = &*get_switch().await; + let mut rng = IpRng::new(fname_hash!()); - let a: Ipv6Addr = "fc00:aabb:ccdd:18:8:20ff:fe1d:b677".parse().unwrap(); - let b: Ipv6Addr = "fe80::8:20ff:fe1d:b677".parse().unwrap(); - let c: Ipv6Addr = "fe80::240:54ff:fe08:808".parse().unwrap(); + let a = rng.host_underlay(); + let b = rng.host_underlay(); + let c = rng.link_local_ipv6(); let (port_id, link_id) = switch.link_id(PhysPort(11)).unwrap(); switch @@ -309,9 +312,11 @@ async fn test_ipv6_set() -> TestResult { #[ignore] async fn test_ipv6_clear() -> TestResult { let switch = &*get_switch().await; - let a: Ipv6Addr = "fc00:aabb:ccdd:18:8:20ff:fe1d:b677".parse().unwrap(); - let b: Ipv6Addr = "fe80::8:20ff:fe1d:b677".parse().unwrap(); - let c: Ipv6Addr = "fe80::240:54ff:fe08:808".parse().unwrap(); + let mut rng = IpRng::new(fname_hash!()); + + let a = rng.host_underlay(); + let b = rng.host_underlay(); + let c = rng.link_local_ipv6(); let (port_id, link_id) = switch.link_id(PhysPort(11)).unwrap(); switch @@ -349,10 +354,11 @@ async fn test_ipv6_clear() -> TestResult { #[ignore] async fn test_ipv6_delete() -> TestResult { let switch = &*get_switch().await; + let mut rng = IpRng::new(fname_hash!()); - let a: Ipv6Addr = "fc00:aabb:ccdd:18:8:20ff:fe1d:b677".parse().unwrap(); - let b: Ipv6Addr = "fe80::8:20ff:fe1d:b677".parse().unwrap(); - let c: Ipv6Addr = "fe80::240:54ff:fe08:808".parse().unwrap(); + let a = rng.host_underlay(); + let b = rng.host_underlay(); + let c = rng.link_local_ipv6(); let (port_id, link_id) = switch.link_id(PhysPort(11)).unwrap(); switch diff --git a/dpd/Cargo.toml b/dpd/Cargo.toml index 05726534..b807ab46 100644 --- a/dpd/Cargo.toml +++ b/dpd/Cargo.toml @@ -75,7 +75,9 @@ transceiver-controller = { workspace = true, features = [ "api-traits" ] } scuffle = { workspace = true, features = ["smf-by-instance"] } [dev-dependencies] +common = { workspace = true, features = [ "test-utils" ] } expectorate.workspace = true +rand.workspace = true regress.workspace = true mockall.workspace = true slog-term.workspace = true diff --git a/dpd/src/addr.rs b/dpd/src/addr.rs new file mode 100644 index 00000000..af027be6 --- /dev/null +++ b/dpd/src/addr.rs @@ -0,0 +1,719 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/ +// +// Copyright 2026 Oxide Computer Company + +//! This module defines the mapping by which DPD tracks +//! Address + Interface relations. + +use std::borrow::Cow; +use std::collections::BTreeSet; +use std::collections::HashMap; +use std::collections::hash_map; +use std::fmt::Display; +use std::net::IpAddr; +use std::net::Ipv6Addr; +use std::ops::Bound; +use std::ops::RangeBounds; +use std::sync::RwLock; + +use slog::debug; + +use aal::AsicId; +use common::addr::IpAddrLike; +use common::addr::RouterIfaceIpAddr; +use common::ports::PortId; +use dpd_types::link::LinkId; +use oxnet::UnicastLinkLocalIpv6Addr; + +use crate::DpdError; +use crate::DpdResult; +use crate::Switch; +use crate::table::port_ip; + +/// A mutex-wrapped [`AddrMap`]. +/// +/// The fns on this outer struct are convenience wrappers for +/// operations on the inner [`AddrMap`]. +/// +/// Assume every method other than `get` acquires a lock. +#[derive(Debug, Default)] +pub struct SyncAddrMap(RwLock); + +impl SyncAddrMap { + /// Returns the inner (lockable) map. + pub fn get(&self) -> &RwLock { + &self.0 + } + + /// Removes all addresses of the declared type belonging to + /// this interface. + /// + /// Only addresses belonging to the named tag are deleted. + /// + /// Returns [`DpdError::AddrClear`] if any addresses could not + /// be deleted. + pub fn clear_addrs( + &self, + switch: &Switch, + iface: AsicAddrIface, + tag: &OptTag, + ) -> DpdResult<()> { + self.0.write().unwrap().try_retain_by_iface::( + switch, + iface, + |_addr, entry_tag| !tag.matches(entry_tag), + ) + } + + /// Removes the address from switch tables if it exists. + /// + /// On success, returns whether anything was actually deleted. + pub fn clear_addr( + &self, + switch: &Switch, + iface: AsicAddrIface, + addr: TableIp, + tag: &OptTag, + ) -> DpdResult { + self.0.write().unwrap().try_clear(switch, addr, iface, tag) + } +} + +/// A switch may have many interfaces and many addresses. An +/// address may belong to exactly one interface, and an interface +/// may have many addresses. This struct abstracts CRUD operations +/// on these addresses while keeping switch tables consistent. +/// +/// Link addresses are scoped to a link lifetime, and loopback +/// addresses belong to the switch itself. This is tracked by the +/// [`AsicAddrIface`] enum. A namespace "tag" is also exposed to +/// help mutually unaware controllers cooperate when reconciling +/// addresses. Tags act as a reference count, and entries are only +/// cleared from the asic when the last tag releases an address. +/// +/// This upholds the following DPD address invariants: +/// - Unicast addresses belong to exactly one interface. +/// - Such an address is in this soft state IFF it's in an asic table. +/// - A link may have at most one IPv6 link local address. +/// - Otherwise, only [`RouterIfaceIpAddr`] addresses may be registered +/// on an interface. +/// +/// # Synchronization +/// +/// Any method with a `switch` parameter will lock asic tables. +/// Never hold an asic table lock while calling such a method. +// +// ### Design +// +// - Using only one of the maps in this struct is simpler and sufficient +// for correctness. This type design was chosen to avoid full-table scans +// on common operations and to obviously enforce uniqueness constraints. +// - IPv4 and IPv6 addresses have different tables in the switch, so +// we could parallelize better by sharding IPv4 and IPv6 soft state +// behind separate locks. I skipped this to avoid additional +// complexity, but it's certainly possible if needed. +// +// ### Invariant +// +// These two tables should uphold the following: +// - An address is in one IFF it's in the other. +// - An address' interface in one is equal to its +// interface in the other. +#[derive(Default, Debug)] +pub struct AddrMap { + addr_to_iface: HashMap, + + // Invariant: If an interface has *a* link local address, it's the + // first address in this BTreeSet. + iface_to_addr: HashMap>, +} + +/// An IP address that may be registered in a unicast address table +/// as belonging to an interface. +// +// Don't re-order these variants. The link local invariant depends on +// link local < routable, which is also tested. +#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord)] +pub enum TableIp { + /// The link local IPv6 address of an interface. Any interface + /// may have at most one of these. + LinkLocal(UnicastLinkLocalIpv6Addr), + + /// A valid non-link-local address that may be registered on + /// an interface. Many of these may exist per interface. + Router(RouterIfaceIpAddr), +} + +impl TableIp { + /// Attempts to construct a [`TableIp`] or returns `None` if + /// the address is not supported. + fn try_new(addr: IpAddr) -> Option { + if let Some(link_local) = Ipv6Addr::from_ip(addr) + .and_then(|v6| UnicastLinkLocalIpv6Addr::new(v6).ok()) + { + return Some(Self::LinkLocal(link_local)); + } + RouterIfaceIpAddr::try_new(addr).map(TableIp::Router).ok() + } +} + +impl From for IpAddr { + fn from(value: TableIp) -> Self { + match value { + TableIp::LinkLocal(addr) => IpAddr::from(addr), + TableIp::Router(addr) => *addr.get(), + } + } +} + +impl From for TableIp { + fn from(value: RouterIfaceIpAddr) -> Self { + Self::Router(value) + } +} + +impl From for TableIp { + fn from(value: UnicastLinkLocalIpv6Addr) -> Self { + Self::LinkLocal(value) + } +} + +impl Display for TableIp { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::LinkLocal(a) => a.fmt(f), + Self::Router(a) => a.fmt(f), + } + } +} + +impl TryFrom<&IpAddr> for TableIp { + type Error = DpdError; + + fn try_from(value: &IpAddr) -> Result { + Self::try_new(*value).ok_or_else(|| { + DpdError::Invalid(format!( + "Entry {value:?} not permitted in address table" + )) + }) + } +} + +impl TryFrom for TableIp { + type Error = DpdError; + + fn try_from(value: IpAddr) -> Result { + (&value).try_into() + } +} + +impl TryFrom<&TableIp> for RouterIfaceIpAddr { + type Error = DpdError; + + fn try_from(value: &TableIp) -> Result { + if let TableIp::Router(addr) = value { + return Ok(*addr); + } + Err(DpdError::Invalid(format!("Not a valid router IP addr: {value:?}"))) + } +} + +impl AddrMap { + /// Writes this address to switch tables. + /// + /// Errs without mutation if this address already + /// belongs to another interface or if an asic + /// operation fails. + /// + /// Return Ok(true) if this registration updated any config + /// within the switch. + /// Returns Ok(false) if the interface was already aware of this + /// registration. + pub fn try_set( + &mut self, + switch: &Switch, + addr: TableIp, + iface: AsicAddrIface, + tag: String, + ) -> DpdResult { + let iface_id = iface.into(); + let addr_entry = match self.addr_to_iface.entry(addr) { + hash_map::Entry::Vacant(empty) => empty, + hash_map::Entry::Occupied(mut addr_entry) => { + Self::cmp_iface(addr, &iface_id, addr_entry.get())?; + debug!( + switch.log, + "Table set: address {addr:?} already exists in soft state with config {:?}", + addr_entry.get() + ); + let new_tag = addr_entry.get_mut().tags.insert(tag); + assert!( + self.iface_to_addr + .get(&iface_id) + .is_some_and(|entry| entry.contains(&addr)), + "If an address exists, then the interface has a copy of that address" + ); + return Ok(new_tag); + } + }; + + let iface_entry = self.iface_to_addr.entry(iface_id); + if let TableIp::LinkLocal(new_ll) = addr + && let hash_map::Entry::Occupied(curr) = &iface_entry + && let Some(old_ll) = Self::link_local(curr.get()) + { + assert_ne!( + &new_ll, old_ll, + "Invariant: Because addr is in iface_to_addr, it should be in addr_to_iface" + ); + return Err(DpdError::Exists(format!( + "Cannot set link local address {new_ll:?} to {iface_id:?}. It already has a link local address: {old_ll:?}" + ))); + } + + Self::set_asic(switch, addr, &iface)?; + addr_entry.insert(AddrSpec { + interface: iface_id, + tags: std::iter::once(tag).collect(), + }); + let new_addr = iface_entry.or_default().insert(addr); + assert!( + new_addr, + "If an entry is new to the address table, it's new to the interface" + ); + Ok(true) + } + + /// Removes this tag's address claim from the interface. + /// Removes the address if this was the only tag. + /// + /// Errs without mutation if this address belongs to + /// another interface or if a switch operation fails. + /// + /// Return Ok(true) if this cleared any config from the switch. + /// Returns Ok(false) if this entry didn't exist. + pub fn try_clear( + &mut self, + switch: &Switch, + addr: TableIp, + iface: AsicAddrIface, + compare_tag: &OptTag, + ) -> DpdResult { + let hash_map::Entry::Occupied(mut addr_entry) = + self.addr_to_iface.entry(addr) + else { + assert!( + self.iface_to_addr + .get(&iface.into()) + .is_none_or(|entry| !entry.contains(&addr)), + "If an address is not in addr_to_iface, then it must not be in iface_to_addr." + ); + debug!( + switch.log, + "Table clear: address {addr:?} does not exist in soft state" + ); + return Ok(false); + }; + + Self::cmp_iface(addr, &iface.into(), addr_entry.get())?; + match compare_tag { + OptTag::Any => { + Self::clear_asic(switch, addr, &iface)?; + } + OptTag::Tag(t) => { + let Some(tag) = addr_entry.get_mut().tags.take(&**t) else { + return Ok(false); + }; + if !addr_entry.get().tags.is_empty() { + return Ok(true); + } + if let Err(e) = Self::clear_asic(switch, addr, &iface) { + addr_entry.get_mut().tags.insert(tag); + return Err(e); + } + } + } + + let removed = addr_entry.remove_entry(); + let hash_map::Entry::Occupied(mut iface_entry) = + self.iface_to_addr.entry(removed.1.interface) + else { + unreachable!("If an addr exists, its interface must exist"); + }; + + let removed = iface_entry.get_mut().remove(&addr); + assert!( + removed, + "Addr was in addr_to_iface, but the reverse map didn't exist" + ); + + if iface_entry.get().is_empty() { + iface_entry.remove_entry(); + } + + Ok(true) + } + + /// Iterates the tagged addresses belonging to this interface + /// for which `A::from_ip` returns `Some`. + pub fn iter_by_iface<'a, A: IpAddrLike + 'a>( + &'a self, + iface: AsicAddrIface, + ) -> impl Iterator)> + 'a { + let iface_id = iface.into(); + self.iface_to_addr + .get(&iface_id) + .into_iter() + .flat_map(|addrs| addrs.iter()) + .filter_map(move |addr| { + let spec = self + .addr_to_iface + .get(addr) + .expect("If an address is in iface_to_addr, it must be in addr_to_iface"); + assert_eq!(spec.interface, iface_id, "Interfaces must concur"); + Some((IpAddrLike::from_ip(IpAddr::from(*addr))?, &spec.tags)) + }) + } + + /// Iterates the addresses belonging to this interface for + /// which `A::from_ip` returns `Some`. + /// + /// Clears all addresses for which `keep(A, tag)` returns false. + /// There may be multiple `(A, tag)` tuples if an address has more + /// than one tag. + /// + /// Attempts to clear all matching addresses. On failure, returns + /// [`DpdError::AddrClear`] with the addresses that could not be deleted. + pub fn try_retain_by_iface( + &mut self, + switch: &Switch, + iface: AsicAddrIface, + keep: impl Fn(&A, &str) -> bool, + ) -> DpdResult<()> { + let hash_map::Entry::Occupied(mut iface_entry) = + self.iface_to_addr.entry(iface.into()) + else { + return Ok(()); + }; + + let mut errors = Vec::new(); + iface_entry.get_mut().retain(|addr| { + let ip = IpAddr::from(*addr); + let Some(user_addr) = A::from_ip(ip) else { + return true; + }; + + let hash_map::Entry::Occupied(mut addr_entry) = + self.addr_to_iface.entry(*addr) + else { + panic!("Addr/iface must be registered bidirectionally"); + }; + assert_eq!(addr_entry.get().interface, iface.into()); + + let backup = addr_entry.get().tags.clone(); + addr_entry.get_mut().tags.retain(|tag| keep(&user_addr, tag)); + + if !addr_entry.get().tags.is_empty() { + return true; + } + + if let Err(e) = Self::clear_asic(switch, *addr, &iface) { + addr_entry.get_mut().tags = backup; + errors.push((iface, IpAddr::from(*addr), e)); + return true; + } + + addr_entry.remove(); + false + }); + + if iface_entry.get().is_empty() { + iface_entry.remove(); + } + + if !errors.is_empty() { + return Err(DpdError::AddrClear(errors)); + } + + Ok(()) + } + + /// Iterates through the tagged addresses within the + /// given range belonging to this interface. + pub fn iface_addr_range<'a>( + &'a self, + iface: AsicAddrIface, + r: impl RangeBounds + 'a, + ) -> impl Iterator)> + 'a { + self::map_bound::<_, TableIp>(r.start_bound()) + .and_then(|start| Some((start, self::map_bound(r.end_bound())?))) + .into_iter() + .flat_map(move |bounds| { + self.iface_to_addr + .get(&iface.into()) + .map(|set| set.range(bounds)) + .into_iter() + .flatten() + .map(|addr| { + let tags = &self + .addr_to_iface + .get(addr) + .expect("If an address is in iface_to_addr, it must be in addr_to_iface") + .tags; + (IpAddr::from(*addr), tags) + }) + }) + } + + /// Retrieves the link local address assigned to this interface + /// or `None` if no link local has been set. + pub fn get_link_local( + &self, + iface: AsicAddrIface, + ) -> Option { + self.iface_to_addr + .get(&iface.into()) + .and_then(Self::link_local) + .copied() + } + + /// Returns err if the requested and current interface + /// entries do not match. + /// + /// Do not mutate an entry if this fails. + fn cmp_iface( + addr: TableIp, + request: &AddrIface, + current: &AddrSpec, + ) -> DpdResult<()> { + if request == ¤t.interface { + return Ok(()); + } + + let interface = match current.interface { + AddrIface::Loopback => "loopback".into(), + AddrIface::Link { port_id, link_id } => { + format!("{port_id}/{link_id}").into() + } + }; + Err(DpdError::AddrIfaceConflict { addr: addr.into(), interface }) + } + + /// Declares this entry in the corresponding asic table. + /// No error if it already existed. + fn set_asic( + switch: &Switch, + addr: TableIp, + id: &AsicAddrIface, + ) -> DpdResult<()> { + let addr = IpAddr::from(addr); + match (addr, id) { + (IpAddr::V4(v4), AsicAddrIface::Link { asic_id, .. }) => { + port_ip::ipv4_set(switch, *asic_id, v4) + } + (IpAddr::V4(v4), AsicAddrIface::Loopback) => { + port_ip::loopback_ipv4_set(switch, v4) + } + (IpAddr::V6(v6), AsicAddrIface::Link { asic_id, .. }) => { + port_ip::ipv6_set(switch, *asic_id, v6) + } + (IpAddr::V6(v6), AsicAddrIface::Loopback) => { + port_ip::loopback_ipv6_set(switch, v6) + } + } + } + + /// Removes this entry from the corresponding asic table. + /// No error if it was not previously present. + fn clear_asic( + switch: &Switch, + addr: TableIp, + id: &AsicAddrIface, + ) -> DpdResult<()> { + let addr = IpAddr::from(addr); + match (addr, id) { + (IpAddr::V4(v4), AsicAddrIface::Link { asic_id, .. }) => { + port_ip::ipv4_clear(switch, *asic_id, v4) + } + (IpAddr::V4(v4), AsicAddrIface::Loopback) => { + port_ip::loopback_ipv4_clear(switch, v4) + } + (IpAddr::V6(v6), AsicAddrIface::Link { asic_id, .. }) => { + port_ip::ipv6_clear(switch, *asic_id, v6) + } + (IpAddr::V6(v6), AsicAddrIface::Loopback) => { + port_ip::loopback_ipv6_clear(switch, v6) + } + } + } + + /// Pulls the link local address out of this set or returns None + /// if there is no link local. + fn link_local( + addrs: &BTreeSet, + ) -> Option<&UnicastLinkLocalIpv6Addr> { + debug_assert!( + addrs.iter().skip(1).all(|addr| matches!(addr, TableIp::Router(_))), + "Invariant: If an interface has a link local, it must be the first in the set" + ); + if let Some(TableIp::LinkLocal(ll)) = addrs.first() { + return Some(ll); + } + None + } +} + +/// Metadata attached to a registered address. +#[derive(PartialEq, Eq, Debug)] +struct AddrSpec { + interface: AddrIface, + tags: BTreeSet, +} + +/// The switch entity to which an IP address uniquely belongs. +/// +/// Use [`crate::link::Link::asic_addr_iface`] to construct a link ID. +/// Loopback has no asic_id. +/// +/// # Correctness +/// +/// The asic_id of a link must be stable across the lifespan of +/// the link. Changing it will yield the switch table equivalent of +/// internally mutating a [`HashMap`] key. +#[derive(Debug, Clone, Copy)] +pub enum AsicAddrIface { + Loopback, + Link { port_id: PortId, link_id: LinkId, asic_id: AsicId }, +} + +/// The switch entity to which an IP address uniquely belongs. +/// +/// # Design +/// +/// Setting and clearing addresses requires an asic id. +/// We already require [`PortId`] and [`LinkId`], so we +/// could easily derive this from the switch using +/// [`Switch::link_asic_port_id`]. +/// +/// However, every operation on the switch adds indirection +/// and locking risk. And the caller always knows its asic_id, +/// so this is redundant work. +/// +/// Since asic ID is a black box to DPD, I'd rather not make +/// soft state depend on it. So we use [`AsicAddrIface`] in +/// the API but only store [`AddrIface`] internally. +#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] +enum AddrIface { + Loopback, + Link { port_id: PortId, link_id: LinkId }, +} + +/// Clients may use DPD tags to cooperatively namespace CRUD +/// operations and avoid disrupting eachother. +/// +/// This type may be used in APIs where tag usage is optional. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum OptTag<'a> { + /// The action or result applies only to resources with this tag. + Tag(Cow<'a, str>), + + /// The action or result applies to this resource regardless of tag(s). + Any, +} + +impl<'a> OptTag<'a> { + /// Constructs a new [`OptTag`] selector matching only + /// the given tag string. + pub const fn exact(tag: &'a str) -> Self { + Self::Tag(Cow::Borrowed(tag)) + } + + /// Returns true if this tag applies to the given tag str. + pub fn matches(&self, other: &str) -> bool { + match self { + Self::Any => true, + Self::Tag(tag) => *tag == other, + } + } +} + +/// Maps a [`Bound`] of type `In` to one of type `Out`. +/// Returns `None` if `TryFrom` fails. +fn map_bound(bound: Bound) -> Option> +where + Out: TryFrom, +{ + match bound { + Bound::Unbounded => Some(Bound::Unbounded), + Bound::Included(input) => { + Some(Bound::Included(Out::try_from(input).ok()?)) + } + Bound::Excluded(input) => { + Some(Bound::Excluded(Out::try_from(input).ok()?)) + } + } +} + +impl crate::link::Link { + /// Constructs an address ownership ID for this link. + pub fn asic_addr_iface(&self) -> AsicAddrIface { + AsicAddrIface::Link { + port_id: self.port_id, + link_id: self.link_id, + asic_id: self.asic_port_id, + } + } +} + +impl From for AddrIface { + fn from(value: AsicAddrIface) -> Self { + match value { + AsicAddrIface::Loopback => Self::Loopback, + AsicAddrIface::Link { port_id, link_id, .. } => { + Self::Link { port_id, link_id } + } + } + } +} + +#[cfg(test)] +mod addr_tests { + use std::collections::BTreeSet; + + use common::addr::RouterIfaceIpAddr; + use common::test_utils::IpRng; + use common::test_utils::fname_hash; + use oxnet::UnicastLinkLocalIpv6Addr; + + use crate::addr::TableIp; + + /// The link local invariant only works if it's ordered ahead + /// of router addrs. That's guaranteed by rustc, but this verifies + /// nothing shifts sneakily. + /// + #[test] + fn link_local_sanity_check() -> anyhow::Result<()> { + let mut addrs = BTreeSet::new(); + let mut rng = IpRng::new(fname_hash!()); + + addrs.insert(TableIp::Router(RouterIfaceIpAddr::try_new( + rng.host_underlay().into(), + )?)); + addrs.insert(TableIp::LinkLocal(UnicastLinkLocalIpv6Addr::new( + rng.link_local_ipv6(), + )?)); + addrs.insert(TableIp::Router(RouterIfaceIpAddr::try_new( + rng.host_underlay().into(), + )?)); + + assert!(matches!(addrs.first(), Some(TableIp::LinkLocal(_)))); + addrs.pop_last(); + assert!(matches!(addrs.first(), Some(TableIp::LinkLocal(_)))); + addrs.pop_first(); + assert!(matches!(addrs.first(), Some(TableIp::Router(_)))); + + Ok(()) + } +} diff --git a/dpd/src/api_server.rs b/dpd/src/api_server.rs index 5f595e6c..dc71771b 100644 --- a/dpd/src/api_server.rs +++ b/dpd/src/api_server.rs @@ -17,6 +17,8 @@ use std::sync::Arc; use asic::tofino_asic::serdes; #[cfg(feature = "tofino_asic")] use asic::tofino_asic::stats; +use common::addr::IpAddrLike; +use common::addr::RouterIfaceIpAddr; use dpd_types::arp::{ ArpEntry, ArpToken, Ipv4ArpParam, Ipv4Token, Ipv6ArpParam, Ipv6Token, }; @@ -98,6 +100,9 @@ use common::ports::TxEq; #[cfg(any(feature = "softnpu", feature = "tofino_asic"))] use common::ports::TxEqSwHw; +use crate::addr::AsicAddrIface; +use crate::addr::OptTag; +use crate::addr::SyncAddrMap; use crate::attached_subnet; use crate::counters; #[cfg(feature = "multicast")] @@ -1124,12 +1129,19 @@ impl DpdApi for DpdApiImpl { DpdError::Invalid("Invalid page limit".to_string()).into() ); }; + + // TODO::cory: include tag in pagination token. let addr = match &pagination.page { WhichPage::First(..) => None, - WhichPage::Next(Ipv4Token { ip }) => Some(*ip), + WhichPage::Next(Ipv4Token { ip }) => Some((IpAddr::V4(*ip), "")), }; - let entries = - switch.list_ipv4_addresses(port_id, link_id, addr, limit)?; + let entries = switch.list_ip_addresses( + port_id, + link_id, + addr, + limit, + |addr, tag| Ipv4Entry { addr: *addr, tag: tag.to_string() }, + )?; ResultsPage::new( entries, &EmptyScanParams {}, @@ -1145,11 +1157,10 @@ impl DpdApi for DpdApiImpl { ) -> Result { let switch: &Switch = rqctx.context(); let path = path.into_inner(); - let port_id = path.port_id; - let link_id = path.link_id; let entry = entry.into_inner(); + let addr = entry.addr.to_ip().try_into()?; switch - .create_ipv4_address(port_id, link_id, entry) + .create_ip_address(path.port_id, path.link_id, addr, entry.tag) .map(|_| HttpResponseUpdatedNoContent()) .map_err(|e| e.into()) } @@ -1163,7 +1174,7 @@ impl DpdApi for DpdApiImpl { let port_id = path.port_id; let link_id = path.link_id; switch - .reset_ipv4_addresses(port_id, link_id) + .reset_addresses::(port_id, link_id) .map(|_| HttpResponseUpdatedNoContent()) .map_err(|e| e.into()) } @@ -1176,9 +1187,9 @@ impl DpdApi for DpdApiImpl { let path = path.into_inner(); let port_id = path.port_id; let link_id = path.link_id; - let address = path.address; + let address = path.address.to_ip().try_into()?; switch - .delete_ipv4_address(port_id, link_id, address) + .delete_ip_address(port_id, link_id, address, &OptTag::Any) .map(|_| HttpResponseDeleted()) .map_err(|e| e.into()) } @@ -1199,12 +1210,19 @@ impl DpdApi for DpdApiImpl { DpdError::Invalid("Invalid page limit".to_string()).into() ); }; + // TODO::cory: include tag in pagination token. let addr = match &pagination.page { WhichPage::First(..) => None, - WhichPage::Next(Ipv6Token { ip }) => Some(*ip), + WhichPage::Next(Ipv6Token { ip }) => Some((IpAddr::V6(*ip), "")), }; - let entries = - switch.list_ipv6_addresses(port_id, link_id, addr, limit)?; + let entries = switch.list_ip_addresses( + port_id, + link_id, + addr, + limit, + |a: &Ipv6Addr, tag| Ipv6Entry { addr: *a, tag: tag.to_string() }, + )?; + ResultsPage::new( entries, &EmptyScanParams {}, @@ -1223,8 +1241,9 @@ impl DpdApi for DpdApiImpl { let port_id = path.port_id; let link_id = path.link_id; let entry = entry.into_inner(); + let addr = entry.addr.to_ip().try_into()?; switch - .create_ipv6_address(port_id, link_id, entry) + .create_ip_address(port_id, link_id, addr, entry.tag) .map(|_| HttpResponseUpdatedNoContent()) .map_err(|e| e.into()) } @@ -1238,7 +1257,7 @@ impl DpdApi for DpdApiImpl { let port_id = path.port_id; let link_id = path.link_id; switch - .reset_ipv6_addresses(port_id, link_id) + .reset_addresses::(port_id, link_id) .map(|_| HttpResponseUpdatedNoContent()) .map_err(|e| e.into()) } @@ -1251,9 +1270,9 @@ impl DpdApi for DpdApiImpl { let path = path.into_inner(); let port_id = path.port_id; let link_id = path.link_id; - let address = path.address; + let address = path.address.to_ip().try_into()?; switch - .delete_ipv6_address(port_id, link_id, address) + .delete_ip_address(port_id, link_id, address, &OptTag::Any) .map(|_| HttpResponseDeleted()) .map_err(|e| e.into()) } @@ -1336,13 +1355,19 @@ impl DpdApi for DpdApiImpl { async fn loopback_ipv4_list( rqctx: RequestContext>, ) -> Result>, HttpError> { - let switch: &Switch = rqctx.context(); - let addrs = match switch.loopback.lock() { - Ok(loopback_data) => { - loopback_data.v4_addrs.iter().cloned().collect() - } - Err(e) => return Err(HttpError::for_internal_error(e.to_string())), - }; + let addrs = rqctx + .context() + .addrs + .get() + .read() + .unwrap() + .iter_by_iface(AsicAddrIface::Loopback) + .flat_map(|(addr, tags)| { + tags.iter() + .map(move |tag| Ipv4Entry { addr, tag: tag.to_string() }) + }) + .collect(); + Ok(HttpResponseOk(addrs)) } @@ -1351,9 +1376,10 @@ impl DpdApi for DpdApiImpl { val: TypedBody, ) -> Result { let switch: &Switch = rqctx.context(); - let addr = val.into_inner(); + let entry = val.into_inner(); + let addr = IpAddr::V4(entry.addr).try_into().map_err(DpdError::from)?; - loopback::add_loopback_ipv4(switch, &addr)?; + loopback::set_loopback(switch, addr, entry.tag)?; Ok(HttpResponseUpdatedNoContent {}) } @@ -1363,8 +1389,11 @@ impl DpdApi for DpdApiImpl { path: Path, ) -> Result { let switch: &Switch = rqctx.context(); - let addr = path.into_inner(); - loopback::delete_loopback_ipv4(switch, &addr.ipv4) + let addr = IpAddr::V4(path.into_inner().ipv4) + .try_into() + .map_err(DpdError::from)?; + + loopback::clear_loopback(switch, addr, &OptTag::Any) .map(|_| HttpResponseDeleted()) .map_err(HttpError::from) } @@ -1372,13 +1401,19 @@ impl DpdApi for DpdApiImpl { async fn loopback_ipv6_list( rqctx: RequestContext>, ) -> Result>, HttpError> { - let switch: &Switch = rqctx.context(); - let addrs = match switch.loopback.lock() { - Ok(loopback_data) => { - loopback_data.v6_addrs.iter().cloned().collect() - } - Err(e) => return Err(HttpError::for_internal_error(e.to_string())), - }; + let addrs = rqctx + .context() + .addrs + .get() + .read() + .unwrap() + .iter_by_iface(AsicAddrIface::Loopback) + .flat_map(|(addr, tags)| { + tags.iter() + .map(move |tag| Ipv6Entry { addr, tag: tag.to_string() }) + }) + .collect(); + Ok(HttpResponseOk(addrs)) } @@ -1387,9 +1422,10 @@ impl DpdApi for DpdApiImpl { val: TypedBody, ) -> Result { let switch: &Switch = rqctx.context(); - let addr = val.into_inner(); + let entry = val.into_inner(); + let addr = IpAddr::V6(entry.addr).try_into().map_err(DpdError::from)?; - loopback::add_loopback_ipv6(switch, &addr)?; + loopback::set_loopback(switch, addr, entry.tag)?; Ok(HttpResponseUpdatedNoContent {}) } @@ -1399,8 +1435,11 @@ impl DpdApi for DpdApiImpl { path: Path, ) -> Result { let switch: &Switch = rqctx.context(); - let addr = path.into_inner(); - loopback::delete_loopback_ipv6(switch, &addr.ipv6) + let addr = IpAddr::V6(path.into_inner().ipv6) + .try_into() + .map_err(DpdError::from)?; + + loopback::clear_loopback(switch, addr, &OptTag::Any) .map(|_| HttpResponseDeleted()) .map_err(HttpError::from) } @@ -1710,7 +1749,7 @@ impl DpdApi for DpdApiImpl { route::reset_ipv4_tag(switch, &tag).await; route::reset_ipv6_tag(switch, &tag).await; switch - .clear_link_addresses(Some(&tag)) + .clear_link_addresses(&OptTag::exact(&tag)) .map(|_| HttpResponseUpdatedNoContent()) .map_err(|e| e.into()) } @@ -1734,7 +1773,7 @@ impl DpdApi for DpdApiImpl { error!(switch.log, "failed to reset route data: {:?}", e); err = Some(e); } - if let Err(e) = switch.clear_link_state() { + if let Err(e) = switch.clear_link_addresses(&OptTag::Any) { error!(switch.log, "failed to clear all link state: {:?}", e); err = Some(e); } @@ -1842,9 +1881,10 @@ impl DpdApi for DpdApiImpl { let query = query.into_inner(); let port_id = path.port_id; let settings = body.into_inner(); + let tag = query.tag.as_deref().unwrap_or(""); switch - .apply_port_settings(port_id, settings, query.tag) + .apply_port_settings(port_id, settings, tag) .await .map(HttpResponseOk) .map_err(HttpError::from) @@ -1859,9 +1899,10 @@ impl DpdApi for DpdApiImpl { let path = path.into_inner(); let query = query.into_inner(); let port_id = path.port_id; + let tag = query.tag.as_deref().unwrap_or(""); switch - .clear_port_settings(port_id, query.tag) + .clear_port_settings(port_id, tag) .await .map(HttpResponseOk) .map_err(HttpError::from) @@ -1876,9 +1917,10 @@ impl DpdApi for DpdApiImpl { let path = path.into_inner(); let query = query.into_inner(); let port_id = path.port_id; + let tag = query.tag.as_deref().unwrap_or(""); switch - .get_port_settings(port_id, query.tag) + .get_port_settings(port_id, tag) .await .map(HttpResponseOk) .map_err(HttpError::from) @@ -2946,24 +2988,31 @@ pub(crate) fn build_info() -> BuildInfo { } } -impl From<&crate::link::Link> for LinkSettings { - fn from(l: &crate::link::Link) -> Self { - let mut addrs: HashSet = HashSet::new(); - for a in &l.ipv4 { - addrs.insert(a.addr.into()); - } - for a in &l.ipv6 { - addrs.insert(a.addr.into()); - } +impl crate::link::Link { + /// Creates a serializable [`LinkSettings`] representation with the + /// current link state and this tag's resources. + /// + /// These serialized link settings never include a link local + /// address. That's managed separately. + pub fn settings(&self, amap: &SyncAddrMap, tag: &str) -> LinkSettings { + let addrs: HashSet = amap + .get() + .read() + .unwrap() + .iter_by_iface::(self.asic_addr_iface()) + .filter(|(_, tags)| tags.contains(tag)) + .map(|(addr, _)| addr.to_ip()) + .collect(); + LinkSettings { params: LinkCreate { - lane: Some(l.link_id), - speed: l.config.speed, - fec: l.config.fec, - autoneg: l.config.autoneg, - kr: l.config.kr, - tx_eq: l.tx_eq, - allow_ddm_traffic: l.config.allow_ddm_traffic, + lane: Some(self.link_id), + speed: self.config.speed, + fec: self.config.fec, + autoneg: self.config.autoneg, + kr: self.config.kr, + tx_eq: self.tx_eq, + allow_ddm_traffic: self.config.allow_ddm_traffic, }, addrs, } diff --git a/dpd/src/link.rs b/dpd/src/link.rs index a833a231..2fb836b5 100644 --- a/dpd/src/link.rs +++ b/dpd/src/link.rs @@ -8,6 +8,9 @@ use crate::MacAddr; use crate::Switch; +use crate::addr::OptTag; +use crate::addr::SyncAddrMap; +use crate::addr::TableIp; use crate::fault::AutonegTracker; use crate::fault::Faultable; use crate::fault::LinkUpTracker; @@ -16,18 +19,17 @@ use crate::ports::Event; use crate::table::mac; #[cfg(feature = "multicast")] use crate::table::mcast; -use crate::table::port_ip; use crate::table::uplink; use crate::transceivers::qsfp_xcvr_mpn; use crate::types::DpdError; use crate::types::DpdResult; +use aal::AsicError; use aal::AsicId; use aal::AsicOps; use aal::AsicResult; use aal::PortHdl; use aal::PortUpdate; -use common::ports::Ipv4Entry; -use common::ports::Ipv6Entry; +use common::addr::IpAddrLike; use common::ports::PortFec; use common::ports::PortId; use common::ports::PortMedia; @@ -50,9 +52,11 @@ use slog::o; use slog_error_chain::InlineErrorChain; use std::collections::BTreeMap; use std::collections::BTreeSet; +use std::collections::btree_map; use std::collections::btree_map::Entry; -use std::net::Ipv4Addr; +use std::net::IpAddr; use std::net::Ipv6Addr; +use std::ops::Bound; use std::sync::Arc; use std::sync::Mutex; use std::time::Duration; @@ -95,15 +99,30 @@ impl LinkMap { } /// Delete a specific link from the map. + /// + /// Fails if the link doesn't exist. + /// Fails if link resources couldn't be fully deleted. pub fn delete_link( &mut self, - port_id: PortId, - link_id: LinkId, + switch: &Switch, + link: &Link, ) -> DpdResult<()> { - match self.0.remove(&(port_id, link_id)) { - Some(_) => Ok(()), - None => Err(DpdError::NoSuchLink { port_id, link_id }), - } + let btree_map::Entry::Occupied(slot) = + self.0.entry((link.port_id, link.link_id)) + else { + return Err(DpdError::NoSuchLink { + port_id: link.port_id, + link_id: link.link_id, + }); + }; + switch.addrs.clear_addrs::( + switch, + link.asic_addr_iface(), + &OptTag::Any, + )?; + slot.remove(); + + Ok(()) } /// Insert a new link into the map. If the map already contains a link @@ -206,9 +225,32 @@ impl Switch { /// A `Link` is a configured Ethernet link. /// -/// This object exists to capture all the Tofino-facing information required to +/// This object exists to capture the Tofino-facing information required to /// keep track of the logical link. It is converted to a `crate::LinkView` for /// exposure in the public API. +/// +/// # Addresses +/// +/// The switch (loopback) and its links may have addresses. Addresses from +/// all these interfaces coexist on the asic, so each address necessarily +/// belongs to exactly one interface. DPD encodes this by storing all +/// addresses in a single mapping defined in [`crate::addr`]. +/// +/// That means a [`Link`]'s addresses are managed by keying into +/// [`crate::addr::AddrMap`] using this link's [`crate::addr::AsicAddrIface`] id. +/// And a link should not be considered "deleted" until its state +/// has been removed from that map. (And by extension, from the asic). +/// +/// ### Link Local +/// +/// Spec: On the Oxide platform, a link shall have at most one +/// IPv6 link local address. That address shall be EUI-64 +/// derived from the physical MAC address. +/// +/// Because MAC addresses are unique, Oxide IPv6 link local +/// addresses are necessarily also unique. By this property, +/// we can reuse the existing [`crate::addr`] machinery +/// to track a link's Link Local as well. #[derive(Debug)] pub struct Link { /// The switch port on which this logical link resides. @@ -236,10 +278,6 @@ pub struct Link { pub link_state: LinkState, /// The kind of media in the link. pub media: PortMedia, - /// A list of IPv4 addresses assigned to this link. - pub ipv4: BTreeSet, - /// A list of IPv6 addresses assigned to this link. - pub ipv6: BTreeSet, /// Tracks the history of linkup/linkdown transitions, allowing us to /// detect flapping links. pub linkup_tracker: LinkUpTracker, @@ -277,31 +315,6 @@ impl From<&Link> for LinkView { } } -impl From for LinkView { - fn from(m: crate::link::Link) -> Self { - Self::from(&m) - } -} - -impl From<&Link> for TfportData { - fn from(m: &Link) -> Self { - Self { - port_id: m.port_id, - link_id: m.link_id, - asic_id: m.asic_port_id, - mac: m.config.mac, - ipv6_enabled: m.ipv6_enabled, - link_local: m.link_local(), - } - } -} - -impl From for TfportData { - fn from(m: Link) -> Self { - Self::from(&m) - } -} - // This struct represents the configuration of the link requested by the // user/sled-agent #[derive(Debug)] @@ -475,8 +488,6 @@ impl Link { fsm_state: asic::PortFsmState::default(), link_state: LinkState::Unknown, media: PortMedia::None, - ipv4: BTreeSet::new(), - ipv6: BTreeSet::new(), linkup_tracker: LinkUpTracker::default(), autoneg_tracker: AutonegTracker::default(), @@ -485,14 +496,6 @@ impl Link { } } - /// Return the link-local address for this link, if one has been added. - pub fn link_local(&self) -> Option { - self.ipv6 - .iter() - .find(|entry| (entry.addr.segments()[0] & 0xffc0) == 0xfe80) - .map(|entry| entry.addr) - } - /// Return the FEC scheme in use for this link. If the link has not yet /// been successfully configured in the hardware, this value will reflect /// the scheme requested by the user - if any. If the link has been @@ -506,6 +509,23 @@ impl Link { self.config.fec } } + + /// Creates a [`TfportData`] view of this link. + pub fn tfport_data(&self, addrs: &SyncAddrMap) -> TfportData { + TfportData { + port_id: self.port_id, + link_id: self.link_id, + asic_id: self.asic_port_id, + mac: self.config.mac, + ipv6_enabled: self.ipv6_enabled, + link_local: addrs + .get() + .read() + .unwrap() + .get_link_local(self.asic_addr_iface()) + .map(Ipv6Addr::from), + } + } } fn from_aal_ber(value: aal::Ber) -> Ber { @@ -681,7 +701,7 @@ impl Switch { .filter_map(|link_lock| { let link = link_lock.lock().unwrap(); if link.port_id == port_id { - Some(TfportData::from(&*link)) + Some(link.tfport_data(&self.addrs)) } else { None } @@ -707,107 +727,53 @@ impl Switch { link_id: LinkId, ) -> DpdResult<()> { let link_lock = self.get_link_lock(port_id, link_id)?; - let mut link = link_lock.lock().unwrap(); - - // Delete all addresses in the switch tables for this link. - if !link.ipv4.is_empty() { - let to_delete = std::mem::take(&mut link.ipv4) - .into_iter() - .map(|entry| entry.addr); - port_ip::ipv4_delete_many(self, link.asic_port_id, to_delete)?; - } - if !link.ipv6.is_empty() { - let to_delete = std::mem::take(&mut link.ipv6) - .into_iter() - .map(|entry| entry.addr); - port_ip::ipv6_delete_many(self, link.asic_port_id, to_delete)?; + { + let mut link = link_lock.lock().unwrap(); + self.addrs.clear_addrs::( + self, + link.asic_addr_iface(), + &OptTag::Any, + )?; + link.config.delete_me = true; } // Notify the reconciliation task that this link's ASIC resources need // to be released. - link.config.delete_me = true; self.reconciler.trigger(port_id, link_id); Ok(()) } - /// Clear all the state associated with all data links. - pub fn clear_link_state(&self) -> DpdResult<()> { + /// Clear all IP addresses belonging to all links on the switch. + /// + /// Optionally scoped to all IP addresses belonging to the specified tag. + pub fn clear_link_addresses(&self, tag: &OptTag) -> DpdResult<()> { let links = self.links.lock().unwrap(); + let mut errors: Option> = None; + for link_lock in links.0.values() { - let mut link = link_lock.lock().unwrap(); - // Clear all IP addresses. - // - // Swap out an empty map with the existing one, so that we can - // retain an iterable for calling `ipv{4,6}_delete_many`. - if !link.ipv4.is_empty() { - let to_delete = std::mem::take(&mut link.ipv4) - .into_iter() - .map(|entry| entry.addr); - port_ip::ipv4_delete_many(self, link.asic_port_id, to_delete)?; - } - if !link.ipv6.is_empty() { - let to_delete = std::mem::take(&mut link.ipv6) - .into_iter() - .map(|entry| entry.addr); - port_ip::ipv6_delete_many(self, link.asic_port_id, to_delete)?; - } + let cleared = { + let link = link_lock.lock().unwrap(); + self.addrs.clear_addrs::( + self, + link.asic_addr_iface(), + tag, + ) + }; + match cleared { + Ok(()) => (), + Err(DpdError::AddrClear(failed)) => { + errors.get_or_insert_default().extend(failed); + } + unexpected @ Err(_) => return unexpected, + }; } - Ok(()) - } - /// Clear any IP addresses associated with all links, optionally restricted - /// to a specified string `tag`. - pub fn clear_link_addresses(&self, tag: Option<&str>) -> DpdResult<()> { - if let Some(tag) = tag { - let links = self.links.lock().unwrap(); - for link_lock in links.0.values() { - let mut link = link_lock.lock().unwrap(); - self.clear_link_addresses_locked(&mut link, tag); - } - Ok(()) - } else { - self.clear_link_state() + if let Some(failed) = errors { + return Err(DpdError::AddrClear(failed)); } - } - fn clear_link_addresses_locked(&self, link: &mut Link, tag: &str) { - // Remove all entries from the set with the provided tag. - // - // TODO-cleanup: It'd be nice to use `drain_filter` here, - // but that is unstable. - let mut to_remove = Vec::new(); - link.ipv4.retain(|entry| { - if entry.tag == tag { - to_remove.push(entry.addr); - false - } else { - true - } - }); - - // Delete the entries from the ASIC tables. - let _ = port_ip::ipv4_delete_many( - self, - link.asic_port_id, - to_remove.into_iter(), - ); - - // TODO-cleanup: See note above about `drain_filter`. - let mut to_remove = Vec::new(); - link.ipv6.retain(|entry| { - if entry.tag == tag { - to_remove.push(entry.addr); - false - } else { - true - } - }); - let _ = port_ip::ipv6_delete_many( - self, - link.asic_port_id, - to_remove.into_iter(), - ); + Ok(()) } // Update the state of a link with a closure. @@ -1055,207 +1021,119 @@ impl Switch { self.link_fetch(port_id, link_id, |link| link.asic_port_id) } - /// Add an IPv4 address to the provided link. - pub fn create_ipv4_address_locked( + /// Adds an IP address to the provided link if it does not currently exist. + /// + /// On success, returns whether any tables were modified. + pub fn set_ip_address_locked( &self, link: &mut Link, - entry: Ipv4Entry, - ) -> DpdResult<()> { - if link.ipv4.contains(&entry) { - Err(DpdError::Exists(format!( - "IP address {} already exists", - entry.addr - ))) - } else { - port_ip::ipv4_add(self, link.asic_port_id, entry.addr)?; - link.ipv4.insert(entry); - Ok(()) - } + addr: TableIp, + tag: String, + ) -> DpdResult { + self.addrs.get().write().unwrap().try_set( + self, + addr, + link.asic_addr_iface(), + tag, + ) } - /// Add an IPv4 address to the specified link. - pub fn create_ipv4_address( + pub fn create_ip_address( &self, port_id: PortId, link_id: LinkId, - entry: Ipv4Entry, + addr: TableIp, + tag: String, ) -> DpdResult<()> { self.link_update(port_id, link_id, |link| { - self.create_ipv4_address_locked(link, entry) - }) - } - - /// List a page of IPv4 addresses associated with the link. - pub fn list_ipv4_addresses( - &self, - port_id: PortId, - link_id: LinkId, - last_address: Option, - limit: usize, - ) -> DpdResult> { - self.link_fetch(port_id, link_id, |link| { - if let Some(addr) = last_address { - // Equality only considers the address, so create an entry - // with an empty tag. - use std::ops::Bound; - let entry = Ipv4Entry { tag: String::new(), addr }; - link.ipv4 - .range((Bound::Excluded(entry), Bound::Unbounded)) - .take(limit) - .cloned() - .collect() - } else { - link.ipv4.iter().take(limit).cloned().collect() + if !self.set_ip_address_locked(link, addr, tag)? { + return Err(DpdError::Exists(format!( + "This tag has already registered address {addr:?}" + ))); } - }) - } - - /// Delete one IPv4 address on the provided link. - pub fn delete_ipv4_address_locked( - &self, - link: &mut Link, - address: Ipv4Addr, - ) -> DpdResult<()> { - let entry = Ipv4Entry { tag: String::new(), addr: address }; - - if link.ipv4.contains(&entry) { - port_ip::ipv4_delete(self, link.asic_port_id, address)?; - link.ipv4.remove(&entry); Ok(()) - } else { - Err(DpdError::NoSuchAddress { - port_id: link.port_id, - link_id: link.link_id, - address: address.into(), - }) - } - } - - /// Delete one IPv4 address on the specified link. - pub fn delete_ipv4_address( - &self, - port_id: PortId, - link_id: LinkId, - address: Ipv4Addr, - ) -> DpdResult<()> { - self.link_update(port_id, link_id, |link| { - self.delete_ipv4_address_locked(link, address) }) } - /// Delete all IPv4 address on the specified link. - pub fn reset_ipv4_addresses( + /// Delete one IP address on the specified link. + pub fn delete_ip_address( &self, port_id: PortId, link_id: LinkId, + addr: TableIp, + tag: &OptTag, ) -> DpdResult<()> { self.link_update(port_id, link_id, |link| { - while let Some(Ipv4Entry { addr, .. }) = link.ipv4.pop_first() { - port_ip::ipv4_delete(self, link.asic_port_id, addr)?; + let iface_id = link.asic_addr_iface(); + if !self.addrs.clear_addr(self, iface_id, addr, tag)? { + return Err(DpdError::NoSuchAddress { + port_id, + link_id, + address: addr.into(), + }); } Ok(()) }) } - /// Add an IPv6 address to the provided link. - pub fn create_ipv6_address_locked( - &self, - link: &mut Link, - entry: Ipv6Entry, - ) -> DpdResult<()> { - if link.ipv6.contains(&entry) { - Err(DpdError::Exists(format!( - "IP address {} already exists", - entry.addr - ))) - } else { - port_ip::ipv6_add(self, link.asic_port_id, entry.addr)?; - link.ipv6.insert(entry); - Ok(()) - } - } - - /// Add an IPv6 address to the specified link. - pub fn create_ipv6_address( + pub fn list_ip_addresses( &self, port_id: PortId, link_id: LinkId, - entry: Ipv6Entry, - ) -> DpdResult<()> { - self.link_update(port_id, link_id, |link| { - self.create_ipv6_address_locked(link, entry) - }) - } - - /// List a page of IPv6 addresses associated with the link. - pub fn list_ipv6_addresses( - &self, - port_id: PortId, - link_id: LinkId, - last_address: Option, + last_entry: Option<(IpAddr, &str)>, limit: usize, - ) -> DpdResult> { - self.link_fetch(port_id, link_id, |link| { - if let Some(addr) = last_address { - // Equality only considers the address, so create an entry - // with an empty tag. - use std::ops::Bound; - let entry = Ipv6Entry { tag: String::new(), addr }; - link.ipv6 - .range((Bound::Excluded(entry), Bound::Unbounded)) - .take(limit) - .cloned() - .collect() - } else { - link.ipv6.iter().take(limit).cloned().collect() - } - }) - } - - /// Delete one IPv6 address on the provided link. - pub fn delete_ipv6_address_locked( - &self, - link: &mut Link, - address: Ipv6Addr, - ) -> DpdResult<()> { - let entry = Ipv6Entry { tag: String::new(), addr: address }; - - if link.ipv6.contains(&entry) { - port_ip::ipv6_delete(self, link.asic_port_id, address)?; - link.ipv6.remove(&entry); - Ok(()) + convert: impl Fn(&A, &str) -> T, + ) -> DpdResult> { + let mut start_addr = None; + let all_tags = (Bound::Unbounded, Bound::Unbounded); + + let (addr_bounds, tag_bounds) = if let Some((addr, tag)) = last_entry { + start_addr = Some(addr); + let addr = (Bound::Excluded(addr), Bound::Unbounded); + let tag = (Bound::Excluded(tag), Bound::Unbounded); + (addr, tag) } else { - Err(DpdError::NoSuchAddress { - port_id: link.port_id, - link_id: link.link_id, - address: address.into(), - }) - } - } + let addr = (Bound::Unbounded, Bound::Unbounded); + let tag = (Bound::Unbounded, Bound::Unbounded); + (addr, tag) + }; - /// Delete one IPv6 address on the specified link. - pub fn delete_ipv6_address( - &self, - port_id: PortId, - link_id: LinkId, - address: Ipv6Addr, - ) -> DpdResult<()> { - self.link_update(port_id, link_id, |link| { - self.delete_ipv6_address_locked(link, address) + self.link_fetch(port_id, link_id, |link| { + self.addrs + .get() + .read() + .unwrap() + .iface_addr_range(link.asic_addr_iface(), addr_bounds) + .flat_map(move |(addr, tags)| { + let range = if start_addr.is_some_and(|a| a == addr) { + tag_bounds + } else { + all_tags + }; + tags.range::(range).map(move |t| (addr, t.as_str())) + }) + .filter_map(|(addr, tag)| { + Some(convert(&A::from_ip(addr)?, tag)) + }) + .take(limit) + .collect() }) } - /// Delete all IPv6 address on the specified link. - pub fn reset_ipv6_addresses( + /// Delete all addresses of the given type on the specified link. + /// + /// Returns [`DpdError::AddrClear`] if any addresses could not be deleted. + pub fn reset_addresses( &self, port_id: PortId, link_id: LinkId, ) -> DpdResult<()> { self.link_update(port_id, link_id, |link| { - while let Some(Ipv6Entry { addr, .. }) = link.ipv6.pop_first() { - port_ip::ipv6_delete(self, link.asic_port_id, addr)?; - } - Ok(()) + self.addrs.clear_addrs::( + self, + link.asic_addr_iface(), + &OptTag::Any, + ) }) } @@ -1644,7 +1522,7 @@ fn set_mac_config( #[cfg(feature = "multicast")] { mac::mcast_mac_set(switch, asic_id, mac)?; - mcast::mcast_egress::add_port_mapping_entry(switch, asic_id)?; + mcast::mcast_egress::set_port_mapping_entry(switch, asic_id)?; } Ok(()) } @@ -1655,7 +1533,7 @@ fn clear_mac_config(switch: &Switch, asic_id: AsicId) -> DpdResult<()> { #[cfg(feature = "multicast")] { mac::mcast_mac_clear(switch, asic_id)?; - mcast::mcast_egress::del_port_mapping_entry(switch, asic_id)?; + mcast::mcast_egress::clear_port_mapping_entry(switch, asic_id)?; } Ok(()) } @@ -1686,14 +1564,21 @@ fn unplumb_link( } if link.plumbed.link_created { - if let Err(e) = switch.asic_hdl.port_delete(link.port_hdl) { - error!(log, "failed to delete ASIC port: {e:?}"); - return Err(e.into()); + match switch.asic_hdl.port_delete(link.port_hdl) { + Ok(()) | Err(AsicError::Missing(_)) => { + link.plumbed.link_created = false; + switch.record_event( + link.asic_port_id, + Event::Admin(AdminEvent::Delete), + ); + } + Err(e) => { + error!(log, "failed to delete ASIC port: {e:?}"); + return Err(e.into()); + } } - link.plumbed.link_created = false; - switch - .record_event(link.asic_port_id, Event::Admin(AdminEvent::Delete)); } + Ok(()) } @@ -1871,10 +1756,20 @@ async fn reconcile_link( } if link.config.delete_me { + if let Err(e) = links.delete_link(switch, &link) { + self::record_plumb_failure( + switch, + &mut link, + "releasing link addresses before deletion", + &e, + ); + error!( + log, + "Failed to release addresses before link deletion: {e:?}" + ); + return; + } switch.free_mac_address(link.config.mac); - links - .delete_link(port_id, link_id) - .expect("link must exist as the links map is locked"); return; } drop(links); diff --git a/dpd/src/loopback.rs b/dpd/src/loopback.rs index f0a4665c..169a6c8a 100644 --- a/dpd/src/loopback.rs +++ b/dpd/src/loopback.rs @@ -4,121 +4,37 @@ // // Copyright 2026 Oxide Computer Company -use crate::{DpdError, DpdResult, Switch, table}; -use aal::AsicError; -use common::ports::{Ipv4Entry, Ipv6Entry}; -use slog::debug; -use slog::warn; -use std::collections::BTreeSet; -use std::net::{Ipv4Addr, Ipv6Addr}; +use common::addr::RouterIfaceIpAddr; -/// The set of configured loopback addresses on the switch. -pub struct LoopbackData { - pub v4_addrs: BTreeSet, - pub v6_addrs: BTreeSet, -} - -/// Initialize loopback data to empty sets of IPv4 and IPv6 addresses. -pub fn init() -> LoopbackData { - LoopbackData { v4_addrs: BTreeSet::new(), v6_addrs: BTreeSet::new() } -} +use crate::addr::AsicAddrIface; +use crate::addr::OptTag; +use crate::{DpdResult, Switch}; /// Add a loopback IPv4 address to the switch. -pub fn add_loopback_ipv4(switch: &Switch, addr: &Ipv4Entry) -> DpdResult<()> { - let mut loopback_data = switch.loopback.lock().unwrap(); - if loopback_data.v4_addrs.contains(addr) { - debug!(switch.log, "loopback entry {} already set", addr.addr); - return Ok(()); - } - match table::port_ip::loopback_ipv4_add(switch, addr.addr) { - Ok(()) => _ = loopback_data.v4_addrs.insert(addr.clone()), - Err(DpdError::Switch(AsicError::Exists)) => { - if !loopback_data.v4_addrs.contains(addr) { - warn!( - switch.log, - "loopback entry {} was present on ASIC but not in soft state", - addr.addr, - ); - loopback_data.v4_addrs.insert(addr.clone()); - } - } - Err(e) => return Err(e), - } - Ok(()) -} - -/// Delete a loopback IPv4 address from the switch. -pub fn delete_loopback_ipv4(switch: &Switch, addr: &Ipv4Addr) -> DpdResult<()> { - let mut loopback_data = switch.loopback.lock().unwrap(); - let entry = Ipv4Entry { addr: *addr, tag: "".into() }; - if !loopback_data.v4_addrs.contains(&entry) { - debug!(switch.log, "loopback entry {} not set", addr); - return Ok(()); - } - match table::port_ip::loopback_ipv4_delete(switch, *addr) { - Ok(()) => _ = loopback_data.v4_addrs.remove(&entry), - Err(DpdError::Switch(AsicError::Missing(_))) => { - if loopback_data.v4_addrs.contains(&entry) { - warn!( - switch.log, - "loopback entry {} was present in soft state but not on ASIC", - addr, - ); - loopback_data.v4_addrs.remove(&entry); - } - } - Err(e) => return Err(e), - } +pub fn set_loopback( + switch: &Switch, + addr: RouterIfaceIpAddr, + tag: String, +) -> DpdResult<()> { + switch.addrs.get().write().unwrap().try_set( + switch, + addr.into(), + AsicAddrIface::Loopback, + tag, + )?; Ok(()) } -/// Add a loopback IPv6 address to the switch. -pub fn add_loopback_ipv6(switch: &Switch, addr: &Ipv6Entry) -> DpdResult<()> { - let mut loopback_data = switch.loopback.lock().unwrap(); - if loopback_data.v6_addrs.contains(addr) { - debug!(switch.log, "loopback entry {} already set", addr.addr); - return Ok(()); - } - - match table::port_ip::loopback_ipv6_add(switch, addr.addr) { - Ok(()) => _ = loopback_data.v6_addrs.insert(addr.clone()), - Err(DpdError::Switch(AsicError::Exists)) => { - if !loopback_data.v6_addrs.contains(addr) { - warn!( - switch.log, - "loopback entry {} was present on ASIC but not in soft state", - addr.addr, - ); - loopback_data.v6_addrs.insert(addr.clone()); - } - } - Err(e) => return Err(e), - } - Ok(()) -} - -/// Delete a loopback IPv6 address from the switch. -pub fn delete_loopback_ipv6(switch: &Switch, addr: &Ipv6Addr) -> DpdResult<()> { - let mut loopback_data = switch.loopback.lock().unwrap(); - let entry = Ipv6Entry { addr: *addr, tag: "".into() }; - if !loopback_data.v6_addrs.contains(&entry) { - debug!(switch.log, "loopback entry {} not set", addr); - return Ok(()); - } - - match table::port_ip::loopback_ipv6_delete(switch, *addr) { - Ok(()) => _ = loopback_data.v6_addrs.remove(&entry), - Err(DpdError::Switch(AsicError::Missing(_))) => { - if loopback_data.v6_addrs.contains(&entry) { - warn!( - switch.log, - "loopback entry {} was present in soft state but not on ASIC", - addr, - ); - loopback_data.v6_addrs.remove(&entry); - } - } - Err(e) => return Err(e), - } +pub fn clear_loopback( + switch: &Switch, + addr: RouterIfaceIpAddr, + tag: &OptTag, +) -> DpdResult<()> { + switch.addrs.get().write().unwrap().try_clear( + switch, + addr.into(), + AsicAddrIface::Loopback, + tag, + )?; Ok(()) } diff --git a/dpd/src/main.rs b/dpd/src/main.rs index 5fc08ec5..14f13739 100644 --- a/dpd/src/main.rs +++ b/dpd/src/main.rs @@ -35,6 +35,7 @@ use tokio::sync::Mutex as TokioMutex; use tokio::time::Duration; use tokio::time::sleep; +use crate::addr::SyncAddrMap; use crate::macaddrs::BaseMac; use crate::port_map::SidecarRevision; use crate::rpw::WorkflowServer; @@ -54,6 +55,7 @@ cfg_if::cfg_if! { } } +mod addr; mod api_server; mod arp; mod attached_subnet; @@ -195,7 +197,7 @@ pub struct Switch { pub arp: Mutex, pub nat: nat::Nat, pub attached_subnet: Mutex, - pub loopback: Mutex, + pub addrs: SyncAddrMap, pub identifiers: Mutex>, pub oximeter_producer: Mutex>, pub oximeter_meta: Mutex>, @@ -308,9 +310,9 @@ impl Switch { counters, routes: TokioMutex::new(route_data), arp: Mutex::new(arp::init()), + addrs: SyncAddrMap::default(), nat: nat::Nat::new(), attached_subnet: Mutex::new(attached_subnet::init()), - loopback: Mutex::new(loopback::init()), switch_ports, identifiers: Mutex::new(None), oximeter_producer: Mutex::new(None), @@ -382,6 +384,28 @@ impl Switch { }) } + /// Calls [`Self::table_entry_add`]. + /// If add fails due to conflict, tries [`Self::table_entry_update`]. + /// Returns err if the first failure is fatal or both calls fail. + pub fn table_entry_set( + &self, + table_type: TableType, + key: &M, + data: &A, + ) -> DpdResult<()> { + match self.table_entry_add(table_type, key, data) { + Err(DpdError::Switch(AsicError::Exists)) => { + self.table_entry_update(table_type, key, data) + } + Err(full @ DpdError::TableFull(_)) => { + // If the table is full and this key does not exist, + // a "does not exist" error is just distracting. + self.table_entry_update(table_type, key, data).map_err(|_| full) + } + other => other, + } + } + /// Delete a single table entry. pub fn table_entry_del( &self, @@ -398,6 +422,21 @@ impl Switch { }) } + /// A variant of [`Self::table_entry_del`] that returns Ok + /// if the deleted resource already does not exist. + pub fn table_entry_clear( + &self, + table_type: TableType, + key: &M, + ) -> DpdResult<()> { + let maybe_deleted = self.table_entry_del(table_type, key); + if matches!(maybe_deleted, Err(DpdError::Switch(AsicError::Missing(_)))) + { + return Ok(()); + } + maybe_deleted + } + /// Fetch all of the entries in a P4 table and return them pub fn table_dump( &self, diff --git a/dpd/src/port_settings.rs b/dpd/src/port_settings.rs index cfcea772..cc0e0a2e 100644 --- a/dpd/src/port_settings.rs +++ b/dpd/src/port_settings.rs @@ -7,11 +7,13 @@ use crate::DpdError; use crate::DpdResult; use crate::Switch; +use crate::addr::OptTag; +use crate::addr::SyncAddrMap; +use crate::addr::TableIp; use crate::link::Link; use crate::link::LinkParams; use aal::AsicOps; -use common::ports::Ipv4Entry; -use common::ports::Ipv6Entry; +use common::addr::RouterIfaceIpAddr; use common::ports::PortFec; use common::ports::PortId; use common::ports::PortSpeed; @@ -23,11 +25,9 @@ use slog::Logger; use slog::debug; use slog::error; use slog::trace; +use std::collections::BTreeMap; use std::collections::BTreeSet; use std::collections::HashMap; -use std::net::IpAddr; -use std::net::Ipv4Addr; -use std::net::Ipv6Addr; use std::sync::Arc; use std::sync::Mutex; use std::sync::MutexGuard; @@ -99,14 +99,24 @@ struct LinkSpec { pub autoneg: bool, pub kr: bool, pub delete_me: bool, - pub ipv4: BTreeSet, - pub ipv6: BTreeSet, + // BTree* so operations follow a consistent order, which + // makes failures easier to reproduce and understand. + pub addrs: BTreeMap>, pub tx_eq: Option, pub allow_ddm_traffic: bool, } -impl From<&Link> for LinkSpec { - fn from(p: &Link) -> Self { +impl LinkSpec { + /// Constructs a new [`LinkSpec`] from the given link. + fn new(p: &Link, amap: &SyncAddrMap) -> Self { + let addrs = amap + .get() + .read() + .unwrap() + .iter_by_iface(p.asic_addr_iface()) + .map(|(ip, tags)| (ip, tags.clone())) + .collect(); + Self { speed: p.config.speed, fec: p.config.fec, @@ -114,40 +124,43 @@ impl From<&Link> for LinkSpec { kr: p.config.kr, tx_eq: p.tx_eq, delete_me: p.config.delete_me, - ipv4: p.ipv4.iter().map(|x| x.addr).collect(), - ipv6: p.ipv6.iter().map(|x| x.addr).collect(), + addrs, allow_ddm_traffic: p.config.allow_ddm_traffic, } } -} -impl From<&LinkSettings> for LinkSpec { - fn from(l: &LinkSettings) -> Self { - Self { + /// Constructs a [`LinkSpec`] based on the given [`LinkSettings`]. + /// + /// If `current` is Some, the resulting LinkSpec includes addresses + /// from [`LinkSettings`] and those of all *other* tags in `current`. + /// + /// Returns error if an address in [`LinkSettings`] is already + /// owned in `current` by another tag. + pub fn try_from_settings( + l: &LinkSettings, + tag: &str, + current: Option<&LinkSpec>, + ) -> DpdResult { + let mut addrs = current.map(|c| c.addrs.clone()).unwrap_or_default(); + addrs.retain(|_, tags| { + tags.remove(tag); + !tags.is_empty() + }); + + for addr in l.addrs.iter().copied().map(RouterIfaceIpAddr::try_new) { + addrs.entry(addr?).or_default().insert(tag.to_string()); + } + + Ok(Self { speed: l.params.speed, fec: l.params.fec, autoneg: l.params.autoneg, kr: l.params.kr, tx_eq: l.params.tx_eq, delete_me: false, - ipv4: l - .addrs - .iter() - .filter_map( - |x| if let IpAddr::V4(a) = x { Some(a) } else { None }, - ) - .copied() - .collect(), - ipv6: l - .addrs - .iter() - .filter_map( - |x| if let IpAddr::V6(a) = x { Some(a) } else { None }, - ) - .copied() - .collect(), + addrs, allow_ddm_traffic: l.params.allow_ddm_traffic, - } + }) } } @@ -194,9 +207,11 @@ impl PortSettingsDiff { self.links.add = links_to_add .map(|id| { - (*id, ChangeNode::Unchanged((&settings.links[&id.0]).into())) + let conf = &settings.links[&id.0]; + let spec = LinkSpec::try_from_settings(conf, ctx.tag, None)?; + Ok((*id, ChangeNode::Unchanged(spec))) }) - .collect(); + .collect::>()?; self.links.delete = links_to_del .map(|id| { @@ -210,24 +225,26 @@ impl PortSettingsDiff { .collect(); self.links.modify = links_to_mod - .map(|id| { - let settings_link = (&settings.links[&id.0]).into(); - let switch_link = ctx.link_spec(*id).expect( + .filter_map(|id| { + let before = ctx.link_spec(*id).expect( "link existence is guaranteed by the locked link map", ); - (id, settings_link, switch_link) - }) - .filter(|(_, settings, switch)| settings != switch) - .map(|(id, settings, switch)| { - ( - *id, - ChangeNode::Unchanged(Modify { - before: switch, - after: settings, - }), - ) + let maybe_after = LinkSpec::try_from_settings( + &settings.links[&id.0], + ctx.tag, + Some(&before), + ); + + match maybe_after { + Err(e) => Some(Err(e)), + Ok(after) if before != after => Some(Ok(( + *id, + ChangeNode::Unchanged(Modify { before, after }), + ))), + Ok(_unchanged) => None, + } }) - .collect(); + .collect::>()?; Ok(()) } @@ -307,7 +324,11 @@ impl PortSettingsDiff { ) -> DpdResult<()> { let mac = ctx.switch.allocate_mac_address(ctx.port_id, link_id)?; rb.wind(move |ctx: &mut Context<'_>| -> DpdResult<()> { - ctx.switch.free_mac_address(mac); + // Only free the mac address if rollback successfully + // released the link. + if ctx.link(link_id).is_err() { + ctx.switch.free_mac_address(mac); + }; Ok(()) }); @@ -341,23 +362,26 @@ impl PortSettingsDiff { .link_map .get_link(port_id, link_id) .expect("the link was just inserted, so it must exist"); - let mut link = link_lock.lock().unwrap(); rb.wind(move |ctx: &mut Context<'_>| -> DpdResult<()> { - (*ctx.link_map) - .delete_link(ctx.port_id, link_id) - .expect("link existence is guaranteed by the link_map lock"); + if let Ok(link) = ctx.link(link_id) { + let link = link.lock().unwrap(); + (*ctx.link_map).delete_link(ctx.switch, &link)?; + }; Ok(()) }); - // Create the IPv4 addresses - for addr in spec.ipv4.iter().copied() { - Self::addr_add_v4(ctx, &mut link, rb, addr)?; - } - - // Create the IPv6 addresses - for addr in spec.ipv6.iter().copied() { - Self::addr_add_v6(ctx, &mut link, rb, addr)?; + let mut link = link_lock.lock().unwrap(); + for (addr, tags) in &spec.addrs { + for tag in tags.iter() { + Self::addr_set( + ctx, + &mut link, + rb, + TableIp::Router(*addr), + tag.clone(), + )?; + } } Ok(()) @@ -381,14 +405,16 @@ impl PortSettingsDiff { Ok(()) }); - // Delete the IPv4 addresses - for addr in spec.ipv4.iter().copied() { - Self::addr_del_v4(ctx, &mut link, rb, addr)?; - } - - // Delete the IPv6 addresses - for addr in spec.ipv6.iter().copied() { - Self::addr_del_v6(ctx, &mut link, rb, addr)?; + for (addr, tags) in &spec.addrs { + for tag in tags.iter() { + Self::addr_clear( + ctx, + &mut link, + rb, + TableIp::Router(*addr), + tag.clone(), + )?; + } } Ok(()) @@ -411,8 +437,7 @@ impl PortSettingsDiff { autoneg: an_before, kr: kr_before, delete_me: delete_before, - ipv4: ref ipv4_before, - ipv6: ref ipv6_before, + addrs: ref addrs_before, tx_eq: tx_eq_before, allow_ddm_traffic: allow_ddm_traffic_before, } = before; @@ -423,8 +448,7 @@ impl PortSettingsDiff { autoneg: an_after, kr: kr_after, delete_me: _, - ipv4: ref ipv4_after, - ipv6: ref ipv6_after, + addrs: ref addrs_after, tx_eq: tx_eq_after, allow_ddm_traffic: allow_ddm_traffic_after, } = after; @@ -472,123 +496,88 @@ impl PortSettingsDiff { Ok(()) }); - // ipv4 addrs - let v4_add: BTreeSet = - ipv4_after.difference(ipv4_before).copied().collect(); - - let v4_del: BTreeSet = - ipv4_before.difference(ipv4_after).copied().collect(); - - for addr in v4_add { - Self::addr_add_v4(ctx, &mut link, rb, addr)?; - } - for addr in v4_del { - Self::addr_del_v4(ctx, &mut link, rb, addr)?; + for (addr, tags) in addrs_after { + for tag in tags.iter() { + if !addrs_before.get(addr).is_some_and(|t| t.contains(tag)) { + Self::addr_set( + ctx, + &mut link, + rb, + TableIp::Router(*addr), + tag.clone(), + )?; + } + } } - // ipv6 addrs - let v6_add: BTreeSet = - ipv6_after.difference(ipv6_before).copied().collect(); - - let v6_del: BTreeSet = - ipv6_before.difference(ipv6_after).copied().collect(); - - for addr in v6_add { - Self::addr_add_v6(ctx, &mut link, rb, addr)?; - } - for addr in v6_del { - Self::addr_del_v6(ctx, &mut link, rb, addr)?; + for (addr, tags) in addrs_before { + for tag in tags.iter() { + if !addrs_after.get(addr).is_some_and(|t| t.contains(tag)) { + Self::addr_clear( + ctx, + &mut link, + rb, + TableIp::Router(*addr), + tag.clone(), + )?; + } + } } Ok(()) } - fn addr_add_v4( + fn addr_set( ctx: &mut Context<'_>, link: &mut Link, rb: &mut Rollback, - addr: Ipv4Addr, + addr: TableIp, + tag: String, ) -> DpdResult<()> { - trace!(ctx.log, "ipv4 add {addr}"); + trace!(ctx.log, "ip add ({addr:?}: {tag})"); // Create address on ASIC first. - let entry = - Ipv4Entry { tag: ctx.tag.clone().unwrap_or("".into()), addr }; let switch = ctx.switch; - switch.create_ipv4_address_locked(link, entry)?; + switch.set_ip_address_locked(link, addr, tag.clone())?; let link_id = link.link_id; rb.wind(move |ctx: &mut Context<'_>| -> DpdResult<()> { let switch = ctx.switch; let link_lock = ctx.link(link_id)?; - let mut link = link_lock.lock().unwrap(); - switch.delete_ipv4_address_locked(&mut link, addr) - }); - Ok(()) - } - - fn addr_del_v4( - ctx: &mut Context<'_>, - link: &mut Link, - rb: &mut Rollback, - addr: Ipv4Addr, - ) -> DpdResult<()> { - trace!(ctx.log, "ipv4 del {addr}"); - let entry = - Ipv4Entry { tag: ctx.tag.clone().unwrap_or("".into()), addr }; - let switch = ctx.switch; - let link_id = link.link_id; - switch.delete_ipv4_address_locked(link, addr)?; - - rb.wind(move |ctx: &mut Context<'_>| -> DpdResult<()> { - let switch = ctx.switch; - let link_lock = ctx.link(link_id)?; - let mut link = link_lock.lock().unwrap(); - switch.create_ipv4_address_locked(&mut link, entry) - }); - Ok(()) - } - - fn addr_add_v6( - ctx: &mut Context<'_>, - link: &mut Link, - rb: &mut Rollback, - addr: Ipv6Addr, - ) -> DpdResult<()> { - trace!(ctx.log, "ipv6 add {addr}"); - // Create address on ASIC first. - let entry = - Ipv6Entry { tag: ctx.tag.clone().unwrap_or("".into()), addr }; - let switch = ctx.switch; - let link_id = link.link_id; - switch.create_ipv6_address_locked(link, entry)?; - - rb.wind(move |ctx: &mut Context<'_>| -> DpdResult<()> { - let switch = ctx.switch; - let link_lock = ctx.link(link_id)?; - let mut link = link_lock.lock().unwrap(); - switch.delete_ipv6_address_locked(&mut link, addr) + let link = link_lock.lock().unwrap(); + switch.addrs.clear_addr( + switch, + link.asic_addr_iface(), + addr, + &OptTag::exact(&tag), + )?; + Ok(()) }); Ok(()) } - fn addr_del_v6( + fn addr_clear( ctx: &mut Context<'_>, link: &mut Link, rb: &mut Rollback, - addr: Ipv6Addr, + addr: TableIp, + tag: String, ) -> DpdResult<()> { - trace!(ctx.log, "ipv6 del {addr}"); + trace!(ctx.log, "ip clear ({addr}: {tag})"); let switch = ctx.switch; let link_id = link.link_id; - switch.delete_ipv6_address_locked(link, addr)?; + switch.addrs.clear_addr( + switch, + link.asic_addr_iface(), + addr, + &OptTag::exact(&tag), + )?; rb.wind(move |ctx: &mut Context<'_>| -> DpdResult<()> { - let entry = - Ipv6Entry { tag: ctx.tag.clone().unwrap_or("".into()), addr }; let switch = ctx.switch; let link_lock = ctx.link(link_id)?; let mut link = link_lock.lock().unwrap(); - switch.create_ipv6_address_locked(&mut link, entry) + switch.set_ip_address_locked(&mut link, addr, tag)?; + Ok(()) }); Ok(()) } @@ -628,18 +617,18 @@ struct Context<'a> { port_id: PortId, switch: &'a Switch, link_map: MutexGuard<'a, crate::link::LinkMap>, - tag: Option, + tag: &'a str, log: Logger, rollback: bool, } macro_rules! context { - ($port_id:expr, $switch:expr) => { + ($port_id:expr, $switch:expr, $tag:expr) => { Context { port_id: $port_id, switch: $switch, link_map: $switch.links.lock().unwrap(), - tag: None, + tag: $tag, log: $switch.log.clone(), rollback: false, } @@ -654,20 +643,22 @@ impl Context<'_> { fn link_spec(&mut self, link_id: LinkId) -> DpdResult { let link_lock = self.link_map.get_link(self.port_id, link_id)?; let link = link_lock.lock().unwrap(); - Ok(LinkSpec::from(&*link)) + Ok(LinkSpec::new(&link, &self.switch.addrs)) } } impl Switch { /// Apply port settings as an atomic transaction. + // + // TODO::cory: Enforce port RouterIfaceIpAddr constraint + // across the port_settings_* api. pub async fn apply_port_settings( &self, port_id: PortId, settings: PortSettings, - tag: Option, + tag: &str, ) -> DpdResult { - let mut ctx = context!(port_id, self); - ctx.tag = tag; + let mut ctx = context!(port_id, self, tag); let mut diff = PortSettingsDiff::calculate(&mut ctx, &settings)?; trace!(self.log, "port settings diff: {:#?}", diff); @@ -680,10 +671,9 @@ impl Switch { pub async fn clear_port_settings( &self, port_id: PortId, - tag: Option, + tag: &str, ) -> DpdResult { - let mut ctx = context!(port_id, self); - ctx.tag = tag; + let mut ctx = context!(port_id, self, tag); let settings = PortSettings::default(); let mut diff = PortSettingsDiff::calculate(&mut ctx, &settings)?; @@ -697,10 +687,9 @@ impl Switch { pub async fn get_port_settings( &self, port_id: PortId, - tag: Option, + tag: &str, ) -> DpdResult { - let mut ctx = context!(port_id, self); - ctx.tag = tag; + let mut ctx = context!(port_id, self, tag); Self::get_port_settings_locked(&mut ctx, false) } @@ -737,7 +726,8 @@ impl Switch { if ignore_deleting && link.config.delete_me { None } else { - Some(((*link_id).into(), LinkSettings::from(&*link))) + let conf = link.settings(&ctx.switch.addrs, ctx.tag); + Some(((*link_id).into(), conf)) } } else { None diff --git a/dpd/src/table/mac.rs b/dpd/src/table/mac.rs index 533577f5..7264decb 100644 --- a/dpd/src/table/mac.rs +++ b/dpd/src/table/mac.rs @@ -37,7 +37,7 @@ fn mac_set_common( let match_key = MacMatchKey { port }; let action_data = MacAction::Rewrite { mac }; - match s.table_entry_add(type_, &match_key, &action_data) { + match s.table_entry_set(type_, &match_key, &action_data) { Ok(_) => { info!(s.log, "set mac on {port} in table {type_}: {mac}",); Ok(()) @@ -55,7 +55,7 @@ fn mac_set_common( fn mac_clear_common(s: &Switch, type_: TableType, port: u16) -> DpdResult<()> { let match_key = MacMatchKey { port }; - match s.table_entry_del(type_, &match_key) { + match s.table_entry_clear(type_, &match_key) { Ok(_) => { info!(s.log, "cleared mac on {port} in table {type_}",); Ok(()) @@ -68,8 +68,6 @@ fn mac_clear_common(s: &Switch, type_: TableType, port: u16) -> DpdResult<()> { } /// Add a new entry to the MAC table. -/// -/// An error is returned if the entry already exists. Use `mac_update` instead. pub fn mac_set(s: &Switch, port: u16, mac: MacAddr) -> DpdResult<()> { mac_set_common(s, TableType::PortMacAddress, port, mac) } @@ -100,8 +98,6 @@ pub fn reset(s: &Switch) -> DpdResult<()> { } /// Add a new entry to the MAC table. -/// -/// An error is returned if the entry already exists. Use `mac_update` instead. #[cfg(feature = "multicast")] pub fn mcast_mac_set(s: &Switch, port: u16, mac: MacAddr) -> DpdResult<()> { mac_set_common(s, TableType::PortMacAddressMcast, port, mac) diff --git a/dpd/src/table/mcast/mcast_egress.rs b/dpd/src/table/mcast/mcast_egress.rs index 10eed2e8..a2c6b170 100644 --- a/dpd/src/table/mcast/mcast_egress.rs +++ b/dpd/src/table/mcast/mcast_egress.rs @@ -186,9 +186,9 @@ pub(crate) fn reset_bitmap_table(s: &Switch) -> DpdResult<()> { s.table_clear(TableType::McastEgressDecapPorts) } -/// Add a port ID entry to the port ID table for converting ASIC port IDs +/// Set a port ID entry to the port ID table for converting ASIC port IDs /// to port numbers. -pub(crate) fn add_port_mapping_entry( +pub(crate) fn set_port_mapping_entry( s: &Switch, asic_port_id: u16, ) -> DpdResult<()> { @@ -200,7 +200,7 @@ pub(crate) fn add_port_mapping_entry( debug!(s.log, "add port id entry {match_key} -> {action_data:?}"); - s.table_entry_add( + s.table_entry_set( TableType::McastEgressPortMapping, &match_key, &action_data, @@ -231,7 +231,7 @@ pub(crate) fn update_port_mapping_entry( /// Delete a port ID entry from the port ID table for converting ASIC port IDs /// to port numbers. -pub(crate) fn del_port_mapping_entry( +pub(crate) fn clear_port_mapping_entry( s: &Switch, asic_port_id: u16, ) -> DpdResult<()> { @@ -239,7 +239,7 @@ pub(crate) fn del_port_mapping_entry( debug!(s.log, "delete port id entry {match_key} -> {asic_port_id}"); - s.table_entry_del(TableType::McastEgressPortMapping, &match_key) + s.table_entry_clear(TableType::McastEgressPortMapping, &match_key) } /// Dump the multicast port mapping table. diff --git a/dpd/src/table/mod.rs b/dpd/src/table/mod.rs index 5a98e7fc..d2f0a052 100644 --- a/dpd/src/table/mod.rs +++ b/dpd/src/table/mod.rs @@ -170,7 +170,7 @@ impl Table { e.into() }) .map(|()| { - self.usage.occupancy -= 1; + self.usage.occupancy = self.usage.occupancy.saturating_sub(1); self.usage.deletes += 1; }) } diff --git a/dpd/src/table/port_ip.rs b/dpd/src/table/port_ip.rs index 322493a5..b1d89f55 100644 --- a/dpd/src/table/port_ip.rs +++ b/dpd/src/table/port_ip.rs @@ -91,12 +91,12 @@ fn match_keys_ipv6(ipv6: Ipv6Addr, port: u16) -> (Ipv6MatchKey, Ipv6MatchKey) { (claim_key, drop_key) } -pub fn loopback_ipv4_add(s: &Switch, ipv4: Ipv4Addr) -> DpdResult<()> { +pub fn loopback_ipv4_set(s: &Switch, ipv4: Ipv4Addr) -> DpdResult<()> { let claim_key = Ipv4MatchKey { dst_addr: ipv4, in_port: MatchMask { val: 0u16.into(), mask: 0u16.into() }, }; - s.table_entry_add(TableType::PortAddrIpv4, &claim_key, &ActionV4::ClaimIpv4) + s.table_entry_set(TableType::PortAddrIpv4, &claim_key, &ActionV4::ClaimIpv4) .map(|_| info!(s.log, "added ipv4 loopback"; "addr" => %ipv4)) .inspect_err(|e| { error!(s.log, "failed to add ipv4 loopback"; @@ -105,12 +105,12 @@ pub fn loopback_ipv4_add(s: &Switch, ipv4: Ipv4Addr) -> DpdResult<()> { }) } -pub fn loopback_ipv4_delete(s: &Switch, ipv4: Ipv4Addr) -> DpdResult<()> { +pub fn loopback_ipv4_clear(s: &Switch, ipv4: Ipv4Addr) -> DpdResult<()> { let claim_key = Ipv4MatchKey { dst_addr: ipv4, in_port: MatchMask { val: 0u16.into(), mask: 0u16.into() }, }; - s.table_entry_del(TableType::PortAddrIpv4, &claim_key) + s.table_entry_clear(TableType::PortAddrIpv4, &claim_key) .map(|_| info!(s.log, "deleted ipv4 loopback"; "addr" => %ipv4)) .inspect_err(|e| { error!(s.log, "failed to delete ipv4 loopback"; @@ -119,12 +119,12 @@ pub fn loopback_ipv4_delete(s: &Switch, ipv4: Ipv4Addr) -> DpdResult<()> { }) } -pub fn loopback_ipv6_add(s: &Switch, ipv6: Ipv6Addr) -> DpdResult<()> { +pub fn loopback_ipv6_set(s: &Switch, ipv6: Ipv6Addr) -> DpdResult<()> { let claim_key = Ipv6MatchKey { dst_addr: ipv6, in_port: MatchMask { val: 0u16.into(), mask: 0u16.into() }, }; - s.table_entry_add(TableType::PortAddrIpv6, &claim_key, &ActionV6::ClaimIpv6) + s.table_entry_set(TableType::PortAddrIpv6, &claim_key, &ActionV6::ClaimIpv6) .map(|_| info!(s.log, "added ipv6 loopback"; "addr" => %ipv6)) .inspect_err(|e| { error!(s.log, "failed to add ipv6 loopback"; @@ -133,12 +133,12 @@ pub fn loopback_ipv6_add(s: &Switch, ipv6: Ipv6Addr) -> DpdResult<()> { }) } -pub fn loopback_ipv6_delete(s: &Switch, ipv6: Ipv6Addr) -> DpdResult<()> { +pub fn loopback_ipv6_clear(s: &Switch, ipv6: Ipv6Addr) -> DpdResult<()> { let claim_key = Ipv6MatchKey { dst_addr: ipv6, in_port: MatchMask { val: 0u16.into(), mask: 0u16.into() }, }; - s.table_entry_del(TableType::PortAddrIpv6, &claim_key) + s.table_entry_clear(TableType::PortAddrIpv6, &claim_key) .map(|_| info!(s.log, "deleted ipv6 loopback"; "addr" => %ipv6)) .inspect_err(|e| { error!(s.log, "failed to delete ipv6 loopback"; @@ -165,27 +165,30 @@ fn endeavour_to_repair( panic!("Repeated repair attempts failed. Giving up."); } -fn ipv4_add_work(s: &Switch, port: u16, ipv4: Ipv4Addr) -> DpdResult<()> { +fn ipv4_set_work(s: &Switch, port: u16, ipv4: Ipv4Addr) -> DpdResult<()> { let (claim_key, drop_key) = match_keys_ipv4(ipv4, port); - s.table_entry_add( + s.table_entry_set( TableType::PortAddrIpv4, &claim_key, &ActionV4::ClaimIpv4, )?; - s.table_entry_add(TableType::PortAddrIpv4, &drop_key, &ActionV4::DropIpv4) + s.table_entry_set(TableType::PortAddrIpv4, &drop_key, &ActionV4::DropIpv4) .inspect_err(|_| { endeavour_to_repair( s, format!("ipv4 address {ipv4} only half added"), - || s.table_entry_del(TableType::PortAddrIpv4, &claim_key), + || s.table_entry_clear(TableType::PortAddrIpv4, &claim_key), ); }) } -/// Add one IPv4 address to the ASIC tables. -pub fn ipv4_add(s: &Switch, port: u16, ipv4: Ipv4Addr) -> DpdResult<()> { - ipv4_add_work(s, port, ipv4) +/// Places this IPv4 address in the appropriate ASIC tables. +/// +/// If the address does not yet exist, an entry is created. +/// If it does, the entry is updated. +pub fn ipv4_set(s: &Switch, port: u16, ipv4: Ipv4Addr) -> DpdResult<()> { + ipv4_set_work(s, port, ipv4) .map(|_| { info!(s.log, "added ipv4 address"; "addr" => %ipv4, @@ -199,16 +202,16 @@ pub fn ipv4_add(s: &Switch, port: u16, ipv4: Ipv4Addr) -> DpdResult<()> { }) } -fn ipv4_delete_work(s: &Switch, port: u16, ipv4: Ipv4Addr) -> DpdResult<()> { +fn ipv4_clear_work(s: &Switch, port: u16, ipv4: Ipv4Addr) -> DpdResult<()> { let (claim_key, drop_key) = match_keys_ipv4(ipv4, port); - s.table_entry_del(TableType::PortAddrIpv4, &claim_key)?; - s.table_entry_del(TableType::PortAddrIpv4, &drop_key).inspect_err(|_| { + s.table_entry_clear(TableType::PortAddrIpv4, &claim_key)?; + s.table_entry_clear(TableType::PortAddrIpv4, &drop_key).inspect_err(|_| { endeavour_to_repair( s, format!("ipv4 address {ipv4} only half deleted"), || { - s.table_entry_add( + s.table_entry_set( TableType::PortAddrIpv4, &claim_key, &ActionV4::ClaimIpv4, @@ -219,8 +222,9 @@ fn ipv4_delete_work(s: &Switch, port: u16, ipv4: Ipv4Addr) -> DpdResult<()> { } /// Delete one IPv4 address from the ASIC tables. -pub fn ipv4_delete(s: &Switch, port: u16, ipv4: Ipv4Addr) -> DpdResult<()> { - ipv4_delete_work(s, port, ipv4) +/// Returns Ok if the address is already not present. +pub fn ipv4_clear(s: &Switch, port: u16, ipv4: Ipv4Addr) -> DpdResult<()> { + ipv4_clear_work(s, port, ipv4) .map(|_| { info!(s.log, "deleted ipv4 address"; "addr" => %ipv4, @@ -234,39 +238,27 @@ pub fn ipv4_delete(s: &Switch, port: u16, ipv4: Ipv4Addr) -> DpdResult<()> { }) } -/// Delete many IPv4 address from the ASIC tables. -pub fn ipv4_delete_many<'a>( - s: &'a Switch, - port: u16, - addrs: impl Iterator + 'a, -) -> DpdResult<()> { - for addr in addrs { - let _ = ipv4_delete(s, port, addr); - } - Ok(()) -} - -fn ipv6_add_work(s: &Switch, port: u16, ipv6: Ipv6Addr) -> DpdResult<()> { +fn ipv6_set_work(s: &Switch, port: u16, ipv6: Ipv6Addr) -> DpdResult<()> { let (claim_key, drop_key) = match_keys_ipv6(ipv6, port); - s.table_entry_add( + s.table_entry_set( TableType::PortAddrIpv6, &claim_key, &ActionV6::ClaimIpv6, )?; - s.table_entry_add(TableType::PortAddrIpv6, &drop_key, &ActionV6::DropIpv6) + s.table_entry_set(TableType::PortAddrIpv6, &drop_key, &ActionV6::DropIpv6) .inspect_err(|_| { endeavour_to_repair( s, format!("ipv6 address {ipv6} only half added"), - || s.table_entry_del(TableType::PortAddrIpv6, &claim_key), + || s.table_entry_clear(TableType::PortAddrIpv6, &claim_key), ); }) } -/// Add one IPv6 address to the ASIC tables. -pub fn ipv6_add(s: &Switch, port: u16, ipv6: Ipv6Addr) -> DpdResult<()> { - ipv6_add_work(s, port, ipv6) +/// IPv6 equivalent of [`ipv4_set`]. +pub fn ipv6_set(s: &Switch, port: u16, ipv6: Ipv6Addr) -> DpdResult<()> { + ipv6_set_work(s, port, ipv6) .map(|_| { info!(s.log, "added ipv6 address"; "addr" => %ipv6, @@ -280,16 +272,16 @@ pub fn ipv6_add(s: &Switch, port: u16, ipv6: Ipv6Addr) -> DpdResult<()> { }) } -fn ipv6_delete_work(s: &Switch, port: u16, ipv6: Ipv6Addr) -> DpdResult<()> { +fn ipv6_clear_work(s: &Switch, port: u16, ipv6: Ipv6Addr) -> DpdResult<()> { let (claim_key, drop_key) = match_keys_ipv6(ipv6, port); - s.table_entry_del(TableType::PortAddrIpv6, &claim_key)?; - s.table_entry_del(TableType::PortAddrIpv6, &drop_key).inspect_err(|_| { + s.table_entry_clear(TableType::PortAddrIpv6, &claim_key)?; + s.table_entry_clear(TableType::PortAddrIpv6, &drop_key).inspect_err(|_| { endeavour_to_repair( s, format!("ipv6 address {ipv6} only half deleted"), || { - s.table_entry_add( + s.table_entry_set( TableType::PortAddrIpv6, &claim_key, &ActionV6::ClaimIpv6, @@ -300,8 +292,8 @@ fn ipv6_delete_work(s: &Switch, port: u16, ipv6: Ipv6Addr) -> DpdResult<()> { } /// Delete one IPv6 address from the ASIC tables. -pub fn ipv6_delete(s: &Switch, port: u16, ipv6: Ipv6Addr) -> DpdResult<()> { - ipv6_delete_work(s, port, ipv6) +pub fn ipv6_clear(s: &Switch, port: u16, ipv6: Ipv6Addr) -> DpdResult<()> { + ipv6_clear_work(s, port, ipv6) .map(|_| { info!(s.log, "deleted ipv6 address"; "addr" => %ipv6, @@ -356,15 +348,3 @@ pub fn ipv6_counter_fetch( ) -> DpdResult> { s.counter_fetch::(force_sync, TableType::PortAddrIpv6) } - -/// Delete many IPv6 address from the ASIC tables. -pub fn ipv6_delete_many<'a>( - s: &'a Switch, - port: u16, - addrs: impl Iterator + 'a, -) -> DpdResult<()> { - for addr in addrs { - let _ = ipv6_delete(s, port, addr); - } - Ok(()) -} diff --git a/dpd/src/table/uplink.rs b/dpd/src/table/uplink.rs index d8f04abd..818c70ff 100644 --- a/dpd/src/table/uplink.rs +++ b/dpd/src/table/uplink.rs @@ -42,7 +42,7 @@ fn set_ingress_uplink(s: &Switch, port: u16) -> DpdResult<()> { let match_key = IngressMatchKey { in_port: port }; let action_data = IngressAction::UplinkPort; - match s.table_entry_add(TableType::UplinkIngress, &match_key, &action_data) + match s.table_entry_set(TableType::UplinkIngress, &match_key, &action_data) { Ok(_) => { info!(s.log, "set uplink on {}", port); @@ -58,7 +58,7 @@ fn set_ingress_uplink(s: &Switch, port: u16) -> DpdResult<()> { fn clear_ingress_uplink(s: &Switch, port: u16) -> DpdResult<()> { let match_key = IngressMatchKey { in_port: port }; - match s.table_entry_del(TableType::UplinkIngress, &match_key) { + match s.table_entry_clear(TableType::UplinkIngress, &match_key) { Ok(_) => { info!(s.log, "cleared uplink on {}", port); Ok(()) @@ -77,7 +77,7 @@ pub fn uplink_set(s: &Switch, port: u16) -> DpdResult<()> { let match_key = EgressMatchKey { out_port: port }; let action_data = EgressAction::Allowed; - match s.table_entry_add(TableType::UplinkEgress, &match_key, &action_data) { + match s.table_entry_set(TableType::UplinkEgress, &match_key, &action_data) { Ok(_) => { info!(s.log, "set guest_traffic_allowed on {}", port); Ok(()) @@ -94,11 +94,12 @@ pub fn uplink_set(s: &Switch, port: u16) -> DpdResult<()> { } /// Remove an entry from the uplink tables. +/// Returns Ok if the entry was not found. pub fn uplink_clear(s: &Switch, port: u16) -> DpdResult<()> { clear_ingress_uplink(s, port)?; let match_key = EgressMatchKey { out_port: port }; - match s.table_entry_del(TableType::UplinkEgress, &match_key) { + match s.table_entry_clear(TableType::UplinkEgress, &match_key) { Ok(_) => { info!(s.log, "cleared guest_traffic_allowed on {}", port); Ok(()) diff --git a/dpd/src/types.rs b/dpd/src/types.rs index 2626b8d7..f390ff31 100644 --- a/dpd/src/types.rs +++ b/dpd/src/types.rs @@ -88,6 +88,13 @@ pub enum DpdError { ResourceExhausted(String), #[error("Tag is required for idempotent validation")] MissingTag, + #[error("Address {addr} is already in use by {interface}")] + AddrIfaceConflict { + addr: IpAddr, + interface: std::borrow::Cow<'static, str>, + }, + #[error("These IP addresses could not be deleted: {0:?}")] + AddrClear(Vec<(crate::addr::AsicAddrIface, IpAddr, DpdError)>), } impl From for DpdError { @@ -287,6 +294,32 @@ impl convert::From for dropshot::HttpError { e @ DpdError::MissingTag => { dropshot::HttpError::for_bad_request(None, format!("{e}")) } + e @ DpdError::AddrIfaceConflict { .. } => dropshot::HttpError { + status_code: dropshot::ErrorStatusCode::CONFLICT, + error_code: None, + internal_message: e.to_string(), + external_message: e.to_string(), + headers: None, + }, + e @ DpdError::AddrClear(_) => { + let msg = e.to_string(); + let DpdError::AddrClear(errs) = e else { + unreachable!(); + }; + errs.into_iter() + .next() + .map(|(_, _, e)| dropshot::HttpError { + internal_message: msg.clone(), + external_message: msg, + ..dropshot::HttpError::from(e) + }) + .unwrap_or_else(|| { + dropshot::HttpError::for_internal_error( + "AddrClear error group contained no errors" + .to_string(), + ) + }) + } } } } @@ -331,3 +364,11 @@ impl convert::From for DpdError { } } } + +impl convert::From for DpdError { + fn from(err: common::addr::RouterIfaceIpAddrErr) -> Self { + DpdError::Invalid(format!( + "Conversion failed to RouterIfaceIpAddrErr: {err:?}" + )) + } +}