From 7d28f5d810cdaf9e23a4a01270cdb17ba9e79dec Mon Sep 17 00:00:00 2001 From: Steve Karam Date: Thu, 3 Sep 2026 11:15:53 -0400 Subject: [PATCH 1/6] sled-agent: Detect switch backend at runtime Replace the switch-asic, switch-stub, and switch-softnpu cargo features with startup detection. The propolis SoftNPU backend is selected by a virtio 9p device answering 9P2000.P4; without one the existing Tofino path is used. Optional switch_backend config added for the stub and SoftNPU zone backends. pumpkind is gated on Oxide hardware. --- package-manifest.toml | 6 ++ sled-agent/Cargo.toml | 3 - sled-agent/src/bootstrap/pre_server.rs | 96 +++++++++++------ sled-agent/src/bootstrap/pumpkind.rs | 31 ++++-- sled-agent/src/bootstrap/server.rs | 7 +- sled-agent/src/config.rs | 17 +++ sled-hardware/src/illumos/mod.rs | 2 + sled-hardware/src/illumos/softnpu.rs | 142 +++++++++++++++++++++++++ sled-hardware/src/lib.rs | 5 +- sled-hardware/src/non_illumos/mod.rs | 7 ++ sled-hardware/src/softnpu.rs | 120 +++++++++++++++++++++ smf/sled-agent/non-gimlet/config.toml | 6 ++ 12 files changed, 393 insertions(+), 49 deletions(-) create mode 100644 sled-hardware/src/illumos/softnpu.rs create mode 100644 sled-hardware/src/softnpu.rs diff --git a/package-manifest.toml b/package-manifest.toml index edb753576ce..663b55bb5b7 100644 --- a/package-manifest.toml +++ b/package-manifest.toml @@ -872,6 +872,8 @@ output.type = "zone" # $ cargo run --release --bin omicron-package -- -t default target create -p dev -m -s stub # $ cargo run --release --bin omicron-package -- package # $ pfexec ./target/release/omicron-package install +# +# The sled-agent config must also set switch_backend = "tofino_stub". [package.switch-stub] service_name = "switch" only_for_targets.switch = "stub" @@ -899,6 +901,10 @@ output.type = "zone" # $ cargo run --release --bin omicron-package -- -t default target create -p dev -m # $ cargo run --release --bin omicron-package -- package # $ pfexec ./target/release/omicron-package install +# +# The sled-agent config selects between a SoftNPU zone on this host +# (switch_backend = "soft_npu_zone") and a propolis SoftNPU device, which is +# detected at startup. [package.switch-softnpu] service_name = "switch" only_for_targets.switch = "softnpu" diff --git a/sled-agent/Cargo.toml b/sled-agent/Cargo.toml index 47577484c32..037d491b7c9 100644 --- a/sled-agent/Cargo.toml +++ b/sled-agent/Cargo.toml @@ -179,6 +179,3 @@ doc = false [features] image-trampoline = [] -switch-asic = [] -switch-stub = [] -switch-softnpu = [] diff --git a/sled-agent/src/bootstrap/pre_server.rs b/sled-agent/src/bootstrap/pre_server.rs index d3578f75037..b435a7e6570 100644 --- a/sled-agent/src/bootstrap/pre_server.rs +++ b/sled-agent/src/bootstrap/pre_server.rs @@ -116,7 +116,7 @@ impl BootstrapAgentStartup { BootstrapNetworking::enable_ipv6_forwarding().await?; // Are we a gimlet or scrimlet? - let sled_mode = sled_mode_from_config(&config)?; + let sled_mode = sled_mode_from_config(&config, &log).await?; // Spawn all important long running tasks that live for the lifetime of // the process and are used by both the bootstrap agent and sled agent @@ -290,45 +290,77 @@ async fn ensure_zfs_ramdisk_dataset() -> Result<(), StartError> { .map_err(StartError::EnsureZfsRamdiskDataset) } -// Combine the `sled_mode` config with the build-time switch type to determine -// the actual sled mode. -fn sled_mode_from_config(config: &Config) -> Result { +// Combine the `sled_mode` and `switch_backend` config with detected switch +// hardware to determine the actual sled mode. +// +// A SoftNPU 9p device selects the propolis SoftNPU backend. Without one, +// `auto` keeps the hardware monitor's Tofino detection and `scrimlet` assumes +// a Tofino ASIC only when a physical sidecar revision is configured. +async fn sled_mode_from_config( + config: &Config, + log: &Logger, +) -> Result { use crate::config::SledMode as SledModeConfig; - let sled_mode = match config.sled_mode { - SledModeConfig::Auto => { - if !cfg!(feature = "switch-asic") { - return Err(StartError::IncorrectBuildPackaging( - "sled-agent was not packaged with `switch-asic`", + use crate::config::SwitchBackend; + + let sled_mode = match (&config.sled_mode, &config.switch_backend) { + (SledModeConfig::Sled, _) => SledMode::Sled, + (SledModeConfig::Scrimlet, SwitchBackend::TofinoStub) => { + SledMode::Scrimlet { asic: DendriteAsic::TofinoStub } + } + (SledModeConfig::Scrimlet, SwitchBackend::SoftNpuZone) => { + if !matches!(config.sidecar_revision, SidecarRevision::SoftZone(_)) + { + return Err(StartError::SledModeConfig( + "switch_backend soft_npu_zone requires \ + sidecar_revision.soft_zone", )); } - SledMode::Auto + SledMode::Scrimlet { asic: DendriteAsic::SoftNpuZone } + } + (SledModeConfig::Auto, SwitchBackend::TofinoStub) + | (SledModeConfig::Auto, SwitchBackend::SoftNpuZone) => { + return Err(StartError::SledModeConfig( + "switch_backend override requires sled_mode = \"scrimlet\"", + )); } - SledModeConfig::Sled => SledMode::Sled, - SledModeConfig::Scrimlet => { - let asic = if cfg!(feature = "switch-asic") { - DendriteAsic::TofinoAsic - } else if cfg!(feature = "switch-stub") { - DendriteAsic::TofinoStub - } else if cfg!(feature = "switch-softnpu") { - match config.sidecar_revision { - SidecarRevision::SoftZone(_) => DendriteAsic::SoftNpuZone, - SidecarRevision::SoftPropolis(_) => { - DendriteAsic::SoftNpuPropolisDevice + (mode, SwitchBackend::Detect) => { + let probe_log = log.clone(); + let softnpu = tokio::task::spawn_blocking(move || { + sled_hardware::find_softnpu_device(&probe_log) + }) + .await + .expect("SoftNPU probe panicked") + .map_err(StartError::DetectSwitch)?; + + match (mode, softnpu) { + (_, Some(path)) => { + if !matches!( + config.sidecar_revision, + SidecarRevision::SoftPropolis(_) + ) { + return Err(StartError::SledModeConfig( + "SoftNPU device present but sidecar_revision is \ + not soft_propolis", + )); } - _ => { - return Err(StartError::IncorrectBuildPackaging( - "sled-agent configured to run on softnpu zone but dosen't \ - have a softnpu sidecar revision", + info!(log, "SoftNPU device detected"; "path" => path); + SledMode::Scrimlet { + asic: DendriteAsic::SoftNpuPropolisDevice, + } + } + (SledModeConfig::Auto, None) => SledMode::Auto, + (SledModeConfig::Scrimlet, None) => { + if !config.sidecar_revision.is_physical() { + return Err(StartError::SledModeConfig( + "sled_mode is scrimlet but no SoftNPU device is \ + present and sidecar_revision is not physical", )); } + SledMode::Scrimlet { asic: DendriteAsic::TofinoAsic } } - } else { - return Err(StartError::IncorrectBuildPackaging( - "sled-agent configured to run on scrimlet but wasn't \ - packaged with switch zone", - )); - }; - SledMode::Scrimlet { asic } + (SledModeConfig::Sled, None) => SledMode::Sled, + } } }; Ok(sled_mode) diff --git a/sled-agent/src/bootstrap/pumpkind.rs b/sled-agent/src/bootstrap/pumpkind.rs index 63be799519d..1c5acfc964d 100644 --- a/sled-agent/src/bootstrap/pumpkind.rs +++ b/sled-agent/src/bootstrap/pumpkind.rs @@ -6,6 +6,10 @@ use thiserror::Error; +const SERVICE_FMRI: &str = "svc:/oxide/pumpkind"; +const MANIFEST_PATH: &str = + "/opt/oxide/pumpkind/lib/svc/manifest/system/pumpkind.xml"; + #[derive(Debug, Error)] pub enum Error { #[error("Error configuring service: {0}")] @@ -13,13 +17,25 @@ pub enum Error { #[error("Error administering service: {0}")] Adm(#[from] smf::AdmError), + + #[error("Error detecting Oxide sled: {0}")] + Detect(anyhow::Error), } -#[cfg(feature = "switch-asic")] +/// Import and enable pumpkind on Oxide sleds with the manifest installed. pub(super) fn enable_pumpkind_service(log: &slog::Logger) -> Result<(), Error> { - const SERVICE_FMRI: &str = "svc:/oxide/pumpkind"; - const MANIFEST_PATH: &str = - "/opt/oxide/pumpkind/lib/svc/manifest/system/pumpkind.xml"; + if !sled_hardware::is_oxide_sled().map_err(Error::Detect)? { + info!(log, "not an Oxide sled; skipping pumpkind"); + return Ok(()); + } + if !std::path::Path::new(MANIFEST_PATH).exists() { + info!( + log, + "pumpkind manifest not installed; skipping"; + "path" => MANIFEST_PATH, + ); + return Ok(()); + } info!(log, "Importing pumpkind service"; "path" => MANIFEST_PATH); smf::Config::import().run(MANIFEST_PATH)?; @@ -32,10 +48,3 @@ pub(super) fn enable_pumpkind_service(log: &slog::Logger) -> Result<(), Error> { Ok(()) } - -#[cfg(not(feature = "switch-asic"))] -pub(super) fn enable_pumpkind_service( - _log: &slog::Logger, -) -> Result<(), Error> { - Ok(()) -} diff --git a/sled-agent/src/bootstrap/server.rs b/sled-agent/src/bootstrap/server.rs index 03b178e5c0a..611ee7d2494 100644 --- a/sled-agent/src/bootstrap/server.rs +++ b/sled-agent/src/bootstrap/server.rs @@ -131,8 +131,11 @@ pub enum StartError { #[error("Failed to enable ipv6-forwarding")] EnableIpv6Forwarding(#[from] illumos_utils::ExecutionError), - #[error("Incorrect binary packaging: {0}")] - IncorrectBuildPackaging(&'static str), + #[error("Invalid sled mode configuration: {0}")] + SledModeConfig(&'static str), + + #[error("Failed to detect switch hardware")] + DetectSwitch(#[source] sled_hardware::SoftNpuDetectError), #[error("Failed to start HardwareManager: {0}")] StartHardwareManager(String), diff --git a/sled-agent/src/config.rs b/sled-agent/src/config.rs index 5f4d2d83be3..b7ba1c7c341 100644 --- a/sled-agent/src/config.rs +++ b/sled-agent/src/config.rs @@ -26,6 +26,18 @@ pub enum SledMode { Scrimlet, } +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum SwitchBackend { + /// Probe for switch hardware: a SoftNPU 9p device, else the Tofino ASIC + #[default] + Detect, + /// Run the stub Dendrite; no switch hardware + TofinoStub, + /// Run Dendrite against a SoftNPU zone on this host + SoftNpuZone, +} + #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "snake_case")] pub enum SidecarRevision { @@ -66,6 +78,11 @@ pub struct Config { pub sled_mode: SledMode, // TODO: Remove once this can be auto-detected. pub sidecar_revision: SidecarRevision, + /// Which switch backend to run when acting as a scrimlet. Defaults to + /// hardware detection; the stub and zone backends must be requested + /// explicitly and require `sled_mode = "scrimlet"`. + #[serde(default)] + pub switch_backend: SwitchBackend, /// Optional percentage of otherwise-unbudgeted DRAM to reserve for guest /// memory, after accounting for expected host OS memory consumption and, if /// set, `vmm_reservoir_size_mb`. diff --git a/sled-hardware/src/illumos/mod.rs b/sled-hardware/src/illumos/mod.rs index 05641b76f99..75845227cab 100644 --- a/sled-hardware/src/illumos/mod.rs +++ b/sled-hardware/src/illumos/mod.rs @@ -27,9 +27,11 @@ use uuid::Uuid; mod gpt; mod partitions; +mod softnpu; mod sysconf; pub use partitions::{NvmeFormattingError, ensure_partition_layout}; +pub use softnpu::find_softnpu_device; const TOFINO_MONITOR: &'static str = "/opt/oxide/sled-agent/tofino-monitor"; diff --git a/sled-hardware/src/illumos/softnpu.rs b/sled-hardware/src/illumos/softnpu.rs new file mode 100644 index 00000000000..ca7ac7e049f --- /dev/null +++ b/sled-hardware/src/illumos/softnpu.rs @@ -0,0 +1,142 @@ +// 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/. + +//! Detection of the propolis SoftNPU 9p device. + +use crate::softnpu::{ + SOFTNPU_9P_VERSION, SoftNpuDetectError, decode_rversion, encode_tversion, +}; +use illumos_devinfo::{DevInfo, Node}; +use slog::{Logger, debug, info, warn}; +use std::fs::{File, OpenOptions}; +use std::io::{Read, Write}; +use std::os::unix::fs::OpenOptionsExt; +use std::time::Duration; + +const VIRTIO_VENDOR_ID: i32 = 0x1af4; +// Transitional and modern virtio 9p PCI device ids. +const VIRTIO_9P_DEVICE_IDS: [i32; 2] = [0x1009, 0x1049]; +const NINEP_MINOR: &str = "9p"; +const OPEN_ATTEMPTS: usize = 3; +const OPEN_RETRY_DELAY: Duration = Duration::from_millis(500); + +/// Returns the devfs path of the SoftNPU 9p device when one is attached. +/// +/// Every virtio 9p node with an attached driver is opened exclusively and +/// asked for its 9P version. Only the propolis SoftNPU handler answers with +/// `9P2000.P4`. A device that stays busy across retries, such as a mounted +/// 9p filesystem, is logged and skipped. +pub fn find_softnpu_device( + log: &Logger, +) -> Result, SoftNpuDetectError> { + let mut devinfo = + DevInfo::new_force_load().map_err(SoftNpuDetectError::DevInfo)?; + let mut walker = devinfo.walk_node(); + while let Some(node) = + walker.next().transpose().map_err(SoftNpuDetectError::DevInfo)? + { + if !is_virtio_9p(&node)? { + continue; + } + let Some(path) = ninep_minor_path(&node)? else { + debug!( + log, + "virtio 9p node has no {NINEP_MINOR} minor"; + "node" => node.node_name(), + ); + continue; + }; + match probe_version(&path) { + Ok(version) if version == SOFTNPU_9P_VERSION => { + info!(log, "found SoftNPU 9p device"; "path" => &path); + return Ok(Some(path)); + } + Ok(version) => { + debug!( + log, + "virtio 9p device is not SoftNPU"; + "path" => &path, + "version" => version, + ); + } + Err(SoftNpuDetectError::Busy { path }) => { + warn!(log, "virtio 9p device busy; skipping"; "path" => path); + } + Err(e) => return Err(e), + } + } + Ok(None) +} + +fn is_virtio_9p(node: &Node<'_>) -> Result { + let mut vendor = None; + let mut device = None; + for prop in node.props() { + let prop = prop.map_err(SoftNpuDetectError::DevInfo)?; + match prop.name().as_str() { + "vendor-id" => vendor = prop.as_i32(), + "device-id" => device = prop.as_i32(), + _ => {} + } + } + Ok(vendor == Some(VIRTIO_VENDOR_ID) + && device.is_some_and(|d| VIRTIO_9P_DEVICE_IDS.contains(&d))) +} + +fn ninep_minor_path( + node: &Node<'_>, +) -> Result, SoftNpuDetectError> { + for minor in node.minors() { + let minor = minor.map_err(SoftNpuDetectError::DevInfo)?; + if minor.name() == NINEP_MINOR { + let path = + minor.devfs_path().map_err(SoftNpuDetectError::DevInfo)?; + return Ok(Some(format!("/devices{path}"))); + } + } + Ok(None) +} + +/// One Tversion/Rversion exchange over the vio9p character device. +/// +/// The driver permits a single exclusive open, so EBUSY means another +/// consumer such as scadm or a 9p mount currently holds the device. +fn probe_version(path: &str) -> Result { + for attempt in 1..=OPEN_ATTEMPTS { + match OpenOptions::new() + .read(true) + .write(true) + .custom_flags(libc::O_EXCL) + .open(path) + { + Ok(file) => return exchange_version(path, file), + Err(e) if e.raw_os_error() == Some(libc::EBUSY) => { + if attempt < OPEN_ATTEMPTS { + std::thread::sleep(OPEN_RETRY_DELAY); + } + } + Err(err) => { + return Err(SoftNpuDetectError::Io { + path: path.to_string(), + err, + }); + } + } + } + Err(SoftNpuDetectError::Busy { path: path.to_string() }) +} + +fn exchange_version( + path: &str, + mut file: File, +) -> Result { + let io = |err| SoftNpuDetectError::Io { path: path.to_string(), err }; + file.write_all(&encode_tversion(SOFTNPU_9P_VERSION)).map_err(io)?; + let mut buf = vec![0u8; 65536]; + let n = file.read(&mut buf).map_err(io)?; + decode_rversion(&buf[..n]).map_err(|reason| SoftNpuDetectError::Protocol { + path: path.to_string(), + reason, + }) +} diff --git a/sled-hardware/src/lib.rs b/sled-hardware/src/lib.rs index d0ad7db5e6b..01beb01f497 100644 --- a/sled-hardware/src/lib.rs +++ b/sled-hardware/src/lib.rs @@ -23,6 +23,8 @@ cfg_if::cfg_if! { pub mod cleanup; pub mod disk; pub use disk::*; +pub mod softnpu; +pub use softnpu::SoftNpuDetectError; pub mod underlay; // The type of networking 'ASIC' the Dendrite service is expected to manage @@ -83,7 +85,8 @@ pub enum ExternalDisks { /// Configuration for forcing a sled to run as a Scrimlet or compute Sled #[derive(Copy, Clone, Debug)] pub enum SledMode { - /// Automatically detect whether to run as a compute sled or Scrimlet (w/ real Tofino ASIC) + /// Run as a compute sled unless a Tofino ASIC is present, in which case + /// run as a Scrimlet Auto, /// Force sled to run as a Gimlet Sled, diff --git a/sled-hardware/src/non_illumos/mod.rs b/sled-hardware/src/non_illumos/mod.rs index 1d962d293de..4a0789a7f6c 100644 --- a/sled-hardware/src/non_illumos/mod.rs +++ b/sled-hardware/src/non_illumos/mod.rs @@ -86,3 +86,10 @@ pub async fn ensure_partition_layout( pub fn is_oxide_sled() -> anyhow::Result { Ok(false) } + +/// Return the devfs path of the SoftNPU 9p device, if one is attached. +pub fn find_softnpu_device( + _log: &Logger, +) -> Result, crate::softnpu::SoftNpuDetectError> { + Ok(None) +} diff --git a/sled-hardware/src/softnpu.rs b/sled-hardware/src/softnpu.rs new file mode 100644 index 00000000000..f15927346b5 --- /dev/null +++ b/sled-hardware/src/softnpu.rs @@ -0,0 +1,120 @@ +// 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/. + +//! 9P wire format used to identify the propolis SoftNPU device. +//! +//! Propolis exposes SoftNPU to a guest as a virtio 9p device whose server +//! answers Tversion with the version string `9P2000.P4`. The PCI ids match +//! any other virtio 9p device, so the version exchange is the discriminator. + +/// Version string served by the propolis SoftNPU 9p handler. +pub const SOFTNPU_9P_VERSION: &str = "9P2000.P4"; + +/// Maximum message size offered in Tversion. +pub const MSIZE: u32 = 8192; + +const TVERSION: u8 = 100; +const RVERSION: u8 = 101; +const NOTAG: u16 = 0xffff; + +#[derive(Debug, thiserror::Error)] +pub enum SoftNpuDetectError { + #[error("failed to walk device tree: {0}")] + DevInfo(anyhow::Error), + + #[error("{path}: device busy")] + Busy { path: String }, + + #[error("{path}: {err}")] + Io { + path: String, + #[source] + err: std::io::Error, + }, + + #[error("{path}: malformed Rversion: {reason}")] + Protocol { path: String, reason: String }, +} + +/// Encode a Tversion message. +/// +/// Layout: size[4] type[1] tag[2] msize[4] version[s], little endian. +pub fn encode_tversion(version: &str) -> Vec { + let version = version.as_bytes(); + let size = (4 + 1 + 2 + 4 + 2 + version.len()) as u32; + let mut msg = Vec::with_capacity(size as usize); + msg.extend_from_slice(&size.to_le_bytes()); + msg.push(TVERSION); + msg.extend_from_slice(&NOTAG.to_le_bytes()); + msg.extend_from_slice(&MSIZE.to_le_bytes()); + msg.extend_from_slice(&(version.len() as u16).to_le_bytes()); + msg.extend_from_slice(version); + msg +} + +/// Decode the version string from an Rversion message. +/// +/// Layout: size[4] type[1] tag[2] msize[4] version[s], little endian. +pub fn decode_rversion(msg: &[u8]) -> Result { + const HEADER_LEN: usize = 4 + 1 + 2 + 4 + 2; + if msg.len() < HEADER_LEN { + return Err(format!("short reply ({} bytes)", msg.len())); + } + if msg[4] != RVERSION { + return Err(format!("unexpected message type {}", msg[4])); + } + let len = u16::from_le_bytes([msg[11], msg[12]]) as usize; + let version = msg + .get(HEADER_LEN..HEADER_LEN + len) + .ok_or_else(|| "truncated version string".to_string())?; + Ok(String::from_utf8_lossy(version).into_owned()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn rversion(version: &[u8]) -> Vec { + let size = (4 + 1 + 2 + 4 + 2 + version.len()) as u32; + let mut msg = Vec::new(); + msg.extend_from_slice(&size.to_le_bytes()); + msg.push(RVERSION); + msg.extend_from_slice(&NOTAG.to_le_bytes()); + msg.extend_from_slice(&MSIZE.to_le_bytes()); + msg.extend_from_slice(&(version.len() as u16).to_le_bytes()); + msg.extend_from_slice(version); + msg + } + + #[test] + fn tversion_layout() { + let msg = encode_tversion(SOFTNPU_9P_VERSION); + assert_eq!(msg.len(), 13 + SOFTNPU_9P_VERSION.len()); + assert_eq!(u32::from_le_bytes(msg[0..4].try_into().unwrap()), 22); + assert_eq!(msg[4], TVERSION); + assert_eq!(&msg[13..], SOFTNPU_9P_VERSION.as_bytes()); + } + + #[test] + fn rversion_roundtrip() { + let msg = rversion(SOFTNPU_9P_VERSION.as_bytes()); + assert_eq!(decode_rversion(&msg).unwrap(), SOFTNPU_9P_VERSION); + let msg = rversion(b"9P2000.L"); + assert_eq!(decode_rversion(&msg).unwrap(), "9P2000.L"); + } + + #[test] + fn rversion_malformed() { + assert!(decode_rversion(&[]).is_err()); + assert!(decode_rversion(&[0; 12]).is_err()); + + let mut msg = rversion(SOFTNPU_9P_VERSION.as_bytes()); + msg[4] = TVERSION; + assert!(decode_rversion(&msg).is_err()); + + let mut msg = rversion(SOFTNPU_9P_VERSION.as_bytes()); + msg.truncate(msg.len() - 1); + assert!(decode_rversion(&msg).is_err()); + } +} diff --git a/smf/sled-agent/non-gimlet/config.toml b/smf/sled-agent/non-gimlet/config.toml index 4f46cd56cb7..735afff5530 100644 --- a/smf/sled-agent/non-gimlet/config.toml +++ b/smf/sled-agent/non-gimlet/config.toml @@ -14,6 +14,12 @@ sled_mode = "scrimlet" # this information. sidecar_revision.soft_zone = { front_port_count = 1, rear_port_count = 1 } +# Selects the switch backend for a scrimlet. "detect" (the default) probes for +# a SoftNPU 9p device and otherwise expects a Tofino ASIC. "soft_npu_zone" runs +# Dendrite against a SoftNPU zone on this host and "tofino_stub" runs the stub +# Dendrite. Both overrides require sled_mode = "scrimlet". +switch_backend = "soft_npu_zone" + # Setting this to true causes sled-agent to always report that its time is # in-sync, rather than querying its NTP zone. skip_timesync = false From bb2b375c8f45afb01fe0e3f54b5faefbf48e3564 Mon Sep 17 00:00:00 2001 From: Steve Karam Date: Thu, 3 Sep 2026 15:37:51 -0400 Subject: [PATCH 2/6] Check tofino before softnpu in switch detection --- sled-agent/src/bootstrap/pre_server.rs | 74 +++++++++++++------------- sled-agent/src/bootstrap/server.rs | 2 +- sled-hardware/src/illumos/mod.rs | 23 +++++++- sled-hardware/src/illumos/softnpu.rs | 59 ++++++++++---------- sled-hardware/src/lib.rs | 29 +++++++++- sled-hardware/src/non_illumos/mod.rs | 6 +-- sled-hardware/src/softnpu.rs | 56 +++++++------------ 7 files changed, 142 insertions(+), 107 deletions(-) diff --git a/sled-agent/src/bootstrap/pre_server.rs b/sled-agent/src/bootstrap/pre_server.rs index b435a7e6570..3bd63d5f320 100644 --- a/sled-agent/src/bootstrap/pre_server.rs +++ b/sled-agent/src/bootstrap/pre_server.rs @@ -38,6 +38,7 @@ use omicron_common::address::Ipv6Subnet; use sled_agent_config_reconciler::ConfigReconcilerSpawnToken; use sled_hardware::DendriteAsic; use sled_hardware::SledMode; +use sled_hardware::SwitchHardware; use sled_hardware::underlay; use sled_hardware::underlay::BootstrapInterface; use slog::Drain; @@ -293,9 +294,9 @@ async fn ensure_zfs_ramdisk_dataset() -> Result<(), StartError> { // Combine the `sled_mode` and `switch_backend` config with detected switch // hardware to determine the actual sled mode. // -// A SoftNPU 9p device selects the propolis SoftNPU backend. Without one, -// `auto` keeps the hardware monitor's Tofino detection and `scrimlet` assumes -// a Tofino ASIC only when a physical sidecar revision is configured. +// Detected hardware wins in the priority order of `SwitchHardware`. With none +// detected, `auto` leaves Tofino detection to the hardware monitor and +// `scrimlet` assumes a Tofino ASIC when a physical sidecar is configured. async fn sled_mode_from_config( config: &Config, log: &Logger, @@ -303,12 +304,22 @@ async fn sled_mode_from_config( use crate::config::SledMode as SledModeConfig; use crate::config::SwitchBackend; - let sled_mode = match (&config.sled_mode, &config.switch_backend) { - (SledModeConfig::Sled, _) => SledMode::Sled, - (SledModeConfig::Scrimlet, SwitchBackend::TofinoStub) => { - SledMode::Scrimlet { asic: DendriteAsic::TofinoStub } + let forced_scrimlet = match config.sled_mode { + SledModeConfig::Sled => return Ok(SledMode::Sled), + SledModeConfig::Auto => false, + SledModeConfig::Scrimlet => true, + }; + + let asic = match config.switch_backend { + SwitchBackend::TofinoStub | SwitchBackend::SoftNpuZone + if !forced_scrimlet => + { + return Err(StartError::SledModeConfig( + "switch_backend override requires sled_mode = \"scrimlet\"", + )); } - (SledModeConfig::Scrimlet, SwitchBackend::SoftNpuZone) => { + SwitchBackend::TofinoStub => DendriteAsic::TofinoStub, + SwitchBackend::SoftNpuZone => { if !matches!(config.sidecar_revision, SidecarRevision::SoftZone(_)) { return Err(StartError::SledModeConfig( @@ -316,25 +327,20 @@ async fn sled_mode_from_config( sidecar_revision.soft_zone", )); } - SledMode::Scrimlet { asic: DendriteAsic::SoftNpuZone } - } - (SledModeConfig::Auto, SwitchBackend::TofinoStub) - | (SledModeConfig::Auto, SwitchBackend::SoftNpuZone) => { - return Err(StartError::SledModeConfig( - "switch_backend override requires sled_mode = \"scrimlet\"", - )); + DendriteAsic::SoftNpuZone } - (mode, SwitchBackend::Detect) => { + SwitchBackend::Detect => { let probe_log = log.clone(); - let softnpu = tokio::task::spawn_blocking(move || { - sled_hardware::find_softnpu_device(&probe_log) + let detected = tokio::task::spawn_blocking(move || { + sled_hardware::detect_switch_hardware(&probe_log) }) .await - .expect("SoftNPU probe panicked") + .expect("switch hardware detection panicked") .map_err(StartError::DetectSwitch)?; - match (mode, softnpu) { - (_, Some(path)) => { + match detected { + Some(SwitchHardware::Tofino) => DendriteAsic::TofinoAsic, + Some(SwitchHardware::SoftNpuPropolis { .. }) => { if !matches!( config.sidecar_revision, SidecarRevision::SoftPropolis(_) @@ -344,26 +350,22 @@ async fn sled_mode_from_config( not soft_propolis", )); } - info!(log, "SoftNPU device detected"; "path" => path); - SledMode::Scrimlet { - asic: DendriteAsic::SoftNpuPropolisDevice, - } + DendriteAsic::SoftNpuPropolisDevice } - (SledModeConfig::Auto, None) => SledMode::Auto, - (SledModeConfig::Scrimlet, None) => { - if !config.sidecar_revision.is_physical() { - return Err(StartError::SledModeConfig( - "sled_mode is scrimlet but no SoftNPU device is \ - present and sidecar_revision is not physical", - )); - } - SledMode::Scrimlet { asic: DendriteAsic::TofinoAsic } + None if !forced_scrimlet => return Ok(SledMode::Auto), + None if config.sidecar_revision.is_physical() => { + DendriteAsic::TofinoAsic + } + None => { + return Err(StartError::SledModeConfig( + "sled_mode is scrimlet but no switch hardware was \ + detected and sidecar_revision is not physical", + )); } - (SledModeConfig::Sled, None) => SledMode::Sled, } } }; - Ok(sled_mode) + Ok(SledMode::Scrimlet { asic }) } #[derive(Debug, Clone)] diff --git a/sled-agent/src/bootstrap/server.rs b/sled-agent/src/bootstrap/server.rs index 611ee7d2494..c20eabbec56 100644 --- a/sled-agent/src/bootstrap/server.rs +++ b/sled-agent/src/bootstrap/server.rs @@ -135,7 +135,7 @@ pub enum StartError { SledModeConfig(&'static str), #[error("Failed to detect switch hardware")] - DetectSwitch(#[source] sled_hardware::SoftNpuDetectError), + DetectSwitch(#[source] sled_hardware::SwitchDetectError), #[error("Failed to start HardwareManager: {0}")] StartHardwareManager(String), diff --git a/sled-hardware/src/illumos/mod.rs b/sled-hardware/src/illumos/mod.rs index 75845227cab..667c0976270 100644 --- a/sled-hardware/src/illumos/mod.rs +++ b/sled-hardware/src/illumos/mod.rs @@ -7,7 +7,9 @@ use crate::ExternalDisks; use crate::HardwareView; use crate::TofinoSnapshot; use crate::TofinoView; -use crate::{DendriteAsic, SledMode, UnparsedDisk}; +use crate::{ + DendriteAsic, SledMode, SwitchDetectError, SwitchHardware, UnparsedDisk, +}; use camino::Utf8PathBuf; use gethostname::gethostname; use illumos_devinfo::{DevInfo, DevLinkType, DevLinks, Node, Property}; @@ -31,10 +33,27 @@ mod softnpu; mod sysconf; pub use partitions::{NvmeFormattingError, ensure_partition_layout}; -pub use softnpu::find_softnpu_device; const TOFINO_MONITOR: &'static str = "/opt/oxide/sled-agent/tofino-monitor"; +/// Detect attached switch hardware, checking each backend in the priority +/// order of [`SwitchHardware`]. A Tofino node counts whether or not its +/// driver is attached; availability is tracked by the hardware monitor. +pub fn detect_switch_hardware( + log: &Logger, +) -> Result, SwitchDetectError> { + let mut devinfo = + DevInfo::new_force_load().map_err(SwitchDetectError::DevInfo)?; + if let Some(node) = tofino::get_tofino_from_devinfo(&mut devinfo) + .map_err(SwitchDetectError::Tofino)? + { + info!(log, "found tofino node"; "path" => node.devfs_path); + return Ok(Some(SwitchHardware::Tofino)); + } + Ok(softnpu::find_softnpu_device(log, &mut devinfo)? + .map(|path| SwitchHardware::SoftNpuPropolis { path })) +} + #[derive(thiserror::Error, Debug)] enum Error { #[error("Failed to access devinfo: {0}")] diff --git a/sled-hardware/src/illumos/softnpu.rs b/sled-hardware/src/illumos/softnpu.rs index ca7ac7e049f..94e580fc1d3 100644 --- a/sled-hardware/src/illumos/softnpu.rs +++ b/sled-hardware/src/illumos/softnpu.rs @@ -4,9 +4,8 @@ //! Detection of the propolis SoftNPU 9p device. -use crate::softnpu::{ - SOFTNPU_9P_VERSION, SoftNpuDetectError, decode_rversion, encode_tversion, -}; +use crate::SwitchDetectError; +use crate::softnpu::{SOFTNPU_9P_VERSION, decode_rversion, encode_tversion}; use illumos_devinfo::{DevInfo, Node}; use slog::{Logger, debug, info, warn}; use std::fs::{File, OpenOptions}; @@ -20,6 +19,12 @@ const VIRTIO_9P_DEVICE_IDS: [i32; 2] = [0x1009, 0x1049]; const NINEP_MINOR: &str = "9p"; const OPEN_ATTEMPTS: usize = 3; const OPEN_RETRY_DELAY: Duration = Duration::from_millis(500); +const REPLY_BUF_LEN: usize = 65536; + +enum Probe { + Version(String), + Busy, +} /// Returns the devfs path of the SoftNPU 9p device when one is attached. /// @@ -27,14 +32,13 @@ const OPEN_RETRY_DELAY: Duration = Duration::from_millis(500); /// asked for its 9P version. Only the propolis SoftNPU handler answers with /// `9P2000.P4`. A device that stays busy across retries, such as a mounted /// 9p filesystem, is logged and skipped. -pub fn find_softnpu_device( +pub(super) fn find_softnpu_device( log: &Logger, -) -> Result, SoftNpuDetectError> { - let mut devinfo = - DevInfo::new_force_load().map_err(SoftNpuDetectError::DevInfo)?; + devinfo: &mut DevInfo, +) -> Result, SwitchDetectError> { let mut walker = devinfo.walk_node(); while let Some(node) = - walker.next().transpose().map_err(SoftNpuDetectError::DevInfo)? + walker.next().transpose().map_err(SwitchDetectError::DevInfo)? { if !is_virtio_9p(&node)? { continue; @@ -47,33 +51,32 @@ pub fn find_softnpu_device( ); continue; }; - match probe_version(&path) { - Ok(version) if version == SOFTNPU_9P_VERSION => { + match probe_version(&path)? { + Probe::Version(version) if version == SOFTNPU_9P_VERSION => { info!(log, "found SoftNPU 9p device"; "path" => &path); return Ok(Some(path)); } - Ok(version) => { + Probe::Version(version) => { debug!( log, "virtio 9p device is not SoftNPU"; - "path" => &path, + "path" => path, "version" => version, ); } - Err(SoftNpuDetectError::Busy { path }) => { + Probe::Busy => { warn!(log, "virtio 9p device busy; skipping"; "path" => path); } - Err(e) => return Err(e), } } Ok(None) } -fn is_virtio_9p(node: &Node<'_>) -> Result { +fn is_virtio_9p(node: &Node<'_>) -> Result { let mut vendor = None; let mut device = None; for prop in node.props() { - let prop = prop.map_err(SoftNpuDetectError::DevInfo)?; + let prop = prop.map_err(SwitchDetectError::DevInfo)?; match prop.name().as_str() { "vendor-id" => vendor = prop.as_i32(), "device-id" => device = prop.as_i32(), @@ -86,12 +89,12 @@ fn is_virtio_9p(node: &Node<'_>) -> Result { fn ninep_minor_path( node: &Node<'_>, -) -> Result, SoftNpuDetectError> { +) -> Result, SwitchDetectError> { for minor in node.minors() { - let minor = minor.map_err(SoftNpuDetectError::DevInfo)?; + let minor = minor.map_err(SwitchDetectError::DevInfo)?; if minor.name() == NINEP_MINOR { let path = - minor.devfs_path().map_err(SoftNpuDetectError::DevInfo)?; + minor.devfs_path().map_err(SwitchDetectError::DevInfo)?; return Ok(Some(format!("/devices{path}"))); } } @@ -102,7 +105,7 @@ fn ninep_minor_path( /// /// The driver permits a single exclusive open, so EBUSY means another /// consumer such as scadm or a 9p mount currently holds the device. -fn probe_version(path: &str) -> Result { +fn probe_version(path: &str) -> Result { for attempt in 1..=OPEN_ATTEMPTS { match OpenOptions::new() .read(true) @@ -110,32 +113,34 @@ fn probe_version(path: &str) -> Result { .custom_flags(libc::O_EXCL) .open(path) { - Ok(file) => return exchange_version(path, file), + Ok(file) => { + return exchange_version(path, file).map(Probe::Version); + } Err(e) if e.raw_os_error() == Some(libc::EBUSY) => { if attempt < OPEN_ATTEMPTS { std::thread::sleep(OPEN_RETRY_DELAY); } } Err(err) => { - return Err(SoftNpuDetectError::Io { + return Err(SwitchDetectError::Io { path: path.to_string(), err, }); } } } - Err(SoftNpuDetectError::Busy { path: path.to_string() }) + Ok(Probe::Busy) } fn exchange_version( path: &str, mut file: File, -) -> Result { - let io = |err| SoftNpuDetectError::Io { path: path.to_string(), err }; +) -> Result { + let io = |err| SwitchDetectError::Io { path: path.to_string(), err }; file.write_all(&encode_tversion(SOFTNPU_9P_VERSION)).map_err(io)?; - let mut buf = vec![0u8; 65536]; + let mut buf = vec![0u8; REPLY_BUF_LEN]; let n = file.read(&mut buf).map_err(io)?; - decode_rversion(&buf[..n]).map_err(|reason| SoftNpuDetectError::Protocol { + decode_rversion(&buf[..n]).map_err(|reason| SwitchDetectError::Protocol { path: path.to_string(), reason, }) diff --git a/sled-hardware/src/lib.rs b/sled-hardware/src/lib.rs index 01beb01f497..25bcb2ba631 100644 --- a/sled-hardware/src/lib.rs +++ b/sled-hardware/src/lib.rs @@ -24,9 +24,36 @@ pub mod cleanup; pub mod disk; pub use disk::*; pub mod softnpu; -pub use softnpu::SoftNpuDetectError; pub mod underlay; +/// Switch hardware attached to a sled, in detection priority order. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum SwitchHardware { + /// Tofino ASIC node in the device tree + Tofino, + /// Propolis SoftNPU virtio 9p device at this devfs path + SoftNpuPropolis { path: String }, +} + +#[derive(Debug, thiserror::Error)] +pub enum SwitchDetectError { + #[error("failed to walk device tree: {0}")] + DevInfo(anyhow::Error), + + #[error("failed to look up tofino node: {0}")] + Tofino(anyhow::Error), + + #[error("{path}: {err}")] + Io { + path: String, + #[source] + err: std::io::Error, + }, + + #[error("{path}: malformed Rversion: {reason}")] + Protocol { path: String, reason: String }, +} + // The type of networking 'ASIC' the Dendrite service is expected to manage #[derive( Copy, Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq, Eq, Hash, diff --git a/sled-hardware/src/non_illumos/mod.rs b/sled-hardware/src/non_illumos/mod.rs index 4a0789a7f6c..2c0bb4bae08 100644 --- a/sled-hardware/src/non_illumos/mod.rs +++ b/sled-hardware/src/non_illumos/mod.rs @@ -87,9 +87,9 @@ pub fn is_oxide_sled() -> anyhow::Result { Ok(false) } -/// Return the devfs path of the SoftNPU 9p device, if one is attached. -pub fn find_softnpu_device( +/// Detect attached switch hardware. +pub fn detect_switch_hardware( _log: &Logger, -) -> Result, crate::softnpu::SoftNpuDetectError> { +) -> Result, crate::SwitchDetectError> { Ok(None) } diff --git a/sled-hardware/src/softnpu.rs b/sled-hardware/src/softnpu.rs index f15927346b5..fc34c0478af 100644 --- a/sled-hardware/src/softnpu.rs +++ b/sled-hardware/src/softnpu.rs @@ -11,38 +11,18 @@ /// Version string served by the propolis SoftNPU 9p handler. pub const SOFTNPU_9P_VERSION: &str = "9P2000.P4"; -/// Maximum message size offered in Tversion. -pub const MSIZE: u32 = 8192; - +const MSIZE: u32 = 8192; const TVERSION: u8 = 100; const RVERSION: u8 = 101; const NOTAG: u16 = 0xffff; - -#[derive(Debug, thiserror::Error)] -pub enum SoftNpuDetectError { - #[error("failed to walk device tree: {0}")] - DevInfo(anyhow::Error), - - #[error("{path}: device busy")] - Busy { path: String }, - - #[error("{path}: {err}")] - Io { - path: String, - #[source] - err: std::io::Error, - }, - - #[error("{path}: malformed Rversion: {reason}")] - Protocol { path: String, reason: String }, -} +const HEADER_LEN: usize = 4 + 1 + 2 + 4 + 2; /// Encode a Tversion message. /// -/// Layout: size[4] type[1] tag[2] msize[4] version[s], little endian. +/// Layout: `size[4] type[1] tag[2] msize[4] version[s]`, little endian. pub fn encode_tversion(version: &str) -> Vec { let version = version.as_bytes(); - let size = (4 + 1 + 2 + 4 + 2 + version.len()) as u32; + let size = (HEADER_LEN + version.len()) as u32; let mut msg = Vec::with_capacity(size as usize); msg.extend_from_slice(&size.to_le_bytes()); msg.push(TVERSION); @@ -55,9 +35,8 @@ pub fn encode_tversion(version: &str) -> Vec { /// Decode the version string from an Rversion message. /// -/// Layout: size[4] type[1] tag[2] msize[4] version[s], little endian. +/// Layout: `size[4] type[1] tag[2] msize[4] version[s]`, little endian. pub fn decode_rversion(msg: &[u8]) -> Result { - const HEADER_LEN: usize = 4 + 1 + 2 + 4 + 2; if msg.len() < HEADER_LEN { return Err(format!("short reply ({} bytes)", msg.len())); } @@ -76,13 +55,13 @@ mod tests { use super::*; fn rversion(version: &[u8]) -> Vec { - let size = (4 + 1 + 2 + 4 + 2 + version.len()) as u32; - let mut msg = Vec::new(); - msg.extend_from_slice(&size.to_le_bytes()); - msg.push(RVERSION); - msg.extend_from_slice(&NOTAG.to_le_bytes()); - msg.extend_from_slice(&MSIZE.to_le_bytes()); - msg.extend_from_slice(&(version.len() as u16).to_le_bytes()); + let mut msg = encode_tversion(""); + msg.truncate(HEADER_LEN); + msg[0..4].copy_from_slice( + &((HEADER_LEN + version.len()) as u32).to_le_bytes(), + ); + msg[4] = RVERSION; + msg[11..13].copy_from_slice(&(version.len() as u16).to_le_bytes()); msg.extend_from_slice(version); msg } @@ -90,10 +69,13 @@ mod tests { #[test] fn tversion_layout() { let msg = encode_tversion(SOFTNPU_9P_VERSION); - assert_eq!(msg.len(), 13 + SOFTNPU_9P_VERSION.len()); - assert_eq!(u32::from_le_bytes(msg[0..4].try_into().unwrap()), 22); + assert_eq!(msg.len(), HEADER_LEN + SOFTNPU_9P_VERSION.len()); + assert_eq!( + u32::from_le_bytes(msg[0..4].try_into().unwrap()), + msg.len() as u32 + ); assert_eq!(msg[4], TVERSION); - assert_eq!(&msg[13..], SOFTNPU_9P_VERSION.as_bytes()); + assert_eq!(&msg[HEADER_LEN..], SOFTNPU_9P_VERSION.as_bytes()); } #[test] @@ -107,7 +89,7 @@ mod tests { #[test] fn rversion_malformed() { assert!(decode_rversion(&[]).is_err()); - assert!(decode_rversion(&[0; 12]).is_err()); + assert!(decode_rversion(&[0; HEADER_LEN - 1]).is_err()); let mut msg = rversion(SOFTNPU_9P_VERSION.as_bytes()); msg[4] = TVERSION; From ae078bc56bcbaa2d5683f9a68189e79a5ae6ecc1 Mon Sep 17 00:00:00 2001 From: Steve Karam Date: Thu, 3 Sep 2026 17:52:54 -0400 Subject: [PATCH 3/6] Add sled mode resolution tests --- sled-agent/src/bootstrap/pre_server.rs | 228 ++++++++++++++++++++----- sled-hardware/src/lib.rs | 2 +- 2 files changed, 187 insertions(+), 43 deletions(-) diff --git a/sled-agent/src/bootstrap/pre_server.rs b/sled-agent/src/bootstrap/pre_server.rs index 3bd63d5f320..886ba99edd8 100644 --- a/sled-agent/src/bootstrap/pre_server.rs +++ b/sled-agent/src/bootstrap/pre_server.rs @@ -15,6 +15,8 @@ use super::pumpkind; use super::server::StartError; use crate::config::Config; use crate::config::SidecarRevision; +use crate::config::SledMode as SledModeConfig; +use crate::config::SwitchBackend; use crate::ddm_reconciler::DdmReconciler; use crate::long_running_tasks::{ LongRunningTaskHandles, LongRunningTaskResult, spawn_all_longrunning_tasks, @@ -292,25 +294,49 @@ async fn ensure_zfs_ramdisk_dataset() -> Result<(), StartError> { } // Combine the `sled_mode` and `switch_backend` config with detected switch -// hardware to determine the actual sled mode. -// -// Detected hardware wins in the priority order of `SwitchHardware`. With none -// detected, `auto` leaves Tofino detection to the hardware monitor and -// `scrimlet` assumes a Tofino ASIC when a physical sidecar is configured. +// hardware to determine the actual sled mode. Hardware is only probed when +// the config leaves the backend to detection. async fn sled_mode_from_config( config: &Config, log: &Logger, ) -> Result { - use crate::config::SledMode as SledModeConfig; - use crate::config::SwitchBackend; + let detected = if matches!(config.sled_mode, SledModeConfig::Sled) + || config.switch_backend != SwitchBackend::Detect + { + None + } else { + let probe_log = log.clone(); + tokio::task::spawn_blocking(move || { + sled_hardware::detect_switch_hardware(&probe_log) + }) + .await + .expect("switch hardware detection panicked") + .map_err(StartError::DetectSwitch)? + }; + resolve_sled_mode( + &config.sled_mode, + &config.switch_backend, + &config.sidecar_revision, + detected, + ) +} - let forced_scrimlet = match config.sled_mode { +// Detected hardware wins in the priority order of `SwitchHardware`. With none +// detected, `auto` leaves Tofino detection to the hardware monitor and +// `scrimlet` assumes a Tofino ASIC when a physical sidecar is configured. +fn resolve_sled_mode( + sled_mode: &SledModeConfig, + switch_backend: &SwitchBackend, + sidecar_revision: &SidecarRevision, + detected: Option, +) -> Result { + let forced_scrimlet = match sled_mode { SledModeConfig::Sled => return Ok(SledMode::Sled), SledModeConfig::Auto => false, SledModeConfig::Scrimlet => true, }; - let asic = match config.switch_backend { + let asic = match switch_backend { SwitchBackend::TofinoStub | SwitchBackend::SoftNpuZone if !forced_scrimlet => { @@ -320,8 +346,7 @@ async fn sled_mode_from_config( } SwitchBackend::TofinoStub => DendriteAsic::TofinoStub, SwitchBackend::SoftNpuZone => { - if !matches!(config.sidecar_revision, SidecarRevision::SoftZone(_)) - { + if !matches!(sidecar_revision, SidecarRevision::SoftZone(_)) { return Err(StartError::SledModeConfig( "switch_backend soft_npu_zone requires \ sidecar_revision.soft_zone", @@ -329,45 +354,164 @@ async fn sled_mode_from_config( } DendriteAsic::SoftNpuZone } - SwitchBackend::Detect => { - let probe_log = log.clone(); - let detected = tokio::task::spawn_blocking(move || { - sled_hardware::detect_switch_hardware(&probe_log) - }) - .await - .expect("switch hardware detection panicked") - .map_err(StartError::DetectSwitch)?; - - match detected { - Some(SwitchHardware::Tofino) => DendriteAsic::TofinoAsic, - Some(SwitchHardware::SoftNpuPropolis { .. }) => { - if !matches!( - config.sidecar_revision, - SidecarRevision::SoftPropolis(_) - ) { - return Err(StartError::SledModeConfig( - "SoftNPU device present but sidecar_revision is \ - not soft_propolis", - )); - } - DendriteAsic::SoftNpuPropolisDevice - } - None if !forced_scrimlet => return Ok(SledMode::Auto), - None if config.sidecar_revision.is_physical() => { - DendriteAsic::TofinoAsic - } - None => { + SwitchBackend::Detect => match detected { + Some(SwitchHardware::Tofino) => DendriteAsic::TofinoAsic, + Some(SwitchHardware::SoftNpuPropolis { .. }) => { + if !matches!(sidecar_revision, SidecarRevision::SoftPropolis(_)) + { return Err(StartError::SledModeConfig( - "sled_mode is scrimlet but no switch hardware was \ - detected and sidecar_revision is not physical", + "SoftNPU device present but sidecar_revision is \ + not soft_propolis", )); } + DendriteAsic::SoftNpuPropolisDevice } - } + None if !forced_scrimlet => return Ok(SledMode::Auto), + None if sidecar_revision.is_physical() => DendriteAsic::TofinoAsic, + None => { + return Err(StartError::SledModeConfig( + "sled_mode is scrimlet but no switch hardware was \ + detected and sidecar_revision is not physical", + )); + } + }, }; Ok(SledMode::Scrimlet { asic }) } +#[cfg(test)] +mod tests { + use super::*; + use crate::config::SoftPortConfig; + + const AUTO: SledModeConfig = SledModeConfig::Auto; + const SLED: SledModeConfig = SledModeConfig::Sled; + const SCRIMLET: SledModeConfig = SledModeConfig::Scrimlet; + const DETECT: SwitchBackend = SwitchBackend::Detect; + const STUB: SwitchBackend = SwitchBackend::TofinoStub; + const ZONE: SwitchBackend = SwitchBackend::SoftNpuZone; + + #[derive(Clone, Copy)] + enum Sidecar { + Physical, + SoftPropolis, + SoftZone, + } + + impl From for SidecarRevision { + fn from(sidecar: Sidecar) -> Self { + let ports = + SoftPortConfig { front_port_count: 2, rear_port_count: 4 }; + match sidecar { + Sidecar::Physical => SidecarRevision::Physical("b".to_string()), + Sidecar::SoftPropolis => SidecarRevision::SoftPropolis(ports), + Sidecar::SoftZone => SidecarRevision::SoftZone(ports), + } + } + } + + #[derive(Clone, Copy)] + enum Found { + Nothing, + Tofino, + SoftNpu, + } + + impl From for Option { + fn from(found: Found) -> Self { + match found { + Found::Nothing => None, + Found::Tofino => Some(SwitchHardware::Tofino), + Found::SoftNpu => Some(SwitchHardware::SoftNpuPropolis { + path: "/devices/pci@0,0/pci1af4,9@6:9p".to_string(), + }), + } + } + } + + #[derive(Debug, PartialEq)] + enum Expect { + Mode(SledMode), + ConfigError, + } + + use DendriteAsic::*; + use Expect::*; + use Found::*; + use Sidecar::*; + + fn scrimlet(asic: DendriteAsic) -> Expect { + Mode(SledMode::Scrimlet { asic }) + } + + #[test] + fn resolve_sled_mode_table() { + let cases = [ + // Production gimlet config: auto with a physical sidecar. Nothing + // detected leaves Tofino detection to the hardware monitor. + (AUTO, DETECT, Physical, Nothing, Mode(SledMode::Auto)), + (AUTO, DETECT, Physical, Tofino, scrimlet(TofinoAsic)), + // gimlet-standalone: forced scrimlet with a physical sidecar, + // including a Tofino whose driver has not attached yet. + (SCRIMLET, DETECT, Physical, Tofino, scrimlet(TofinoAsic)), + (SCRIMLET, DETECT, Physical, Nothing, scrimlet(TofinoAsic)), + // Tofino wins under any sidecar config. + (AUTO, DETECT, SoftPropolis, Tofino, scrimlet(TofinoAsic)), + (AUTO, DETECT, SoftZone, Tofino, scrimlet(TofinoAsic)), + // A forced sled ignores attached hardware. + (SLED, DETECT, Physical, Tofino, Mode(SledMode::Sled)), + (SLED, DETECT, SoftPropolis, SoftNpu, Mode(SledMode::Sled)), + (SLED, STUB, Physical, Nothing, Mode(SledMode::Sled)), + // Voxel: forced scrimlets carry a SoftNPU device; auto works the + // same way with the device deciding. + ( + SCRIMLET, + DETECT, + SoftPropolis, + SoftNpu, + scrimlet(SoftNpuPropolisDevice), + ), + ( + AUTO, + DETECT, + SoftPropolis, + SoftNpu, + scrimlet(SoftNpuPropolisDevice), + ), + (AUTO, DETECT, SoftPropolis, Nothing, Mode(SledMode::Auto)), + // Forced scrimlet with nothing detected needs a physical sidecar. + (SCRIMLET, DETECT, SoftPropolis, Nothing, ConfigError), + (SCRIMLET, DETECT, SoftZone, Nothing, ConfigError), + // SoftNPU needs a soft_propolis sidecar. + (SCRIMLET, DETECT, Physical, SoftNpu, ConfigError), + (AUTO, DETECT, SoftZone, SoftNpu, ConfigError), + // Overrides: dev SoftNPU zone and stub Dendrite, forced scrimlet + // only, and the zone needs a soft_zone sidecar. + (SCRIMLET, ZONE, SoftZone, Nothing, scrimlet(SoftNpuZone)), + (SCRIMLET, ZONE, SoftPropolis, Nothing, ConfigError), + (SCRIMLET, STUB, Physical, Nothing, scrimlet(TofinoStub)), + (AUTO, STUB, Physical, Nothing, ConfigError), + (AUTO, ZONE, SoftZone, Nothing, ConfigError), + ]; + + for (i, (mode, backend, sidecar, found, expected)) in + cases.into_iter().enumerate() + { + let actual = match resolve_sled_mode( + &mode, + &backend, + &sidecar.into(), + found.into(), + ) { + Ok(mode) => Mode(mode), + Err(StartError::SledModeConfig(_)) => ConfigError, + Err(e) => panic!("case {i}: unexpected error {e:?}"), + }; + assert_eq!(actual, expected, "case {i}"); + } + } +} + #[derive(Debug, Clone)] pub(crate) struct BootstrapNetworking { pub(crate) bootstrap_etherstub: dladm::Etherstub, diff --git a/sled-hardware/src/lib.rs b/sled-hardware/src/lib.rs index 25bcb2ba631..66727a86bcd 100644 --- a/sled-hardware/src/lib.rs +++ b/sled-hardware/src/lib.rs @@ -110,7 +110,7 @@ pub enum ExternalDisks { } /// Configuration for forcing a sled to run as a Scrimlet or compute Sled -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum SledMode { /// Run as a compute sled unless a Tofino ASIC is present, in which case /// run as a Scrimlet From 5d176e0be7e5cc0d3288c1bef10005ec31f2cd7c Mon Sep 17 00:00:00 2001 From: Steve Karam Date: Fri, 4 Sep 2026 10:21:00 -0400 Subject: [PATCH 4/6] Skip 9p devices that fail to probe. Only devinfo or tofino lookup issues deserve to be fatal. --- sled-hardware/src/illumos/softnpu.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/sled-hardware/src/illumos/softnpu.rs b/sled-hardware/src/illumos/softnpu.rs index 94e580fc1d3..a2d35c12ea0 100644 --- a/sled-hardware/src/illumos/softnpu.rs +++ b/sled-hardware/src/illumos/softnpu.rs @@ -8,6 +8,7 @@ use crate::SwitchDetectError; use crate::softnpu::{SOFTNPU_9P_VERSION, decode_rversion, encode_tversion}; use illumos_devinfo::{DevInfo, Node}; use slog::{Logger, debug, info, warn}; +use slog_error_chain::InlineErrorChain; use std::fs::{File, OpenOptions}; use std::io::{Read, Write}; use std::os::unix::fs::OpenOptionsExt; @@ -30,8 +31,9 @@ enum Probe { /// /// Every virtio 9p node with an attached driver is opened exclusively and /// asked for its 9P version. Only the propolis SoftNPU handler answers with -/// `9P2000.P4`. A device that stays busy across retries, such as a mounted -/// 9p filesystem, is logged and skipped. +/// `9P2000.P4`. A device that stays busy across retries, fails to open, or +/// answers with anything other than an Rversion is logged and skipped. Only +/// device tree failures are fatal. pub(super) fn find_softnpu_device( log: &Logger, devinfo: &mut DevInfo, @@ -51,12 +53,12 @@ pub(super) fn find_softnpu_device( ); continue; }; - match probe_version(&path)? { - Probe::Version(version) if version == SOFTNPU_9P_VERSION => { + match probe_version(&path) { + Ok(Probe::Version(version)) if version == SOFTNPU_9P_VERSION => { info!(log, "found SoftNPU 9p device"; "path" => &path); return Ok(Some(path)); } - Probe::Version(version) => { + Ok(Probe::Version(version)) => { debug!( log, "virtio 9p device is not SoftNPU"; @@ -64,9 +66,20 @@ pub(super) fn find_softnpu_device( "version" => version, ); } - Probe::Busy => { + Ok(Probe::Busy) => { warn!(log, "virtio 9p device busy; skipping"; "path" => path); } + Err( + e @ (SwitchDetectError::Io { .. } + | SwitchDetectError::Protocol { .. }), + ) => { + warn!( + log, + "virtio 9p device probe failed; skipping"; + "error" => InlineErrorChain::new(&e), + ); + } + Err(e) => return Err(e), } } Ok(None) From 967b0323876116e3088d51cb8c601e86f077a9a4 Mon Sep 17 00:00:00 2001 From: Steve Karam Date: Fri, 4 Sep 2026 10:52:13 -0400 Subject: [PATCH 5/6] Use the tofino snapshot for switch detection --- sled-hardware/src/illumos/mod.rs | 10 ++++------ sled-hardware/src/lib.rs | 3 --- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/sled-hardware/src/illumos/mod.rs b/sled-hardware/src/illumos/mod.rs index 667c0976270..8fef107041e 100644 --- a/sled-hardware/src/illumos/mod.rs +++ b/sled-hardware/src/illumos/mod.rs @@ -37,17 +37,15 @@ pub use partitions::{NvmeFormattingError, ensure_partition_layout}; const TOFINO_MONITOR: &'static str = "/opt/oxide/sled-agent/tofino-monitor"; /// Detect attached switch hardware, checking each backend in the priority -/// order of [`SwitchHardware`]. A Tofino node counts whether or not its -/// driver is attached; availability is tracked by the hardware monitor. +/// order of [`SwitchHardware`]. Tofino presence uses the same snapshot the +/// hardware monitor polls, so startup detection and `is_scrimlet` agree. pub fn detect_switch_hardware( log: &Logger, ) -> Result, SwitchDetectError> { let mut devinfo = DevInfo::new_force_load().map_err(SwitchDetectError::DevInfo)?; - if let Some(node) = tofino::get_tofino_from_devinfo(&mut devinfo) - .map_err(SwitchDetectError::Tofino)? - { - info!(log, "found tofino node"; "path" => node.devfs_path); + if get_tofino_snapshot(log, &mut devinfo).exists { + info!(log, "found tofino asic"); return Ok(Some(SwitchHardware::Tofino)); } Ok(softnpu::find_softnpu_device(log, &mut devinfo)? diff --git a/sled-hardware/src/lib.rs b/sled-hardware/src/lib.rs index 66727a86bcd..49306e72077 100644 --- a/sled-hardware/src/lib.rs +++ b/sled-hardware/src/lib.rs @@ -40,9 +40,6 @@ pub enum SwitchDetectError { #[error("failed to walk device tree: {0}")] DevInfo(anyhow::Error), - #[error("failed to look up tofino node: {0}")] - Tofino(anyhow::Error), - #[error("{path}: {err}")] Io { path: String, From c6b42fc64ad6b4ed2c14f2932b97b7b8234f9e78 Mon Sep 17 00:00:00 2001 From: Steve Karam Date: Fri, 4 Sep 2026 12:05:36 -0400 Subject: [PATCH 6/6] Validate Rversion fields and pass the probe into sled mode resolution --- sled-agent/src/bootstrap/pre_server.rs | 216 +++++++++++++------------ sled-hardware/src/illumos/softnpu.rs | 89 +++++----- sled-hardware/src/softnpu.rs | 99 ++++++++---- 3 files changed, 228 insertions(+), 176 deletions(-) diff --git a/sled-agent/src/bootstrap/pre_server.rs b/sled-agent/src/bootstrap/pre_server.rs index 886ba99edd8..c863f2372fa 100644 --- a/sled-agent/src/bootstrap/pre_server.rs +++ b/sled-agent/src/bootstrap/pre_server.rs @@ -40,6 +40,7 @@ use omicron_common::address::Ipv6Subnet; use sled_agent_config_reconciler::ConfigReconcilerSpawnToken; use sled_hardware::DendriteAsic; use sled_hardware::SledMode; +use sled_hardware::SwitchDetectError; use sled_hardware::SwitchHardware; use sled_hardware::underlay; use sled_hardware::underlay::BootstrapInterface; @@ -293,42 +294,39 @@ async fn ensure_zfs_ramdisk_dataset() -> Result<(), StartError> { .map_err(StartError::EnsureZfsRamdiskDataset) } -// Combine the `sled_mode` and `switch_backend` config with detected switch -// hardware to determine the actual sled mode. Hardware is only probed when -// the config leaves the backend to detection. +// Combine the `sled_mode` and `switch_backend` config with switch hardware to +// determine the actual sled mode. Detection touches devinfo and device nodes, +// so the whole decision runs on a blocking thread. async fn sled_mode_from_config( config: &Config, log: &Logger, ) -> Result { - let detected = if matches!(config.sled_mode, SledModeConfig::Sled) - || config.switch_backend != SwitchBackend::Detect - { - None - } else { - let probe_log = log.clone(); - tokio::task::spawn_blocking(move || { - sled_hardware::detect_switch_hardware(&probe_log) - }) - .await - .expect("switch hardware detection panicked") - .map_err(StartError::DetectSwitch)? - }; - resolve_sled_mode( - &config.sled_mode, - &config.switch_backend, - &config.sidecar_revision, - detected, - ) + let sled_mode = config.sled_mode.clone(); + let switch_backend = config.switch_backend.clone(); + let sidecar_revision = config.sidecar_revision.clone(); + let log = log.clone(); + tokio::task::spawn_blocking(move || { + resolve_sled_mode( + &sled_mode, + &switch_backend, + &sidecar_revision, + || sled_hardware::detect_switch_hardware(&log), + ) + }) + .await + .expect("sled mode resolution panicked") } // Detected hardware wins in the priority order of `SwitchHardware`. With none // detected, `auto` leaves Tofino detection to the hardware monitor and // `scrimlet` assumes a Tofino ASIC when a physical sidecar is configured. +// `detect` runs only when the config leaves the backend to detection; it is +// a parameter so the decision can be tested without hardware. fn resolve_sled_mode( sled_mode: &SledModeConfig, switch_backend: &SwitchBackend, sidecar_revision: &SidecarRevision, - detected: Option, + detect: impl FnOnce() -> Result, SwitchDetectError>, ) -> Result { let forced_scrimlet = match sled_mode { SledModeConfig::Sled => return Ok(SledMode::Sled), @@ -354,27 +352,33 @@ fn resolve_sled_mode( } DendriteAsic::SoftNpuZone } - SwitchBackend::Detect => match detected { - Some(SwitchHardware::Tofino) => DendriteAsic::TofinoAsic, - Some(SwitchHardware::SoftNpuPropolis { .. }) => { - if !matches!(sidecar_revision, SidecarRevision::SoftPropolis(_)) - { + SwitchBackend::Detect => { + match detect().map_err(StartError::DetectSwitch)? { + Some(SwitchHardware::Tofino) => DendriteAsic::TofinoAsic, + Some(SwitchHardware::SoftNpuPropolis { .. }) => { + if !matches!( + sidecar_revision, + SidecarRevision::SoftPropolis(_) + ) { + return Err(StartError::SledModeConfig( + "SoftNPU device present but sidecar_revision is \ + not soft_propolis", + )); + } + DendriteAsic::SoftNpuPropolisDevice + } + None if !forced_scrimlet => return Ok(SledMode::Auto), + None if sidecar_revision.is_physical() => { + DendriteAsic::TofinoAsic + } + None => { return Err(StartError::SledModeConfig( - "SoftNPU device present but sidecar_revision is \ - not soft_propolis", + "sled_mode is scrimlet but no switch hardware was \ + detected and sidecar_revision is not physical", )); } - DendriteAsic::SoftNpuPropolisDevice - } - None if !forced_scrimlet => return Ok(SledMode::Auto), - None if sidecar_revision.is_physical() => DendriteAsic::TofinoAsic, - None => { - return Err(StartError::SledModeConfig( - "sled_mode is scrimlet but no switch hardware was \ - detected and sidecar_revision is not physical", - )); } - }, + } }; Ok(SledMode::Scrimlet { asic }) } @@ -391,42 +395,30 @@ mod tests { const STUB: SwitchBackend = SwitchBackend::TofinoStub; const ZONE: SwitchBackend = SwitchBackend::SoftNpuZone; - #[derive(Clone, Copy)] - enum Sidecar { - Physical, - SoftPropolis, - SoftZone, + fn ports() -> SoftPortConfig { + SoftPortConfig { front_port_count: 2, rear_port_count: 4 } } - impl From for SidecarRevision { - fn from(sidecar: Sidecar) -> Self { - let ports = - SoftPortConfig { front_port_count: 2, rear_port_count: 4 }; - match sidecar { - Sidecar::Physical => SidecarRevision::Physical("b".to_string()), - Sidecar::SoftPropolis => SidecarRevision::SoftPropolis(ports), - Sidecar::SoftZone => SidecarRevision::SoftZone(ports), - } - } + fn physical() -> SidecarRevision { + SidecarRevision::Physical("b".to_string()) } - #[derive(Clone, Copy)] - enum Found { - Nothing, - Tofino, - SoftNpu, + fn soft_propolis() -> SidecarRevision { + SidecarRevision::SoftPropolis(ports()) } - impl From for Option { - fn from(found: Found) -> Self { - match found { - Found::Nothing => None, - Found::Tofino => Some(SwitchHardware::Tofino), - Found::SoftNpu => Some(SwitchHardware::SoftNpuPropolis { - path: "/devices/pci@0,0/pci1af4,9@6:9p".to_string(), - }), - } - } + fn soft_zone() -> SidecarRevision { + SidecarRevision::SoftZone(ports()) + } + + fn tofino() -> Option { + Some(SwitchHardware::Tofino) + } + + fn softnpu() -> Option { + Some(SwitchHardware::SoftNpuPropolis { + path: "/devices/pci@0,0/pci1af4,9@6:9p".to_string(), + }) } #[derive(Debug, PartialEq)] @@ -437,8 +429,6 @@ mod tests { use DendriteAsic::*; use Expect::*; - use Found::*; - use Sidecar::*; fn scrimlet(asic: DendriteAsic) -> Expect { Mode(SledMode::Scrimlet { asic }) @@ -449,49 +439,49 @@ mod tests { let cases = [ // Production gimlet config: auto with a physical sidecar. Nothing // detected leaves Tofino detection to the hardware monitor. - (AUTO, DETECT, Physical, Nothing, Mode(SledMode::Auto)), - (AUTO, DETECT, Physical, Tofino, scrimlet(TofinoAsic)), - // gimlet-standalone: forced scrimlet with a physical sidecar, - // including a Tofino whose driver has not attached yet. - (SCRIMLET, DETECT, Physical, Tofino, scrimlet(TofinoAsic)), - (SCRIMLET, DETECT, Physical, Nothing, scrimlet(TofinoAsic)), + (AUTO, DETECT, physical(), None, Mode(SledMode::Auto)), + (AUTO, DETECT, physical(), tofino(), scrimlet(TofinoAsic)), + // gimlet-standalone: scrimlet with a physical sidecar, including + // a Tofino whose driver has not attached yet. + (SCRIMLET, DETECT, physical(), tofino(), scrimlet(TofinoAsic)), + (SCRIMLET, DETECT, physical(), None, scrimlet(TofinoAsic)), // Tofino wins under any sidecar config. - (AUTO, DETECT, SoftPropolis, Tofino, scrimlet(TofinoAsic)), - (AUTO, DETECT, SoftZone, Tofino, scrimlet(TofinoAsic)), - // A forced sled ignores attached hardware. - (SLED, DETECT, Physical, Tofino, Mode(SledMode::Sled)), - (SLED, DETECT, SoftPropolis, SoftNpu, Mode(SledMode::Sled)), - (SLED, STUB, Physical, Nothing, Mode(SledMode::Sled)), - // Voxel: forced scrimlets carry a SoftNPU device; auto works the - // same way with the device deciding. + (AUTO, DETECT, soft_propolis(), tofino(), scrimlet(TofinoAsic)), + (AUTO, DETECT, soft_zone(), tofino(), scrimlet(TofinoAsic)), + // A sled ignores attached hardware. + (SLED, DETECT, physical(), tofino(), Mode(SledMode::Sled)), + (SLED, DETECT, soft_propolis(), softnpu(), Mode(SledMode::Sled)), + (SLED, STUB, physical(), None, Mode(SledMode::Sled)), + // Voxel: scrimlets carry a SoftNPU device; auto works the same + // way with the device deciding. ( SCRIMLET, DETECT, - SoftPropolis, - SoftNpu, + soft_propolis(), + softnpu(), scrimlet(SoftNpuPropolisDevice), ), ( AUTO, DETECT, - SoftPropolis, - SoftNpu, + soft_propolis(), + softnpu(), scrimlet(SoftNpuPropolisDevice), ), - (AUTO, DETECT, SoftPropolis, Nothing, Mode(SledMode::Auto)), - // Forced scrimlet with nothing detected needs a physical sidecar. - (SCRIMLET, DETECT, SoftPropolis, Nothing, ConfigError), - (SCRIMLET, DETECT, SoftZone, Nothing, ConfigError), + (AUTO, DETECT, soft_propolis(), None, Mode(SledMode::Auto)), + // Scrimlet with nothing detected needs a physical sidecar. + (SCRIMLET, DETECT, soft_propolis(), None, ConfigError), + (SCRIMLET, DETECT, soft_zone(), None, ConfigError), // SoftNPU needs a soft_propolis sidecar. - (SCRIMLET, DETECT, Physical, SoftNpu, ConfigError), - (AUTO, DETECT, SoftZone, SoftNpu, ConfigError), - // Overrides: dev SoftNPU zone and stub Dendrite, forced scrimlet - // only, and the zone needs a soft_zone sidecar. - (SCRIMLET, ZONE, SoftZone, Nothing, scrimlet(SoftNpuZone)), - (SCRIMLET, ZONE, SoftPropolis, Nothing, ConfigError), - (SCRIMLET, STUB, Physical, Nothing, scrimlet(TofinoStub)), - (AUTO, STUB, Physical, Nothing, ConfigError), - (AUTO, ZONE, SoftZone, Nothing, ConfigError), + (SCRIMLET, DETECT, physical(), softnpu(), ConfigError), + (AUTO, DETECT, soft_zone(), softnpu(), ConfigError), + // Overrides: dev SoftNPU zone and stub Dendrite, scrimlet only, + // and the zone needs a soft_zone sidecar. + (SCRIMLET, ZONE, soft_zone(), None, scrimlet(SoftNpuZone)), + (SCRIMLET, ZONE, soft_propolis(), None, ConfigError), + (SCRIMLET, STUB, physical(), None, scrimlet(TofinoStub)), + (AUTO, STUB, physical(), None, ConfigError), + (AUTO, ZONE, soft_zone(), None, ConfigError), ]; for (i, (mode, backend, sidecar, found, expected)) in @@ -500,8 +490,8 @@ mod tests { let actual = match resolve_sled_mode( &mode, &backend, - &sidecar.into(), - found.into(), + &sidecar, + || Ok(found), ) { Ok(mode) => Mode(mode), Err(StartError::SledModeConfig(_)) => ConfigError, @@ -510,6 +500,22 @@ mod tests { assert_eq!(actual, expected, "case {i}"); } } + + // Hardware is only probed when the backend is left to detection. + #[test] + fn no_probe_without_detect() { + let cases = [ + (SLED, DETECT, physical()), + (SCRIMLET, STUB, physical()), + (SCRIMLET, ZONE, soft_zone()), + ]; + for (mode, backend, sidecar) in cases { + resolve_sled_mode(&mode, &backend, &sidecar, || { + panic!("probe ran") + }) + .unwrap(); + } + } } #[derive(Debug, Clone)] diff --git a/sled-hardware/src/illumos/softnpu.rs b/sled-hardware/src/illumos/softnpu.rs index a2d35c12ea0..0cdfa4b6eb3 100644 --- a/sled-hardware/src/illumos/softnpu.rs +++ b/sled-hardware/src/illumos/softnpu.rs @@ -38,51 +38,62 @@ pub(super) fn find_softnpu_device( log: &Logger, devinfo: &mut DevInfo, ) -> Result, SwitchDetectError> { - let mut walker = devinfo.walk_node(); - while let Some(node) = - walker.next().transpose().map_err(SwitchDetectError::DevInfo)? - { - if !is_virtio_9p(&node)? { - continue; + for node in devinfo.walk_node() { + let node = node.map_err(SwitchDetectError::DevInfo)?; + if let Some(path) = probe_node(log, &node)? { + return Ok(Some(path)); } - let Some(path) = ninep_minor_path(&node)? else { + } + Ok(None) +} + +/// Returns the devfs path when `node` is the SoftNPU 9p device. +fn probe_node( + log: &Logger, + node: &Node<'_>, +) -> Result, SwitchDetectError> { + if !is_virtio_9p(node)? { + return Ok(None); + } + let Some(path) = ninep_minor_path(node)? else { + debug!( + log, + "virtio 9p node has no {NINEP_MINOR} minor"; + "node" => node.node_name(), + ); + return Ok(None); + }; + match probe_version(&path) { + Ok(Probe::Version(version)) if version == SOFTNPU_9P_VERSION => { + info!(log, "found SoftNPU 9p device"; "path" => &path); + Ok(Some(path)) + } + Ok(Probe::Version(version)) => { debug!( log, - "virtio 9p node has no {NINEP_MINOR} minor"; - "node" => node.node_name(), + "virtio 9p device is not SoftNPU"; + "path" => path, + "version" => version, ); - continue; - }; - match probe_version(&path) { - Ok(Probe::Version(version)) if version == SOFTNPU_9P_VERSION => { - info!(log, "found SoftNPU 9p device"; "path" => &path); - return Ok(Some(path)); - } - Ok(Probe::Version(version)) => { - debug!( - log, - "virtio 9p device is not SoftNPU"; - "path" => path, - "version" => version, - ); - } - Ok(Probe::Busy) => { - warn!(log, "virtio 9p device busy; skipping"; "path" => path); - } - Err( - e @ (SwitchDetectError::Io { .. } - | SwitchDetectError::Protocol { .. }), - ) => { - warn!( - log, - "virtio 9p device probe failed; skipping"; - "error" => InlineErrorChain::new(&e), - ); - } - Err(e) => return Err(e), + Ok(None) + } + Ok(Probe::Busy) => { + warn!(log, "virtio 9p device busy; skipping"; "path" => path); + Ok(None) } + Err( + e @ (SwitchDetectError::Io { .. } + | SwitchDetectError::Protocol { .. }), + ) => { + warn!( + log, + "virtio 9p device probe failed; skipping"; + "error" => InlineErrorChain::new(&e), + ); + Ok(None) + } + Err(e) => Err(e), } - Ok(None) } fn is_virtio_9p(node: &Node<'_>) -> Result { diff --git a/sled-hardware/src/softnpu.rs b/sled-hardware/src/softnpu.rs index fc34c0478af..535c8fedee0 100644 --- a/sled-hardware/src/softnpu.rs +++ b/sled-hardware/src/softnpu.rs @@ -11,41 +11,69 @@ /// Version string served by the propolis SoftNPU 9p handler. pub const SOFTNPU_9P_VERSION: &str = "9P2000.P4"; +/// Maximum message size offered in Tversion. The server must answer with a +/// value no larger than this. const MSIZE: u32 = 8192; const TVERSION: u8 = 100; const RVERSION: u8 = 101; const NOTAG: u16 = 0xffff; -const HEADER_LEN: usize = 4 + 1 + 2 + 4 + 2; + +// Field offsets in a version message, laid out as +// size[4] type[1] tag[2] msize[4] version[s], little endian. The string +// field is a 2-byte length followed by the bytes. +const TYPE_OFFSET: usize = 4; +const TAG_OFFSET: usize = 5; +const MSIZE_OFFSET: usize = 7; +const VERSION_LEN_OFFSET: usize = 11; +const VERSION_OFFSET: usize = 13; + +/// Total size of a version message carrying `version`. +fn msg_size(version: &[u8]) -> usize { + VERSION_OFFSET + version.len() +} /// Encode a Tversion message. -/// -/// Layout: `size[4] type[1] tag[2] msize[4] version[s]`, little endian. pub fn encode_tversion(version: &str) -> Vec { let version = version.as_bytes(); - let size = (HEADER_LEN + version.len()) as u32; - let mut msg = Vec::with_capacity(size as usize); + let size: u32 = + msg_size(version).try_into().expect("version message fits in u32"); + let len: u16 = + version.len().try_into().expect("version string fits in u16"); + let mut msg = Vec::with_capacity(msg_size(version)); msg.extend_from_slice(&size.to_le_bytes()); msg.push(TVERSION); msg.extend_from_slice(&NOTAG.to_le_bytes()); msg.extend_from_slice(&MSIZE.to_le_bytes()); - msg.extend_from_slice(&(version.len() as u16).to_le_bytes()); + msg.extend_from_slice(&len.to_le_bytes()); msg.extend_from_slice(version); msg } -/// Decode the version string from an Rversion message. -/// -/// Layout: `size[4] type[1] tag[2] msize[4] version[s]`, little endian. +/// Decode the version string from an Rversion message. The reply must carry +/// NOTAG and an msize no larger than the one offered in Tversion. pub fn decode_rversion(msg: &[u8]) -> Result { - if msg.len() < HEADER_LEN { + if msg.len() < VERSION_OFFSET { return Err(format!("short reply ({} bytes)", msg.len())); } - if msg[4] != RVERSION { - return Err(format!("unexpected message type {}", msg[4])); + if msg[TYPE_OFFSET] != RVERSION { + return Err(format!("unexpected message type {}", msg[TYPE_OFFSET])); } - let len = u16::from_le_bytes([msg[11], msg[12]]) as usize; + let tag = u16::from_le_bytes([msg[TAG_OFFSET], msg[TAG_OFFSET + 1]]); + if tag != NOTAG { + return Err(format!("unexpected tag {tag:#x}")); + } + let msize = u32::from_le_bytes( + msg[MSIZE_OFFSET..MSIZE_OFFSET + 4].try_into().unwrap(), + ); + if msize == 0 || msize > MSIZE { + return Err(format!("invalid msize {msize}")); + } + let len = usize::from(u16::from_le_bytes([ + msg[VERSION_LEN_OFFSET], + msg[VERSION_LEN_OFFSET + 1], + ])); let version = msg - .get(HEADER_LEN..HEADER_LEN + len) + .get(VERSION_OFFSET..VERSION_OFFSET + len) .ok_or_else(|| "truncated version string".to_string())?; Ok(String::from_utf8_lossy(version).into_owned()) } @@ -54,49 +82,56 @@ pub fn decode_rversion(msg: &[u8]) -> Result { mod tests { use super::*; - fn rversion(version: &[u8]) -> Vec { - let mut msg = encode_tversion(""); - msg.truncate(HEADER_LEN); - msg[0..4].copy_from_slice( - &((HEADER_LEN + version.len()) as u32).to_le_bytes(), - ); - msg[4] = RVERSION; - msg[11..13].copy_from_slice(&(version.len() as u16).to_le_bytes()); - msg.extend_from_slice(version); + fn rversion(version: &[u8], tag: u16, msize: u32) -> Vec { + let mut msg = encode_tversion(std::str::from_utf8(version).unwrap()); + msg[TYPE_OFFSET] = RVERSION; + msg[TAG_OFFSET..TAG_OFFSET + 2].copy_from_slice(&tag.to_le_bytes()); + msg[MSIZE_OFFSET..MSIZE_OFFSET + 4] + .copy_from_slice(&msize.to_le_bytes()); msg } #[test] fn tversion_layout() { let msg = encode_tversion(SOFTNPU_9P_VERSION); - assert_eq!(msg.len(), HEADER_LEN + SOFTNPU_9P_VERSION.len()); + assert_eq!(msg.len(), msg_size(SOFTNPU_9P_VERSION.as_bytes())); assert_eq!( u32::from_le_bytes(msg[0..4].try_into().unwrap()), msg.len() as u32 ); - assert_eq!(msg[4], TVERSION); - assert_eq!(&msg[HEADER_LEN..], SOFTNPU_9P_VERSION.as_bytes()); + assert_eq!(msg[TYPE_OFFSET], TVERSION); + assert_eq!(&msg[VERSION_OFFSET..], SOFTNPU_9P_VERSION.as_bytes()); } #[test] fn rversion_roundtrip() { - let msg = rversion(SOFTNPU_9P_VERSION.as_bytes()); + let msg = rversion(SOFTNPU_9P_VERSION.as_bytes(), NOTAG, MSIZE); assert_eq!(decode_rversion(&msg).unwrap(), SOFTNPU_9P_VERSION); - let msg = rversion(b"9P2000.L"); + let msg = rversion(b"9P2000.L", NOTAG, MSIZE / 2); assert_eq!(decode_rversion(&msg).unwrap(), "9P2000.L"); } #[test] fn rversion_malformed() { assert!(decode_rversion(&[]).is_err()); - assert!(decode_rversion(&[0; HEADER_LEN - 1]).is_err()); + assert!(decode_rversion(&[0; VERSION_OFFSET - 1]).is_err()); - let mut msg = rversion(SOFTNPU_9P_VERSION.as_bytes()); - msg[4] = TVERSION; + let mut msg = rversion(SOFTNPU_9P_VERSION.as_bytes(), NOTAG, MSIZE); + msg[TYPE_OFFSET] = TVERSION; assert!(decode_rversion(&msg).is_err()); - let mut msg = rversion(SOFTNPU_9P_VERSION.as_bytes()); + let mut msg = rversion(SOFTNPU_9P_VERSION.as_bytes(), NOTAG, MSIZE); msg.truncate(msg.len() - 1); assert!(decode_rversion(&msg).is_err()); } + + #[test] + fn rversion_bad_tag_or_msize() { + let msg = rversion(SOFTNPU_9P_VERSION.as_bytes(), 1, MSIZE); + assert!(decode_rversion(&msg).is_err()); + let msg = rversion(SOFTNPU_9P_VERSION.as_bytes(), NOTAG, MSIZE + 1); + assert!(decode_rversion(&msg).is_err()); + let msg = rversion(SOFTNPU_9P_VERSION.as_bytes(), NOTAG, 0); + assert!(decode_rversion(&msg).is_err()); + } }