diff --git a/crates/agent/src/ethernet_virtualization.rs b/crates/agent/src/ethernet_virtualization.rs index be409c621e..04de2a42f3 100644 --- a/crates/agent/src/ethernet_virtualization.rs +++ b/crates/agent/src/ethernet_virtualization.rs @@ -137,6 +137,52 @@ pub struct ServiceAddresses { pub nameservers: Vec, } +fn build_dhcp_ntp_servers( + nc: &rpc::ManagedHostNetworkConfigResponse, + service_addrs: &ServiceAddresses, +) -> Vec { + // Start with the NTP servers from the service addresses. + let mut ntp_servers = service_addrs + .ntpservers + .iter() + .filter_map(|x| match x { + IpAddr::V4(x) => Some(*x), + _ => None, + }) + .collect::>(); + + // If the site has configured NTP servers, use them instead. + if !nc.ntp_servers.is_empty() { + let site_ntp_servers: Vec = nc.ntp_servers + .iter() + .filter_map(|s| match IpAddr::from_str(s) { + Ok(IpAddr::V4(ip)) => Some(ip), + Ok(IpAddr::V6(_)) => { + tracing::debug!( + ntp_server = %s, + "IPv6 NTP server from ManagedHostNetworkConfigResponse is ignored for DHCPv4 config" + ); + None + } + Err(e) => { + tracing::debug!( + ntp_server = %s, + error = %e, + "Invalid NTP server IP from ManagedHostNetworkConfigResponse, ignoring" + ); + None + } + }) + .collect(); + + if !site_ntp_servers.is_empty() { + ntp_servers = site_ntp_servers; + } + } + + ntp_servers +} + /// How we tell HBN to notice the new file we wrote #[derive(Debug)] struct PostAction { @@ -852,14 +898,7 @@ async fn update_dhcp_via_grpc( }) .collect::>(); - let ntpservers_v4 = service_addrs - .ntpservers - .iter() - .filter_map(|x| match x { - IpAddr::V4(x) => Some(*x), - _ => None, - }) - .collect::>(); + let ntpservers_v4 = build_dhcp_ntp_servers(network_config, service_addrs); let pxe_ip_v4 = service_addrs .pxe_ips @@ -1265,14 +1304,7 @@ fn write_dhcp_v4_server_config( }) .collect::>(); - let ntpservers_v4 = service_addrs - .ntpservers - .iter() - .filter_map(|x| match x { - IpAddr::V4(x) => Some(*x), - _ => None, - }) - .collect::>(); + let ntpservers_v4 = build_dhcp_ntp_servers(nc, service_addrs); let pxe_ip_v4 = service_addrs .pxe_ips @@ -1731,6 +1763,54 @@ mod tests { carbide_host_support::init_logging().unwrap(); } + #[test] + fn test_build_dhcp_ntp_servers() { + let service_addrs = ServiceAddresses { + pxe_ips: vec![], + ntpservers: vec![IpAddr::from([192, 0, 2, 20])], + nameservers: vec![], + }; + let nc = rpc::ManagedHostNetworkConfigResponse { + ntp_servers: vec!["198.51.100.1".to_string(), "198.51.100.2".to_string()], + ..Default::default() + }; + + let out = build_dhcp_ntp_servers(&nc, &service_addrs); + assert_eq!( + out, + vec![ + Ipv4Addr::from([198, 51, 100, 1]), + Ipv4Addr::from([198, 51, 100, 2]) + ] + ); + } + + #[test] + fn test_build_dhcp_ntp_servers_fallback() { + let service_addrs = ServiceAddresses { + pxe_ips: vec![], + ntpservers: vec![IpAddr::from([192, 0, 2, 20])], + nameservers: vec![], + }; + + let empty_nc = rpc::ManagedHostNetworkConfigResponse::default(); + + assert_eq!( + build_dhcp_ntp_servers(&empty_nc, &service_addrs), + vec![Ipv4Addr::from([192, 0, 2, 20])] + ); + + let invalid_nc = rpc::ManagedHostNetworkConfigResponse { + ntp_servers: vec!["not-an-ip".to_string(), "2001:db8::1".to_string()], + ..Default::default() + }; + + assert_eq!( + build_dhcp_ntp_servers(&invalid_nc, &service_addrs), + vec![Ipv4Addr::from([192, 0, 2, 20])] + ); + } + #[test] fn test_hostname() -> Result<(), Box> { let syscall_h = super::hostname()?; @@ -2626,6 +2706,7 @@ mod tests { // yes it's in there twice I dunno either dhcp_servers: vec!["10.217.5.197".to_string(), "10.217.5.197".to_string()], + ntp_servers: vec![], vni_device: "vxlan48".to_string(), managed_host_config: Some(netconf), @@ -3089,6 +3170,7 @@ mod tests { // yes it's in there twice I dunno either dhcp_servers: vec!["10.217.5.197".to_string(), "10.217.5.197".to_string()], + ntp_servers: vec![], vni_device: "vxlan48".to_string(), managed_host_config: Some(netconf), @@ -3284,6 +3366,7 @@ mod tests { routing_profile: None, traffic_intercept_config: None, dhcp_servers: vec![], + ntp_servers: vec![], vni_device: "vxlan48".to_string(), managed_host_config: Some(netconf), managed_host_config_version: "V1-T1".to_string(), diff --git a/crates/agent/src/tests/full.rs b/crates/agent/src/tests/full.rs index 965fc53386..e77937d752 100644 --- a/crates/agent/src/tests/full.rs +++ b/crates/agent/src/tests/full.rs @@ -868,6 +868,7 @@ async fn handle_netconf(AxumState(state): AxumState>>) -> impl }), dhcp_servers: vec!["127.0.0.1".to_string()], + ntp_servers: vec![], vni_device: "".to_string(), managed_host_config: Some(rpc::forge::ManagedHostNetworkConfig { diff --git a/crates/api-model/src/rpc_conv/dhcp_record.rs b/crates/api-model/src/rpc_conv/dhcp_record.rs index fab5f68ae2..f679bcc8a2 100644 --- a/crates/api-model/src/rpc_conv/dhcp_record.rs +++ b/crates/api-model/src/rpc_conv/dhcp_record.rs @@ -32,6 +32,7 @@ impl From for rpc::forge::DhcpRecord { gateway: record.gateway.map(|gw| gw.to_string()), booturl: None, // TODO(ajf): extend database, synthesize URL last_invalidation_time: Some(record.last_invalidation_time.into()), + ntp_servers: vec![], } } } diff --git a/crates/api/src/cfg/file.rs b/crates/api/src/cfg/file.rs index 0be12cd0f0..f46f894814 100644 --- a/crates/api/src/cfg/file.rs +++ b/crates/api/src/cfg/file.rs @@ -112,6 +112,10 @@ pub struct CarbideConfig { #[serde(default)] pub dhcp_servers: Vec, + /// NTP server IP addresses for the site. + #[serde(default)] + pub ntp_servers: Vec, + /// Route server IP addresses for L2VPN (Ethernet /// Virtual) network support on DPUs. #[serde(default)] diff --git a/crates/api/src/dhcp/discover.rs b/crates/api/src/dhcp/discover.rs index 64e30a9f9a..f81bc62af4 100644 --- a/crates/api/src/dhcp/discover.rs +++ b/crates/api/src/dhcp/discover.rs @@ -51,6 +51,7 @@ async fn handle_overlay_from_dpa( dpa_if: &mut DpaInterface, macaddr: MacAddress, desired_addr: IpAddr, + ntp_servers: &[String], ) -> Result>, CarbideError> { let IpAddr::V4(ip_v4_addr) = desired_addr else { return Err(CarbideError::internal( @@ -79,6 +80,7 @@ async fn handle_overlay_from_dpa( mtu: SPX_MTU, fqdn: String::new(), prefix, + ntp_servers: ntp_servers.to_vec(), }))) } @@ -89,6 +91,7 @@ async fn handle_underlay_from_dpa( dpa_if: &mut DpaInterface, macaddr: MacAddress, relay_address: String, + ntp_servers: &[String], ) -> Result>, CarbideError> { // The relay address and the mac address should differ only in bit 0 let relay_addr = Ipv4Addr::from_str(&relay_address)?; @@ -118,6 +121,7 @@ async fn handle_underlay_from_dpa( mtu: SPX_MTU, fqdn: String::new(), prefix, + ntp_servers: ntp_servers.to_vec(), }))) } @@ -155,10 +159,24 @@ async fn handle_dhcp_from_dpa( let mut dpa_if = dpa_ifs.remove(0); if let Some(addr) = desired_address { - return handle_overlay_from_dpa(txn, &mut dpa_if, macaddr, addr).await; + return handle_overlay_from_dpa( + txn, + &mut dpa_if, + macaddr, + addr, + &api.runtime_config.ntp_servers, + ) + .await; } - handle_underlay_from_dpa(txn, &mut dpa_if, macaddr, relay_address).await + handle_underlay_from_dpa( + txn, + &mut dpa_if, + macaddr, + relay_address, + &api.runtime_config.ntp_servers, + ) + .await } pub async fn discover_dhcp( @@ -394,7 +412,7 @@ pub async fn discover_dhcp( let mut txn = api.txn_begin().await?; - let record: rpc::DhcpRecord = db::dhcp_record::find_by_mac_address( + let mut record: rpc::DhcpRecord = db::dhcp_record::find_by_mac_address( &mut txn, &parsed_mac, &machine_interface.segment_id, @@ -404,5 +422,8 @@ pub async fn discover_dhcp( .into(); txn.commit().await?; + + record.ntp_servers = api.runtime_config.ntp_servers.clone(); + Ok(Response::new(record)) } diff --git a/crates/api/src/handlers/dpu.rs b/crates/api/src/handlers/dpu.rs index c6190fb6dc..03804d0798 100644 --- a/crates/api/src/handlers/dpu.rs +++ b/crates/api/src/handlers/dpu.rs @@ -595,6 +595,7 @@ pub(crate) async fn get_managed_host_network_config_inner( asn, dhcp_servers: api.eth_data.dhcp_servers.clone(), route_servers, + ntp_servers: api.runtime_config.ntp_servers.clone(), // TODO: Automatically add the prefix(es?) from the IPv4 loopback // pool to deny_prefixes. The database stores the pool in an // exploded representation, so we either need to reconstruct the diff --git a/crates/api/src/tests/common/api_fixtures/mod.rs b/crates/api/src/tests/common/api_fixtures/mod.rs index e8874cf286..8cfb21828d 100644 --- a/crates/api/src/tests/common/api_fixtures/mod.rs +++ b/crates/api/src/tests/common/api_fixtures/mod.rs @@ -1138,6 +1138,7 @@ pub fn get_config() -> CarbideConfig { asn: 0, datacenter_asn: 0, dhcp_servers: vec![], + ntp_servers: vec![], route_servers: vec![], enable_route_servers: false, deny_prefixes: vec![], diff --git a/crates/api/src/tests/machine_dhcp.rs b/crates/api/src/tests/machine_dhcp.rs index 2acddc099c..375b0fabf5 100644 --- a/crates/api/src/tests/machine_dhcp.rs +++ b/crates/api/src/tests/machine_dhcp.rs @@ -151,6 +151,51 @@ async fn test_machine_dhcp_with_api(pool: sqlx::PgPool) -> Result<(), Box Result<(), Box> { + let mut config = get_config(); + config.ntp_servers = vec!["198.51.100.10".to_string(), "198.51.100.11".to_string()]; + let env = + create_test_env_with_overrides(pool.clone(), TestEnvOverrides::with_config(config)).await; + + let response = env + .api + .discover_dhcp( + DhcpDiscovery::builder("FF:FF:FF:FF:FF:FF", FIXTURE_DHCP_RELAY_ADDRESS).tonic_request(), + ) + .await? + .into_inner(); + + assert_eq!( + response.ntp_servers, + vec!["198.51.100.10".to_string(), "198.51.100.11".to_string()] + ); + Ok(()) +} + +#[crate::sqlx_test] +async fn test_discover_dhcp_returns_empty_ntp_servers_when_site_not_configured( + pool: sqlx::PgPool, +) -> Result<(), Box> { + let mut config = get_config(); + config.ntp_servers = vec![]; + let env = + create_test_env_with_overrides(pool.clone(), TestEnvOverrides::with_config(config)).await; + + let response = env + .api + .discover_dhcp( + DhcpDiscovery::builder("FF:FF:FF:FF:FF:EE", FIXTURE_DHCP_RELAY_ADDRESS).tonic_request(), + ) + .await? + .into_inner(); + + assert_eq!(response.ntp_servers, Vec::::new()); + Ok(()) +} + #[crate::sqlx_test] async fn test_multiple_machines_dhcp_with_api( pool: sqlx::PgPool, diff --git a/crates/dhcp-server/src/main.rs b/crates/dhcp-server/src/main.rs index ba3a85da44..21630ccb6b 100644 --- a/crates/dhcp-server/src/main.rs +++ b/crates/dhcp-server/src/main.rs @@ -549,6 +549,7 @@ impl Test { gateway: Some("10.217.132.193".to_string()), booturl: None, last_invalidation_time: None, + ntp_servers: vec![], }) } } diff --git a/crates/dhcp-server/src/modes/dpu.rs b/crates/dhcp-server/src/modes/dpu.rs index fa346e642b..2a465927c3 100644 --- a/crates/dhcp-server/src/modes/dpu.rs +++ b/crates/dhcp-server/src/modes/dpu.rs @@ -44,6 +44,7 @@ fn from_host_conf(value: &InterfaceInfo, interface_id: MachineInterfaceId) -> Dh gateway: Some(value.gateway.to_string()), booturl: value.booturl.clone(), last_invalidation_time: None, + ntp_servers: vec![], } } diff --git a/crates/dhcp/src/lib.rs b/crates/dhcp/src/lib.rs index f026a154d1..2e22ed2b8d 100644 --- a/crates/dhcp/src/lib.rs +++ b/crates/dhcp/src/lib.rs @@ -160,7 +160,8 @@ pub unsafe extern "C" fn carbide_set_config_mqtt_server(mqttserver: *const c_cha } } -/// Take the NTP servers for configuring NTP in the dhcp responses +/// Take the NTP servers for DHCP option 42 when the Carbide API `DhcpRecord` has an empty +/// `ntp_servers` list. /// /// # Safety /// Function is unsafe as it dereferences a raw pointer given to it. Caller is responsible diff --git a/crates/dhcp/src/machine.rs b/crates/dhcp/src/machine.rs index 198fd849df..9bfd1ea375 100644 --- a/crates/dhcp/src/machine.rs +++ b/crates/dhcp/src/machine.rs @@ -257,7 +257,15 @@ pub extern "C" fn machine_get_nameservers(ctx: *mut Machine) -> *mut libc::c_cha pub extern "C" fn machine_get_ntpservers(ctx: *mut Machine) -> *mut libc::c_char { assert!(!ctx.is_null()); - let ntpservers = CString::new(CONFIG.read().unwrap().ntpservers.clone()).unwrap(); + let machine = unsafe { &*ctx }; + + let ntp_csv = if !machine.inner.ntp_servers.is_empty() { + machine.inner.ntp_servers.join(",") + } else { + CONFIG.read().unwrap().ntpservers.clone() + }; + + let ntpservers = CString::new(ntp_csv).unwrap(); log::debug!("Ntp servers are {ntpservers:?}"); ntpservers.into_raw() @@ -436,8 +444,9 @@ mod test { use rpc::forge as rpc; + use crate::carbide_set_config_ntp; use crate::discovery::Discovery; - use crate::machine::{Machine, machine_get_filename}; + use crate::machine::{Machine, machine_get_filename, machine_get_ntpservers}; use crate::vendor_class::VendorClass; #[test] @@ -505,4 +514,63 @@ mod test { assert_eq!(cstr, CString::new("https://foobar").unwrap()); } + + #[test] + fn test_machine_get_ntpservers() { + unsafe { + let s = CString::new("10.0.0.1").unwrap(); + carbide_set_config_ntp(s.as_ptr()); + } + + // Test with ntp servers in the dhcp record + let mut machine = Box::new(Machine { + inner: rpc::DhcpRecord { + ntp_servers: vec!["198.51.100.1".to_string(), "198.51.100.2".to_string()], + ..Default::default() + }, + discovery_info: Discovery { + relay_address: "127.0.0.1".parse().unwrap(), + mac_address: "00:00:00:00:00:01".parse().unwrap(), + _client_system: None, + vendor_class: None, + link_select_address: None, + circuit_id: None, + remote_id: None, + desired_address: None, + }, + vendor_class: None, + }); + + let raw = machine_get_ntpservers(&mut *machine); + let cstr = unsafe { CString::from_raw(raw) }; + assert_eq!(cstr.to_str().unwrap(), "198.51.100.1,198.51.100.2"); + + // Test with no ntp servers in the dhcp record + unsafe { + let s = CString::new("10.0.0.2,10.0.0.3").unwrap(); + carbide_set_config_ntp(s.as_ptr()); + } + + let mut machine = Box::new(Machine { + inner: rpc::DhcpRecord { + ntp_servers: vec![], + ..Default::default() + }, + discovery_info: Discovery { + relay_address: "127.0.0.1".parse().unwrap(), + mac_address: "00:00:00:00:00:02".parse().unwrap(), + _client_system: None, + vendor_class: None, + link_select_address: None, + circuit_id: None, + remote_id: None, + desired_address: None, + }, + vendor_class: None, + }); + + let raw = machine_get_ntpservers(&mut *machine); + let cstr = unsafe { CString::from_raw(raw) }; + assert_eq!(cstr.to_str().unwrap(), "10.0.0.2,10.0.0.3"); + } } diff --git a/crates/dhcp/src/mock_api_server.rs b/crates/dhcp/src/mock_api_server.rs index 7664182a82..9c3db2357c 100644 --- a/crates/dhcp/src/mock_api_server.rs +++ b/crates/dhcp/src/mock_api_server.rs @@ -63,6 +63,7 @@ pub fn base_dhcp_response(mac_address: MacAddress) -> rpc::DhcpRecord { gateway: Some("172.20.0.1".to_string()), booturl: None, last_invalidation_time: None, + ntp_servers: vec!["198.51.100.10".to_string(), "198.51.100.11".to_string()], } } diff --git a/crates/rpc/proto/forge.proto b/crates/rpc/proto/forge.proto index 891b25c155..a17a33e470 100644 --- a/crates/rpc/proto/forge.proto +++ b/crates/rpc/proto/forge.proto @@ -3651,6 +3651,9 @@ message DhcpRecord { // The last time any underlay or admin DHCP record managed by Forge got invalidated optional google.protobuf.Timestamp last_invalidation_time = 12; + + // Per site NTP server IPs + repeated string ntp_servers = 13; } message NetworkSegmentList { @@ -3843,6 +3846,10 @@ message ManagedHostNetworkConfigResponse { // for them. bool stateful_acls_enabled = 21; + // site-level configured NTP server IPs + // If empty, the agent may fall back to resolving carbide-ntp.forge for compatibility. + repeated string ntp_servers = 22; + // Enable forge DHCP on HBN. // Deprecated: It is always enabled now. bool enable_dhcp = 100;