From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 234F31FF0AA for ; Tue, 20 Oct 2026 15:09:41 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id C092D21678; Tue, 20 Oct 2026 15:09:38 +0200 (CEST) Message-ID: Date: Tue, 20 Oct 2026 15:09:26 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Stefan Hanreich Subject: Re: [PATCH proxmox-ebpf v3 02/17] dhcp: add the subsystem that loads the responder and keeps its records To: pve-devel@lists.proxmox.com References: <20261020080005.470757-1-h.laimer@proxmox.com> <20261020080005.470757-3-h.laimer@proxmox.com> Content-Language: en-US In-Reply-To: <20261020080005.470757-3-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.414 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: DUG7JCSYTPU5KJLH6I33FSZ7M4T2XMVW X-Message-ID-Hash: DUG7JCSYTPU5KJLH6I33FSZ7M4T2XMVW X-MailFrom: s.hanreich@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 10/20/26 10:03 AM, Hannes Laimer wrote: > The subsystem loads the program, pins a map with one entry per guest > interface and attaches the program to the interfaces it serves. > Callers run a full pass over every guest interface, plug or unplug one > interface, or change the records of a guest's interfaces. > > A load can leave the map empty, and only a full pass is handed every > guest interface. So the full pass is the only step that loads. A plug > or a record change that finds nothing loaded reports that a full pass > has to run first. > > A hotplug moves a tap before the guest config names the new bridge, > and no lock closes that gap. So every entry names the vnet its writer > saw the interface on. A pass or the record change of a mapping edit > leaves an entry naming another vnet alone. A plug writes outright, so > plugs share the lock. The edit's record change reads the entry before > it writes, so it takes the lock exclusively. > > Signed-off-by: Hannes Laimer > --- > src/dhcp/mod.rs | 1031 +++++++++++++++++++++++++++++++++++++++++++++ > src/dhcp/types.rs | 64 +++ > src/lib.rs | 3 + > src/subsystem.rs | 2 +- > 4 files changed, 1099 insertions(+), 1 deletion(-) > create mode 100644 src/dhcp/mod.rs > create mode 100644 src/dhcp/types.rs > > diff --git a/src/dhcp/mod.rs b/src/dhcp/mod.rs > new file mode 100644 > index 0000000..d0c4148 > --- /dev/null > +++ b/src/dhcp/mod.rs > @@ -0,0 +1,1031 @@ > +//! The dhcp subsystem, a per-tap DHCPv4 responder answering from a pinned map of one entry per > +//! interface. > +//! > +//! The map holds nothing of its own. The SDN side hands in every guest interface with the answer > +//! it gets, if any. A full pass makes the kernel match that for the interfaces it names. A record > +//! change or a plug writes its own interfaces only. A linked interface the pass does not name > +//! keeps its link and its entry. A plug runs under the shared lock and writes its entry whatever > +//! is there. A pass runs under the exclusive lock, and so does the record change an edit pushes, > +//! since it reads the entry's vnet before it writes. A guest's own start holds the guest lock, so > +//! nothing else plugs its interfaces meanwhile and it writes its records under the shared lock > +//! without a read. > +//! > +//! A hotplug plugs the tap before the guest config names the new bridge, and no lock orders that. > +//! So every entry names the vnet its writer saw the interface on. A pass or an edit's record change > +//! leaves an entry alone that names another vnet than the configs do, and the link follows such an > +//! entry. A plug replaces it. Plugs of one interface are serialized by the guest lock, so the plug > +//! is the latest word on where its interface sits. An unplug drops the interface's entry. An > +//! interface that went away without a link keeps its entry until the next pass. One plugged onto a > +//! bridge this subsystem does not serve trades its entry for a marker naming that place. > +mod types; > + > +use std::collections::{HashMap, HashSet}; > +use std::io::ErrorKind; > +use std::net::Ipv4Addr; > + > +use anyhow::{Context, bail}; > +use aya::maps::MapError; > + > +use self::types::*; > +use crate::subsystem::{ApplyLock, Exclusive, NotLoaded, Shared, TcPrograms}; > +use crate::tc::{Direction, LinkPin}; > +use crate::tc_programs; > + > +/// One guest interface as the configs know it. `serve` says whether a pass links it. A plug > +/// links its interface regardless. `record` is the answer it gets there, and without one the > +/// interface gets no answer. `vnet` is the vnet it sits on, none for a plain bridge. > +pub struct Iface { > + pub name: String, > + pub vnet: Option, > + pub serve: bool, > + pub record: Option, > +} > + > +/// One interface's answer, the address and every option the reply carries. > +pub struct Record { > + pub ip: Ipv4Addr, > + pub prefixlen: u8, > + pub server_id: Ipv4Addr, > + pub lease: u32, > + pub router: Option, > + pub dns: Option, > + pub mtu: Option, > + pub mac: [u8; 6], proxmox-network-types has a MAC address type that would fit here and in other structs > +} > + > +/// What became of an unplugged interface. > +pub enum Unplugged { > + /// Gone, or going with the unplug. > + Gone, > + /// Plugged onto a bridge this subsystem does not serve, the vnet it sits on now, none for a > + /// plain bridge. > + MovedTo(Option), > +} > + > +/// What a single-entry step did. > +#[derive(Debug, PartialEq, Eq)] > +pub enum Outcome { > + /// Nothing is left to do. The entry is in place, or its interface is not here. > + Done, > + /// Nothing is loaded to write into. Only a full pass loads, so the caller runs one and retries. > + NeedsFullPass, > +} > + > +const RECORDS_MAP: &str = "dhcp_iface_records"; > +const MAPS: [&str; 1] = [RECORDS_MAP]; > + > +// bump with any change to the key or value layout of a map in dhcp.bpf.c > +const SCHEMA_VERSION: u32 = 1; > + > +// requests only arrive from the guest side and replies leave through a redirect, so only ingress > +// carries a program > +const PROGRAM_NAMES: [(Direction, &str); 1] = [(Direction::Ingress, "tc_dhcp_ingress")]; > + > +type RecordsMap = aya::maps::HashMap; > + > +static PROGRAMS: TcPrograms = tc_programs! { > + name: "dhcp", > + programs: &PROGRAM_NAMES, > + maps: &MAPS, > + schema_version: SCHEMA_VERSION, > +}; > + > +pub struct DhcpSubsystem { > + programs: &'static TcPrograms, > +} > + > +impl Default for DhcpSubsystem { > + fn default() -> Self { > + Self::new() > + } > +} > + > +impl DhcpSubsystem { > + pub fn new() -> Self { > + Self { > + programs: &PROGRAMS, > + } > + } > + > + /// The lock a plug runs under, held until [`attach`](SharedLock::attach) returns. Plugs run > + /// next to each other and wait only behind an exclusive holder. > + pub fn lock_shared(&self) -> anyhow::Result { > + Ok(SharedLock { > + lock: self.programs.lock_shared()?, > + }) > + } > + > + /// The lock a full pass or a record change runs under, held until the change is written. > + pub fn lock_exclusive(&self) -> anyhow::Result { > + Ok(ExclusiveLock { > + lock: self.programs.lock_exclusive()?, > + }) > + } > + > + /// A tap unplug. The interface may be gone by then, so its link is found by the name the pin > + /// carries. [`Unplugged`] decides what becomes of the entry. > + pub fn detach(&self, iface: &str, unplugged: Unplugged) -> anyhow::Result<()> { > + // resolved before the lock wait, an interface recreated under the name meanwhile is not > + // this unplug's and keeps its link > + let now = ifindex_of(iface)?; > + // every guest unplug on the host lands here. Most never had a link and must not wait > + // behind a pass. One without a link whose interface is gone leaves its entry to the > + // next pass > + if !self.programs.link_pinned(iface) && now.is_none() { > + return Ok(()); > + } > + let lock = self.programs.lock_shared()?; > + > + if !lock.is_current() { > + log::debug!("dhcp detach: nothing current, {iface} is left to the next full pass"); > + return Ok(()); > + } > + // the map is opened and the marker built before the links go, so a failure there > + // leaves them attached > + let mut map: RecordsMap = lock.hash_map(RECORDS_MAP)?; > + let marker = match &unplugged { > + Unplugged::Gone => None, > + Unplugged::MovedTo(vnet) => Some(DhcpRecord::removed(place(vnet.as_deref())?)), > + }; > + // an interface without a link has an entry as well, a marker where it is not served > + let mut ifindexes = lock.detach_named(iface, |pin| { the name confused me a bit here, maybe detach_named_if or something would be more fitting? > + Some(pin.ifindex) != now && pin_alive(pin).unwrap_or(false) > + })?; > + if let Some(now) = now.filter(|now| !ifindexes.contains(now)) { > + ifindexes.push(now); > + } > + // the marker is the moved interface's. Older incarnations under the name are gone > + unplug_entries(&mut map, &ifindexes, now.zip(marker)) > + } > +} > + > +/// The shared lock of the responder, what a plug holds, and the record change of a guest's own > +/// start. > +pub struct SharedLock { > + lock: ApplyLock<'static, Shared>, > +} > + > +impl SharedLock { > + /// A tap plug, the entry goes in and the program on. The plug loads nothing, so with nothing > + /// pinned the caller runs a full pass first. > + pub fn attach(&self, iface: &Iface) -> anyhow::Result { > + if !self.lock.is_current() { > + return Ok(Outcome::NeedsFullPass); > + } > + let Some(ifindex) = ifindex_of(iface.name.as_str())? else { > + log::info!("dhcp attach: {} is gone, nothing to do", iface.name); > + return Ok(Outcome::Done); > + }; > + // the entry goes in before the program runs, a marker when there is no record or one > + // the program cannot carry. A guest start must not fail on a bad record > + let mut map: RecordsMap = self.lock.hash_map(RECORDS_MAP)?; > + let entry = match iface.entry() { > + Ok(entry) => entry, > + Err(e) => { > + log::warn!( > + "dhcp: {} attaches with nothing to answer: {e:#}", > + iface.name > + ); > + DhcpRecord::removed(iface.place()?) > + } > + }; > + write_entry(&mut map, ifindex, &entry, Elsewhere::Override)?; > + match self.lock.attach_iface(ifindex) { > + Ok(()) => Ok(Outcome::Done), > + Err(e) if e.downcast_ref::().is_some() => Ok(Outcome::NeedsFullPass), > + Err(e) => Err(e).with_context(|| format!("dhcp responder for {}", iface.name)), > + } > + } > + > + /// A record change for a caller that holds the guest lock. Nothing else plugs the guest's > + /// interfaces under that lock, so each entry is written outright, one atomic write, without > + /// reading what it holds. > + pub fn update(&self, ifaces: &[Iface]) -> anyhow::Result { > + update_entries(&self.lock, ifaces, Elsewhere::Override) > + } > +} > + > +/// The exclusive lock of the responder, what a full pass and a record change hold. > +pub struct ExclusiveLock { > + lock: ApplyLock<'static, Exclusive>, > +} > + > +impl ExclusiveLock { > + /// The full pass. Afterward every interface the configs name that exists here has its entry, > + /// a record where it is served with one and a marker otherwise, and a link is on every > + /// served interface and on every interface whose entry names another vnet. Entries of > + /// interfaces that are gone are dropped, as are entries of live interfaces the configs do > + /// not name and that have no link. Bad input leaves its interface as it is, with a warning. > + pub fn apply(&self, ifaces: &[Iface]) -> anyhow::Result<()> { > + self.lock.ensure_loaded()?; > + let present = present_ifindexes(ifaces)?; > + // a link the configs do not name stays while its interface lives under the pin's > + // name. A reused index is not that interface > + let mut unknown_alive = HashSet::new(); > + for pin in self.lock.attached()? { > + if !present.contains_key(&pin.ifindex) && pin_alive(&pin)? { > + unknown_alive.insert(pin.ifindex); > + } > + } > + let mut map: RecordsMap = self.lock.hash_map(RECORDS_MAP)?; > + let (elsewhere, failed) = sync_records(&mut map, &present, &unknown_alive)?; > + // a failed write or removal leaves its entry as it is. The links are still put right > + // and every failure is reported once the rest is done > + let links = self > + .lock > + .reconcile(&wanted_links(&present, &elsewhere, &unknown_alive)); > + match (failed, links) { > + (0, links) => links, > + (failed, Ok(())) => bail!("{failed} entries could not be changed"), > + (failed, Err(e)) => { > + Err(e).with_context(|| format!("{failed} entries could not be changed")) > + } > + } > + } > + > + /// A record change. Interfaces not found here are skipped. The rest are written whether they > + /// have a link yet or not. A record the reply cannot carry leaves its interface as it is. > + /// With nothing loaded the caller runs a full pass first, like a plug. > + pub fn update(&self, ifaces: &[Iface]) -> anyhow::Result { > + update_entries(&self.lock, ifaces, Elsewhere::Skip) > + } > + > + /// Detach everywhere and drop the pinned state, for the last zone leaving the backend. > + pub fn clear(&self) -> anyhow::Result<()> { > + self.lock.clear() > + } > +} > + > +fn update_entries( > + lock: &ApplyLock<'static, M>, > + ifaces: &[Iface], > + elsewhere: Elsewhere, > +) -> anyhow::Result { > + if !lock.is_current() { > + return Ok(Outcome::NeedsFullPass); > + } > + let mut map: RecordsMap = lock.hash_map(RECORDS_MAP)?; > + let mut failed = 0usize; > + for iface in ifaces { > + let ifindex = match ifindex_of(iface.name.as_str()) { > + Ok(Some(ifindex)) => ifindex, > + Ok(None) => continue, > + Err(e) => { > + log::warn!("dhcp: {e:#}"); > + failed += 1; > + continue; > + } > + }; > + let entry = match iface.entry() { > + Ok(entry) => entry, > + Err(e) => { > + log::warn!("dhcp: {} is left as it is: {e:#}", iface.name); > + continue; > + } > + }; > + if let Err(e) = write_entry(&mut map, ifindex, &entry, elsewhere) { > + log::warn!("dhcp: writing the entry of {}: {e:#}", iface.name); > + failed += 1; > + } > + } > + if failed > 0 { > + bail!("{failed} interfaces could not be updated"); > + } > + Ok(Outcome::Done) > +} > + > +const NO_VNET: [u8; 8] = [0; 8]; > + > +// the map update flag that refuses a key the map does not hold, from the kernel's uapi bpf.h > +const BPF_EXIST: u64 = 2; > + > +fn unplug_entries( > + map: &mut impl RecordStore, > + ifindexes: &[u32], > + moved: Option<(u32, DhcpRecord)>, > +) -> anyhow::Result<()> { > + for &ifindex in ifindexes { > + match moved { > + // only an existing entry becomes a marker, so an interface without one gets none > + Some((current, marker)) if current == ifindex => map.mark(ifindex, marker)?, > + _ => map.remove(ifindex)?, > + } > + } > + Ok(()) > +} > + > +// the vnet id padded with zeros, all zero for none > +fn place(vnet: Option<&str>) -> anyhow::Result<[u8; 8]> { > + let Some(vnet) = vnet else { > + return Ok(NO_VNET); > + }; > + let bytes = vnet.as_bytes(); > + if bytes.is_empty() || bytes.len() > NO_VNET.len() { > + bail!("vnet id {vnet:?} does not fit a record"); > + } > + let mut place = NO_VNET; > + place[..bytes.len()].copy_from_slice(bytes); > + Ok(place) > +} > + > +// the record map as the writers see it, so their decisions run against a plain map in tests > +trait RecordStore { > + fn get(&self, ifindex: u32) -> anyhow::Result>; > + fn insert(&mut self, ifindex: u32, rec: DhcpRecord) -> anyhow::Result<()>; > + // replace an entry that exists and do nothing for one that does not, in one step, so it is > + // safe under the shared lock > + fn mark(&mut self, ifindex: u32, rec: DhcpRecord) -> anyhow::Result<()>; > + // a missing entry is removed already > + fn remove(&mut self, ifindex: u32) -> anyhow::Result<()>; > + fn entries(&self) -> anyhow::Result>; > +} > + > +impl RecordStore for RecordsMap { > + fn get(&self, ifindex: u32) -> anyhow::Result> { > + match aya::maps::HashMap::get(self, &ifindex, 0) { > + Ok(live) => Ok(Some(live)), > + Err(MapError::KeyNotFound) => Ok(None), > + Err(e) => Err(e.into()), > + } > + } > + > + fn insert(&mut self, ifindex: u32, rec: DhcpRecord) -> anyhow::Result<()> { > + aya::maps::HashMap::insert(self, ifindex, rec, 0)?; > + Ok(()) > + } > + > + fn mark(&mut self, ifindex: u32, rec: DhcpRecord) -> anyhow::Result<()> { > + match aya::maps::HashMap::insert(self, ifindex, rec, BPF_EXIST) { > + Ok(()) => Ok(()), > + // the kernel answers a missing key with ENOENT, and aya passes that through as is > + Err(MapError::SyscallError(e)) if e.io_error.kind() == ErrorKind::NotFound => Ok(()), > + Err(e) => Err(e.into()), > + } > + } > + > + fn remove(&mut self, ifindex: u32) -> anyhow::Result<()> { > + match aya::maps::HashMap::remove(self, &ifindex) { > + Ok(()) => Ok(()), > + // the kernel answers a missing key with ENOENT, and aya passes that through as is > + Err(MapError::SyscallError(e)) if e.io_error.kind() == ErrorKind::NotFound => Ok(()), > + Err(e) => Err(e.into()), > + } > + } > + > + // a record the listing cannot read is one the pass cannot compare against, so the pass > + // fails rather than writes past it > + fn entries(&self) -> anyhow::Result> { > + self.iter() > + .collect::, _>>() > + .context("listing the records") > + } > +} > + > +// what a write does with an entry naming another vnet than its own > +#[derive(Clone, Copy)] > +enum Elsewhere { > + // a record change, its configs are stale for the interface > + Skip, > + // a plug, or a record change under the guest lock, both see where the interface sits now > + Override, > +} > + > +fn write_entry( > + map: &mut impl RecordStore, > + ifindex: u32, > + rec: &DhcpRecord, > + elsewhere: Elsewhere, > +) -> anyhow::Result<()> { > + if matches!(elsewhere, Elsewhere::Skip) > + && let Some(live) = map.get(ifindex)? > + && live.vnet != rec.vnet > + { > + log::debug!("dhcp: ifindex {ifindex} sits elsewhere than the configs say, leaving it"); > + return Ok(()); > + } > + map.insert(ifindex, *rec) > +} > + > +// entries of live links the pass does not know stay, since their plug wrote them. The other > +// unknown ones go with their interface. Returns the interfaces whose entry names another vnet > +// than the configs, and how many writes failed > +fn sync_records( > + map: &mut impl RecordStore, > + present: &HashMap, > + unknown_alive: &HashSet, > +) -> anyhow::Result<(HashSet, usize)> { > + let live: HashMap = map.entries()?.into_iter().collect(); > + let dead: Vec = live > + .keys() > + .filter(|ifindex| !present.contains_key(ifindex) && !unknown_alive.contains(ifindex)) > + .copied() > + .collect(); > + let mut failed = 0usize; > + for &ifindex in &dead { > + if let Err(e) = map.remove(ifindex) { > + log::warn!("dhcp: removing the record of ifindex {ifindex}: {e:#}"); error message is missing an error or some other prefix? > + failed += 1; > + } > + } > + let mut written = 0usize; > + let mut elsewhere = HashSet::new(); > + > + for (ifindex, iface) in present { > + // bad input leaves its interface as it is and the pass goes on > + let desired = match iface.entry() { > + Ok(desired) => desired, > + Err(e) => { > + log::warn!("dhcp: {} is left as it is: {e:#}", iface.name); > + continue; > + } > + }; > + match live.get(ifindex) { > + Some(entry) if entry.vnet != desired.vnet => { > + elsewhere.insert(*ifindex); > + } > + > + Some(entry) if *entry == desired => {} > + _ => match map.insert(*ifindex, desired) { > + Ok(()) => written += 1, > + Err(e) => { > + log::warn!("dhcp: writing the entry of {}: {e:#}", iface.name); > + failed += 1; > + } > + }, > + } > + } > + > + if written + dead.len() + elsewhere.len() > 0 { > + log::info!( > + "dhcp: {written} entries written, {} removed, {} left where their plug put them", > + dead.len(), > + elsewhere.len() > + ); > + } else { > + log::debug!("dhcp: {} records, no changes", live.len()); > + } > + Ok((elsewhere, failed)) > +} > + > +// the interface's index here, none when it does not exist. Any other failure says nothing about > +// the interface, and a step taking it for gone would detach what it could not resolve > +fn ifindex_of(name: &str) -> anyhow::Result> { > + match nix::net::if_::if_nametoindex(name) { > + Ok(ifindex) => Ok(Some(ifindex)), > + Err(nix::errno::Errno::ENODEV) => Ok(None), > + Err(e) => Err(e).with_context(|| format!("resolve interface {name}")), > + } > +} > + > +// the named interfaces that exist here by index, the names come from configs cluster-wide and > +// most belong to other nodes > +fn present_ifindexes(ifaces: &[Iface]) -> anyhow::Result> { > + let mut present = HashMap::new(); > + for iface in ifaces { > + if let Some(ifindex) = ifindex_of(iface.name.as_str())? { > + present.insert(ifindex, iface); > + } > + } > + Ok(present) > +} Could we use if_nameindex here instead of querying if_nametoindex for every interface? > +// the link follows the configs where they agree with the entry. An entry naming another vnet > +// keeps its link, and a served interface without a mapping gets one. So a record change filling > +// a marker attaches nothing itself. A live link the configs do not know stays > +fn wanted_links( > + present: &HashMap, > + elsewhere: &HashSet, > + unknown_alive: &HashSet, > +) -> HashSet { > + let mut wanted: HashSet = present > + .iter() > + .filter(|(ifindex, iface)| elsewhere.contains(ifindex) || iface.serve) > + .map(|(ifindex, _)| *ifindex) > + .collect(); > + > + wanted.extend(unknown_alive); > + wanted > +} > + > +// whether the link's interface still exists under the name and index its pin carries > +fn pin_alive(pin: &LinkPin) -> anyhow::Result { > + Ok(ifindex_of(pin.name.as_str())? == Some(pin.ifindex)) > +} > + > +impl Iface { > + // a marker where nothing is served > + fn entry(&self) -> anyhow::Result { > + let vnet = self.place()?; > + match &self.record { > + Some(record) if self.serve => record > + .entry(vnet) > + .with_context(|| format!("record for {}", self.name)), > + _ => Ok(DhcpRecord::removed(vnet)), > + } > + } > + > + fn place(&self) -> anyhow::Result<[u8; 8]> { > + place(self.vnet.as_deref()).with_context(|| format!("interface {}", self.name)) > + } > +} > + > +impl Record { > + // refused here is what the reply cannot carry. Zero stands for absent in the entry, so an > + // address or an MTU of zero cannot be told from none > + fn entry(&self, vnet: [u8; 8]) -> anyhow::Result { > + if self.prefixlen == 0 || self.prefixlen > 32 { > + bail!("prefixlen {} out of range", self.prefixlen); > + } > + if self.ip.is_unspecified() { > + bail!("no address"); > + } > + if self.server_id.is_unspecified() { > + bail!("no server address"); > + } > + if self.lease == 0 { > + bail!("lease of zero seconds"); > + } > + if self.mtu.is_some_and(|mtu| mtu < 68) { > + bail!("mtu below the IPv4 minimum of 68"); > + } > + if self.mac == [0; 6] { > + bail!("no MAC"); > + } > + let netmask = u32::MAX << (32 - self.prefixlen); > + Ok(DhcpRecord { > + ip: u32::from(self.ip).to_be(), > + netmask: netmask.to_be(), > + router: u32::from(self.router.unwrap_or(Ipv4Addr::UNSPECIFIED)).to_be(), > + dns: u32::from(self.dns.unwrap_or(Ipv4Addr::UNSPECIFIED)).to_be(), > + server_id: u32::from(self.server_id).to_be(), > + lease: self.lease.to_be(), > + mtu: self.mtu.unwrap_or(0).to_be(), > + vnet, > + mac: self.mac, > + }) > + } > +} > + > +#[cfg(test)] > +mod test { > + use std::collections::BTreeMap; > + > + use super::*; > + > + fn record() -> Record { > + Record { > + ip: Ipv4Addr::new(10, 0, 0, 100), > + prefixlen: 24, > + server_id: Ipv4Addr::new(10, 0, 0, 1), > + lease: 300, > + router: Some(Ipv4Addr::new(10, 0, 0, 1)), > + dns: Some(Ipv4Addr::new(1, 1, 1, 1)), > + mtu: Some(1500), > + mac: MAC, > + } > + } > + > + const MAC: [u8; 6] = [0xda, 0x65, 0x8f, 0x18, 0x9b, 0x6f]; > + > + const VNET: [u8; 8] = *b"vnet1\0\0\0"; > + > + #[test] > + fn converts_full_record() { > + let rec = record().entry(VNET).unwrap(); > + assert_eq!( > + u32::from_be(rec.ip), > + u32::from(Ipv4Addr::new(10, 0, 0, 100)) > + ); > + assert_eq!(u32::from_be(rec.netmask), 0xffffff00); > + assert_eq!( > + u32::from_be(rec.router), > + u32::from(Ipv4Addr::new(10, 0, 0, 1)) > + ); > + assert_eq!(u32::from_be(rec.dns), u32::from(Ipv4Addr::new(1, 1, 1, 1))); > + assert_eq!( > + u32::from_be(rec.server_id), > + u32::from(Ipv4Addr::new(10, 0, 0, 1)) > + ); > + assert_eq!(u32::from_be(rec.lease), 300); > + assert_eq!(u16::from_be(rec.mtu), 1500); > + assert_eq!(rec.vnet, VNET); > + assert_eq!(rec.mac, MAC); > + > + let rec = Record { > + router: None, > + dns: None, > + mtu: None, > + ..record() > + } > + .entry(VNET) > + .unwrap(); > + assert_eq!(rec.router, 0); > + assert_eq!(rec.dns, 0); > + assert_eq!(rec.mtu, 0); > + } > + > + #[test] > + fn entries_name_the_vnet_of_the_interface() { > + let iface = Iface { > + name: "tap100i0".into(), > + vnet: Some("testvnet".into()), > + serve: true, > + record: Some(record()), > + }; > + assert_eq!(&iface.entry().unwrap().vnet, b"testvnet"); > + let iface = Iface { > + name: "tap100i0".into(), > + vnet: None, > + serve: false, > + record: None, > + }; > + assert_eq!(iface.entry().unwrap().vnet, NO_VNET); > + let iface = Iface { > + name: "tap100i0".into(), > + vnet: Some("ninechars".into()), > + serve: true, > + record: Some(record()), > + }; > + assert!(iface.entry().is_err()); > + let iface = Iface { > + name: "tap100i0".into(), > + vnet: Some(String::new()), > + serve: true, > + record: Some(record()), > + }; > + assert!(iface.entry().is_err()); > + } > + > + #[test] > + fn unserved_interfaces_get_a_marker() { > + let iface = Iface { > + name: "tap100i0".into(), > + vnet: Some("vnet1".into()), > + serve: false, > + record: Some(record()), > + }; > + assert_eq!(iface.entry().unwrap().ip, 0); > + assert_eq!(iface.entry().unwrap().vnet, VNET); > + let iface = Iface { > + name: "tap100i0".into(), > + vnet: Some("vnet1".into()), > + serve: true, > + record: None, > + }; > + assert_eq!(iface.entry().unwrap().ip, 0); > + let iface = Iface { > + name: "tap100i0".into(), > + vnet: Some("vnet1".into()), > + serve: true, > + record: Some(record()), > + }; > + assert_ne!(iface.entry().unwrap().ip, 0); > + } > + > + #[test] > + fn refuses_what_the_reply_cannot_carry() { > + for rec in [ > + Record { > + prefixlen: 33, > + ..record() > + }, > + Record { > + prefixlen: 0, > + ..record() > + }, > + Record { > + server_id: Ipv4Addr::UNSPECIFIED, > + ..record() > + }, > + Record { > + lease: 0, > + ..record() > + }, > + Record { > + ip: Ipv4Addr::UNSPECIFIED, > + ..record() > + }, > + Record { > + mtu: Some(67), > + ..record() > + }, > + Record { > + mac: [0; 6], > + ..record() > + }, > + ] { > + assert!(rec.entry(VNET).is_err()); > + } > + let rec = Record { > + mtu: Some(68), > + ..record() > + }; > + assert_eq!(u16::from_be(rec.entry(VNET).unwrap().mtu), 68); > + > + let rec = Record { > + prefixlen: 32, > + ..record() > + }; > + assert_eq!(u32::from_be(rec.entry(VNET).unwrap().netmask), u32::MAX); > + let rec = Record { > + prefixlen: 1, > + ..rec > + }; > + assert_eq!(u32::from_be(rec.entry(VNET).unwrap().netmask), 1 << 31); > + } > + > + impl RecordStore for BTreeMap { > + fn get(&self, ifindex: u32) -> anyhow::Result> { > + Ok(BTreeMap::get(self, &ifindex).copied()) > + } > + > + fn insert(&mut self, ifindex: u32, rec: DhcpRecord) -> anyhow::Result<()> { > + BTreeMap::insert(self, ifindex, rec); > + Ok(()) > + } > + > + fn mark(&mut self, ifindex: u32, rec: DhcpRecord) -> anyhow::Result<()> { > + if let Some(entry) = BTreeMap::get_mut(self, &ifindex) { > + *entry = rec; > + } > + Ok(()) > + } > + > + fn remove(&mut self, ifindex: u32) -> anyhow::Result<()> { > + BTreeMap::remove(self, &ifindex); > + Ok(()) > + } > + > + fn entries(&self) -> anyhow::Result> { > + Ok(self.iter().map(|(k, v)| (*k, *v)).collect()) > + } > + } > + > + const OTHER: [u8; 8] = *b"vnet2\0\0\0"; > + > + fn entry(vnet: [u8; 8]) -> DhcpRecord { > + record().entry(vnet).unwrap() > + } > + > + fn other_answer(vnet: [u8; 8]) -> DhcpRecord { > + Record { > + ip: Ipv4Addr::new(10, 0, 0, 200), > + ..record() > + } > + .entry(vnet) > + .unwrap() > + } > + > + #[test] > + fn a_record_change_writes_on_its_own_vnet() { > + let mut map = BTreeMap::new(); > + write_entry(&mut map, 1, &entry(VNET), Elsewhere::Skip).unwrap(); > + assert_eq!(map[&1], entry(VNET)); > + write_entry(&mut map, 1, &other_answer(VNET), Elsewhere::Skip).unwrap(); > + assert_eq!(map[&1], other_answer(VNET)); > + // an entry naming another vnet was placed by a plug and stays > + map.insert(1, entry(OTHER)); > + write_entry(&mut map, 1, &other_answer(VNET), Elsewhere::Skip).unwrap(); > + assert_eq!(map[&1], entry(OTHER)); > + } > + > + #[test] > + fn a_plug_writes_whatever_is_there() { > + let mut map = BTreeMap::new(); > + write_entry(&mut map, 1, &entry(VNET), Elsewhere::Override).unwrap(); > + assert_eq!(map[&1], entry(VNET)); > + map.insert(1, entry(OTHER)); > + write_entry(&mut map, 1, &other_answer(VNET), Elsewhere::Override).unwrap(); > + assert_eq!(map[&1], other_answer(VNET)); > + } > + > + #[test] > + fn an_unplug_removes_or_marks_the_entries() { > + let mut map = BTreeMap::new(); > + map.insert(1, entry(VNET)); > + map.insert(2, entry(VNET)); > + map.insert(3, entry(VNET)); > + unplug_entries(&mut map, &[1, 2], None).unwrap(); > + assert_eq!(map.keys().copied().collect::>(), [3]); > + // a missing entry is removed already > + unplug_entries(&mut map, &[1], None).unwrap(); > + // a move marks the interface that moved, and an older incarnation under its name is gone > + map.insert(4, entry(VNET)); > + let marker = DhcpRecord::removed(NO_VNET); > + unplug_entries(&mut map, &[3, 4], Some((4, marker))).unwrap(); > + assert_eq!(map.keys().copied().collect::>(), [4]); > + assert_eq!(map[&4].ip, 0); > + assert_eq!(map[&4].vnet, NO_VNET); > + // an interface without an entry gets no marker > + unplug_entries(&mut map, &[5], Some((5, marker))).unwrap(); > + assert!(!map.contains_key(&5)); > + } > + > + fn iface(name: &str, vnet: Option<&str>, serve: bool, record: Option) -> Iface { > + Iface { > + name: name.into(), > + vnet: vnet.map(String::from), > + serve, > + record, > + } > + } > + > + #[test] > + fn the_pass_syncs_the_records_it_knows() { > + let mut map = BTreeMap::new(); > + map.insert(1, entry(VNET)); // same answer as the config > + map.insert(2, other_answer(VNET)); // a changed answer > + map.insert(4, entry(VNET)); // not served anymore > + map.insert(6, entry(VNET)); // its interface is gone > + map.insert(7, entry(VNET)); // a live link the configs do not name > + map.insert(8, entry(VNET)); // its mapping is gone > + let ifaces = [ > + iface("a", Some("vnet1"), true, Some(record())), > + iface("b", Some("vnet1"), true, Some(record())), > + iface("d", Some("vnet1"), false, None), > + iface("e", Some("vnet1"), true, Some(record())), // no entry yet > + iface("f", Some("vnet1"), true, None), // no entry, served, no mapping > + iface("g", Some("vnet1"), true, None), > + ]; > + let present: HashMap = > + [1, 2, 4, 5, 9, 8].into_iter().zip(ifaces.iter()).collect(); > + let unknown_alive = HashSet::from([7]); > + let (elsewhere, failed) = sync_records(&mut map, &present, &unknown_alive).unwrap(); > + > + assert!(elsewhere.is_empty()); > + assert_eq!(failed, 0); > + assert_eq!(map[&1], entry(VNET)); > + assert_eq!(map[&2], entry(VNET), "a changed answer is rewritten"); > + assert_eq!( > + map[&4].ip, 0, > + "an interface not served anymore keeps a marker" > + ); > + assert_eq!(map[&5], entry(VNET), "a new interface gets its entry"); > + assert!(!map.contains_key(&6), "a gone interface loses its entry"); > + assert_eq!( > + map[&9].ip, 0, > + "a served interface without a mapping gets a marker" > + ); > + assert_eq!(map[&8].ip, 0, "a mapping that is gone leaves a marker"); > + assert!( > + map.contains_key(&7), > + "a live link the configs do not name keeps its entry" > + ); > + } > + > + #[test] > + fn the_links_follow_the_configs_or_the_entry() { > + let ifaces = [ > + iface("a", Some("vnet1"), true, Some(record())), // served, entry agrees > + iface("b", Some("vnet1"), false, None), // not served > + iface("c", Some("vnet1"), false, None), // config stale, entry has a record > + iface("d", Some("vnet1"), true, Some(record())), // config stale, entry is a marker > + ]; > + let present: HashMap = (1..=4).zip(ifaces.iter()).collect(); > + let elsewhere = HashSet::from([3, 4]); > + let unknown_alive = HashSet::from([9]); > + assert_eq!( > + wanted_links(&present, &elsewhere, &unknown_alive), > + HashSet::from([1, 3, 4, 9]) > + ); > + } > + > + // a store that refuses one interface's writes > + struct Flaky(BTreeMap, u32); > + > + impl RecordStore for Flaky { > + fn get(&self, ifindex: u32) -> anyhow::Result> { > + RecordStore::get(&self.0, ifindex) > + } > + > + fn insert(&mut self, ifindex: u32, rec: DhcpRecord) -> anyhow::Result<()> { > + if ifindex == self.1 { > + bail!("map full"); > + } > + RecordStore::insert(&mut self.0, ifindex, rec) > + } > + > + fn mark(&mut self, ifindex: u32, rec: DhcpRecord) -> anyhow::Result<()> { > + RecordStore::mark(&mut self.0, ifindex, rec) > + } > + > + fn remove(&mut self, ifindex: u32) -> anyhow::Result<()> { > + if ifindex == self.1 { > + bail!("map busy"); > + } > + RecordStore::remove(&mut self.0, ifindex) > + } > + > + fn entries(&self) -> anyhow::Result> { > + RecordStore::entries(&self.0) > + } > + } > + > + #[test] > + fn bad_input_leaves_its_interface_alone() { > + let mut map = BTreeMap::new(); > + map.insert(1, entry(VNET)); > + let bad = Record { > + lease: 0, > + ..record() > + }; > + let ifaces = [ > + iface("a", Some("vnet1"), true, Some(bad)), > + iface("b", Some("vnet1"), true, Some(record())), > + ]; > + let present: HashMap = (1..=2).zip(ifaces.iter()).collect(); > + let (elsewhere, failed) = sync_records(&mut map, &present, &HashSet::new()).unwrap(); > + assert!(elsewhere.is_empty()); > + assert_eq!(failed, 0); > + assert_eq!(map[&1], entry(VNET), "the old entry stays"); > + assert_eq!(map[&2], entry(VNET), "the rest is written"); > + } > + > + #[test] > + fn a_failed_write_does_not_stop_the_pass() { > + let mut map = Flaky(BTreeMap::new(), 2); > + let ifaces = [ > + iface("a", Some("vnet1"), true, Some(record())), > + iface("b", Some("vnet1"), true, Some(record())), > + iface("c", Some("vnet1"), true, Some(record())), > + ]; > + let present: HashMap = (1..=3).zip(ifaces.iter()).collect(); > + let (elsewhere, failed) = sync_records(&mut map, &present, &HashSet::new()).unwrap(); > + assert!(elsewhere.is_empty()); > + assert_eq!(failed, 1); > + assert!(map.0.contains_key(&1)); > + assert!(!map.0.contains_key(&2)); > + assert!(map.0.contains_key(&3)); > + } > + > + // a store whose listing fails > + struct Unlistable(BTreeMap); > + > + impl RecordStore for Unlistable { > + fn get(&self, ifindex: u32) -> anyhow::Result> { > + RecordStore::get(&self.0, ifindex) > + } > + > + fn insert(&mut self, ifindex: u32, rec: DhcpRecord) -> anyhow::Result<()> { > + RecordStore::insert(&mut self.0, ifindex, rec) > + } > + > + fn mark(&mut self, ifindex: u32, rec: DhcpRecord) -> anyhow::Result<()> { > + RecordStore::mark(&mut self.0, ifindex, rec) > + } > + > + fn remove(&mut self, ifindex: u32) -> anyhow::Result<()> { > + RecordStore::remove(&mut self.0, ifindex) > + } > + > + fn entries(&self) -> anyhow::Result> { > + bail!("map gone") > + } > + } > + > + #[test] > + fn a_failed_listing_fails_the_pass_before_it_writes() { > + let mut map = Unlistable(BTreeMap::new()); > + map.0.insert(1, entry(OTHER)); > + let ifaces = [iface("a", Some("vnet1"), true, Some(record()))]; > + let present: HashMap = [(1, &ifaces[0])].into_iter().collect(); > + assert!(sync_records(&mut map, &present, &HashSet::new()).is_err()); > + assert_eq!(map.0[&1].vnet, OTHER); > + } > + > + #[test] > + fn a_failed_removal_counts_like_a_failed_write() { > + let mut map = Flaky(BTreeMap::new(), 2); > + map.0.insert(2, entry(VNET)); > + let ifaces = [iface("a", Some("vnet1"), true, Some(record()))]; > + let present: HashMap = [(1, &ifaces[0])].into_iter().collect(); > + let (_, failed) = sync_records(&mut map, &present, &HashSet::new()).unwrap(); > + assert_eq!(failed, 1); > + assert!(map.0.contains_key(&1)); > + assert!(map.0.contains_key(&2)); > + } > + > + #[test] > + fn the_pass_leaves_entries_of_another_vnet_to_their_link() { > + let mut map = BTreeMap::new(); > + map.insert(1, entry(OTHER)); // a plug the configs trail, with a record > + map.insert(2, DhcpRecord::removed(NO_VNET)); // moved to a plain bridge > + map.insert(3, DhcpRecord::removed(NO_VNET)); // the configs caught up with that > + let ifaces = [ > + iface("a", Some("vnet1"), true, Some(record())), > + iface("b", Some("vnet1"), true, Some(record())), > + iface("c", None, false, None), > + ]; > + let present: HashMap = (1..=3).zip(ifaces.iter()).collect(); > + let (elsewhere, _) = sync_records(&mut map, &present, &HashSet::new()).unwrap(); > + > + assert_eq!(elsewhere, HashSet::from([1, 2])); > + > + assert_eq!(map[&1].vnet, OTHER); > + assert_eq!(map[&2].vnet, NO_VNET); > + assert_eq!( > + map[&3].ip, 0, > + "a marker the configs agree with stays as it is" > + ); > + } > +} > diff --git a/src/dhcp/types.rs b/src/dhcp/types.rs > new file mode 100644 > index 0000000..71a1b94 > --- /dev/null > +++ b/src/dhcp/types.rs > @@ -0,0 +1,64 @@ > +//! Keep in sync with 'bpf/types.h'. > + > +/// Addresses, lease and mtu are stored in network byte order, so a reply copies them into the > +/// packet as-is. > +#[repr(C)] > +#[derive(Copy, Clone, Debug, PartialEq, Eq)] > +pub struct DhcpRecord { > + /// 0 = removed, the entry is a marker naming its vnet > + pub ip: u32, > + pub netmask: u32, > + /// 0 = not served > + pub router: u32, > + /// 0 = not served > + pub dns: u32, > + pub server_id: u32, > + pub lease: u32, > + /// 0 = not served > + pub mtu: u16, > + /// The vnet the writer saw the interface on, the id padded with zeros, all zero for none. > + /// Never read by the program. > + pub vnet: [u8; 8], > + /// The NIC's MAC, a request carrying another is passed on. Zero in a marker. > + pub mac: [u8; 6], > +} > + > +impl DhcpRecord { > + /// The marker a removal leaves behind, on the vnet the writer saw the interface on. > + pub fn removed(vnet: [u8; 8]) -> Self { > + Self { > + ip: 0, > + netmask: 0, > + router: 0, > + dns: 0, > + server_id: 0, > + lease: 0, > + mtu: 0, > + vnet, > + mac: [0; 6], > + } > + } > +} > + > +unsafe impl aya::Pod for DhcpRecord {} > + > +#[cfg(test)] > +mod layout { > + use core::mem::{offset_of, size_of}; > + > + use super::*; > + > + #[test] > + fn matches_bpf_abi() { > + assert_eq!(size_of::(), 40); > + assert_eq!(offset_of!(DhcpRecord, ip), 0); > + assert_eq!(offset_of!(DhcpRecord, netmask), 4); > + assert_eq!(offset_of!(DhcpRecord, router), 8); > + assert_eq!(offset_of!(DhcpRecord, dns), 12); > + assert_eq!(offset_of!(DhcpRecord, server_id), 16); > + assert_eq!(offset_of!(DhcpRecord, lease), 20); > + assert_eq!(offset_of!(DhcpRecord, mtu), 24); > + assert_eq!(offset_of!(DhcpRecord, vnet), 26); > + assert_eq!(offset_of!(DhcpRecord, mac), 34); > + } > +} > diff --git a/src/lib.rs b/src/lib.rs > index 572d861..a8c94ba 100644 > --- a/src/lib.rs > +++ b/src/lib.rs > @@ -1,5 +1,8 @@ > //! eBPF subsystems, one per concern, each behind a cargo feature of its name. Nothing here runs > //! on its own, consumers pull in only the subsystem they drive. > > +#[cfg(feature = "dhcp")] > +pub mod dhcp; > + > pub mod subsystem; > pub mod tc; > diff --git a/src/subsystem.rs b/src/subsystem.rs > index 3bf1634..6014dbe 100644 > --- a/src/subsystem.rs > +++ b/src/subsystem.rs > @@ -42,7 +42,7 @@ const RUN_ROOT: &str = "/run/proxmox-ebpf"; > > /// The subsystems in the order their programs run on an interface, the first runs first. A name > /// missing from the list fails the build. > -pub const RUN_ORDER: &[&str] = &[]; > +pub const RUN_ORDER: &[&str] = &["dhcp"]; > > const fn rank_of(name: &str) -> usize { > let mut i = 0;