self-review nits

oxidecomputer · May 23, 2024 · 98bebdc · 98bebdc
1 parent 4024b66
commit 98bebdc
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 61 deletions.
diff --git 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<const N: u8> {
@@ -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
@@ -3433,56 +3433,6 @@ mod test {
         assert!("hash:super_random".parse::<Digest>().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
@@ -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
@@ -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,