Add optional VLAN ID to uplink address configuration (#5853)

.github/buildomat/jobs/deploy.sh

@@ -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

clients/sled-agent-client/src/lib.rs

@@ -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 = {

clients/wicketd-client/src/lib.rs

@@ -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]},
@@ -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>,

common/src/api/external/mod.rs

@@ -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

common/src/api/internal/shared.rs

@@ -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;
 
 /// 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
@@ -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>,
     /// Switch the port belongs to.
     pub switch: SwitchLocation,
     /// Nmae of the port this config applies to.
@@ -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 {
@@ -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 }
     }
 }

dev-tools/xtask/src/virtual_hardware.rs

@@ -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";
 

docs/how-to-run.adoc

@@ -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.

nexus/db-model/src/schema.rs

@@ -322,6 +322,7 @@ table! {
         rsvd_address_lot_block_id -> Uuid,
         address -> Inet,
         interface_name -> Text,
+        vlan_id -> Nullable<Int4>,
     }
 }
 

nexus/db-model/src/schema_versions.rs

@@ -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
 ///
@@ -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"),

nexus/db-model/src/switch_port.rs

@@ -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 {
@@ -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()),
         }
     }
 }
@@ -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()),
         }
     }
 }

nexus/db-queries/src/db/datastore/switch_port.rs

@@ -549,6 +549,7 @@ impl DataStore {
                                 rsvd_block.id,
                                 address.address.into(),
                                 interface_name.clone(),
+				address.vlan_id
                             ));
 
                         }

nexus/src/app/background/sync_switch_configuration.rs

@@ -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,
 };
 use std::{
     collections::{hash_map::Entry, HashMap, HashSet},
@@ -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 {
@@ -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
@@ -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,
@@ -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) {

nexus/src/app/rack.rs

@@ -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();
 
@@ -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