Fixes improves OPTE installation and improves errors

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"

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,17 @@ 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";
+
+/// Names of VNICs used as underlay devices for the xde driver.
+pub const XDE_VNIC_NAMES: [&str; 2] = ["net0", "net1"];
+
+/// Prefix used to identify xde data links.
+pub const XDE_LINK_PREFIX: &str = "opte";
+
 pub const DLADM: &str = "/usr/sbin/dladm";
 
 /// Errors returned from [`Dladm::find_physical`].
@@ -164,15 +176,24 @@ impl Dladm {
         Ok(())
     }
 
-    /// Returns all VNICs that may be managed by the Sled Agent.
-    pub fn get_vnics() -> Result<Vec<String>, GetVnicError> {
+    /// Returns VNICs that may be managed by the Sled Agent, optionally
+    /// restricted to a particular kind.
+    pub fn get_vnics(
+        kind: Option<VnicKind>,
+    ) -> Result<Vec<String>, GetVnicError> {
         let mut command = std::process::Command::new(PFEXEC);
         let cmd = command.args(&[DLADM, "show-vnic", "-p", "-o", "LINK"]);
         let output = execute(cmd).map_err(|err| GetVnicError { err })?;
 
         let vnics = String::from_utf8_lossy(&output.stdout)
             .lines()
-            .filter(|vnic| vnic.starts_with(VNIC_PREFIX))
+            .filter(|name| {
+                if let Some(kind) = kind {
+                    VnicKind::from_name(name) == kind
+                } else {
+                    false
+                }
+            })
             .map(|s| s.to_owned())
             .collect();
         Ok(vnics)

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, XDE_VNIC_NAMES,
 };
 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<S: AsRef<str>>(&self, scope: S) -> Self {
@@ -82,6 +82,31 @@ 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,
+    XdeUnderlay,
+    Other,
+}
+
+impl VnicKind {
+    /// Infer the kind from a VNIC's name.
+    pub fn from_name(name: &str) -> Self {
+        if name.starts_with(VNIC_PREFIX) {
+            VnicKind::OxideControl
+        } else if name.starts_with(VNIC_PREFIX_GUEST) {
+            VnicKind::Guest
+        } else if XDE_VNIC_NAMES.contains(&name) {
+            VnicKind::XdeUnderlay
+        } else {
+            VnicKind::Other
+        }
+    }
+}
+
 /// Represents an allocated VNIC on the system.
 /// The VNIC is de-allocated when it goes out of scope.
 ///
@@ -92,12 +117,17 @@ impl VnicAllocator {
 pub struct Vnic {
     name: String,
     deleted: bool,
+    kind: VnicKind,
 }
 
 impl Vnic {
     /// Takes ownership of an existing VNIC.
     pub fn wrap_existing<S: AsRef<str>>(name: S) -> Self {
-        Vnic { name: name.as_ref().to_owned(), deleted: false }
+        Vnic {
+            name: name.as_ref().to_owned(),
+            deleted: false,
+            kind: VnicKind::from_name(name.as_ref()),
+        }
     }
 
     /// Deletes a NIC (if it has not already been deleted).
@@ -113,6 +143,10 @@ impl Vnic {
     pub fn name(&self) -> &str {
         &self.name
     }
+
+    pub fn kind(&self) -> VnicKind {
+        self.kind
+    }
 }
 
 impl Drop for Vnic {

sled-agent/src/instance_manager.rs

@@ -255,7 +255,7 @@ mod test {
         zones_get_ctx.expect().return_once(|| Ok(vec![]));
 
         let dladm_get_vnics_ctx = MockDladm::get_vnics_context();
-        dladm_get_vnics_ctx.expect().return_once(|| Ok(vec![]));
+        dladm_get_vnics_ctx.expect().return_once(|_| Ok(vec![]));
 
         let im = InstanceManager::new(
             log,
@@ -337,7 +337,7 @@ mod test {
         zones_get_ctx.expect().return_once(|| Ok(vec![]));
 
         let dladm_get_vnics_ctx = MockDladm::get_vnics_context();
-        dladm_get_vnics_ctx.expect().return_once(|| Ok(vec![]));
+        dladm_get_vnics_ctx.expect().return_once(|_| Ok(vec![]));
 
         let im = InstanceManager::new(
             log,

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(())
+}

sled-agent/src/opte/opte.rs

@@ -9,7 +9,9 @@ use crate::illumos::addrobj::AddrObject;
 use crate::illumos::dladm;
 use crate::illumos::dladm::Dladm;
 use crate::illumos::dladm::PhysicalLink;
+use crate::illumos::dladm::XDE_LINK_PREFIX;
 use crate::illumos::vnic::Vnic;
+use crate::illumos::vnic::VnicKind;
 use crate::illumos::zone::Zones;
 use ipnetwork::IpNetwork;
 use macaddr::MacAddr6;
@@ -37,7 +39,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 +64,7 @@ impl OptePortAllocator {
     }
 
     fn next(&self) -> String {
-        format!("opte{}", self.next_id())
+        format!("{}{}", XDE_LINK_PREFIX, self.next_id())
     }
 
     fn next_id(&self) -> u64 {
@@ -258,16 +268,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 +329,8 @@ fn find_chelsio_links() -> Result<Vec<PhysicalLink>, 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(Dladm::get_vnics(Some(VnicKind::XdeUnderlay))?
+        .into_iter()
+        .map(PhysicalLink)
+        .collect())
 }

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,32 @@ 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);
+        // Delete VNICs in this order:
+        //
+        // - Oxide control VNICs
+        // - Guest VNICs over xde devices
+        let vnics = Dladm::get_vnics(Some(VnicKind::OxideControl))?
+            .into_iter()
+            .chain(Dladm::get_vnics(Some(VnicKind::Guest))?);
+        for vnic in vnics {
+            warn!(
+              log,
+              "Deleting existing VNIC";
+                "vnic_name" => &vnic,
+                "vnic_kind" => ?VnicKind::from_name(&vnic),
+            );
             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,