From 35034f75d81821bf7f7901485143ff5f72608e9f Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Mon, 23 Oct 2023 18:58:58 -0700 Subject: [PATCH] Re-introduce support for setting a VLAN for an uplink from RSS (#4319) Going from the previous `UplinkConfig` format to the `PortConfigV1` introduced with the BGP work seems to have lost the ability to configure the VLAN ID for reaching a gateway. The actual APIs and functionality is still there so just needed to be wired back up. --- common/src/api/internal/shared.rs | 3 + nexus/src/app/rack.rs | 1 + .../app/sagas/switch_port_settings_apply.rs | 6 +- nexus/tests/integration_tests/switch_port.rs | 31 ++++++--- openapi/bootstrap-agent.json | 7 +++ openapi/nexus-internal.json | 7 +++ openapi/sled-agent.json | 7 +++ openapi/wicketd.json | 7 +++ schema/rss-sled-plan.json | 9 +++ sled-agent/src/bootstrap/early_networking.rs | 63 ++++++++++++++++++- sled-agent/src/rack_setup/service.rs | 1 + wicket/src/rack_setup/config_template.toml | 3 + wicket/src/rack_setup/config_toml.rs | 23 +++++-- wicket/src/ui/panes/rack_setup.rs | 7 ++- wicketd/src/rss_config.rs | 1 + 15 files changed, 161 insertions(+), 15 deletions(-) diff --git a/common/src/api/internal/shared.rs b/common/src/api/internal/shared.rs index 784da8fcc6..971dbbabbf 100644 --- a/common/src/api/internal/shared.rs +++ b/common/src/api/internal/shared.rs @@ -111,6 +111,8 @@ pub struct RouteConfig { pub destination: IpNetwork, /// The nexthop/gateway address. pub nexthop: IpAddr, + /// The VLAN ID the gateway is reachable over. + pub vid: Option, } #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, JsonSchema)] @@ -137,6 +139,7 @@ impl From for PortConfigV1 { routes: vec![RouteConfig { destination: "0.0.0.0/0".parse().unwrap(), nexthop: value.gateway_ip.into(), + vid: value.uplink_vid, }], addresses: vec![value.uplink_cidr.into()], switch: value.switch, diff --git a/nexus/src/app/rack.rs b/nexus/src/app/rack.rs index 907c3ffa78..21918d2687 100644 --- a/nexus/src/app/rack.rs +++ b/nexus/src/app/rack.rs @@ -663,6 +663,7 @@ impl super::Nexus { .map(|r| SledRouteConfig { destination: r.dst, nexthop: r.gw.ip(), + vid: r.vid.map(Into::into), }) .collect(), addresses: info.addresses.iter().map(|a| a.address).collect(), diff --git a/nexus/src/app/sagas/switch_port_settings_apply.rs b/nexus/src/app/sagas/switch_port_settings_apply.rs index 93dc45751a..8442080979 100644 --- a/nexus/src/app/sagas/switch_port_settings_apply.rs +++ b/nexus/src/app/sagas/switch_port_settings_apply.rs @@ -911,7 +911,11 @@ pub(crate) async fn bootstore_update( routes: settings .routes .iter() - .map(|r| RouteConfig { destination: r.dst, nexthop: r.gw.ip() }) + .map(|r| RouteConfig { + destination: r.dst, + nexthop: r.gw.ip(), + vid: r.vid.map(Into::into), + }) .collect(), addresses: settings.addresses.iter().map(|a| a.address).collect(), switch: switch_location, diff --git a/nexus/tests/integration_tests/switch_port.rs b/nexus/tests/integration_tests/switch_port.rs index fada45694d..d4fd10f819 100644 --- a/nexus/tests/integration_tests/switch_port.rs +++ b/nexus/tests/integration_tests/switch_port.rs @@ -128,11 +128,18 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) { settings.routes.insert( "phy0".into(), RouteConfig { - routes: vec![Route { - dst: "1.2.3.0/24".parse().unwrap(), - gw: "1.2.3.4".parse().unwrap(), - vid: None, - }], + routes: vec![ + Route { + dst: "1.2.3.0/24".parse().unwrap(), + gw: "1.2.3.4".parse().unwrap(), + vid: None, + }, + Route { + dst: "5.6.7.0/24".parse().unwrap(), + gw: "5.6.7.8".parse().unwrap(), + vid: Some(5), + }, + ], }, ); // addresses @@ -159,7 +166,7 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) { .unwrap(); assert_eq!(created.links.len(), 1); - assert_eq!(created.routes.len(), 1); + assert_eq!(created.routes.len(), 2); assert_eq!(created.addresses.len(), 1); let link0 = &created.links[0]; @@ -178,6 +185,11 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) { let route0 = &created.routes[0]; assert_eq!(route0.dst, "1.2.3.0/24".parse().unwrap()); assert_eq!(route0.gw, "1.2.3.4".parse().unwrap()); + assert_eq!(route0.vlan_id, None); + let route1 = &created.routes[1]; + assert_eq!(route1.dst, "5.6.7.0/24".parse().unwrap()); + assert_eq!(route1.gw, "5.6.7.8".parse().unwrap()); + assert_eq!(route1.vlan_id, Some(5)); let addr0 = &created.addresses[0]; assert_eq!(addr0.address, "203.0.113.10/24".parse().unwrap()); @@ -195,7 +207,7 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) { .unwrap(); assert_eq!(roundtrip.links.len(), 1); - assert_eq!(roundtrip.routes.len(), 1); + assert_eq!(roundtrip.routes.len(), 2); assert_eq!(roundtrip.addresses.len(), 1); let link0 = &roundtrip.links[0]; @@ -214,6 +226,11 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) { let route0 = &roundtrip.routes[0]; assert_eq!(route0.dst, "1.2.3.0/24".parse().unwrap()); assert_eq!(route0.gw, "1.2.3.4".parse().unwrap()); + assert_eq!(route0.vlan_id, None); + let route1 = &roundtrip.routes[1]; + assert_eq!(route1.dst, "5.6.7.0/24".parse().unwrap()); + assert_eq!(route1.gw, "5.6.7.8".parse().unwrap()); + assert_eq!(route1.vlan_id, Some(5)); let addr0 = &roundtrip.addresses[0]; assert_eq!(addr0.address, "203.0.113.10/24".parse().unwrap()); diff --git a/openapi/bootstrap-agent.json b/openapi/bootstrap-agent.json index 6dcf756737..0a954fff0d 100644 --- a/openapi/bootstrap-agent.json +++ b/openapi/bootstrap-agent.json @@ -908,6 +908,13 @@ "description": "The nexthop/gateway address.", "type": "string", "format": "ip" + }, + "vid": { + "nullable": true, + "description": "The VLAN ID the gateway is reachable over.", + "type": "integer", + "format": "uint16", + "minimum": 0 } }, "required": [ diff --git a/openapi/nexus-internal.json b/openapi/nexus-internal.json index 411c52ddff..1a3be03de1 100644 --- a/openapi/nexus-internal.json +++ b/openapi/nexus-internal.json @@ -4507,6 +4507,13 @@ "description": "The nexthop/gateway address.", "type": "string", "format": "ip" + }, + "vid": { + "nullable": true, + "description": "The VLAN ID the gateway is reachable over.", + "type": "integer", + "format": "uint16", + "minimum": 0 } }, "required": [ diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index 486662853c..ec070a3a0b 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -2669,6 +2669,13 @@ "description": "The nexthop/gateway address.", "type": "string", "format": "ip" + }, + "vid": { + "nullable": true, + "description": "The VLAN ID the gateway is reachable over.", + "type": "integer", + "format": "uint16", + "minimum": 0 } }, "required": [ diff --git a/openapi/wicketd.json b/openapi/wicketd.json index 75db82e8e1..9fd05a9ca7 100644 --- a/openapi/wicketd.json +++ b/openapi/wicketd.json @@ -2493,6 +2493,13 @@ "description": "The nexthop/gateway address.", "type": "string", "format": "ip" + }, + "vid": { + "nullable": true, + "description": "The VLAN ID the gateway is reachable over.", + "type": "integer", + "format": "uint16", + "minimum": 0 } }, "required": [ diff --git a/schema/rss-sled-plan.json b/schema/rss-sled-plan.json index 39a9a68acc..7ba74ffc0a 100644 --- a/schema/rss-sled-plan.json +++ b/schema/rss-sled-plan.json @@ -581,6 +581,15 @@ "description": "The nexthop/gateway address.", "type": "string", "format": "ip" + }, + "vid": { + "description": "The VLAN ID the gateway is reachable over.", + "type": [ + "integer", + "null" + ], + "format": "uint16", + "minimum": 0.0 } } }, diff --git a/sled-agent/src/bootstrap/early_networking.rs b/sled-agent/src/bootstrap/early_networking.rs index 6c19080e9c..c539f6fdfd 100644 --- a/sled-agent/src/bootstrap/early_networking.rs +++ b/sled-agent/src/bootstrap/early_networking.rs @@ -509,7 +509,7 @@ impl<'a> EarlyNetworkSetup<'a> { { dpd_port_settings.v4_routes.insert( dst.to_string(), - RouteSettingsV4 { link_id: link_id.0, nexthop, vid: None }, + RouteSettingsV4 { link_id: link_id.0, nexthop, vid: r.vid }, ); } if let (IpNetwork::V6(dst), IpAddr::V6(nexthop)) = @@ -517,7 +517,7 @@ impl<'a> EarlyNetworkSetup<'a> { { dpd_port_settings.v6_routes.insert( dst.to_string(), - RouteSettingsV6 { link_id: link_id.0, nexthop, vid: None }, + RouteSettingsV6 { link_id: link_id.0, nexthop, vid: r.vid }, ); } } @@ -785,6 +785,65 @@ mod tests { routes: vec![RouteConfig { destination: "0.0.0.0/0".parse().unwrap(), nexthop: uplink.gateway_ip.into(), + vid: None, + }], + addresses: vec![uplink.uplink_cidr.into()], + switch: uplink.switch, + port: uplink.uplink_port, + uplink_port_speed: uplink.uplink_port_speed, + uplink_port_fec: uplink.uplink_port_fec, + bgp_peers: vec![], + }], + bgp: vec![], + }), + }, + }; + + assert_eq!(expected, v1); + } + + #[test] + fn serialized_early_network_config_v0_to_v1_conversion_with_vid() { + let v0 = EarlyNetworkConfigV0 { + generation: 1, + rack_subnet: Ipv6Addr::UNSPECIFIED, + ntp_servers: Vec::new(), + rack_network_config: Some(RackNetworkConfigV0 { + infra_ip_first: Ipv4Addr::UNSPECIFIED, + infra_ip_last: Ipv4Addr::UNSPECIFIED, + uplinks: vec![UplinkConfig { + gateway_ip: Ipv4Addr::UNSPECIFIED, + switch: SwitchLocation::Switch0, + uplink_port: "Port0".to_string(), + uplink_port_speed: PortSpeed::Speed100G, + uplink_port_fec: PortFec::None, + uplink_cidr: "192.168.0.1/16".parse().unwrap(), + uplink_vid: Some(10), + }], + }), + }; + + let v0_serialized = serde_json::to_vec(&v0).unwrap(); + let bootstore_conf = + bootstore::NetworkConfig { generation: 1, blob: v0_serialized }; + + let v1 = EarlyNetworkConfig::try_from(bootstore_conf).unwrap(); + let v0_rack_network_config = v0.rack_network_config.unwrap(); + let uplink = v0_rack_network_config.uplinks[0].clone(); + let expected = EarlyNetworkConfig { + generation: 1, + schema_version: 1, + body: EarlyNetworkConfigBody { + ntp_servers: v0.ntp_servers.clone(), + rack_network_config: Some(RackNetworkConfigV1 { + rack_subnet: Ipv6Network::new(v0.rack_subnet, 56).unwrap(), + infra_ip_first: v0_rack_network_config.infra_ip_first, + infra_ip_last: v0_rack_network_config.infra_ip_last, + ports: vec![PortConfigV1 { + routes: vec![RouteConfig { + destination: "0.0.0.0/0".parse().unwrap(), + nexthop: uplink.gateway_ip.into(), + vid: Some(10), }], addresses: vec![uplink.uplink_cidr.into()], switch: uplink.switch, diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index 7f6469d2c0..b54a8f0ba0 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -591,6 +591,7 @@ impl ServiceInner { .map(|r| NexusTypes::RouteConfig { destination: r.destination, nexthop: r.nexthop, + vid: r.vid, }) .collect(), addresses: config.addresses.clone(), diff --git a/wicket/src/rack_setup/config_template.toml b/wicket/src/rack_setup/config_template.toml index 617b61fadc..da07f42cd4 100644 --- a/wicket/src/rack_setup/config_template.toml +++ b/wicket/src/rack_setup/config_template.toml @@ -47,6 +47,9 @@ infra_ip_last = "" [[rack_network_config.ports]] # Routes associated with this port. # { nexthop = "1.2.3.4", destination = "0.0.0.0/0" } +# Can also optionally specify a VLAN id if the next hop is reachable +# over an 802.1Q tagged L2 segment. +# { nexthop = "5.6.7.8", destination = "5.6.7.0/24", vid = 5 } routes = [] # Addresses associated with this port. diff --git a/wicket/src/rack_setup/config_toml.rs b/wicket/src/rack_setup/config_toml.rs index e087c9aa7c..9616939308 100644 --- a/wicket/src/rack_setup/config_toml.rs +++ b/wicket/src/rack_setup/config_toml.rs @@ -245,6 +245,12 @@ fn populate_network_table( r.destination.to_string(), )), ); + if let Some(vid) = r.vid { + route.insert( + "vid", + Value::Integer(Formatted::new(vid.into())), + ); + } routes.push(Value::InlineTable(route)); } uplink.insert("routes", Item::Value(Value::Array(routes))); @@ -379,6 +385,7 @@ mod tests { .map(|r| InternalRouteConfig { destination: r.destination, nexthop: r.nexthop, + vid: r.vid, }) .collect(), addresses: config.addresses.clone(), @@ -478,10 +485,18 @@ mod tests { infra_ip_last: "172.30.0.10".parse().unwrap(), ports: vec![PortConfigV1 { addresses: vec!["172.30.0.1/24".parse().unwrap()], - routes: vec![RouteConfig { - destination: "0.0.0.0/0".parse().unwrap(), - nexthop: "172.30.0.10".parse().unwrap(), - }], + routes: vec![ + RouteConfig { + destination: "0.0.0.0/0".parse().unwrap(), + nexthop: "172.30.0.10".parse().unwrap(), + vid: None, + }, + RouteConfig { + destination: "10.20.0.0/16".parse().unwrap(), + nexthop: "10.0.0.20".parse().unwrap(), + vid: Some(20), + }, + ], bgp_peers: vec![BgpPeerConfig { asn: 47, addr: "10.2.3.4".parse().unwrap(), diff --git a/wicket/src/ui/panes/rack_setup.rs b/wicket/src/ui/panes/rack_setup.rs index 086d01ce9d..36febe404f 100644 --- a/wicket/src/ui/panes/rack_setup.rs +++ b/wicket/src/ui/panes/rack_setup.rs @@ -718,7 +718,12 @@ fn rss_config_text<'a>( vec![ Span::styled(" • Route : ", label_style), Span::styled( - format!("{} -> {}", r.destination, r.nexthop), + format!( + "{} -> {} (vid={})", + r.destination, + r.nexthop, + r.vid.unwrap_or(0), + ), ok_style, ), ] diff --git a/wicketd/src/rss_config.rs b/wicketd/src/rss_config.rs index a96acc56a0..d446ed71d1 100644 --- a/wicketd/src/rss_config.rs +++ b/wicketd/src/rss_config.rs @@ -511,6 +511,7 @@ fn validate_rack_network_config( .map(|r| BaRouteConfig { destination: r.destination, nexthop: r.nexthop, + vid: r.vid, }) .collect(), addresses: config.addresses.clone(),