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