diff --git a/Cargo.lock b/Cargo.lock index 3faa816e3b7..ffd8b170df5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2045,7 +2045,7 @@ dependencies = [ [[package]] name = "illumos-ddi-dki" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=cb1767c#cb1767c80d4e9d97cb79901eed3c9d08e1fb3826" +source = "git+https://github.com/oxidecomputer/opte?rev=b998015#b9980158540d15d44cfc5d17fc0a5d1848c5e1ae" dependencies = [ "illumos-sys-hdrs", ] @@ -2053,7 +2053,7 @@ dependencies = [ [[package]] name = "illumos-sys-hdrs" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=cb1767c#cb1767c80d4e9d97cb79901eed3c9d08e1fb3826" +source = "git+https://github.com/oxidecomputer/opte?rev=b998015#b9980158540d15d44cfc5d17fc0a5d1848c5e1ae" [[package]] name = "impl-trait-for-tuples" @@ -3028,7 +3028,7 @@ dependencies = [ [[package]] name = "opte" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=cb1767c#cb1767c80d4e9d97cb79901eed3c9d08e1fb3826" +source = "git+https://github.com/oxidecomputer/opte?rev=b998015#b9980158540d15d44cfc5d17fc0a5d1848c5e1ae" dependencies = [ "anymap", "cfg-if 0.1.10", @@ -3045,7 +3045,7 @@ dependencies = [ [[package]] name = "opte-ioctl" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=cb1767c#cb1767c80d4e9d97cb79901eed3c9d08e1fb3826" +source = "git+https://github.com/oxidecomputer/opte?rev=b998015#b9980158540d15d44cfc5d17fc0a5d1848c5e1ae" dependencies = [ "libc", "libnet", diff --git a/sled-agent/Cargo.toml b/sled-agent/Cargo.toml index c26cbb1fba4..53ef4a60622 100644 --- a/sled-agent/Cargo.toml +++ b/sled-agent/Cargo.toml @@ -48,8 +48,8 @@ vsss-rs = { version = "2.0.0-pre2", default-features = false, features = ["std"] zone = "0.1" [target.'cfg(target_os = "illumos")'.dependencies] -opte-ioctl = { git = "https://github.com/oxidecomputer/opte", rev = "cb1767c" } -opte = { git = "https://github.com/oxidecomputer/opte", rev = "cb1767c", features = [ "api", "std" ] } +opte-ioctl = { git = "https://github.com/oxidecomputer/opte", rev = "b998015" } +opte = { git = "https://github.com/oxidecomputer/opte", rev = "b998015", features = [ "api", "std" ] } [dev-dependencies] expectorate = "1.0.5" diff --git a/sled-agent/src/illumos/dladm.rs b/sled-agent/src/illumos/dladm.rs index 6bf9c182414..69e22ca8161 100644 --- a/sled-agent/src/illumos/dladm.rs +++ b/sled-agent/src/illumos/dladm.rs @@ -5,6 +5,7 @@ //! Utilities for poking at data links. use crate::common::vlan::VlanID; +use crate::illumos::vnic::VnicKind; use crate::illumos::{execute, ExecutionError, PFEXEC}; use omicron_common::api::external::MacAddr; use serde::{Deserialize, Serialize}; @@ -13,6 +14,11 @@ use std::str::FromStr; pub const VNIC_PREFIX: &str = "ox"; pub const VNIC_PREFIX_CONTROL: &str = "oxControl"; +/// Prefix used to name VNICs over xde devices / OPTE ports. +// TODO-correctness: Remove this when `xde` devices can be directly used beneath +// Viona, and thus plumbed directly to guests. +pub const VNIC_PREFIX_GUEST: &str = "vopte"; + pub const DLADM: &str = "/usr/sbin/dladm"; /// Errors returned from [`Dladm::find_physical`]. @@ -164,7 +170,7 @@ impl Dladm { Ok(()) } - /// Returns all VNICs that may be managed by the Sled Agent. + /// Returns VNICs that may be managed by the Sled Agent. pub fn get_vnics() -> Result, GetVnicError> { let mut command = std::process::Command::new(PFEXEC); let cmd = command.args(&[DLADM, "show-vnic", "-p", "-o", "LINK"]); @@ -172,8 +178,14 @@ impl Dladm { let vnics = String::from_utf8_lossy(&output.stdout) .lines() - .filter(|vnic| vnic.starts_with(VNIC_PREFIX)) - .map(|s| s.to_owned()) + .filter_map(|name| { + // Ensure this is a kind of VNIC that the sled agent could be + // responsible for. + match VnicKind::from_name(name) { + Some(_) => Some(name.to_owned()), + None => None, + } + }) .collect(); Ok(vnics) } diff --git a/sled-agent/src/illumos/running_zone.rs b/sled-agent/src/illumos/running_zone.rs index 08227d42ec9..68a0a28a995 100644 --- a/sled-agent/src/illumos/running_zone.rs +++ b/sled-agent/src/illumos/running_zone.rs @@ -221,11 +221,14 @@ impl RunningZone { }, )?; + let control_vnic = Vnic::wrap_existing(vnic_name) + .expect("Failed to wrap valid control VNIC"); + Ok(Self { inner: InstalledZone { log: log.new(o!("zone" => zone_name.to_string())), name: zone_name.to_string(), - control_vnic: Vnic::wrap_existing(vnic_name), + control_vnic, // TODO(https://github.com/oxidecomputer/omicron/issues/725) // // Re-initialize guest_vnic state by inspecting the zone. diff --git a/sled-agent/src/illumos/vnic.rs b/sled-agent/src/illumos/vnic.rs index c3d118ad1ea..03aa872fce5 100644 --- a/sled-agent/src/illumos/vnic.rs +++ b/sled-agent/src/illumos/vnic.rs @@ -6,7 +6,7 @@ use crate::illumos::dladm::{ CreateVnicError, DeleteVnicError, PhysicalLink, VNIC_PREFIX, - VNIC_PREFIX_CONTROL, + VNIC_PREFIX_CONTROL, VNIC_PREFIX_GUEST, }; use omicron_common::api::external::MacAddr; use std::sync::{ @@ -60,7 +60,7 @@ impl VnicAllocator { debug_assert!(name.starts_with(VNIC_PREFIX)); debug_assert!(name.starts_with(VNIC_PREFIX_CONTROL)); Dladm::create_vnic(&self.data_link, &name, mac, None)?; - Ok(Vnic { name, deleted: false }) + Ok(Vnic { name, deleted: false, kind: VnicKind::OxideControl }) } fn new_superscope>(&self, scope: S) -> Self { @@ -82,6 +82,32 @@ impl VnicAllocator { } } +/// Represents the kind of a VNIC, such as whether it's for guest networking or +/// communicating with Oxide services. +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum VnicKind { + OxideControl, + Guest, +} + +impl VnicKind { + /// Infer the kind from a VNIC's name, if this one the sled agent can + /// manage, and `None` otherwise. + pub fn from_name(name: &str) -> Option { + if name.starts_with(VNIC_PREFIX) { + Some(VnicKind::OxideControl) + } else if name.starts_with(VNIC_PREFIX_GUEST) { + Some(VnicKind::Guest) + } else { + None + } + } +} + +#[derive(thiserror::Error, Debug)] +#[error("VNIC with name '{0}' is not valid for sled agent management")] +pub struct InvalidVnicKind(String); + /// Represents an allocated VNIC on the system. /// The VNIC is de-allocated when it goes out of scope. /// @@ -92,12 +118,22 @@ impl VnicAllocator { pub struct Vnic { name: String, deleted: bool, + kind: VnicKind, } impl Vnic { /// Takes ownership of an existing VNIC. - pub fn wrap_existing>(name: S) -> Self { - Vnic { name: name.as_ref().to_owned(), deleted: false } + pub fn wrap_existing>( + name: S, + ) -> Result { + match VnicKind::from_name(name.as_ref()) { + Some(kind) => Ok(Vnic { + name: name.as_ref().to_owned(), + deleted: false, + kind, + }), + None => Err(InvalidVnicKind(name.as_ref().to_owned())), + } } /// Deletes a NIC (if it has not already been deleted). @@ -113,6 +149,10 @@ impl Vnic { pub fn name(&self) -> &str { &self.name } + + pub fn kind(&self) -> VnicKind { + self.kind + } } impl Drop for Vnic { diff --git a/sled-agent/src/opte/mock_opte.rs b/sled-agent/src/opte/mock_opte.rs index e048b449817..cdd475b0c19 100644 --- a/sled-agent/src/opte/mock_opte.rs +++ b/sled-agent/src/opte/mock_opte.rs @@ -183,3 +183,8 @@ pub fn initialize_xde_driver(log: &Logger) -> Result<(), Error> { slog::warn!(log, "`xde` driver is a fiction on non-illumos systems"); Ok(()) } + +pub fn delete_all_xde_devices(log: &Logger) -> Result<(), Error> { + slog::warn!(log, "`xde` driver is a fiction on non-illumos systems"); + Ok(()) +} diff --git a/sled-agent/src/opte/opte.rs b/sled-agent/src/opte/opte.rs index 6f62dd021ef..ad4dd88d463 100644 --- a/sled-agent/src/opte/opte.rs +++ b/sled-agent/src/opte/opte.rs @@ -28,6 +28,12 @@ use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering; use std::sync::Arc; +// Names of VNICs used as underlay devices for the xde driver. +const XDE_VNIC_NAMES: [&str; 2] = ["net0", "net1"]; + +// Prefix used to identify xde data links. +const XDE_LINK_PREFIX: &str = "opte"; + #[derive(thiserror::Error, Debug)] pub enum Error { #[error("Failure interacting with the OPTE ioctl(2) interface: {0}")] @@ -37,7 +43,15 @@ pub enum Error { CreateVnic(#[from] dladm::CreateVnicError), #[error("Failed to create an IPv6 link-local address for xde underlay devices: {0}")] - UnderlayDevice(#[from] crate::illumos::ExecutionError), + UnderlayDeviceAddress(#[from] crate::illumos::ExecutionError), + + #[error("Failed to get VNICs for xde underlay devices: {0}")] + GetVnic(#[from] crate::illumos::dladm::GetVnicError), + + #[error( + "No xde driver configuration file exists at '/kernel/drv/xde.conf'" + )] + NoXdeConf, #[error(transparent)] BadAddrObj(#[from] addrobj::ParseError), @@ -54,7 +68,7 @@ impl OptePortAllocator { } fn next(&self) -> String { - format!("opte{}", self.next_id()) + format!("{}{}", XDE_LINK_PREFIX, self.next_id()) } fn next_id(&self) -> u64 { @@ -150,7 +164,9 @@ impl OptePortAllocator { Some(omicron_common::api::external::MacAddr(mac)), None, )?; - Some(Vnic::wrap_existing(vnic_name)) + // Safety: We're explicitly creating the VNIC with the prefix + // `VNIC_PREFIX_GUEST`, so this call must return Some(_). + Some(Vnic::wrap_existing(vnic_name).unwrap()) }; Ok(OptePort { @@ -258,16 +274,41 @@ impl Drop for OptePort { } } +/// Delete all xde devices on the system. +pub fn delete_all_xde_devices(log: &Logger) -> Result<(), Error> { + let hdl = OpteHdl::open(OpteHdl::DLD_CTL)?; + for port_info in hdl.list_ports()?.ports.into_iter() { + let name = &port_info.name; + info!( + log, + "deleting existing OPTE port and xde device"; + "device_name" => name + ); + hdl.delete_xde(name)?; + } + Ok(()) +} + /// Initialize the underlay devices required for the xde kernel module. /// /// The xde driver needs information about the physical devices out which it can /// send traffic from the guests. pub fn initialize_xde_driver(log: &Logger) -> Result<(), Error> { + if !std::path::Path::new("/kernel/drv/xde.conf").exists() { + return Err(Error::NoXdeConf); + } let underlay_nics = find_chelsio_links()?; info!(log, "using '{:?}' as data links for xde driver", underlay_nics); if underlay_nics.len() < 2 { + const MESSAGE: &str = concat!( + "There must be at least two underlay NICs for the xde ", + "driver to operate. These are currently created by ", + "`./tools/create_virtual_hardware.sh`. Please ensure that ", + "script has been run, and that two VNICs named `net{0,1}` ", + "exist on the system." + ); return Err(Error::Opte(opte_ioctl::Error::InvalidArgument( - String::from("There must be at least two underlay NICs"), + String::from(MESSAGE), ))); } for nic in &underlay_nics { @@ -294,5 +335,8 @@ fn find_chelsio_links() -> Result, Error> { // `Dladm` to get the real Chelsio links on a Gimlet. These will likely be // called `cxgbeN`, but we explicitly call them `netN` to be clear that // they're likely VNICs for the time being. - Ok((0..2).map(|i| PhysicalLink(format!("net{}", i))).collect()) + Ok(XDE_VNIC_NAMES + .into_iter() + .map(|name| PhysicalLink(name.to_string())) + .collect()) } diff --git a/sled-agent/src/sled_agent.rs b/sled-agent/src/sled_agent.rs index e4017304d1c..fcea2c627eb 100644 --- a/sled-agent/src/sled_agent.rs +++ b/sled-agent/src/sled_agent.rs @@ -5,6 +5,7 @@ //! Sled agent implementation use crate::config::Config; +use crate::illumos::vnic::VnicKind; use crate::illumos::zfs::{ Mountpoint, ZONE_ZFS_DATASET, ZONE_ZFS_DATASET_MOUNTPOINT, }; @@ -148,7 +149,7 @@ impl SledAgent { // to leave the running Zones intact). let zones = Zones::get()?; for z in zones { - warn!(log, "Deleting zone: {}", z.name()); + warn!(log, "Deleting existing zone"; "zone_name" => z.name()); Zones::halt_and_remove_logged(&log, z.name())?; } @@ -162,18 +163,28 @@ impl SledAgent { // This should be accessible via: // $ dladm show-linkprop -c -p zone -o LINK,VALUE // - // Note that this currently deletes only VNICs that start with the - // prefix the sled-agent uses. We'll need to generate an alert or - // otherwise handle VNICs that we _don't_ expect. - let vnics = Dladm::get_vnics()?; - for vnic in vnics - .iter() - .filter(|vnic| vnic.starts_with(crate::illumos::dladm::VNIC_PREFIX)) - { - warn!(log, "Deleting VNIC: {}", vnic); + // Note that we don't currently delete the VNICs in any particular + // order. That should be OK, since we're definitely deleting the guest + // VNICs before the xde devices, which is the main constraint. + for vnic in Dladm::get_vnics()? { + warn!( + log, + "Deleting existing VNIC"; + "vnic_name" => &vnic, + "vnic_kind" => ?VnicKind::from_name(&vnic).unwrap(), + ); Dladm::delete_vnic(&vnic)?; } + // Also delete any extant xde devices. These should also eventually be + // recovered / tracked, to avoid interruption of any guests that are + // still running. That's currently irrelevant, since we're deleting the + // zones anyway. + // + // This is also tracked by + // https://github.com/oxidecomputer/omicron/issues/725. + crate::opte::delete_all_xde_devices(&log)?; + let storage = StorageManager::new( &log, *id, diff --git a/tools/install_opte.sh b/tools/install_opte.sh index b2c49694edb..1c50249c2c4 100755 --- a/tools/install_opte.sh +++ b/tools/install_opte.sh @@ -59,10 +59,58 @@ function sha_from_url { curl -L "$SHA_URL" 2> /dev/null | cut -d ' ' -f 1 } +# Echo the stickiness, 'sticky' or 'non-sticky' of the `helios-dev` publisher +function helios_dev_stickiness { + local LINE="$(pkg publisher | grep '^helios-dev')" + if [[ -z "$LINE" ]]; then + echo "Expected a publisher named helios-dev, exiting!" + exit 1 + fi + if [[ -z "$(echo "$LINE" | grep 'non-sticky')" ]]; then + echo "sticky" + else + echo "non-sticky" + fi +} + +# Ensure that the `helios-dev` publisher is non-sticky. This does not modify the +# publisher, if it is already non-sticky. +function ensure_helios_dev_is_non_sticky { + local STICKINESS="$(helios_dev_stickiness)" + if [[ "$STICKINESS" = "sticky" ]]; then + pkg set-publisher --non-sticky helios-dev + STICKINESS="$(helios_dev_stickiness)" + if [[ "$STICKINESS" = "sticky" ]]; then + echo "Failed to make helios-dev publisher non-sticky" + exit 1 + fi + else + echo "helios-dev publisher is already non-sticky" + fi +} + +function verify_publisher_search_order { + local EXPECTED=("on-nightly" "helios-netdev" "helios-dev") + local N_EXPECTED="${#EXPECTED[*]}" + readarray -t ACTUAL < <(pkg publisher -H | cut -d ' ' -f 1) + local N_ACTUAL="${#ACTUAL[*]}" + if [[ $N_EXPECTED -ne $N_ACTUAL ]]; then + echo "Mismatched number of publishers, expected: $N_EXPECTED, found: $N_ACTUAL" + exit 1 + fi + for ((i=0;i