Skip to content

Commit 8fc6e7e

Browse files
committed
fix(agent): widen basic agent config types from Ipv4Addr to IpAddr
Still chipping away at [IPv6 support](NVIDIA#84). 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)). - Removing some more API guards and enhancing the `IdentifyAddressFamily` trait ([NVIDIA#324](NVIDIA#324)). - Adding `AAAA` record support to DNS ([NVIDIA#332](NVIDIA#332)). - Backend DHCP plumbing updates ([NVIDIA#335](NVIDIA#335)). - Adding a new `ResourcePoolType::Ipv6` type ([NVIDIA#344](NVIDIA#344)). - Adding a new `ResourcePoolType::Ipv6Prefix` type ([NVIDIA#345](NVIDIA#345)). All of that work has been on the "core" side -- the API, database, allocator, DNS, resource pools, etc. *This* PR is the first in a new series of changes focused on the agent side. The goal is, of course, to have IPv6 support plumbed through the agent, so it can make its way to HBN (via `nvue`) for configuring addresses, routes, ACLs, BGP, etc. Similar to "core" efforts, I'm taking a simple starting point by widening from `Ipv4Addr` to `IpAddr` across some of the more low-hanging agent config structs, CLI argument types, and service address types. Just like previous changes being a noop, this is no different, and the test-verified rendered template output remains the same. Changes included: - `InterfacesConfig.loopback_ip` going from `Ipv4Addr` to `IpAddr`. This only gets used as `conf.loopback_ip.to_string()` to feed templates. - `FrrConfig.loopback_ip` going from `Ipv4Addr` to `IpAddr`. Same thing as above -- only used as `conf.loopback_ip.to_string()` to feed templates. - `loopback_ip` going from `Ipv4Addr` to `IpAddr` in `NvueOptions`, `FrrOptions`, and `InterfacesOptions`. Since these are all parsed via `IpAddr::from_str`, they continued to work as-is. - `ServiceAddresses.pxe_ip` going from `Ipv4Addr` to `IpAddr`. Despite the field name, this is the UEFI HTTP boot server address, which would carry over to DHCPv6 just fine. - `ServiceAddresses.ntpservers` going from `Vec<Ipv4Addr>` to `Vec<IpAddr>`. NTP servers are still a DHCPv6 "feature". - At the `dhcp::build_server_config()` call site, added IPv4 filtering for `pxe_ip`, `ntpservers`, and `nameservers` (which was already filtered) before passing to the DHCPv4 config builder. `ServiceAddresses` now holds both families, but DHCPv4 options can only carry IPv4 addresses. - Left comments on things that are intentionally staying IPv4 for now -- the `DhcpOptions` CLI struct (DHCPv4-specific), and the internal HBN bridge prefix parsing (`169.254.x.x` link-local plumbing, though an IPv6 equivalent may be needed in the future). The existing tests (including `test_write_frr`) continue to pass, rendering templates as expected. IPv6 templates are NOT being introduced here yet -- for now, the important bit is widening types to `IpAddr` and making sure it continues to work as a drop-in with IPv4 Signed-off-by: Chet Nichols III <chetn@nvidia.com>
1 parent d7b9f59 commit 8fc6e7e

6 files changed

Lines changed: 58 additions & 28 deletions

File tree

crates/agent/src/command_line.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* See the License for the specific language governing permissions and
1515
* limitations under the License.
1616
*/
17-
use std::net::Ipv4Addr;
17+
use std::net::{IpAddr, Ipv4Addr};
1818
use std::path::PathBuf;
1919

2020
use carbide_uuid::machine::MachineId;
@@ -88,7 +88,7 @@ pub struct NvueOptions {
8888
pub site_global_vpc_vni: Option<u32>,
8989

9090
#[clap(long)]
91-
pub loopback_ip: Ipv4Addr,
91+
pub loopback_ip: IpAddr,
9292

9393
#[clap(long)]
9494
pub asn: u32,
@@ -222,7 +222,7 @@ pub struct FrrOptions {
222222
#[clap(long)]
223223
pub asn: u32,
224224
#[clap(long)]
225-
pub loopback_ip: Ipv4Addr,
225+
pub loopback_ip: IpAddr,
226226
#[clap(long, help = "Format is 'id,host_route', e.g. --vlan 1,xyz. Repeats.")]
227227
pub vlan: Vec<String>,
228228
#[clap(long, default_value = "etv")]
@@ -243,7 +243,7 @@ pub struct InterfacesOptions {
243243
#[clap(long, help = "Full path of interfaces file")]
244244
pub path: String,
245245
#[clap(long)]
246-
pub loopback_ip: Ipv4Addr,
246+
pub loopback_ip: IpAddr,
247247
#[clap(long, help = "Blank for admin network, vxlan48 for tenant networks")]
248248
pub vni_device: String,
249249
#[clap(
@@ -261,6 +261,9 @@ pub struct DhcpOptions {
261261
pub path: String,
262262
#[clap(long, help = "vlan numeric id. Repeats")]
263263
pub vlan: Vec<u32>,
264+
// Note that these will be staying IPv4 only for now. This
265+
// config block is pretty tailored towards DHCPv4, and may
266+
// get refactored a bit as part of adding DHCPv6 support.
264267
#[clap(long, help = "DHCP server IP address. Repeats")]
265268
pub dhcp: Vec<Ipv4Addr>,
266269
#[clap(long, help = "Remote ID to be filled in Option 82 - Agent Remote ID")]

crates/agent/src/ethernet_virtualization.rs

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,14 @@ struct DhcpServerPaths {
143143
host_config: FPath,
144144
}
145145

146-
/// Stores addresses of dependent services that the DHCP module announces
146+
/// Stores addresses of dependent services that the DHCP module announces.
147+
/// Note that these can apply to both IPv4 and IPv6; pxe_ip is actually
148+
/// UEFI HTTP boot in this case, and NTP is still NTP. We should be able
149+
/// to leverage this struct even in DHCPv6 land (whereas other things don't
150+
/// really carry through to DHCPv6).
147151
pub struct ServiceAddresses {
148-
pub pxe_ip: Ipv4Addr,
149-
pub ntpservers: Vec<Ipv4Addr>,
152+
pub pxe_ip: IpAddr,
153+
pub ntpservers: Vec<IpAddr>,
150154
pub nameservers: Vec<IpAddr>,
151155
}
152156

@@ -519,6 +523,9 @@ pub async fn update_traffic_intercept_bridging(
519523
}
520524
};
521525

526+
// IPv4 only — internal HBN bridge plumbing uses 169.254.x.x link-local
527+
// addressing for DPU↔HBN communication. An IPv6 equivalent (fe80:: or
528+
// similar) may be needed in the future for dual-stack bridging.
522529
let bridge_prefix = bridge_config
523530
.internal_bridge_routing_prefix
524531
.parse::<Ipv4Net>()?;
@@ -1093,14 +1100,33 @@ fn write_dhcp_server_config(
10931100

10941101
let loopback_ip = mh_nc.loopback_ip.parse()?;
10951102

1096-
let nameservers = service_addrs
1103+
// Filter to IPv4 for the DHCPv4 server config — ServiceAddresses holds
1104+
// both families, but DHCPv4 options can only carry IPv4 addresses.
1105+
let nameservers_v4 = service_addrs
10971106
.nameservers
10981107
.iter()
10991108
.filter_map(|x| match x {
11001109
IpAddr::V4(x) => Some(*x),
11011110
_ => None,
11021111
})
11031112
.collect::<Vec<Ipv4Addr>>();
1113+
let ntpservers_v4 = service_addrs
1114+
.ntpservers
1115+
.iter()
1116+
.filter_map(|x| match x {
1117+
IpAddr::V4(x) => Some(*x),
1118+
_ => None,
1119+
})
1120+
.collect::<Vec<Ipv4Addr>>();
1121+
let pxe_ip_v4 = match service_addrs.pxe_ip {
1122+
IpAddr::V4(v4) => v4,
1123+
IpAddr::V6(_) => {
1124+
return Err(eyre::eyre!(
1125+
"DHCPv4 server config requires an IPv4 PXE/UEFI HTTP boot address, got {}",
1126+
service_addrs.pxe_ip
1127+
));
1128+
}
1129+
};
11041130

11051131
let mut has_changes = false;
11061132

@@ -1120,12 +1146,8 @@ fn write_dhcp_server_config(
11201146
Err(err) => tracing::error!("Write DHCP server {}: {err:#}", dhcp_server_path.server),
11211147
}
11221148

1123-
let next_contents = dhcp::build_server_config(
1124-
service_addrs.pxe_ip,
1125-
service_addrs.ntpservers.clone(),
1126-
nameservers,
1127-
loopback_ip,
1128-
)?;
1149+
let next_contents =
1150+
dhcp::build_server_config(pxe_ip_v4, ntpservers_v4, nameservers_v4, loopback_ip)?;
11291151
match write(
11301152
next_contents,
11311153
&dhcp_server_path.config,
@@ -2974,11 +2996,11 @@ mod tests {
29742996
let ip = FPath(PathBuf::from(i.path()));
29752997

29762998
let service_addrs = ServiceAddresses {
2977-
pxe_ip: Ipv4Addr::from([10, 0, 0, 1]),
2999+
pxe_ip: IpAddr::from([10, 0, 0, 1]),
29783000
ntpservers: vec![
2979-
Ipv4Addr::from([127, 0, 0, 1]),
2980-
Ipv4Addr::from([127, 0, 0, 2]),
2981-
Ipv4Addr::from([127, 0, 0, 3]),
3001+
IpAddr::from([127, 0, 0, 1]),
3002+
IpAddr::from([127, 0, 0, 2]),
3003+
IpAddr::from([127, 0, 0, 3]),
29823004
],
29833005
nameservers: vec![IpAddr::from([10, 1, 1, 1])],
29843006
};
@@ -3042,7 +3064,7 @@ mod tests {
30423064
assert!(host_config_str.contains("mtu: 1500"));
30433065

30443066
let service_addrs = ServiceAddresses {
3045-
pxe_ip: Ipv4Addr::from([10, 0, 0, 1]),
3067+
pxe_ip: IpAddr::from([10, 0, 0, 1]),
30463068
ntpservers: vec![],
30473069
nameservers: vec![IpAddr::from([10, 1, 1, 1])],
30483070
};

crates/agent/src/frr.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* See the License for the specific language governing permissions and
1515
* limitations under the License.
1616
*/
17-
use std::net::Ipv4Addr;
17+
use std::net::IpAddr;
1818

1919
use gtmpl_derive::Gtmpl;
2020

@@ -76,7 +76,7 @@ pub fn blank() -> String {
7676
/// What we need in order to generate an frr.conf
7777
pub struct FrrConfig {
7878
pub asn: u32,
79-
pub loopback_ip: Ipv4Addr,
79+
pub loopback_ip: IpAddr,
8080
pub uplinks: Vec<String>,
8181
pub access_vlans: Vec<FrrVlanConfig>,
8282
pub vpc_vni: Option<u32>,
@@ -116,6 +116,8 @@ struct TmplFrrConfigParameters {
116116

117117
#[cfg(test)]
118118
mod tests {
119+
use std::net::Ipv4Addr;
120+
119121
use super::{FrrConfig, build};
120122
use crate::HBNDeviceNames;
121123

@@ -128,7 +130,7 @@ mod tests {
128130
.iter()
129131
.map(|x| x.to_string())
130132
.collect(),
131-
loopback_ip: [192, 168, 0, 1].into(),
133+
loopback_ip: Ipv4Addr::from([192, 168, 0, 1]).into(),
132134
access_vlans: vec![],
133135
vpc_vni: None,
134136
route_servers: vec![],

crates/agent/src/interfaces.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* See the License for the specific language governing permissions and
1515
* limitations under the License.
1616
*/
17-
use std::net::Ipv4Addr;
17+
use std::net::IpAddr;
1818

1919
use gtmpl_derive::Gtmpl;
2020
use serde::Deserialize;
@@ -55,7 +55,7 @@ pub fn blank() -> String {
5555
}
5656

5757
pub struct InterfacesConfig {
58-
pub loopback_ip: Ipv4Addr,
58+
pub loopback_ip: IpAddr,
5959
pub uplinks: Vec<String>,
6060
pub vni_device: String,
6161
pub networks: Vec<Network>,

crates/agent/src/main_loop.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
use std::collections::HashSet;
1919
use std::ffi::OsStr;
20-
use std::net::{IpAddr, Ipv4Addr};
20+
use std::net::IpAddr;
2121
use std::ops::Add;
2222
use std::path::PathBuf;
2323
use std::str::FromStr;
@@ -230,13 +230,13 @@ pub async fn setup_and_run(
230230

231231
let nameservers = url_resolver.nameservers();
232232
ServiceAddresses {
233-
pxe_ip,
234-
ntpservers,
233+
pxe_ip: pxe_ip.into(),
234+
ntpservers: ntpservers.into_iter().map(IpAddr::from).collect(),
235235
nameservers,
236236
}
237237
} else {
238238
ServiceAddresses {
239-
pxe_ip: Ipv4Addr::from([127, 0, 0, 1]),
239+
pxe_ip: IpAddr::from([127, 0, 0, 1]),
240240
ntpservers: vec![],
241241
nameservers: vec![IpAddr::from([127, 0, 0, 1])],
242242
}

crates/agent/src/nvue.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,9 @@ pub fn build(conf: NvueConfig) -> eyre::Result<String> {
302302
vf_intercept_hbn_representor_ip,
303303
public_prefix_internal_next_hop,
304304
intercept_bridge_prefix_len,
305+
// IPv4 only — internal HBN bridge plumbing uses 169.254.x.x link-local
306+
// addressing for DPU↔HBN communication. An IPv6 equivalent (fe80:: or
307+
// similar) may be needed in the future for dual-stack bridging.
305308
) = if let Some(bridge_prefix) = conf
306309
.internal_bridge_routing_prefix
307310
.map(|p| p.parse::<Ipv4Net>())

0 commit comments

Comments
 (0)