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

Add optional VLAN ID to uplink address configuration #5853

Merged
merged 20 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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 .github/buildomat/jobs/deploy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ infra_ip_last = \"$UPLINK_IP\"
/^routes/c\\
routes = \\[{nexthop = \"$GATEWAY_IP\", destination = \"0.0.0.0/0\"}\\]
/^addresses/c\\
addresses = \\[\"$UPLINK_IP/24\"\\]
addresses = \\[{address = \"$UPLINK_IP/24\"} \\]
}
" pkg/config-rss.toml
diff -u pkg/config-rss.toml{~,} || true
Expand Down
3 changes: 2 additions & 1 deletion clients/sled-agent-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ progenitor::generate_api!(
BgpConfig = { derives = [Eq, Hash] },
BgpPeerConfig = { derives = [Eq, Hash] },
OmicronPhysicalDiskConfig = { derives = [Eq, Hash, PartialOrd, Ord] },
PortConfigV1 = { derives = [Eq, Hash] },
PortConfigV2 = { derives = [Eq, Hash] },
RouteConfig = { derives = [Eq, Hash] },
UplinkAddressConfig = { derives = [Eq, Hash] },
VirtualNetworkInterfaceHost = { derives = [Eq, Hash] },
},
crates = {
Expand Down
4 changes: 2 additions & 2 deletions clients/wicketd-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ progenitor::generate_api!(
GetLocationResponse = { derives = [PartialEq, Eq, PartialOrd, Ord] },
ImageVersion = { derives = [PartialEq, Eq, PartialOrd, Ord]},
RackInitId = { derives = [PartialEq, Eq, PartialOrd, Ord] },
RackNetworkConfigV1 = { derives = [PartialEq, Eq, PartialOrd, Ord] },
RackNetworkConfigV2 = { derives = [PartialEq, Eq, PartialOrd, Ord] },
RackOperationStatus = { derives = [PartialEq, Eq, PartialOrd, Ord] },
RackResetId = { derives = [PartialEq, Eq, PartialOrd, Ord] },
RackV1Inventory = { derives = [PartialEq, Eq, PartialOrd, Ord]},
Expand Down Expand Up @@ -62,7 +62,7 @@ progenitor::generate_api!(
Ipv4Range = omicron_common::address::Ipv4Range,
Ipv6Range = omicron_common::address::Ipv6Range,
M2Slot = installinator_common::M2Slot,
PortConfigV1 = omicron_common::api::internal::shared::PortConfigV1,
PortConfigV2 = omicron_common::api::internal::shared::PortConfigV2,
PortFec = omicron_common::api::internal::shared::PortFec,
PortSpeed = omicron_common::api::internal::shared::PortSpeed,
ProgressEventForGenericSpec = update_engine::events::ProgressEvent<update_engine::NestedSpec>,
Expand Down
3 changes: 3 additions & 0 deletions common/src/api/external/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2553,6 +2553,9 @@ pub struct SwitchPortAddressConfig {
/// The IP address and prefix.
pub address: oxnet::IpNet,

/// An optional VLAN ID
pub vlan_id: Option<u16>,

/// The interface name this address belongs to.
// TODO: https://github.com/oxidecomputer/omicron/issues/3050
// Use `Name` instead of `String` for `interface_name` type
Expand Down
133 changes: 83 additions & 50 deletions common/src/api/internal/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,21 +152,25 @@ pub enum SourceNatConfigError {
UnalignedPortPair { first_port: u16, last_port: u16 },
}

// We alias [`PortConfig`] to the current version of the protocol, so
// that we can convert between versions as necessary.
pub type PortConfig = PortConfigV2;

// We alias [`RackNetworkConfig`] to the current version of the protocol, so
// that we can convert between versions as necessary.
pub type RackNetworkConfig = RackNetworkConfigV1;
pub type RackNetworkConfig = RackNetworkConfigV2;
Nieuwejaar marked this conversation as resolved.
Show resolved Hide resolved

/// Initial network configuration
#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, JsonSchema)]
pub struct RackNetworkConfigV1 {
pub struct RackNetworkConfigV2 {
pub rack_subnet: Ipv6Net,
// TODO: #3591 Consider making infra-ip ranges implicit for uplinks
/// First ip address to be used for configuring network infrastructure
pub infra_ip_first: Ipv4Addr,
/// Last ip address to be used for configuring network infrastructure
pub infra_ip_last: Ipv4Addr,
/// Uplinks for connecting the rack to external networks
pub ports: Vec<PortConfigV1>,
pub ports: Vec<PortConfig>,
/// BGP configurations for connecting the rack to external networks
pub bgp: Vec<BgpConfig>,
/// BFD configuration for connecting the rack to external networks
Expand Down Expand Up @@ -299,12 +303,81 @@ pub struct RouteConfig {
pub vlan_id: Option<u16>,
}

#[derive(
Clone, Debug, Deserialize, Serialize, PartialEq, Eq, JsonSchema, Hash,
)]
pub struct UplinkAddressConfig {
pub address: IpNet,
/// The VLAN id (if any) associated with this address.
#[serde(default)]
pub vlan_id: Option<u16>,
}

impl UplinkAddressConfig {
pub fn addr(&self) -> IpAddr {
self.address.addr()
}
}

impl std::fmt::Display for UplinkAddressConfig {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self.vlan_id {
None => write!(f, "{}", self.address),
Some(v) => write!(f, "{};{}", self.address, v),
}
}
}

#[derive(Debug, PartialEq, Eq, Deserialize, Serialize)]
pub struct UplinkAddressConfigError(String);

impl std::fmt::Display for UplinkAddressConfigError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "parse switch location error: {}", self.0)
}
}

/// Convert a string into an UplinkAddressConfig.
/// 192.168.1.1/24 => UplinkAddressConfig { 192.168.1.1/24, None }
/// 192.168.1.1/24;200 => UplinkAddressConfig { 192.168.1.1/24, Some(200) }
impl FromStr for UplinkAddressConfig {
type Err = UplinkAddressConfigError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let fields: Vec<&str> = s.split(';').collect();
let (address, vlan_id) = match fields.len() {
1 => Ok((fields[0], None)),
2 => Ok((fields[0], Some(fields[1]))),
_ => Err(UplinkAddressConfigError(format!(
"not a valid uplink address: {s}"
))),
}?;
let address = address.parse().map_err(|_| {
UplinkAddressConfigError(format!(
"not a valid ip address: {address}"
))
})?;
let vlan_id = match vlan_id {
None => Ok(None),
Some(v) => match v.parse() {
Err(_) => Err(format!("invalid vlan id: {v}")),
Ok(vlan_id) if vlan_id > 1 && vlan_id < 4096 => {
Ok(Some(vlan_id))
}
Ok(vlan_id) => Err(format!("vlan id out of range: {vlan_id}")),
},
}
.map_err(|e| UplinkAddressConfigError(e))?;
Ok(UplinkAddressConfig { address, vlan_id })
}
}

#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, JsonSchema)]
pub struct PortConfigV1 {
pub struct PortConfigV2 {
/// The set of routes associated with this port.
pub routes: Vec<RouteConfig>,
/// This port's addresses.
pub addresses: Vec<IpNet>,
/// This port's addresses and optional vlan IDs
pub addresses: Vec<UplinkAddressConfig>,
Nieuwejaar marked this conversation as resolved.
Show resolved Hide resolved
/// Switch the port belongs to.
pub switch: SwitchLocation,
/// Nmae of the port this config applies to.
Expand All @@ -320,46 +393,6 @@ pub struct PortConfigV1 {
pub autoneg: bool,
}

impl From<UplinkConfig> for PortConfigV1 {
fn from(value: UplinkConfig) -> Self {
PortConfigV1 {
routes: vec![RouteConfig {
destination: "0.0.0.0/0".parse().unwrap(),
nexthop: value.gateway_ip.into(),
vlan_id: None,
}],
addresses: vec![value.uplink_cidr.into()],
switch: value.switch,
port: value.uplink_port,
uplink_port_speed: value.uplink_port_speed,
uplink_port_fec: value.uplink_port_fec,
bgp_peers: vec![],
autoneg: false,
}
}
}

/// Deprecated, use PortConfigV1 instead. Cannot actually deprecate due to
/// <https://github.com/serde-rs/serde/issues/2195>
#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, JsonSchema)]
pub struct UplinkConfig {
/// Gateway address
pub gateway_ip: Ipv4Addr,
/// Switch to use for uplink
pub switch: SwitchLocation,
/// Switchport to use for external connectivity
pub uplink_port: String,
/// Speed for the Switchport
pub uplink_port_speed: PortSpeed,
/// Forward Error Correction setting for the uplink port
pub uplink_port_fec: PortFec,
/// IP Address and prefix (e.g., `192.168.0.1/16`) to apply to switchport
/// (must be in infra_ip pool)
pub uplink_cidr: Ipv4Net,
/// VLAN id to use for uplink
pub uplink_vid: Option<u16>,
}

/// A set of switch uplinks.
#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)]
pub struct SwitchPorts {
Expand All @@ -372,12 +405,12 @@ pub struct HostPortConfig {
pub port: String,

/// IP Address and prefix (e.g., `192.168.0.1/16`) to apply to switchport
/// (must be in infra_ip pool)
pub addrs: Vec<IpNet>,
/// (must be in infra_ip pool). May also include an optional VLAN ID.
pub addrs: Vec<UplinkAddressConfig>,
}

impl From<PortConfigV1> for HostPortConfig {
fn from(x: PortConfigV1) -> Self {
impl From<PortConfigV2> for HostPortConfig {
fn from(x: PortConfigV2) -> Self {
Self { port: x.port, addrs: x.addresses }
}
}
Expand Down
2 changes: 1 addition & 1 deletion dev-tools/xtask/src/virtual_hardware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ const ZPOOL: &'static str = "/usr/sbin/zpool";
const ZONEADM: &'static str = "/usr/sbin/zoneadm";

const SIDECAR_LITE_COMMIT: &'static str =
"960f11afe859e0316088e04578aedb700fba6159";
"de6fab7885a6bbc5327accffd2a872a31e2f1cb6";
const SOFTNPU_COMMIT: &'static str = "3203c51cf4473d30991b522062ac0df2e045c2f2";
const PXA_MAC_DEFAULT: &'static str = "a8:e1:de:01:70:1d";

Expand Down
2 changes: 1 addition & 1 deletion docs/how-to-run.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ routes = [{nexthop = "192.168.1.199", destination = "0.0.0.0/0"}]
# Addresses associated with this port.
# For softnpu, an address within the "infra" block above that will be used for
# the softnpu uplink port. You can just pick the first address in that pool.
addresses = ["192.168.1.30/24"]
addresses = [{address = "192.168.1.30/24"}]
# Name of the uplink port. This should always be "qsfp0" when using softnpu.
port = "qsfp0"
# The speed of this port.
Expand Down
1 change: 1 addition & 0 deletions nexus/db-model/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ table! {
rsvd_address_lot_block_id -> Uuid,
address -> Inet,
interface_name -> Text,
vlan_id -> Nullable<Int4>,
}
}

Expand Down
3 changes: 2 additions & 1 deletion nexus/db-model/src/schema_versions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::collections::BTreeMap;
///
/// This must be updated when you change the database schema. Refer to
/// schema/crdb/README.adoc in the root of this repository for details.
pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(72, 0, 0);
pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(73, 0, 0);

/// List of all past database schema versions, in *reverse* order
///
Expand All @@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy<Vec<KnownVersion>> = Lazy::new(|| {
// | leaving the first copy as an example for the next person.
// v
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
KnownVersion::new(73, "add-vlan-to-uplink"),
KnownVersion::new(72, "fix-provisioning-counters"),
KnownVersion::new(71, "add-saga-unwound-vmm-state"),
KnownVersion::new(70, "separate-instance-and-vmm-states"),
Expand Down
4 changes: 4 additions & 0 deletions nexus/db-model/src/switch_port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,7 @@ pub struct SwitchPortAddressConfig {
pub rsvd_address_lot_block_id: Uuid,
pub address: IpNetwork,
pub interface_name: String,
pub vlan_id: Option<SqlU16>,
}

impl SwitchPortAddressConfig {
Expand All @@ -731,13 +732,15 @@ impl SwitchPortAddressConfig {
rsvd_address_lot_block_id: Uuid,
address: IpNetwork,
interface_name: String,
vlan_id: Option<u16>,
) -> Self {
Self {
port_settings_id,
address_lot_block_id,
rsvd_address_lot_block_id,
address,
interface_name,
vlan_id: vlan_id.map(|x| x.into()),
}
}
}
Expand All @@ -749,6 +752,7 @@ impl Into<external::SwitchPortAddressConfig> for SwitchPortAddressConfig {
address_lot_block_id: self.address_lot_block_id,
address: self.address.into(),
interface_name: self.interface_name,
vlan_id: self.vlan_id.map(|x| x.into()),
}
}
}
1 change: 1 addition & 0 deletions nexus/db-queries/src/db/datastore/switch_port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,7 @@ impl DataStore {
rsvd_block.id,
address.address.into(),
interface_name.clone(),
address.vlan_id
));

}
Expand Down
27 changes: 19 additions & 8 deletions nexus/src/app/background/sync_switch_configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ use omicron_common::{
use serde_json::json;
use sled_agent_client::types::{
BgpConfig as SledBgpConfig, BgpPeerConfig as SledBgpPeerConfig,
EarlyNetworkConfig, EarlyNetworkConfigBody, HostPortConfig, PortConfigV1,
RackNetworkConfigV1, RouteConfig as SledRouteConfig,
EarlyNetworkConfig, EarlyNetworkConfigBody, HostPortConfig, PortConfigV2,
RackNetworkConfigV2, RouteConfig as SledRouteConfig, UplinkAddressConfig,
Nieuwejaar marked this conversation as resolved.
Show resolved Hide resolved
};
use std::{
collections::{hash_map::Entry, HashMap, HashSet},
Expand Down Expand Up @@ -901,7 +901,7 @@ impl BackgroundTask for SwitchPortSettingsManager {

bgp.dedup();

let mut ports: Vec<PortConfigV1> = vec![];
let mut ports: Vec<PortConfigV2> = vec![];

for (location, port, change) in &changes {
let PortSettingsChange::Apply(info) = change else {
Expand All @@ -922,8 +922,12 @@ impl BackgroundTask for SwitchPortSettingsManager {
},
};

let mut port_config = PortConfigV1 {
addresses: info.addresses.iter().map(|a| a.address.into()).collect(),
let mut port_config = PortConfigV2 {
addresses: info.addresses.iter().map(|a|
UplinkAddressConfig {
address: a.address.into(),
vlan_id: a.vlan_id.map(|v| v.into())
}).collect(),
autoneg: info
.links
.get(0) //TODO breakout support
Expand Down Expand Up @@ -1096,10 +1100,10 @@ impl BackgroundTask for SwitchPortSettingsManager {

let mut desired_config = EarlyNetworkConfig {
generation: 0,
schema_version: 1,
schema_version: 2,
body: EarlyNetworkConfigBody {
ntp_servers,
rack_network_config: Some(RackNetworkConfigV1 {
rack_network_config: Some(RackNetworkConfigV2 {
rack_subnet: subnet,
infra_ip_first,
infra_ip_last,
Expand Down Expand Up @@ -1401,7 +1405,14 @@ fn uplinks(
};
let config = HostPortConfig {
port: port.port_name.clone(),
addrs: config.addresses.iter().map(|a| a.address.into()).collect(),
addrs: config
.addresses
.iter()
.map(|a| UplinkAddressConfig {
address: a.address.into(),
vlan_id: a.vlan_id.map(|v| v.into()),
})
.collect(),
};

match uplinks.entry(*location) {
Expand Down
9 changes: 7 additions & 2 deletions nexus/src/app/rack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,8 @@ impl super::Nexus {
.iter()
.map(|a| Address {
address_lot: NameOrId::Name(address_lot_name.clone()),
address: (*a),
address: a.address,
vlan_id: a.vlan_id,
})
.collect();

Expand All @@ -565,7 +566,11 @@ impl super::Nexus {
let routes: Vec<Route> = uplink_config
.routes
.iter()
.map(|r| Route { dst: r.destination, gw: r.nexthop, vid: None })
.map(|r| Route {
dst: r.destination,
gw: r.nexthop,
vid: r.vlan_id,
})
.collect();

port_settings_params
Expand Down
Loading
Loading