From 0f84ff4f0a9085b6d733e62274104d9743f337b6 Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Thu, 13 Aug 2026 18:49:00 -0700 Subject: [PATCH] Support IPv6 addresses in the OPTE zone setup service - Add the new `--create-v6` parameter to the `zone-setup` binary for setting up OPTE ports in a service zone. Also adds stronger types around IPv4 addresses for the gateway / private IP arguments. Includes a number of tests for those types, and all combinations of arguments we could provide. - Ensure we create both IPv4 and IPv6 DHCP address objects in the current zone, depending on how the SMF properties are set. These properties are now set in the sled-agent for both IPv4 and IPv6. - Move the code from the `RunningZone` which does the `route(8)` shenanigans to work around stlouis#326 (setting up a default route to the OPTE virtual gateway), and which also waits for the DHCPv6 address on the link. This was all duplicated, and moving it here means we can delete the workaround in one place in the future. --- illumos-utils/src/ipadm.rs | 20 +- illumos-utils/src/opte/mod.rs | 5 + illumos-utils/src/opte/port.rs | 1 + illumos-utils/src/route.rs | 217 ++++++++++++++++---- illumos-utils/src/running_zone.rs | 105 ++-------- illumos-utils/src/zone.rs | 118 ++++++++++- sled-agent/src/services.rs | 72 ++++++- smf/opte-interface-setup/manifest.xml | 3 +- zone-setup/src/bin/zone-setup.rs | 273 ++++++++++++++++++++++---- 9 files changed, 622 insertions(+), 192 deletions(-) diff --git a/illumos-utils/src/ipadm.rs b/illumos-utils/src/ipadm.rs index b66c19ac4f2..b4c53585bc5 100644 --- a/illumos-utils/src/ipadm.rs +++ b/illumos-utils/src/ipadm.rs @@ -29,6 +29,9 @@ const ADDROBJ_ALREADY_EXISTS: &str = "Address object already exists"; pub enum AddrObjType { DHCP, + // NOTE: This can result in more than one address, if there is a DHCPv6 + // server on the same link as the addrobj. That happens most often for OPTE + // ports used in zones that need external connectivity. AddrConf, Static(IpAddr), } @@ -83,6 +86,9 @@ impl Ipadm { /// Remove any scope from an IPv6 address. /// e.g. fe80::8:20ff:fed0:8687%oxControlService1/10 -> /// fe80::8:20ff:fed0:8687/10 + // + // TODO-cleanup: This could directly parse the line into an IpNet if + // possible, rather than emitting a new string. fn remove_addr_scope(input: &str) -> String { if let Some(pos) = input.find('%') { let (base, rest) = input.split_at(pos); @@ -98,6 +104,10 @@ impl Ipadm { /// Return the IP network associated with an address object, or None if /// there is no address object with this name. + // + // TODO-correctness: There can be many addresses associated with an addrobj, + // e.g., for IPv6 where we have link-local + DHCPv6 addresses. This will + // ignore all but the first. pub async fn addrobj_addr( addrobj: &str, ) -> Result, ExecutionError> { @@ -173,6 +183,7 @@ impl Ipadm { Ok(()) } + /// Create a link-local IPv6 addrconf address and a static IPv6 address. pub async fn create_static_and_autoconfigured_addrs( datalink: &str, listen_addr: &Ipv6Addr, @@ -192,15 +203,6 @@ impl Ipadm { Ok(()) } - // Create gateway on the IP interface if it doesn't already exist - pub async fn create_opte_gateway( - opte_iface: &String, - ) -> Result<(), ExecutionError> { - let addrobj = format!("{}/public", opte_iface); - Self::ensure_ip_addrobj_exists(&addrobj, AddrObjType::DHCP).await?; - Ok(()) - } - /// Set TCP recv_buf to 1 MB. pub async fn set_tcp_recv_buf() -> Result<(), ExecutionError> { let mut cmd = Command::new(PFEXEC); diff --git a/illumos-utils/src/opte/mod.rs b/illumos-utils/src/opte/mod.rs index 8cb3f658ac1..5c268e1df8a 100644 --- a/illumos-utils/src/opte/mod.rs +++ b/illumos-utils/src/opte/mod.rs @@ -43,6 +43,11 @@ use std::net::Ipv4Addr; use std::net::Ipv6Addr; /// Information about the gateway for an OPTE port +/// +/// TODO-remove: This only exists to communicate the destination for a default +/// IPv4 route from the port's private IP to the OPTE "virtual gateway". We can +/// remove this entirely when we resolve +/// . #[derive(Debug, Clone, Copy)] #[allow(dead_code)] pub struct Gateway { diff --git a/illumos-utils/src/opte/port.rs b/illumos-utils/src/opte/port.rs index 4eee3dfe775..e301fa88797 100644 --- a/illumos-utils/src/opte/port.rs +++ b/illumos-utils/src/opte/port.rs @@ -114,6 +114,7 @@ impl Port { &self.inner.name } + // TODO-remove: pub fn gateway(&self) -> &Gateway { &self.inner.gateway } diff --git a/illumos-utils/src/route.rs b/illumos-utils/src/route.rs index 480656ddef0..65f53415344 100644 --- a/illumos-utils/src/route.rs +++ b/illumos-utils/src/route.rs @@ -5,6 +5,7 @@ //! Utilities for manipulating the routing tables. use crate::zone::ROUTE; +use crate::zone::ZLOGIN; use crate::{ ExecutionError, PFEXEC, command_to_string, execute_async, output_to_exec_error, @@ -16,64 +17,106 @@ use omicron_common::address::{ use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use tokio::process::Command; -/// Wraps commands for interacting with routing tables. -pub struct Route {} +#[derive(Clone, Copy, Debug)] +pub enum RouteDestination { + /// The "default" route, suitable for any gateway. + /// + /// This is only used in OPTE routing setup. + Default, + /// An AZ IPv6 /48 subnet. Used only for underlay routing setup. + Subnet(Ipv6Subnet), +} -#[derive(Debug, Clone, Copy)] -pub enum Gateway { - Ipv4(Ipv4Addr), - Ipv6(Ipv6Addr), +impl RouteDestination { + const fn check_gateway(&self, gateway: IpAddr) -> Result<(), RouteError> { + match self { + // Works for either + RouteDestination::Default => Ok(()), + RouteDestination::Subnet(_) => { + if gateway.is_ipv6() { + Ok(()) + } else { + Err(RouteError::IncompatibleGatewayAndDestination) + } + } + } + } } -impl Route { - pub async fn ensure_default_route_with_gateway( - gateway: Gateway, - datalink: &str, - ) -> Result<(), ExecutionError> { - Self::ensure_route_with_gateway("default", gateway, datalink).await +impl core::fmt::Display for RouteDestination { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + RouteDestination::Default => f.write_str("default"), + RouteDestination::Subnet(net) => net.fmt(f), + } } +} + +#[derive(Debug, thiserror::Error)] +pub enum RouteError { + #[error( + "Destination and gateway are incompatible, because they \ + have different IP versions" + )] + IncompatibleGatewayAndDestination, + #[error(transparent)] + Exec(#[from] ExecutionError), +} +/// Wraps commands for interacting with routing tables. +pub struct Route {} + +impl Route { + /// Ensure there is an interface route to the underlay subnet on the + /// provided link. pub async fn ensure_underlay_route_with_gateway( gateway: Ipv6Addr, datalink: &str, - ) -> Result<(), ExecutionError> { + ) -> Result<(), RouteError> { // Route to the underlay AZ's /48 by deriving it from the gateway IP. - let underlay_az: Ipv6Subnet = - Ipv6Subnet::new(gateway); - let gateway = Gateway::Ipv6(gateway); + let underlay_az = Ipv6Subnet::new(gateway); Self::ensure_route_with_gateway( - &underlay_az.to_string(), - gateway, + None, + RouteDestination::Subnet(underlay_az), + IpAddr::V6(gateway), datalink, ) .await } + /// Ensure there is an interface route for the `gateway_ip` to + /// `destination` on the provided `datalink`. async fn ensure_route_with_gateway( - destination: &str, - gateway: Gateway, + zone: Option<&str>, + destination: RouteDestination, + gateway_ip: IpAddr, datalink: &str, - ) -> Result<(), ExecutionError> { + ) -> Result<(), RouteError> { + destination.check_gateway(gateway_ip)?; + let destination = destination.to_string(); let inet; let gw; - match gateway { - Gateway::Ipv4(addr) => { + match gateway_ip { + IpAddr::V4(addr) => { inet = "-inet"; gw = addr.to_string(); } - Gateway::Ipv6(addr) => { + IpAddr::V6(addr) => { inet = "-inet6"; gw = addr.to_string(); } } // Add the desired route if it doesn't already exist let mut cmd = Command::new(PFEXEC); + if let Some(zone) = zone { + cmd.args([ZLOGIN, zone]); + } let cmd = cmd.args(&[ ROUTE, "-n", "get", inet, - destination, + &destination, inet, &gw, "-ifp", @@ -81,10 +124,10 @@ impl Route { ]); let out = cmd.output().await.map_err(|err| { - ExecutionError::ExecutionStart { + RouteError::Exec(ExecutionError::ExecutionStart { command: command_to_string(cmd.as_std()), err, - } + }) })?; match out.status.code() { Some(0) => (), @@ -93,42 +136,114 @@ impl Route { // When that is the case, we'll add the route. Some(ESRCH) => { let mut cmd = Command::new(PFEXEC); + if let Some(zone) = zone { + cmd.args([ZLOGIN, zone]); + } let cmd = cmd.args(&[ ROUTE, "add", inet, - destination, + &destination, inet, &gw, "-ifp", datalink, ]); - execute_async(cmd).await?; + execute_async(cmd).await.map_err(RouteError::from)?; } Some(_) | None => { - return Err(output_to_exec_error(cmd.as_std(), &out)); + return Err(RouteError::Exec(output_to_exec_error( + cmd.as_std(), + &out, + ))); } }; Ok(()) } - pub async fn ensure_opte_route( - gateway: &Ipv4Addr, - iface: &String, - opte_ip: &IpAddr, + /// Configure an IPv4 route to the OPTE virtual gateway. + /// + /// # Details + /// + /// OPTE acts as the "virtual gateway" for all traffic from the private IP + /// address. By design, we always configure OPTE with a /32 or /128 address, + /// which means there are no other addresses "on-link", i.e., whose + /// addresses can be resolved through ARP or NDP. OPTE itself, however, is + /// on-link, and receives all traffic from the guest. + /// + /// But before that happens, the illumos kernel looks at an IP packet from + /// the guest and has to decide where to route it. If the destination + /// address is on-link, then the kernel will send an ARP or NDP request for + /// that address, resolve it, and there we go. But like we said above, _no_ + /// addresses are on-link. So how does the kernel learn any routes? + /// + /// For IPv6, this all happens automagically through NDP. We can create an + /// IPv6 link-local address with just the MAC address, and then send out + /// Router Solicitations. OPTE will respond with Router Advertisements, + /// advertising itself as a default router. The guest side will + /// automatically learn to send all traffic to OPTE's virtual gateway + /// address. (The actual address is _also_ learned through NDP. It's great.) + /// + /// For IPv4, things are harder. The mechanism for learning a default route + /// is the DHCP Classless Static Route Option (#121, in RFC 3442). When the + /// guest asks for a DHCP server and gets a lease back, that can include + /// this information about the virtual gateway and route, similar to NDP. + /// Unfortunately, the illumos `dhcpagent` doesn't understand this option. + /// Therefore, we need to manually program this information using + /// `route(8)`. + /// + /// This method adds a route to the single-host virtual gateway address + /// provided in `gateway_ip`, and then ensures there's also a default route + /// that sends all traffic from the guest out to the gateway. + /// + /// TODO-remove: We should pull all this shenanigans out when we resolve + /// https://github.com/oxidecomputer/stlouis/issues/326. Doing so is tracked + /// by https://github.com/oxidecomputer/omicron/issues/2931. At that point, + /// the only thing we'll need to do is create the DHCP / addrconf `ipadm` + /// addrobjs for V4 and / or V6, and then the protocols will do the rest. + pub async fn configure_opte_virtual_gateway_ipv4_route( + zone: Option<&str>, + opte_port: &str, + gateway_ip: &Ipv4Addr, + private_ip: &Ipv4Addr, + ) -> Result<(), RouteError> { + Self::ensure_opte_route(zone, opte_port, gateway_ip, private_ip) + .await?; + Self::ensure_route_with_gateway( + zone, + RouteDestination::Default, + IpAddr::V4(*gateway_ip), + opte_port, + ) + .await + } + + /// Ensure there is a host route from the private IP to the OPTE virtual + /// gateway address, for the provided port. + async fn ensure_opte_route( + zone: Option<&str>, + opte_port: &str, + gateway_ip: &Ipv4Addr, + private_ip: &Ipv4Addr, ) -> Result<(), ExecutionError> { // Add the desired route if it doesn't already exist let mut cmd = Command::new(PFEXEC); + let gateway_ip = gateway_ip.to_string(); + let private_ip = private_ip.to_string(); + if let Some(zone) = zone { + cmd.args([ZLOGIN, zone]); + } let cmd = cmd.args(&[ ROUTE, "-n", "get", + "-inet", "-host", - &gateway.to_string(), - &opte_ip.to_string(), + &gateway_ip, + &private_ip, "-interface", "-ifp", - &iface.to_string(), + opte_port, ]); let out = cmd.output().await.map_err(|err| { @@ -144,15 +259,19 @@ impl Route { // When that is the case, we'll add the route. Some(ESRCH) => { let mut cmd = Command::new(PFEXEC); + if let Some(zone) = zone { + cmd.args([ZLOGIN, zone]); + } let cmd = cmd.args(&[ ROUTE, "add", + "-inet", "-host", - &gateway.to_string(), - &opte_ip.to_string(), + &gateway_ip, + &private_ip, "-interface", "-ifp", - &iface.to_string(), + opte_port, ]); execute_async(cmd).await?; } @@ -182,3 +301,21 @@ impl Route { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + const V4_ADDR: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 1); + const V4: IpAddr = IpAddr::V4(V4_ADDR); + const V6_ADDR: Ipv6Addr = Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 1); + const V6: IpAddr = IpAddr::V6(V6_ADDR); + + #[test] + fn check_gateway() { + let subnet = Ipv6Subnet::new(V6_ADDR); + assert!(RouteDestination::Default.check_gateway(V4).is_ok()); + assert!(RouteDestination::Default.check_gateway(V6).is_ok()); + assert!(RouteDestination::Subnet(subnet).check_gateway(V4).is_err()); + assert!(RouteDestination::Subnet(subnet).check_gateway(V6).is_ok()); + } +} diff --git a/illumos-utils/src/running_zone.rs b/illumos-utils/src/running_zone.rs index a4ab0829aca..b356dbd73eb 100644 --- a/illumos-utils/src/running_zone.rs +++ b/illumos-utils/src/running_zone.rs @@ -21,7 +21,6 @@ use camino_tempfile::Utf8TempDir; use debug_ignore::DebugIgnore; use ipnetwork::IpNetwork; use omicron_common::address::{AZ_PREFIX_LENGTH, Ipv6Subnet}; -use omicron_common::backoff; use omicron_common::resolvable_files::ResolvableFileSource; use omicron_uuid_kinds::OmicronZoneUuid; pub use oxlog::is_oxide_smf_log_file; @@ -86,17 +85,7 @@ pub enum EnsureAddressError { EnsureAddressError(#[from] crate::zone::EnsureAddressError), #[error(transparent)] - GetAddressesError(#[from] crate::zone::GetAddressesError), - - #[error("Failed ensuring link-local address in {zone}")] - LinkLocal { - zone: String, - #[source] - err: crate::ExecutionError, - }, - - #[error("Failed to find non-link-local address in {zone}")] - NoDhcpV6Addr { zone: String }, + OptePortSetup(#[from] crate::zone::OptePortSetupError), #[error( "Cannot allocate bootstrap {address} in {zone}: missing bootstrap vnic" @@ -107,10 +96,6 @@ pub enum EnsureAddressError { "Failed ensuring address in {zone}: missing opte port ({port_idx})" )] MissingOptePort { zone: String, port_idx: usize }, - - // TODO-remove(#2931): See comment in `ensure_address_for_port` - #[error(transparent)] - OpteGatewayConfig(#[from] RunCommandError), } #[cfg(target_os = "illumos")] @@ -374,7 +359,12 @@ impl RunningZone { } })?; let zone = Some(self.inner.name.as_ref()); - if let Some(gateway) = port.gateway().ipv4_addr() { + // Both the v4 gateway and the v4 private address are present exactly + // when the port has an IPv4 configuration, so this matches both or + // neither. + if let (Some(gateway), Some(private_ip)) = + (port.gateway().ipv4_addr(), port.ipv4_addr()) + { let v4_name = format!("{}4", name); let addrobj = AddrObject::new(port.name(), &v4_name).map_err(|err| { @@ -384,26 +374,13 @@ impl RunningZone { err, } })?; - let addr = - Zones::ensure_address(zone, &addrobj, AddressRequest::Dhcp) - .await?; - // TODO-remove(#2931): OPTE's DHCP "server" returns the list of routes - // to add via option 121 (Classless Static Route). The illumos DHCP - // client currently does not support this option, so we add the routes - // manually here. - let gateway_ip = gateway.to_string(); - let private_ip = addr.ip(); - self.run_cmd(&[ - ROUTE, - "add", - "-host", - &gateway_ip, - &private_ip.to_string(), - "-interface", - "-ifp", - port.name(), - ])?; - self.run_cmd(&[ROUTE, "add", "-inet", "default", &gateway_ip])?; + Zones::configure_opte_ipv4_port( + zone, + &addrobj, + *gateway, + *private_ip, + ) + .await?; } if port.gateway().ipv6_addr().is_some() { let v6_name = format!("{}6", name); @@ -415,58 +392,8 @@ impl RunningZone { err, } })?; - // If the port is using IPv6 addressing we still want it to use - // DHCP(v6) which requires first creating a link-local address. - Zones::ensure_has_link_local_v6_address(zone, &addrobj) - .await - .map_err(|err| EnsureAddressError::LinkLocal { - zone: self.inner.name.clone(), - err, - })?; - - // Unlike DHCPv4, there's no blocking `ipadm` call we can - // make as it just happens in the background. So we just poll - // until we find a non link-local address. - backoff::retry_notify( - backoff::retry_policy_local(), - || async { - // Grab all the address on the addrobj. There should - // always be at least one (the link-local we added) - let addrs = Zones::get_all_addresses(zone, &addrobj) - .await - .map_err(|e| { - backoff::BackoffError::permanent( - EnsureAddressError::from(e), - ) - })?; - - // Look for a non link-local addr - addrs - .into_iter() - .find(|addr| match addr { - IpNetwork::V6(ip) => { - !ip.ip().is_unicast_link_local() - } - _ => false, - }) - .ok_or_else(|| { - backoff::BackoffError::transient( - EnsureAddressError::NoDhcpV6Addr { - zone: self.inner.name.clone(), - }, - ) - }) - }, - |error, delay| { - slog::debug!( - self.inner.log, - "No non link-local address yet (retrying in {:?})", - delay; - error - ); - }, - ) - .await?; + Zones::configure_opte_ipv6_port(zone, &addrobj, &self.inner.log) + .await?; } Ok(()) } diff --git a/illumos-utils/src/zone.rs b/illumos-utils/src/zone.rs index 35dbb65ea7c..b564164a152 100644 --- a/illumos-utils/src/zone.rs +++ b/illumos-utils/src/zone.rs @@ -11,15 +11,18 @@ use ipnetwork::IpNetworkError; use sled_agent_types::inventory::OmicronZoneConfig; use slog::Logger; use slog::info; -use std::net::{IpAddr, Ipv6Addr}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use tokio::process::Command; use crate::ExecutionError; use crate::addrobj::AddrObject; use crate::dladm::{EtherstubVnic, VNIC_PREFIX_BOOTSTRAP, VNIC_PREFIX_CONTROL}; +use crate::route::Route; +use crate::route::RouteError; use crate::zpool::PathInPool; use crate::{PFEXEC, execute_async}; use omicron_common::address::SLED_PREFIX_LENGTH; +use omicron_common::backoff; use omicron_uuid_kinds::OmicronZoneUuid; const DLADM: &str = "/usr/sbin/dladm"; @@ -215,6 +218,25 @@ pub struct GetAddressesError { err: anyhow::Error, } +/// Errors configuring the addresses and routes for an OPTE port. +#[derive(Debug, thiserror::Error)] +pub enum OptePortSetupError { + #[error(transparent)] + EnsureAddress(#[from] EnsureAddressError), + #[error(transparent)] + GetAddresses(#[from] GetAddressesError), + #[error("Failed to create link-local IPv6 address for {addrobj}")] + LinkLocal { + addrobj: AddrObject, + #[source] + err: crate::ExecutionError, + }, + #[error(transparent)] + Route(#[from] RouteError), + #[error("Failed to wait for a DHCPv6 address on {addrobj}")] + NoDhcpV6Addr { addrobj: AddrObject }, +} + /// Describes the type of addresses which may be requested from a zone. #[derive(Copy, Clone, Debug)] // TODO-cleanup: Remove, along with moving to IPv6 addressing everywhere. @@ -858,6 +880,100 @@ impl Zones { Ok(()) } + /// Configure a V4-capable OPTE port. + /// + /// This ensures the DHCP private address object exists, and then (until + /// stlouis#326 lands) manually programs the routes to OPTE's virtual + /// gateway. See [`Route::configure_opte_virtual_gateway_ipv4_route`] for + /// why those routes are needed. + /// + /// `zone` selects where the commands run: `None` runs them in the current + /// zone (the `zone-setup` service, which runs inside the zone itself), + /// while `Some(name)` reaches into that zone from the global zone (the + /// probe path). + pub async fn configure_opte_ipv4_port( + zone: Option<&str>, + addrobj: &AddrObject, + gateway: Ipv4Addr, + private_ip: Ipv4Addr, + ) -> Result<(), OptePortSetupError> { + // Creating the DHCP address blocks until we get a lease, so the + // address is usable by the time this returns. + Self::ensure_address(zone, addrobj, AddressRequest::Dhcp).await?; + Route::configure_opte_virtual_gateway_ipv4_route( + zone, + addrobj.interface(), + &gateway, + &private_ip, + ) + .await?; + Ok(()) + } + + /// Configure a V6-capable OPTE port. + /// + /// This ensures the addrconf private address object exists, and then waits + /// for a DHCPv6 address to be assigned. Unlike IPv4, there are no routes to + /// program: IPv6 routing to OPTE's virtual gateway is learned entirely + /// through NDP. + /// + /// See [`Self::configure_opte_ipv4_port`] for the meaning of `zone`. + pub async fn configure_opte_ipv6_port( + zone: Option<&str>, + addrobj: &AddrObject, + log: &Logger, + ) -> Result<(), OptePortSetupError> { + // Creating the addrconf link-local address also kicks off DHCPv6, via + // OPTE's Router Advertisements. + Self::ensure_has_link_local_v6_address(zone, addrobj).await.map_err( + |err| OptePortSetupError::LinkLocal { + addrobj: addrobj.clone(), + err, + }, + )?; + + // Unlike DHCPv4, there's no blocking call we can make: the DHCPv6 + // address is configured in the background, after the NDP exchange + // completes. Poll until a non-link-local address shows up on the + // address object. + backoff::retry_notify( + backoff::retry_policy_local(), + || async { + let addrs = Self::get_all_addresses(zone, addrobj) + .await + .map_err(|err| { + backoff::BackoffError::permanent( + OptePortSetupError::from(err), + ) + })?; + if addrs.iter().any(|addr| { + matches!( + addr, + IpNetwork::V6(ip) if !ip.ip().is_unicast_link_local() + ) + }) { + Ok(()) + } else { + Err(backoff::BackoffError::transient( + OptePortSetupError::NoDhcpV6Addr { + addrobj: addrobj.clone(), + }, + )) + } + }, + |error, delay| { + slog::debug!( + log, + "No non-link-local IPv6 address yet (retrying)"; + "delay" => ?delay, + "error" => ?error, + ); + }, + ) + .await?; + Ok(()) + } + // TODO(https://github.com/oxidecomputer/omicron/issues/821): We // should remove this function when Sled Agents are provided IPv6 addresses // from RSS. Edit to this TODO: we still need this for the bootstrap network diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index f88b6d975c0..6f4f6f9db00 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -1364,16 +1364,70 @@ impl ServiceManager { let opte_interface = port.name(); - // TODO-completeness: This needs to support dual-stack OPTE ports. - // See https://github.com/oxidecomputer/omicron/issues/9309. - let opte_gateway = port.gateway().ipv4_or_ipv6_addr().to_string(); - let opte_ip = port.ipv4_or_ipv6_addr().to_string(); + // Write out IPv4 and / or IPv6 details to the SMF service properties. + // + // IMPORTANT: + // + // This particular bit of code represents a "cross-consolidation + // interface". The sled-agent and the SMF service / zone-setup binary + // have to agree on an interface, but they're built in different + // software images. The sled-agent is part of the host OS image, and + // updated first during a live-update. The SMF properties are part of an + // Omicron service zone, and built / installed separately. So we have to + // be pretty careful about evolving these to avoid incompatibilities. + // + // Prior to this R23, there is no way to create any service zones with + // IPv6 addresses. Everything is IPv4 for services. + // + // (1) Old sled-agent, old `opte-interface-setup` SMF service + // (2) Old sled-agent, new service + // (3) New sled-agent, old service + // (4) New sled-agent, new service + // + // In case (1), things will work the same way as prior to this change. + // The sled-agent will fill out only the `config/gateway` and + // `config/ip` properties with either the IPv4 or IPv6 address. + // + // (2) is not possible. Updates always proceed with the host OS being + // updated first (reconfigurator-driven) or at the same time (mupdate). + // + // (3) In this case, the sled-agent will set the IPv4 properties as + // before, and will _also_ set the new SMF property + // `config/create_ipv6`. That property will be ignored by the old + // binary, which is fine because (1) that's how it works now, and (2) + // there are no IPv6 control plane zones. Since all old zones also have + // an IPv4 address, we don't have to worry about this value being unset, + // and becoming the default "unknown", which the `zone-setup` binary + // will fail to parse as an IPv4 address. + // + // (4) Everything is fine here, the sled-agent and SMF service are on + // the same version. The sled-agent writes out the IPv4 / IPv6 + // properties, and the SMF service knows how to interpret them. Note + // that this also works for deployment that is completely new on R23 and + // which _only_ uses IPv6 control plane zones. In that case, only the + // IPv6-related SMF properties are filled, which the binary knows how to + // interpret. + let mut config_builder = PropertyGroupBuilder::new("config") + .add_property("interface", "astring", opte_interface); + + // NOTE: Both the gateway / IP are either None or Some(_). That's + // guaranteed by the construction of `Port`'s `PrivateIpConfig`. + // + // If there is no IPv4 address, these properties will be left at their + // default values of "unknown", which the zone-setup binary understands + // means "don't set up IPv4 at all". + if let (Some(gateway_ip), Some(private_ip)) = + (port.gateway().ipv4_addr(), port.ipv4_addr()) + { + config_builder = config_builder + .add_property("gateway", "astring", gateway_ip.to_string()) + .add_property("ip", "astring", private_ip.to_string()); + } - let mut config_builder = PropertyGroupBuilder::new("config"); - config_builder = config_builder - .add_property("interface", "astring", opte_interface) - .add_property("gateway", "astring", &opte_gateway) - .add_property("ip", "astring", &opte_ip); + if port.ipv6_addr().is_some() { + config_builder = + config_builder.add_property("create_ipv6", "boolean", "true"); + } Ok(ServiceBuilder::new("oxide/opte-interface-setup") .add_property_group(config_builder) diff --git a/smf/opte-interface-setup/manifest.xml b/smf/opte-interface-setup/manifest.xml index dc1301d8460..262fe5c01b6 100644 --- a/smf/opte-interface-setup/manifest.xml +++ b/smf/opte-interface-setup/manifest.xml @@ -18,7 +18,7 @@ @@ -29,6 +29,7 @@ + diff --git a/zone-setup/src/bin/zone-setup.rs b/zone-setup/src/bin/zone-setup.rs index 98493e2371c..b6b9a1d120d 100644 --- a/zone-setup/src/bin/zone-setup.rs +++ b/zone-setup/src/bin/zone-setup.rs @@ -10,7 +10,7 @@ use clap::{ArgAction, Args, Parser, Subcommand}; use illumos_utils::ExecutionError; use illumos_utils::addrobj::{AddrObject, IPV6_LINK_LOCAL_ADDROBJ_NAME}; use illumos_utils::ipadm::Ipadm; -use illumos_utils::route::{Gateway, Route}; +use illumos_utils::route::{Route, RouteError}; use illumos_utils::svcadm::Svcadm; use illumos_utils::zone::{AddressRequest, Zones}; use omicron_common::address::Ipv6Subnet; @@ -22,7 +22,7 @@ use slog::{Logger, info}; use std::fmt::Write as _; use std::fs::{OpenOptions, metadata, read_to_string, set_permissions, write}; use std::io::Write as _; -use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; +use std::net::{Ipv4Addr, Ipv6Addr}; use std::os::unix::fs::chown; use std::path::PathBuf; use uzers::{get_group_by_name, get_user_by_name}; @@ -71,15 +71,52 @@ struct CommonNetworkingArgs { static_addrs: Vec, } +/// During the migration to support IPv6 control plane zones, we need a way for +/// this program to know that it should set up an IPv4 address / routing on an +/// OPTE port. In this case, the sled-agent will not set the SMF properties for +/// the `gateway` and `ip` arguments. They'll be left as "unknown". This type +/// handles this case, and converts the literal string into `None`. +#[derive(Clone, Copy, Debug, PartialEq)] +struct MaybeIpv4Addr(Option); + +impl core::str::FromStr for MaybeIpv4Addr { + type Err = std::net::AddrParseError; + + fn from_str(s: &str) -> Result { + if s == "unknown" { + return Ok(Self(None)); + } + s.parse().map(|a| Self(Some(a))) + } +} + +impl core::fmt::Display for MaybeIpv4Addr { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self.0 { + Some(a) => a.fmt(f), + None => "none".fmt(f), + } + } +} + #[derive(Debug, Args)] struct OpteInterfaceArgs { #[arg(short, long, value_parser = parse_string_rejecting_unknown)] interface: String, - /// OPTE-specific gateway for external connectivity via boundary services + /// OPTE's virtual gateway address for external connectivity via boundary + /// services #[arg(short, long)] - gateway: Ipv4Addr, + gateway: MaybeIpv4Addr, + /// The private IPv4 address for the OPTE port. #[arg(short = 'p', long)] - ip: IpAddr, + ip: MaybeIpv4Addr, + /// Configure the OPTE port to support IPv6. + // + // NOTE: We're using the "set" action because this is populated by SMF, + // which doesn't let us conditionally add the whole flag. So we need to do + // things like `--create-ipv6 `. + #[arg(long, action = ArgAction::Set, default_value_t = false)] + create_ipv6: bool, } #[derive(Debug, Args)] @@ -675,7 +712,7 @@ async fn ensure_underlay_route_via_gateway_with_retries( log: &Logger, ) -> anyhow::Result<()> { // Helper to attach error context in the retry loop below. - let err_with_context = |err: ExecutionError| { + let err_with_context = |err: RouteError| { anyhow!(err).context(format!( "failed to ensure default route via gateway {gateway}", )) @@ -695,7 +732,7 @@ async fn ensure_underlay_route_via_gateway_with_retries( Route::ensure_underlay_route_with_gateway(gateway, datalink) .await .map_err(|err| match err { - ExecutionError::CommandFailure(ref e) => { + RouteError::Exec(ExecutionError::CommandFailure(ref e)) => { if e.stdout.contains("Network is unreachable") { BackoffError::transient(err_with_context(err)) } else { @@ -717,50 +754,200 @@ async fn ensure_underlay_route_via_gateway_with_retries( .await } +fn check_opte_config( + gateway_ip: &MaybeIpv4Addr, + private_ip: &MaybeIpv4Addr, + create_ipv6: bool, +) -> anyhow::Result> { + let pair = match (gateway_ip.0, private_ip.0) { + (None, None) => None, + (None, Some(_)) | (Some(_), None) => anyhow::bail!( + "Gateway and private IPv4 address must both be \ + specified, or neither can be specified" + ), + (Some(gw), Some(ip)) => Some((gw, ip)), + }; + anyhow::ensure!( + pair.is_some() || create_ipv6, + "Neither IPv4 nor IPv6 setup requested!" + ); + Ok(pair) +} + async fn opte_interface_set_up( args: OpteInterfaceArgs, log: &Logger, ) -> anyhow::Result<()> { - let OpteInterfaceArgs { interface, gateway, ip } = args; + let OpteInterfaceArgs { interface, gateway, ip, create_ipv6 } = args; + let maybe_ipv4_data = check_opte_config(&gateway, &ip, create_ipv6)?; info!( log, - "Creating gateway on the OPTE IP interface if it doesn't already exist"; - "OPTE interface" => ?interface + "Configuring OPTE port"; + "interface" => %interface, + "gateway_ip" => %gateway, + "private_ip" => %ip, + "create_ipv6" => %create_ipv6, ); - Ipadm::create_opte_gateway(&interface).await.with_context(|| { - format!("failed to create OPTE gateway on interface {interface}") - })?; + if let Some((gateway_ip, private_ip)) = maybe_ipv4_data { + info!(log, "Configuring IPv4 for OPTE port"; "interface" => %interface); + let v4_addrobj = + AddrObject::new(&interface, "public").with_context(|| { + format!("invalid IPv4 addrobj name for interface {interface}") + })?; + // The `zone-setup` service runs inside the target zone, so all + // networking commands run in the current zone (`None`). + Zones::configure_opte_ipv4_port( + None, + &v4_addrobj, + gateway_ip, + private_ip, + ) + .await + .with_context(|| { + format!("failed to configure IPv4 OPTE port on {interface}") + })?; + } + if create_ipv6 { + info!(log, "Configuring IPv6 for OPTE port"; "interface" => %interface); + let v6_addrobj = + AddrObject::new(&interface, "publicv6").with_context(|| { + format!("invalid IPv6 addrobj name for interface {interface}") + })?; + // The `zone-setup` service runs inside the target zone, so all + // networking commands run in the current zone (`None`). + Zones::configure_opte_ipv6_port(None, &v6_addrobj, log) + .await + .with_context(|| { + format!("failed to configure IPv6 OPTE port on {interface}") + })?; + } + Ok(()) +} - info!( - log, "Ensuring there is a gateway route"; - "OPTE gateway" => ?gateway, - "OPTE interface" => ?interface, - "OPTE IP" => ?ip, - ); - Route::ensure_opte_route(&gateway, &interface, &ip).await.with_context( - || { - format!( - "failed to ensure OPTE gateway route on interface {interface} \ - with gateway {gateway} and IP {ip}", - ) - }, - )?; +#[cfg(test)] +mod tests { + use super::MaybeIpv4Addr; + use super::ZoneSetup; + use super::ZoneSetupCommand; + use super::check_opte_config; + use clap::Parser as _; + use std::net::Ipv4Addr; + + #[test] + fn test_maybe_ipv4_addr_from_str() { + assert_eq!(MaybeIpv4Addr(None), "unknown".parse().unwrap()); + assert_eq!( + MaybeIpv4Addr(Some(Ipv4Addr::new(172, 20, 0, 1))), + "172.20.0.1".parse().unwrap() + ); + assert!("".parse::().is_err()); + assert!("abcd".parse::().is_err()); + assert!("fd00::1".parse::().is_err()); + } - info!( - log, "Ensuring there is a default route"; - "gateway" => ?gateway, - ); - Route::ensure_default_route_with_gateway( - Gateway::Ipv4(gateway), - interface.as_str(), - ) - .await - .with_context(|| { - format!( - "failed to ensure default route on interface {interface} via \ - gateway {gateway}" - ) - })?; + #[test] + fn test_opte_interface_args_parsing() { + let s = ZoneSetup::try_parse_from([ + "zone-setup", + "opte-interface", + "-i", + "foo", + "-g", + "unknown", + "-p", + "unknown", + "--create-ipv6", + "false", + ]) + .unwrap(); + let ZoneSetupCommand::OpteInterface(args) = &s.command else { + panic!( + "Expected zone-setup argv to parse as OPTE interface args, \ + but found: {:#?}", + s.command, + ); + }; + assert_eq!(args.gateway, MaybeIpv4Addr(None)); + assert_eq!(args.ip, MaybeIpv4Addr(None)); + + let s = ZoneSetup::try_parse_from([ + "zone-setup", + "opte-interface", + "-i", + "foo", + "-g", + "172.20.0.1", + "-p", + "172.20.0.1", + "--create-ipv6", + "false", + ]) + .unwrap(); + let ZoneSetupCommand::OpteInterface(args) = &s.command else { + panic!( + "Expected zone-setup argv to parse as OPTE interface args, \ + but found: {:#?}", + s.command, + ); + }; + assert_eq!( + args.gateway, + MaybeIpv4Addr(Some(Ipv4Addr::new(172, 20, 0, 1))) + ); + assert_eq!(args.ip, MaybeIpv4Addr(Some(Ipv4Addr::new(172, 20, 0, 1)))); + + assert!( + ZoneSetup::try_parse_from([ + "zone-setup", + "opte-interface", + "-i", + "foo", + "-g", + "aaaa", + "-p", + "172.20.0.1", + "--create-ipv6", + "false", + ]) + .is_err() + ); + assert!( + ZoneSetup::try_parse_from([ + "zone-setup", + "opte-interface", + "-i", + "foo", + "-g", + "172.20.0.1", + "-p", + "aaaa", + "--create-ipv6", + "false", + ]) + .is_err() + ); + } - Ok(()) + #[test] + fn test_check_opte_config() { + let none = MaybeIpv4Addr(None); + let addr = MaybeIpv4Addr(Some(Ipv4Addr::new(1, 1, 1, 1))); + // Nothing at all + assert!(check_opte_config(&none, &none, false).is_err()); + + // One of the IPv4 / gateway is missing + assert!(check_opte_config(&none, &addr, false).is_err()); + assert!(check_opte_config(&addr, &none, false).is_err()); + assert!(check_opte_config(&none, &addr, true).is_err()); + assert!(check_opte_config(&addr, &none, true).is_err()); + + // Both IPv4 / gateway, no IPv6 + assert!(check_opte_config(&addr, &addr, false).is_ok()); + + // Neither IPv4 / gateway, yes IPv6 + assert!(check_opte_config(&none, &none, true).is_ok()); + + // Both IPv4 and IPv6 + assert!(check_opte_config(&addr, &addr, true).is_ok()); + } }