Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sled-agent] Allocate VNICs over etherstubs, fix inter-zone routing #1066

Merged
merged 18 commits into from
Jun 1, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sled-agent/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub struct Config {
/// Optional list of zpools to be used as "discovered disks".
pub zpools: Option<Vec<ZpoolName>>,

/// The data link on which to allocate VNICs.
/// The data link on which we infer the bootstrap address.
///
/// If unsupplied, we default to the first physical device.
pub data_link: Option<PhysicalLink>,
Expand Down
50 changes: 46 additions & 4 deletions sled-agent/src/illumos/dladm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ pub const VNIC_PREFIX_GUEST: &str = "vopte";

pub const DLADM: &str = "/usr/sbin/dladm";

pub const ETHERSTUB_NAME: &str = "oxStub0";

/// Errors returned from [`Dladm::find_physical`].
#[derive(thiserror::Error, Debug)]
pub enum FindPhysicalLinkError {
Expand Down Expand Up @@ -49,7 +51,7 @@ pub enum GetMacError {
#[error("Failed to create VNIC {name} on link {link:?}: {err}")]
pub struct CreateVnicError {
name: String,
link: PhysicalLink,
link: String,
#[source]
err: ExecutionError,
}
Expand All @@ -75,11 +77,51 @@ pub struct DeleteVnicError {
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)]
pub struct PhysicalLink(pub String);

/// The name of an etherstub
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)]
pub struct Etherstub(pub String);

pub trait VnicSource {
fn name(&self) -> &str;
}

impl VnicSource for PhysicalLink {
fn name(&self) -> &str {
&self.0
}
}

impl VnicSource for Etherstub {
fn name(&self) -> &str {
&self.0
}
}

/// Wraps commands for interacting with data links.
pub struct Dladm {}

#[cfg_attr(test, mockall::automock, allow(dead_code))]
impl Dladm {
/// Creates an etherstub, or returns one which already exists.
pub fn create_etherstub() -> Result<Etherstub, ExecutionError> {
if let Ok(stub) = Self::get_etherstub() {
return Ok(stub);
}
let mut command = std::process::Command::new(PFEXEC);
let cmd =
command.args(&[DLADM, "create-etherstub", "-t", ETHERSTUB_NAME]);
rcgoodfellow marked this conversation as resolved.
Show resolved Hide resolved
execute(cmd)?;
Ok(Etherstub(ETHERSTUB_NAME.to_string()))
}

/// Finds an etherstub.
fn get_etherstub() -> Result<Etherstub, ExecutionError> {
let mut command = std::process::Command::new(PFEXEC);
let cmd = command.args(&[DLADM, "show-etherstub", ETHERSTUB_NAME]);
execute(cmd)?;
Ok(Etherstub(ETHERSTUB_NAME.to_string()))
}

/// Returns the name of the first observed physical data link.
pub fn find_physical() -> Result<PhysicalLink, FindPhysicalLinkError> {
let mut command = std::process::Command::new(PFEXEC);
Expand Down Expand Up @@ -136,7 +178,7 @@ impl Dladm {
/// * `mac`: An optional unicast MAC address for the newly created NIC.
/// * `vlan`: An optional VLAN ID for VLAN tagging.
pub fn create_vnic(
physical: &PhysicalLink,
source: &impl VnicSource,
vnic_name: &str,
mac: Option<MacAddr>,
vlan: Option<VlanID>,
Expand All @@ -147,7 +189,7 @@ impl Dladm {
"create-vnic".to_string(),
"-t".to_string(),
"-l".to_string(),
physical.0.to_string(),
source.name().to_string(),
];

if let Some(mac) = mac {
Expand All @@ -164,7 +206,7 @@ impl Dladm {
let cmd = command.args(&args);
execute(cmd).map_err(|err| CreateVnicError {
name: vnic_name.to_string(),
link: physical.clone(),
link: source.name().to_string(),
err,
})?;
Ok(())
Expand Down
12 changes: 6 additions & 6 deletions sled-agent/src/illumos/vnic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
//! API for controlling a single instance.

use crate::illumos::dladm::{
CreateVnicError, DeleteVnicError, PhysicalLink, VNIC_PREFIX,
CreateVnicError, DeleteVnicError, Etherstub, VNIC_PREFIX,
VNIC_PREFIX_CONTROL, VNIC_PREFIX_GUEST,
};
use omicron_common::api::external::MacAddr;
Expand All @@ -26,7 +26,7 @@ use crate::illumos::dladm::MockDladm as Dladm;
pub struct VnicAllocator {
value: Arc<AtomicU64>,
scope: String,
data_link: PhysicalLink,
data_link: Etherstub,
}

impl VnicAllocator {
Expand All @@ -41,11 +41,11 @@ impl VnicAllocator {
///
/// VnicAllocator::new("Storage") produces
/// - oxControlStorage[NNN]
pub fn new<S: AsRef<str>>(scope: S, physical_link: PhysicalLink) -> Self {
pub fn new<S: AsRef<str>>(scope: S, etherstub: Etherstub) -> Self {
Self {
value: Arc::new(AtomicU64::new(0)),
scope: scope.as_ref().to_string(),
data_link: physical_link,
data_link: etherstub,
}
}

Expand Down Expand Up @@ -171,7 +171,7 @@ mod test {
#[test]
fn test_allocate() {
let allocator =
VnicAllocator::new("Foo", PhysicalLink("mylink".to_string()));
VnicAllocator::new("Foo", Etherstub("mystub".to_string()));
assert_eq!("oxFoo0", allocator.next());
assert_eq!("oxFoo1", allocator.next());
assert_eq!("oxFoo2", allocator.next());
Expand All @@ -180,7 +180,7 @@ mod test {
#[test]
fn test_allocate_within_scopes() {
let allocator =
VnicAllocator::new("Foo", PhysicalLink("mylink".to_string()));
VnicAllocator::new("Foo", Etherstub("mystub".to_string()));
assert_eq!("oxFoo0", allocator.next());
let allocator = allocator.new_superscope("Baz");
assert_eq!("oxBazFoo1", allocator.next());
Expand Down
6 changes: 3 additions & 3 deletions sled-agent/src/instance_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

//! API for controlling multiple instances on a sled.

use crate::illumos::dladm::PhysicalLink;
use crate::illumos::dladm::Etherstub;
use crate::illumos::vnic::VnicAllocator;
use crate::nexus::NexusClient;
use crate::opte::OptePortAllocator;
Expand Down Expand Up @@ -54,15 +54,15 @@ impl InstanceManager {
pub fn new(
log: Logger,
nexus_client: Arc<NexusClient>,
physical_link: PhysicalLink,
etherstub: Etherstub,
underlay_addr: Ipv6Addr,
) -> InstanceManager {
InstanceManager {
inner: Arc::new(InstanceManagerInternal {
log: log.new(o!("component" => "InstanceManager")),
nexus_client,
instances: Mutex::new(BTreeMap::new()),
vnic_allocator: VnicAllocator::new("Instance", physical_link),
vnic_allocator: VnicAllocator::new("Instance", etherstub),
underlay_addr,
port_allocator: OptePortAllocator::new(),
}),
Expand Down
17 changes: 7 additions & 10 deletions sled-agent/src/services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@

//! Support for miscellaneous services managed by the sled.

use crate::illumos::dladm::PhysicalLink;
use crate::illumos::dladm::Etherstub;
use crate::illumos::running_zone::{InstalledZone, RunningZone};
use crate::illumos::vnic::VnicAllocator;
use crate::illumos::zone::{AddressRequest, Zones};
use crate::illumos::zone::AddressRequest;
use crate::params::{ServiceEnsureBody, ServiceRequest};
use omicron_common::address::{DNS_PORT, DNS_SERVER_PORT};
use slog::Logger;
Expand Down Expand Up @@ -77,7 +77,6 @@ pub struct ServiceManager {
config_path: Option<PathBuf>,
zones: Mutex<Vec<RunningZone>>,
vnic_allocator: VnicAllocator,
physical_link: PhysicalLink,
}

impl ServiceManager {
Expand All @@ -86,25 +85,21 @@ impl ServiceManager {
///
/// Args:
/// - `log`: The logger
/// - `physical_link`: A physical link on which to allocate datalinks.
/// - `etherstub`: An etherstub on which to allocate VNICs.
/// - `config_path`: An optional path to a configuration file to store
/// the record of services. By default, [`default_services_config_path`]
/// is used.
pub async fn new(
log: Logger,
physical_link: PhysicalLink,
etherstub: Etherstub,
config_path: Option<PathBuf>,
) -> Result<Self, Error> {
debug!(log, "Creating new ServiceManager");
let mgr = Self {
log: log.new(o!("component" => "ServiceManager")),
config_path,
zones: Mutex::new(vec![]),
vnic_allocator: VnicAllocator::new(
"Service",
physical_link.clone(),
),
physical_link,
vnic_allocator: VnicAllocator::new("Service", etherstub),
};

let config_path = mgr.services_config_path();
Expand Down Expand Up @@ -202,6 +197,7 @@ impl ServiceManager {
);
}

/*
info!(self.log, "GZ addresses: {:#?}", service.gz_addresses);
for addr in &service.gz_addresses {
info!(
Expand All @@ -224,6 +220,7 @@ impl ServiceManager {
err,
})?;
}
*/

debug!(self.log, "importing manifest");

Expand Down
12 changes: 9 additions & 3 deletions sled-agent/src/sled_agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ pub enum Error {
#[error("Physical link not in config, nor found automatically: {0}")]
FindPhysicalLink(#[from] crate::illumos::dladm::FindPhysicalLinkError),

#[error("Failed to acquire etherstub: {0}")]
Etherstub(crate::illumos::ExecutionError),

#[error("Failed to lookup VNICs on boot: {0}")]
GetVnics(#[from] crate::illumos::dladm::GetVnicError),

Expand Down Expand Up @@ -117,6 +120,9 @@ impl SledAgent {
));
info!(&log, "created sled agent");

let etherstub =
Dladm::create_etherstub().map_err(|e| Error::Etherstub(e))?;

let data_link = if let Some(link) = config.data_link.clone() {
link
} else {
Expand Down Expand Up @@ -199,7 +205,7 @@ impl SledAgent {
&parent_log,
*id,
nexus_client.clone(),
data_link.clone(),
etherstub.clone(),
)
.await;
if let Some(pools) = &config.zpools {
Expand All @@ -215,11 +221,11 @@ impl SledAgent {
let instances = InstanceManager::new(
parent_log.clone(),
nexus_client.clone(),
data_link.clone(),
etherstub.clone(),
*sled_address.ip(),
);
let services =
ServiceManager::new(parent_log.clone(), data_link.clone(), None)
ServiceManager::new(parent_log.clone(), etherstub.clone(), None)
.await?;

Ok(SledAgent {
Expand Down
6 changes: 3 additions & 3 deletions sled-agent/src/storage_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

//! Management of sled-local storage.

use crate::illumos::dladm::PhysicalLink;
use crate::illumos::dladm::Etherstub;
use crate::illumos::running_zone::{InstalledZone, RunningZone};
use crate::illumos::vnic::VnicAllocator;
use crate::illumos::zone::AddressRequest;
Expand Down Expand Up @@ -889,7 +889,7 @@ impl StorageManager {
log: &Logger,
sled_id: Uuid,
nexus_client: Arc<NexusClient>,
physical_link: PhysicalLink,
etherstub: Etherstub,
) -> Self {
let log = log.new(o!("component" => "StorageManager"));
let pools = Arc::new(Mutex::new(HashMap::new()));
Expand All @@ -902,7 +902,7 @@ impl StorageManager {
pools: pools.clone(),
new_pools_rx,
new_filesystems_rx,
vnic_allocator: VnicAllocator::new("Storage", physical_link),
vnic_allocator: VnicAllocator::new("Storage", etherstub),
};
StorageManager {
pools,
Expand Down
4 changes: 3 additions & 1 deletion smf/sled-agent/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ zpools = [
"oxp_f4b4dc87-ab46-49fb-a4b4-d361ae214c03",
]

# An optional data link on which VNICs should be created.
# An optional data link from which we extract a MAC address.
smklein marked this conversation as resolved.
Show resolved Hide resolved
# This is used as a unique identifier for the bootstrap address.
#
# If empty, this will be equivalent to the first result from:
# $ dladm show-phys -p -o LINK
# data_link = "igb0"
Expand Down