Skip to content

Commit f1df425

Browse files
committed
fix(api): removing a few more ipv6 guards in api handlers
Continuing to chip away at [IPv6 support](NVIDIA#84) work. Work thus far has included: - Moving to `IpNetwork` and `IpAddress` throughout ([NVIDIA#192](NVIDIA#192)). - Accepting IPv6 site prefixes and network segments. ([NVIDIA#204](NVIDIA#204)). - Making the IP allocator family-aware ([NVIDIA#217](NVIDIA#217)). - Making the prefix allocator family-aware ([NVIDIA#237](NVIDIA#237)). This PR is a little less exciting, and removes a few additional `"if ip.is_ipv6()"` guards at the API layer. With the previous PRs in place to make the backend support dual stack environments, these guards are no longer needed: the downstream code (such as database queries and instance lookups) all work with `IpAddr` and `inet` columns natively.* *Note: The DPA overlay/underlay guards in `dhcp/discover.rs` are **intentionally kept** here -- DPA will, for the foreseeable future, only support IPv4, because the Algo IP mechanism, and our inference of an underlay IP based on the `giaddr`, is IPv4 only; we don't even know how it will support IPv6 yet.* Changes here include: - Removed the IPv6 guard in the address finder `search()` function -- you can search IPv6 addresses for `StaticData`, `ResourcePools`, InstanceAddresses`, `MachineAddresses`, `BmcIp`, `ExploredEndpoint`, `LoopbackIp`, `NetworkSegment`, `RouteServers`, and `DpaAddresses` (even though DPA won't have an IPv6 address). The match on `NetworkSegment` did get small tweak -- it was hard-coded to `/32`, for single interfaces, so now it will do `/32` or `/128`. In practice, and as we get closer with IPv6, I'm not sure what prefix we'll allocate to single interfaces, so the `/128` will probably change here. - Removed the IPv6 guard from `get_cloud_init_instructions()` -- all of the downstream code from here is dual stack and supports IPv6. BUT, I did leave a note that, even though everything downstream is IPv6-capable, that it doesn't really mean much until we integrate DHCPv6 support to actually hand out IPv6 addresses, which once we do it, things should "just work". - A small tweak in `admin-cli` tests around parsing IP addresses. Just made it support IPv6. - Improve how we determine interface prefix lengths -- instead of just hard-coding values for IPv4 or IPv6, I'm seeing if we can get some mileage out of the `IdentifyAddressFamily` trait that we have hanging out, with an `.interface_prefix_len()` function on that. I had also considered an `InterfacePrefix` enum with `InterfacePrefix::Ipv4` and `InterfacePrefix::Ipv6`, but kept going back and forth. Signed-off-by: Chet Nichols III <chetn@nvidia.com>
1 parent fcd8404 commit f1df425

8 files changed

Lines changed: 58 additions & 39 deletions

File tree

crates/admin-cli/src/ip/args.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,5 +26,5 @@ pub enum Cmd {
2626
#[derive(Parser, Debug, Clone)]
2727
pub struct IpFind {
2828
/// The IP address we are looking to identify
29-
pub ip: std::net::Ipv4Addr,
29+
pub ip: std::net::IpAddr,
3030
}

crates/admin-cli/src/ip/tests.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ fn verify_cmd_structure() {
4343
// including testing required arguments, as well as optional
4444
// flag-specific checking.
4545

46-
// parse_find_with_valid_ip ensures find parses valid
46+
// parse_find_with_valid_ip ensures find parses a valid
4747
// IPv4 address.
4848
#[test]
4949
fn parse_find_with_valid_ip() {
@@ -84,12 +84,13 @@ fn parse_find_invalid_ip_fails() {
8484
assert!(result.is_err(), "should fail with invalid IP");
8585
}
8686

87-
// parse_find_ipv6_fails ensures find fails with IPv6
88-
// (expects IPv4).
87+
// parse_find_ipv6 ensures find parses a valid IPv6 address.
8988
#[test]
90-
fn parse_find_ipv6_fails() {
91-
let result = Cmd::try_parse_from(["ip", "find", "::1"]);
92-
assert!(result.is_err(), "should fail with IPv6 address");
89+
fn parse_find_ipv6() {
90+
let cmd = Cmd::try_parse_from(["ip", "find", "::1"]).expect("should parse IPv6 address");
91+
match cmd {
92+
Cmd::Find(args) => assert_eq!(args.ip.to_string(), "::1"),
93+
}
9394
}
9495

9596
// parse_find_missing_ip_fails ensures find requires

crates/api-db/src/ip_allocator.rs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
2020

2121
use carbide_uuid::instance::InstanceId;
2222
use carbide_uuid::network::NetworkPrefixId;
23+
use forge_network::ip::IdentifyAddressFamily;
2324
use ipnetwork::{IpNetwork, Ipv4Network, Ipv6Network};
2425
use model::address_selection_strategy::AddressSelectionStrategy;
2526
use model::network_prefix::NetworkPrefix;
@@ -349,7 +350,10 @@ fn build_allocated_networks(
349350
if let Some(gateway) = segment_prefix.gateway
350351
&& segment_prefix.prefix.contains(gateway)
351352
{
352-
allocated_ips.push(IpNetwork::new(gateway, max_prefix_len(gateway))?);
353+
allocated_ips.push(IpNetwork::new(
354+
gateway,
355+
gateway.address_family().interface_prefix_len(),
356+
)?);
353357
}
354358

355359
// Next, exclude the first "N" number of addresses in the segment
@@ -363,7 +367,7 @@ fn build_allocated_networks(
363367
.iter()
364368
.take(segment_prefix.num_reserved as usize)
365369
{
366-
let next_net = IpNetwork::new(next_ip, max_prefix_len(next_ip))?;
370+
let next_net = IpNetwork::new(next_ip, next_ip.address_family().interface_prefix_len())?;
367371
allocated_ips.push(next_net);
368372
}
369373

@@ -377,10 +381,13 @@ fn build_allocated_networks(
377381
if should_reserve_network_broadcast {
378382
let network_addr = segment_prefix.prefix.network();
379383
let broadcast_addr = segment_prefix.prefix.broadcast();
380-
allocated_ips.push(IpNetwork::new(network_addr, max_prefix_len(network_addr))?);
384+
allocated_ips.push(IpNetwork::new(
385+
network_addr,
386+
network_addr.address_family().interface_prefix_len(),
387+
)?);
381388
allocated_ips.push(IpNetwork::new(
382389
broadcast_addr,
383-
max_prefix_len(broadcast_addr),
390+
broadcast_addr.address_family().interface_prefix_len(),
384391
)?);
385392
}
386393

@@ -446,14 +453,6 @@ fn get_network_size(ip_network: &IpNetwork) -> u32 {
446453
}
447454
}
448455

449-
/// Returns the maximum prefix length for a single host address (32 for IPv4, 128 for IPv6).
450-
fn max_prefix_len(ip: IpAddr) -> u8 {
451-
match ip {
452-
IpAddr::V4(_) => 32,
453-
IpAddr::V6(_) => 128,
454-
}
455-
}
456-
457456
// get_network_details computes some details to be
458457
// used by next_available_prefix, including the
459458
// base IP for the network segment, the broadcast IP,

crates/api/Cargo.toml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,7 @@ x509-parser = { features = ["verify"], workspace = true }
173173

174174
[features]
175175
default = ["linux-build"]
176-
linux-build = [
177-
"tss-esapi",
178-
]
176+
linux-build = ["tss-esapi"]
179177

180178
[build-dependencies]
181179
carbide-version = { path = "../version" }

crates/api/src/handlers/finder.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use carbide_uuid::machine::MachineInterfaceId;
2727
use carbide_uuid::network::NetworkSegmentId;
2828
use carbide_uuid::vpc::VpcId;
2929
use db::{DatabaseError, ObjectColumnFilter, instance, network_segment, vpc};
30+
use forge_network::ip::IdentifyAddressFamily;
3031
use model::network_segment::NetworkSegmentSearchConfig;
3132
use model::resource_pool::ResourcePoolEntryState;
3233
use model::route_server::RouteServerSourceType;
@@ -201,11 +202,6 @@ async fn search(
201202
ip: &str,
202203
) -> Result<Option<rpc::IpAddressMatch>, CarbideError> {
203204
let addr: IpAddr = ip.parse()?;
204-
if addr.is_ipv6() {
205-
return Err(CarbideError::InvalidArgument(
206-
"Ipv6 not yet supported".to_string(),
207-
));
208-
}
209205

210206
let mut txn = api.txn_begin().await?;
211207

@@ -330,7 +326,10 @@ async fn search(
330326

331327
// Network segment that contains this IP address
332328
NetworkSegment => {
333-
let out = db::network_prefix::containing_prefix(&mut txn, &format!("{ip}/32")).await?;
329+
let host_prefix = addr.address_family().interface_prefix_len();
330+
let out =
331+
db::network_prefix::containing_prefix(&mut txn, &format!("{ip}/{host_prefix}"))
332+
.await?;
334333
out.first().map(|prefix| {
335334
let message = format!(
336335
"{ip} is in prefix {} of segment {}, gateway {}",

crates/api/src/handlers/pxe.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,14 @@ pub(crate) async fn get_cloud_init_instructions(
5757
let ip: IpAddr = ip_str
5858
.parse()
5959
.map_err(|e| Status::invalid_argument(format!("Failed parsing IP '{ip_str}': {e}")))?;
60-
if ip.is_ipv6() {
61-
return Err(CarbideError::internal("IPv6 not supported".to_string()).into());
62-
}
6360

61+
// Note that this code path supports IPv6 at the *API layer*, but won't be
62+
// able to be exercised until DHCPv6 is working, which is a whole other thing
63+
// we need to work on: machines need an IPv6 address before they can request
64+
// cloud-init instructions over IPv6, and while we've made changes to site
65+
// prefix, network segment, and IP allocators behind the scenes for supporting
66+
// dual stacking interfaces, none of that means much until DHCPv6 is working
67+
// to actually hand those addresses out.
6468
let instructions = match db::instance_address::find_by_address(&mut txn, ip).await? {
6569
None => {
6670
// assume there is no instance associated with this IP and check if there is an interface associated with it

crates/api/src/network_segment/allocate.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
1818

1919
use carbide_uuid::network::NetworkSegmentId;
2020
use carbide_uuid::vpc::{VpcId, VpcPrefixId};
21+
use forge_network::ip::IdentifyAddressFamily;
2122
use ipnetwork::IpNetwork;
2223
use itertools::Itertools;
2324
use model::network_prefix::NewNetworkPrefix;
@@ -44,12 +45,6 @@ fn u128_to_ip(val: u128, is_v6: bool) -> IpAddr {
4445
}
4546
}
4647

47-
/// max_prefix_bits returns the maximum prefix length for the address
48-
/// family (32 for IPv4, 128 for IPv6).
49-
fn max_prefix_bits(prefix: &IpNetwork) -> u32 {
50-
if prefix.is_ipv6() { 128 } else { 32 }
51-
}
52-
5348
/// wrap_to_prefix_start returns `addr` if it falls within the VPC prefix,
5449
/// otherwise wraps around to the start of the VPC prefix.
5550
fn wrap_to_prefix_start(addr: IpAddr, vpc_prefix: &IpNetwork) -> IpAddr {
@@ -131,7 +126,7 @@ impl PrefixAllocator {
131126
last_used_prefix: Option<IpNetwork>,
132127
prefix: u8,
133128
) -> CarbideResult<PrefixAllocator> {
134-
let max_bits = max_prefix_bits(&vpc_prefix) as u8;
129+
let max_bits = vpc_prefix.address_family().interface_prefix_len();
135130
if prefix > max_bits {
136131
return Err(CarbideError::InvalidArgument(format!(
137132
"prefix length {prefix} exceeds maximum for address family ({max_bits})"
@@ -253,7 +248,7 @@ impl Iterator for PrefixIterator {
253248
return Some(IpNetwork::new(ip, self.prefix).unwrap());
254249
}
255250

256-
let max_bits = max_prefix_bits(&self.vpc_prefix);
251+
let max_bits = self.vpc_prefix.address_family().interface_prefix_len() as u32;
257252
// Number of host bits in the needed prefix.
258253
let host_bits: u32 = max_bits - self.prefix as u32;
259254
// Mask for the network portion of the address.

crates/network/src/ip/address_family.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,17 @@ pub enum IpAddressFamily {
2222
Ipv6,
2323
}
2424

25+
impl IpAddressFamily {
26+
/// Returns the prefix length for a single interface address in this family
27+
/// (32 for IPv4, 128 for IPv6).
28+
pub const fn interface_prefix_len(self) -> u8 {
29+
match self {
30+
IpAddressFamily::Ipv4 => 32,
31+
IpAddressFamily::Ipv6 => 128,
32+
}
33+
}
34+
}
35+
2536
pub trait IdentifyAddressFamily {
2637
/// Return the address family for this value.
2738
fn address_family(&self) -> IpAddressFamily;
@@ -101,4 +112,16 @@ mod tests {
101112
Err(42)
102113
)
103114
}
115+
116+
#[test]
117+
fn test_interface_prefix_len() {
118+
assert_eq!(IpAddressFamily::Ipv4.interface_prefix_len(), 32);
119+
assert_eq!(IpAddressFamily::Ipv6.interface_prefix_len(), 128);
120+
121+
// Also test via the IdentifyAddressFamily trait on IpAddr.
122+
let v4: IpAddr = "10.0.0.1".parse().unwrap();
123+
let v6: IpAddr = "fd00::1".parse().unwrap();
124+
assert_eq!(v4.address_family().interface_prefix_len(), 32);
125+
assert_eq!(v6.address_family().interface_prefix_len(), 128);
126+
}
104127
}

0 commit comments

Comments
 (0)