Skip to content

Commit

Permalink
[network metrics] instance network interface metrics
Browse files Browse the repository at this point in the history
Overview
========

This is related to the ongoing work in [plumbing instance/guest
metrics](#742) through to an
oximeter producer in propolis.

Includes:
* An update to `NetworkInterfaceRequest` to include an interface_id, which is part of the request on the [sled instance in Omicron](oxidecomputer/omicron#6414).
* Within initialization, with concrete types, we create interface_ids mapping nic UUIDs => `minor` number of the device instance of viona.
  - The device instance number matches the Kstat we'll look for.
  - We track this mapping via a type alias `InterfaceIdentifiers`, which is passed through from Machine initialization to VM objects (and VM objects shared), the latter of which we can use as part of registering the oximeter producer.
* A new stats module for collecting network_interface metrics.
  - In `to_samples` we generate multiple network interface targets based on vnics per instance, appropriately updating the `interface_id`, which is tracked in `interface_ids`.
* Move kstat_types out of virtual machine-specific stats and use its mocking for other stats tests

Dependencies
------------

- [ ] oxidecomputer/omicron#6414
- [ ] https://code.illumos.org/c/illumos-gate/+/3630
  • Loading branch information
zeeshanlakhani committed Aug 23, 2024
1 parent 64350cd commit 70d7020
Show file tree
Hide file tree
Showing 20 changed files with 730 additions and 190 deletions.
26 changes: 23 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ p9ds = { git = "https://github.com/oxidecomputer/p9fs" }
softnpu = { git = "https://github.com/oxidecomputer/softnpu" }

# Omicron-related
illumos-utils = { git = "https://github.com/oxidecomputer/omicron", branch = "main" }
internal-dns = { git = "https://github.com/oxidecomputer/omicron", branch = "main" }
nexus-client = { git = "https://github.com/oxidecomputer/omicron", branch = "main" }
omicron-common = { git = "https://github.com/oxidecomputer/omicron", branch = "main" }
Expand All @@ -96,7 +97,7 @@ bitflags = "2.4"
bitstruct = "0.1"
bitvec = "1.0"
byteorder = "1"
bytes = "1.1"
bytes = "1.7.1"
camino = "1.1.6"
cargo_metadata = "0.18.1"
cc = "1.0.73"
Expand Down Expand Up @@ -144,9 +145,9 @@ serde_json = "1.0"
serde_test = "1.0.138"
slog = "2.7"
slog-async = "2.8"
slog-bunyan = "2.4.0"
slog-bunyan = "2.5"
slog-dtrace = "0.3"
slog-term = "2.8"
slog-term = "2.9.1"
strum = "0.26"
syn = "1.0"
tar = "0.4"
Expand All @@ -164,3 +165,22 @@ tracing-subscriber = "0.3.14"
usdt = { version = "0.5", default-features = false }
uuid = "1.3.2"
zerocopy = "0.7.34"


#
# It's common during development to use a local copy of various complex
# dependencies. If you want to use those, uncomment one of these blocks.
#
# [patch."https://github.com/oxidecomputer/omicron"]
# illumos-utils = { path = "../omicron/illumos-utils" }
# internal-dns = { path = "../omicron/internal-dns" }
# nexus-client = { path = "../omicron/clients/nexus-client" }
# omicron-common = { path = "../omicron/common" }
# omicron-zone-package = "0.9.0"
# oximeter-instruments = { path = "../omicron/oximeter/instruments", default-features = false, features = ["kstat"] }
# oximeter-producer = { path = "../omicron/oximeter/producer" }
# oximeter = { path = "../omicron/oximeter/oximeter" }
# sled-agent-client = { path = "../omicron/clients/sled-agent-client" }
# [patch."https://github.com/oxidecomputer/crucible"]
# crucible = { path = "../crucible/upstairs" }
# crucible-client-types = { path = "../crucible/crucible-client-types" }
15 changes: 7 additions & 8 deletions bin/propolis-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,6 @@ use clap::{Parser, Subcommand};
use futures::{future, SinkExt};
use newtype_uuid::{GenericUuid, TypedUuid, TypedUuidKind, TypedUuidTag};
use propolis_client::types::InstanceMetadata;
use slog::{o, Drain, Level, Logger};
use tokio::io::{AsyncReadExt, AsyncWriteExt};
use tokio_tungstenite::tungstenite::{
protocol::{frame::coding::CloseCode, CloseFrame},
Message,
};
use uuid::Uuid;

use propolis_client::{
support::{InstanceSerialConsoleHelper, WSClientOffset},
types::{
Expand All @@ -33,6 +25,13 @@ use propolis_client::{
},
Client,
};
use slog::{o, Drain, Level, Logger};
use tokio::io::{AsyncReadExt, AsyncWriteExt};
use tokio_tungstenite::tungstenite::{
protocol::{frame::coding::CloseCode, CloseFrame},
Message,
};
use uuid::Uuid;

#[derive(Debug, Parser)]
#[clap(about, version)]
Expand Down
16 changes: 11 additions & 5 deletions bin/propolis-server/src/lib/initializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,10 @@ use std::os::unix::fs::FileTypeExt;
use std::sync::Arc;
use std::time::{SystemTime, UNIX_EPOCH};

use crate::serial::Serial;
use crate::stats::virtual_machine::VirtualMachine;
use crate::vm::{BlockBackendMap, CrucibleBackendMap, DeviceMap};
use anyhow::{Context, Result};
use crucible_client_types::VolumeConstructionRequest;
pub use nexus_client::Client as NexusClient;
use oximeter::types::ProducerRegistry;

use propolis::block;
use propolis::chardev::{self, BlockingSource, Source};
use propolis::common::{Lifecycle, GB, MB, PAGE_SIZE};
Expand All @@ -37,7 +33,14 @@ use propolis_api_types::instance_spec::{self, v0::InstanceSpecV0};
use propolis_api_types::InstanceProperties;
use slog::info;

// Arbitrary ROM limit for now
use crate::serial::Serial;
use crate::stats::VirtualMachine;
use crate::vm::BlockBackendMap;
use crate::vm::CrucibleBackendMap;
use crate::vm::DeviceMap;
use crate::vm::InterfaceIdentifiers;

/// Arbitrary ROM limit for now
const MAX_ROM_SIZE: usize = 0x20_0000;

fn get_spec_guest_ram_limits(spec: &InstanceSpecV0) -> (usize, usize) {
Expand Down Expand Up @@ -111,6 +114,7 @@ pub struct MachineInitializer<'a> {
pub(crate) log: slog::Logger,
pub(crate) machine: &'a Machine,
pub(crate) devices: DeviceMap,
pub(crate) interface_ids: InterfaceIdentifiers,
pub(crate) block_backends: BlockBackendMap,
pub(crate) crucible_backends: CrucibleBackendMap,
pub(crate) spec: &'a InstanceSpecV0,
Expand Down Expand Up @@ -633,6 +637,8 @@ impl<'a> MachineInitializer<'a> {
)?;
self.devices
.insert(format!("pci-virtio-viona-{}", bdf), viona.clone());
self.interface_ids
.push((vnic_spec.interface_id, viona.instance_id()?));
chipset.pci_attach(bdf, viona);
}
Ok(())
Expand Down
1 change: 1 addition & 0 deletions bin/propolis-server/src/lib/spec/api_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ pub(super) fn parse_nic_from_request(
let (device_name, backend_name) = super::pci_path_to_nic_names(pci_path);
let device_spec = NetworkDeviceV0::VirtioNic(VirtioNic {
backend_name: backend_name.clone(),
interface_id: nic.interface_id,
pci_path,
});

Expand Down
4 changes: 4 additions & 0 deletions bin/propolis-server/src/lib/spec/config_toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use propolis_api_types::instance_spec::{
PciPath,
};
use thiserror::Error;
use uuid::Uuid;

#[cfg(feature = "falcon")]
use propolis_api_types::instance_spec::components::devices::{
Expand Down Expand Up @@ -291,6 +292,9 @@ pub(super) fn parse_network_device_from_config(

let device_spec = NetworkDeviceV0::VirtioNic(VirtioNic {
backend_name: backend_name.clone(),
// We don't allow for configuration to specify the interface_id, so we
// generate a new one.
interface_id: Uuid::new_v4(),
pci_path,
});

Expand Down
1 change: 0 additions & 1 deletion bin/propolis-server/src/lib/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ impl ServerSpecBuilder {
) -> Result<(), ServerSpecBuilderError> {
self.builder
.add_network_device(api_request::parse_nic_from_request(nic)?)?;

Ok(())
}

Expand Down
Loading

0 comments on commit 70d7020

Please sign in to comment.