From 82cc4c9f7456838c986fd0e91cdd6c6f0a61ebda Mon Sep 17 00:00:00 2001 From: Evan Cameron Date: Wed, 21 Jan 2026 22:28:21 -0500 Subject: [PATCH 1/3] feat: interface@ip parsed in config --- libs/config/src/lib.rs | 175 ++++++++++++++++++++++++++++++++++-- libs/config/src/v4.rs | 111 ++++++++++++----------- libs/config/src/v6.rs | 2 +- libs/config/src/wire/mod.rs | 67 +++++++++++++- libs/config/src/wire/v6.rs | 4 +- 5 files changed, 294 insertions(+), 65 deletions(-) diff --git a/libs/config/src/lib.rs b/libs/config/src/lib.rs index 535f782..8f4e5b4 100644 --- a/libs/config/src/lib.rs +++ b/libs/config/src/lib.rs @@ -120,7 +120,7 @@ pub fn backup_ivp4_interface(interface: Option<&str>) -> Result { /// Returns: /// - interfaces matching the list supplied that are 'up' and have an IPv4 /// - OR any 'up' interfaces that also have an IPv4 -pub fn v4_find_interfaces(interfaces: Option>) -> Result> { +pub fn v4_find_interfaces(interfaces: Option<&[wire::Interface]>) -> Result> { let found_interfaces = pnet::datalink::interfaces() .into_iter() .filter(|e| e.is_up() && !e.ips.is_empty() && e.ips.iter().any(|i| i.is_ipv4())) @@ -131,7 +131,7 @@ pub fn v4_find_interfaces(interfaces: Option>) -> Result>) -> Result> { +pub fn v6_find_interfaces(interfaces: Option<&[wire::Interface]>) -> Result> { let found_interfaces = pnet::datalink::interfaces() .into_iter() .filter(|e| e.is_up() && !e.ips.is_empty() && e.ips.iter().any(|i| i.is_ipv6())) @@ -141,17 +141,27 @@ pub fn v6_find_interfaces(interfaces: Option>) -> Result, - interfaces: Option>, + interfaces: Option<&[wire::Interface]>, ) -> Result> { Ok(match interfaces { Some(interfaces) => interfaces .iter() - .map( - |interface| match found_interfaces.iter().find(|i| &i.name == interface) { + .map(|interface| { + match found_interfaces.iter().find(|i| { + i.name == interface.name + && interface + .addr + .map(|addr| i.ips.iter().any(|ip| ip.contains(addr))) + .unwrap_or(true) + }) { Some(i) => Ok(i.clone()), - None => bail!("unable to find interface {}", interface), - }, - ) + None => bail!( + "unable to find interface {} with ip {:#?}", + interface.name, + interface.addr + ), + } + }) .collect::, _>>()?, None => found_interfaces, }) @@ -229,3 +239,152 @@ impl PersistIdentifier { Ok(Duid::from(duid_bytes)) } } + +#[cfg(test)] +mod test { + use std::net::IpAddr; + + use dora_core::{pnet::ipnetwork::IpNetwork, prelude::NetworkInterface}; + + use crate::wire; + + fn mock_interface(name: &str, ip_str: &str, prefix: u8) -> NetworkInterface { + let ip = ip_str.parse::().unwrap(); + NetworkInterface { + name: name.to_string(), + description: String::new(), + index: 0, + mac: None, + ips: vec![IpNetwork::new(ip, prefix).unwrap()], + flags: 0, + } + } + + #[test] + fn test_found_or_default() { + let found = vec![mock_interface("eth0", "192.168.1.10", 24)]; + let result = crate::found_or_default(found.clone(), None).unwrap(); + assert!(!result.is_empty()); + + // no IP + let found = vec![mock_interface("eth0", "192.168.1.10", 24)]; + let config = vec![wire::Interface { + name: "eth0".to_string(), + addr: None, + }]; + let result = crate::found_or_default(found, Some(&config)).unwrap(); + assert_eq!(result.len(), 1); + assert_eq!(result[0].name, "eth0"); + + // matching ip + let found = vec![mock_interface("eth0", "192.168.1.10", 24)]; + let config = vec![wire::Interface { + name: "eth0".to_string(), + addr: Some("192.168.1.10".parse().unwrap()), + }]; + let result = crate::found_or_default(found, Some(&config)).unwrap(); + assert_eq!(result.len(), 1); + assert_eq!(result[0].name, "eth0"); + + // System interface has 192.168.1.1/24, config asks for 192.168.1.50 + let found = vec![mock_interface("eth0", "192.168.1.10", 24)]; + let config = vec![wire::Interface { + name: "eth0".to_string(), + addr: Some("192.168.1.50".parse().unwrap()), + }]; + let result = crate::found_or_default(found, Some(&config)).unwrap(); + assert_eq!(result.len(), 1); + assert_eq!(result[0].name, "eth0"); + + // System interface has 192.168.1.10, config asks for 10.0.0.1 + let found = vec![mock_interface("eth0", "192.168.1.10", 24)]; + let config = vec![wire::Interface { + name: "eth0".to_string(), + addr: Some("10.0.0.1".parse().unwrap()), + }]; + let result = crate::found_or_default(found, Some(&config)); + assert!(result.is_err()); + } + + #[test] + fn test_not_found_interface() { + let found = vec![mock_interface("eth0", "192.168.1.10", 24)]; + let config = vec![wire::Interface { + name: "eth0".to_string(), + addr: Some([192, 168, 0, 10].into()), + }]; + let result = crate::found_or_default(found, Some(&config)); + assert!(result.is_err()); + + let found = vec![mock_interface("eth0", "192.168.1.10", 24)]; + let config = vec![wire::Interface { + name: "eth1".to_string(), // Wrong name + addr: None, + }]; + let result = crate::found_or_default(found, Some(&config)); + assert!(result.is_err()); + } + + #[test] + fn test_find_by_name_and_ipv6_in_subnet() { + // System interface has 2001:db8::1/64, config asks for 2001:db8::dead:beef + let found = vec![mock_interface("eth1", "2001:db8::1", 64)]; + let config = vec![wire::Interface { + name: "eth1".to_string(), + addr: Some("2001:db8::dead:beef".parse().unwrap()), + }]; + let result = crate::found_or_default(found, Some(&config)).unwrap(); + assert_eq!(result.len(), 1); + assert_eq!(result[0].name, "eth1"); + } + + #[test] + fn test_fail_on_ipv6_mismatch() { + // System interface has 2001:db8::1, config asks for fd00::1 + let found = vec![mock_interface("eth1", "2001:db8::1", 64)]; + let config = vec![wire::Interface { + name: "eth1".to_string(), + addr: Some("fd00::1".parse().unwrap()), + }]; + let result = crate::found_or_default(found, Some(&config)); + assert!(result.is_err()); + } + + #[test] + fn test_multiple_interfaces_find_by_ip() { + let found = vec![ + mock_interface("eth0", "192.168.1.10", 24), + mock_interface("eth1", "10.0.0.5", 8), + ]; + let config = vec![wire::Interface { + name: "eth1".to_string(), + addr: Some("10.0.0.5".parse().unwrap()), + }]; + let result = crate::found_or_default(found, Some(&config)).unwrap(); + assert_eq!(result.len(), 1); + assert_eq!(result[0].name, "eth1"); + } + + #[test] + fn test_multiple_config_interfaces_selects_all() { + let found = vec![ + mock_interface("eth0", "192.168.1.10", 24), + mock_interface("eth1", "10.0.0.5", 8), + mock_interface("lo", "127.0.0.1", 8), + ]; + let config = vec![ + wire::Interface { + name: "eth0".to_string(), + addr: None, + }, + wire::Interface { + name: "eth1".to_string(), + addr: Some("10.0.0.5".parse().unwrap()), + }, + ]; + let result = crate::found_or_default(found, Some(&config)).unwrap(); + assert_eq!(result.len(), 2); + assert!(result.iter().any(|i| i.name == "eth0")); + assert!(result.iter().any(|i| i.name == "eth1")); + } +} diff --git a/libs/config/src/v4.rs b/libs/config/src/v4.rs index 76728ce..2ac91a8 100644 --- a/libs/config/src/v4.rs +++ b/libs/config/src/v4.rs @@ -53,7 +53,7 @@ pub struct Config { impl TryFrom for Config { type Error = anyhow::Error; fn try_from(cfg: wire::Config) -> Result { - let interfaces = crate::v4_find_interfaces(cfg.interfaces.clone())?; + let interfaces = crate::v4_find_interfaces(cfg.interfaces.as_deref())?; debug!(?interfaces, "using v4 interfaces"); // transform wire::Config into a more optimized format @@ -61,55 +61,8 @@ impl TryFrom for Config { .networks .into_iter() .map(|(subnet, net)| { - let wire::v4::Net { - ranges, - reservations, - ping_check, - probation_period, - authoritative, - server_id, - ping_timeout_ms, - server_name, - file_name, - } = net; - - let ranges = ranges.into_iter().map(|range| range.into()).collect(); - let reserved_macs = reservations - .iter() - .filter_map(|res| match &res.condition { - wire::v4::Condition::Mac(mac) => Some((*mac, res.into())), - _ => None, - }) - .collect(); - let reserved_opts = reservations - .iter() - .filter_map(|res| { - match &res.condition { - wire::v4::Condition::Options(match_opts) => { - // TODO: we only support matching on a single option currently. - // A reservation can match on chaddr OR a single option value. - match match_opts.values.0.iter().next() { - Some((code, opt)) => Some((*code, (opt.clone(), res.into()))), - _ => None, - } - } - _ => None, - } - }) - .collect(); - let network = Network { - server_id, - subnet, - ping_check, - probation_period: Duration::from_secs(probation_period), - ranges, - reserved_macs, - reserved_opts, - authoritative, - ping_timeout_ms: Duration::from_millis(ping_timeout_ms), - server_name, - file_name, - }; + let mut network: Network = net.into(); + network.set_subnet(subnet); // set total addr space for metrics dora_core::metrics::TOTAL_AVAILABLE_ADDRS.set(network.total_addrs() as i64); (subnet, network) @@ -313,6 +266,59 @@ impl Config { } } +impl From for Network { + fn from(net: wire::v4::Net) -> Self { + let wire::v4::Net { + ranges, + reservations, + ping_check, + probation_period, + authoritative, + server_id, + ping_timeout_ms, + server_name, + file_name, + } = net; + + let ranges = ranges.into_iter().map(|range| range.into()).collect(); + let reserved_macs = reservations + .iter() + .filter_map(|res| match &res.condition { + wire::v4::Condition::Mac(mac) => Some((*mac, res.into())), + _ => None, + }) + .collect(); + let reserved_opts = reservations + .iter() + .filter_map(|res| match &res.condition { + wire::v4::Condition::Options(match_opts) => { + // TODO: we only support matching on a single option currently. + // A reservation can match on chaddr OR a single option value. + match match_opts.values.0.iter().next() { + Some((code, opt)) => Some((*code, (opt.clone(), res.into()))), + _ => None, + } + } + _ => None, + }) + .collect(); + + Network { + server_id, + subnet: Default::default(), // This will be set in the calling function + ping_check, + probation_period: Duration::from_secs(probation_period), + ranges, + reserved_macs, + reserved_opts, + authoritative, + ping_timeout_ms: Duration::from_millis(ping_timeout_ms), + server_name, + file_name, + } + } +} + #[derive(Debug, Default, Clone, PartialEq, Eq)] pub struct Network { /// optional server id that will be used when talking with clients on this network @@ -695,11 +701,12 @@ impl FloodThreshold { #[cfg(test)] mod tests { - use dora_core::dhcproto::v4; + use std::net::Ipv4Addr; - use crate::wire::v4::{Options, Opts}; + use dora_core::dhcproto::v4; use super::*; + use crate::wire::v4::{Options, Opts}; pub static SAMPLE_YAML: &str = include_str!("../sample/config.yaml"); pub static V4_JSON: &str = include_str!("../sample/config_v4.json"); diff --git a/libs/config/src/v6.rs b/libs/config/src/v6.rs index 2e6cbe8..996cfab 100644 --- a/libs/config/src/v6.rs +++ b/libs/config/src/v6.rs @@ -264,7 +264,7 @@ impl TryFrom for Config { type Error = anyhow::Error; fn try_from(cfg: wire::v6::Config) -> Result { - let interfaces = crate::v6_find_interfaces(cfg.interfaces)?; + let interfaces = crate::v6_find_interfaces(cfg.interfaces.as_deref())?; // DUID-LLT is the default, will need config options to do others let link_local = interfaces .iter() diff --git a/libs/config/src/wire/mod.rs b/libs/config/src/wire/mod.rs index a07f283..395a8af 100644 --- a/libs/config/src/wire/mod.rs +++ b/libs/config/src/wire/mod.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, num::NonZeroU32, time::Duration}; +use std::{collections::HashMap, net::IpAddr, num::NonZeroU32, time::Duration}; use ipnet::Ipv4Net; use serde::{Deserialize, Serialize}; @@ -12,7 +12,7 @@ pub mod v6; /// top-level config type #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] pub struct Config { - pub interfaces: Option>, + pub interfaces: Option>, #[serde(default = "default_chaddr_only")] pub chaddr_only: bool, pub flood_protection_threshold: Option, @@ -29,6 +29,42 @@ pub struct Config { pub ddns: Option, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(try_from = "String", into = "String")] +pub struct Interface { + pub name: String, + pub addr: Option, +} + +impl TryFrom for Interface { + type Error = anyhow::Error; + + fn try_from(s: String) -> Result { + let mut iter = s.split('@'); + let name = iter + .next() + .ok_or_else(|| anyhow::Error::msg("missing interface"))? + .to_owned(); + if name.is_empty() { + return Err(anyhow::Error::msg("missing interface")); + } + Ok(Self { + name, + addr: iter.next().map(|s| s.parse::()).transpose()?, + }) + } +} + +impl From for String { + fn from(iface: Interface) -> Self { + if let Some(addr) = iface.addr { + format!("{}@{}", iface.name, addr) + } else { + iface.name + } + } +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] pub struct FloodThreshold { pub packets: NonZeroU32, @@ -94,6 +130,7 @@ pub(crate) enum MaybeList { #[cfg(test)] mod tests { + use super::*; pub static EXAMPLE: &str = include_str!("../../../../example.yaml"); @@ -106,4 +143,30 @@ mod tests { let s = serde_yaml::to_string(&cfg).unwrap(); println!("{s}"); } + + #[test] + fn test_interface() { + let iface = Interface { + name: "eth0".to_string(), + addr: Some([192, 168, 1, 1].into()), + }; + + let s = serde_json::to_string(&iface).unwrap(); + assert_eq!(s, "\"eth0@192.168.1.1\""); + + let err = serde_json::from_str::("\"@192.168.1.1\""); + assert!(err.is_err()); + + let json_test: Interface = serde_json::from_str(&s).unwrap(); + assert_eq!(iface, json_test); + + let no_addr = Interface { + name: "lo".to_string(), + addr: None, + }; + let json_no_addr = serde_json::to_string(&no_addr).unwrap(); + assert_eq!(json_no_addr, "\"lo\""); + let test_no_addr: Interface = serde_json::from_str(&json_no_addr).unwrap(); + assert_eq!(no_addr, test_no_addr); + } } diff --git a/libs/config/src/wire/v6.rs b/libs/config/src/wire/v6.rs index 7be6c9e..46faf95 100644 --- a/libs/config/src/wire/v6.rs +++ b/libs/config/src/wire/v6.rs @@ -11,13 +11,13 @@ use std::{collections::HashMap, net::Ipv6Addr, ops::RangeInclusive}; use crate::{ v6::DEFAULT_SERVER_ID_FILE_PATH, - wire::{MaybeList, MinMax}, + wire::{Interface, MaybeList, MinMax}, }; /// top-level config type #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, Default)] pub struct Config { - pub interfaces: Option>, + pub interfaces: Option>, pub server_id: Option, pub networks: HashMap, // TODO: better defaults than blank? pull information from the system From eb835f39790d02510ba01bf0696e74866d2b938b Mon Sep 17 00:00:00 2001 From: Evan Cameron Date: Thu, 22 Jan 2026 21:54:04 -0500 Subject: [PATCH 2/3] add component tests --- bin/tests/basic.rs | 66 +++++++++++++++++++++++++++ bin/tests/common/env.rs | 10 ++++ bin/tests/test_configs/interface.yaml | 22 +++++++++ 3 files changed, 98 insertions(+) create mode 100644 bin/tests/test_configs/interface.yaml diff --git a/bin/tests/basic.rs b/bin/tests/basic.rs index ff26221..e22f76f 100644 --- a/bin/tests/basic.rs +++ b/bin/tests/basic.rs @@ -833,3 +833,69 @@ fn test_cache_threshold() -> Result<()> { drop(_srv); Ok(()) } + +/// send a discover to an ip that is not configured in the `interfaces` list +#[test] +#[traced_test] +fn test_interface_no_match() -> Result<()> { + let _srv = DhcpServerEnv::start( + "interface.yaml", + "interface.db", + "dora_test", + "dhcpcli", + "dhcpsrv", + "192.168.2.1", + ); + // use veth_cli created in start() + let settings = ClientSettingsBuilder::default() + .iface_name("dhcpcli") + .target("192.168.2.2".parse::().unwrap()) + .port(9900_u16) + .build()?; + // create a client that sends dhcpv4 messages + let mut client = Client::::new(settings); + // create DISCOVER msg & send + let msg_args = DiscoverBuilder::default() + .giaddr([192, 168, 2, 1]) + .build()?; + let resp = client.run(MsgType::Discover(msg_args)); + + assert!(resp.is_err()); + + // pedantic drop + drop(_srv); + Ok(()) +} + +/// send a discover to an ip that is configured in the `interfaces` list +#[test] +#[traced_test] +fn test_interface_match() -> Result<()> { + let _srv = DhcpServerEnv::start( + "interface.yaml", + "interface.db", + "dora_test", + "dhcpcli", + "dhcpsrv", + "192.168.2.1", + ); + // use veth_cli created in start() + let settings = ClientSettingsBuilder::default() + .iface_name("dhcpcli") + .target("192.168.2.1".parse::().unwrap()) + .port(9900_u16) + .build()?; + // create a client that sends dhcpv4 messages + let mut client = Client::::new(settings); + // create DISCOVER msg & send + let msg_args = DiscoverBuilder::default() + .giaddr([192, 168, 2, 1]) + .build()?; + let resp = client.run(MsgType::Discover(msg_args))?; + + assert_eq!(resp.opts().msg_type().unwrap(), v4::MessageType::Offer); + + // pedantic drop + drop(_srv); + Ok(()) +} diff --git a/bin/tests/common/env.rs b/bin/tests/common/env.rs index 5aed07f..7c442d0 100644 --- a/bin/tests/common/env.rs +++ b/bin/tests/common/env.rs @@ -23,6 +23,12 @@ impl DhcpServerEnv { veth_srv: &str, srv_ip: &str, ) -> Self { + // Clean up any leftover resources from previous failed tests + // This is necessary because if start() panics before returning Self, + // Drop won't run and resources won't be cleaned up + remove_test_veth_nics(veth_cli); + remove_test_net_namespace(netns); + create_test_net_namespace(netns); create_test_veth_nics(netns, srv_ip, veth_cli, veth_srv); Self { @@ -57,6 +63,8 @@ impl Drop for DhcpServerEnv { const SUDO: &str = "sudo"; fn create_test_net_namespace(netns: &str) { + // Clean up any existing namespace first (from failed test runs) + run_cmd_ignore_failure(&format!("{SUDO} ip netns del {netns}")); run_cmd(&format!("{SUDO} ip netns add {netns}")); } @@ -65,6 +73,8 @@ fn remove_test_net_namespace(netns: &str) { } fn create_test_veth_nics(netns: &str, srv_ip: &str, veth_cli: &str, veth_srv: &str) { + // Clean up any existing veth interfaces first (from failed test runs) + run_cmd_ignore_failure(&format!("{SUDO} ip link del {veth_cli}")); run_cmd(&format!( "{SUDO} ip link add {veth_cli} type veth peer name {veth_srv}", )); diff --git a/bin/tests/test_configs/interface.yaml b/bin/tests/test_configs/interface.yaml new file mode 100644 index 0000000..e8f93f7 --- /dev/null +++ b/bin/tests/test_configs/interface.yaml @@ -0,0 +1,22 @@ +interfaces: + - dhcpsrv@192.168.2.1 +networks: + 192.168.2.0/24: + ranges: + - + start: 192.168.2.100 + end: 192.168.2.150 + config: + lease_time: + default: 3600 + min: 1200 + max: 4800 + options: + values: + 1: + type: ip + value: 192.168.2.1 + 3: + type: ip + value: + - 192.168.2.1 From 532b2ef0bf6189e9a27b8ce734efafb3ade4e377 Mon Sep 17 00:00:00 2001 From: Evan Cameron Date: Fri, 23 Jan 2026 17:45:59 -0500 Subject: [PATCH 3/3] cargo fmt --- libs/config/src/wire/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/config/src/wire/mod.rs b/libs/config/src/wire/mod.rs index 7cf161f..3006026 100644 --- a/libs/config/src/wire/mod.rs +++ b/libs/config/src/wire/mod.rs @@ -247,7 +247,7 @@ mod tests { let test_no_addr: Interface = serde_json::from_str(&json_no_addr).unwrap(); assert_eq!(no_addr, test_no_addr); } - + #[test] fn test_parse_duration() { assert_eq!(parse_duration("3600s").unwrap(), 3600);