Skip to content

Commit

Permalink
Track more kinds of datalinks on a sled
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 committed Aug 2, 2024
1 parent 3a78f15 commit 26bc426
Show file tree
Hide file tree
Showing 17 changed files with 917 additions and 363 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 26bc426

Please sign in to comment.