diff --git a/illumos-utils/src/addrobj.rs b/illumos-utils/src/addrobj.rs index d63aa42bdf..5a93d9a1c7 100644 --- a/illumos-utils/src/addrobj.rs +++ b/illumos-utils/src/addrobj.rs @@ -5,7 +5,17 @@ //! API for operating on addrobj objects. /// The name provided to all link-local IPv6 addresses. -pub const IPV6_LINK_LOCAL_NAME: &str = "ll"; +pub const IPV6_LINK_LOCAL_ADDROBJ_NAME: &str = "ll"; + +/// The name provided to all static IPv6 underlay addresses. +pub const IPV6_STATIC_ADDROBJ_NAME: &str = "omicron6"; + +/// The name provided to all static IPv4 addresses, usually for public OPTE +/// interfaces. +pub const IPV4_STATIC_ADDROBJ_NAME: &str = "omicron4"; + +/// The name provided to DHCP-configured addresses, of either family. +pub const DHCP_ADDROBJ_NAME: &str = "omicron"; /// Describes an "addrobj", which is the combination of an interface /// with an associated name. @@ -59,7 +69,7 @@ impl AddrObject { /// Create a new addrobj on the same interface with the IPv6 link-local /// name. pub fn link_local_on_same_interface(&self) -> Result { - self.on_same_interface(IPV6_LINK_LOCAL_NAME) + self.on_same_interface(IPV6_LINK_LOCAL_ADDROBJ_NAME) } pub fn new(interface: &str, name: &str) -> Result { @@ -76,7 +86,7 @@ impl AddrObject { /// A link-local IPv6 addrobj over the provided interface. pub fn link_local(interface: &str) -> Result { - Self::new(interface, IPV6_LINK_LOCAL_NAME) + Self::new(interface, IPV6_LINK_LOCAL_ADDROBJ_NAME) } pub fn interface(&self) -> &str { diff --git a/illumos-utils/src/ipadm.rs b/illumos-utils/src/ipadm.rs index 1c9e1e234e..70662b6ccd 100644 --- a/illumos-utils/src/ipadm.rs +++ b/illumos-utils/src/ipadm.rs @@ -4,6 +4,7 @@ //! Utilities for managing IP interfaces. +use crate::addrobj::{IPV6_LINK_LOCAL_ADDROBJ_NAME, IPV6_STATIC_ADDROBJ_NAME}; use crate::zone::IPADM; use crate::{execute, ExecutionError, PFEXEC}; use std::net::Ipv6Addr; @@ -11,23 +12,29 @@ use std::net::Ipv6Addr; /// Wraps commands for interacting with interfaces. pub struct Ipadm {} +/// Expected error message contents when showing an addrobj that doesn't exist. +const ADDROBJ_NOT_FOUND_ERR: &str = "Address object not found"; + +/// Expected error message when an interface already exists. +const INTERFACE_ALREADY_EXISTS: &str = "Interface already exists"; + #[cfg_attr(any(test, feature = "testing"), mockall::automock)] impl Ipadm { - // Remove current IP interface and create a new temporary one. - pub fn set_temp_interface_for_datalink( + /// Ensure that an IP interface exists on the provided datalink. + pub fn ensure_ip_interface_exists( datalink: &str, ) -> Result<(), ExecutionError> { - let mut cmd = std::process::Command::new(PFEXEC); - let cmd = cmd.args(&[IPADM, "delete-if", datalink]); - // First we remove IP interface if it already exists. If it doesn't - // exist and the command returns an error we continue anyway as - // the next step is to create it. - let _ = execute(cmd); - let mut cmd = std::process::Command::new(PFEXEC); let cmd = cmd.args(&[IPADM, "create-if", "-t", datalink]); - execute(cmd)?; - Ok(()) + match execute(cmd) { + Ok(_) => Ok(()), + Err(ExecutionError::CommandFailure(info)) + if info.stderr.contains(INTERFACE_ALREADY_EXISTS) => + { + Ok(()) + } + Err(e) => Err(e), + } } // Set MTU to 9000 on both IPv4 and IPv6 @@ -65,11 +72,13 @@ impl Ipadm { listen_addr: &Ipv6Addr, ) -> Result<(), ExecutionError> { // Create auto-configured address on the IP interface if it doesn't already exist - let addrobj = format!("{}/ll", datalink); + let addrobj = format!("{}/{}", datalink, IPV6_LINK_LOCAL_ADDROBJ_NAME); let mut cmd = std::process::Command::new(PFEXEC); let cmd = cmd.args(&[IPADM, "show-addr", &addrobj]); match execute(cmd) { - Err(_) => { + Err(ExecutionError::CommandFailure(info)) + if info.stderr.contains(ADDROBJ_NOT_FOUND_ERR) => + { let mut cmd = std::process::Command::new(PFEXEC); let cmd = cmd.args(&[ IPADM, @@ -81,15 +90,18 @@ impl Ipadm { ]); execute(cmd)?; } + Err(other) => return Err(other), Ok(_) => (), }; // Create static address on the IP interface if it doesn't already exist - let addrobj = format!("{}/omicron6", datalink); + let addrobj = format!("{}/{}", datalink, IPV6_STATIC_ADDROBJ_NAME); let mut cmd = std::process::Command::new(PFEXEC); let cmd = cmd.args(&[IPADM, "show-addr", &addrobj]); match execute(cmd) { - Err(_) => { + Err(ExecutionError::CommandFailure(info)) + if info.stderr.contains(ADDROBJ_NOT_FOUND_ERR) => + { let mut cmd = std::process::Command::new(PFEXEC); let cmd = cmd.args(&[ IPADM, @@ -101,11 +113,11 @@ impl Ipadm { &listen_addr.to_string(), &addrobj, ]); - execute(cmd)?; + execute(cmd).map(|_| ()) } - Ok(_) => (), - }; - Ok(()) + Err(other) => Err(other), + Ok(_) => Ok(()), + } } // Create gateway on the IP interface if it doesn't already exist diff --git a/illumos-utils/src/running_zone.rs b/illumos-utils/src/running_zone.rs index ea24a6f502..6dfd692794 100644 --- a/illumos-utils/src/running_zone.rs +++ b/illumos-utils/src/running_zone.rs @@ -4,7 +4,10 @@ //! Utilities to manage running zones. -use crate::addrobj::AddrObject; +use crate::addrobj::{ + AddrObject, DHCP_ADDROBJ_NAME, IPV4_STATIC_ADDROBJ_NAME, + IPV6_STATIC_ADDROBJ_NAME, +}; use crate::dladm::Etherstub; use crate::link::{Link, VnicAllocator}; use crate::opte::{Port, PortTicket}; @@ -360,7 +363,11 @@ impl RunningZone { } pub fn control_interface(&self) -> AddrObject { - AddrObject::new(self.inner.get_control_vnic_name(), "omicron6").unwrap() + AddrObject::new( + self.inner.get_control_vnic_name(), + IPV6_STATIC_ADDROBJ_NAME, + ) + .unwrap() } /// Runs a command within the Zone, return the output. @@ -547,10 +554,10 @@ impl RunningZone { addrtype: AddressRequest, ) -> Result { let name = match addrtype { - AddressRequest::Dhcp => "omicron", + AddressRequest::Dhcp => DHCP_ADDROBJ_NAME, AddressRequest::Static(net) => match net.ip() { - std::net::IpAddr::V4(_) => "omicron4", - std::net::IpAddr::V6(_) => "omicron6", + std::net::IpAddr::V4(_) => IPV4_STATIC_ADDROBJ_NAME, + std::net::IpAddr::V6(_) => IPV6_STATIC_ADDROBJ_NAME, }, }; self.ensure_address_with_name(addrtype, name).await diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index 2ce61b21ec..bc5ba370da 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -43,7 +43,7 @@ use camino::{Utf8Path, Utf8PathBuf}; use dpd_client::{types as DpdTypes, Client as DpdClient, Error as DpdError}; use dropshot::HandlerTaskMode; use illumos_utils::addrobj::AddrObject; -use illumos_utils::addrobj::IPV6_LINK_LOCAL_NAME; +use illumos_utils::addrobj::IPV6_LINK_LOCAL_ADDROBJ_NAME; use illumos_utils::dladm::{ Dladm, Etherstub, EtherstubVnic, GetSimnetError, PhysicalLink, }; @@ -2879,7 +2879,7 @@ impl ServiceManager { // cabled together. AddrObject::new( &format!("tfportrear{}_0", i), - IPV6_LINK_LOCAL_NAME, + IPV6_LINK_LOCAL_ADDROBJ_NAME, ) .unwrap() }) @@ -2891,7 +2891,7 @@ impl ServiceManager { .map(|i| { AddrObject::new( &i.to_string(), - IPV6_LINK_LOCAL_NAME, + IPV6_LINK_LOCAL_ADDROBJ_NAME, ) .unwrap() }) @@ -3648,7 +3648,7 @@ impl ServiceManager { } } Err(e) => { - info!(self.inner.log, "chronyc command failed: {}", e); + error!(self.inner.log, "chronyc command failed: {}", e); Err(Error::NtpZoneNotReady) } } diff --git a/smf/zone-network-setup/manifest.xml b/smf/zone-network-setup/manifest.xml index 722618eb47..a612d5ac29 100644 --- a/smf/zone-network-setup/manifest.xml +++ b/smf/zone-network-setup/manifest.xml @@ -6,21 +6,26 @@ - - + - + - - + + + - + + + diff --git a/zone-setup/src/bin/zone-setup.rs b/zone-setup/src/bin/zone-setup.rs index 75864ec0ea..800dcfa368 100644 --- a/zone-setup/src/bin/zone-setup.rs +++ b/zone-setup/src/bin/zone-setup.rs @@ -6,7 +6,7 @@ use anyhow::anyhow; use clap::{arg, command, value_parser, Arg, ArgMatches, Command}; -use illumos_utils::addrobj::{AddrObject, IPV6_LINK_LOCAL_NAME}; +use illumos_utils::addrobj::{AddrObject, IPV6_LINK_LOCAL_ADDROBJ_NAME}; use illumos_utils::ipadm::Ipadm; use illumos_utils::route::{Gateway, Route}; use illumos_utils::svcadm::Svcadm; @@ -232,7 +232,7 @@ async fn do_run() -> Result<(), CmdError> { ) .subcommand( Command::new(CHRONY_SETUP_CMD) - .about("Sets up Chrony configuration for NTP zone") + .about("Sets up Chrony configuration for NTP zone") .arg( arg!(-f --file "Chrony configuration file") .default_value(CHRONY_CONFIG_FILE) @@ -328,7 +328,7 @@ async fn switch_zone_setup( for link in &links { Zones::ensure_has_link_local_v6_address( None, - &AddrObject::new(link, IPV6_LINK_LOCAL_NAME).unwrap(), + &AddrObject::new(link, IPV6_LINK_LOCAL_ADDROBJ_NAME).unwrap(), ) .map_err(|err| { CmdError::Failure(anyhow!( @@ -635,7 +635,7 @@ maxslewrate 2708.333 })?; if old_file.clone().is_some_and(|f| f != new_config) { - info!(&log, "Chrony configuration file has changed"; + info!(&log, "Chrony configuration file has changed"; "old configuration file" => ?old_file, "new configuration file" => ?new_config,); } @@ -663,13 +663,15 @@ async fn common_nw_set_up( )) })?; - // TODO: remove when https://github.com/oxidecomputer/stlouis/issues/435 is - // addressed - info!(&log, "Ensuring a temporary IP interface is created"; "data link" => ?datalink); - Ipadm::set_temp_interface_for_datalink(&datalink) + info!( + &log, + "Ensuring IP interface exists on datalink"; + "datalink" => datalink + ); + Ipadm::ensure_ip_interface_exists(datalink) .map_err(|err| CmdError::Failure(anyhow!(err)))?; - info!(&log, "Setting MTU to 9000 for IPv6 and IPv4"; "data link" => ?datalink); + info!(&log, "Setting MTU to 9000 for IPv6 and IPv4"; "datalink" => ?datalink); Ipadm::set_interface_mtu(&datalink) .map_err(|err| CmdError::Failure(anyhow!(err)))?; @@ -705,11 +707,11 @@ async fn common_nw_set_up( if gw.is_empty() { info!(&log, "Underlay is not available yet. Not ensuring there is a default route"); } else { - // We can safely retrieve the first address only as the CLI only accepts a single item. + // We can safely retrieve the first address only as the CLI only accepts a single item. let gw = gw.first().unwrap(); // Ensuring default route with gateway must happen after peer agents have been initialized. - // Omicron zones will be able ensure a default route with gateway immediately, but the + // Omicron zones will be able ensure a default route with gateway immediately, but the // switch zone on the secondary scrimlet might need a few tries while it waits. retry_notify( retry_policy_local(),