[sled-agent] Allocate VNICs over etherstubs, fix inter-zone routing (#1066)

- Switches all VNIC allocation to occur over an "etherstub" device, called stub0.
- Allocate all GZ addresses (bootstrap, Sled, addrconf) over an "etherstub"-allocated VNIC, called underlay0.

Author: smklein

sled-agent/src/bootstrap/agent.rs

@@ -154,19 +154,23 @@ impl Agent {
                 err,
             })?;
 
-        let data_link = if let Some(link) = sled_config.data_link.clone() {
-            link
-        } else {
-            Dladm::find_physical().map_err(|err| {
+        let etherstub = Dladm::create_etherstub().map_err(|e| {
+            BootstrapError::SledError(format!(
+                "Can't access etherstub device: {}",
+                e
+            ))
+        })?;
+
+        let etherstub_vnic =
+            Dladm::create_etherstub_vnic(&etherstub).map_err(|e| {
                 BootstrapError::SledError(format!(
-                    "Can't access physical link, and none in config: {}",
-                    err
+                    "Can't access etherstub VNIC device: {}",
+                    e
                 ))
-            })?
-        };
+            })?;
 
         Zones::ensure_has_global_zone_v6_address(
-            data_link,
+            etherstub_vnic,
             address,
             "bootstrap6",
         )

sled-agent/src/config.rs

@@ -27,7 +27,7 @@ pub struct Config {
     /// Optional list of zpools to be used as "discovered disks".
     pub zpools: Option<Vec<ZpoolName>>,
 
-    /// The data link on which to allocate VNICs.
+    /// The data link on which we infer the bootstrap address.
     ///
     /// If unsupplied, we default to the first physical device.
     pub data_link: Option<PhysicalLink>,

sled-agent/src/illumos/dladm.rs

@@ -19,8 +19,15 @@ pub const VNIC_PREFIX_CONTROL: &str = "oxControl";
 // Viona, and thus plumbed directly to guests.
 pub const VNIC_PREFIX_GUEST: &str = "vopte";
 
+/// Path to the DLADM command.
 pub const DLADM: &str = "/usr/sbin/dladm";
 
+/// The name of the etherstub to be created for the underlay.
+pub const ETHERSTUB_NAME: &str = "stub0";
+
+/// The name of the etherstub VNIC to be created in the global zone.
+pub const ETHERSTUB_VNIC_NAME: &str = "underlay0";
+
 /// Errors returned from [`Dladm::find_physical`].
 #[derive(thiserror::Error, Debug)]
 pub enum FindPhysicalLinkError {
@@ -49,7 +56,7 @@ pub enum GetMacError {
 #[error("Failed to create VNIC {name} on link {link:?}: {err}")]
 pub struct CreateVnicError {
     name: String,
-    link: PhysicalLink,
+    link: String,
     #[source]
     err: ExecutionError,
 }
@@ -75,11 +82,77 @@ pub struct DeleteVnicError {
 #[derive(Debug, Clone, Deserialize, Serialize, PartialEq)]
 pub struct PhysicalLink(pub String);
 
+/// The name of an etherstub
+#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)]
+pub struct Etherstub(pub String);
+
+/// The name of an etherstub's underlay VNIC
+#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)]
+pub struct EtherstubVnic(pub String);
+
+/// Identifies that an object may be used to create a VNIC.
+pub trait VnicSource {
+    fn name(&self) -> &str;
+}
+
+impl VnicSource for Etherstub {
+    fn name(&self) -> &str {
+        &self.0
+    }
+}
+
+impl VnicSource for PhysicalLink {
+    fn name(&self) -> &str {
+        &self.0
+    }
+}
+
 /// Wraps commands for interacting with data links.
 pub struct Dladm {}
 
 #[cfg_attr(test, mockall::automock, allow(dead_code))]
 impl Dladm {
+    /// Creates an etherstub, or returns one which already exists.
+    pub fn create_etherstub() -> Result<Etherstub, ExecutionError> {
+        if let Ok(stub) = Self::get_etherstub() {
+            return Ok(stub);
+        }
+        let mut command = std::process::Command::new(PFEXEC);
+        let cmd =
+            command.args(&[DLADM, "create-etherstub", "-t", ETHERSTUB_NAME]);
+        execute(cmd)?;
+        Ok(Etherstub(ETHERSTUB_NAME.to_string()))
+    }
+
+    /// Finds an etherstub.
+    fn get_etherstub() -> Result<Etherstub, ExecutionError> {
+        let mut command = std::process::Command::new(PFEXEC);
+        let cmd = command.args(&[DLADM, "show-etherstub", ETHERSTUB_NAME]);
+        execute(cmd)?;
+        Ok(Etherstub(ETHERSTUB_NAME.to_string()))
+    }
+
+    /// Creates a VNIC on top of the etherstub.
+    ///
+    /// This VNIC is not tracked like [`crate::illumos::vnic::Vnic`], because
+    /// it is expected to exist for the lifetime of the sled.
+    pub fn create_etherstub_vnic(
+        source: &Etherstub,
+    ) -> Result<EtherstubVnic, CreateVnicError> {
+        if let Ok(vnic) = Self::get_etherstub_vnic() {
+            return Ok(vnic);
+        }
+        Self::create_vnic(source, ETHERSTUB_VNIC_NAME, None, None)?;
+        Ok(EtherstubVnic(ETHERSTUB_VNIC_NAME.to_string()))
+    }
+
+    fn get_etherstub_vnic() -> Result<EtherstubVnic, ExecutionError> {
+        let mut command = std::process::Command::new(PFEXEC);
+        let cmd = command.args(&[DLADM, "show-vnic", ETHERSTUB_VNIC_NAME]);
+        execute(cmd)?;
+        Ok(EtherstubVnic(ETHERSTUB_VNIC_NAME.to_string()))
+    }
+
     /// Returns the name of the first observed physical data link.
     pub fn find_physical() -> Result<PhysicalLink, FindPhysicalLinkError> {
         let mut command = std::process::Command::new(PFEXEC);
@@ -135,8 +208,8 @@ impl Dladm {
     /// * `vnic_name`: Exact name of the VNIC to be created.
     /// * `mac`: An optional unicast MAC address for the newly created NIC.
     /// * `vlan`: An optional VLAN ID for VLAN tagging.
-    pub fn create_vnic(
-        physical: &PhysicalLink,
+    pub fn create_vnic<T: VnicSource + 'static>(
+        source: &T,
         vnic_name: &str,
         mac: Option<MacAddr>,
         vlan: Option<VlanID>,
@@ -147,7 +220,7 @@ impl Dladm {
             "create-vnic".to_string(),
             "-t".to_string(),
             "-l".to_string(),
-            physical.0.to_string(),
+            source.name().to_string(),
         ];
 
         if let Some(mac) = mac {
@@ -164,7 +237,7 @@ impl Dladm {
         let cmd = command.args(&args);
         execute(cmd).map_err(|err| CreateVnicError {
             name: vnic_name.to_string(),
-            link: physical.clone(),
+            link: source.name().to_string(),
             err,
         })?;
         Ok(())

sled-agent/src/illumos/mod.rs

@@ -15,19 +15,20 @@ pub mod zfs;
 pub mod zone;
 pub mod zpool;
 
-const PFEXEC: &str = "/usr/bin/pfexec";
+pub const PFEXEC: &str = "/usr/bin/pfexec";
 
 #[derive(thiserror::Error, Debug)]
 pub enum ExecutionError {
     #[error("Failed to start execution of [{command}]: {err}")]
     ExecutionStart { command: String, err: std::io::Error },
 
     #[error(
-        "Command [{command}] executed and failed with status: {status}. Output: {stderr}"
+        "Command [{command}] executed and failed with status: {status}. Stdout: {stdout}, Stderr: {stderr}"
     )]
     CommandFailure {
         command: String,
         status: std::process::ExitStatus,
+        stdout: String,
         stderr: String,
     },
 }
@@ -63,6 +64,7 @@ mod inner {
                     .collect::<Vec<String>>()
                     .join(" "),
                 status: output.status,
+                stdout: String::from_utf8_lossy(&output.stdout).to_string(),
                 stderr: String::from_utf8_lossy(&output.stderr).to_string(),
             });
         }

sled-agent/src/illumos/running_zone.rs

@@ -171,6 +171,21 @@ impl RunningZone {
         Ok(network)
     }
 
+    pub async fn add_route(
+        &self,
+        destination: ipnetwork::Ipv6Network,
+    ) -> Result<(), RunCommandError> {
+        self.run_cmd(&[
+            "/usr/sbin/route",
+            "add",
+            "-inet6",
+            &format!("{}/{}", destination.network(), destination.prefix()),
+            "-inet6",
+            &destination.ip().to_string(),
+        ])?;
+        Ok(())
+    }
+
     /// Looks up a running zone based on the `zone_prefix`, if one already exists.
     ///
     /// - If the zone was found, is running, and has a network interface, it is

sled-agent/src/illumos/vnic.rs

@@ -5,7 +5,7 @@
 //! API for controlling a single instance.
 
 use crate::illumos::dladm::{
-    CreateVnicError, DeleteVnicError, PhysicalLink, VNIC_PREFIX,
+    CreateVnicError, DeleteVnicError, Etherstub, VNIC_PREFIX,
     VNIC_PREFIX_CONTROL, VNIC_PREFIX_GUEST,
 };
 use omicron_common::api::external::MacAddr;
@@ -26,7 +26,7 @@ use crate::illumos::dladm::MockDladm as Dladm;
 pub struct VnicAllocator {
     value: Arc<AtomicU64>,
     scope: String,
-    data_link: PhysicalLink,
+    data_link: Etherstub,
 }
 
 impl VnicAllocator {
@@ -41,11 +41,11 @@ impl VnicAllocator {
     ///
     /// VnicAllocator::new("Storage") produces
     /// - oxControlStorage[NNN]
-    pub fn new<S: AsRef<str>>(scope: S, physical_link: PhysicalLink) -> Self {
+    pub fn new<S: AsRef<str>>(scope: S, etherstub: Etherstub) -> Self {
         Self {
             value: Arc::new(AtomicU64::new(0)),
             scope: scope.as_ref().to_string(),
-            data_link: physical_link,
+            data_link: etherstub,
         }
     }
 
@@ -171,7 +171,7 @@ mod test {
     #[test]
     fn test_allocate() {
         let allocator =
-            VnicAllocator::new("Foo", PhysicalLink("mylink".to_string()));
+            VnicAllocator::new("Foo", Etherstub("mystub".to_string()));
         assert_eq!("oxFoo0", allocator.next());
         assert_eq!("oxFoo1", allocator.next());
         assert_eq!("oxFoo2", allocator.next());
@@ -180,7 +180,7 @@ mod test {
     #[test]
     fn test_allocate_within_scopes() {
         let allocator =
-            VnicAllocator::new("Foo", PhysicalLink("mylink".to_string()));
+            VnicAllocator::new("Foo", Etherstub("mystub".to_string()));
         assert_eq!("oxFoo0", allocator.next());
         let allocator = allocator.new_superscope("Baz");
         assert_eq!("oxBazFoo1", allocator.next());

sled-agent/src/illumos/zone.rs

@@ -10,9 +10,10 @@ use slog::Logger;
 use std::net::{IpAddr, Ipv6Addr};
 
 use crate::illumos::addrobj::AddrObject;
-use crate::illumos::dladm::{PhysicalLink, VNIC_PREFIX_CONTROL};
+use crate::illumos::dladm::{EtherstubVnic, VNIC_PREFIX_CONTROL};
 use crate::illumos::zfs::ZONE_ZFS_DATASET_MOUNTPOINT;
 use crate::illumos::{execute, PFEXEC};
+use omicron_common::address::SLED_PREFIX;
 
 const DLADM: &str = "/usr/sbin/dladm";
 const IPADM: &str = "/usr/sbin/ipadm";
@@ -98,13 +99,14 @@ pub struct EnsureAddressError {
 
 /// Errors from [`Zones::ensure_has_global_zone_v6_address`].
 #[derive(thiserror::Error, Debug)]
-#[error("Failed to create address {address} with name {name} in the GZ on {link:?}: {err}")]
+#[error("Failed to create address {address} with name {name} in the GZ on {link:?}: {err}. Note to developers: {extra_note}")]
 pub struct EnsureGzAddressError {
     address: Ipv6Addr,
-    link: PhysicalLink,
+    link: EtherstubVnic,
     name: String,
     #[source]
     err: anyhow::Error,
+    extra_note: String,
 }
 
 /// Describes the type of addresses which may be requested from a zone.
@@ -122,7 +124,7 @@ impl AddressRequest {
     pub fn new_static(ip: IpAddr, prefix: Option<u8>) -> Self {
         let prefix = prefix.unwrap_or_else(|| match ip {
             IpAddr::V4(_) => 24,
-            IpAddr::V6(_) => 64,
+            IpAddr::V6(_) => SLED_PREFIX,
         });
         let addr = IpNetwork::new(ip, prefix).unwrap();
         AddressRequest::Static(addr)
@@ -543,13 +545,13 @@ impl Zones {
     // should remove this function when Sled Agents are provided IPv6 addresses
     // from RSS.
     pub fn ensure_has_global_zone_v6_address(
-        link: PhysicalLink,
+        link: EtherstubVnic,
         address: Ipv6Addr,
         name: &str,
     ) -> Result<(), EnsureGzAddressError> {
         // Call the guts of this function within a closure to make it easier
         // to wrap the error with appropriate context.
-        |link: PhysicalLink, address, name| -> Result<(), anyhow::Error> {
+        |link: EtherstubVnic, address, name| -> Result<(), anyhow::Error> {
             let gz_link_local_addrobj = AddrObject::new(&link.0, "linklocal")
                 .map_err(|err| anyhow!(err))?;
             Self::ensure_has_link_local_v6_address(
@@ -582,6 +584,22 @@ impl Zones {
             link,
             name: name.to_string(),
             err,
+            extra_note:
+                r#"As of https://github.com/oxidecomputer/omicron/pull/1066, we are changing the
+                physical device on which Global Zone addresses are allocated.
+
+                Before this PR, we allocated addresses and VNICs directly on a physical link.
+                After this PR, we are allocating them on etherstubs.
+
+                As a result, however, if your machine previously ran Omicron, it
+                may have addresses on the physical link which we'd like to
+                allocate from the etherstub instead.
+
+                This can be fixed with the following commands:
+
+                $ pfexec ipadm delete-addr <your-link>/bootstrap6
+                $ pfexec ipadm delete-addr <your-link>/sled6
+                $ pfexec ipadm delete-addr <your-link>/internaldns"#.to_string()
         })?;
         Ok(())
     }

sled-agent/src/instance.rs

@@ -715,7 +715,7 @@ impl Instance {
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::illumos::dladm::PhysicalLink;
+    use crate::illumos::dladm::Etherstub;
     use crate::mocks::MockNexusClient;
     use crate::opte::OptePortAllocator;
     use crate::params::InstanceStateRequested;
@@ -786,7 +786,7 @@ mod test {
         let log = logger();
         let vnic_allocator = VnicAllocator::new(
             "Test".to_string(),
-            PhysicalLink("mylink".to_string()),
+            Etherstub("mylink".to_string()),
         );
         let port_allocator = OptePortAllocator::new();
         let nexus_client = MockNexusClient::default();