Stop abusing OPTE's source NAT configuration for external IPs (#1479)

- Update OPTE dep to include the API that allows setting an external IP
  address explicitly, rather than abusing the SNAT configuration for
  that
- Carve up SNAT addresses into port ranges of 16K. We previously used
  the entire port range, since OPTE used the whole range anyway to allow
  inbound connections on any port. This allows 16K outbound connections
  from guests that don't need inbound, which is a reasonable starting
  point.
- Add explicit SNAT and external IP addresses to the sled agent and
  client-library types, as well as the representation of instances and
  OPTE ports.

nexus/src/app/instance.rs

@@ -30,10 +30,10 @@ use omicron_common::api::external::ListResultVec;
 use omicron_common::api::external::LookupResult;
 use omicron_common::api::external::UpdateResult;
 use omicron_common::api::internal::nexus;
-use sled_agent_client::types::ExternalIp;
 use sled_agent_client::types::InstanceRuntimeStateMigrateParams;
 use sled_agent_client::types::InstanceRuntimeStateRequested;
 use sled_agent_client::types::InstanceStateRequested;
+use sled_agent_client::types::SourceNatConfig;
 use sled_agent_client::Client as SledAgentClient;
 use std::sync::Arc;
 use uuid::Uuid;
@@ -538,6 +538,8 @@ impl super::Nexus {
             .partition(|ip| ip.kind == IpKind::SNat);
 
         // Sanity checks on the number and kind of each IP address.
+        // TODO-correctness: Handle multiple IP addresses, see
+        //  https://github.com/oxidecomputer/omicron/issues/1467
         if external_ips.len() > MAX_EXTERNAL_IPS_PER_INSTANCE {
             return Err(Error::internal_error(
                 format!(
@@ -549,18 +551,15 @@ impl super::Nexus {
                 .as_str(),
             ));
         }
+        let external_ips =
+            external_ips.into_iter().map(|model| model.ip.ip()).collect();
         if snat_ip.len() != 1 {
             return Err(Error::internal_error(
                 "Expected exactly one SNAT IP address for an instance",
             ));
         }
-
-        // For now, we take the Ephemeral IP, if it exists, or the SNAT IP if not.
-        // TODO-correctness: Handle multiple IP addresses, see
-        //  https://github.com/oxidecomputer/omicron/issues/1467
-        let external_ip = ExternalIp::from(
-            external_ips.into_iter().chain(snat_ip).next().unwrap(),
-        );
+        let source_nat =
+            SourceNatConfig::from(snat_ip.into_iter().next().unwrap());
 
         // Gather the SSH public keys of the actor make the request so
         // that they may be injected into the new image via cloud-init.
@@ -600,7 +599,8 @@ impl super::Nexus {
                 db_instance.runtime().clone(),
             ),
             nics,
-            external_ip,
+            source_nat,
+            external_ips,
             disks: disk_reqs,
             cloud_init_bytes: Some(base64::encode(
                 db_instance.generate_cidata(&public_keys)?,

nexus/src/app/sagas/instance_migrate.rs

@@ -15,13 +15,13 @@ use omicron_common::api::external::Error;
 use omicron_common::api::internal::nexus::InstanceRuntimeState;
 use serde::Deserialize;
 use serde::Serialize;
-use sled_agent_client::types::ExternalIp;
 use sled_agent_client::types::InstanceEnsureBody;
 use sled_agent_client::types::InstanceHardware;
 use sled_agent_client::types::InstanceMigrateParams;
 use sled_agent_client::types::InstanceRuntimeStateMigrateParams;
 use sled_agent_client::types::InstanceRuntimeStateRequested;
 use sled_agent_client::types::InstanceStateRequested;
+use sled_agent_client::types::SourceNatConfig;
 use std::net::Ipv6Addr;
 use std::sync::Arc;
 use steno::new_action_noop_undo;
@@ -189,24 +189,21 @@ async fn sim_instance_migrate(
             .as_str(),
         )));
     }
+    let external_ips =
+        external_ips.into_iter().map(|model| model.ip.ip()).collect();
     if snat_ip.len() != 1 {
         return Err(ActionError::action_failed(Error::internal_error(
             "Expected exactly one SNAT IP address for an instance",
         )));
     }
-
-    // For now, we take the Ephemeral IP, if it exists, or the SNAT IP if not.
-    // TODO-correctness: Handle multiple IP addresses, see
-    //  https://github.com/oxidecomputer/omicron/issues/1467
-    let external_ip = ExternalIp::from(
-        external_ips.into_iter().chain(snat_ip).next().unwrap(),
-    );
+    let source_nat = SourceNatConfig::from(snat_ip.into_iter().next().unwrap());
 
     let instance_hardware = InstanceHardware {
         runtime: runtime.into(),
         // TODO: populate NICs
         nics: vec![],
-        external_ip,
+        source_nat,
+        external_ips,
         // TODO: populate disks
         disks: vec![],
         // TODO: populate cloud init bytes

nexus/src/db/model/external_ip.rs

@@ -68,7 +68,7 @@ pub struct InstanceExternalIp {
     pub last_port: SqlU16,
 }
 
-impl From<InstanceExternalIp> for sled_agent_client::types::ExternalIp {
+impl From<InstanceExternalIp> for sled_agent_client::types::SourceNatConfig {
     fn from(eip: InstanceExternalIp) -> Self {
         Self {
             ip: eip.ip.ip(),

nexus/src/db/queries/external_ip.rs

@@ -49,11 +49,7 @@ const INSTANCE_EXTERNAL_IP_FROM_CLAUSE: InstanceExternalIpFromClause =
 // instead to check if a candidate port range has any overlap with an existing
 // port range, which is more complicated. That's deferred until we actually have
 // that situation (which may be as soon as allocating ephemeral IPs).
-//
-// TODO-correctness: We're currently providing the entire port range, even for
-// source NAT. We'd like to chunk this up in to something more like quadrants,
-// e.g., ranges `[0, 16384)`, `[16384, 32768)`, etc.
-const NUM_SOURCE_NAT_PORTS: usize = 1 << 16;
+const NUM_SOURCE_NAT_PORTS: usize = 1 << 14;
 const MAX_PORT: i32 = u16::MAX as _;
 
 /// Select the next available IP address and port range for an instance's

openapi/sled-agent.json

@@ -709,34 +709,6 @@
           "request_id"
         ]
       },
-      "ExternalIp": {
-        "description": "An external IP address used for external connectivity for an instance.",
-        "type": "object",
-        "properties": {
-          "first_port": {
-            "description": "The first port used for instance NAT, inclusive.",
-            "type": "integer",
-            "format": "uint16",
-            "minimum": 0
-          },
-          "ip": {
-            "description": "The external address provided to the instance",
-            "type": "string",
-            "format": "ip"
-          },
-          "last_port": {
-            "description": "The last port used for instance NAT, also inclusive.",
-            "type": "integer",
-            "format": "uint16",
-            "minimum": 0
-          }
-        },
-        "required": [
-          "first_port",
-          "ip",
-          "last_port"
-        ]
-      },
       "Generation": {
         "description": "Generation numbers stored in the database, used for optimistic concurrency control",
         "type": "integer",
@@ -798,8 +770,13 @@
               "$ref": "#/components/schemas/DiskRequest"
             }
           },
-          "external_ip": {
-            "$ref": "#/components/schemas/ExternalIp"
+          "external_ips": {
+            "description": "Zero or more external IP addresses (either floating or ephemeral), provided to an instance to allow inbound connectivity.",
+            "type": "array",
+            "items": {
+              "type": "string",
+              "format": "ip"
+            }
           },
           "nics": {
             "type": "array",
@@ -809,13 +786,17 @@
           },
           "runtime": {
             "$ref": "#/components/schemas/InstanceRuntimeState"
+          },
+          "source_nat": {
+            "$ref": "#/components/schemas/SourceNatConfig"
           }
         },
         "required": [
           "disks",
-          "external_ip",
+          "external_ips",
           "nics",
-          "runtime"
+          "runtime",
+          "source_nat"
         ]
       },
       "InstanceMigrateParams": {
@@ -1236,6 +1217,34 @@
         "format": "uint8",
         "minimum": 0
       },
+      "SourceNatConfig": {
+        "description": "An IP address and port range used for instance source NAT, i.e., making outbound network connections from guests.",
+        "type": "object",
+        "properties": {
+          "first_port": {
+            "description": "The first port used for instance NAT, inclusive.",
+            "type": "integer",
+            "format": "uint16",
+            "minimum": 0
+          },
+          "ip": {
+            "description": "The external address provided to the instance",
+            "type": "string",
+            "format": "ip"
+          },
+          "last_port": {
+            "description": "The last port used for instance NAT, also inclusive.",
+            "type": "integer",
+            "format": "uint16",
+            "minimum": 0
+          }
+        },
+        "required": [
+          "first_port",
+          "ip",
+          "last_port"
+        ]
+      },
       "UpdateArtifact": {
         "description": "Description of a single update artifact.",
         "type": "object",

sled-agent/Cargo.toml

@@ -55,8 +55,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 = "eb9e0c687e3c072dbc7d4782475f4ba5a6f50258" }
-opte = { git = "https://github.com/oxidecomputer/opte", rev = "eb9e0c687e3c072dbc7d4782475f4ba5a6f50258", features = [ "api", "std" ] }
+opte-ioctl = { git = "https://github.com/oxidecomputer/opte", rev = "23884d35aa7908e23accaa77f125a370ddf5c606" }
+opte = { git = "https://github.com/oxidecomputer/opte", rev = "23884d35aa7908e23accaa77f125a370ddf5c606", features = [ "api", "std" ] }
 
 [dev-dependencies]
 expectorate = "1.0.5"

sled-agent/src/instance.rs

@@ -16,8 +16,8 @@ use crate::instance_manager::InstanceTicket;
 use crate::nexus::LazyNexusClient;
 use crate::opte::PortManager;
 use crate::opte::PortTicket;
-use crate::params::ExternalIp;
 use crate::params::NetworkInterface;
+use crate::params::SourceNatConfig;
 use crate::params::{
     InstanceHardware, InstanceMigrateParams, InstanceRuntimeStateRequested,
     InstanceSerialConsoleData,
@@ -219,7 +219,8 @@ struct InstanceInner {
 
     // Guest NIC and OPTE port information
     requested_nics: Vec<NetworkInterface>,
-    external_ip: ExternalIp,
+    source_nat: SourceNatConfig,
+    external_ips: Vec<IpAddr>,
 
     // Disk related properties
     requested_disks: Vec<DiskRequest>,
@@ -480,7 +481,8 @@ impl Instance {
             vnic_allocator,
             port_manager,
             requested_nics: initial.nics,
-            external_ip: initial.external_ip,
+            source_nat: initial.source_nat,
+            external_ips: initial.external_ips,
             requested_disks: initial.disks,
             cloud_init_bytes: initial.cloud_init_bytes,
             state: InstanceStates::new(initial.runtime),
@@ -502,12 +504,16 @@ impl Instance {
         let mut opte_ports = Vec::with_capacity(inner.requested_nics.len());
         let mut port_tickets = Vec::with_capacity(inner.requested_nics.len());
         for nic in inner.requested_nics.iter() {
-            let external_ip =
-                if nic.primary { Some(inner.external_ip) } else { None };
+            let (snat, external_ips) = if nic.primary {
+                (Some(inner.source_nat), Some(inner.external_ips.clone()))
+            } else {
+                (None, None)
+            };
             let port = inner.port_manager.create_port(
                 *inner.id(),
                 nic,
-                external_ip,
+                snat,
+                external_ips,
             )?;
             port_tickets.push(port.ticket());
             opte_ports.push(port);
@@ -793,8 +799,8 @@ mod test {
     use crate::illumos::dladm::Etherstub;
     use crate::nexus::LazyNexusClient;
     use crate::opte::PortManager;
-    use crate::params::ExternalIp;
     use crate::params::InstanceStateRequested;
+    use crate::params::SourceNatConfig;
     use chrono::Utc;
     use macaddr::MacAddr6;
     use omicron_common::api::external::{
@@ -839,11 +845,12 @@ mod test {
                 time_updated: Utc::now(),
             },
             nics: vec![],
-            external_ip: ExternalIp {
+            source_nat: SourceNatConfig {
                 ip: IpAddr::from(Ipv4Addr::new(10, 0, 0, 1)),
                 first_port: 0,
-                last_port: 1 << 14 - 1,
+                last_port: 16_384,
             },
+            external_ips: vec![],
             disks: vec![],
             cloud_init_bytes: None,
         }

sled-agent/src/instance_manager.rs

@@ -230,8 +230,8 @@ mod test {
     use crate::illumos::{dladm::MockDladm, zone::MockZones};
     use crate::instance::MockInstance;
     use crate::nexus::LazyNexusClient;
-    use crate::params::ExternalIp;
     use crate::params::InstanceStateRequested;
+    use crate::params::SourceNatConfig;
     use chrono::Utc;
     use macaddr::MacAddr6;
     use omicron_common::api::external::{
@@ -271,11 +271,12 @@ mod test {
                 time_updated: Utc::now(),
             },
             nics: vec![],
-            external_ip: ExternalIp {
+            source_nat: SourceNatConfig {
                 ip: IpAddr::from(Ipv4Addr::new(10, 0, 0, 1)),
                 first_port: 0,
                 last_port: 1 << 14 - 1,
             },
+            external_ips: vec![],
             disks: vec![],
             cloud_init_bytes: None,
         }