From 4cc622209eb0d9d705511794edc3a13187f13643 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Fri, 11 Sep 2026 08:30:51 +0000 Subject: [PATCH 01/29] a sketch of dynamic external ddm peers --- ddm-api-types/src/external_peers.rs | 5 + ddm-api-types/src/lib.rs | 1 + .../src/external_peers/external_peers.rs | 12 +++ .../versions/src/external_peers/mod.rs | 5 + ddm-api-types/versions/src/latest.rs | 4 + ddm-api-types/versions/src/lib.rs | 2 + ddm-api/src/lib.rs | 11 ++ ddm/src/admin.rs | 95 +++++++++++++++- ddm/src/db.rs | 11 +- ddm/src/defaults.rs | 19 ++++ ddm/src/lib.rs | 1 + ddm/src/sm/mod.rs | 28 +++-- ddm/src/sm/state.rs | 101 ++++++++++++------ ddmd/src/main.rs | 26 +++-- .../ddm-admin-2.0.0-45d40c.json.gitstub | 1 + ...5d40c.json => ddm-admin-3.0.0-2f4dd6.json} | 43 +++++++- openapi/ddm-admin/ddm-admin-latest.json | 2 +- 17 files changed, 315 insertions(+), 52 deletions(-) create mode 100644 ddm-api-types/src/external_peers.rs create mode 100644 ddm-api-types/versions/src/external_peers/external_peers.rs create mode 100644 ddm-api-types/versions/src/external_peers/mod.rs create mode 100644 ddm/src/defaults.rs create mode 100644 openapi/ddm-admin/ddm-admin-2.0.0-45d40c.json.gitstub rename openapi/ddm-admin/{ddm-admin-2.0.0-45d40c.json => ddm-admin-3.0.0-2f4dd6.json} (94%) diff --git a/ddm-api-types/src/external_peers.rs b/ddm-api-types/src/external_peers.rs new file mode 100644 index 000000000..24a75854f --- /dev/null +++ b/ddm-api-types/src/external_peers.rs @@ -0,0 +1,5 @@ +// 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/. + +pub use ddm_api_types_versions::latest::external_peers::*; diff --git a/ddm-api-types/src/lib.rs b/ddm-api-types/src/lib.rs index 2c22d1240..c041b73d6 100644 --- a/ddm-api-types/src/lib.rs +++ b/ddm-api-types/src/lib.rs @@ -19,4 +19,5 @@ pub mod admin; pub mod db; pub mod exchange; +pub mod external_peers; pub mod net; diff --git a/ddm-api-types/versions/src/external_peers/external_peers.rs b/ddm-api-types/versions/src/external_peers/external_peers.rs new file mode 100644 index 000000000..f974d57e5 --- /dev/null +++ b/ddm-api-types/versions/src/external_peers/external_peers.rs @@ -0,0 +1,12 @@ +// 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/. + +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use std::collections::BTreeSet; + +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] +pub struct SetExternalPeers { + pub interfaces: BTreeSet, +} diff --git a/ddm-api-types/versions/src/external_peers/mod.rs b/ddm-api-types/versions/src/external_peers/mod.rs new file mode 100644 index 000000000..74788e4b1 --- /dev/null +++ b/ddm-api-types/versions/src/external_peers/mod.rs @@ -0,0 +1,5 @@ +// 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/. + +pub mod external_peers; diff --git a/ddm-api-types/versions/src/latest.rs b/ddm-api-types/versions/src/latest.rs index a2c1b47f0..b9866183f 100644 --- a/ddm-api-types/versions/src/latest.rs +++ b/ddm-api-types/versions/src/latest.rs @@ -25,3 +25,7 @@ pub mod exchange { pub mod net { pub use crate::v1::net::TunnelOrigin; } + +pub mod external_peers { + pub use crate::v3::external_peers::SetExternalPeers; +} diff --git a/ddm-api-types/versions/src/lib.rs b/ddm-api-types/versions/src/lib.rs index 7d88b9274..34abae1eb 100644 --- a/ddm-api-types/versions/src/lib.rs +++ b/ddm-api-types/versions/src/lib.rs @@ -34,3 +34,5 @@ pub mod latest; pub mod v1; #[path = "peer_durations/mod.rs"] pub mod v2; +#[path = "external_peers/mod.rs"] +pub mod v3; diff --git a/ddm-api/src/lib.rs b/ddm-api/src/lib.rs index 623b8376b..3437d3129 100644 --- a/ddm-api/src/lib.rs +++ b/ddm-api/src/lib.rs @@ -26,6 +26,7 @@ api_versions!([ // | example for the next person. // v // (next_int, IDENT), + (3, EXTERNAL_PEERS), (2, PEER_DURATIONS), (1, INITIAL), ]); @@ -170,4 +171,14 @@ pub trait DdmAdminApi { async fn disable_stats( ctx: RequestContext, ) -> Result; + + #[endpoint { + method = POST, + path = "/external_peers", + versions = VERSION_EXTERNAL_PEERS.., + }] + async fn set_external_peers( + ctx: RequestContext, + request: TypedBody, + ) -> Result; } diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index 21d373f4d..eec6f165a 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -3,13 +3,21 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use crate::db::Db; -use crate::sm::{AdminEvent, Event, PrefixSet, SmContext}; +use crate::defaults::{ + DISCOVERY_READ_TIMEOUT, EXCHANGE_TCP_PORT, EXCHANGE_TIMEOUT, + EXPIRE_THRESHOLD, IP_ADDR_WAIT, SOLICIT_INTERVAL, millis_u64, +}; +use crate::sm::{ + AdminEvent, Event, InterfaceState, PrefixSet, SessionStats, SmContext, + StateMachine, +}; use camino::Utf8PathBuf; use ddm_api::DdmAdminApi; use ddm_api::ddm_admin_api_mod; use ddm_api_types::admin::{EnableStatsRequest, ExpirePathParams, PrefixMap}; -use ddm_api_types::db::{PeerInfo, TunnelRoute}; +use ddm_api_types::db::{PeerInfo, RouterKind, TunnelRoute}; use ddm_api_types::exchange::PathVector; +use ddm_api_types::external_peers::SetExternalPeers; use ddm_api_types::net::TunnelOrigin; use dropshot::ApiDescription; use dropshot::ApiDescriptionBuildErrors; @@ -27,11 +35,11 @@ use oxnet::Ipv6Net; use slog::{Logger, error, info, o}; use slog_error_chain::InlineErrorChain; use std::collections::{HashMap, HashSet}; -use std::net::{IpAddr, SocketAddr, SocketAddrV4, SocketAddrV6}; +use std::net::{IpAddr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6}; use std::sync::Arc; use std::sync::Mutex; use std::sync::atomic::{AtomicU64, Ordering}; -use std::sync::mpsc::Sender; +use std::sync::mpsc::{Sender, channel}; use tokio::spawn; use tokio::task::JoinHandle; @@ -429,6 +437,85 @@ impl DdmAdminApi for DdmAdminApiImpl { Ok(HttpResponseUpdatedNoContent()) } + + async fn set_external_peers( + ctx: RequestContext, + request: TypedBody, + ) -> Result { + let mut ctx = lock!(ctx.context()); + let rq = request.into_inner(); + + let current = ctx.db.get_external_peers(); + let to_create = rq.interfaces.difference(¤t); + let to_remove = current.difference(&rq.interfaces); + + for ifx in to_create.into_iter() { + let (tx, rx) = channel(); + + let config = crate::sm::Config { + solicit_interval: millis_u64(SOLICIT_INTERVAL), + expire_threshold: millis_u64(EXPIRE_THRESHOLD), + discovery_read_timeout: millis_u64(DISCOVERY_READ_TIMEOUT), + ip_addr_wait: millis_u64(IP_ADDR_WAIT), + exchange_timeout: millis_u64(EXCHANGE_TIMEOUT), + exchange_port: EXCHANGE_TCP_PORT, + aobj_name: format!("{ifx}/ll"), + if_name: String::default(), // initialized in state machine + if_index: 0, // initialized in state machine + // External peers are only a thing for transit routers. + kind: RouterKind::Transit, + dpd: Some(crate::sm::DpdConfig { + // Transit DDM routers always talk to their local dpd in the + // switch zone. + host: String::from("localhost"), + // TODO: using the default dpd port might not be right for some + // test environments. + port: dpd_client::default_port(), + }), + addr: Ipv6Addr::UNSPECIFIED, + }; + let sm_ctx = SmContext { + config, + db: ctx.db.clone(), + event_channels: ctx.event_channels.clone(), + tx: tx.clone(), + log: ctx.log.clone(), + hostname: hostname::get() + .expect("failed to get hostname") + .to_string_lossy() + .to_string(), + rt: Arc::new(tokio::runtime::Handle::current()), + iface: Arc::new(InterfaceState::external()), + stats: Arc::new(SessionStats::default()), + }; + let mut sm = StateMachine { + ctx: sm_ctx.clone(), + rx: Some(rx), + }; + + sm.run().unwrap(); + + ctx.peers.push(sm_ctx.clone()); + + crate::sm::state::send( + Event::Admin(AdminEvent::NewExternalPeer(tx.clone())), + &mut ctx.event_channels, + ); + ctx.event_channels.push(tx); + + // TODO oxstats server + } + + for ifx in to_remove.into_iter() { + for p in &ctx.peers { + if &p.config.if_name == ifx { + let _ = p.tx.send(Event::Admin(AdminEvent::Shutdown)); + } + } + } + + Ok(HttpResponseUpdatedNoContent()) + } } pub fn api_description() diff --git a/ddm/src/db.rs b/ddm/src/db.rs index d72fd7b20..deab0ec07 100644 --- a/ddm/src/db.rs +++ b/ddm/src/db.rs @@ -9,7 +9,7 @@ use oxnet::{IpNet, Ipv6Net}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use slog::{Logger, error}; -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeSet, HashMap, HashSet}; use std::net::Ipv6Addr; use std::sync::{Arc, Mutex}; @@ -47,6 +47,7 @@ pub struct Db { pub struct DbData { pub imported: HashSet, pub imported_tunnel: HashSet, + pub external_peers: BTreeSet, } const _: () = { @@ -261,6 +262,14 @@ impl Db { } result } + + pub fn get_external_peers(&self) -> BTreeSet { + lock!(self.data).external_peers.clone() + } + + pub fn set_external_peers(&mut self, value: BTreeSet) { + lock!(self.data).external_peers = value; + } } #[derive( diff --git a/ddm/src/defaults.rs b/ddm/src/defaults.rs new file mode 100644 index 000000000..ad1f8ba84 --- /dev/null +++ b/ddm/src/defaults.rs @@ -0,0 +1,19 @@ +// 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/. + +use std::time::Duration; + +pub const SOLICIT_INTERVAL: Duration = Duration::from_millis(2000); +pub const EXPIRE_THRESHOLD: Duration = Duration::from_millis(5000); +pub const DISCOVERY_READ_TIMEOUT: Duration = Duration::from_millis(1000); +pub const IP_ADDR_WAIT: Duration = Duration::from_millis(1000); +pub const EXCHANGE_TIMEOUT: Duration = Duration::from_millis(3000); + +pub const EXCHANGE_TCP_PORT: u16 = 0xdddd; + +pub const fn millis_u64(d: Duration) -> u64 { + let x = d.as_millis(); + assert!(x <= u64::MAX as u128); + x as u64 +} diff --git a/ddm/src/lib.rs b/ddm/src/lib.rs index 6a5e2a68a..19326ec0a 100644 --- a/ddm/src/lib.rs +++ b/ddm/src/lib.rs @@ -4,6 +4,7 @@ pub mod admin; pub mod db; +pub mod defaults; pub mod discovery; pub mod exchange; pub mod oxstats; diff --git a/ddm/src/sm/mod.rs b/ddm/src/sm/mod.rs index 4a52efb55..1ec50d5eb 100644 --- a/ddm/src/sm/mod.rs +++ b/ddm/src/sm/mod.rs @@ -23,9 +23,9 @@ use std::time::{Duration, Instant}; use thiserror::Error; #[cfg(all(feature = "backend", target_os = "illumos"))] -mod state; +pub(crate) mod state; -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum AdminEvent { /// Announce a set of IPv6 prefixes Announce(PrefixSet), @@ -38,27 +38,34 @@ pub enum AdminEvent { /// Synchronize with active peers by pulling their prefixes. Sync, + + /// A new external peer has been added to the router that can be reached + /// using the provided sender. + NewExternalPeer(Sender), + + /// Shutdown on recipt of this event. + Shutdown, } -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum PrefixSet { Underlay(HashSet), Tunnel(HashSet), } -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum PeerEvent { Push(ddm_protocol::v3::Update), } -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum NeighborEvent { Advertise((Ipv6Addr, Version)), SolicitFail, Expire, } -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum Event { Neighbor(NeighborEvent), Peer(PeerEvent), @@ -184,12 +191,20 @@ pub struct PeerIdentity { pub struct InterfaceState { pub if_index: Mutex, pub if_name: Mutex, + pub external: bool, pub fsm_state: Mutex, pub last_fsm_state_change: Mutex, pub peer_identity: Mutex>, } impl InterfaceState { + pub fn external() -> Self { + Self { + external: true, + ..Default::default() + } + } + pub fn transition(&self, state: FsmState) { *lock!(self.fsm_state) = state; *lock!(self.last_fsm_state_change) = Instant::now(); @@ -215,6 +230,7 @@ impl Default for InterfaceState { Self { if_index: Mutex::new(0), if_name: Mutex::new(String::new()), + external: false, fsm_state: Mutex::new(FsmState::Init), last_fsm_state_change: Mutex::new(Instant::now()), peer_identity: Mutex::new(None), diff --git a/ddm/src/sm/state.rs b/ddm/src/sm/state.rs index ecb8ffa68..b73e0af8b 100644 --- a/ddm/src/sm/state.rs +++ b/ddm/src/sm/state.rs @@ -20,7 +20,7 @@ use std::collections::HashSet; use std::net::IpAddr; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::mpsc::Receiver; +use std::sync::mpsc::{Receiver, Sender}; use std::thread::{sleep, spawn}; use std::time::Duration; @@ -34,10 +34,13 @@ impl StateMachine { let mut rx = self.rx.take().unwrap(); let log = self.ctx.log.clone(); spawn(move || { - let mut state: Box = - Box::new(Init::new(ctx.clone(), log.clone())); + let mut state: Option> = + Some(Box::new(Init::new(ctx.clone(), log.clone()))); loop { - (state, rx) = state.run(rx); + (state, rx) = match &mut state { + Some(st) => st.run(rx), + None => break, + } } }); @@ -49,7 +52,7 @@ trait State { fn run( &mut self, event: Receiver, - ) -> (Box, Receiver); + ) -> (Option>, Receiver); } struct Init { @@ -63,11 +66,23 @@ impl Init { } } +pub(crate) fn send(e: Event, event_channels: &mut Vec>) { + let mut dead_channels = Vec::default(); + for (i, c) in event_channels.iter().enumerate() { + if c.send(e.clone()).is_err() { + dead_channels.push(i); + } + } + for i in dead_channels { + event_channels.remove(i); + } +} + impl State for Init { fn run( &mut self, event: Receiver, - ) -> (Box, Receiver) { + ) -> (Option>, Receiver) { self.ctx.iface.transition(FsmState::Init); self.ctx.iface.clear_peer(); loop { @@ -125,7 +140,10 @@ impl State for Init { ) .unwrap(); // TODO unwrap return ( - Box::new(Solicit::new(self.ctx.clone(), self.log.clone())), + Some(Box::new(Solicit::new( + self.ctx.clone(), + self.log.clone(), + ))), event, ); } @@ -147,7 +165,7 @@ impl State for Solicit { fn run( &mut self, event: Receiver, - ) -> (Box, Receiver) { + ) -> (Option>, Receiver) { self.ctx.iface.transition(FsmState::Solicit); loop { let e = match event.recv() { @@ -170,12 +188,12 @@ impl State for Solicit { "transition solicit -> exchange" ); return ( - Box::new(Exchange::new( + Some(Box::new(Exchange::new( self.ctx.clone(), addr, version, self.log.clone(), - )), + ))), event, ); } @@ -187,7 +205,10 @@ impl State for Solicit { "exiting solicit state due to failed solicit", ); return ( - Box::new(Init::new(self.ctx.clone(), self.log.clone())), + Some(Box::new(Init::new( + self.ctx.clone(), + self.log.clone(), + ))), event, ); } @@ -199,6 +220,12 @@ impl State for Solicit { e ); } + Event::Admin(AdminEvent::NewExternalPeer(tx)) => { + self.ctx.event_channels.push(tx); + } + Event::Admin(AdminEvent::Shutdown) => { + return (None, event); + } Event::Admin(e) => { wrn!( self.log, @@ -362,9 +389,10 @@ impl Exchange { }; let push = Update { underlay, tunnel }; - for ec in &self.ctx.event_channels { - ec.send(Event::Peer(PeerEvent::Push(push.clone()))).unwrap(); - } + send( + Event::Peer(PeerEvent::Push(push.clone())), + &mut self.ctx.event_channels, + ); } pull_stop.store(true, Ordering::Relaxed); } @@ -374,7 +402,7 @@ impl State for Exchange { fn run( &mut self, event: Receiver, - ) -> (Box, Receiver) { + ) -> (Option>, Receiver) { self.ctx.iface.transition(FsmState::Exchange); let exchange_thread = loop { match exchange::handler( @@ -453,10 +481,10 @@ impl State for Exchange { ); self.expire_peer(&exchange_thread, &pull_stop); return ( - Box::new(Solicit::new( + Some(Box::new(Solicit::new( self.ctx.clone(), self.log.clone(), - )), + ))), event, ); } @@ -488,10 +516,10 @@ impl State for Exchange { ); self.expire_peer(&exchange_thread, &pull_stop); return ( - Box::new(Solicit::new( + Some(Box::new(Solicit::new( self.ctx.clone(), self.log.clone(), - )), + ))), event, ); } @@ -529,10 +557,10 @@ impl State for Exchange { ); self.expire_peer(&exchange_thread, &pull_stop); return ( - Box::new(Solicit::new( + Some(Box::new(Solicit::new( self.ctx.clone(), self.log.clone(), - )), + ))), event, ); } @@ -564,10 +592,10 @@ impl State for Exchange { ); self.expire_peer(&exchange_thread, &pull_stop); return ( - Box::new(Solicit::new( + Some(Box::new(Solicit::new( self.ctx.clone(), self.log.clone(), - )), + ))), event, ); } @@ -582,10 +610,10 @@ impl State for Exchange { ); self.expire_peer(&exchange_thread, &pull_stop); return ( - Box::new(Solicit::new( + Some(Box::new(Solicit::new( self.ctx.clone(), self.log.clone(), - )), + ))), event, ); } @@ -606,6 +634,12 @@ impl State for Exchange { ); } } + Event::Admin(AdminEvent::NewExternalPeer(tx)) => { + self.ctx.event_channels.push(tx); + } + Event::Admin(AdminEvent::Shutdown) => { + return (None, event); + } Event::Peer(PeerEvent::Push(update)) => { inf!( self.log, @@ -640,10 +674,10 @@ impl State for Exchange { ); self.expire_peer(&exchange_thread, &pull_stop); return ( - Box::new(Solicit::new( + Some(Box::new(Solicit::new( self.ctx.clone(), self.log.clone(), - )), + ))), event, ); } @@ -672,10 +706,10 @@ impl State for Exchange { ); self.expire_peer(&exchange_thread, &pull_stop); return ( - Box::new(Solicit::new( + Some(Box::new(Solicit::new( self.ctx.clone(), self.log.clone(), - )), + ))), event, ); } @@ -690,10 +724,10 @@ impl State for Exchange { ); self.expire_peer(&exchange_thread, &pull_stop); return ( - Box::new(Solicit::new( + Some(Box::new(Solicit::new( self.ctx.clone(), self.log.clone(), - )), + ))), event, ); } @@ -706,7 +740,10 @@ impl State for Exchange { ); self.expire_peer(&exchange_thread, &pull_stop); return ( - Box::new(Init::new(self.ctx.clone(), self.log.clone())), + Some(Box::new(Init::new( + self.ctx.clone(), + self.log.clone(), + ))), event, ); } diff --git a/ddmd/src/main.rs b/ddmd/src/main.rs index 671e5cdd1..052122e67 100644 --- a/ddmd/src/main.rs +++ b/ddmd/src/main.rs @@ -6,6 +6,10 @@ use camino::Utf8PathBuf; use clap::Parser; use ddm::admin::{HandlerContext, RouterStats}; use ddm::db::Db; +use ddm::defaults::{ + DISCOVERY_READ_TIMEOUT, EXCHANGE_TCP_PORT, EXCHANGE_TIMEOUT, + EXPIRE_THRESHOLD, IP_ADDR_WAIT, SOLICIT_INTERVAL, millis_u64, +}; #[cfg(all(feature = "backend", target_os = "illumos"))] use ddm::sm::{DpdConfig, InterfaceState, SmContext, StateMachine}; #[cfg(not(all(feature = "backend", target_os = "illumos")))] @@ -24,6 +28,14 @@ use uuid::Uuid; mod signal; mod smf; +// macro_rules! u64_millis { +// ($x:expr) => { +// $x.as_millis() +// .try_into() +// .expect(&format!("{} as u64", stringify!($x))) +// }; +// } + #[derive(Debug, Parser)] #[command(version, about, long_about = None, styles = get_styles())] struct Arg { @@ -32,26 +44,26 @@ struct Arg { addresses: Vec, /// How long to wait between solicitations (milliseconds). - #[arg(long, default_value_t = 2000)] + #[arg(long, default_value_t = millis_u64(SOLICIT_INTERVAL))] solicit_interval: u64, /// How long to wait without a solicitation response before expiring a peer /// (milliseconds). - #[arg(long, default_value_t = 5000)] + #[arg(long, default_value_t = millis_u64(EXPIRE_THRESHOLD))] expire_threshold: u64, /// How often to check for link failure while waiting for discovery messges /// (milliseconds). - #[arg(long, default_value_t = 1000)] + #[arg(long, default_value_t = millis_u64(DISCOVERY_READ_TIMEOUT))] discovery_read_timeout: u64, /// How long to wait between attempts to get an IP address for a specified /// address object (milliseconds). - #[arg(long, default_value_t = 1000)] + #[arg(long, default_value_t = millis_u64(IP_ADDR_WAIT))] ip_addr_wait: u64, - /// How long to wait for a response to exchange messages. - #[arg(long, default_value_t = 3000)] + /// How long to wait for a response to exchange messages (milliseconds). + #[arg(long, default_value_t = millis_u64(EXCHANGE_TIMEOUT))] pub exchange_timeout: u64, /// Address to listen on for the admin API. @@ -67,7 +79,7 @@ struct Arg { kind: RouterKind, /// The tcp port to listen on for exchange messages. - #[arg(long, default_value_t = 0xdddd)] + #[arg(long, default_value_t = EXCHANGE_TCP_PORT)] exchange_port: u16, /// Whether or not to use Dendrite as the underlying routing and forwarding diff --git a/openapi/ddm-admin/ddm-admin-2.0.0-45d40c.json.gitstub b/openapi/ddm-admin/ddm-admin-2.0.0-45d40c.json.gitstub new file mode 100644 index 000000000..b5ab91ce1 --- /dev/null +++ b/openapi/ddm-admin/ddm-admin-2.0.0-45d40c.json.gitstub @@ -0,0 +1 @@ +561931c41eafde867f931229e799d1b30df7a1d4:openapi/ddm-admin/ddm-admin-2.0.0-45d40c.json diff --git a/openapi/ddm-admin/ddm-admin-2.0.0-45d40c.json b/openapi/ddm-admin/ddm-admin-3.0.0-2f4dd6.json similarity index 94% rename from openapi/ddm-admin/ddm-admin-2.0.0-45d40c.json rename to openapi/ddm-admin/ddm-admin-3.0.0-2f4dd6.json index e891ca0e6..a4aa65022 100644 --- a/openapi/ddm-admin/ddm-admin-2.0.0-45d40c.json +++ b/openapi/ddm-admin/ddm-admin-3.0.0-2f4dd6.json @@ -6,7 +6,7 @@ "url": "https://oxide.computer", "email": "api@oxide.computer" }, - "version": "2.0.0" + "version": "3.0.0" }, "paths": { "/disable-stats": { @@ -51,6 +51,32 @@ } } }, + "/external_peers": { + "post": { + "operationId": "set_external_peers", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SetExternalPeers" + } + } + }, + "required": true + }, + "responses": { + "204": { + "description": "resource updated" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/originated": { "get": { "operationId": "get_originated", @@ -590,6 +616,21 @@ 1 ] }, + "SetExternalPeers": { + "type": "object", + "properties": { + "interfaces": { + "type": "array", + "items": { + "type": "string" + }, + "uniqueItems": true + } + }, + "required": [ + "interfaces" + ] + }, "TunnelOrigin": { "type": "object", "properties": { diff --git a/openapi/ddm-admin/ddm-admin-latest.json b/openapi/ddm-admin/ddm-admin-latest.json index 4eb6e8dbc..7a1dd1602 120000 --- a/openapi/ddm-admin/ddm-admin-latest.json +++ b/openapi/ddm-admin/ddm-admin-latest.json @@ -1 +1 @@ -ddm-admin-2.0.0-45d40c.json \ No newline at end of file +ddm-admin-3.0.0-2f4dd6.json \ No newline at end of file From bad4ba1e577d50142f0567c06f354650b32c81b3 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Fri, 11 Sep 2026 08:59:08 +0000 Subject: [PATCH 02/29] linux chores --- ddm/Cargo.toml | 4 ++-- ddm/src/admin.rs | 2 +- ddm/src/sm/mod.rs | 19 +++++++++++++++++++ ddm/src/sm/state.rs | 16 ++-------------- 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/ddm/Cargo.toml b/ddm/Cargo.toml index 1168489a9..a66460942 100644 --- a/ddm/Cargo.toml +++ b/ddm/Cargo.toml @@ -36,15 +36,15 @@ oximeter-producer.workspace = true oxnet.workspace = true uuid.workspace = true ddm-api.workspace = true +dpd-client.workspace = true # illumos-only deps used by the routing state machine and platform sys layer. # Gated by the `backend` feature so stub builds (e.g. Linux test fixtures # running `ddmd` with `--api-only`) link cleanly. libnet = { workspace = true, optional = true } -dpd-client = { workspace = true, optional = true } opte-ioctl = { workspace = true, optional = true } oxide-vpc = { workspace = true, optional = true } [features] default = ["backend"] -backend = ["dep:libnet", "dep:dpd-client", "dep:opte-ioctl", "dep:oxide-vpc"] +backend = ["dep:libnet", "dep:opte-ioctl", "dep:oxide-vpc"] diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index eec6f165a..331faee95 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -497,7 +497,7 @@ impl DdmAdminApi for DdmAdminApiImpl { ctx.peers.push(sm_ctx.clone()); - crate::sm::state::send( + crate::sm::send( Event::Admin(AdminEvent::NewExternalPeer(tx.clone())), &mut ctx.event_channels, ); diff --git a/ddm/src/sm/mod.rs b/ddm/src/sm/mod.rs index 1ec50d5eb..3353940b2 100644 --- a/ddm/src/sm/mod.rs +++ b/ddm/src/sm/mod.rs @@ -274,3 +274,22 @@ pub struct StateMachine { pub ctx: SmContext, pub rx: Option>, } + +#[cfg(not(target_os = "illumos"))] +impl StateMachine { + pub fn run(&mut self) -> Result<(), SmError> { + Ok(()) + } +} + +pub(crate) fn send(e: Event, event_channels: &mut Vec>) { + let mut dead_channels = Vec::default(); + for (i, c) in event_channels.iter().enumerate() { + if c.send(e.clone()).is_err() { + dead_channels.push(i); + } + } + for i in dead_channels { + event_channels.remove(i); + } +} diff --git a/ddm/src/sm/state.rs b/ddm/src/sm/state.rs index b73e0af8b..711934e27 100644 --- a/ddm/src/sm/state.rs +++ b/ddm/src/sm/state.rs @@ -20,7 +20,7 @@ use std::collections::HashSet; use std::net::IpAddr; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::mpsc::{Receiver, Sender}; +use std::sync::mpsc::Receiver; use std::thread::{sleep, spawn}; use std::time::Duration; @@ -66,18 +66,6 @@ impl Init { } } -pub(crate) fn send(e: Event, event_channels: &mut Vec>) { - let mut dead_channels = Vec::default(); - for (i, c) in event_channels.iter().enumerate() { - if c.send(e.clone()).is_err() { - dead_channels.push(i); - } - } - for i in dead_channels { - event_channels.remove(i); - } -} - impl State for Init { fn run( &mut self, @@ -389,7 +377,7 @@ impl Exchange { }; let push = Update { underlay, tunnel }; - send( + super::send( Event::Peer(PeerEvent::Push(push.clone())), &mut self.ctx.event_channels, ); From 6743700bdc5564803ce9831d2a7d76b40fb89b0f Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Fri, 11 Sep 2026 09:38:15 +0000 Subject: [PATCH 03/29] plumb ddmadm --- ddmadm/src/main.rs | 11 +++++++++++ ddmd/src/main.rs | 8 -------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/ddmadm/src/main.rs b/ddmadm/src/main.rs index 21d56fdf3..659f3546e 100644 --- a/ddmadm/src/main.rs +++ b/ddmadm/src/main.rs @@ -6,6 +6,7 @@ use anyhow::Result; use clap::Parser; use colored::*; use ddm_admin_client::Client; +use ddm_admin_client::types::SetExternalPeers; use ddm_api_types_versions::latest::db::PeerStatus; use ddm_api_types_versions::latest::net as types; use mg_common::cli::oxide_cli_style; @@ -65,6 +66,9 @@ enum SubCommand { /// Sync prefix information from peers. Sync, + + /// Set external peers as a list of interfaces. + SetExternalPeers { ifx: Vec }, } #[derive(Debug, Parser)] @@ -265,6 +269,13 @@ async fn run() -> Result<()> { SubCommand::Sync => { client.sync().await?; } + SubCommand::SetExternalPeers { ifx } => { + client + .set_external_peers(&SetExternalPeers { + interfaces: ifx.clone(), + }) + .await?; + } } Ok(()) diff --git a/ddmd/src/main.rs b/ddmd/src/main.rs index 052122e67..8660a967a 100644 --- a/ddmd/src/main.rs +++ b/ddmd/src/main.rs @@ -28,14 +28,6 @@ use uuid::Uuid; mod signal; mod smf; -// macro_rules! u64_millis { -// ($x:expr) => { -// $x.as_millis() -// .try_into() -// .expect(&format!("{} as u64", stringify!($x))) -// }; -// } - #[derive(Debug, Parser)] #[command(version, about, long_about = None, styles = get_styles())] struct Arg { From 671c90b23aa51d954a1a60fe9f81098c68518452 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Fri, 11 Sep 2026 19:56:46 +0000 Subject: [PATCH 04/29] dynamic peer shutdown logic --- ddm/src/admin.rs | 23 +++++- ddm/src/discovery/runtime.rs | 30 +++++++- ddm/src/exchange/runtime.rs | 137 +++++++++++++++++------------------ ddm/src/sm/mod.rs | 3 +- ddm/src/sm/state.rs | 31 ++++---- ddmd/src/main.rs | 1 + 6 files changed, 134 insertions(+), 91 deletions(-) diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index 331faee95..1cba3d402 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -448,6 +448,7 @@ impl DdmAdminApi for DdmAdminApiImpl { let current = ctx.db.get_external_peers(); let to_create = rq.interfaces.difference(¤t); let to_remove = current.difference(&rq.interfaces); + ctx.db.set_external_peers(rq.interfaces.clone()); for ifx in to_create.into_iter() { let (tx, rx) = channel(); @@ -487,6 +488,7 @@ impl DdmAdminApi for DdmAdminApiImpl { rt: Arc::new(tokio::runtime::Handle::current()), iface: Arc::new(InterfaceState::external()), stats: Arc::new(SessionStats::default()), + discovery_stop: None, }; let mut sm = StateMachine { ctx: sm_ctx.clone(), @@ -506,13 +508,30 @@ impl DdmAdminApi for DdmAdminApiImpl { // TODO oxstats server } - for ifx in to_remove.into_iter() { + let mut remove_idx = Vec::default(); + info!(ctx.log, "removing peers"; + "to_remove" => ?to_remove, + "current" => ?ctx + .peers.iter().map(|x| &x.config.aobj_name).collect::>(), + ); + + for (i, ifx) in to_remove.into_iter().enumerate() { for p in &ctx.peers { - if &p.config.if_name == ifx { + if p.config.aobj_name.contains(ifx) { let _ = p.tx.send(Event::Admin(AdminEvent::Shutdown)); + remove_idx.push(i); + info!( + ctx.log, + "removing external peeer on interface {ifx}" + ); + } else { + info!(ctx.log, "{ifx} != {}", p.config.if_name); } } } + for i in remove_idx { + ctx.peers.remove(i); + } Ok(HttpResponseUpdatedNoContent()) } diff --git a/ddm/src/discovery/runtime.rs b/ddm/src/discovery/runtime.rs index 2cd8c53fc..d2ab6c6da 100644 --- a/ddm/src/discovery/runtime.rs +++ b/ddm/src/discovery/runtime.rs @@ -18,7 +18,7 @@ use serde::{Deserialize, Serialize}; use slog::Logger; use socket2::{Domain, Protocol, SockAddr, Socket, Type}; use std::mem::MaybeUninit; -use std::net::{Ipv6Addr, SocketAddrV6}; +use std::net::{Ipv6Addr, Shutdown, SocketAddrV6}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::Sender; use std::sync::{Arc, RwLock}; @@ -98,7 +98,7 @@ pub(crate) fn handler( iface: Arc, stats: Arc, log: Logger, -) -> Result<(), DiscoveryError> { +) -> Result, DiscoveryError> { // listening on 2 sockets, solicitations are sent to DDM_MADDR, but // advertisements are sent to the unicast source addresses of a // solicitation. Binding to a link-scoped multicast address is required for @@ -123,6 +123,7 @@ pub(crate) fn handler( let uc_sa: SockAddr = SocketAddrV6::new(config.addr, DDM_PORT, 0, config.if_index).into(); uc.bind(&uc_sa)?; + uc.set_reuse_address(true)?; uc.set_read_timeout(Some(Duration::from_millis( config.discovery_read_timeout, )))?; @@ -153,9 +154,9 @@ pub(crate) fn handler( stop.clone(), stats.clone(), )?; - expire(ctx, stop, stats.clone())?; + expire(ctx, stop.clone(), stats.clone())?; - Ok(()) + Ok(stop) } fn send_solicitations( @@ -165,6 +166,10 @@ fn send_solicitations( ) { spawn(move || { loop { + if stop.load(Ordering::Relaxed) { + inf!(ctx.log, ctx.config.if_name, "stopping solicitor"); + break; + } if let Err(e) = solicit(&ctx) { err!(ctx.log, ctx.config.if_name, "solicit failed: {}", e); stop.store(true, Ordering::Relaxed); @@ -230,6 +235,11 @@ fn expire( // sockets by trying to listen on a unicast address that a socket // waiting to be dropped is already listening on. if stop.load(Ordering::Relaxed) { + inf!( + &ctx.log, + ctx.config.if_name, + "stopping discovery expiration thread", + ); let event = ctx.event.clone(); let log = ctx.log.clone(); let if_name = ctx.config.if_name.clone(); @@ -258,6 +268,18 @@ fn listen( handle_msg(&ctx, msg, &addr, &stats); }; if stop.load(Ordering::Relaxed) { + inf!( + &ctx.log, + ctx.config.if_name, + "stopping discovery handler" + ); + if let Err(e) = s.shutdown(Shutdown::Both) { + wrn!( + &ctx.log, + ctx.config.if_name, + "failed to shut down discovery socket {e:?}", + ); + } break; } } diff --git a/ddm/src/exchange/runtime.rs b/ddm/src/exchange/runtime.rs index 653824e43..ebd9257ce 100644 --- a/ddm/src/exchange/runtime.rs +++ b/ddm/src/exchange/runtime.rs @@ -29,13 +29,14 @@ use http_body_util::BodyExt; use hyper::body::Bytes; use hyper_util::client::legacy::Client; use hyper_util::rt::TokioExecutor; +use mg_common::lock; use slog::{Logger, o}; use std::collections::HashSet; use std::net::{Ipv6Addr, SocketAddrV6}; use std::sync::Arc; +use std::sync::Mutex; use std::sync::atomic::Ordering; use std::time::Duration; -use tokio::sync::Mutex; use tokio::time::timeout; const UNIT_EXCHANGE_SERVER: &str = "exchange_server"; @@ -44,7 +45,6 @@ const UNIT_EXCHANGE_SERVER: &str = "exchange_server"; pub struct HandlerContext { ctx: SmContext, peer: Ipv6Addr, - log: Logger, } pub(crate) fn announce_underlay( @@ -151,25 +151,19 @@ fn do_pull_common( } pub(crate) fn pull( - ctx: SmContext, + ctx: &mut SmContext, addr: Ipv6Addr, version: Version, rt: Arc, - log: Logger, ) -> Result<(), ExchangeError> { let pr: v3::PullResponse = match version { - Version::V2 => do_pull_v2(&ctx, &addr, &rt)?.into(), - Version::V3 => do_pull(&ctx, &addr, &rt)?, + Version::V2 => do_pull_v2(ctx, &addr, &rt)?.into(), + Version::V3 => do_pull(ctx, &addr, &rt)?, }; let update = v3::Update::announce(pr); - let hctx = HandlerContext { - ctx, - peer: addr, - log: log.clone(), - }; - handle_update(&update, &hctx); + handle_update(&update, ctx, addr); Ok(()) } @@ -271,7 +265,6 @@ pub fn handler( ) -> Result, String> { let context = Arc::new(Mutex::new(HandlerContext { ctx: ctx.clone(), - log: log.clone(), peer, })); @@ -365,9 +358,12 @@ async fn push_handler_common( ctx: RequestContext>>, update: v3::Update, ) -> Result { - let ctx = ctx.context().lock().await.clone(); + let rq_ctx: Arc> = ctx.context().clone(); + tokio::task::spawn_blocking(move || { - handle_update(&update, &ctx); + let mut actx = lock!(rq_ctx); + let peer = actx.peer; + handle_update(&update, &mut actx.ctx, peer); }) .await .map_err(|e| { @@ -384,7 +380,7 @@ async fn push_handler_common( async fn pull_handler_v2( ctx: RequestContext>>, ) -> Result, HttpError> { - let ctx = ctx.context().lock().await.clone(); + let ctx = lock!(ctx.context()); let mut underlay = HashSet::new(); let mut tunnel = HashSet::new(); @@ -460,7 +456,7 @@ async fn pull_handler_v2( async fn pull_handler( ctx: RequestContext>>, ) -> Result, HttpError> { - let ctx = ctx.context().lock().await.clone(); + let ctx = lock!(ctx.context()); let mut underlay = HashSet::new(); let mut tunnel = HashSet::new(); @@ -529,50 +525,56 @@ async fn pull_handler( })) } -fn handle_update(update: &v3::Update, ctx: &HandlerContext) { - ctx.ctx - .stats - .updates_received - .fetch_add(1, Ordering::Relaxed); +fn handle_update( + update: &v3::Update, + ctx: &mut SmContext, + peer_addr: Ipv6Addr, +) { + ctx.stats.updates_received.fetch_add(1, Ordering::Relaxed); if let Some(underlay_update) = &update.underlay { - handle_underlay_update(underlay_update, ctx); + handle_underlay_update(underlay_update, ctx, peer_addr); } if let Some(tunnel_update) = &update.tunnel { - handle_tunnel_update(tunnel_update, ctx); + handle_tunnel_update(tunnel_update, ctx, peer_addr); } // distribute updates - if ctx.ctx.config.kind == RouterKind::Transit { + if ctx.config.kind == RouterKind::Transit { dbg!( ctx.log, - ctx.ctx.config.if_name, + ctx.config.if_name, "redistributing update to {} peers", - ctx.ctx.event_channels.len() + ctx.event_channels.len() ); let underlay = update .underlay .as_ref() - .map(|update| update.with_path_element(ctx.ctx.hostname.clone())); + .map(|update| update.with_path_element(ctx.hostname.clone())); let push = v3::Update { underlay, tunnel: update.tunnel.clone(), }; - for ec in &ctx.ctx.event_channels { - ec.send(Event::Peer(PeerEvent::Push(push.clone()))).unwrap(); - } + crate::sm::send( + Event::Peer(PeerEvent::Push(push.clone())), + &mut ctx.event_channels, + ); } } -fn handle_tunnel_update(update: &v3::TunnelUpdate, ctx: &HandlerContext) { +fn handle_tunnel_update( + update: &v3::TunnelUpdate, + ctx: &mut SmContext, + peer_addr: Ipv6Addr, +) { let mut import = HashSet::new(); let mut remove = HashSet::new(); - let db = &ctx.ctx.db; + let db = &ctx.db; let before = effective_route_set(&db.imported_tunnel()); @@ -584,7 +586,7 @@ fn handle_tunnel_update(update: &v3::TunnelUpdate, ctx: &HandlerContext) { vni: x.vni, metric: x.metric, }, - nexthop: ctx.peer, + nexthop: peer_addr, }); } db.import_tunnel(&import); @@ -597,7 +599,7 @@ fn handle_tunnel_update(update: &v3::TunnelUpdate, ctx: &HandlerContext) { vni: x.vni, metric: x.metric, }, - nexthop: ctx.peer, + nexthop: peer_addr, }); } db.delete_import_tunnel(&remove); @@ -607,72 +609,66 @@ fn handle_tunnel_update(update: &v3::TunnelUpdate, ctx: &HandlerContext) { let to_add = after.difference(&before).copied().collect(); let to_del = before.difference(&after).copied().collect(); - if let Err(e) = crate::sys::add_tunnel_routes( - &ctx.log, - &ctx.ctx.config.if_name, - &to_add, - ) { + if let Err(e) = + crate::sys::add_tunnel_routes(&ctx.log, &ctx.config.if_name, &to_add) + { err!( ctx.log, - ctx.ctx.config.if_name, + ctx.config.if_name, "add tunnel routes: {e}: {:#?}", import, ) } - if let Err(e) = crate::sys::remove_tunnel_routes( - &ctx.log, - &ctx.ctx.config.if_name, - &to_del, - ) { + if let Err(e) = + crate::sys::remove_tunnel_routes(&ctx.log, &ctx.config.if_name, &to_del) + { err!( ctx.log, - ctx.ctx.config.if_name, + ctx.config.if_name, "remove tunnel routes: {e}: {:#?}", import, ) } - ctx.ctx - .stats + ctx.stats .imported_underlay_prefixes - .store(ctx.ctx.db.imported_tunnel_count() as u64, Ordering::Relaxed); + .store(ctx.db.imported_tunnel_count() as u64, Ordering::Relaxed); } -fn handle_underlay_update(update: &v3::UnderlayUpdate, ctx: &HandlerContext) { +fn handle_underlay_update( + update: &v3::UnderlayUpdate, + ctx: &mut SmContext, + peer_addr: Ipv6Addr, +) { let mut import = HashSet::new(); let mut add = Vec::new(); - let db = &ctx.ctx.db; + let db = &ctx.db; for prefix in &update.announce { import.insert(Route { destination: prefix.destination, - nexthop: ctx.peer, - ifname: ctx.ctx.config.if_name.clone(), + nexthop: peer_addr, + ifname: ctx.config.if_name.clone(), path: prefix.path.clone(), }); let mut r = crate::sys::Route::new( prefix.destination.addr().into(), prefix.destination.width(), - ctx.peer.into(), + peer_addr.into(), ); - r.ifname.clone_from(&ctx.ctx.config.if_name); + r.ifname.clone_from(&ctx.config.if_name); add.push(r); } db.import(&import); - crate::sys::add_underlay_routes( - &ctx.log, - &ctx.ctx.config, - add, - &ctx.ctx.rt, - ); + crate::sys::add_underlay_routes(&ctx.log, &ctx.config, add, &ctx.rt); let mut withdraw = HashSet::new(); for prefix in &update.withdraw { withdraw.insert(Route { destination: prefix.destination, - nexthop: ctx.peer, - ifname: ctx.ctx.config.if_name.clone(), + nexthop: peer_addr, + ifname: ctx.config.if_name.clone(), path: prefix.path.clone(), }); } @@ -694,20 +690,19 @@ fn handle_underlay_update(update: &v3::UnderlayUpdate, ctx: &HandlerContext) { w.destination.width(), w.nexthop.into(), ); - r.ifname.clone_from(&ctx.ctx.config.if_name); + r.ifname.clone_from(&ctx.config.if_name); del.push(r); } } crate::sys::remove_underlay_routes( &ctx.log, - &ctx.ctx.config.if_name, - &ctx.ctx.config.dpd, + &ctx.config.if_name, + &ctx.config.dpd, del, - &ctx.ctx.rt, + &ctx.rt, ); - ctx.ctx - .stats + ctx.stats .imported_underlay_prefixes - .store(ctx.ctx.db.imported_count() as u64, Ordering::Relaxed); + .store(ctx.db.imported_count() as u64, Ordering::Relaxed); } diff --git a/ddm/src/sm/mod.rs b/ddm/src/sm/mod.rs index 3353940b2..c9e6372c3 100644 --- a/ddm/src/sm/mod.rs +++ b/ddm/src/sm/mod.rs @@ -16,7 +16,7 @@ use oxnet::Ipv6Net; use slog::Logger; use std::collections::HashSet; use std::net::Ipv6Addr; -use std::sync::atomic::AtomicU64; +use std::sync::atomic::{AtomicBool, AtomicU64}; use std::sync::mpsc::{Receiver, Sender}; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; @@ -268,6 +268,7 @@ pub struct SmContext { pub iface: Arc, pub stats: Arc, pub log: Logger, + pub discovery_stop: Option>, } pub struct StateMachine { diff --git a/ddm/src/sm/state.rs b/ddm/src/sm/state.rs index 711934e27..61f0403a2 100644 --- a/ddm/src/sm/state.rs +++ b/ddm/src/sm/state.rs @@ -118,7 +118,7 @@ impl State for Init { // Now that we have an ip address to run discovery on, start the // discovery handler and jump into the solicit state. - discovery::handler( + let discovery_stop = discovery::handler( self.ctx.hostname.clone(), self.ctx.config.clone(), self.ctx.tx.clone(), @@ -127,6 +127,7 @@ impl State for Init { self.ctx.log.clone(), ) .unwrap(); // TODO unwrap + self.ctx.discovery_stop = Some(discovery_stop); return ( Some(Box::new(Solicit::new( self.ctx.clone(), @@ -212,6 +213,9 @@ impl State for Solicit { self.ctx.event_channels.push(tx); } Event::Admin(AdminEvent::Shutdown) => { + if let Some(x) = self.ctx.discovery_stop.as_mut() { + x.store(true, Ordering::Relaxed); + } return (None, event); } Event::Admin(e) => { @@ -249,23 +253,20 @@ impl Exchange { } } - fn initial_pull(&self, stop: Arc) { - let ctx = self.ctx.clone(); + fn initial_pull(&mut self, stop: Arc) { + //let ctx = self.ctx.clone(); let peer = self.peer; let version = self.version; let rt = self.ctx.rt.clone(); let log = self.log.clone(); let interval = self.ctx.config.solicit_interval; let if_name = self.ctx.config.if_name.clone(); + let mut ctx = self.ctx.clone(); spawn(move || { - while let Err(e) = crate::exchange::pull( - ctx.clone(), - peer, - version, - rt.clone(), - log.clone(), - ) { + while let Err(e) = + crate::exchange::pull(&mut ctx, peer, version, rt.clone()) + { sleep(Duration::from_millis(interval)); wrn!(log, if_name, "exchange pull: {}", e); if stop.load(Ordering::Relaxed) { @@ -607,12 +608,12 @@ impl State for Exchange { } } Event::Admin(AdminEvent::Sync) => { + let rt = self.ctx.rt.clone(); if let Err(e) = crate::exchange::pull( - self.ctx.clone(), + &mut self.ctx, self.peer, self.version, - self.ctx.rt.clone(), - self.log.clone(), + rt, ) { err!( self.log, @@ -626,6 +627,10 @@ impl State for Exchange { self.ctx.event_channels.push(tx); } Event::Admin(AdminEvent::Shutdown) => { + if let Some(x) = self.ctx.discovery_stop.as_mut() { + x.store(true, Ordering::Relaxed); + } + self.expire_peer(&exchange_thread, &pull_stop); return (None, event); } Event::Peer(PeerEvent::Push(update)) => { diff --git a/ddmd/src/main.rs b/ddmd/src/main.rs index 8660a967a..1617dd54e 100644 --- a/ddmd/src/main.rs +++ b/ddmd/src/main.rs @@ -277,6 +277,7 @@ fn start_state_machines( rt: rt.clone(), iface: Arc::new(InterfaceState::default()), stats: Arc::new(ddm::sm::SessionStats::default()), + discovery_stop: None, }; let sm = StateMachine { ctx, rx: Some(rx) }; From d91e8becfa53f1059c5e09747ee296c4fcb64670 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Sat, 12 Sep 2026 08:06:44 +0000 Subject: [PATCH 05/29] test environment for external peers, various fixes --- .github/buildomat/jobs/test-ddm-sextet.sh | 18 ++ Cargo.lock | 2 +- Cargo.toml | 2 +- .../src/external_peers/external_peers.rs | 2 +- ddm/src/admin.rs | 10 +- ddm/src/discovery/runtime.rs | 2 +- ddm/src/sm/state.rs | 1 - ddmadm/src/main.rs | 6 +- ...f4dd6.json => ddm-admin-3.0.0-40f03b.json} | 4 +- openapi/ddm-admin/ddm-admin-latest.json | 2 +- ...l => softnpu-quartet-sidecar.quartet.toml} | 0 .../conf/softnpu-sextet-sidecar_a.sextet.toml | 7 + .../conf/softnpu-sextet-sidecar_b.sextet.toml | 7 + ...io.toml => softnpu-trio-sidecar.trio.toml} | 0 tests/src/ddm.rs | 278 +++++++++++++++++- 15 files changed, 313 insertions(+), 28 deletions(-) create mode 100644 .github/buildomat/jobs/test-ddm-sextet.sh rename openapi/ddm-admin/{ddm-admin-3.0.0-2f4dd6.json => ddm-admin-3.0.0-40f03b.json} (99%) rename tests/conf/{softnpu-quartet.toml => softnpu-quartet-sidecar.quartet.toml} (100%) create mode 100644 tests/conf/softnpu-sextet-sidecar_a.sextet.toml create mode 100644 tests/conf/softnpu-sextet-sidecar_b.sextet.toml rename tests/conf/{softnpu-trio.toml => softnpu-trio-sidecar.trio.toml} (100%) diff --git a/.github/buildomat/jobs/test-ddm-sextet.sh b/.github/buildomat/jobs/test-ddm-sextet.sh new file mode 100644 index 000000000..975e1fbd5 --- /dev/null +++ b/.github/buildomat/jobs/test-ddm-sextet.sh @@ -0,0 +1,18 @@ +#!/bin/bash +#: +#: name = "test-ddm-sextet" +#: variety = "basic" +#: target = "helios-3.0" +#: rust_toolchain = "stable" +#: output_rules = [ +#: "/work/*.log", +#: ] + +source .github/buildomat/test-ddm-common.sh + +# +# trio tests +# + +banner "trio" +pfexec cargo test --release -p mg-tests test_external_peer_sextet -- --nocapture diff --git a/Cargo.lock b/Cargo.lock index ad01e50e1..19b81b40f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9469,7 +9469,7 @@ dependencies = [ [[package]] name = "ztest" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/falcon?branch=main#021420c1e2f1ac9c66759d5df44979940a1ac483" +source = "git+https://github.com/oxidecomputer/falcon?branch=ry%2Fztest-dl-privs#3acbd55db9d280f19c32b03fa93fce1a8fad3571" dependencies = [ "anyhow", "libnet", diff --git a/Cargo.toml b/Cargo.toml index 2bbc6489a..0cafc102a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -112,7 +112,7 @@ clap = { version = "4.6.6", features = ["derive", "unstable-styles", "env"] } tabwriter = { version = "1", features = ["ansi_formatting"] } colored = "3.1" strum = { version = "0.28", features = ["derive"] } -ztest = { git = "https://github.com/oxidecomputer/falcon", branch = "main" } +ztest = { git = "https://github.com/oxidecomputer/falcon", branch = "ry/ztest-dl-privs" } libfalcon = { git = "https://github.com/oxidecomputer/falcon", branch = "main" } anstyle = "1.0.14" nom = "8.0" diff --git a/ddm-api-types/versions/src/external_peers/external_peers.rs b/ddm-api-types/versions/src/external_peers/external_peers.rs index f974d57e5..14b9f96f4 100644 --- a/ddm-api-types/versions/src/external_peers/external_peers.rs +++ b/ddm-api-types/versions/src/external_peers/external_peers.rs @@ -8,5 +8,5 @@ use std::collections::BTreeSet; #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] pub struct SetExternalPeers { - pub interfaces: BTreeSet, + pub address_objects: BTreeSet, } diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index 1cba3d402..ecc984299 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -446,11 +446,11 @@ impl DdmAdminApi for DdmAdminApiImpl { let rq = request.into_inner(); let current = ctx.db.get_external_peers(); - let to_create = rq.interfaces.difference(¤t); - let to_remove = current.difference(&rq.interfaces); - ctx.db.set_external_peers(rq.interfaces.clone()); + let to_create = rq.address_objects.difference(¤t); + let to_remove = current.difference(&rq.address_objects); + ctx.db.set_external_peers(rq.address_objects.clone()); - for ifx in to_create.into_iter() { + for addr_obj in to_create.into_iter() { let (tx, rx) = channel(); let config = crate::sm::Config { @@ -460,7 +460,7 @@ impl DdmAdminApi for DdmAdminApiImpl { ip_addr_wait: millis_u64(IP_ADDR_WAIT), exchange_timeout: millis_u64(EXCHANGE_TIMEOUT), exchange_port: EXCHANGE_TCP_PORT, - aobj_name: format!("{ifx}/ll"), + aobj_name: addr_obj.clone(), if_name: String::default(), // initialized in state machine if_index: 0, // initialized in state machine // External peers are only a thing for transit routers. diff --git a/ddm/src/discovery/runtime.rs b/ddm/src/discovery/runtime.rs index d2ab6c6da..165e578a2 100644 --- a/ddm/src/discovery/runtime.rs +++ b/ddm/src/discovery/runtime.rs @@ -123,7 +123,7 @@ pub(crate) fn handler( let uc_sa: SockAddr = SocketAddrV6::new(config.addr, DDM_PORT, 0, config.if_index).into(); uc.bind(&uc_sa)?; - uc.set_reuse_address(true)?; + //uc.set_reuse_address(true)?; uc.set_read_timeout(Some(Duration::from_millis( config.discovery_read_timeout, )))?; diff --git a/ddm/src/sm/state.rs b/ddm/src/sm/state.rs index 61f0403a2..d39017a36 100644 --- a/ddm/src/sm/state.rs +++ b/ddm/src/sm/state.rs @@ -254,7 +254,6 @@ impl Exchange { } fn initial_pull(&mut self, stop: Arc) { - //let ctx = self.ctx.clone(); let peer = self.peer; let version = self.version; let rt = self.ctx.rt.clone(); diff --git a/ddmadm/src/main.rs b/ddmadm/src/main.rs index 659f3546e..c22c9ec1b 100644 --- a/ddmadm/src/main.rs +++ b/ddmadm/src/main.rs @@ -68,7 +68,7 @@ enum SubCommand { Sync, /// Set external peers as a list of interfaces. - SetExternalPeers { ifx: Vec }, + SetExternalPeers { addr_obj: Vec }, } #[derive(Debug, Parser)] @@ -269,10 +269,10 @@ async fn run() -> Result<()> { SubCommand::Sync => { client.sync().await?; } - SubCommand::SetExternalPeers { ifx } => { + SubCommand::SetExternalPeers { addr_obj } => { client .set_external_peers(&SetExternalPeers { - interfaces: ifx.clone(), + address_objects: addr_obj.clone(), }) .await?; } diff --git a/openapi/ddm-admin/ddm-admin-3.0.0-2f4dd6.json b/openapi/ddm-admin/ddm-admin-3.0.0-40f03b.json similarity index 99% rename from openapi/ddm-admin/ddm-admin-3.0.0-2f4dd6.json rename to openapi/ddm-admin/ddm-admin-3.0.0-40f03b.json index a4aa65022..6cf57e3c9 100644 --- a/openapi/ddm-admin/ddm-admin-3.0.0-2f4dd6.json +++ b/openapi/ddm-admin/ddm-admin-3.0.0-40f03b.json @@ -619,7 +619,7 @@ "SetExternalPeers": { "type": "object", "properties": { - "interfaces": { + "address_objects": { "type": "array", "items": { "type": "string" @@ -628,7 +628,7 @@ } }, "required": [ - "interfaces" + "address_objects" ] }, "TunnelOrigin": { diff --git a/openapi/ddm-admin/ddm-admin-latest.json b/openapi/ddm-admin/ddm-admin-latest.json index 7a1dd1602..43e4b563f 120000 --- a/openapi/ddm-admin/ddm-admin-latest.json +++ b/openapi/ddm-admin/ddm-admin-latest.json @@ -1 +1 @@ -ddm-admin-3.0.0-2f4dd6.json \ No newline at end of file +ddm-admin-3.0.0-40f03b.json \ No newline at end of file diff --git a/tests/conf/softnpu-quartet.toml b/tests/conf/softnpu-quartet-sidecar.quartet.toml similarity index 100% rename from tests/conf/softnpu-quartet.toml rename to tests/conf/softnpu-quartet-sidecar.quartet.toml diff --git a/tests/conf/softnpu-sextet-sidecar_a.sextet.toml b/tests/conf/softnpu-sextet-sidecar_a.sextet.toml new file mode 100644 index 000000000..87bce5690 --- /dev/null +++ b/tests/conf/softnpu-sextet-sidecar_a.sextet.toml @@ -0,0 +1,7 @@ +p4_program = "/opt/libsidecar_lite.so" +ports = [ + { sidecar = "sw0", scrimlet = "sq0", mtu = 1500 }, + { sidecar = "sw1", scrimlet = "sq1", mtu = 1500 }, + { sidecar = "sw2", scrimlet = "sr0", mtu = 1500 }, + { sidecar = "sw3", scrimlet = "sr1", mtu = 1500 }, +] diff --git a/tests/conf/softnpu-sextet-sidecar_b.sextet.toml b/tests/conf/softnpu-sextet-sidecar_b.sextet.toml new file mode 100644 index 000000000..e799a1ee2 --- /dev/null +++ b/tests/conf/softnpu-sextet-sidecar_b.sextet.toml @@ -0,0 +1,7 @@ +p4_program = "/opt/libsidecar_lite.so" +ports = [ + { sidecar = "sw4", scrimlet = "sq2", mtu = 1500 }, + { sidecar = "sw5", scrimlet = "sq3", mtu = 1500 }, + { sidecar = "sw6", scrimlet = "sr2", mtu = 1500 }, + { sidecar = "sw7", scrimlet = "sr3", mtu = 1500 }, +] diff --git a/tests/conf/softnpu-trio.toml b/tests/conf/softnpu-trio-sidecar.trio.toml similarity index 100% rename from tests/conf/softnpu-trio.toml rename to tests/conf/softnpu-trio-sidecar.trio.toml diff --git a/tests/src/ddm.rs b/tests/src/ddm.rs index 988ed973d..c40d0acef 100644 --- a/tests/src/ddm.rs +++ b/tests/src/ddm.rs @@ -7,6 +7,7 @@ use client_common::{eprintln_nopipe, println_nopipe}; use ddm_admin_client::Client; use ddm_api_types_versions::latest::net::TunnelOrigin; use slog::{Drain, Logger}; +use std::collections::BTreeMap; use std::env; use std::net::Ipv6Addr; use std::thread::sleep; @@ -60,7 +61,7 @@ impl<'a> SoftnpuZone<'a> { ifx: &[&'a str], testname: &'a str, ) -> Result { - let softnpu_mount = format!("/tmp/softnpu/{}", testname); + let softnpu_mount = format!("/tmp/softnpu/{testname}/{name}"); std::fs::create_dir_all(&softnpu_mount)?; let fs = &[FsMount::new(&softnpu_mount, "/opt/mnt")]; @@ -91,7 +92,10 @@ impl<'a> SoftnpuZone<'a> { )?; self.zfs.copy_workspace_to_zone( &self.zone.name, - &format!("tests/conf/softnpu-{}.toml", self.testname), + &format!( + "tests/conf/softnpu-{}-{}.toml", + self.testname, self.zone.name + ), "opt/softnpu.toml", )?; self.zone.zexec(&format!( @@ -135,6 +139,7 @@ struct RouterZone<'a> { zone: Zone, transit: bool, testname: String, + port_map: BTreeMap, } impl<'a> RouterZone<'a> { @@ -144,7 +149,7 @@ impl<'a> RouterZone<'a> { mgmt: &'a str, rtr_ifx: &[&'a str], ) -> Result { - Self::new(name, zfs, mgmt, rtr_ifx, false, "") + Self::new(name, zfs, mgmt, rtr_ifx, false, "", "") } fn transit( @@ -153,8 +158,9 @@ impl<'a> RouterZone<'a> { mgmt: &'a str, rtr_ifx: &[&'a str], testname: &str, + softnpu_name: &str, ) -> Result { - Self::new(name, zfs, mgmt, rtr_ifx, true, testname) + Self::new(name, zfs, mgmt, rtr_ifx, true, testname, softnpu_name) } fn new( @@ -164,12 +170,14 @@ impl<'a> RouterZone<'a> { rtr_ifx: &[&'a str], transit: bool, testname: &str, + softnpu_name: &str, ) -> Result { let mut ifx = vec![mgmt]; ifx.extend_from_slice(rtr_ifx); let fs = if transit { - let softnpu_mount = format!("/tmp/softnpu/{}", testname); + let softnpu_mount = + format!("/tmp/softnpu/{testname}/{softnpu_name}"); std::fs::create_dir_all(&softnpu_mount)?; vec![FsMount::new(&softnpu_mount, "/opt/mnt")] } else { @@ -183,19 +191,45 @@ impl<'a> RouterZone<'a> { zone, transit, testname: testname.into(), + port_map: BTreeMap::default(), }) } + fn set_port_map(&mut self, pm: BTreeMap) { + self.port_map = pm; + } + fn stop_router(&self) -> Result { self.zone.zexec("pkill ddmd") } fn start_router(&self, restart_dpd: bool) -> Result<()> { - let addrs = self.ifx[1..] + let mapped_ports = self.ifx[1..] + .iter() + .map(|x| x.to_string()) + .map(|x| self.port_map.get(&x).unwrap_or(&x).clone()) + .collect::>(); + + let rear_ports = mapped_ports + .iter() + .filter(|&x| x.contains("rear")) + .cloned() + .collect::>(); + let front_ports = mapped_ports .iter() - .map(|x| format!("-a {}/v6", x)) - .collect::>() - .join(" "); + .filter(|&x| x.contains("qsfp")) + .cloned() + .collect::>(); + + let addrs = if self.transit { + &rear_ports + } else { + &mapped_ports + } + .iter() + .map(|x| format!("-a {}/v6", x)) + .collect::>() + .join(" "); let ddm = "/opt/ddmd"; let extra_args = format!( @@ -216,11 +250,17 @@ impl<'a> RouterZone<'a> { self.zone.zexec( "svccfg -s dendrite setprop config/uds_path = /opt/mnt", )?; + /* self.zone.zexec( "svccfg -s dendrite setprop config/port_config = /opt/dpd-ports.toml")?; + */ + self.zone.zexec(&format!( + "svccfg -s dendrite setprop config/front_ports = {}", + front_ports.len(), + ))?; self.zone.zexec(&format!( "svccfg -s dendrite setprop config/rear_ports = {}", - self.ifx.len() - 1 + rear_ports.len(), ))?; self.zone.zexec("svcadm refresh dendrite:default")?; self.zone.zexec("svcadm enable dendrite:default")?; @@ -265,7 +305,18 @@ impl<'a> RouterZone<'a> { ), )?; - for ifx in &self.ifx[1..] { + for (link, vnic) in &self.port_map { + self.zone + .zexec(&format!("dladm create-vnic -t -l {link} {vnic}"))?; + } + + let mapped_ports = self.ifx[1..] + .iter() + .map(|x| x.to_string()) + .map(|x| self.port_map.get(&x).unwrap_or(&x).clone()) + .collect::>(); + + for ifx in &mapped_ports { self.zone.zcmd( &z, &format!("ipadm create-addr -t -T addrconf {}/v6", ifx), @@ -437,6 +488,7 @@ async fn test_trio() -> Result<()> { &mg1.name, &[&tf0_sr0.end_a, &tf1_sr1.end_a], "trio", + "sidecar.trio", )?; println_nopipe!("waiting for zones to come up"); @@ -653,7 +705,7 @@ async fn run_trio_tests( #[tokio::test] async fn test_quartet() -> Result<()> { - // A quartet of servers in a star topology. + // A quartet of routers in a star topology. // // sled1 // ,----------, @@ -726,6 +778,7 @@ async fn test_quartet() -> Result<()> { &mgt1.name, &[&tf0_sr0.end_a, &tf1_sr1.end_a, &tf2_sr2.end_a], "quartet", + "sidecar.quartet", )?; println_nopipe!("waiting for zones to come up"); @@ -802,6 +855,207 @@ async fn run_quartet_tests( Ok(()) } +#[tokio::test] +async fn test_external_peer_sextet() -> Result<()> { + // A sextet of routers in a multi-rack topology. + // + // sled1 + // ,----------, + // ,-----, ,-----, + // ,-| sl0 | | mg2 |-* + // scrimletA sidecarA | '-----' '-----' + // ,-----------, ,-----------------, | '----------' + // | ,-----, ,-----, ,-----, ,-----, | + // | | tr0a|--| sr0 |-| |-| sw2 |-' sled2 + // | '-----' '-----' | | '-----' ,----------, + // ,-----, ,-----, ,-----, |soft | ,-----, ,-----, ,-----, + // *-| mg1 | | tr1a|--| sr1 |-| npu|-| sw3 |---| sl1 | | mg3 |-* + // '-----' '-----' '-----' | | '-----' '-----' '-----' + // | ,-----, ,-----, | | ,-----, '----------' + // | | tq0a|--| sq0 |-| |-| sw0 |----, + // | '-----' '-----' | | '-----' | + // | ,-----, ,-----, | | ,-----, | + // | | tq1a|--| sq1 |-| |-| sw1 |-, | + // | '-----' '-----' '-----' '-----' | | + // '-----------' '-----------------' | | + // | | + // | | + // | | + // scrimletB sidecarB | | + // ,-----------, ,-----------------, | | + // | ,-----, ,-----, ,-----, ,-----, | | + // | | tq0b|--| sq2 |-| |-| sw4 |-' | + // | '-----' '-----' | | '-----' | + // ,-----, ,-----, ,-----, |soft | ,-----, | + // *-| mg4 | | tq1b|--| sq3 |-| npu|-| sw5 |----' sled3 + // '-----' '-----' '-----' | | '-----' ,----------, + // | ,-----, ,-----, | | ,-----, ,-----, ,-----, + // | | tr0b|--| sr2 |-| |-| sw6 |---| sl2 | | mg5 |-* + // | '-----' '-----' | | '-----' '-----' '-----' + // | ,-----, ,-----, | | ,-----, '----------' + // | | tr1b|--| sr3 |-| |-| sw7 |-, + // | '-----' '-----' '-----' '-----' | sled4 + // '-----------' '-----------------' | ,----------, + // | ,-----, ,-----, + // '-| sl3 | | mg6 |-* + // '-----' '-----' + // '----------' + + // Scrimlet A <-> Sidecar A + let tqa_sq_0 = SimnetLink::new("tqa0", "sq0")?; + let tqa_sq_1 = SimnetLink::new("tqa1", "sq1")?; + let tra_sr_0 = SimnetLink::new("tra0", "sr0")?; + let tra_sr_1 = SimnetLink::new("tra1", "sr1")?; + + // Sidecar A <-> Sleds + let sl0_sw2 = SimnetLink::new("sl0", "sw2")?; + let sl1_sw3 = SimnetLink::new("sl1", "sw3")?; + + // Scrimlet B <-> Sidecar B + let tqb_sq_0 = SimnetLink::new("tqb0", "sq2")?; + let tqb_sq_1 = SimnetLink::new("tqb1", "sq3")?; + let trb_sr_0 = SimnetLink::new("trb0", "sr2")?; + let trb_sr_1 = SimnetLink::new("trb1", "sr3")?; + + // Sidecar B <-> Sleds + let sl2_sw6 = SimnetLink::new("sl2", "sw6")?; + let sl3_sw7 = SimnetLink::new("sl3", "sw7")?; + + // Sidecar A <-> Sidecar B + let sw0_sw4 = SimnetLink::new("sw0", "sw5")?; + let sw1_sw5 = SimnetLink::new("sw1", "sw4")?; + + let mgmt0 = Etherstub::new("mgmt0")?; + let mg0 = Vnic::new("mg0", &mgmt0.name)?; + let mgs1 = Vnic::new("mgs1", &mgmt0.name)?; + let mgs2 = Vnic::new("mgs2", &mgmt0.name)?; + let mgs3 = Vnic::new("mgs3", &mgmt0.name)?; + let mgs4 = Vnic::new("mgs4", &mgmt0.name)?; + let mgs5 = Vnic::new("mgs5", &mgmt0.name)?; + let mgs6 = Vnic::new("mgs6", &mgmt0.name)?; + + let _mgip = Ip::new("10.0.0.254/24", &mg0.name, "test")?; + + let zfs = Zfs::new("mgtest")?; + + let sidecar_a = SoftnpuZone::new( + "sidecar_a.sextet", + &zfs, + &[ + &tqa_sq_0.end_b, + &tqa_sq_1.end_b, + &tra_sr_0.end_b, + &tra_sr_1.end_b, + &sl0_sw2.end_b, + &sl1_sw3.end_b, + &sw0_sw4.end_a, + &sw1_sw5.end_a, + ], + "sextet", + )?; + + let sidecar_b = SoftnpuZone::new( + "sidecar_b.sextet", + &zfs, + &[ + &tqb_sq_0.end_b, + &tqb_sq_1.end_b, + &trb_sr_0.end_b, + &trb_sr_1.end_b, + &sl2_sw6.end_b, + &sl3_sw7.end_b, + &sw0_sw4.end_b, + &sw1_sw5.end_b, + ], + "sextet", + )?; + + println_nopipe!("start zone s1"); + let s1 = + RouterZone::server("s1.sextet", &zfs, &mgs2.name, &[&sl0_sw2.end_a])?; + + println_nopipe!("start zone s2"); + let s2 = + RouterZone::server("s2.sextet", &zfs, &mgs3.name, &[&sl1_sw3.end_a])?; + + println_nopipe!("start zone s3"); + let s3 = + RouterZone::server("s3.sextet", &zfs, &mgs5.name, &[&sl2_sw6.end_a])?; + + println_nopipe!("start zone s4"); + let s4 = + RouterZone::server("s4.sextet", &zfs, &mgs6.name, &[&sl3_sw7.end_a])?; + + println_nopipe!("start zone t1"); + let mut t1 = RouterZone::transit( + "t1.sextet", + &zfs, + &mgs1.name, + &[ + &tqa_sq_0.end_a, + &tqa_sq_1.end_a, + &tra_sr_0.end_a, + &tra_sr_1.end_a, + ], + "sextet", + "sidecar_a.sextet", + )?; + t1.set_port_map(BTreeMap::from([ + ("tra0".into(), "tfportrear0_0".into()), + ("tra1".into(), "tfportrear1_0".into()), + ("tqa0".into(), "tfportqsfp0_0".into()), + ("tqa1".into(), "tfportqsfp1_0".into()), + ])); + + println_nopipe!("start zone t2"); + let mut t2 = RouterZone::transit( + "t2.sextet", + &zfs, + &mgs4.name, + &[ + &tqb_sq_0.end_a, + &tqb_sq_1.end_a, + &trb_sr_0.end_a, + &trb_sr_1.end_a, + ], + "sextet", + "sidecar_b.sextet", + )?; + t2.set_port_map(BTreeMap::from([ + ("trb0".into(), "tfportrear0_0".into()), + ("trb1".into(), "tfportrear1_0".into()), + ("tqb0".into(), "tfportqsfp0_0".into()), + ("tqb1".into(), "tfportqsfp1_0".into()), + ])); + + println_nopipe!("waiting for zones to come up"); + sleep(Duration::from_secs(10)); + + sidecar_a.setup()?; + sidecar_b.setup()?; + s1.setup(1)?; + s2.setup(2)?; + s3.setup(3)?; + s4.setup(4)?; + t1.setup(5)?; + t2.setup(6)?; + + run_topo!(run_sextet_tests(&s1, &s2, &s3, &s4, &t1, &t2).await)?; + + Ok(()) +} + +async fn run_sextet_tests( + _zs1: &RouterZone<'_>, + _zs2: &RouterZone<'_>, + _zs3: &RouterZone<'_>, + _zs4: &RouterZone<'_>, + _zt1: &RouterZone<'_>, + _zt2: &RouterZone<'_>, +) -> Result<()> { + Ok(()) +} + async fn prefix_count(c: &Client) -> Result { Ok(c.get_prefixes() .await? From 7a25f46b76aeec7afac0d582f65a4d7004fda03d Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Sun, 13 Sep 2026 00:00:39 +0000 Subject: [PATCH 06/29] add tests, fix things uncovered by tests --- ddm/src/admin.rs | 18 ++--- ddm/src/sm/mod.rs | 8 ++- ddm/src/sm/state.rs | 30 +++++++- tests/src/ddm.rs | 169 +++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 211 insertions(+), 14 deletions(-) diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index ecc984299..721c0af5a 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -448,6 +448,13 @@ impl DdmAdminApi for DdmAdminApiImpl { let current = ctx.db.get_external_peers(); let to_create = rq.address_objects.difference(¤t); let to_remove = current.difference(&rq.address_objects); + + info!(ctx.log, "peer change request"; + "requested" => ?rq.address_objects, + "to_create" => ?to_create, + "to_remove" => ?to_remove, + "current" => ?current, + ); ctx.db.set_external_peers(rq.address_objects.clone()); for addr_obj in to_create.into_iter() { @@ -509,14 +516,9 @@ impl DdmAdminApi for DdmAdminApiImpl { } let mut remove_idx = Vec::default(); - info!(ctx.log, "removing peers"; - "to_remove" => ?to_remove, - "current" => ?ctx - .peers.iter().map(|x| &x.config.aobj_name).collect::>(), - ); - for (i, ifx) in to_remove.into_iter().enumerate() { - for p in &ctx.peers { + for ifx in to_remove.into_iter() { + for (i, p) in ctx.peers.iter().enumerate() { if p.config.aobj_name.contains(ifx) { let _ = p.tx.send(Event::Admin(AdminEvent::Shutdown)); remove_idx.push(i); @@ -525,7 +527,7 @@ impl DdmAdminApi for DdmAdminApiImpl { "removing external peeer on interface {ifx}" ); } else { - info!(ctx.log, "{ifx} != {}", p.config.if_name); + info!(ctx.log, "{ifx} != {}", p.config.aobj_name); } } } diff --git a/ddm/src/sm/mod.rs b/ddm/src/sm/mod.rs index c9e6372c3..108e94586 100644 --- a/ddm/src/sm/mod.rs +++ b/ddm/src/sm/mod.rs @@ -290,7 +290,11 @@ pub(crate) fn send(e: Event, event_channels: &mut Vec>) { dead_channels.push(i); } } - for i in dead_channels { - event_channels.remove(i); + // we need to remove in descending order, so we don't remove `i` and then + // try to remove `i+1` later which wlll be a _diffrent_ item than we grabbed + // the index for. Removing from the top down causes no shifting for subsequent + // index removals. + for i in dead_channels.iter().rev() { + event_channels.remove(*i); } } diff --git a/ddm/src/sm/state.rs b/ddm/src/sm/state.rs index d39017a36..91eaadea9 100644 --- a/ddm/src/sm/state.rs +++ b/ddm/src/sm/state.rs @@ -74,6 +74,21 @@ impl State for Init { self.ctx.iface.transition(FsmState::Init); self.ctx.iface.clear_peer(); loop { + // Check for shutdown + while let Ok(e) = event.try_recv() { + match e { + Event::Admin(AdminEvent::Shutdown) => { + return (None, event); + } + _ => { + wrn!( + self.log, + self.ctx.config.aobj_name, + "event unhandled in init state: {e:?}" + ); + } + } + } let info = match get_ipaddr_info(&self.ctx.config.aobj_name) { Ok(info) => info, Err(e) => { @@ -118,15 +133,24 @@ impl State for Init { // Now that we have an ip address to run discovery on, start the // discovery handler and jump into the solicit state. - let discovery_stop = discovery::handler( + let discovery_stop = match discovery::handler( self.ctx.hostname.clone(), self.ctx.config.clone(), self.ctx.tx.clone(), self.ctx.iface.clone(), self.ctx.stats.clone(), self.ctx.log.clone(), - ) - .unwrap(); // TODO unwrap + ) { + Ok(stop) => stop, + Err(e) => { + wrn!( + self.log, + self.ctx.config.if_name, + "failed to start discovery handler: {e}", + ); + continue; + } + }; self.ctx.discovery_stop = Some(discovery_stop); return ( Some(Box::new(Solicit::new( diff --git a/tests/src/ddm.rs b/tests/src/ddm.rs index c40d0acef..5d6d4fea3 100644 --- a/tests/src/ddm.rs +++ b/tests/src/ddm.rs @@ -5,11 +5,13 @@ use anyhow::{Result, anyhow}; use client_common::{eprintln_nopipe, println_nopipe}; use ddm_admin_client::Client; +use ddm_admin_client::types::SetExternalPeers; use ddm_api_types_versions::latest::net::TunnelOrigin; use slog::{Drain, Logger}; use std::collections::BTreeMap; use std::env; use std::net::Ipv6Addr; +use std::ops::{Deref, DerefMut}; use std::thread::sleep; use std::time::Duration; use zone::Zlogin; @@ -855,7 +857,7 @@ async fn run_quartet_tests( Ok(()) } -#[tokio::test] +#[tokio::test(flavor = "multi_thread")] async fn test_external_peer_sextet() -> Result<()> { // A sextet of routers in a multi-rack topology. // @@ -1053,6 +1055,171 @@ async fn run_sextet_tests( _zt1: &RouterZone<'_>, _zt2: &RouterZone<'_>, ) -> Result<()> { + let log = init_logger(); + + // A ddm client that dumps out information when it drops. Primarily used for + // debugging test failures when an assert pops. + struct DropDump { + c: Client, + name: String, + } + impl Deref for DropDump { + type Target = Client; + fn deref(&self) -> &Self::Target { + &self.c + } + } + impl DerefMut for DropDump { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.c + } + } + impl Drop for DropDump { + fn drop(&mut self) { + // Async just loves to make things difficult, it's taken over all + // the things, but heaven forbid you need to do an async thing in + // the most basic of object lifecycle management traits ... + let rt = tokio::runtime::Handle::current(); + let c = self.c.clone(); + let name = self.name.clone(); + tokio::task::block_in_place(|| { + rt.block_on(async move { + println_nopipe!("{name}:"); + if let Ok(peers) = c.get_peers().await { + println_nopipe!("peers: {peers:#?}"); + } + if let Ok(prefixes) = c.get_prefixes().await { + println_nopipe!("prefixes: {prefixes:#?}"); + } + }); + }); + } + } + + macro_rules! drop_dump { + ($name:ident, $endpoint:expr) => { + let $name = DropDump { + c: Client::new($endpoint, log.clone()), + name: stringify!($name).to_string(), + }; + }; + } + + #[derive(Default)] + struct PeerCounts { + s1: usize, + s2: usize, + s3: usize, + s4: usize, + t1: usize, + t2: usize, + } + impl PeerCounts { + fn server(mut self, c: usize) -> Self { + self.s1 = c; + self.s2 = c; + self.s3 = c; + self.s4 = c; + self + } + fn transit(mut self, c: usize) -> Self { + self.t1 = c; + self.t2 = c; + self + } + } + + drop_dump!(s1, "http://10.0.0.1:8000"); + drop_dump!(s2, "http://10.0.0.2:8000"); + drop_dump!(s3, "http://10.0.0.3:8000"); + drop_dump!(s4, "http://10.0.0.4:8000"); + drop_dump!(t1, "http://10.0.0.5:8000"); + drop_dump!(t2, "http://10.0.0.6:8000"); + + // While this would be better as a simple lambda function, when an assert + // pops within we only see the line number here and all that's available + // in RUST_BACKTRACE=1 is a pile of useless tokio noise. + macro_rules! assert_peer_count { + ($client:expr, $count:expr) => {{ + println_nopipe!( + "ensure {} has {} peers", + stringify!($client), + $count + ); + wait_for_eq!( + $client.get_peers().await.map(|x| x.len()).ok(), + Some($count) + ); + }}; + } + + macro_rules! assert_peer_counts { + ($c:expr) => {{ + assert_peer_count!(s1, $c.s1); + assert_peer_count!(s2, $c.s2); + assert_peer_count!(s3, $c.s3); + assert_peer_count!(s4, $c.s4); + assert_peer_count!(t1, $c.t1); + assert_peer_count!(t2, $c.t2); + }}; + } + + // + // Starting out we should have just the backplane peers. + // + + assert_peer_counts!(PeerCounts::default().server(1).transit(2)); + + // + // Specifying two external peers should result in two additional peers for + // each transit router and no changes for the number of server router peers. + // + + let ext_peers_both = SetExternalPeers { + address_objects: ["tfportqsfp0_0/v6", "tfportqsfp1_0/v6"] + .map(String::from) + .to_vec(), + }; + t1.set_external_peers(&ext_peers_both).await?; + t2.set_external_peers(&ext_peers_both).await?; + assert_peer_counts!(PeerCounts::default().server(1).transit(4)); + + // + // Going down to the first peer should result in three peering sessions + // per transit router. Note in the model above that qsfp0/qsfp1 are cross + // connected between the two transit routers. Here we are connecting + // swA/qsfp0 <-> swB/qsfp1 + // + + let ext_peers_qsfp0 = SetExternalPeers { + address_objects: ["tfportqsfp0_0/v6"].map(String::from).to_vec(), + }; + let ext_peers_qsfp1 = SetExternalPeers { + address_objects: ["tfportqsfp1_0/v6"].map(String::from).to_vec(), + }; + t1.set_external_peers(&ext_peers_qsfp0).await?; + t2.set_external_peers(&ext_peers_qsfp1).await?; + assert_peer_counts!(PeerCounts::default().server(1).transit(3)); + + // + // Go back to full peering and then switch to swA/qsfp1 <-> swB/qsfp0 + // + + t1.set_external_peers(&ext_peers_both).await?; + t2.set_external_peers(&ext_peers_both).await?; + assert_peer_counts!(PeerCounts::default().server(1).transit(4)); + t1.set_external_peers(&ext_peers_qsfp1).await?; + t2.set_external_peers(&ext_peers_qsfp0).await?; + assert_peer_counts!(PeerCounts::default().server(1).transit(3)); + + // + // Switch from swA/qsfp1 <-> swB/qsfp0 to swA/qsfp0 <-> swB/qsfp1 + // + + t1.set_external_peers(&ext_peers_qsfp0).await?; + t2.set_external_peers(&ext_peers_qsfp1).await?; + assert_peer_counts!(PeerCounts::default().server(1).transit(3)); + Ok(()) } From 747d7bad49031add116dda112a994cbd14ab5045 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Sun, 13 Sep 2026 04:32:17 +0000 Subject: [PATCH 07/29] add combinatorial tests and fix what that surfaced --- Cargo.lock | 1 + ddm/src/admin.rs | 13 +++++--- ddm/src/sm/mod.rs | 7 +++-- ddm/src/sm/state.rs | 3 ++ tests/Cargo.toml | 1 + tests/src/ddm.rs | 77 ++++++++++++++++++++++++++++++++++++++++----- 6 files changed, 86 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 19b81b40f..ff47c16e3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3958,6 +3958,7 @@ dependencies = [ "ddm-admin-client", "ddm-api-types-versions", "mg-common", + "rand 0.10.2", "slog", "slog-async", "slog-envlogger", diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index 721c0af5a..af0c8a70a 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -34,7 +34,7 @@ use mg_common::lock; use oxnet::Ipv6Net; use slog::{Logger, error, info, o}; use slog_error_chain::InlineErrorChain; -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeSet, HashMap, HashSet}; use std::net::{IpAddr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6}; use std::sync::Arc; use std::sync::Mutex; @@ -515,13 +515,14 @@ impl DdmAdminApi for DdmAdminApiImpl { // TODO oxstats server } - let mut remove_idx = Vec::default(); + // Ensure our indices are unique and ordered. + let mut remove_idx = BTreeSet::default(); for ifx in to_remove.into_iter() { for (i, p) in ctx.peers.iter().enumerate() { if p.config.aobj_name.contains(ifx) { let _ = p.tx.send(Event::Admin(AdminEvent::Shutdown)); - remove_idx.push(i); + remove_idx.insert(i); info!( ctx.log, "removing external peeer on interface {ifx}" @@ -531,8 +532,10 @@ impl DdmAdminApi for DdmAdminApiImpl { } } } - for i in remove_idx { - ctx.peers.remove(i); + // remove peers back to front so we don't shift the order our from under + // ourselves for the indexes we just gathered. + for i in remove_idx.iter().rev() { + ctx.peers.remove(*i); } Ok(HttpResponseUpdatedNoContent()) diff --git a/ddm/src/sm/mod.rs b/ddm/src/sm/mod.rs index 108e94586..98cf7a46a 100644 --- a/ddm/src/sm/mod.rs +++ b/ddm/src/sm/mod.rs @@ -14,7 +14,7 @@ use ddm_api_types::net::TunnelOrigin; use mg_common::lock; use oxnet::Ipv6Net; use slog::Logger; -use std::collections::HashSet; +use std::collections::{BTreeSet, HashSet}; use std::net::Ipv6Addr; use std::sync::atomic::{AtomicBool, AtomicU64}; use std::sync::mpsc::{Receiver, Sender}; @@ -284,10 +284,11 @@ impl StateMachine { } pub(crate) fn send(e: Event, event_channels: &mut Vec>) { - let mut dead_channels = Vec::default(); + // Ensure our indices are unique and ordered. + let mut dead_channels = BTreeSet::default(); for (i, c) in event_channels.iter().enumerate() { if c.send(e.clone()).is_err() { - dead_channels.push(i); + dead_channels.insert(i); } } // we need to remove in descending order, so we don't remove `i` and then diff --git a/ddm/src/sm/state.rs b/ddm/src/sm/state.rs index 91eaadea9..db332ba26 100644 --- a/ddm/src/sm/state.rs +++ b/ddm/src/sm/state.rs @@ -148,6 +148,9 @@ impl State for Init { self.ctx.config.if_name, "failed to start discovery handler: {e}", ); + sleep(Duration::from_millis( + self.ctx.config.solicit_interval, + )); continue; } }; diff --git a/tests/Cargo.toml b/tests/Cargo.toml index 3bc726298..e321e33ff 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -18,3 +18,4 @@ slog-async.workspace = true tokio.workspace = true ztest.workspace = true uuid.workspace = true +rand.workspace = true diff --git a/tests/src/ddm.rs b/tests/src/ddm.rs index 5d6d4fea3..0400d4666 100644 --- a/tests/src/ddm.rs +++ b/tests/src/ddm.rs @@ -235,7 +235,7 @@ impl<'a> RouterZone<'a> { let ddm = "/opt/ddmd"; let extra_args = format!( - "--rack-uuid {} --sled-uuid {}", + "--rack-uuid {} --sled-uuid {} --solicit-interval 200 --expire-threshold 500", uuid::Uuid::new_v4(), uuid::Uuid::new_v4(), ); @@ -1137,8 +1137,9 @@ async fn run_sextet_tests( drop_dump!(t2, "http://10.0.0.6:8000"); // While this would be better as a simple lambda function, when an assert - // pops within we only see the line number here and all that's available - // in RUST_BACKTRACE=1 is a pile of useless tokio noise. + // pops within we only see the line number of the asserting statement in the + // lambda and all that's available in RUST_BACKTRACE=1 is a pile of useless + // tokio noise. macro_rules! assert_peer_count { ($client:expr, $count:expr) => {{ println_nopipe!( @@ -1175,10 +1176,12 @@ async fn run_sextet_tests( // each transit router and no changes for the number of server router peers. // + // The address objects for each qsfp on each switch in the test environment. + const QSFP0: &str = "tfportqsfp0_0/v6"; + const QSFP1: &str = "tfportqsfp1_0/v6"; + let ext_peers_both = SetExternalPeers { - address_objects: ["tfportqsfp0_0/v6", "tfportqsfp1_0/v6"] - .map(String::from) - .to_vec(), + address_objects: [QSFP0, QSFP1].map(String::from).to_vec(), }; t1.set_external_peers(&ext_peers_both).await?; t2.set_external_peers(&ext_peers_both).await?; @@ -1192,10 +1195,10 @@ async fn run_sextet_tests( // let ext_peers_qsfp0 = SetExternalPeers { - address_objects: ["tfportqsfp0_0/v6"].map(String::from).to_vec(), + address_objects: [QSFP0].map(String::from).to_vec(), }; let ext_peers_qsfp1 = SetExternalPeers { - address_objects: ["tfportqsfp1_0/v6"].map(String::from).to_vec(), + address_objects: [QSFP1].map(String::from).to_vec(), }; t1.set_external_peers(&ext_peers_qsfp0).await?; t2.set_external_peers(&ext_peers_qsfp1).await?; @@ -1220,6 +1223,64 @@ async fn run_sextet_tests( t2.set_external_peers(&ext_peers_qsfp1).await?; assert_peer_counts!(PeerCounts::default().server(1).transit(3)); + // + // Go to no external peers + // + + let ext_peers_none = SetExternalPeers { + address_objects: Vec::default(), + }; + t1.set_external_peers(&ext_peers_none).await?; + t2.set_external_peers(&ext_peers_none).await?; + assert_peer_counts!(PeerCounts::default().server(1).transit(2)); + + // + // A bit of combinatorial exercise + // + + fn peer_is_set(x: &SetExternalPeers, s: &str) -> bool { + x.address_objects.contains(&String::from(s)) + } + + fn expected_external_peerings( + x: &SetExternalPeers, + y: &SetExternalPeers, + ) -> PeerCounts { + // The count starts at two because each transit router has two + // backplane connections that we each expect to have a server + // peering session. + let mut ext_count: usize = 2; + if peer_is_set(x, QSFP0) && peer_is_set(y, QSFP1) { + ext_count += 1; + } + if peer_is_set(x, QSFP1) && peer_is_set(y, QSFP0) { + ext_count += 1; + } + PeerCounts::default().server(1).transit(ext_count) + } + + let choices = [ + ext_peers_none, + ext_peers_qsfp0, + ext_peers_qsfp1, + ext_peers_both, + ]; + + const N: usize = 100; + for i in 0..N { + println_nopipe!("{i}/{N}"); + let a: usize = rand::random_range(0..choices.len()); + let b: usize = rand::random_range(0..choices.len()); + + let x = &choices[a]; + let y = &choices[b]; + + t1.set_external_peers(x).await?; + t2.set_external_peers(y).await?; + let counts = expected_external_peerings(x, y); + assert_peer_counts!(counts); + } + Ok(()) } From d1bf043c605a561fe92d6da20818f5a57c48501e Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Sun, 13 Sep 2026 05:51:37 +0000 Subject: [PATCH 08/29] add GET endpoint for external peers --- Cargo.lock | 3 +- Cargo.toml | 2 +- ddm-admin-client/src/lib.rs | 1 + .../src/external_peers/external_peers.rs | 4 +- ddm-api-types/versions/src/latest.rs | 2 +- ddm-api/src/lib.rs | 11 +++- ddm/src/admin.rs | 16 ++++-- ddmadm/Cargo.toml | 1 + ddmadm/src/main.rs | 22 ++++++-- ...0f03b.json => ddm-admin-3.0.0-224165.json} | 53 ++++++++++++------ openapi/ddm-admin/ddm-admin-latest.json | 2 +- tests/src/ddm.rs | 54 ++++++++++++------- 12 files changed, 119 insertions(+), 52 deletions(-) rename openapi/ddm-admin/{ddm-admin-3.0.0-40f03b.json => ddm-admin-3.0.0-224165.json} (96%) diff --git a/Cargo.lock b/Cargo.lock index ff47c16e3..1c2023a88 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1452,6 +1452,7 @@ version = "0.1.0" dependencies = [ "anyhow", "clap", + "client-common", "colored", "ddm-admin-client", "ddm-api-types-versions", @@ -9470,7 +9471,7 @@ dependencies = [ [[package]] name = "ztest" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/falcon?branch=ry%2Fztest-dl-privs#3acbd55db9d280f19c32b03fa93fce1a8fad3571" +source = "git+https://github.com/oxidecomputer/falcon?branch=main#c7952be660468c17c546f1fbaa03cf50c8f904cd" dependencies = [ "anyhow", "libnet", diff --git a/Cargo.toml b/Cargo.toml index 0cafc102a..2bbc6489a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -112,7 +112,7 @@ clap = { version = "4.6.6", features = ["derive", "unstable-styles", "env"] } tabwriter = { version = "1", features = ["ansi_formatting"] } colored = "3.1" strum = { version = "0.28", features = ["derive"] } -ztest = { git = "https://github.com/oxidecomputer/falcon", branch = "ry/ztest-dl-privs" } +ztest = { git = "https://github.com/oxidecomputer/falcon", branch = "main" } libfalcon = { git = "https://github.com/oxidecomputer/falcon", branch = "main" } anstyle = "1.0.14" nom = "8.0" diff --git a/ddm-admin-client/src/lib.rs b/ddm-admin-client/src/lib.rs index 77988121c..53d89ab90 100644 --- a/ddm-admin-client/src/lib.rs +++ b/ddm-admin-client/src/lib.rs @@ -23,5 +23,6 @@ progenitor::generate_api!( PeerInfo = ddm_api_types_versions::latest::db::PeerInfo, PeerStatus = ddm_api_types_versions::latest::db::PeerStatus, Duration = std::time::Duration, + ExternalPeers = ddm_api_types_versions::latest::external_peers::ExternalPeers, } ); diff --git a/ddm-api-types/versions/src/external_peers/external_peers.rs b/ddm-api-types/versions/src/external_peers/external_peers.rs index 14b9f96f4..fea719441 100644 --- a/ddm-api-types/versions/src/external_peers/external_peers.rs +++ b/ddm-api-types/versions/src/external_peers/external_peers.rs @@ -6,7 +6,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::collections::BTreeSet; -#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] -pub struct SetExternalPeers { +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, PartialEq, Eq)] +pub struct ExternalPeers { pub address_objects: BTreeSet, } diff --git a/ddm-api-types/versions/src/latest.rs b/ddm-api-types/versions/src/latest.rs index b9866183f..64fc60327 100644 --- a/ddm-api-types/versions/src/latest.rs +++ b/ddm-api-types/versions/src/latest.rs @@ -27,5 +27,5 @@ pub mod net { } pub mod external_peers { - pub use crate::v3::external_peers::SetExternalPeers; + pub use crate::v3::external_peers::ExternalPeers; } diff --git a/ddm-api/src/lib.rs b/ddm-api/src/lib.rs index 3437d3129..58b179b4c 100644 --- a/ddm-api/src/lib.rs +++ b/ddm-api/src/lib.rs @@ -179,6 +179,15 @@ pub trait DdmAdminApi { }] async fn set_external_peers( ctx: RequestContext, - request: TypedBody, + request: TypedBody, ) -> Result; + + #[endpoint { + method = GET, + path = "/external_peers", + versions = VERSION_EXTERNAL_PEERS.., + }] + async fn get_external_peers( + ctx: RequestContext, + ) -> Result, HttpError>; } diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index af0c8a70a..6ca3e1bc6 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -17,7 +17,7 @@ use ddm_api::ddm_admin_api_mod; use ddm_api_types::admin::{EnableStatsRequest, ExpirePathParams, PrefixMap}; use ddm_api_types::db::{PeerInfo, RouterKind, TunnelRoute}; use ddm_api_types::exchange::PathVector; -use ddm_api_types::external_peers::SetExternalPeers; +use ddm_api_types::external_peers::ExternalPeers; use ddm_api_types::net::TunnelOrigin; use dropshot::ApiDescription; use dropshot::ApiDescriptionBuildErrors; @@ -440,7 +440,7 @@ impl DdmAdminApi for DdmAdminApiImpl { async fn set_external_peers( ctx: RequestContext, - request: TypedBody, + request: TypedBody, ) -> Result { let mut ctx = lock!(ctx.context()); let rq = request.into_inner(); @@ -511,8 +511,6 @@ impl DdmAdminApi for DdmAdminApiImpl { &mut ctx.event_channels, ); ctx.event_channels.push(tx); - - // TODO oxstats server } // Ensure our indices are unique and ordered. @@ -540,6 +538,16 @@ impl DdmAdminApi for DdmAdminApiImpl { Ok(HttpResponseUpdatedNoContent()) } + + async fn get_external_peers( + ctx: RequestContext, + ) -> Result, HttpError> { + let ctx = lock!(ctx.context()); + + Ok(HttpResponseOk(ExternalPeers { + address_objects: ctx.db.get_external_peers(), + })) + } } pub fn api_description() diff --git a/ddmadm/Cargo.toml b/ddmadm/Cargo.toml index cde57b1c4..25341da3f 100644 --- a/ddmadm/Cargo.toml +++ b/ddmadm/Cargo.toml @@ -5,6 +5,7 @@ edition = "2024" [dependencies] mg-common = { path = "../mg-common" } +client-common = { path = "../client-common" } ddm-admin-client = { path = "../ddm-admin-client" } ddm-api-types-versions.workspace = true anyhow.workspace = true diff --git a/ddmadm/src/main.rs b/ddmadm/src/main.rs index c22c9ec1b..60a1145ed 100644 --- a/ddmadm/src/main.rs +++ b/ddmadm/src/main.rs @@ -4,10 +4,11 @@ use anyhow::Result; use clap::Parser; +use client_common::println_nopipe; use colored::*; use ddm_admin_client::Client; -use ddm_admin_client::types::SetExternalPeers; use ddm_api_types_versions::latest::db::PeerStatus; +use ddm_api_types_versions::latest::external_peers::ExternalPeers; use ddm_api_types_versions::latest::net as types; use mg_common::cli::oxide_cli_style; use mg_common::format_duration_human; @@ -67,8 +68,13 @@ enum SubCommand { /// Sync prefix information from peers. Sync, - /// Set external peers as a list of interfaces. - SetExternalPeers { addr_obj: Vec }, + /// Set external peers as a list of address objects. + SetExternalPeers { + addr_obj: Vec, + }, + + // Get external peers + GetExternalPeers, } #[derive(Debug, Parser)] @@ -271,11 +277,17 @@ async fn run() -> Result<()> { } SubCommand::SetExternalPeers { addr_obj } => { client - .set_external_peers(&SetExternalPeers { - address_objects: addr_obj.clone(), + .set_external_peers(&ExternalPeers { + address_objects: addr_obj.iter().cloned().collect(), }) .await?; } + SubCommand::GetExternalPeers => { + let peers = client.get_external_peers().await?.into_inner(); + for p in peers.address_objects { + println_nopipe!("{p}"); + } + } } Ok(()) diff --git a/openapi/ddm-admin/ddm-admin-3.0.0-40f03b.json b/openapi/ddm-admin/ddm-admin-3.0.0-224165.json similarity index 96% rename from openapi/ddm-admin/ddm-admin-3.0.0-40f03b.json rename to openapi/ddm-admin/ddm-admin-3.0.0-224165.json index 6cf57e3c9..408c184f9 100644 --- a/openapi/ddm-admin/ddm-admin-3.0.0-40f03b.json +++ b/openapi/ddm-admin/ddm-admin-3.0.0-224165.json @@ -52,13 +52,34 @@ } }, "/external_peers": { + "get": { + "operationId": "get_external_peers", + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ExternalPeers" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + }, "post": { "operationId": "set_external_peers", "requestBody": { "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/SetExternalPeers" + "$ref": "#/components/schemas/ExternalPeers" } } }, @@ -440,6 +461,21 @@ "request_id" ] }, + "ExternalPeers": { + "type": "object", + "properties": { + "address_objects": { + "type": "array", + "items": { + "type": "string" + }, + "uniqueItems": true + } + }, + "required": [ + "address_objects" + ] + }, "IpNet": { "x-rust-type": { "crate": "oxnet", @@ -616,21 +652,6 @@ 1 ] }, - "SetExternalPeers": { - "type": "object", - "properties": { - "address_objects": { - "type": "array", - "items": { - "type": "string" - }, - "uniqueItems": true - } - }, - "required": [ - "address_objects" - ] - }, "TunnelOrigin": { "type": "object", "properties": { diff --git a/openapi/ddm-admin/ddm-admin-latest.json b/openapi/ddm-admin/ddm-admin-latest.json index 43e4b563f..597684728 120000 --- a/openapi/ddm-admin/ddm-admin-latest.json +++ b/openapi/ddm-admin/ddm-admin-latest.json @@ -1 +1 @@ -ddm-admin-3.0.0-40f03b.json \ No newline at end of file +ddm-admin-3.0.0-224165.json \ No newline at end of file diff --git a/tests/src/ddm.rs b/tests/src/ddm.rs index 0400d4666..43ab22839 100644 --- a/tests/src/ddm.rs +++ b/tests/src/ddm.rs @@ -5,10 +5,10 @@ use anyhow::{Result, anyhow}; use client_common::{eprintln_nopipe, println_nopipe}; use ddm_admin_client::Client; -use ddm_admin_client::types::SetExternalPeers; +use ddm_api_types_versions::latest::external_peers::ExternalPeers; use ddm_api_types_versions::latest::net::TunnelOrigin; use slog::{Drain, Logger}; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::env; use std::net::Ipv6Addr; use std::ops::{Deref, DerefMut}; @@ -234,6 +234,8 @@ impl<'a> RouterZone<'a> { .join(" "); let ddm = "/opt/ddmd"; + + // Tighter solicit interval and expire threshold are to speed up tests. let extra_args = format!( "--rack-uuid {} --sled-uuid {} --solicit-interval 200 --expire-threshold 500", uuid::Uuid::new_v4(), @@ -252,10 +254,6 @@ impl<'a> RouterZone<'a> { self.zone.zexec( "svccfg -s dendrite setprop config/uds_path = /opt/mnt", )?; - /* - self.zone.zexec( - "svccfg -s dendrite setprop config/port_config = /opt/dpd-ports.toml")?; - */ self.zone.zexec(&format!( "svccfg -s dendrite setprop config/front_ports = {}", front_ports.len(), @@ -1180,8 +1178,8 @@ async fn run_sextet_tests( const QSFP0: &str = "tfportqsfp0_0/v6"; const QSFP1: &str = "tfportqsfp1_0/v6"; - let ext_peers_both = SetExternalPeers { - address_objects: [QSFP0, QSFP1].map(String::from).to_vec(), + let ext_peers_both = ExternalPeers { + address_objects: [QSFP0, QSFP1].map(String::from).into(), }; t1.set_external_peers(&ext_peers_both).await?; t2.set_external_peers(&ext_peers_both).await?; @@ -1194,11 +1192,11 @@ async fn run_sextet_tests( // swA/qsfp0 <-> swB/qsfp1 // - let ext_peers_qsfp0 = SetExternalPeers { - address_objects: [QSFP0].map(String::from).to_vec(), + let ext_peers_qsfp0 = ExternalPeers { + address_objects: [QSFP0].map(String::from).into(), }; - let ext_peers_qsfp1 = SetExternalPeers { - address_objects: [QSFP1].map(String::from).to_vec(), + let ext_peers_qsfp1 = ExternalPeers { + address_objects: [QSFP1].map(String::from).into(), }; t1.set_external_peers(&ext_peers_qsfp0).await?; t2.set_external_peers(&ext_peers_qsfp1).await?; @@ -1227,8 +1225,8 @@ async fn run_sextet_tests( // Go to no external peers // - let ext_peers_none = SetExternalPeers { - address_objects: Vec::default(), + let ext_peers_none = ExternalPeers { + address_objects: BTreeSet::default(), }; t1.set_external_peers(&ext_peers_none).await?; t2.set_external_peers(&ext_peers_none).await?; @@ -1238,18 +1236,21 @@ async fn run_sextet_tests( // A bit of combinatorial exercise // - fn peer_is_set(x: &SetExternalPeers, s: &str) -> bool { + fn peer_is_set(x: &ExternalPeers, s: &str) -> bool { x.address_objects.contains(&String::from(s)) } fn expected_external_peerings( - x: &SetExternalPeers, - y: &SetExternalPeers, + x: &ExternalPeers, + y: &ExternalPeers, ) -> PeerCounts { - // The count starts at two because each transit router has two - // backplane connections that we each expect to have a server - // peering session. + // The count starts at two because each transit router has two backplane + // connections that we each expect to have a server peering session on. let mut ext_count: usize = 2; + + // A peering is expected when qsfp0 and qsfp1 are configured as an + // external router in either direction. This is a property of the + // testing topology (see diagram in test_external_peer_sextet). if peer_is_set(x, QSFP0) && peer_is_set(y, QSFP1) { ext_count += 1; } @@ -1259,6 +1260,8 @@ async fn run_sextet_tests( PeerCounts::default().server(1).transit(ext_count) } + // The choices we have for each switch are none, one or both peers where the + // one case can be either of the peers. let choices = [ ext_peers_none, ext_peers_qsfp0, @@ -1266,6 +1269,11 @@ async fn run_sextet_tests( ext_peers_both, ]; + // Go through 100 rounds. Ideally we'd have more than this, but peer + // expiration and re-establishment is currently a second or two, so at 100 + // rounds this is already taking over a minute. It'd be nice to have really + // quick peering timer settings for tests so we can rapidly iterate through + // sweeps like this. const N: usize = 100; for i in 0..N { println_nopipe!("{i}/{N}"); @@ -1279,6 +1287,12 @@ async fn run_sextet_tests( t2.set_external_peers(y).await?; let counts = expected_external_peerings(x, y); assert_peer_counts!(counts); + + let xx = t1.get_external_peers().await?.into_inner(); + let yy = t2.get_external_peers().await?.into_inner(); + + assert_eq!(x, &xx, "t1 reports different peers than we set"); + assert_eq!(y, &yy, "t2 reports different peers than we set"); } Ok(()) From bf29b37a3842c640a778579605c09fb1e66408a8 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Sun, 13 Sep 2026 17:22:45 +0000 Subject: [PATCH 09/29] plumb stats so they work with dynamic external peers --- ddm/src/admin.rs | 12 +++++---- ddm/src/discovery/runtime.rs | 1 - ddm/src/oxstats.rs | 21 +++++++-------- ddm/src/sm/mod.rs | 2 ++ ddmd/src/main.rs | 51 +++++++++++++++++------------------- ddmd/src/smf.rs | 10 ++++--- 6 files changed, 49 insertions(+), 48 deletions(-) diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index 6ca3e1bc6..8b45f6651 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -396,9 +396,12 @@ impl DdmAdminApi for DdmAdminApiImpl { request: TypedBody, ) -> Result { let rq = request.into_inner(); - let ctx = lock!(ctx.context()); + let (jh, log) = { + let ctx = lock!(ctx.context()); + (ctx.stats_handler.clone(), ctx.log.clone()) + }; - let mut jh = lock!(ctx.stats_handler); + let mut jh = lock!(jh); if jh.is_none() { let hostname = hostname::get() .expect("failed to get hostname") @@ -407,12 +410,11 @@ impl DdmAdminApi for DdmAdminApiImpl { *jh = Some( crate::oxstats::start_server( DDM_STATS_PORT, - ctx.peers.clone(), - ctx.stats.clone(), + ctx.context().clone(), hostname, rq.rack_id, rq.sled_id, - ctx.log.clone(), + log, ) .map_err(|e| { HttpError::for_internal_error(format!( diff --git a/ddm/src/discovery/runtime.rs b/ddm/src/discovery/runtime.rs index 165e578a2..7f6be155b 100644 --- a/ddm/src/discovery/runtime.rs +++ b/ddm/src/discovery/runtime.rs @@ -123,7 +123,6 @@ pub(crate) fn handler( let uc_sa: SockAddr = SocketAddrV6::new(config.addr, DDM_PORT, 0, config.if_index).into(); uc.bind(&uc_sa)?; - //uc.set_reuse_address(true)?; uc.set_read_timeout(Some(Duration::from_millis( config.discovery_read_timeout, )))?; diff --git a/ddm/src/oxstats.rs b/ddm/src/oxstats.rs index a641d728f..a1a5babfa 100644 --- a/ddm/src/oxstats.rs +++ b/ddm/src/oxstats.rs @@ -2,7 +2,7 @@ // 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/. -use crate::{admin::RouterStats, sm::SmContext}; +use crate::admin::HandlerContext; use chrono::{DateTime, Utc}; use mg_common::{ lock, @@ -15,7 +15,7 @@ use oximeter::{ }; use oximeter_producer::{ConfigLogging, ConfigLoggingLevel, LogConfig}; use slog::Logger; -use std::sync::atomic::Ordering; +use std::sync::{Mutex, atomic::Ordering}; use std::{net::SocketAddr, sync::Arc, time::Duration}; use tokio::task::JoinHandle; use uuid::Uuid; @@ -46,8 +46,7 @@ pub(crate) struct Stats { hostname: String, rack_id: Uuid, sled_id: Uuid, - peers: Vec, - router_stats: Arc, + ctx: Arc>, } macro_rules! ddm_session_counter { @@ -135,12 +134,14 @@ impl Producer for Stats { // level stats. let mut samples: Vec = Vec::with_capacity(2 + 13); + let ctx = lock!(self.ctx); + samples.push(ddm_router_quantity!( self.hostname.clone().into(), self.rack_id, self.sled_id, OriginatedUnderlayPrefixes, - self.router_stats.originated_underlay_prefixes + ctx.stats.originated_underlay_prefixes )); samples.push(ddm_router_quantity!( @@ -148,10 +149,10 @@ impl Producer for Stats { self.rack_id, self.sled_id, OriginatedTunnelEndpoints, - self.router_stats.originated_tunnel_endpoints + ctx.stats.originated_tunnel_endpoints )); - for peer in &self.peers { + for peer in &ctx.peers { let if_name = lock!(peer.iface.if_name).clone(); samples.push(ddm_session_counter!( self.start_time, @@ -268,8 +269,7 @@ impl Producer for Stats { #[allow(clippy::too_many_arguments)] pub fn start_server( port: u16, - peers: Vec, - router_stats: Arc, + ctx: Arc>, hostname: String, rack_id: Uuid, sled_id: Uuid, @@ -284,11 +284,10 @@ pub fn start_server( let stats_producer = Stats { start_time: chrono::offset::Utc::now(), - peers, + ctx, hostname, rack_id, sled_id, - router_stats, }; registry.register_producer(stats_producer).unwrap(); diff --git a/ddm/src/sm/mod.rs b/ddm/src/sm/mod.rs index 98cf7a46a..5cade77f1 100644 --- a/ddm/src/sm/mod.rs +++ b/ddm/src/sm/mod.rs @@ -283,6 +283,8 @@ impl StateMachine { } } +/// Send an event to all channels in the list, removing any channels from the +/// list that are dead. pub(crate) fn send(e: Event, event_channels: &mut Vec>) { // Ensure our indices are unique and ordered. let mut dead_channels = BTreeSet::default(); diff --git a/ddmd/src/main.rs b/ddmd/src/main.rs index 1617dd54e..65cfd6ba2 100644 --- a/ddmd/src/main.rs +++ b/ddmd/src/main.rs @@ -17,6 +17,7 @@ use ddm::sm::{DpdConfig, SmContext, StateMachine}; #[cfg(all(feature = "backend", target_os = "illumos"))] use ddm::sys::Route; use ddm_api_types::db::RouterKind; +use mg_common::lock; use signal::handle_signals; use slog::{Drain, Logger, error}; use std::net::{IpAddr, Ipv6Addr}; @@ -172,41 +173,37 @@ async fn run() { let router_stats = Arc::new(RouterStats::default()); let peers: Vec = sms.iter().map(|x| x.ctx.clone()).collect(); - let stats_handler = if arg.with_stats { - if let (Some(rack_uuid), Some(sled_uuid)) = - (arg.rack_uuid, arg.sled_uuid) - { - match ddm::oxstats::start_server( - arg.oximeter_port, - peers.clone(), - router_stats.clone(), - hostname.clone(), - rack_uuid, - sled_uuid, - log.clone(), - ) { - Ok(handler) => Some(handler), - Err(e) => { - error!(log, "failed to start stats server: {e}"); - None - } - } - } else { - None - } - } else { - None - }; - let context = Arc::new(Mutex::new(HandlerContext { event_channels, db, stats: router_stats, peers, - stats_handler: Arc::new(Mutex::new(stats_handler)), + stats_handler: Arc::new(Mutex::new(None)), log: log.clone(), })); + if arg.with_stats + && let (Some(rack_uuid), Some(sled_uuid)) = + (arg.rack_uuid, arg.sled_uuid) + { + let h = match ddm::oxstats::start_server( + arg.oximeter_port, + context.clone(), + hostname.clone(), + rack_uuid, + sled_uuid, + log.clone(), + ) { + Ok(handler) => Some(handler), + Err(e) => { + error!(log, "failed to start stats server: {e}"); + None + } + }; + let ctx = lock!(context); + *lock!(ctx.stats_handler) = h; + } + if let Err(e) = sig_tx.send(context.clone()).await { error!(log, "send context to signal handler {e}"); } diff --git a/ddmd/src/smf.rs b/ddmd/src/smf.rs index 8bcfb69e8..353a13e86 100644 --- a/ddmd/src/smf.rs +++ b/ddmd/src/smf.rs @@ -60,14 +60,16 @@ fn refresh_stats_server( } }; - let context = lock!(ctx); - let mut handler = lock!(context.stats_handler); + let (handler, log) = { + let ctx = lock!(ctx); + (ctx.stats_handler.clone(), ctx.log.clone()) + }; + let mut handler = lock!(handler); if handler.is_none() { info!(log, "starting stats server on smf refresh"); match ddm::oxstats::start_server( DDM_STATS_PORT, - context.peers.clone(), - context.stats.clone(), + ctx.clone(), hostname, props.rack_uuid, props.sled_uuid, From 43a8b77b1e205705f03a49464a9eca42e49f0eae Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Mon, 14 Sep 2026 19:40:55 +0000 Subject: [PATCH 10/29] add redistribution tests and fix issues they uncovered --- Cargo.lock | 1 + ddm-protocol/src/v3.rs | 16 ++++++ ddm/src/admin.rs | 10 ++-- ddm/src/db.rs | 10 ++++ ddm/src/exchange/runtime.rs | 14 ++++-- ddm/src/sm/mod.rs | 1 + ddm/src/sm/state.rs | 50 ++++++++++++++++++- ddmd/src/main.rs | 1 + tests/Cargo.toml | 1 + tests/src/ddm.rs | 99 +++++++++++++++++++++++++++++++++++++ 10 files changed, 193 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1c2023a88..f147d6570 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3959,6 +3959,7 @@ dependencies = [ "ddm-admin-client", "ddm-api-types-versions", "mg-common", + "oxnet", "rand 0.10.2", "slog", "slog-async", diff --git a/ddm-protocol/src/v3.rs b/ddm-protocol/src/v3.rs index 80cfcb925..0420d8b30 100644 --- a/ddm-protocol/src/v3.rs +++ b/ddm-protocol/src/v3.rs @@ -117,6 +117,22 @@ impl UnderlayUpdate { .collect(), } } + pub fn break_loops(&self, hostname: &String) -> Self { + Self { + announce: self + .announce + .iter() + .filter(|x| !x.path.contains(hostname)) + .cloned() + .collect(), + withdraw: self + .withdraw + .iter() + .filter(|x| !x.path.contains(hostname)) + .cloned() + .collect(), + } + } } impl From for Update { diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index 8b45f6651..0cffc016d 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -498,6 +498,7 @@ impl DdmAdminApi for DdmAdminApiImpl { iface: Arc::new(InterfaceState::external()), stats: Arc::new(SessionStats::default()), discovery_stop: None, + first_run: true, }; let mut sm = StateMachine { ctx: sm_ctx.clone(), @@ -508,10 +509,11 @@ impl DdmAdminApi for DdmAdminApiImpl { ctx.peers.push(sm_ctx.clone()); - crate::sm::send( - Event::Admin(AdminEvent::NewExternalPeer(tx.clone())), - &mut ctx.event_channels, - ); + // XXX needs to happen once peer is in exchange? + // crate::sm::send( + // Event::Admin(AdminEvent::NewExternalPeer(tx.clone())), + // &mut ctx.event_channels, + // ); ctx.event_channels.push(tx); } diff --git a/ddm/src/db.rs b/ddm/src/db.rs index deab0ec07..03d498847 100644 --- a/ddm/src/db.rs +++ b/ddm/src/db.rs @@ -4,6 +4,7 @@ use ddm_api_types::db::TunnelRoute; use ddm_api_types::net::TunnelOrigin; +use ddm_protocol::v3::PathVector; use mg_common::lock; use oxnet::{IpNet, Ipv6Net}; use schemars::JsonSchema; @@ -282,6 +283,15 @@ pub struct Route { pub path: Vec, } +impl From for PathVector { + fn from(val: Route) -> Self { + Self { + destination: val.destination, + path: val.path, + } + } +} + #[derive(Debug, Clone)] pub enum EffectiveTunnelRouteSet { /// The routes in the contained set are active with priority greater than diff --git a/ddm/src/exchange/runtime.rs b/ddm/src/exchange/runtime.rs index ebd9257ce..8eabfd3d9 100644 --- a/ddm/src/exchange/runtime.rs +++ b/ddm/src/exchange/runtime.rs @@ -550,10 +550,11 @@ fn handle_update( ctx.event_channels.len() ); - let underlay = update - .underlay - .as_ref() - .map(|update| update.with_path_element(ctx.hostname.clone())); + let underlay = update.underlay.as_ref().map(|update| { + update + .break_loops(&ctx.hostname) + .with_path_element(ctx.hostname.clone()) + }); let push = v3::Update { underlay, @@ -646,6 +647,11 @@ fn handle_underlay_update( let db = &ctx.db; for prefix in &update.announce { + // Skip announcements with ourselves in the path e.g. path vector + // loop breaking. + if prefix.path.contains(&ctx.hostname) { + continue; + } import.insert(Route { destination: prefix.destination, nexthop: peer_addr, diff --git a/ddm/src/sm/mod.rs b/ddm/src/sm/mod.rs index 5cade77f1..484820d06 100644 --- a/ddm/src/sm/mod.rs +++ b/ddm/src/sm/mod.rs @@ -269,6 +269,7 @@ pub struct SmContext { pub stats: Arc, pub log: Logger, pub discovery_stop: Option>, + pub first_run: bool, } pub struct StateMachine { diff --git a/ddm/src/sm/state.rs b/ddm/src/sm/state.rs index db332ba26..20fa19885 100644 --- a/ddm/src/sm/state.rs +++ b/ddm/src/sm/state.rs @@ -340,7 +340,7 @@ impl Exchange { ) { exchange_thread.abort(); self.ctx.iface.clear_peer(); - let (to_remove, to_remove_tnl) = + let (mut to_remove, to_remove_tnl) = self.ctx.db.remove_nexthop_routes(self.peer); let mut routes: Vec = Vec::new(); for x in &to_remove { @@ -377,6 +377,24 @@ impl Exchange { self.ctx.event_channels.len() ); + // Only send withdraws for expirations that result in a total loss + // of reachability to a destination. + let imported = self.ctx.db.imported(); + dbg!(self.log, self.ctx.config.if_name, "imported: {imported:#?}"); + dbg!( + self.log, + self.ctx.config.if_name, + "to_remove: {to_remove:#?}" + ); + to_remove.retain(|x| { + !imported.iter().any(|y| y.destination == x.destination) + }); + dbg!( + self.log, + self.ctx.config.if_name, + "to_remove (retained): {to_remove:#?}" + ); + let underlay = if to_remove.is_empty() { None } else { @@ -449,6 +467,14 @@ impl State for Exchange { // loop below. self.initial_pull(pull_stop.clone()); + if self.ctx.iface.external && self.ctx.first_run { + self.ctx.first_run = false; + crate::sm::send( + Event::Admin(AdminEvent::NewExternalPeer(self.ctx.tx.clone())), + &mut self.ctx.event_channels, + ); + } + loop { let e = match event.recv() { Ok(e) => e, @@ -650,7 +676,27 @@ impl State for Exchange { } } Event::Admin(AdminEvent::NewExternalPeer(tx)) => { - self.ctx.event_channels.push(tx); + if tx + .send(Event::Peer(PeerEvent::Push(Update { + underlay: Some( + UnderlayUpdate { + announce: self + .ctx + .db + .imported() + .into_iter() + .map(Into::into) + .collect(), + withdraw: HashSet::default(), + } + .with_path_element(self.ctx.hostname.clone()), + ), + tunnel: None, + }))) + .is_ok() + { + self.ctx.event_channels.push(tx.clone()); + } } Event::Admin(AdminEvent::Shutdown) => { if let Some(x) = self.ctx.discovery_stop.as_mut() { diff --git a/ddmd/src/main.rs b/ddmd/src/main.rs index 65cfd6ba2..7c78dc15b 100644 --- a/ddmd/src/main.rs +++ b/ddmd/src/main.rs @@ -275,6 +275,7 @@ fn start_state_machines( iface: Arc::new(InterfaceState::default()), stats: Arc::new(ddm::sm::SessionStats::default()), discovery_stop: None, + first_run: true, }; let sm = StateMachine { ctx, rx: Some(rx) }; diff --git a/tests/Cargo.toml b/tests/Cargo.toml index e321e33ff..6e08b83b7 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -19,3 +19,4 @@ tokio.workspace = true ztest.workspace = true uuid.workspace = true rand.workspace = true +oxnet.workspace = true diff --git a/tests/src/ddm.rs b/tests/src/ddm.rs index 43ab22839..56a30585f 100644 --- a/tests/src/ddm.rs +++ b/tests/src/ddm.rs @@ -7,6 +7,7 @@ use client_common::{eprintln_nopipe, println_nopipe}; use ddm_admin_client::Client; use ddm_api_types_versions::latest::external_peers::ExternalPeers; use ddm_api_types_versions::latest::net::TunnelOrigin; +use oxnet::Ipv6Net; use slog::{Drain, Logger}; use std::collections::{BTreeMap, BTreeSet}; use std::env; @@ -48,6 +49,12 @@ macro_rules! softnpu_dump { }}; } +macro_rules! ip6_net { + ($x:expr) => { + $x.parse().unwrap() + }; +} + const ZONE_BRAND: &str = "omicron1"; struct SoftnpuZone<'a> { @@ -1127,6 +1134,13 @@ async fn run_sextet_tests( } } + struct PeerReachablePrefixes { + s1: BTreeSet, + s2: BTreeSet, + s3: BTreeSet, + s4: BTreeSet, + } + drop_dump!(s1, "http://10.0.0.1:8000"); drop_dump!(s2, "http://10.0.0.2:8000"); drop_dump!(s3, "http://10.0.0.3:8000"); @@ -1163,6 +1177,55 @@ async fn run_sextet_tests( }}; } + macro_rules! assert_peer_reach { + ($client:expr, $reach:expr) => {{ + println_nopipe!( + "ensure {} has imported prefixes {:?}", + stringify!($client), + $reach + ); + wait_for_eq!( + $client + .get_prefixes() + .await + .map(|x| x + .values() + .cloned() + .into_iter() + .flat_map(|x| x + .clone() + .into_iter() + .map(|y| y.destination)) + .collect::>()) + .ok(), + Some($reach) + ); + }}; + } + + macro_rules! assert_reach { + ($r:expr) => {{ + assert_peer_reach!(s1, $r.s1.clone()); + assert_peer_reach!(s2, $r.s2.clone()); + assert_peer_reach!(s3, $r.s3.clone()); + assert_peer_reach!(s4, $r.s4.clone()); + }}; + } + + // + // Initialize announcements for each server peer + // + + let s1_origin: Vec = [ip6_net!("fd00:1::/64")].into(); + let s2_origin: Vec = [ip6_net!("fd00:2::/64")].into(); + let s3_origin: Vec = [ip6_net!("fd00:3::/64")].into(); + let s4_origin: Vec = [ip6_net!("fd00:4::/64")].into(); + + s1.advertise_prefixes(&s1_origin).await?; + s2.advertise_prefixes(&s2_origin).await?; + s3.advertise_prefixes(&s3_origin).await?; + s4.advertise_prefixes(&s4_origin).await?; + // // Starting out we should have just the backplane peers. // @@ -1260,6 +1323,39 @@ async fn run_sextet_tests( PeerCounts::default().server(1).transit(ext_count) } + let expected_reachable_prefixes = + |x: &ExternalPeers, y: &ExternalPeers| -> PeerReachablePrefixes { + let counts = expected_external_peerings(x, y); + // Servers can always see the originated prefixes of other routers + // reachable over a single hop transit router path (e.g. in the same + // rack). + let mut reach = PeerReachablePrefixes { + s1: s2_origin.iter().cloned().collect(), + s2: s1_origin.iter().cloned().collect(), + s3: s4_origin.iter().cloned().collect(), + s4: s3_origin.iter().cloned().collect(), + }; + // If there is any peering between transit routers, each server router + // should see prefixes originated from the router adjacent to their + // transit router. + if counts.t1 > 2 && counts.t2 > 2 { + // Origins from servers connected to t2 propagating to servers + // connected to t1. + reach.s1.extend(&s3_origin); + reach.s1.extend(&s4_origin); + reach.s2.extend(&s3_origin); + reach.s2.extend(&s4_origin); + + // Origins from servers connected to t1 propagating to servers + // connected to t2. + reach.s3.extend(&s1_origin); + reach.s3.extend(&s2_origin); + reach.s4.extend(&s1_origin); + reach.s4.extend(&s2_origin); + } + reach + }; + // The choices we have for each switch are none, one or both peers where the // one case can be either of the peers. let choices = [ @@ -1293,6 +1389,9 @@ async fn run_sextet_tests( assert_eq!(x, &xx, "t1 reports different peers than we set"); assert_eq!(y, &yy, "t2 reports different peers than we set"); + + let reach = expected_reachable_prefixes(x, y); + assert_reach!(reach); } Ok(()) From 9eea2ce5c0bdd980d87a20ef138fb00c4f3ccbdd Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Mon, 14 Sep 2026 23:03:46 +0000 Subject: [PATCH 11/29] allow routes on qsfp through to dpd --- ddm/src/admin.rs | 5 ----- ddm/src/sys.rs | 47 +++++++++++++++++++++++++++++++---------------- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index 0cffc016d..bfbd845f2 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -509,11 +509,6 @@ impl DdmAdminApi for DdmAdminApiImpl { ctx.peers.push(sm_ctx.clone()); - // XXX needs to happen once peer is in exchange? - // crate::sm::send( - // Event::Admin(AdminEvent::NewExternalPeer(tx.clone())), - // &mut ctx.event_channels, - // ); ctx.event_channels.push(tx); } diff --git a/ddm/src/sys.rs b/ddm/src/sys.rs index c251ab600..48f13b8dc 100644 --- a/ddm/src/sys.rs +++ b/ddm/src/sys.rs @@ -178,23 +178,38 @@ pub fn add_routes_dendrite( // TODO this is gross, use link type properties rather than futzing // around with strings. - let Some(egress_port_num) = ifname - .strip_prefix("tfportrear") - .and_then(|x| x.strip_suffix("_0")) - .map(|x| x.trim()) - .and_then(|x| x.parse::().ok()) - else { - err!(log, ifname, "expected tfportrear"); - continue; - }; - // TODO this assumes ddm only operates on rear ports, which will not be - // true for multi-rack deployments. - let port_name = format!("rear{}", egress_port_num); - let port_id = match types::Rear::try_from(&port_name) { - Ok(rear) => PortId::Rear(rear), - Err(e) => { - err!(log, ifname, "bad port name ({port_name}): {e}"); + let port_id = { + if let Some(egress_port_num) = ifname + .strip_prefix("tfportrear") + .and_then(|x| x.strip_suffix("_0")) + .map(|x| x.trim()) + .and_then(|x| x.parse::().ok()) + { + let port_name = format!("rear{}", egress_port_num); + match types::Rear::try_from(&port_name) { + Ok(rear) => PortId::Rear(rear), + Err(e) => { + err!(log, ifname, "bad port name ({port_name}): {e}"); + continue; + } + } + } else if let Some(egress_port_num) = ifname + .strip_prefix("tfportqsfp") + .and_then(|x| x.strip_suffix("_0")) + .map(|x| x.trim()) + .and_then(|x| x.parse::().ok()) + { + let port_name = format!("qsfp{}", egress_port_num); + match types::Qsfp::try_from(&port_name) { + Ok(qsfp) => PortId::Qsfp(qsfp), + Err(e) => { + err!(log, ifname, "bad port name ({port_name}): {e}"); + continue; + } + } + } else { + err!(log, ifname, "expected tfportrear or tfportqsfp"); continue; } }; From b9000765c47a5f7b613bc30b61e2ded036b20017 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Tue, 15 Sep 2026 02:38:10 +0000 Subject: [PATCH 12/29] limit ext peer removal to ext interfaces, plumb tunables --- ddm/src/admin.rs | 37 +++++++++++++++++++++++++++++------- ddm/src/discovery/runtime.rs | 22 ++++++++------------- ddm/src/exchange/runtime.rs | 4 +--- ddm/src/sm/mod.rs | 13 ++++++------- ddm/src/sm/state.rs | 10 ++++------ ddmd/src/main.rs | 24 +++++++++++++++++------ 6 files changed, 67 insertions(+), 43 deletions(-) diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index bfbd845f2..5dfbf5ad2 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -5,7 +5,7 @@ use crate::db::Db; use crate::defaults::{ DISCOVERY_READ_TIMEOUT, EXCHANGE_TCP_PORT, EXCHANGE_TIMEOUT, - EXPIRE_THRESHOLD, IP_ADDR_WAIT, SOLICIT_INTERVAL, millis_u64, + EXPIRE_THRESHOLD, IP_ADDR_WAIT, SOLICIT_INTERVAL, }; use crate::sm::{ AdminEvent, Event, InterfaceState, PrefixSet, SessionStats, SmContext, @@ -40,6 +40,7 @@ use std::sync::Arc; use std::sync::Mutex; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::mpsc::{Sender, channel}; +use std::time::Duration; use tokio::spawn; use tokio::task::JoinHandle; @@ -60,9 +61,31 @@ pub struct HandlerContext { pub stats: Arc, pub peers: Vec, pub stats_handler: Arc>>>, + pub tunables: Tunables, pub log: Logger, } +#[derive(Clone)] +pub struct Tunables { + pub solicit_interval: Duration, + pub expire_threshold: Duration, + pub discovery_read_timeout: Duration, + pub ip_addr_wait: Duration, + pub exchange_timeout: Duration, +} + +impl Default for Tunables { + fn default() -> Self { + Self { + solicit_interval: SOLICIT_INTERVAL, + expire_threshold: EXPIRE_THRESHOLD, + discovery_read_timeout: DISCOVERY_READ_TIMEOUT, + ip_addr_wait: IP_ADDR_WAIT, + exchange_timeout: EXCHANGE_TIMEOUT, + } + } +} + pub fn handler( addr: IpAddr, port: u16, @@ -463,11 +486,11 @@ impl DdmAdminApi for DdmAdminApiImpl { let (tx, rx) = channel(); let config = crate::sm::Config { - solicit_interval: millis_u64(SOLICIT_INTERVAL), - expire_threshold: millis_u64(EXPIRE_THRESHOLD), - discovery_read_timeout: millis_u64(DISCOVERY_READ_TIMEOUT), - ip_addr_wait: millis_u64(IP_ADDR_WAIT), - exchange_timeout: millis_u64(EXCHANGE_TIMEOUT), + solicit_interval: ctx.tunables.solicit_interval, + expire_threshold: ctx.tunables.expire_threshold, + discovery_read_timeout: ctx.tunables.discovery_read_timeout, + ip_addr_wait: ctx.tunables.ip_addr_wait, + exchange_timeout: ctx.tunables.exchange_timeout, exchange_port: EXCHANGE_TCP_PORT, aobj_name: addr_obj.clone(), if_name: String::default(), // initialized in state machine @@ -517,7 +540,7 @@ impl DdmAdminApi for DdmAdminApiImpl { for ifx in to_remove.into_iter() { for (i, p) in ctx.peers.iter().enumerate() { - if p.config.aobj_name.contains(ifx) { + if p.iface.external && p.config.aobj_name.contains(ifx) { let _ = p.tx.send(Event::Admin(AdminEvent::Shutdown)); remove_idx.insert(i); info!( diff --git a/ddm/src/discovery/runtime.rs b/ddm/src/discovery/runtime.rs index 7f6be155b..3ef839d50 100644 --- a/ddm/src/discovery/runtime.rs +++ b/ddm/src/discovery/runtime.rs @@ -23,7 +23,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::Sender; use std::sync::{Arc, RwLock}; use std::thread::{sleep, spawn}; -use std::time::{Duration, Instant}; +use std::time::Instant; const DDM_MADDR: Ipv6Addr = Ipv6Addr::new(0xff02, 0, 0, 0, 0, 0, 0, 0xdd); const DDM_PORT: u16 = 0xddd; @@ -116,16 +116,12 @@ pub(crate) fn handler( mc.bind(&mc_sa)?; mc.join_multicast_v6(&DDM_MADDR, config.if_index)?; mc.set_multicast_loop_v6(false)?; - mc.set_read_timeout(Some(Duration::from_millis( - config.discovery_read_timeout, - )))?; + mc.set_read_timeout(Some(config.discovery_read_timeout))?; let uc_sa: SockAddr = SocketAddrV6::new(config.addr, DDM_PORT, 0, config.if_index).into(); uc.bind(&uc_sa)?; - uc.set_read_timeout(Some(Duration::from_millis( - config.discovery_read_timeout, - )))?; + uc.set_read_timeout(Some(config.discovery_read_timeout))?; let ctx = HandlerContext { mc_socket: Arc::new(mc), @@ -175,7 +171,7 @@ fn send_solicitations( break; } stats.solicitations_sent.fetch_add(1, Ordering::Relaxed); - sleep(Duration::from_millis(ctx.config.solicit_interval)); + sleep(ctx.config.solicit_interval); } }); } @@ -201,7 +197,7 @@ fn expire( }; if let Some(nbr) = &*guard { let dt = Instant::now().duration_since(nbr.last_seen); - if dt.as_millis() > u128::from(ctx.config.expire_threshold) { + if dt > ctx.config.expire_threshold { wrn!( &ctx.log, ctx.config.if_name, @@ -216,9 +212,7 @@ fn expire( ctx.log.clone(), &ctx.config.if_name, ); - } else if dt.as_millis() - > u128::from(ctx.config.solicit_interval) - { + } else if dt > ctx.config.solicit_interval { wrn!( &ctx.log, ctx.config.if_name, @@ -245,11 +239,11 @@ fn expire( let wait = ctx.config.discovery_read_timeout; drop(ctx); // Ensure read handlers have registered the stop event. - sleep(Duration::from_millis(wait)); + sleep(wait); emit_solicit_fail(event, log, &if_name); break; } - sleep(Duration::from_millis(ctx.config.solicit_interval)); + sleep(ctx.config.solicit_interval); } }); Ok(()) diff --git a/ddm/src/exchange/runtime.rs b/ddm/src/exchange/runtime.rs index 8eabfd3d9..ba3578542 100644 --- a/ddm/src/exchange/runtime.rs +++ b/ddm/src/exchange/runtime.rs @@ -238,9 +238,7 @@ fn send_update_common( let resp = client.request(req); rt.block_on(async move { - match timeout(Duration::from_millis(config.exchange_timeout), resp) - .await - { + match timeout(config.exchange_timeout, resp).await { Ok(_) => Ok(()), Err(e) => { err!( diff --git a/ddm/src/sm/mod.rs b/ddm/src/sm/mod.rs index 484820d06..a9f16f3ec 100644 --- a/ddm/src/sm/mod.rs +++ b/ddm/src/sm/mod.rs @@ -131,22 +131,21 @@ pub struct Config { /// Link local Ipv6 address this state machine is associated with pub addr: Ipv6Addr, - /// How long to wait between solicitations (milliseconds). - pub solicit_interval: u64, + /// How long to wait between solicitations. + pub solicit_interval: Duration, /// How often to check for link failure while waiting for discovery messges. - pub discovery_read_timeout: u64, + pub discovery_read_timeout: Duration, /// How long to wait between attempts to get an IP address for a specified /// address object. - pub ip_addr_wait: u64, + pub ip_addr_wait: Duration, /// How long to wait without a solicitation response before expiring a peer - /// (milliseconds). - pub expire_threshold: u64, + pub expire_threshold: Duration, /// How long to wait for a response to exchange messages. - pub exchange_timeout: u64, + pub exchange_timeout: Duration, /// The kind of router this is, server or transit. pub kind: RouterKind, diff --git a/ddm/src/sm/state.rs b/ddm/src/sm/state.rs index 20fa19885..06f6f2ab9 100644 --- a/ddm/src/sm/state.rs +++ b/ddm/src/sm/state.rs @@ -99,7 +99,7 @@ impl State for Init { &self.ctx.config.aobj_name, e ); - sleep(Duration::from_millis(self.ctx.config.ip_addr_wait)); + sleep(self.ctx.config.ip_addr_wait); continue; } }; @@ -112,7 +112,7 @@ impl State for Init { "specified address {} is not IPv6", &self.ctx.config.aobj_name ); - sleep(Duration::from_millis(self.ctx.config.ip_addr_wait)); + sleep(self.ctx.config.ip_addr_wait); continue; } }; @@ -148,9 +148,7 @@ impl State for Init { self.ctx.config.if_name, "failed to start discovery handler: {e}", ); - sleep(Duration::from_millis( - self.ctx.config.solicit_interval, - )); + sleep(self.ctx.config.solicit_interval); continue; } }; @@ -293,7 +291,7 @@ impl Exchange { while let Err(e) = crate::exchange::pull(&mut ctx, peer, version, rt.clone()) { - sleep(Duration::from_millis(interval)); + sleep(interval); wrn!(log, if_name, "exchange pull: {}", e); if stop.load(Ordering::Relaxed) { break; diff --git a/ddmd/src/main.rs b/ddmd/src/main.rs index 7c78dc15b..e2c0b3d8c 100644 --- a/ddmd/src/main.rs +++ b/ddmd/src/main.rs @@ -4,7 +4,7 @@ use camino::Utf8PathBuf; use clap::Parser; -use ddm::admin::{HandlerContext, RouterStats}; +use ddm::admin::{HandlerContext, RouterStats, Tunables}; use ddm::db::Db; use ddm::defaults::{ DISCOVERY_READ_TIMEOUT, EXCHANGE_TCP_PORT, EXCHANGE_TIMEOUT, @@ -24,6 +24,7 @@ use std::net::{IpAddr, Ipv6Addr}; #[cfg(all(feature = "backend", target_os = "illumos"))] use std::sync::mpsc::channel; use std::sync::{Arc, Mutex}; +use std::time::Duration; use uuid::Uuid; mod signal; @@ -179,6 +180,15 @@ async fn run() { stats: router_stats, peers, stats_handler: Arc::new(Mutex::new(None)), + tunables: Tunables { + solicit_interval: Duration::from_millis(arg.solicit_interval), + expire_threshold: Duration::from_millis(arg.expire_threshold), + discovery_read_timeout: Duration::from_millis( + arg.discovery_read_timeout, + ), + ip_addr_wait: Duration::from_millis(arg.ip_addr_wait), + exchange_timeout: Duration::from_millis(arg.exchange_timeout), + }, log: log.clone(), })); @@ -250,11 +260,13 @@ fn start_state_machines( let (tx, rx) = channel(); let config = ddm::sm::Config { - solicit_interval: arg.solicit_interval, - expire_threshold: arg.expire_threshold, - discovery_read_timeout: arg.discovery_read_timeout, - ip_addr_wait: arg.ip_addr_wait, - exchange_timeout: arg.exchange_timeout, + solicit_interval: Duration::from_millis(arg.solicit_interval), + expire_threshold: Duration::from_millis(arg.expire_threshold), + discovery_read_timeout: Duration::from_millis( + arg.discovery_read_timeout, + ), + ip_addr_wait: Duration::from_millis(arg.ip_addr_wait), + exchange_timeout: Duration::from_millis(arg.exchange_timeout), exchange_port: arg.exchange_port, aobj_name: name.clone(), if_name: String::new(), From 2ea011a19dba2cc0ffb0faa93c612c062efbed97 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Tue, 15 Sep 2026 04:16:39 +0000 Subject: [PATCH 13/29] check for new peers in exchange/init --- ddm/src/sm/state.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ddm/src/sm/state.rs b/ddm/src/sm/state.rs index 06f6f2ab9..5bcf9a089 100644 --- a/ddm/src/sm/state.rs +++ b/ddm/src/sm/state.rs @@ -74,12 +74,15 @@ impl State for Init { self.ctx.iface.transition(FsmState::Init); self.ctx.iface.clear_peer(); loop { - // Check for shutdown + // Check for shutdown or new peers while let Ok(e) = event.try_recv() { match e { Event::Admin(AdminEvent::Shutdown) => { return (None, event); } + Event::Admin(AdminEvent::NewExternalPeer(tx)) => { + self.ctx.event_channels.push(tx); + } _ => { wrn!( self.log, From 3d29fe560e50774f79d5ad20b437464873b97b00 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Tue, 15 Sep 2026 04:44:56 +0000 Subject: [PATCH 14/29] remove dupe event channels vec at handler ctx level --- ddm/src/admin.rs | 47 ++++++++++++++++++++++++++--------------------- ddmd/src/main.rs | 13 ++++--------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index 5dfbf5ad2..8ec38679c 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -32,7 +32,7 @@ use dropshot::RequestContext; use dropshot::TypedBody; use mg_common::lock; use oxnet::Ipv6Net; -use slog::{Logger, error, info, o}; +use slog::{Logger, debug, error, info, o}; use slog_error_chain::InlineErrorChain; use std::collections::{BTreeSet, HashMap, HashSet}; use std::net::{IpAddr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6}; @@ -56,7 +56,6 @@ pub struct RouterStats { #[derive(Clone)] pub struct HandlerContext { - pub event_channels: Vec>, pub db: Db, pub stats: Arc, pub peers: Vec, @@ -65,6 +64,12 @@ pub struct HandlerContext { pub log: Logger, } +impl HandlerContext { + pub fn event_channels(&self) -> impl Iterator> { + self.peers.iter().map(|x| &x.tx) + } +} + #[derive(Clone)] pub struct Tunables { pub solicit_interval: Duration, @@ -190,7 +195,7 @@ impl DdmAdminApi for DdmAdminApiImpl { let addr = params.into_inner().addr; let ctx = lock!(ctx.context()); - for e in &ctx.event_channels { + for e in ctx.event_channels() { e.send(Event::Admin(AdminEvent::Expire(addr))) .map_err(|e| { HttpError::for_internal_error(format!( @@ -269,7 +274,7 @@ impl DdmAdminApi for DdmAdminApiImpl { .originate(&prefixes) .map_err(|e| HttpError::for_internal_error(e.to_string()))?; - for e in &ctx.event_channels { + for e in ctx.event_channels() { e.send(Event::Admin(AdminEvent::Announce(PrefixSet::Underlay( prefixes.clone(), )))) @@ -305,7 +310,7 @@ impl DdmAdminApi for DdmAdminApiImpl { .originate_tunnel(&endpoints) .map_err(|e| HttpError::for_internal_error(e.to_string()))?; - for e in &ctx.event_channels { + for e in ctx.event_channels() { e.send(Event::Admin(AdminEvent::Announce(PrefixSet::Tunnel( endpoints.clone(), )))) @@ -339,7 +344,7 @@ impl DdmAdminApi for DdmAdminApiImpl { .withdraw(&prefixes) .map_err(|e| HttpError::for_internal_error(e.to_string()))?; - for e in &ctx.event_channels { + for e in ctx.event_channels() { e.send(Event::Admin(AdminEvent::Withdraw(PrefixSet::Underlay( prefixes.clone(), )))) @@ -375,7 +380,7 @@ impl DdmAdminApi for DdmAdminApiImpl { .withdraw_tunnel(&endpoints) .map_err(|e| HttpError::for_internal_error(e.to_string()))?; - for e in &ctx.event_channels { + for e in ctx.event_channels() { e.send(Event::Admin(AdminEvent::Withdraw(PrefixSet::Tunnel( endpoints.clone(), )))) @@ -405,7 +410,7 @@ impl DdmAdminApi for DdmAdminApiImpl { ) -> Result { let ctx = lock!(ctx.context()); - for e in &ctx.event_channels { + for e in ctx.event_channels() { e.send(Event::Admin(AdminEvent::Sync)).map_err(|e| { HttpError::for_internal_error(format!("admin event send: {e}")) })?; @@ -510,7 +515,7 @@ impl DdmAdminApi for DdmAdminApiImpl { let sm_ctx = SmContext { config, db: ctx.db.clone(), - event_channels: ctx.event_channels.clone(), + event_channels: ctx.event_channels().cloned().collect(), tx: tx.clone(), log: ctx.log.clone(), hostname: hostname::get() @@ -531,24 +536,24 @@ impl DdmAdminApi for DdmAdminApiImpl { sm.run().unwrap(); ctx.peers.push(sm_ctx.clone()); - - ctx.event_channels.push(tx); } // Ensure our indices are unique and ordered. let mut remove_idx = BTreeSet::default(); - for ifx in to_remove.into_iter() { + for aobj in to_remove.into_iter() { for (i, p) in ctx.peers.iter().enumerate() { - if p.iface.external && p.config.aobj_name.contains(ifx) { - let _ = p.tx.send(Event::Admin(AdminEvent::Shutdown)); - remove_idx.insert(i); - info!( - ctx.log, - "removing external peeer on interface {ifx}" - ); - } else { - info!(ctx.log, "{ifx} != {}", p.config.aobj_name); + if p.iface.external { + if &p.config.aobj_name == aobj { + let _ = p.tx.send(Event::Admin(AdminEvent::Shutdown)); + remove_idx.insert(i); + info!( + ctx.log, + "removing external peeer for address object {aobj}" + ); + } else { + debug!(ctx.log, "{aobj} != {}", p.config.aobj_name); + } } } } diff --git a/ddmd/src/main.rs b/ddmd/src/main.rs index e2c0b3d8c..e9eaed019 100644 --- a/ddmd/src/main.rs +++ b/ddmd/src/main.rs @@ -166,8 +166,7 @@ async fn run() { .to_string_lossy() .to_string(); - let (sms, event_channels) = - start_state_machines(&arg, &db, &dpd, &hostname, &rt, &log); + let sms = start_state_machines(&arg, &db, &dpd, &hostname, &rt, &log); termination_handler(db.clone(), dpd.clone(), rt.clone(), log.clone()); @@ -175,7 +174,6 @@ async fn run() { let peers: Vec = sms.iter().map(|x| x.ctx.clone()).collect(); let context = Arc::new(Mutex::new(HandlerContext { - event_channels, db, stats: router_stats, peers, @@ -245,12 +243,9 @@ fn start_state_machines( hostname: &str, rt: &Arc, log: &Logger, -) -> ( - Vec, - Vec>, -) { +) -> Vec { if arg.api_only { - return (Vec::new(), Vec::new()); + return Vec::new(); } let mut sms = Vec::new(); @@ -311,7 +306,7 @@ fn start_state_machines( sm.run().unwrap(); } - (sms, event_channels) + sms } /// Non-illumos variant: the routing state machine depends on illumos From e9478de80e8ba804c6c6a0e64ed89dc6d8136c9d Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Tue, 15 Sep 2026 04:53:49 +0000 Subject: [PATCH 15/29] more tunable plumbing --- ddm/src/admin.rs | 16 +++++++++------- ddmd/src/main.rs | 3 +++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index 8ec38679c..c4e0dde35 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -77,6 +77,9 @@ pub struct Tunables { pub discovery_read_timeout: Duration, pub ip_addr_wait: Duration, pub exchange_timeout: Duration, + pub dpd_port: u16, + pub dpd_host: String, + pub exchange_tcp_port: u16, } impl Default for Tunables { @@ -87,6 +90,9 @@ impl Default for Tunables { discovery_read_timeout: DISCOVERY_READ_TIMEOUT, ip_addr_wait: IP_ADDR_WAIT, exchange_timeout: EXCHANGE_TIMEOUT, + dpd_port: dpd_client::default_port(), + dpd_host: "localhost".into(), + exchange_tcp_port: EXCHANGE_TCP_PORT, } } } @@ -496,19 +502,15 @@ impl DdmAdminApi for DdmAdminApiImpl { discovery_read_timeout: ctx.tunables.discovery_read_timeout, ip_addr_wait: ctx.tunables.ip_addr_wait, exchange_timeout: ctx.tunables.exchange_timeout, - exchange_port: EXCHANGE_TCP_PORT, + exchange_port: ctx.tunables.exchange_tcp_port, aobj_name: addr_obj.clone(), if_name: String::default(), // initialized in state machine if_index: 0, // initialized in state machine // External peers are only a thing for transit routers. kind: RouterKind::Transit, dpd: Some(crate::sm::DpdConfig { - // Transit DDM routers always talk to their local dpd in the - // switch zone. - host: String::from("localhost"), - // TODO: using the default dpd port might not be right for some - // test environments. - port: dpd_client::default_port(), + host: ctx.tunables.dpd_host.clone(), + port: ctx.tunables.dpd_port, }), addr: Ipv6Addr::UNSPECIFIED, }; diff --git a/ddmd/src/main.rs b/ddmd/src/main.rs index e9eaed019..a0a6a2817 100644 --- a/ddmd/src/main.rs +++ b/ddmd/src/main.rs @@ -186,6 +186,9 @@ async fn run() { ), ip_addr_wait: Duration::from_millis(arg.ip_addr_wait), exchange_timeout: Duration::from_millis(arg.exchange_timeout), + dpd_port: arg.dpd_port, + dpd_host: arg.dpd_host.clone(), + exchange_tcp_port: arg.exchange_port, }, log: log.clone(), })); From ddb1989d1bb56b6016d8831c47ad459ae7cf29b7 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Tue, 15 Sep 2026 05:04:01 +0000 Subject: [PATCH 16/29] linux bs --- ddm/src/sm/mod.rs | 6 +++++- ddmd/src/main.rs | 7 ++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/ddm/src/sm/mod.rs b/ddm/src/sm/mod.rs index a9f16f3ec..b855a6bf0 100644 --- a/ddm/src/sm/mod.rs +++ b/ddm/src/sm/mod.rs @@ -14,7 +14,7 @@ use ddm_api_types::net::TunnelOrigin; use mg_common::lock; use oxnet::Ipv6Net; use slog::Logger; -use std::collections::{BTreeSet, HashSet}; +use std::collections::HashSet; use std::net::Ipv6Addr; use std::sync::atomic::{AtomicBool, AtomicU64}; use std::sync::mpsc::{Receiver, Sender}; @@ -22,6 +22,9 @@ use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; use thiserror::Error; +#[cfg(target_os = "illumos")] +use std::collections::BTreeSet; + #[cfg(all(feature = "backend", target_os = "illumos"))] pub(crate) mod state; @@ -285,6 +288,7 @@ impl StateMachine { /// Send an event to all channels in the list, removing any channels from the /// list that are dead. +#[cfg(target_os = "illumos")] pub(crate) fn send(e: Event, event_channels: &mut Vec>) { // Ensure our indices are unique and ordered. let mut dead_channels = BTreeSet::default(); diff --git a/ddmd/src/main.rs b/ddmd/src/main.rs index a0a6a2817..f18c25431 100644 --- a/ddmd/src/main.rs +++ b/ddmd/src/main.rs @@ -323,11 +323,8 @@ fn start_state_machines( _hostname: &str, _rt: &Arc, _log: &Logger, -) -> ( - Vec, - Vec>, -) { - (Vec::new(), Vec::new()) +) -> Vec { + Vec::new() } /// Install a Ctrl-C handler that withdraws ddmd's imported routes from the From 20cfcc336f92b53ecb161508c7f26e906aa4086c Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Tue, 15 Sep 2026 05:10:49 +0000 Subject: [PATCH 17/29] plumb dendrite tunable --- ddm/src/admin.rs | 14 ++++++++++---- ddmd/src/main.rs | 1 + 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index c4e0dde35..3ac4716dc 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -77,6 +77,7 @@ pub struct Tunables { pub discovery_read_timeout: Duration, pub ip_addr_wait: Duration, pub exchange_timeout: Duration, + pub dendrite: bool, pub dpd_port: u16, pub dpd_host: String, pub exchange_tcp_port: u16, @@ -93,6 +94,7 @@ impl Default for Tunables { dpd_port: dpd_client::default_port(), dpd_host: "localhost".into(), exchange_tcp_port: EXCHANGE_TCP_PORT, + dendrite: true, } } } @@ -508,10 +510,14 @@ impl DdmAdminApi for DdmAdminApiImpl { if_index: 0, // initialized in state machine // External peers are only a thing for transit routers. kind: RouterKind::Transit, - dpd: Some(crate::sm::DpdConfig { - host: ctx.tunables.dpd_host.clone(), - port: ctx.tunables.dpd_port, - }), + dpd: if ctx.tunables.dendrite { + Some(crate::sm::DpdConfig { + host: ctx.tunables.dpd_host.clone(), + port: ctx.tunables.dpd_port, + }) + } else { + None + }, addr: Ipv6Addr::UNSPECIFIED, }; let sm_ctx = SmContext { diff --git a/ddmd/src/main.rs b/ddmd/src/main.rs index f18c25431..85a8539c4 100644 --- a/ddmd/src/main.rs +++ b/ddmd/src/main.rs @@ -186,6 +186,7 @@ async fn run() { ), ip_addr_wait: Duration::from_millis(arg.ip_addr_wait), exchange_timeout: Duration::from_millis(arg.exchange_timeout), + dendrite: arg.dendrite, dpd_port: arg.dpd_port, dpd_host: arg.dpd_host.clone(), exchange_tcp_port: arg.exchange_port, From ea4f02db811d6e1771099d96e9c384e4df865a4a Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Tue, 15 Sep 2026 05:22:57 +0000 Subject: [PATCH 18/29] reject external peers api calls for non-transit routers --- ddm/src/admin.rs | 16 ++++++++++++++++ ddmd/src/main.rs | 1 + tests/src/ddm.rs | 2 +- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index 3ac4716dc..71db4d583 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -61,6 +61,7 @@ pub struct HandlerContext { pub peers: Vec, pub stats_handler: Arc>>>, pub tunables: Tunables, + pub router_kind: RouterKind, pub log: Logger, } @@ -481,6 +482,14 @@ impl DdmAdminApi for DdmAdminApiImpl { request: TypedBody, ) -> Result { let mut ctx = lock!(ctx.context()); + + if ctx.router_kind != RouterKind::Transit { + return Err(HttpError::for_bad_request( + None, + "external peers only supported for transit routers".into(), + )); + } + let rq = request.into_inner(); let current = ctx.db.get_external_peers(); @@ -579,6 +588,13 @@ impl DdmAdminApi for DdmAdminApiImpl { ) -> Result, HttpError> { let ctx = lock!(ctx.context()); + if ctx.router_kind != RouterKind::Transit { + return Err(HttpError::for_bad_request( + None, + "external peers only supported for transit routers".into(), + )); + } + Ok(HttpResponseOk(ExternalPeers { address_objects: ctx.db.get_external_peers(), })) diff --git a/ddmd/src/main.rs b/ddmd/src/main.rs index 85a8539c4..7dc3036db 100644 --- a/ddmd/src/main.rs +++ b/ddmd/src/main.rs @@ -178,6 +178,7 @@ async fn run() { stats: router_stats, peers, stats_handler: Arc::new(Mutex::new(None)), + router_kind: arg.kind, tunables: Tunables { solicit_interval: Duration::from_millis(arg.solicit_interval), expire_threshold: Duration::from_millis(arg.expire_threshold), diff --git a/tests/src/ddm.rs b/tests/src/ddm.rs index 56a30585f..da94bfa4a 100644 --- a/tests/src/ddm.rs +++ b/tests/src/ddm.rs @@ -244,7 +244,7 @@ impl<'a> RouterZone<'a> { // Tighter solicit interval and expire threshold are to speed up tests. let extra_args = format!( - "--rack-uuid {} --sled-uuid {} --solicit-interval 200 --expire-threshold 500", + "--rack-uuid {} --sled-uuid {} --solicit-interval 20 --expire-threshold 50", uuid::Uuid::new_v4(), uuid::Uuid::new_v4(), ); From 067205a75f686a9db619ef956b7a20a52d058e87 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Tue, 15 Sep 2026 06:44:53 +0000 Subject: [PATCH 19/29] exchange: don't hold lock across underlying tokio ops The exchange endpoints all talk to dendrite over tokio, and they all take a context lock. If we hold the context lock across the tokio ops if we get enough simultaneous requests we'll stop tokio from being able to run while at the same time depending on tokio to make progress for the blocking lock to release the dam. --- ddm/src/exchange/runtime.rs | 52 ++++++++++++------------------------- ddm/src/sm/mod.rs | 16 ++++++++++++ ddm/src/sm/state.rs | 22 +++++++++++++--- tests/src/ddm.rs | 2 +- 4 files changed, 53 insertions(+), 39 deletions(-) diff --git a/ddm/src/exchange/runtime.rs b/ddm/src/exchange/runtime.rs index ba3578542..573d55689 100644 --- a/ddm/src/exchange/runtime.rs +++ b/ddm/src/exchange/runtime.rs @@ -11,7 +11,7 @@ use super::ExchangeError; use crate::db::{Route, effective_route_set}; use crate::discovery::Version; use crate::sm::{Config, Event, PeerEvent, SmContext}; -use crate::{dbg, err, inf, wrn}; +use crate::{err, inf, wrn}; use ddm_api_types::db::{RouterKind, TunnelRoute}; use ddm_protocol::{v2, v3}; use dropshot::ApiDescription; @@ -151,7 +151,7 @@ fn do_pull_common( } pub(crate) fn pull( - ctx: &mut SmContext, + ctx: &SmContext, addr: Ipv6Addr, version: Version, rt: Arc, @@ -356,12 +356,10 @@ async fn push_handler_common( ctx: RequestContext>>, update: v3::Update, ) -> Result { - let rq_ctx: Arc> = ctx.context().clone(); + let ctx = lock!(ctx.context()).clone(); tokio::task::spawn_blocking(move || { - let mut actx = lock!(rq_ctx); - let peer = actx.peer; - handle_update(&update, &mut actx.ctx, peer); + handle_update(&update, &ctx.ctx, ctx.peer); }) .await .map_err(|e| { @@ -523,11 +521,7 @@ async fn pull_handler( })) } -fn handle_update( - update: &v3::Update, - ctx: &mut SmContext, - peer_addr: Ipv6Addr, -) { +fn handle_update(update: &v3::Update, ctx: &SmContext, peer_addr: Ipv6Addr) { ctx.stats.updates_received.fetch_add(1, Ordering::Relaxed); if let Some(underlay_update) = &update.underlay { @@ -541,34 +535,22 @@ fn handle_update( // distribute updates if ctx.config.kind == RouterKind::Transit { - dbg!( - ctx.log, - ctx.config.if_name, - "redistributing update to {} peers", - ctx.event_channels.len() - ); - - let underlay = update.underlay.as_ref().map(|update| { - update - .break_loops(&ctx.hostname) - .with_path_element(ctx.hostname.clone()) - }); - - let push = v3::Update { - underlay, - tunnel: update.tunnel.clone(), - }; - - crate::sm::send( - Event::Peer(PeerEvent::Push(push.clone())), - &mut ctx.event_channels, - ); + if let Err(e) = ctx + .tx + .send(Event::Peer(PeerEvent::Redistribute(update.clone()))) + { + wrn!( + ctx.log, + ctx.config.if_name, + "failed to send update to SM: {e:?}" + ); + } } } fn handle_tunnel_update( update: &v3::TunnelUpdate, - ctx: &mut SmContext, + ctx: &SmContext, peer_addr: Ipv6Addr, ) { let mut import = HashSet::new(); @@ -637,7 +619,7 @@ fn handle_tunnel_update( fn handle_underlay_update( update: &v3::UnderlayUpdate, - ctx: &mut SmContext, + ctx: &SmContext, peer_addr: Ipv6Addr, ) { let mut import = HashSet::new(); diff --git a/ddm/src/sm/mod.rs b/ddm/src/sm/mod.rs index b855a6bf0..064aef898 100644 --- a/ddm/src/sm/mod.rs +++ b/ddm/src/sm/mod.rs @@ -58,13 +58,29 @@ pub enum PrefixSet { #[derive(Debug, Clone)] pub enum PeerEvent { + /// Upon reception of this event, a state machine is to push the update to + /// its peer. Push(ddm_protocol::v3::Update), + + /// Upon reception of this event, a state machine is to redistribute the + /// update to it's sibling routers through it's event channels. The state + /// machine is responsible maintaining the path vector and performing loop + /// breaking. + Redistribute(ddm_protocol::v3::Update), } #[derive(Debug, Clone)] pub enum NeighborEvent { + /// An event sent from the discovery subsystem to the state machine letting + /// it know the link local ipv6 address of the peer and it's version. Advertise((Ipv6Addr, Version)), + + /// An event sent from the discovery subsystem to the state machine letting + /// it know that solicitation has failed. SolicitFail, + + /// An event sent from the discovery subsystem to the state machine letting + /// it know that the peer has expired. Expire, } diff --git a/ddm/src/sm/state.rs b/ddm/src/sm/state.rs index 5bcf9a089..2e14ec114 100644 --- a/ddm/src/sm/state.rs +++ b/ddm/src/sm/state.rs @@ -288,11 +288,11 @@ impl Exchange { let log = self.log.clone(); let interval = self.ctx.config.solicit_interval; let if_name = self.ctx.config.if_name.clone(); - let mut ctx = self.ctx.clone(); + let ctx = self.ctx.clone(); spawn(move || { while let Err(e) = - crate::exchange::pull(&mut ctx, peer, version, rt.clone()) + crate::exchange::pull(&ctx, peer, version, rt.clone()) { sleep(interval); wrn!(log, if_name, "exchange pull: {}", e); @@ -663,7 +663,7 @@ impl State for Exchange { Event::Admin(AdminEvent::Sync) => { let rt = self.ctx.rt.clone(); if let Err(e) = crate::exchange::pull( - &mut self.ctx, + &self.ctx, self.peer, self.version, rt, @@ -781,6 +781,22 @@ impl State for Exchange { } } } + Event::Peer(PeerEvent::Redistribute(mut update)) => { + dbg!( + self.log, + self.ctx.config.if_name, + "redistributing update to {} peers", + self.ctx.event_channels.len() + ); + update.underlay = update.underlay.map(|u| { + u.break_loops(&self.ctx.hostname) + .with_path_element(self.ctx.hostname.clone()) + }); + super::send( + Event::Peer(PeerEvent::Push(update)), + &mut self.ctx.event_channels, + ); + } Event::Neighbor(NeighborEvent::Expire) => { wrn!( self.log, diff --git a/tests/src/ddm.rs b/tests/src/ddm.rs index da94bfa4a..56a30585f 100644 --- a/tests/src/ddm.rs +++ b/tests/src/ddm.rs @@ -244,7 +244,7 @@ impl<'a> RouterZone<'a> { // Tighter solicit interval and expire threshold are to speed up tests. let extra_args = format!( - "--rack-uuid {} --sled-uuid {} --solicit-interval 20 --expire-threshold 50", + "--rack-uuid {} --sled-uuid {} --solicit-interval 200 --expire-threshold 500", uuid::Uuid::new_v4(), uuid::Uuid::new_v4(), ); From 257b0e9ea8a9646b53c7ac4c88d4fea376945454 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Tue, 15 Sep 2026 07:09:10 +0000 Subject: [PATCH 20/29] extend reach tests to include transit routers --- ddm/src/exchange/runtime.rs | 17 ++++++++--------- tests/src/ddm.rs | 25 +++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/ddm/src/exchange/runtime.rs b/ddm/src/exchange/runtime.rs index 573d55689..aa2902776 100644 --- a/ddm/src/exchange/runtime.rs +++ b/ddm/src/exchange/runtime.rs @@ -534,17 +534,16 @@ fn handle_update(update: &v3::Update, ctx: &SmContext, peer_addr: Ipv6Addr) { // distribute updates - if ctx.config.kind == RouterKind::Transit { - if let Err(e) = ctx + if ctx.config.kind == RouterKind::Transit + && let Err(e) = ctx .tx .send(Event::Peer(PeerEvent::Redistribute(update.clone()))) - { - wrn!( - ctx.log, - ctx.config.if_name, - "failed to send update to SM: {e:?}" - ); - } + { + wrn!( + ctx.log, + ctx.config.if_name, + "failed to send update to SM: {e:?}" + ); } } diff --git a/tests/src/ddm.rs b/tests/src/ddm.rs index 56a30585f..1821ea39b 100644 --- a/tests/src/ddm.rs +++ b/tests/src/ddm.rs @@ -1139,6 +1139,8 @@ async fn run_sextet_tests( s2: BTreeSet, s3: BTreeSet, s4: BTreeSet, + t1: BTreeSet, + t2: BTreeSet, } drop_dump!(s1, "http://10.0.0.1:8000"); @@ -1209,6 +1211,8 @@ async fn run_sextet_tests( assert_peer_reach!(s2, $r.s2.clone()); assert_peer_reach!(s3, $r.s3.clone()); assert_peer_reach!(s4, $r.s4.clone()); + assert_peer_reach!(t1, $r.t1.clone()); + assert_peer_reach!(t2, $r.t2.clone()); }}; } @@ -1220,6 +1224,16 @@ async fn run_sextet_tests( let s2_origin: Vec = [ip6_net!("fd00:2::/64")].into(); let s3_origin: Vec = [ip6_net!("fd00:3::/64")].into(); let s4_origin: Vec = [ip6_net!("fd00:4::/64")].into(); + let t1_origin: Vec = s1_origin + .clone() + .into_iter() + .chain(s2_origin.clone()) + .collect(); + let t2_origin: Vec = s3_origin + .clone() + .into_iter() + .chain(s4_origin.clone()) + .collect(); s1.advertise_prefixes(&s1_origin).await?; s2.advertise_prefixes(&s2_origin).await?; @@ -1328,17 +1342,20 @@ async fn run_sextet_tests( let counts = expected_external_peerings(x, y); // Servers can always see the originated prefixes of other routers // reachable over a single hop transit router path (e.g. in the same - // rack). + // rack). Transit routers can always see the originated prefixes of + // their directly connected server routers. let mut reach = PeerReachablePrefixes { s1: s2_origin.iter().cloned().collect(), s2: s1_origin.iter().cloned().collect(), s3: s4_origin.iter().cloned().collect(), s4: s3_origin.iter().cloned().collect(), + t1: t1_origin.iter().cloned().collect(), + t2: t2_origin.iter().cloned().collect(), }; // If there is any peering between transit routers, each server router // should see prefixes originated from the router adjacent to their // transit router. - if counts.t1 > 2 && counts.t2 > 2 { + if counts.t1 > 2 || counts.t2 > 2 { // Origins from servers connected to t2 propagating to servers // connected to t1. reach.s1.extend(&s3_origin); @@ -1352,6 +1369,10 @@ async fn run_sextet_tests( reach.s3.extend(&s2_origin); reach.s4.extend(&s1_origin); reach.s4.extend(&s2_origin); + + // Origins from transit routers propagate to each other. + reach.t1.extend(&t2_origin); + reach.t2.extend(&t1_origin); } reach }; From 9be4cdb0c53e441f5bf18b59b8a3cddb8773cf33 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Tue, 15 Sep 2026 07:27:10 +0000 Subject: [PATCH 21/29] test that all nexthops are peers --- tests/src/ddm.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/src/ddm.rs b/tests/src/ddm.rs index 1821ea39b..cb934b6e2 100644 --- a/tests/src/ddm.rs +++ b/tests/src/ddm.rs @@ -1216,6 +1216,35 @@ async fn run_sextet_tests( }}; } + macro_rules! assert_nexthops_are_peers { + ($client:expr) => {{ + let pfx_nexthops = + $client.get_prefixes().await.expect("get prefixes"); + let peers = $client + .get_peers() + .await + .expect("get peers") + .values() + .map(|x| x.addr.to_string()) + .collect::>(); + + for p in pfx_nexthops.keys() { + assert!(peers.contains(p), "nexthop {p} is not a peer"); + } + }}; + } + + macro_rules! assert_all_nexthops_are_peers { + () => {{ + assert_nexthops_are_peers!(s1); + assert_nexthops_are_peers!(s2); + assert_nexthops_are_peers!(s3); + assert_nexthops_are_peers!(s4); + assert_nexthops_are_peers!(t1); + assert_nexthops_are_peers!(t2); + }}; + } + // // Initialize announcements for each server peer // @@ -1413,6 +1442,8 @@ async fn run_sextet_tests( let reach = expected_reachable_prefixes(x, y); assert_reach!(reach); + + assert_all_nexthops_are_peers!(); } Ok(()) From 4ba4c1ffd98db06a69300a70fac5400ca8be6bc5 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Tue, 15 Sep 2026 09:54:25 +0000 Subject: [PATCH 22/29] close possible shutdown race with pull threads --- ddm/src/exchange/runtime.rs | 7 ++- ddm/src/sm/state.rs | 91 +++++++++++++++++++++++++++++-------- 2 files changed, 79 insertions(+), 19 deletions(-) diff --git a/ddm/src/exchange/runtime.rs b/ddm/src/exchange/runtime.rs index aa2902776..8117cac74 100644 --- a/ddm/src/exchange/runtime.rs +++ b/ddm/src/exchange/runtime.rs @@ -35,7 +35,7 @@ use std::collections::HashSet; use std::net::{Ipv6Addr, SocketAddrV6}; use std::sync::Arc; use std::sync::Mutex; -use std::sync::atomic::Ordering; +use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Duration; use tokio::time::timeout; @@ -155,12 +155,17 @@ pub(crate) fn pull( addr: Ipv6Addr, version: Version, rt: Arc, + stop: Arc, ) -> Result<(), ExchangeError> { let pr: v3::PullResponse = match version { Version::V2 => do_pull_v2(ctx, &addr, &rt)?.into(), Version::V3 => do_pull(ctx, &addr, &rt)?, }; + if stop.load(Ordering::Relaxed) { + return Ok(()); + } + let update = v3::Update::announce(pr); handle_update(&update, ctx, addr); diff --git a/ddm/src/sm/state.rs b/ddm/src/sm/state.rs index 2e14ec114..ca4cedb38 100644 --- a/ddm/src/sm/state.rs +++ b/ddm/src/sm/state.rs @@ -21,7 +21,7 @@ use std::net::IpAddr; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::Receiver; -use std::thread::{sleep, spawn}; +use std::thread::{JoinHandle, sleep, spawn}; use std::time::Duration; use crate::discovery::Version; @@ -281,7 +281,7 @@ impl Exchange { } } - fn initial_pull(&mut self, stop: Arc) { + fn initial_pull(&mut self, stop: Arc) -> JoinHandle<()> { let peer = self.peer; let version = self.version; let rt = self.ctx.rt.clone(); @@ -291,16 +291,20 @@ impl Exchange { let ctx = self.ctx.clone(); spawn(move || { - while let Err(e) = - crate::exchange::pull(&ctx, peer, version, rt.clone()) - { + while let Err(e) = crate::exchange::pull( + &ctx, + peer, + version, + rt.clone(), + stop.clone(), + ) { sleep(interval); wrn!(log, if_name, "exchange pull: {}", e); if stop.load(Ordering::Relaxed) { break; } } - }); + }) } fn wait_for_exchange_server_to_start(&self) { @@ -338,7 +342,18 @@ impl Exchange { &mut self, exchange_thread: &tokio::task::JoinHandle<()>, pull_stop: &AtomicBool, + initial_pull: JoinHandle<()>, ) { + // Stop other threads that may update the rib after we clear for expiry. + pull_stop.store(true, Ordering::Relaxed); + if let Err(e) = initial_pull.join() { + err!( + self.log, + self.ctx.config.if_name, + "failed to join initial pull thread: {e:?}", + ); + } + exchange_thread.abort(); self.ctx.iface.clear_peer(); let (mut to_remove, to_remove_tnl) = @@ -428,7 +443,6 @@ impl Exchange { &mut self.ctx.event_channels, ); } - pull_stop.store(true, Ordering::Relaxed); } } @@ -466,7 +480,7 @@ impl State for Exchange { // Do an initial pull, in the event that exchange events are fired while // this pull is taking place, they will be queued and handled in the // loop below. - self.initial_pull(pull_stop.clone()); + let initial_pull = self.initial_pull(pull_stop.clone()); if self.ctx.iface.external && self.ctx.first_run { self.ctx.first_run = false; @@ -521,7 +535,11 @@ impl State for Exchange { "expiring peer {} due to failed announce", self.peer, ); - self.expire_peer(&exchange_thread, &pull_stop); + self.expire_peer( + &exchange_thread, + &pull_stop, + initial_pull, + ); return ( Some(Box::new(Solicit::new( self.ctx.clone(), @@ -556,7 +574,11 @@ impl State for Exchange { "expiring peer {} due to failed tunnel announce", self.peer, ); - self.expire_peer(&exchange_thread, &pull_stop); + self.expire_peer( + &exchange_thread, + &pull_stop, + initial_pull, + ); return ( Some(Box::new(Solicit::new( self.ctx.clone(), @@ -597,7 +619,11 @@ impl State for Exchange { "expiring peer {} due to failed withdraw", self.peer, ); - self.expire_peer(&exchange_thread, &pull_stop); + self.expire_peer( + &exchange_thread, + &pull_stop, + initial_pull, + ); return ( Some(Box::new(Solicit::new( self.ctx.clone(), @@ -632,7 +658,11 @@ impl State for Exchange { "expiring peer {} due to failed tunnel withdraw", self.peer, ); - self.expire_peer(&exchange_thread, &pull_stop); + self.expire_peer( + &exchange_thread, + &pull_stop, + initial_pull, + ); return ( Some(Box::new(Solicit::new( self.ctx.clone(), @@ -650,7 +680,11 @@ impl State for Exchange { "administratively expiring peer {}", peer, ); - self.expire_peer(&exchange_thread, &pull_stop); + self.expire_peer( + &exchange_thread, + &pull_stop, + initial_pull, + ); return ( Some(Box::new(Solicit::new( self.ctx.clone(), @@ -667,6 +701,7 @@ impl State for Exchange { self.peer, self.version, rt, + pull_stop.clone(), ) { err!( self.log, @@ -703,7 +738,11 @@ impl State for Exchange { if let Some(x) = self.ctx.discovery_stop.as_mut() { x.store(true, Ordering::Relaxed); } - self.expire_peer(&exchange_thread, &pull_stop); + self.expire_peer( + &exchange_thread, + &pull_stop, + initial_pull, + ); return (None, event); } Event::Peer(PeerEvent::Push(update)) => { @@ -738,7 +777,11 @@ impl State for Exchange { "expiring peer {} due to failed announce", self.peer, ); - self.expire_peer(&exchange_thread, &pull_stop); + self.expire_peer( + &exchange_thread, + &pull_stop, + initial_pull, + ); return ( Some(Box::new(Solicit::new( self.ctx.clone(), @@ -770,7 +813,11 @@ impl State for Exchange { "expiring peer {} due to failed withdraw", self.peer, ); - self.expire_peer(&exchange_thread, &pull_stop); + self.expire_peer( + &exchange_thread, + &pull_stop, + initial_pull, + ); return ( Some(Box::new(Solicit::new( self.ctx.clone(), @@ -804,7 +851,11 @@ impl State for Exchange { "expiring peer {} due to discovery event", self.peer, ); - self.expire_peer(&exchange_thread, &pull_stop); + self.expire_peer( + &exchange_thread, + &pull_stop, + initial_pull, + ); return ( Some(Box::new(Solicit::new( self.ctx.clone(), @@ -820,7 +871,11 @@ impl State for Exchange { "expiring peer {} due to failed solicit", self.peer, ); - self.expire_peer(&exchange_thread, &pull_stop); + self.expire_peer( + &exchange_thread, + &pull_stop, + initial_pull, + ); return ( Some(Box::new(Init::new( self.ctx.clone(), From 999fdd67d024c9c05e5d201a06d284e41357e721 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Thu, 17 Sep 2026 06:42:55 +0000 Subject: [PATCH 23/29] nicolas' feedback --- .github/buildomat/jobs/test-ddm-sextet.sh | 4 ++-- ddm/src/exchange/runtime.rs | 9 ++++----- ddm/src/sm/mod.rs | 8 ++++---- ddm/src/sm/state.rs | 6 ++++-- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/.github/buildomat/jobs/test-ddm-sextet.sh b/.github/buildomat/jobs/test-ddm-sextet.sh index 975e1fbd5..eb54c8f8b 100644 --- a/.github/buildomat/jobs/test-ddm-sextet.sh +++ b/.github/buildomat/jobs/test-ddm-sextet.sh @@ -11,8 +11,8 @@ source .github/buildomat/test-ddm-common.sh # -# trio tests +# sextet tests # -banner "trio" +banner "sextet" pfexec cargo test --release -p mg-tests test_external_peer_sextet -- --nocapture diff --git a/ddm/src/exchange/runtime.rs b/ddm/src/exchange/runtime.rs index 8117cac74..db6f18c98 100644 --- a/ddm/src/exchange/runtime.rs +++ b/ddm/src/exchange/runtime.rs @@ -29,14 +29,13 @@ use http_body_util::BodyExt; use hyper::body::Bytes; use hyper_util::client::legacy::Client; use hyper_util::rt::TokioExecutor; -use mg_common::lock; use slog::{Logger, o}; use std::collections::HashSet; use std::net::{Ipv6Addr, SocketAddrV6}; use std::sync::Arc; -use std::sync::Mutex; use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Duration; +use tokio::sync::Mutex; use tokio::time::timeout; const UNIT_EXCHANGE_SERVER: &str = "exchange_server"; @@ -361,7 +360,7 @@ async fn push_handler_common( ctx: RequestContext>>, update: v3::Update, ) -> Result { - let ctx = lock!(ctx.context()).clone(); + let ctx = ctx.context().lock().await.clone(); tokio::task::spawn_blocking(move || { handle_update(&update, &ctx.ctx, ctx.peer); @@ -381,7 +380,7 @@ async fn push_handler_common( async fn pull_handler_v2( ctx: RequestContext>>, ) -> Result, HttpError> { - let ctx = lock!(ctx.context()); + let ctx = ctx.context().lock().await.clone(); let mut underlay = HashSet::new(); let mut tunnel = HashSet::new(); @@ -457,7 +456,7 @@ async fn pull_handler_v2( async fn pull_handler( ctx: RequestContext>>, ) -> Result, HttpError> { - let ctx = lock!(ctx.context()); + let ctx = ctx.context().lock().await.clone(); let mut underlay = HashSet::new(); let mut tunnel = HashSet::new(); diff --git a/ddm/src/sm/mod.rs b/ddm/src/sm/mod.rs index 064aef898..8dc85144f 100644 --- a/ddm/src/sm/mod.rs +++ b/ddm/src/sm/mod.rs @@ -46,7 +46,7 @@ pub enum AdminEvent { /// using the provided sender. NewExternalPeer(Sender), - /// Shutdown on recipt of this event. + /// Shutdown on receipt of this event. Shutdown, } @@ -64,8 +64,8 @@ pub enum PeerEvent { /// Upon reception of this event, a state machine is to redistribute the /// update to it's sibling routers through it's event channels. The state - /// machine is responsible maintaining the path vector and performing loop - /// breaking. + /// machine is responsible for maintaining the path vector and performing + /// loop breaking. Redistribute(ddm_protocol::v3::Update), } @@ -314,7 +314,7 @@ pub(crate) fn send(e: Event, event_channels: &mut Vec>) { } } // we need to remove in descending order, so we don't remove `i` and then - // try to remove `i+1` later which wlll be a _diffrent_ item than we grabbed + // try to remove `i+1` later which wlll be a _different_ item than we grabbed // the index for. Removing from the top down causes no shifting for subsequent // index removals. for i in dead_channels.iter().rev() { diff --git a/ddm/src/sm/state.rs b/ddm/src/sm/state.rs index ca4cedb38..4f70d74b2 100644 --- a/ddm/src/sm/state.rs +++ b/ddm/src/sm/state.rs @@ -394,7 +394,7 @@ impl Exchange { ); // Only send withdraws for expirations that result in a total loss - // of reachability to a destination. + // of reachability to a destination for the given path. let imported = self.ctx.db.imported(); dbg!(self.log, self.ctx.config.if_name, "imported: {imported:#?}"); dbg!( @@ -403,7 +403,9 @@ impl Exchange { "to_remove: {to_remove:#?}" ); to_remove.retain(|x| { - !imported.iter().any(|y| y.destination == x.destination) + !imported + .iter() + .any(|y| y.destination == x.destination && y.path == x.path) }); dbg!( self.log, From 886ee97ad4607e75e6ce9b7c9c2087b5dd18fa89 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Thu, 17 Sep 2026 22:07:39 +0000 Subject: [PATCH 24/29] log loop breaking, PUSH->PUT, use sled id as router id --- ddm-api/src/lib.rs | 2 +- ddm/src/admin.rs | 11 ++++-- ddm/src/exchange/runtime.rs | 10 ++--- ddm/src/sm/mod.rs | 9 ++++- ddm/src/sm/state.rs | 37 +++++++++++++++---- ddmd/src/main.rs | 16 ++++++-- ...24165.json => ddm-admin-3.0.0-3255d9.json} | 2 +- openapi/ddm-admin/ddm-admin-latest.json | 2 +- 8 files changed, 66 insertions(+), 23 deletions(-) rename openapi/ddm-admin/{ddm-admin-3.0.0-224165.json => ddm-admin-3.0.0-3255d9.json} (99%) diff --git a/ddm-api/src/lib.rs b/ddm-api/src/lib.rs index 58b179b4c..e7c73464f 100644 --- a/ddm-api/src/lib.rs +++ b/ddm-api/src/lib.rs @@ -173,7 +173,7 @@ pub trait DdmAdminApi { ) -> Result; #[endpoint { - method = POST, + method = PUT, path = "/external_peers", versions = VERSION_EXTERNAL_PEERS.., }] diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index 71db4d583..50d2c26c3 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -43,6 +43,7 @@ use std::sync::mpsc::{Sender, channel}; use std::time::Duration; use tokio::spawn; use tokio::task::JoinHandle; +use uuid::Uuid; pub const DDM_STATS_PORT: u16 = 8001; @@ -62,6 +63,9 @@ pub struct HandlerContext { pub stats_handler: Arc>>>, pub tunables: Tunables, pub router_kind: RouterKind, + pub rack_id: Option, + pub sled_id: Option, + pub router_id: String, pub log: Logger, } @@ -528,6 +532,8 @@ impl DdmAdminApi for DdmAdminApiImpl { None }, addr: Ipv6Addr::UNSPECIFIED, + rack_id: ctx.rack_id, + sled_id: ctx.sled_id, }; let sm_ctx = SmContext { config, @@ -535,10 +541,7 @@ impl DdmAdminApi for DdmAdminApiImpl { event_channels: ctx.event_channels().cloned().collect(), tx: tx.clone(), log: ctx.log.clone(), - hostname: hostname::get() - .expect("failed to get hostname") - .to_string_lossy() - .to_string(), + router_id: ctx.router_id.clone(), rt: Arc::new(tokio::runtime::Handle::current()), iface: Arc::new(InterfaceState::external()), stats: Arc::new(SessionStats::default()), diff --git a/ddm/src/exchange/runtime.rs b/ddm/src/exchange/runtime.rs index db6f18c98..9af01e7df 100644 --- a/ddm/src/exchange/runtime.rs +++ b/ddm/src/exchange/runtime.rs @@ -396,7 +396,7 @@ async fn pull_handler_v2( destination: route.destination, path: route.path.clone(), }; - pv.path.push(ctx.ctx.hostname.clone()); + pv.path.push(ctx.ctx.router_id.clone()); underlay.insert(pv); } for route in &ctx.ctx.db.imported_tunnel() { @@ -415,7 +415,7 @@ async fn pull_handler_v2( for prefix in &originated { let pv = v3::PathVector { destination: *prefix, - path: vec![ctx.ctx.hostname.clone()], + path: vec![ctx.ctx.router_id.clone()], }; underlay.insert(pv); } @@ -472,7 +472,7 @@ async fn pull_handler( destination: route.destination, path: route.path.clone(), }; - pv.path.push(ctx.ctx.hostname.clone()); + pv.path.push(ctx.ctx.router_id.clone()); underlay.insert(pv); } for route in &ctx.ctx.db.imported_tunnel() { @@ -491,7 +491,7 @@ async fn pull_handler( for prefix in &originated { let pv = v3::PathVector { destination: *prefix, - path: vec![ctx.ctx.hostname.clone()], + path: vec![ctx.ctx.router_id.clone()], }; underlay.insert(pv); } @@ -632,7 +632,7 @@ fn handle_underlay_update( for prefix in &update.announce { // Skip announcements with ourselves in the path e.g. path vector // loop breaking. - if prefix.path.contains(&ctx.hostname) { + if prefix.path.contains(&ctx.router_id) { continue; } import.insert(Route { diff --git a/ddm/src/sm/mod.rs b/ddm/src/sm/mod.rs index 8dc85144f..8d3fdc60f 100644 --- a/ddm/src/sm/mod.rs +++ b/ddm/src/sm/mod.rs @@ -21,6 +21,7 @@ use std::sync::mpsc::{Receiver, Sender}; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; use thiserror::Error; +use uuid::Uuid; #[cfg(target_os = "illumos")] use std::collections::BTreeSet; @@ -174,6 +175,12 @@ pub struct Config { /// Dendrite dpd config pub dpd: Option, + + /// Rack ID + pub rack_id: Option, + + /// Sled ID + pub sled_id: Option, } #[derive(Clone)] @@ -282,7 +289,7 @@ pub struct SmContext { pub tx: Sender, pub event_channels: Vec>, pub rt: Arc, - pub hostname: String, + pub router_id: String, pub iface: Arc, pub stats: Arc, pub log: Logger, diff --git a/ddm/src/sm/state.rs b/ddm/src/sm/state.rs index 4f70d74b2..67e004b00 100644 --- a/ddm/src/sm/state.rs +++ b/ddm/src/sm/state.rs @@ -137,7 +137,7 @@ impl State for Init { // Now that we have an ip address to run discovery on, start the // discovery handler and jump into the solicit state. let discovery_stop = match discovery::handler( - self.ctx.hostname.clone(), + self.ctx.router_id.clone(), self.ctx.config.clone(), self.ctx.tx.clone(), self.ctx.iface.clone(), @@ -423,7 +423,7 @@ impl Exchange { destination: x.destination, path: { let mut ps = x.path.clone(); - ps.push(self.ctx.hostname.clone()); + ps.push(self.ctx.router_id.clone()); ps }, }) @@ -513,7 +513,7 @@ impl State for Exchange { .iter() .map(|x| PathVector { destination: *x, - path: vec![self.ctx.hostname.clone()], + path: vec![self.ctx.router_id.clone()], }) .collect(); if let Err(e) = crate::exchange::announce_underlay( @@ -597,7 +597,7 @@ impl State for Exchange { .iter() .map(|x| PathVector { destination: *x, - path: vec![self.ctx.hostname.clone()], + path: vec![self.ctx.router_id.clone()], }) .collect(); if let Err(e) = crate::exchange::withdraw_underlay( @@ -727,7 +727,7 @@ impl State for Exchange { .collect(), withdraw: HashSet::default(), } - .with_path_element(self.ctx.hostname.clone()), + .with_path_element(self.ctx.router_id.clone()), ), tunnel: None, }))) @@ -838,8 +838,31 @@ impl State for Exchange { self.ctx.event_channels.len() ); update.underlay = update.underlay.map(|u| { - u.break_loops(&self.ctx.hostname) - .with_path_element(self.ctx.hostname.clone()) + let sans_loops = u.break_loops(&self.ctx.router_id); + + let announce_loop = + u.announce.difference(&sans_loops.announce); + let withdraw_loop = + u.withdraw.difference(&sans_loops.withdraw); + + for x in announce_loop { + wrn!( + self.log, + self.ctx.config.if_name, + "loop detected: dropping announcement: {:?}", + x, + ) + } + for x in withdraw_loop { + wrn!( + self.log, + self.ctx.config.if_name, + "loop detected: dropping withdraw {:?}", + x, + ) + } + + sans_loops.with_path_element(self.ctx.router_id.clone()) }); super::send( Event::Peer(PeerEvent::Push(update)), diff --git a/ddmd/src/main.rs b/ddmd/src/main.rs index 7dc3036db..004dc9776 100644 --- a/ddmd/src/main.rs +++ b/ddmd/src/main.rs @@ -166,7 +166,12 @@ async fn run() { .to_string_lossy() .to_string(); - let sms = start_state_machines(&arg, &db, &dpd, &hostname, &rt, &log); + let router_id = match arg.sled_uuid { + Some(id) => id.to_string(), + None => hostname.clone(), + }; + + let sms = start_state_machines(&arg, &db, &dpd, &router_id, &rt, &log); termination_handler(db.clone(), dpd.clone(), rt.clone(), log.clone()); @@ -193,6 +198,9 @@ async fn run() { exchange_tcp_port: arg.exchange_port, }, log: log.clone(), + rack_id: arg.rack_uuid, + sled_id: arg.sled_uuid, + router_id: router_id.clone(), })); if arg.with_stats @@ -245,7 +253,7 @@ fn start_state_machines( arg: &Arg, db: &Db, dpd: &Option, - hostname: &str, + router_id: &str, rt: &Arc, log: &Logger, ) -> Vec { @@ -274,6 +282,8 @@ fn start_state_machines( kind: arg.kind, dpd: dpd.clone(), addr: Ipv6Addr::UNSPECIFIED, + sled_id: arg.sled_uuid, + rack_id: arg.rack_uuid, }; let ctx = SmContext { @@ -282,7 +292,7 @@ fn start_state_machines( event_channels: Vec::new(), tx: tx.clone(), log: log.clone(), - hostname: hostname.to_string(), + router_id: router_id.to_string(), rt: rt.clone(), iface: Arc::new(InterfaceState::default()), stats: Arc::new(ddm::sm::SessionStats::default()), diff --git a/openapi/ddm-admin/ddm-admin-3.0.0-224165.json b/openapi/ddm-admin/ddm-admin-3.0.0-3255d9.json similarity index 99% rename from openapi/ddm-admin/ddm-admin-3.0.0-224165.json rename to openapi/ddm-admin/ddm-admin-3.0.0-3255d9.json index 408c184f9..894267329 100644 --- a/openapi/ddm-admin/ddm-admin-3.0.0-224165.json +++ b/openapi/ddm-admin/ddm-admin-3.0.0-3255d9.json @@ -73,7 +73,7 @@ } } }, - "post": { + "put": { "operationId": "set_external_peers", "requestBody": { "content": { diff --git a/openapi/ddm-admin/ddm-admin-latest.json b/openapi/ddm-admin/ddm-admin-latest.json index 597684728..49ff483ef 120000 --- a/openapi/ddm-admin/ddm-admin-latest.json +++ b/openapi/ddm-admin/ddm-admin-latest.json @@ -1 +1 @@ -ddm-admin-3.0.0-224165.json \ No newline at end of file +ddm-admin-3.0.0-3255d9.json \ No newline at end of file From 26fcf262ba3be2af6156a5a010cd834e18ae42c7 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Thu, 17 Sep 2026 22:48:13 +0000 Subject: [PATCH 25/29] ddmadm: path element per line now that these are uuids, this is the ohly way --- ddmadm/src/main.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/ddmadm/src/main.rs b/ddmadm/src/main.rs index 60a1145ed..3cd230a46 100644 --- a/ddmadm/src/main.rs +++ b/ddmadm/src/main.rs @@ -180,13 +180,16 @@ async fn run() -> Result<()> { for pv in &mut destinations { // show path from perspective of this node, e.g. nearest node // first - pv.path.reverse(); - let strpath = pv.path.join(" "); - writeln!( - &mut tw, - "{}\t{}\t{}", - pv.destination, nexthop, strpath, - )?; + if let Some(p) = pv.path.pop() { + writeln!( + &mut tw, + "{}\t{}\t{}", + pv.destination, nexthop, p, + )?; + } + for p in &pv.path { + writeln!(&mut tw, "\t\t{}", p,)?; + } } } tw.flush()?; From 182a8df88a961e111eb83bad974b06ccaa3b83a3 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Sat, 19 Sep 2026 06:06:23 +0000 Subject: [PATCH 26/29] add an explicit router-id parameter to ddmd --- ddmd/src/main.rs | 8 ++++++-- smf/ddm/manifest.xml | 1 + smf/ddm_method_script.sh | 6 ++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/ddmd/src/main.rs b/ddmd/src/main.rs index 004dc9776..5cdb48816 100644 --- a/ddmd/src/main.rs +++ b/ddmd/src/main.rs @@ -113,6 +113,10 @@ struct Arg { #[arg(long)] sled_uuid: Option, + // Explicitly set router id instead of using hostname + #[arg(long)] + router_id: Option, + /// Serve only the admin API. Skips the routing state machine /// (discovery, exchange, route synchronization), allowing test fixtures /// to obtain a real `ddmd` admin endpoint without the kernel-level @@ -166,8 +170,8 @@ async fn run() { .to_string_lossy() .to_string(); - let router_id = match arg.sled_uuid { - Some(id) => id.to_string(), + let router_id = match &arg.router_id { + Some(id) => id.clone(), None => hostname.clone(), }; diff --git a/smf/ddm/manifest.xml b/smf/ddm/manifest.xml index 5e5da1cf2..86c3ac819 100644 --- a/smf/ddm/manifest.xml +++ b/smf/ddm/manifest.xml @@ -29,6 +29,7 @@ + diff --git a/smf/ddm_method_script.sh b/smf/ddm_method_script.sh index 706da75f6..2b1bada64 100755 --- a/smf/ddm_method_script.sh +++ b/smf/ddm_method_script.sh @@ -46,6 +46,12 @@ if [[ "$val" != 'unknown' ]]; then args+=( "$val" ) fi +val=$(svcprop -c -p config/sled_uuid "${SMF_FMRI}") +if [[ "$val" != 'unknown' ]]; then + args+=( '--router-id' ) + args+=( "$val" ) +fi + for x in $(svcprop -c -p config/interfaces "${SMF_FMRI}"); do args+=( '-a' ) args+=( "$x" ) From 23b7c04ee23d7303c1f56153fe48d8b04c194265 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Sat, 19 Sep 2026 06:40:29 +0000 Subject: [PATCH 27/29] :facepalm: --- smf/ddm_method_script.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smf/ddm_method_script.sh b/smf/ddm_method_script.sh index 2b1bada64..a1e32a11b 100755 --- a/smf/ddm_method_script.sh +++ b/smf/ddm_method_script.sh @@ -46,7 +46,7 @@ if [[ "$val" != 'unknown' ]]; then args+=( "$val" ) fi -val=$(svcprop -c -p config/sled_uuid "${SMF_FMRI}") +val=$(svcprop -c -p config/router_id "${SMF_FMRI}") if [[ "$val" != 'unknown' ]]; then args+=( '--router-id' ) args+=( "$val" ) From e34fe56bae9c37f2e748cba0e6dfb1a5991d1512 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Mon, 28 Sep 2026 16:09:44 +0000 Subject: [PATCH 28/29] ajs feeback --- ddm/src/admin.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddm/src/admin.rs b/ddm/src/admin.rs index 50d2c26c3..9adc372ca 100644 --- a/ddm/src/admin.rs +++ b/ddm/src/admin.rs @@ -577,7 +577,7 @@ impl DdmAdminApi for DdmAdminApiImpl { } } } - // remove peers back to front so we don't shift the order our from under + // remove peers back to front so we don't shift the order out from under // ourselves for the indexes we just gathered. for i in remove_idx.iter().rev() { ctx.peers.remove(*i); From d2bfae353c74e90be55c6a17993fcb2fbb49399f Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Fri, 2 Oct 2026 05:29:17 +0000 Subject: [PATCH 29/29] trey's feedback --- .github/buildomat/jobs/test-ddm-sextet.sh | 0 ddm-protocol/src/v3.rs | 7 +------ 2 files changed, 1 insertion(+), 6 deletions(-) mode change 100644 => 100755 .github/buildomat/jobs/test-ddm-sextet.sh diff --git a/.github/buildomat/jobs/test-ddm-sextet.sh b/.github/buildomat/jobs/test-ddm-sextet.sh old mode 100644 new mode 100755 diff --git a/ddm-protocol/src/v3.rs b/ddm-protocol/src/v3.rs index 0420d8b30..ab4aed6e5 100644 --- a/ddm-protocol/src/v3.rs +++ b/ddm-protocol/src/v3.rs @@ -125,12 +125,7 @@ impl UnderlayUpdate { .filter(|x| !x.path.contains(hostname)) .cloned() .collect(), - withdraw: self - .withdraw - .iter() - .filter(|x| !x.path.contains(hostname)) - .cloned() - .collect(), + withdraw: self.withdraw.clone(), } } }