Reorganize OPTE port management

common/src/api/external/mod.rs

@@ -1716,6 +1716,12 @@ impl JsonSchema for L4PortRange {
 )]
 pub struct MacAddr(pub macaddr::MacAddr6);
 
+impl From<macaddr::MacAddr6> for MacAddr {
+    fn from(mac: macaddr::MacAddr6) -> Self {
+        Self(mac)
+    }
+}
+
 impl FromStr for MacAddr {
     type Err = macaddr::ParseError;
 

sled-agent/src/illumos/dladm.rs

@@ -90,6 +90,18 @@ pub struct SetLinkpropError {
     err: ExecutionError,
 }
 
+/// Errors returned from [`Dladm::reset_linkprop`].
+#[derive(thiserror::Error, Debug)]
+#[error(
+    "Failed to reset link property \"{prop_name}\" on vnic {link_name}: {err}"
+)]
+pub struct ResetLinkpropError {
+    link_name: String,
+    prop_name: String,
+    #[source]
+    err: ExecutionError,
+}
+
 /// The name of a physical datalink.
 #[derive(Debug, Clone, Deserialize, Serialize, PartialEq)]
 pub struct PhysicalLink(pub String);
@@ -346,4 +358,26 @@ impl Dladm {
         })?;
         Ok(())
     }
+
+    /// Reset a link property on a VNIC
+    pub fn reset_linkprop(
+        vnic: &str,
+        prop_name: &str,
+    ) -> Result<(), ResetLinkpropError> {
+        let mut command = std::process::Command::new(PFEXEC);
+        let cmd = command.args(&[
+            DLADM,
+            "reset-linkprop",
+            "-t",
+            "-p",
+            prop_name,
+            vnic,
+        ]);
+        execute(cmd).map_err(|err| ResetLinkpropError {
+            link_name: vnic.to_string(),
+            prop_name: prop_name.to_string(),
+            err,
+        })?;
+        Ok(())
+    }
 }

sled-agent/src/illumos/running_zone.rs

@@ -9,7 +9,7 @@ use crate::illumos::dladm::Etherstub;
 use crate::illumos::svc::wait_for_service;
 use crate::illumos::vnic::{Vnic, VnicAllocator};
 use crate::illumos::zone::{AddressRequest, ZONE_PREFIX};
-use crate::opte::OptePort;
+use crate::opte::Port;
 use ipnetwork::IpNetwork;
 use slog::Logger;
 use std::net::{Ipv4Addr, Ipv6Addr};
@@ -292,16 +292,14 @@ impl RunningZone {
                 log: log.new(o!("zone" => zone_name.to_string())),
                 name: zone_name.to_string(),
                 control_vnic,
-                // TODO(https://github.com/oxidecomputer/omicron/issues/725)
-                //
-                // Re-initialize guest_vnic state by inspecting the zone.
                 opte_ports: vec![],
                 physical_nic: None,
             },
         })
     }
 
-    pub fn get_opte_ports(&self) -> &Vec<OptePort> {
+    /// Return references to the OPTE ports for this zone.
+    pub fn opte_ports(&self) -> &[Port] {
         &self.inner.opte_ports
     }
 }
@@ -348,7 +346,7 @@ pub struct InstalledZone {
     control_vnic: Vnic,
 
     // OPTE devices for the guest network interfaces
-    opte_ports: Vec<OptePort>,
+    opte_ports: Vec<Port>,
 
     // Physical NIC possibly provisioned to the zone.
     // TODO: Remove once Nexus traffic is transmitted over OPTE.
@@ -385,7 +383,7 @@ impl InstalledZone {
         unique_name: Option<&str>,
         datasets: &[zone::Dataset],
         devices: &[zone::Device],
-        opte_ports: Vec<OptePort>,
+        opte_ports: Vec<Port>,
         physical_nic: Option<Vnic>,
     ) -> Result<InstalledZone, InstallZoneError> {
         let control_vnic = vnic_allocator.new_control(None).map_err(|err| {
@@ -401,7 +399,7 @@ impl InstalledZone {
 
         let net_device_names: Vec<String> = opte_ports
             .iter()
-            .map(|port| port.vnic().name().to_string())
+            .map(|port| port.vnic_name().to_string())
             .chain(std::iter::once(control_vnic.name().to_string()))
             .chain(physical_nic.as_ref().map(|vnic| vnic.name().to_string()))
             .collect();

sled-agent/src/instance.rs

@@ -14,8 +14,8 @@ use crate::illumos::vnic::VnicAllocator;
 use crate::illumos::zone::{AddressRequest, PROPOLIS_ZONE_PREFIX};
 use crate::instance_manager::InstanceTicket;
 use crate::nexus::NexusClient;
-use crate::opte::OptePort;
-use crate::opte::OptePortAllocator;
+use crate::opte::PortManager;
+use crate::opte::PortTicket;
 use crate::params::ExternalIp;
 use crate::params::NetworkInterface;
 use crate::params::{
@@ -32,7 +32,6 @@ use propolis_client::api::DiskRequest;
 use propolis_client::Client as PropolisClient;
 use slog::Logger;
 use std::net::IpAddr;
-use std::net::Ipv6Addr;
 use std::net::SocketAddr;
 use std::sync::Arc;
 use tokio::task::JoinHandle;
@@ -157,7 +156,9 @@ struct RunningState {
     // Connection to Propolis.
     client: Arc<PropolisClient>,
     // Object representing membership in the "instance manager".
-    ticket: InstanceTicket,
+    instance_ticket: InstanceTicket,
+    // Object representing the instance's OPTE ports in the port manager
+    port_ticket: Option<PortTicket>,
     // Handle to task monitoring for Propolis state changes.
     monitor_task: Option<JoinHandle<()>>,
     // Handle to the zone.
@@ -190,6 +191,7 @@ impl Drop for RunningState {
 struct PropolisSetup {
     client: Arc<PropolisClient>,
     running_zone: RunningZone,
+    port_ticket: Option<PortTicket>,
 }
 
 struct InstanceInner {
@@ -207,9 +209,11 @@ struct InstanceInner {
     // NIC-related properties
     vnic_allocator: VnicAllocator<Etherstub>,
 
-    // OPTE port related properties
-    underlay_addr: Ipv6Addr,
-    port_allocator: OptePortAllocator,
+    // Reference to the port manager for creating OPTE ports when starting the
+    // instance
+    port_manager: PortManager,
+
+    // Guest NIC and OPTE port information
     requested_nics: Vec<NetworkInterface>,
     external_ip: ExternalIp,
 
@@ -288,21 +292,20 @@ impl InstanceInner {
     async fn ensure(
         &mut self,
         instance: Instance,
-        ticket: InstanceTicket,
+        instance_ticket: InstanceTicket,
         setup: PropolisSetup,
         migrate: Option<InstanceMigrateParams>,
     ) -> Result<(), Error> {
-        let PropolisSetup { client, running_zone } = setup;
+        let PropolisSetup { client, running_zone, port_ticket } = setup;
 
-        let nics = self
-            .requested_nics
+        let nics = running_zone
+            .opte_ports()
             .iter()
-            .zip(running_zone.get_opte_ports().iter())
-            .map(|(nic, port)| propolis_client::api::NetworkInterfaceRequest {
+            .map(|port| propolis_client::api::NetworkInterfaceRequest {
                 // TODO-correctness: Remove `.vnic()` call when we use the port
                 // directly.
-                name: port.vnic().name().to_string(),
-                slot: propolis_client::api::Slot(nic.slot),
+                name: port.vnic_name().to_string(),
+                slot: propolis_client::api::Slot(port.slot()),
             })
             .collect();
 
@@ -350,7 +353,8 @@ impl InstanceInner {
 
         self.running_state = Some(RunningState {
             client,
-            ticket,
+            instance_ticket,
+            port_ticket,
             monitor_task,
             _running_zone: running_zone,
         });
@@ -401,15 +405,14 @@ mockall::mock! {
         pub fn new(
             log: Logger,
             id: Uuid,
-            vnic_allocator: VnicAllocator<Etherstub>,
-            underlay_addr: Ipv6Addr,
-            port_allocator: OptePortAllocator,
             initial: InstanceHardware,
+            vnic_allocator: VnicAllocator<Etherstub>,
+            port_manager: PortManager,
             nexus_client: Arc<NexusClient>,
         ) -> Result<Self, Error>;
         pub async fn start(
             &self,
-            ticket: InstanceTicket,
+            instance_ticket: InstanceTicket,
             migrate: Option<InstanceMigrateParams>,
         ) -> Result<(), Error>;
         pub async fn transition(
@@ -434,23 +437,20 @@ impl Instance {
     /// Arguments:
     /// * `log`: Logger for dumping debug information.
     /// * `id`: UUID of the instance to be created.
+    /// * `initial`: State of the instance at initialization time.
     /// * `vnic_allocator`: A unique (to the sled) ID generator to
     /// refer to a VNIC. (This exists because of a restriction on VNIC name
     /// lengths, otherwise the UUID would be used instead).
-    /// * `underlay_addr`: The IPv6 underlay address for the sled hosting this
-    /// instance.
-    /// * `port_allocator`: A unique (to the sled) ID generator to
-    /// refer to an OPTE port for the guest network interfaces.
-    /// * `initial`: State of the instance at initialization time.
+    /// * `port_manager`: Handle the the object responsible for managing OPTE
+    /// ports.
     /// * `nexus_client`: Connection to Nexus, used for sending notifications.
     // TODO: This arg list is getting a little long; can we clean this up?
     pub fn new(
         log: Logger,
         id: Uuid,
-        vnic_allocator: VnicAllocator<Etherstub>,
-        underlay_addr: Ipv6Addr,
-        port_allocator: OptePortAllocator,
         initial: InstanceHardware,
+        vnic_allocator: VnicAllocator<Etherstub>,
+        port_manager: PortManager,
         nexus_client: Arc<NexusClient>,
     ) -> Result<Self, Error> {
         info!(log, "Instance::new w/initial HW: {:?}", initial);
@@ -472,8 +472,7 @@ impl Instance {
             propolis_id: initial.runtime.propolis_id,
             propolis_ip: initial.runtime.propolis_addr.unwrap().ip(),
             vnic_allocator,
-            underlay_addr,
-            port_allocator,
+            port_manager,
             requested_nics: initial.nics,
             external_ip: initial.external_ip,
             requested_disks: initial.disks,
@@ -489,34 +488,23 @@ impl Instance {
         Ok(Instance { inner })
     }
 
-    fn create_opte_ports(
+    async fn setup_propolis_locked(
         &self,
         inner: &mut MutexGuard<'_, InstanceInner>,
-    ) -> Result<Vec<OptePort>, Error> {
-        let mut ports = Vec::with_capacity(inner.requested_nics.len());
+    ) -> Result<PropolisSetup, Error> {
+        // Create OPTE ports for the instance
+        let mut opte_ports = Vec::with_capacity(inner.requested_nics.len());
         for nic in inner.requested_nics.iter() {
-            let vni = crate::opte::Vni::new(nic.vni).expect("Invalid VNI");
             let external_ip =
                 if nic.primary { Some(inner.external_ip) } else { None };
-            let port = inner.port_allocator.new_port(
-                nic.ip,
-                *nic.mac,
-                ipnetwork::IpNetwork::from(nic.subnet),
-                vni,
-                inner.underlay_addr,
+            let port = inner.port_manager.create_port(
+                *inner.id(),
+                nic,
                 external_ip,
             )?;
-            info!(inner.log, "created OPTE port for guest"; "port_info" => ?port);
-            ports.push(port);
+            opte_ports.push(port);
         }
-        Ok(ports)
-    }
-
-    async fn setup_propolis_locked(
-        &self,
-        inner: &mut MutexGuard<'_, InstanceInner>,
-    ) -> Result<PropolisSetup, Error> {
-        let opte_ports = self.create_opte_ports(inner)?;
+        let port_ticket = opte_ports.first().map(|port| port.ticket());
 
         // Create a zone for the propolis instance, using the previously
         // configured VNICs.
@@ -545,8 +533,8 @@ impl Instance {
         let network = running_zone.ensure_address(addr_request).await?;
         info!(inner.log, "Created address {} for zone: {}", network, zname);
 
-        let gateway = inner.underlay_addr;
-        running_zone.add_default_route(gateway).await?;
+        let gateway = inner.port_manager.underlay_ip();
+        running_zone.add_default_route(*gateway).await?;
 
         // Run Propolis in the Zone.
         let smf_service_name = "svc:/system/illumos/propolis-server";
@@ -646,13 +634,13 @@ impl Instance {
         // don't need to worry about initialization races.
         wait_for_http_server(&inner.log, &client).await?;
 
-        Ok(PropolisSetup { client, running_zone })
+        Ok(PropolisSetup { client, running_zone, port_ticket })
     }
 
     /// Begins the execution of the instance's service (Propolis).
     pub async fn start(
         &self,
-        ticket: InstanceTicket,
+        instance_ticket: InstanceTicket,
         migrate: Option<InstanceMigrateParams>,
     ) -> Result<(), Error> {
         let mut inner = self.inner.lock().await;
@@ -662,7 +650,7 @@ impl Instance {
 
         // Ensure the instance exists in the Propolis Server before we start
         // using it.
-        inner.ensure(self.clone(), ticket, setup, migrate).await?;
+        inner.ensure(self.clone(), instance_ticket, setup, migrate).await?;
 
         Ok(())
     }
@@ -675,7 +663,14 @@ impl Instance {
         warn!(inner.log, "Halting and removing zone: {}", zname);
         Zones::halt_and_remove_logged(&inner.log, &zname).unwrap();
 
-        inner.running_state.as_mut().unwrap().ticket.terminate();
+        // Remove ourselves from the instance manager's map of instances.
+        let running_state = inner.running_state.as_mut().unwrap();
+        running_state.instance_ticket.terminate();
+
+        // And remove the OPTE ports from the port manager
+        if let Some(ticket) = running_state.port_ticket.as_mut() {
+            ticket.release();
+        }
 
         Ok(())
     }
@@ -766,7 +761,7 @@ mod test {
     use super::*;
     use crate::illumos::dladm::Etherstub;
     use crate::mocks::MockNexusClient;
-    use crate::opte::OptePortAllocator;
+    use crate::opte::PortManager;
     use crate::params::ExternalIp;
     use crate::params::InstanceStateRequested;
     use chrono::Utc;
@@ -846,19 +841,20 @@ mod test {
             "Test".to_string(),
             Etherstub("mylink".to_string()),
         );
+        let underlay_ip = std::net::Ipv6Addr::new(
+            0xfd00, 0x1de, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
+        );
         let mac = MacAddr6::from([0u8; 6]);
-        let port_allocator = OptePortAllocator::new(mac);
+        let port_manager =
+            PortManager::new(log.new(slog::o!()), underlay_ip, mac);
         let nexus_client = MockNexusClient::default();
 
         let inst = Instance::new(
             log.clone(),
             test_uuid(),
-            vnic_allocator,
-            std::net::Ipv6Addr::new(
-                0xfd00, 0x1de, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
-            ),
-            port_allocator,
             new_initial_instance(),
+            vnic_allocator,
+            port_manager,
             Arc::new(nexus_client),
         )
         .unwrap();