Skip to content

Commit

Permalink
Track more kinds of datalinks on a sled (#6208)
Browse files Browse the repository at this point in the history
- Drop the old `physical_data_link:*` timeseries, in favor of an
expanded `sled_data_link:*`. This includes the sled identifiers, and
also the _kind_ of link, which incorporates physical, VNIC, and OPTE
devices. Expunge the old timeseries.
- Make the existing metrics manager into a small wrapper around a
background task. Add message types for asking the task to start / stop
tracking various things, for now just VNICs and OPTE ports. Physical
links can also be tracked (but not untracked), which the sled agent does
immediately after creating the task.
- Add the metrics request queue to the instance manager, instance, and
instance runner, and have the runner start / stop tracking the control
VNIC and OPTE ports after booting the zone and before stopping it
respectively.
- Add the metrics request queue to the probe manager, and also start /
stop tracking the links in the zones.
- Add the metrics queue to the service manager. This one is more
complicated, because this object has to exist before the `SledAgent`
object itself, in order to start the switch zone. Instead, the manager
is provided the queue when it's notified that the `SledAgent` exists,
and at the same time tries to use the queue to notify the metrics task
about the control VNIC that must have already been plumbed into the
zone. The service manager also tracks / untracks the VNICs and OPTE
ports for the _Omicron_ zones it starts, which is much simpler.
- Add some helper methods into the `{Running,Installed}Zone}` types for
listing the names of the control VNIC, bootstrap VNIC, and any OPTE port
names. These are used to tell the metrics task what links to track.
- Clean up a few straggling comments or references to the VNICs that
were previously required between OPTE ports and the guest Viona driver.
Those were removed in #5989.
  • Loading branch information
bnaecker authored Aug 7, 2024
1 parent 95f1842 commit afe7040
Show file tree
Hide file tree
Showing 17 changed files with 956 additions and 348 deletions.
8 changes: 3 additions & 5 deletions illumos-utils/src/dladm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ pub const VNIC_PREFIX: &str = "ox";
pub const VNIC_PREFIX_CONTROL: &str = "oxControl";
pub const VNIC_PREFIX_BOOTSTRAP: &str = "oxBootstrap";

/// Prefix used to name VNICs over xde devices / OPTE ports.
// TODO-correctness: Remove this when `xde` devices can be directly used beneath
// Viona, and thus plumbed directly to guests.
pub const VNIC_PREFIX_GUEST: &str = "vopte";

/// Path to the DLADM command.
pub const DLADM: &str = "/usr/sbin/dladm";

Expand All @@ -42,6 +37,9 @@ pub const BOOTSTRAP_ETHERSTUB_VNIC_NAME: &str = "bootstrap0";
/// The prefix for Chelsio link names.
pub const CHELSIO_LINK_PREFIX: &str = "cxgbe";

/// The prefix for OPTE link names
pub const OPTE_LINK_PREFIX: &str = "opte";

/// Errors returned from [`Dladm::find_physical`].
#[derive(thiserror::Error, Debug)]
pub enum FindPhysicalLinkError {
Expand Down
18 changes: 14 additions & 4 deletions illumos-utils/src/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use crate::destructor::{Deletable, Destructor};
use crate::dladm::{
CreateVnicError, DeleteVnicError, VnicSource, VNIC_PREFIX,
VNIC_PREFIX_BOOTSTRAP, VNIC_PREFIX_CONTROL, VNIC_PREFIX_GUEST,
VNIC_PREFIX_BOOTSTRAP, VNIC_PREFIX_CONTROL,
};
use omicron_common::api::external::MacAddr;
use std::sync::{
Expand Down Expand Up @@ -125,7 +125,6 @@ impl<DL: VnicSource + Clone> VnicAllocator<DL> {
pub enum LinkKind {
Physical,
OxideControlVnic,
GuestVnic,
OxideBootstrapVnic,
}

Expand All @@ -135,14 +134,20 @@ impl LinkKind {
pub fn from_name(name: &str) -> Option<Self> {
if name.starts_with(VNIC_PREFIX) {
Some(LinkKind::OxideControlVnic)
} else if name.starts_with(VNIC_PREFIX_GUEST) {
Some(LinkKind::GuestVnic)
} else if name.starts_with(VNIC_PREFIX_BOOTSTRAP) {
Some(LinkKind::OxideBootstrapVnic)
} else {
None
}
}

/// Return `true` if this link is a VNIC.
pub const fn is_vnic(&self) -> bool {
match self {
LinkKind::Physical => false,
LinkKind::OxideControlVnic | LinkKind::OxideBootstrapVnic => true,
}
}
}

#[derive(thiserror::Error, Debug)]
Expand Down Expand Up @@ -203,6 +208,11 @@ impl Link {
pub fn kind(&self) -> LinkKind {
self.kind
}

/// Return true if this is a VNIC.
pub fn is_vnic(&self) -> bool {
self.kind.is_vnic()
}
}

impl Drop for Link {
Expand Down
40 changes: 19 additions & 21 deletions illumos-utils/src/opte/port_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

//! Manager for all OPTE ports on a Helios system
use crate::dladm::OPTE_LINK_PREFIX;
use crate::opte::opte_firewall_rules;
use crate::opte::params::VirtualNetworkInterfaceHost;
use crate::opte::params::VpcFirewallRule;
Expand Down Expand Up @@ -52,9 +53,6 @@ use std::sync::Arc;
use std::sync::Mutex;
use uuid::Uuid;

// Prefix used to identify xde data links.
const XDE_LINK_PREFIX: &str = "opte";

/// Stored routes (and usage count) for a given VPC/subnet.
#[derive(Debug, Clone)]
struct RouteSet {
Expand Down Expand Up @@ -85,7 +83,7 @@ impl PortManagerInner {
fn next_port_name(&self) -> String {
format!(
"{}{}",
XDE_LINK_PREFIX,
OPTE_LINK_PREFIX,
self.next_port_id.fetch_add(1, Ordering::SeqCst)
)
}
Expand Down Expand Up @@ -265,8 +263,9 @@ impl PortManager {
// So we:
//
// - create the xde device
// - create the vnic, cleaning up the xde device if that fails
// - add both to the Port
// - create the port ticket
// - create the port
// - add both to the PortManager's map
//
// The Port object's drop implementation will clean up both of those, if
// any of the remaining fallible operations fail.
Expand All @@ -289,21 +288,6 @@ impl PortManager {
)?;
hdl
};

// Initialize firewall rules for the new port.
let rules = opte_firewall_rules(firewall_rules, &vni, &mac);
debug!(
self.inner.log,
"Setting firewall rules";
"port_name" => &port_name,
"rules" => ?&rules,
);
#[cfg(target_os = "illumos")]
hdl.set_fw_rules(&oxide_vpc::api::SetFwRulesReq {
port_name: port_name.clone(),
rules,
})?;

let (port, ticket) = {
let mut ports = self.inner.ports.lock().unwrap();
let ticket = PortTicket::new(nic.id, nic.kind, self.inner.clone());
Expand All @@ -326,6 +310,20 @@ impl PortManager {
(port, ticket)
};

// Initialize firewall rules for the new port.
let rules = opte_firewall_rules(firewall_rules, &vni, &mac);
debug!(
self.inner.log,
"Setting firewall rules";
"port_name" => &port_name,
"rules" => ?&rules,
);
#[cfg(target_os = "illumos")]
hdl.set_fw_rules(&oxide_vpc::api::SetFwRulesReq {
port_name: port_name.clone(),
rules,
})?;

// Check locally to see whether we have any routes from the
// control plane for this port already installed. If not,
// create a record to show that we're interested in receiving
Expand Down
23 changes: 23 additions & 0 deletions illumos-utils/src/running_zone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,22 @@ impl RunningZone {
self.inner.zonepath.pool.as_ref()
}

/// Return the name of a bootstrap VNIC in the zone, if any.
pub fn bootstrap_vnic_name(&self) -> Option<&str> {
self.inner.get_bootstrap_vnic_name()
}

/// Return the name of the control VNIC.
pub fn control_vnic_name(&self) -> &str {
self.inner.get_control_vnic_name()
}

/// Return the names of any OPTE ports in the zone.
pub fn opte_port_names(&self) -> impl Iterator<Item = &str> {
self.inner.opte_ports().map(|port| port.name())
}

/// Return the control IP address.
pub fn control_interface(&self) -> AddrObject {
AddrObject::new(
self.inner.get_control_vnic_name(),
Expand Down Expand Up @@ -939,10 +955,17 @@ impl InstalledZone {
zone_name
}

/// Get the name of the bootstrap VNIC in the zone, if any.
pub fn get_bootstrap_vnic_name(&self) -> Option<&str> {
self.bootstrap_vnic.as_ref().map(|link| link.name())
}

/// Get the name of the control VNIC in the zone.
pub fn get_control_vnic_name(&self) -> &str {
self.control_vnic.name()
}

/// Return the name of the zone itself.
pub fn name(&self) -> &str {
&self.name
}
Expand Down
6 changes: 6 additions & 0 deletions oximeter/db/schema/replicated/8/timeseries-to-delete.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
physical_data_link:bytes_received
physical_data_link:bytes_sent
physical_data_link:errors_received
physical_data_link:errors_sent
physical_data_link:packets_received
physical_data_link:packets_sent
6 changes: 6 additions & 0 deletions oximeter/db/schema/single-node/8/timeseries-to-delete.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
physical_data_link:bytes_received
physical_data_link:bytes_sent
physical_data_link:errors_received
physical_data_link:errors_sent
physical_data_link:packets_received
physical_data_link:packets_sent
2 changes: 1 addition & 1 deletion oximeter/db/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use uuid::Uuid;
/// - [`crate::Client::initialize_db_with_version`]
/// - [`crate::Client::ensure_schema`]
/// - The `clickhouse-schema-updater` binary in this crate
pub const OXIMETER_VERSION: u64 = 7;
pub const OXIMETER_VERSION: u64 = 8;

// Wrapper type to represent a boolean in the database.
//
Expand Down
Loading

0 comments on commit afe7040

Please sign in to comment.