feat: site-level ntp_servers config for dhcp server

crates/agent/src/ethernet_virtualization.rs

@@ -137,6 +137,52 @@ pub struct ServiceAddresses {
     pub nameservers: Vec<IpAddr>,
 }
 
+fn build_dhcp_ntp_servers(
+    nc: &rpc::ManagedHostNetworkConfigResponse,
+    service_addrs: &ServiceAddresses,
+) -> Vec<Ipv4Addr> {
+    // 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::<Vec<Ipv4Addr>>();
+
+    // If the site has configured NTP servers, use them instead.
+    if !nc.ntp_servers.is_empty() {
+        let site_ntp_servers: Vec<Ipv4Addr> = 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::<Vec<Ipv4Addr>>();
 
-    let ntpservers_v4 = service_addrs
-        .ntpservers
-        .iter()
-        .filter_map(|x| match x {
-            IpAddr::V4(x) => Some(*x),
-            _ => None,
-        })
-        .collect::<Vec<Ipv4Addr>>();
+    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::<Vec<Ipv4Addr>>();
 
-    let ntpservers_v4 = service_addrs
-        .ntpservers
-        .iter()
-        .filter_map(|x| match x {
-            IpAddr::V4(x) => Some(*x),
-            _ => None,
-        })
-        .collect::<Vec<Ipv4Addr>>();
+    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<dyn std::error::Error>> {
         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(),

crates/agent/src/tests/full.rs

@@ -868,6 +868,7 @@ async fn handle_netconf(AxumState(state): AxumState<Arc<Mutex<State>>>) -> impl
         }),
 
         dhcp_servers: vec!["127.0.0.1".to_string()],
+        ntp_servers: vec![],
         vni_device: "".to_string(),
 
         managed_host_config: Some(rpc::forge::ManagedHostNetworkConfig {

crates/api-model/src/rpc_conv/dhcp_record.rs

@@ -32,6 +32,7 @@ impl From<DhcpRecord> 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![],
         }
     }
 }

crates/api/src/cfg/file.rs

@@ -112,6 +112,10 @@ pub struct CarbideConfig {
     #[serde(default)]
     pub dhcp_servers: Vec<String>,
 
+    /// NTP server IP addresses for the site.
+    #[serde(default)]
+    pub ntp_servers: Vec<String>,
+
     /// Route server IP addresses for L2VPN (Ethernet
     /// Virtual) network support on DPUs.
     #[serde(default)]

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<Option<Response<rpc::DhcpRecord>>, 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<Option<Response<rpc::DhcpRecord>>, 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))
 }

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

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![],

crates/api/src/tests/machine_dhcp.rs

@@ -151,6 +151,51 @@ async fn test_machine_dhcp_with_api(pool: sqlx::PgPool) -> Result<(), Box<dyn st
     Ok(())
 }
 
+#[crate::sqlx_test]
+async fn test_discover_dhcp_includes_site_ntp_server_ips(
+    pool: sqlx::PgPool,
+) -> Result<(), Box<dyn std::error::Error>> {
+    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<dyn std::error::Error>> {
+    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::<String>::new());
+    Ok(())
+}
+
 #[crate::sqlx_test]
 async fn test_multiple_machines_dhcp_with_api(
     pool: sqlx::PgPool,

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![],
         })
     }
 }

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![],
     }
 }
 

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