From 98bebdc223d8fa974801aedb488890e9942a0f87 Mon Sep 17 00:00:00 2001 From: "Adam H. Leventhal" Date: Thu, 23 May 2024 10:12:56 -0700 Subject: [PATCH] self-review nits --- common/src/address.rs | 4 +- common/src/api/external/mod.rs | 50 ------------------- nexus/src/app/instance_network.rs | 5 +- .../integration_tests/subnet_allocation.rs | 9 ++-- 4 files changed, 7 insertions(+), 61 deletions(-) diff --git a/common/src/address.rs b/common/src/address.rs index c36dee7cd0..c0aeb1a70f 100644 --- a/common/src/address.rs +++ b/common/src/address.rs @@ -184,7 +184,7 @@ pub const CP_SERVICES_RESERVED_ADDRESSES: u16 = 0xFFFF; // to assume that addresses in this subnet are available. pub const SLED_RESERVED_ADDRESSES: u16 = 32; -/// Wraps an [`Ipv6Network`] with a compile-time prefix length. +/// Wraps an [`Ipv6Net`] with a compile-time prefix length. #[derive(Debug, Clone, Copy, JsonSchema, Serialize, Hash, PartialEq, Eq)] #[schemars(rename = "Ipv6Subnet")] pub struct Ipv6Subnet { @@ -245,7 +245,6 @@ pub struct DnsSubnet { } impl DnsSubnet { - // TODO why are we returning a subnet? /// Returns the DNS server address within the subnet. /// /// This is the first address within the subnet. @@ -257,7 +256,6 @@ impl DnsSubnet { .unwrap() } - // TODO why are we returning a subnet? /// Returns the address which the Global Zone should create /// to be able to contact the DNS server. /// diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 0f4773580c..9454a20efa 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -3433,56 +3433,6 @@ mod test { assert!("hash:super_random".parse::().is_err()); } - // #[test] - // fn test_ipnet_first_last_address() { - // use oxnet::IpNet; - // use std::net::IpAddr; - // use std::net::Ipv4Addr; - // use std::net::Ipv6Addr; - - // let net: IpNet = "fd00::/128".parse().unwrap(); - // assert_eq!( - // net.first_address(), - // IpAddr::from(Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 0)), - // ); - // assert_eq!( - // net.last_address(), - // IpAddr::from(Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 0)), - // ); - - // let net: IpNet = "fd00::/64".parse().unwrap(); - // assert_eq!( - // net.first_address(), - // IpAddr::from(Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 0)), - // ); - // assert_eq!( - // net.last_address(), - // IpAddr::from(Ipv6Addr::new( - // 0xfd00, 0, 0, 0, 0xffff, 0xffff, 0xffff, 0xffff - // )), - // ); - - // let net: IpNet = "10.0.0.0/16".parse().unwrap(); - // assert_eq!( - // net.first_address(), - // IpAddr::from(Ipv4Addr::new(10, 0, 0, 0)), - // ); - // assert_eq!( - // net.last_address(), - // IpAddr::from(Ipv4Addr::new(10, 0, 255, 255)), - // ); - - // let net: IpNet = "10.0.0.0/32".parse().unwrap(); - // assert_eq!( - // net.first_address(), - // IpAddr::from(Ipv4Addr::new(10, 0, 0, 0)), - // ); - // assert_eq!( - // net.last_address(), - // IpAddr::from(Ipv4Addr::new(10, 0, 0, 0)), - // ); - // } - #[test] fn test_macaddr() { use super::MacAddr; diff --git a/nexus/src/app/instance_network.rs b/nexus/src/app/instance_network.rs index aab4d275cd..3c607bae78 100644 --- a/nexus/src/app/instance_network.rs +++ b/nexus/src/app/instance_network.rs @@ -510,7 +510,7 @@ pub(crate) async fn instance_ensure_dpd_config( )); } - let sled_address = Ipv6Net::new(*sled_ip_address.ip(), 128).unwrap(); + let sled_address = Ipv6Net::host_net(*sled_ip_address.ip()); // If all of our IPs are attached or are guaranteed to be owned // by the saga calling this fn, then we need to disregard and @@ -651,7 +651,7 @@ pub(crate) async fn probe_ensure_dpd_config( } } - let sled_address = Ipv6Net::new(sled_ip_address, 128).unwrap(); + let sled_address = Ipv6Net::host_net(sled_ip_address); for target_ip in ips .iter() @@ -1009,7 +1009,6 @@ async fn ensure_nat_entry( match target_ip.ip { IpNetwork::V4(v4net) => { let nat_entry = Ipv4NatValues { - // TODO could simplify this conversion? external_address: Ipv4Net::from(v4net).into(), first_port: target_ip.first_port, last_port: target_ip.last_port, diff --git a/nexus/tests/integration_tests/subnet_allocation.rs b/nexus/tests/integration_tests/subnet_allocation.rs index 60a5ccced4..794c769da4 100644 --- a/nexus/tests/integration_tests/subnet_allocation.rs +++ b/nexus/tests/integration_tests/subnet_allocation.rs @@ -132,14 +132,13 @@ async fn test_subnet_allocation(cptestctx: &ControlPlaneTestContext) { }, ]); - // Create enough instances to fill the subnet. There are subnet.size() total - // addresses, 6 of which are reserved. - let subnet_size = match subnet.size() { + // Create enough instances to fill the subnet. There are subnet.size() + // total addresses, 6 of which are reserved. + let subnet_size_minus_1 = match subnet.size() { Some(n) => n - 1, None => u32::MAX, } as usize; - let subnet_size = subnet_size - NUM_INITIAL_RESERVED_IP_ADDRESSES; - // let subnet_size = subnet.size() as usize - n_reserved_addresses; + let subnet_size = subnet_size_minus_1 - NUM_INITIAL_RESERVED_IP_ADDRESSES; for i in 0..subnet_size { create_instance_with( client,