* [PATCH proxmox v8 0/6] add proxmox-ebpf library
@ 2026-11-04 17:29 Hannes Laimer
2026-11-04 17:29 ` [PATCH proxmox v8 1/6] ebpf: add the crate with its BPF build Hannes Laimer
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Hannes Laimer @ 2026-11-04 17:29 UTC (permalink / raw)
To: pve-devel
proxmox-ebpf holds the eBPF programs and the code that loads, attaches
and updates them, grouped into subsystems. It is a library only, whoever
needs a subsystem pulls in just that one through a cargo feature and
calls it from their side. This series is the shared part, the actual
subsystems come as their own series on top.
The core is the shared subsystem code. It takes care of everything from
loading and attaching per interface to pinning in bpffs and tearing down
again. So nothing has to keep running, and each call picks up where the
last one left off. The build records a hash of the compiled object, the
crate's version and a schema version per subsystem, so a call can tell
what changed and an older build leaves a newer one's state alone. A
changed program or map schema is swapped onto the existing links
without interrupting traffic. The new maps are filled before the links
move onto them. The BPF C code is also built natively against a small
shim, so the parsing and packet building logic can be tested without a
kernel. Packaging is debcargo like the other Rust crates.
Every change runs under a lock per subsystem, a full apply takes it
exclusively and a single-interface step shared, like the pmxcfs domain
locks. The holder reads the input of its change under the lock and
writes before it lets go. So a change and a full apply reach the kernel
in the order they read their input, and nothing has to order them after
the fact. A holder that never returns makes the waiters time out instead
of hanging them. A subsystem can also hand over the whole content of its
maps. The programs then move onto the new content one link at a time and
never read a mix of old and new entries, and a write the kernel refuses
leaves the old content in place. The programs of different subsystems on
one interface run in a fixed order. An attach places its link relative
to the links the other subsystems already hold there, so the order holds
however the attaches are timed, and an update keeps every link in its
place.
The crate goes into the proxmox workspace, as the first one there with a
build script. Building needs clang-21, libbpf-dev and
proxmox-kernel-libc-dev, the programs compile against the UAPI headers
from there.
v8:
* include map names and directions in program fingerprint in addition
to the object
* the repair after an interrupted replacement only moves links onto
programs the live set has, it does not detach links of a direction
the new build dropped before the new build is in, that broke a
replace that died while staging and then failed its fill
* include pkg version in order file, a newer build will seed the list
and merge the existing one in, and older build will merge its list
into the existing one. this solves the problem of whatever build is
run first seeds the order list at boot, basically making it random
* add missing context to err
v7:
* include rust pkg version next to fingerprint, so we can avoid
flip-flopping the loaded programs in case a worker stays around
during/after an upgrade
* differentiate between a link swap failing on open or on the swap
itself
* replacing the maps and clearing take the lock guard mutably, so a map
ref opened before cannot be written after the exchange, it would hit
the retired map
* the repair after an interrupted replacement also detaches the links
of a direction the update dropped, a retry that failed afterwards
left them attached
* add context to io errors
* merge a run order change, don't blindly replace, this covers cases
where something was added and reordered in one
v6:
* loop all links at the start of every replace, moving links to live/
is done one-by-one, so it can fail. a failed pass then has some links
still on staging/, this makes sure we dont delete state that may
still be attached. the kernel checks if it already exists on a swap,
so this is a no-op, and at worst an atomic swap for links that still
has staging/ attached
* only allow adding to the run order, at least for live updates.
removal or re-orders are applied on the next reboot
* enforce that map refs live as long as the lock they were taken under
* a pin that cannot be opened stays and fails the step instead of
being unlinked, which under a descriptor shortage detached a working
link
* the harness tests run a C program of their own against the shim
* move the crate into the proxmox workspace, the vendored vmlinux.h is
gone, the programs build against the UAPI headers of
proxmox-kernel-libc-dev with a versioned clang
v5:
* add a call that replaces the maps as a whole, it uses more space and
writes everything instead of only what changed, but leaves the old
content in place instead of a mix, in cases where the write fails
with ENOMEM. Subsystems choose which they need
* install/upgrade now also use this newly added replace path, this
simplifies the process and also closes a hole where attached programs
would read empty maps, shortly before they were filled after a schema
version bump
* the run order of the subsystems is written to
`/run/proxmox-ebpf/order` by a full apply and every attach places by
it, so a stale process running an older version attaches correctly
against subsystems added by a process running a newer version
* set correct permissions on `/run/..`
* a build whose schema is older than the pinned one no longer counts
its state as current, so its single steps back off as well
* add `interrupted()` on the lock guard, true when a replacement left
its staged set or an old schema directory behind, a full pass
replaces then
* an attach checks after attaching that the interface still has the
index of its pin, one recreated under the same name in between is
reported as gone
* a map missing from a pinned set is reported by name
v4, thanks @Stefan!:
* drop the generation counter, a change takes the apply lock before it
reads its input instead, so the lock order is the input order
* new, the links of different subsystems on an interface are now
ordered
* the tc link plumbing is its own commit ahead of the subsystem code
* subsystem programs are now a list of direction and name pairs
* add short note for why not proxmox-sys's locking
* improve tc error handling
* fix smaller spacing and code stuff
proxmox:
Hannes Laimer (6):
ebpf: add the crate with its BPF build
ebpf: tc: add the link plumbing
ebpf: subsystem: add the shared tc program handling
ebpf: subsystem: place subsystem links in a fixed order
ebpf: tests: add a native harness for the BPF C programs
ebpf: debian: package the crate as a Rust library
Cargo.toml | 2 +
proxmox-ebpf/Cargo.toml | 22 +
proxmox-ebpf/build.rs | 190 +++
proxmox-ebpf/debian/changelog | 5 +
proxmox-ebpf/debian/control | 50 +
proxmox-ebpf/debian/copyright | 18 +
proxmox-ebpf/debian/debcargo.toml | 13 +
proxmox-ebpf/include/bpf_debug.h | 12 +
proxmox-ebpf/src/bpf-shim/bpf/bpf_helpers.h | 34 +
proxmox-ebpf/src/bpf-shim/bpf_debug.h | 10 +
proxmox-ebpf/src/bpf-shim/linux/bpf.h | 25 +
proxmox-ebpf/src/lib.rs | 6 +
proxmox-ebpf/src/subsystem.rs | 1492 +++++++++++++++++++
proxmox-ebpf/src/tc.rs | 329 ++++
proxmox-ebpf/tests/bpf/fixture.bpf.c | 37 +
proxmox-ebpf/tests/common/mod.rs | 212 +++
proxmox-ebpf/tests/harness.rs | 84 ++
17 files changed, 2541 insertions(+)
create mode 100644 proxmox-ebpf/Cargo.toml
create mode 100644 proxmox-ebpf/build.rs
create mode 100644 proxmox-ebpf/debian/changelog
create mode 100644 proxmox-ebpf/debian/control
create mode 100644 proxmox-ebpf/debian/copyright
create mode 100644 proxmox-ebpf/debian/debcargo.toml
create mode 100644 proxmox-ebpf/include/bpf_debug.h
create mode 100644 proxmox-ebpf/src/bpf-shim/bpf/bpf_helpers.h
create mode 100644 proxmox-ebpf/src/bpf-shim/bpf_debug.h
create mode 100644 proxmox-ebpf/src/bpf-shim/linux/bpf.h
create mode 100644 proxmox-ebpf/src/lib.rs
create mode 100644 proxmox-ebpf/src/subsystem.rs
create mode 100644 proxmox-ebpf/src/tc.rs
create mode 100644 proxmox-ebpf/tests/bpf/fixture.bpf.c
create mode 100644 proxmox-ebpf/tests/common/mod.rs
create mode 100644 proxmox-ebpf/tests/harness.rs
Summary over all repositories:
17 files changed, 2541 insertions(+), 0 deletions(-)
--
Generated by murpp 0.12.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH proxmox v8 1/6] ebpf: add the crate with its BPF build
2026-11-04 17:29 [PATCH proxmox v8 0/6] add proxmox-ebpf library Hannes Laimer
@ 2026-11-04 17:29 ` Hannes Laimer
2026-11-04 17:29 ` [PATCH proxmox v8 2/6] ebpf: tc: add the link plumbing Hannes Laimer
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2026-11-04 17:29 UTC (permalink / raw)
To: pve-devel
Each subsystem is a cargo feature. The build script compiles the C
programs of the enabled ones with clang, against the kernel's UAPI
headers.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
Cargo.toml | 2 +
proxmox-ebpf/Cargo.toml | 21 +++++++
proxmox-ebpf/build.rs | 99 ++++++++++++++++++++++++++++++++
proxmox-ebpf/include/bpf_debug.h | 12 ++++
proxmox-ebpf/src/lib.rs | 3 +
5 files changed, 137 insertions(+)
create mode 100644 proxmox-ebpf/Cargo.toml
create mode 100644 proxmox-ebpf/build.rs
create mode 100644 proxmox-ebpf/include/bpf_debug.h
create mode 100644 proxmox-ebpf/src/lib.rs
diff --git a/Cargo.toml b/Cargo.toml
index 87e43b545793..a98c05810e3a 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -20,6 +20,7 @@ members = [
"proxmox-dns-api",
"proxmox-fixed-string",
"proxmox-docgen",
+ "proxmox-ebpf",
"proxmox-http",
"proxmox-http-error",
"proxmox-human-byte",
@@ -90,6 +91,7 @@ rust-version = "1.88"
# external dependencies
anyhow = "1.0"
+aya = "0.13"
base32 = "0.4"
base64 = "0.22"
bitflags = "2.4"
diff --git a/proxmox-ebpf/Cargo.toml b/proxmox-ebpf/Cargo.toml
new file mode 100644
index 000000000000..4cfd0b3258ed
--- /dev/null
+++ b/proxmox-ebpf/Cargo.toml
@@ -0,0 +1,21 @@
+[package]
+name = "proxmox-ebpf"
+version = "0.1.0"
+authors.workspace = true
+edition.workspace = true
+exclude.workspace = true
+homepage.workspace = true
+license.workspace = true
+repository.workspace = true
+rust-version.workspace = true
+
+[features]
+# When enabled, BPF programs compile with -DBPF_DEBUG so their DBG() macros
+# expand to bpf_printk. Off by default
+bpf-debug = []
+
+[dependencies]
+anyhow.workspace = true
+aya.workspace = true
+log.workspace = true
+nix = { workspace = true, features = ["fs"] }
diff --git a/proxmox-ebpf/build.rs b/proxmox-ebpf/build.rs
new file mode 100644
index 000000000000..9cb143d955a8
--- /dev/null
+++ b/proxmox-ebpf/build.rs
@@ -0,0 +1,99 @@
+use std::env;
+use std::path::{Path, PathBuf};
+use std::process::Command;
+
+// one clang version wherever the crate is built, the package depends on it by name
+const CLANG: &str = "clang-21";
+
+fn main() {
+ let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap());
+ let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap());
+ let src_dir = manifest_dir.join("src");
+ let global_include = manifest_dir.join("include");
+ let bpf_debug = env::var_os("CARGO_FEATURE_BPF_DEBUG").is_some();
+
+ // re-run when a subsystem appears
+ println!("cargo:rerun-if-changed={}", src_dir.display());
+ println!("cargo:rerun-if-changed={}", global_include.display());
+ println!("cargo:rerun-if-changed=build.rs");
+ println!("cargo:rerun-if-env-changed=CARGO_FEATURE_BPF_DEBUG");
+
+ for sub_entry in std::fs::read_dir(&src_dir).expect("read src/") {
+ let sub_path = sub_entry.expect("read src/ entry").path();
+ let bpf_dir = sub_path.join("bpf");
+ if !bpf_dir.is_dir() {
+ continue;
+ }
+ // each subsystem is a cargo feature of the same name, skip disabled ones
+ let feature = sub_path
+ .file_name()
+ .and_then(|s| s.to_str())
+ .expect("subsystem dir name")
+ .to_uppercase()
+ .replace('-', "_");
+ if env::var_os(format!("CARGO_FEATURE_{feature}")).is_none() {
+ continue;
+ }
+ println!("cargo:rerun-if-changed={}", bpf_dir.display());
+
+ for entry in std::fs::read_dir(&bpf_dir).expect("read bpf dir") {
+ let path = entry.expect("read bpf entry").path();
+ let is_program = path
+ .file_name()
+ .and_then(|s| s.to_str())
+ .is_some_and(|s| s.ends_with(".bpf.c"));
+ if !is_program {
+ continue;
+ }
+ compile_bpf(&path, &bpf_dir, &global_include, &out_dir, bpf_debug);
+ }
+ }
+}
+
+fn multiarch() -> String {
+ let out = Command::new("dpkg-architecture")
+ .arg("-qDEB_HOST_MULTIARCH")
+ .output()
+ .expect("failed to invoke dpkg-architecture, is dpkg-dev installed?");
+ assert!(out.status.success(), "dpkg-architecture failed");
+ String::from_utf8(out.stdout).unwrap().trim().to_owned()
+}
+
+fn compile_bpf(
+ src: &Path,
+ local_include: &Path,
+ global_include: &Path,
+ out_dir: &Path,
+ debug: bool,
+) {
+ // foo.bpf.c -> foo.bpf.o (file_stem strips only the final extension)
+ let stem = src.file_stem().unwrap().to_str().unwrap();
+ let obj_path = out_dir.join(format!("{stem}.o"));
+
+ let mut cmd = Command::new(CLANG);
+ // we need -g for clang to emit BTF
+ cmd.args(["-O2", "-g", "-Wall", "-target", "bpf"])
+ .arg("-I")
+ .arg(global_include)
+ .arg("-I")
+ .arg(local_include)
+ // the UAPI headers reach asm/ through the multiarch directory, which clang searches for
+ // the host target only
+ .arg("-I")
+ .arg(format!("/usr/include/{}", multiarch()));
+ if debug {
+ cmd.arg("-DBPF_DEBUG");
+ }
+ let status = cmd
+ .arg("-c")
+ .arg(src)
+ .arg("-o")
+ .arg(&obj_path)
+ .status()
+ .unwrap_or_else(|e| panic!("failed to invoke {CLANG}: {e}"));
+ assert!(
+ status.success(),
+ "clang failed to compile {}",
+ src.display()
+ );
+}
diff --git a/proxmox-ebpf/include/bpf_debug.h b/proxmox-ebpf/include/bpf_debug.h
new file mode 100644
index 000000000000..29bd0b0f7ef9
--- /dev/null
+++ b/proxmox-ebpf/include/bpf_debug.h
@@ -0,0 +1,12 @@
+#ifndef BPF_DEBUG_H
+#define BPF_DEBUG_H
+
+#include <bpf/bpf_helpers.h>
+
+#ifdef BPF_DEBUG
+#define DBG(fmt, ...) bpf_printk(fmt, ##__VA_ARGS__)
+#else
+#define DBG(fmt, ...) ((void)0)
+#endif
+
+#endif
diff --git a/proxmox-ebpf/src/lib.rs b/proxmox-ebpf/src/lib.rs
new file mode 100644
index 000000000000..b19895faaa18
--- /dev/null
+++ b/proxmox-ebpf/src/lib.rs
@@ -0,0 +1,3 @@
+#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
+//! 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.
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH proxmox v8 2/6] ebpf: tc: add the link plumbing
2026-11-04 17:29 [PATCH proxmox v8 0/6] add proxmox-ebpf library Hannes Laimer
2026-11-04 17:29 ` [PATCH proxmox v8 1/6] ebpf: add the crate with its BPF build Hannes Laimer
@ 2026-11-04 17:29 ` Hannes Laimer
2026-11-04 17:29 ` [PATCH proxmox v8 3/6] ebpf: subsystem: add the shared tc program handling Hannes Laimer
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2026-11-04 17:29 UTC (permalink / raw)
To: pve-devel
Attach, swap and detach of tcx links on an interface, and the naming
of their pins in bpffs. A swap rebinds the pinned link to a new
program in place, so an update never interrupts traffic.
What a failed step means for the interface is the caller's decision.
So a lost race against another attach, a vanished interface and a pin
already gone each come back as their own error, read from the
kernel's answer.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
proxmox-ebpf/src/lib.rs | 2 +
proxmox-ebpf/src/tc.rs | 313 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 315 insertions(+)
create mode 100644 proxmox-ebpf/src/tc.rs
diff --git a/proxmox-ebpf/src/lib.rs b/proxmox-ebpf/src/lib.rs
index b19895faaa18..6281a024fd2b 100644
--- a/proxmox-ebpf/src/lib.rs
+++ b/proxmox-ebpf/src/lib.rs
@@ -1,3 +1,5 @@
#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
//! 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.
+
+pub mod tc;
diff --git a/proxmox-ebpf/src/tc.rs b/proxmox-ebpf/src/tc.rs
new file mode 100644
index 000000000000..1217703e8906
--- /dev/null
+++ b/proxmox-ebpf/src/tc.rs
@@ -0,0 +1,313 @@
+//! TC link plumbing shared by all subsystems. How a subsystem's programs are attached as tcx
+//! links (kernel 6.6 and newer), swapped and detached, and how their pins are named in bpffs,
+//! see [`LinkPin`].
+
+use std::{io::ErrorKind, path::Path};
+
+use anyhow::Context;
+use aya::{
+ pin::PinError,
+ programs::{
+ ProgramError, SchedClassifier, TcAttachType,
+ links::{FdLink, LinkError, LinkOrder, PinnedLink},
+ tc::TcAttachOptions,
+ },
+};
+
+/// Why a link step did not go through, with the kernel's own answer where it gives one, so a
+/// caller can tell a lost race or a vanished interface from a real failure.
+#[derive(Debug)]
+pub enum Error {
+ /// A link of the program is in place already, another attach of the interface won the race.
+ AlreadyAttached,
+ /// The interface is gone, its index names no netdev anymore.
+ IfaceGone,
+ /// The pin was gone by the time it was opened or removed, another attach or unplug took it.
+ PinGone,
+ Other(anyhow::Error),
+}
+
+impl std::fmt::Display for Error {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match self {
+ Self::AlreadyAttached => f.write_str("link already in place"),
+ Self::IfaceGone => f.write_str("interface is gone"),
+ Self::PinGone => f.write_str("pin is gone"),
+ Self::Other(e) if f.alternate() => write!(f, "{e:#}"),
+ Self::Other(e) => write!(f, "{e}"),
+ }
+ }
+}
+
+impl std::error::Error for Error {
+ fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+ match self {
+ Self::Other(e) => e.chain().nth(1),
+ _ => None,
+ }
+ }
+}
+
+impl From<anyhow::Error> for Error {
+ fn from(e: anyhow::Error) -> Self {
+ Self::Other(e)
+ }
+}
+
+// EEXIST on the link create is the kernel refusing a second link of the program in the chain
+impl From<ProgramError> for Error {
+ fn from(e: ProgramError) -> Self {
+ match e {
+ ProgramError::SyscallError(s) if s.io_error.kind() == ErrorKind::AlreadyExists => {
+ Self::AlreadyAttached
+ }
+ e => Self::Other(e.into()),
+ }
+ }
+}
+
+// EEXIST on the pin is a link of this subsystem pinned at the path already, the interface has
+// a link either way
+impl From<PinError> for Error {
+ fn from(e: PinError) -> Self {
+ match e {
+ PinError::SyscallError(s) if s.io_error.kind() == ErrorKind::AlreadyExists => {
+ Self::AlreadyAttached
+ }
+ e => Self::Other(e.into()),
+ }
+ }
+}
+
+impl From<LinkError> for Error {
+ fn from(e: LinkError) -> Self {
+ match e {
+ LinkError::SyscallError(s) if s.io_error.kind() == ErrorKind::NotFound => Self::PinGone,
+ e => Self::Other(e.into()),
+ }
+ }
+}
+
+impl From<std::io::Error> for Error {
+ fn from(e: std::io::Error) -> Self {
+ if e.kind() == ErrorKind::NotFound {
+ Self::PinGone
+ } else {
+ Self::Other(e.into())
+ }
+ }
+}
+
+/// The tc hook a program attaches to, the interface's ingress or egress.
+#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)]
+pub enum Direction {
+ /// Packets arriving on the interface.
+ Ingress,
+ /// Packets leaving the interface.
+ Egress,
+}
+
+impl Direction {
+ /// The lowercase name, as it stands in pin file names.
+ pub fn as_str(self) -> &'static str {
+ match self {
+ Self::Ingress => "ingress",
+ Self::Egress => "egress",
+ }
+ }
+ /// The hook as aya's attach call takes it.
+ pub(crate) fn aya_type(self) -> TcAttachType {
+ match self {
+ Self::Ingress => TcAttachType::Ingress,
+ Self::Egress => TcAttachType::Egress,
+ }
+ }
+ fn parse(s: &str) -> Option<Self> {
+ match s {
+ "ingress" => Some(Self::Ingress),
+ "egress" => Some(Self::Egress),
+ _ => None,
+ }
+ }
+}
+
+/// One pinned link as its pin file names it. The interface name is part of the file name, so an
+/// unplug reported after the interface is gone still finds the pin and a reused index does not
+/// pass for the old link. The fields are separated by a colon, which the kernel forbids in
+/// interface names. A dot inside the name is written as a colon as well, since bpffs refuses a
+/// dot in a file name.
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct LinkPin {
+ /// The interface's index when the link was attached.
+ pub ifindex: u32,
+ /// The interface's name when the link was attached.
+ pub name: String,
+ /// The hook the link is attached to.
+ pub dir: Direction,
+}
+
+impl LinkPin {
+ /// The pin's file name, the inverse of [`parse`](Self::parse).
+ pub fn filename(&self) -> String {
+ format!(
+ "{}:{}:{}",
+ self.ifindex,
+ self.dir.as_str(),
+ self.name.replace('.', ":")
+ )
+ }
+
+ /// The pin a file name stands for, none when it is not one of ours.
+ pub fn parse(filename: &str) -> Option<Self> {
+ let mut parts = filename.splitn(3, ':');
+ let ifindex = parts.next()?.parse().ok()?;
+ let dir = Direction::parse(parts.next()?)?;
+ let name = parts.next()?.replace(':', ".");
+ let pin = LinkPin { ifindex, name, dir };
+ // a file name that does not round-trip is not one of ours, and neither is one the kernel
+ // would refuse as an interface name
+ let valid_name = !pin.name.is_empty()
+ && pin.name.len() < 16
+ && pin.name != "."
+ && pin.name != ".."
+ && !pin.name.chars().any(|c| c.is_whitespace() || c == '/');
+ (pin.filename() == filename && valid_name).then_some(pin)
+ }
+}
+
+/// Attach `prog` on the interface and direction of `pin` as a tcx link and pin it at
+/// `pin_path`. tcx (kernel 6.6 and newer) runs its programs on the netdev hook ahead of the
+/// filters on the interface's qdiscs, such as the `ingress` qdisc the rate limiter installs.
+/// `TC_ACT_UNSPEC` hands a packet on to the next program and then to the filters. Any other
+/// verdict ends the chain right there. So a subsystem's program returns `TC_ACT_UNSPEC` for
+/// whatever it leaves alone. Programs of several subsystems run in attach order, each new link
+/// goes last.
+pub fn attach_and_pin(
+ prog: &mut SchedClassifier,
+ pin: &LinkPin,
+ pin_path: &Path,
+) -> Result<(), Error> {
+ let link_id = prog
+ .attach_with_options(
+ &pin.name,
+ pin.dir.aya_type(),
+ TcAttachOptions::TcxOrder(LinkOrder::last()),
+ )
+ .map_err(|e| match Error::from(e) {
+ // the interface can go away between the name lookup and the attach
+ Error::Other(_) if matches!(iface_name(pin.ifindex), Ok(None)) => Error::IfaceGone,
+ e => e,
+ })?;
+ let link = prog.take_link(link_id)?;
+ let fd_link: FdLink = link.try_into()?;
+ // the attach goes by name, so an interface recreated under it since the lookup got the link
+ if iface_name(pin.ifindex)?.as_deref() != Some(pin.name.as_str()) {
+ return Err(Error::IfaceGone);
+ }
+ // a lost race against another attach of the same interface shows up here as well, the
+ // unpinned link detaches again when its fd drops
+ fd_link.pin(pin_path)?;
+ Ok(())
+}
+
+/// Rebind a pinned link to `prog` via LINK_UPDATE. Atomic, traffic sees no detach/reattach gap.
+pub fn swap_pinned_link(prog: &mut SchedClassifier, pin_path: &Path) -> Result<(), Error> {
+ let pinned = PinnedLink::from_pin(pin_path)?;
+ let fd_link: FdLink = pinned.into();
+ let link = fd_link.try_into()?;
+ let new_id = prog.attach_to_link(link)?;
+ // take the handle out of aya's internal tracking, we have the pin
+ let _ = prog.take_link(new_id)?;
+ Ok(())
+}
+
+pub fn detach_pinned_link(pin_path: &Path) -> Result<(), Error> {
+ let pinned = PinnedLink::from_pin(pin_path)?;
+ let _fd_link = pinned.unpin()?;
+ Ok(())
+}
+
+/// The name of an interface by index, none when it is gone. Any other failure says nothing
+/// about the interface and is not taken as its absence, so the lookup reads errno itself.
+pub fn iface_name(ifindex: u32) -> anyhow::Result<Option<String>> {
+ let mut buf = [0u8; nix::libc::IF_NAMESIZE];
+ // SAFETY: the buffer holds IF_NAMESIZE bytes, the most the call writes
+ let name = unsafe { nix::libc::if_indextoname(ifindex, buf.as_mut_ptr().cast()) };
+ if name.is_null() {
+ return match nix::errno::Errno::last() {
+ nix::errno::Errno::ENXIO => Ok(None),
+ e => Err(e).with_context(|| format!("name of interface {ifindex}")),
+ };
+ }
+ let name = std::ffi::CStr::from_bytes_until_nul(&buf)?
+ .to_str()
+ .with_context(|| format!("name of interface {ifindex} is not UTF-8"))?;
+ Ok(Some(name.to_owned()))
+}
+
+/// The pins in `links_dir`, see [`LinkPin`], none for a missing directory. A file that is not a
+/// pin is skipped.
+pub fn read_pinned_links(links_dir: &Path) -> anyhow::Result<Vec<LinkPin>> {
+ let mut out = Vec::new();
+ let dir = match std::fs::read_dir(links_dir) {
+ Ok(d) => d,
+ Err(e) if matches!(e.kind(), ErrorKind::NotFound | ErrorKind::NotADirectory) => {
+ return Ok(out);
+ }
+ Err(e) => return Err(e.into()),
+ };
+ for entry in dir {
+ let entry = entry?;
+ let name = entry.file_name();
+ match name.to_str().and_then(LinkPin::parse) {
+ Some(pin) => out.push(pin),
+ None => log::debug!(
+ "unrecognized pin file {} in {}",
+ name.to_string_lossy(),
+ links_dir.display()
+ ),
+ }
+ }
+ Ok(out)
+}
+
+#[cfg(test)]
+mod test {
+ use super::*;
+
+ #[test]
+ fn pin_names_round_trip() {
+ for name in ["tap100i0", "veth100i31", "vmbr0.100", "bond0.5.7"] {
+ let pin = LinkPin {
+ ifindex: 42,
+ name: name.into(),
+ dir: Direction::Ingress,
+ };
+ assert!(!pin.filename().contains('.'), "{}", pin.filename());
+ assert_eq!(LinkPin::parse(&pin.filename()), Some(pin));
+ }
+ assert_eq!(
+ LinkPin::parse("7:egress:eno1").map(|p| p.name),
+ Some("eno1".into())
+ );
+ }
+
+ #[test]
+ fn pin_names_are_canonical() {
+ for bad in [
+ "042:ingress:tap0",
+ "+42:ingress:tap0",
+ "42:ingress:",
+ "42:ingress",
+ "42:Ingress:tap0",
+ "42-ingress",
+ "4294967296:ingress:tap0",
+ "42:ingress:tap 0",
+ "42:ingress::",
+ "42:ingress:::",
+ "42:ingress:sixteencharsname",
+ ] {
+ assert_eq!(LinkPin::parse(bad), None, "{bad}");
+ }
+ }
+}
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH proxmox v8 3/6] ebpf: subsystem: add the shared tc program handling
2026-11-04 17:29 [PATCH proxmox v8 0/6] add proxmox-ebpf library Hannes Laimer
2026-11-04 17:29 ` [PATCH proxmox v8 1/6] ebpf: add the crate with its BPF build Hannes Laimer
2026-11-04 17:29 ` [PATCH proxmox v8 2/6] ebpf: tc: add the link plumbing Hannes Laimer
@ 2026-11-04 17:29 ` Hannes Laimer
2026-11-05 16:20 ` Wolfgang Bumiller
2026-11-04 17:29 ` [PATCH proxmox v8 4/6] ebpf: subsystem: place subsystem links in a fixed order Hannes Laimer
` (2 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Hannes Laimer @ 2026-11-04 17:29 UTC (permalink / raw)
To: pve-devel
A subsystem owns tc classifier programs and their maps, attached per
interface and pinned in bpffs under the object's fingerprint, the
crate's version and the schema version. So state survives between
the one-shot calls of its consumer, re-running never interrupts
traffic, and an older build does not install over a newer one.
Changes run under one lock per subsystem, taken before the change
reads its input, so they reach the kernel in the order they read it.
A subsystem writes its maps entry by entry through the live maps,
or hands over the whole content. For the latter, and whenever its
own programs and maps are not pinned yet, the object is loaded with
fresh maps in a staging directory, the content is written there, and
only then do the links move onto the new programs, one link update
each, with the maps coming along since a program holds its maps. A
write that stops halfway, such as an insert the kernel cannot
allocate for, leaves the live maps whole instead of a mix of old and
new entries. The pins of maps and programs switch in one rename, so a
plug always finds the maps its program reads.
A subsystem describes its programs with a macro, from its name, the
program names, the maps and the schema version. The object is found
by the subsystem's name, and its fingerprint is derived from its
bytes.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
proxmox-ebpf/src/lib.rs | 1 +
proxmox-ebpf/src/subsystem.rs | 1067 +++++++++++++++++++++++++++++++++
proxmox-ebpf/src/tc.rs | 32 +-
3 files changed, 1088 insertions(+), 12 deletions(-)
create mode 100644 proxmox-ebpf/src/subsystem.rs
diff --git a/proxmox-ebpf/src/lib.rs b/proxmox-ebpf/src/lib.rs
index 6281a024fd2b..ab0763ab3d3d 100644
--- a/proxmox-ebpf/src/lib.rs
+++ b/proxmox-ebpf/src/lib.rs
@@ -2,4 +2,5 @@
//! 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.
+pub mod subsystem;
pub mod tc;
diff --git a/proxmox-ebpf/src/subsystem.rs b/proxmox-ebpf/src/subsystem.rs
new file mode 100644
index 000000000000..39272060cd4e
--- /dev/null
+++ b/proxmox-ebpf/src/subsystem.rs
@@ -0,0 +1,1067 @@
+//! The shared subsystem code. A subsystem has one tc program per direction it declares, with
+//! their maps and links. They are pinned under `/sys/fs/bpf/proxmox-ebpf/<name>/`, the links in
+//! `links/` and the maps and programs under `<schema>/live/`. The programs are pinned by the
+//! fingerprint of their object and selection and the version of the crate they were built into,
+//! so the pinned state itself says which build it belongs to, and a build does not install over
+//! a newer one.
+//!
+//! The loaded BPF stays in the kernel between invocations. [`ApplyLock::replace_maps`] loads the
+//! object with fresh maps and moves the links onto it, a full apply calls it when what is pinned
+//! is not this build's, when the last replacement stopped halfway, or whenever it wants the
+//! content of its maps switched as a whole. Every other step works on what is pinned. Each
+//! subsystem owns one [`TcPrograms`].
+//!
+//! Every change runs under the subsystem's apply lock, a full apply exclusively and a
+//! single-interface step shared. The holder reads the input of its change under the lock and
+//! writes before it lets go, so a full apply and every step reach the kernel in the order they
+//! read their input. Two steps of one interface do not order each other.
+use std::{
+ collections::HashSet,
+ fs::File,
+ io::ErrorKind,
+ marker::PhantomData,
+ ops::{Deref, DerefMut},
+ os::unix::fs::{DirBuilderExt, OpenOptionsExt},
+ path::{Path, PathBuf},
+ time::{Duration, Instant},
+};
+
+use anyhow::Context;
+use aya::{EbpfLoader, programs::SchedClassifier};
+use nix::{
+ errno::Errno,
+ fcntl::{Flock, FlockArg, RenameFlags, renameat2},
+};
+
+use crate::tc::{self, Direction};
+
+const PIN_ROOT: &str = "/sys/fs/bpf/proxmox-ebpf";
+const RUN_ROOT: &str = "/run/proxmox-ebpf";
+
+// FNV-1a over the object, the program selection and the map names, so the same object with
+// another program chosen for a direction is another build
+const fn build_fingerprint(obj: &[u8], programs: &[(Direction, &str)], maps: &[&str]) -> u64 {
+ let mut hash = fnv1a(0xcbf29ce484222325, obj);
+ let mut i = 0;
+ while i < programs.len() {
+ hash = fnv1a(hash, programs[i].0.as_str().as_bytes());
+ hash = fnv1a(hash, programs[i].1.as_bytes());
+ i += 1;
+ }
+ let mut i = 0;
+ while i < maps.len() {
+ hash = fnv1a(hash, maps[i].as_bytes());
+ i += 1;
+ }
+ hash
+}
+
+// one field, ended by a zero byte so two fields cannot read as one
+const fn fnv1a(mut hash: u64, bytes: &[u8]) -> u64 {
+ let mut i = 0;
+ while i < bytes.len() {
+ hash ^= bytes[i] as u64;
+ hash = hash.wrapping_mul(0x100000001b3);
+ i += 1;
+ }
+ hash.wrapping_mul(0x100000001b3)
+}
+
+// one number of a cargo version as the environment hands it to the macro, digits only
+const fn version_part(s: &str) -> u64 {
+ let bytes = s.as_bytes();
+ let mut n = 0u64;
+ let mut i = 0;
+ while i < bytes.len() {
+ assert!(bytes[i].is_ascii_digit(), "a cargo version part is digits");
+ n = n * 10 + (bytes[i] - b'0') as u64;
+ i += 1;
+ }
+ n
+}
+
+/// A subsystem's [`TcPrograms`] from its name. The object is the build's `<name>.bpf.o`, the
+/// fingerprint follows from the bytes and the version is the crate's own.
+#[macro_export]
+macro_rules! tc_programs {
+ (
+ name: $name:literal,
+ programs: $programs:expr,
+ maps: $maps:expr,
+ schema_version: $schema:expr $(,)?
+ ) => {
+ // built at compile time wherever the macro is used, a let binding included
+ const {
+ const OBJ: &[u8] =
+ ::aya::include_bytes_aligned!(concat!(env!("OUT_DIR"), "/", $name, ".bpf.o"));
+ $crate::subsystem::TcPrograms::new(
+ $name,
+ OBJ,
+ $programs,
+ $maps,
+ $schema,
+ [
+ env!("CARGO_PKG_VERSION_MAJOR"),
+ env!("CARGO_PKG_VERSION_MINOR"),
+ env!("CARGO_PKG_VERSION_PATCH"),
+ ],
+ )
+ }
+ };
+}
+
+const LOCK_WAIT: Duration = Duration::from_secs(10);
+const LOCK_POLL: Duration = Duration::from_millis(10);
+
+// flock takes a read-only descriptor, so nobody but root gets to open the lock files
+fn private_dir(dir: &Path) -> std::io::Result<()> {
+ std::fs::DirBuilder::new()
+ .recursive(true)
+ .mode(0o700)
+ .create(dir)
+}
+
+// flock has no timeout, so the wait retries a nonblocking flock with a sleep in between.
+//
+// not proxmox-sys lock because: blocks and gives up on a timer signal, and under pve-rs any signal
+// perl handles cuts the blocked wait short the same way, since perl installs its handlers without
+// restart
+fn flock_within(
+ path: &Path,
+ exclusive: bool,
+ what: &str,
+ subsystem: &str,
+) -> anyhow::Result<Flock<File>> {
+ let mut file = File::options()
+ .create(true)
+ .truncate(false)
+ .write(true)
+ .mode(0o600)
+ .open(path)
+ .with_context(|| format!("{subsystem}: open {what} {}", path.display()))?;
+ let arg = if exclusive {
+ FlockArg::LockExclusiveNonblock
+ } else {
+ FlockArg::LockSharedNonblock
+ };
+ let deadline = Instant::now() + LOCK_WAIT;
+ loop {
+ match Flock::lock(file, arg) {
+ Ok(lock) => return Ok(lock),
+ Err((returned, Errno::EWOULDBLOCK)) if Instant::now() < deadline => {
+ file = returned;
+ std::thread::sleep(LOCK_POLL);
+ }
+ Err((_, Errno::EWOULDBLOCK)) => anyhow::bail!(
+ "{subsystem}: {what} still held after {}s",
+ LOCK_WAIT.as_secs()
+ ),
+ Err((_, e)) => {
+ return Err(e)
+ .with_context(|| format!("{subsystem}: take {what} {}", path.display()));
+ }
+ }
+ }
+}
+
+/// A step that needs the programs pinned found none. Only [`ApplyLock::replace_maps`] under the
+/// exclusive lock brings them up.
+#[derive(Debug)]
+pub struct NotLoaded;
+
+impl std::fmt::Display for NotLoaded {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ f.write_str("programs not loaded")
+ }
+}
+
+impl std::error::Error for NotLoaded {}
+
+/// A subsystem's description of its tc programs, written with the `tc_programs!` macro. The
+/// loaded programs live pinned in the kernel and are reached through those pins.
+pub struct TcPrograms {
+ /// The subsystem's name, its directory under the pin root.
+ name: &'static str,
+ /// The embedded BPF object.
+ obj: &'static [u8],
+ /// FNV-1a of `obj`, `programs` and `maps`, the programs are pinned under it.
+ fingerprint: u64,
+ /// The version of the crate the object was built into, pinned next to the fingerprint. A
+ /// build does not install over a higher one.
+ version: [u64; 3],
+ /// The directions the object carries a program for, each with that program's name in the
+ /// object. One entry per direction.
+ programs: &'static [(Direction, &'static str)],
+ /// The object's pinned maps. A missing one means the state is not current.
+ maps: &'static [&'static str],
+ /// Bumped with any change to a map's key or value layout. Each schema has its own live set,
+ /// so a step never writes into maps of another layout, and a build leaves the state of a
+ /// newer schema alone.
+ schema_version: u32,
+}
+
+impl TcPrograms {
+ /// What [`tc_programs!`](crate::tc_programs) expands to, with the fingerprint derived and the
+ /// version parsed at compile time.
+ pub const fn new(
+ name: &'static str,
+ obj: &'static [u8],
+ programs: &'static [(Direction, &'static str)],
+ maps: &'static [&'static str],
+ schema_version: u32,
+ version: [&'static str; 3],
+ ) -> Self {
+ Self {
+ name,
+ obj,
+ fingerprint: build_fingerprint(obj, programs, maps),
+ version: [
+ version_part(version[0]),
+ version_part(version[1]),
+ version_part(version[2]),
+ ],
+ programs,
+ maps,
+ schema_version,
+ }
+ }
+
+ fn pin_root(&self) -> PathBuf {
+ PathBuf::from(PIN_ROOT).join(self.name)
+ }
+ fn schema_root(&self) -> PathBuf {
+ self.pin_root().join(self.schema_version.to_string())
+ }
+ fn live_dir(&self) -> PathBuf {
+ schema_live_dir(&self.schema_root())
+ }
+ fn staging_dir(&self) -> PathBuf {
+ self.schema_root().join("staging")
+ }
+ fn links_dir(&self) -> PathBuf {
+ links_dir_of(self.name)
+ }
+ fn link_pin_path(&self, pin: &tc::LinkPin) -> PathBuf {
+ self.links_dir().join(pin.filename())
+ }
+ // the build directory of a set of maps and programs, the live one or a staged one
+ fn prog_dir_in(&self, set: &Path) -> PathBuf {
+ set.join("prog")
+ .join(build_dir_name(self.version, self.fingerprint))
+ }
+ // the builds with programs pinned in the live set, each with its directory
+ fn pinned_builds(&self) -> anyhow::Result<Vec<(PathBuf, [u64; 3], u64)>> {
+ let entries = match std::fs::read_dir(schema_progs_dir(&self.schema_root())) {
+ Ok(entries) => entries,
+ Err(e) if e.kind() == ErrorKind::NotFound => return Ok(Vec::new()),
+ Err(e) => return Err(e.into()),
+ };
+ let mut builds = Vec::new();
+ for entry in entries {
+ let entry = entry?;
+ if let Some((version, fingerprint)) =
+ entry.file_name().to_str().and_then(parse_build_dir_name)
+ {
+ builds.push((entry.path(), version, fingerprint));
+ }
+ }
+ Ok(builds)
+ }
+ // the directory this build's programs are pinned in, named by whichever version installed
+ // them, a release with the same object installs nothing
+ fn own_prog_dir(&self) -> anyhow::Result<Option<PathBuf>> {
+ Ok(self
+ .pinned_builds()?
+ .into_iter()
+ .find(|&(_, _, fingerprint)| fingerprint == self.fingerprint)
+ .map(|(dir, _, _)| dir))
+ }
+ fn newer_version_installed(&self) -> anyhow::Result<Option<[u64; 3]>> {
+ Ok(self
+ .pinned_builds()?
+ .into_iter()
+ .map(|(_, version, _)| version)
+ .filter(|&version| version > self.version)
+ .max())
+ }
+ fn program_name(&self, dir: Direction) -> Option<&'static str> {
+ self.programs
+ .iter()
+ .find(|&&(d, _)| d == dir)
+ .map(|&(_, name)| name)
+ }
+ fn run_dir(&self) -> PathBuf {
+ PathBuf::from(RUN_ROOT).join(self.name)
+ }
+ fn hash_map<K: aya::Pod, V: aya::Pod>(
+ &self,
+ name: &str,
+ ) -> anyhow::Result<aya::maps::HashMap<aya::maps::MapData, K, V>> {
+ self.hash_map_in(&self.live_dir(), name)
+ }
+
+ fn hash_map_in<K: aya::Pod, V: aya::Pod>(
+ &self,
+ set: &Path,
+ name: &str,
+ ) -> anyhow::Result<aya::maps::HashMap<aya::maps::MapData, K, V>> {
+ let path = set.join(name);
+ if !path.exists() {
+ // a set comes up with every map the object pins, so an existing set lacks only a map
+ // the object never had under that name
+ if set.exists() {
+ anyhow::bail!("{}: no map {name} pinned in {}", self.name, set.display());
+ }
+ return Err(NotLoaded.into());
+ }
+ let map = aya::maps::MapData::from_pin(path)
+ .with_context(|| format!("{}: open pinned map {name}", self.name))?;
+ Ok(aya::maps::HashMap::try_from(aya::maps::Map::HashMap(map))?)
+ }
+
+ // open one direction's pinned program, NotLoaded when it is not pinned
+ fn program(&self, dir: Direction) -> anyhow::Result<SchedClassifier> {
+ let path = self.own_prog_dir()?.map(|d| d.join(dir.as_str()));
+ let Some(path) = path.filter(|p| p.exists()) else {
+ return Err(NotLoaded.into());
+ };
+ SchedClassifier::from_pin(path)
+ .with_context(|| format!("{}: open pinned program {}", self.name, dir.as_str()))
+ }
+
+ // one direction's program pinned in the live set, by whichever build pinned it there
+ fn live_program(&self, dir: Direction) -> anyhow::Result<Option<SchedClassifier>> {
+ for (build, _, _) in self.pinned_builds()? {
+ let path = build.join(dir.as_str());
+ if !path.exists() {
+ continue;
+ }
+ let prog = SchedClassifier::from_pin(&path).with_context(|| {
+ format!("{}: open pinned program {}", self.name, path.display())
+ })?;
+ return Ok(Some(prog));
+ }
+ Ok(None)
+ }
+
+ // the apply lock under /run. A full apply and a teardown take it exclusively, a
+ // single-interface step shared. A wait times out instead of hanging. Shared holders do not
+ // yield to a waiting exclusive one, so a node attaching interfaces without pause can keep a
+ // full apply waiting until it times out. Every write to the pinned state is a method of the
+ // returned guard, so a step cannot run outside the lock
+ fn lock<M>(&self, exclusive: bool) -> anyhow::Result<ApplyLock<'_, M>> {
+ let dir = self.run_dir();
+ private_dir(&dir).with_context(|| {
+ format!("{}: create the run directory {}", self.name, dir.display())
+ })?;
+ let lock = flock_within(&dir.join("lock"), exclusive, "apply lock", self.name)?;
+ Ok(ApplyLock {
+ programs: self,
+ _lock: lock,
+ _mode: PhantomData,
+ })
+ }
+
+ /// Whether a link of the interface name is pinned, read without the lock. Most interfaces
+ /// never had a link and their unplug must not wait behind a full apply. A miss is a snapshot,
+ /// a pin arriving after it is another attach's. A failed listing counts as a hit.
+ pub fn link_pinned(&self, name: &str) -> bool {
+ self.live_links()
+ .map_or(true, |links| links.iter().any(|pin| pin.name == name))
+ }
+
+ /// Take the apply lock shared, for a single-interface step.
+ pub fn lock_shared(&self) -> anyhow::Result<ApplyLock<'_, Shared>> {
+ self.lock(false)
+ }
+
+ /// Take the apply lock exclusively, for a full apply or a teardown.
+ pub fn lock_exclusive(&self) -> anyhow::Result<ApplyLock<'_, Exclusive>> {
+ self.lock(true)
+ }
+
+ fn is_current(&self) -> bool {
+ let programs_pinned = match self.own_prog_dir() {
+ Ok(Some(dir)) => self
+ .programs
+ .iter()
+ .all(|&(d, _)| dir.join(d.as_str()).exists()),
+ _ => false,
+ };
+ programs_pinned
+ && self.maps_pinned()
+ // a failed listing cannot rule a newer schema out
+ && matches!(self.newer_schema_installed(), Ok(None))
+ }
+
+ fn maps_pinned(&self) -> bool {
+ self.maps
+ .iter()
+ .all(|map| self.live_dir().join(map).exists())
+ }
+
+ // the directories under the pin root other than this build's schema and the links. One whose
+ // name is not a version counts as well and goes with the old schemas
+ fn other_schemas(&self) -> anyhow::Result<Vec<(PathBuf, Option<u32>)>> {
+ let entries = match std::fs::read_dir(self.pin_root()) {
+ Ok(entries) => entries,
+ Err(e) if e.kind() == ErrorKind::NotFound => return Ok(Vec::new()),
+ Err(e) => return Err(e.into()),
+ };
+ let mut others = Vec::new();
+ for entry in entries {
+ let entry = entry?;
+ if !entry.file_type()?.is_dir() || entry.file_name() == LINKS_DIR {
+ continue;
+ }
+ let schema = entry
+ .file_name()
+ .to_str()
+ .and_then(|s| s.parse::<u32>().ok());
+ if schema != Some(self.schema_version) {
+ others.push((entry.path(), schema));
+ }
+ }
+ Ok(others)
+ }
+
+ fn newer_schema_installed(&self) -> anyhow::Result<Option<u32>> {
+ let newer = self
+ .other_schemas()?
+ .iter()
+ .filter(|(dir, _)| any_program_pinned(&schema_progs_dir(dir)))
+ .filter_map(|(_, schema)| *schema)
+ .find(|&s| s > self.schema_version);
+ Ok(newer)
+ }
+
+ fn interrupted(&self) -> bool {
+ let other_pinned = match self.other_schemas() {
+ Ok(others) => others
+ .iter()
+ .any(|(dir, _)| any_program_pinned(&schema_progs_dir(dir))),
+ // a failed listing cannot rule a leftover out
+ Err(_) => true,
+ };
+ self.staging_dir().exists() || other_pinned
+ }
+
+ fn tear_down(&self) -> anyhow::Result<()> {
+ remove_dir_if_present(&self.pin_root())
+ .with_context(|| format!("{}: remove the pinned state", self.name))
+ }
+
+ // this build's object with all of its programs loaded and its maps created and pinned under
+ // `set`. Everything is loaded before any link is touched, so a verifier rejection of one
+ // direction leaves the old state whole
+ fn load(&self, set: &Path) -> anyhow::Result<aya::Ebpf> {
+ let mut bpf = EbpfLoader::new()
+ .map_pin_path(set)
+ .load(self.obj)
+ .with_context(|| format!("{}: load the object", self.name))?;
+ for &(_, name) in self.programs {
+ let prog: &mut SchedClassifier = bpf
+ .program_mut(name)
+ .with_context(|| format!("{}: program {name} not found in object", self.name))?
+ .try_into()?;
+ prog.load()
+ .with_context(|| format!("{}: load program {name}", self.name))?;
+ }
+ Ok(bpf)
+ }
+
+ // move every pinned link onto the loaded programs
+ fn swap_links(&self, bpf: &mut aya::Ebpf) -> anyhow::Result<()> {
+ let live = self.live_links()?;
+ for &(dir, _) in self.programs {
+ self.swap_links_onto(dir, self.loaded(bpf, dir)?, &live)?;
+ }
+ Ok(())
+ }
+
+ // put the live set on every link, a link on it already stays as it is, and detach the links
+ // of a direction the live set has no program for, they run a retired program. The live set
+ // decides, not this build's directions, since a run that died before its exchange left the
+ // old set live, and that keeps its links until the exchange
+ fn repair_links(&self) -> anyhow::Result<()> {
+ let live = self.live_links()?;
+ for dir in Direction::ALL {
+ match self.live_program(dir)? {
+ Some(mut prog) => self.swap_links_onto(dir, &mut prog, &live)?,
+ None => {
+ for pin in live.iter().filter(|pin| pin.dir == dir) {
+ self.reclaim_link(pin)?;
+ }
+ }
+ }
+ }
+ Ok(())
+ }
+
+ // the links of one direction onto `prog`. One that cannot be swapped is unpinned, the next
+ // full apply attaches it again if it is still wanted
+ fn swap_links_onto(
+ &self,
+ dir: Direction,
+ prog: &mut SchedClassifier,
+ links: &[tc::LinkPin],
+ ) -> anyhow::Result<()> {
+ for pin in links.iter().filter(|pin| pin.dir == dir) {
+ let path = self.link_pin_path(pin);
+ // a pin that does not open says nothing about the link, one that opens and cannot be
+ // updated is a link the kernel let go
+ let link = match tc::open_pinned_link(&path) {
+ Ok(link) => link,
+ Err(tc::Error::PinGone) => continue,
+ Err(e) => {
+ return Err(e)
+ .with_context(|| format!("{}: open {}", self.name, pin.filename()));
+ }
+ };
+ if let Err(e) = tc::swap_link(prog, link) {
+ log::warn!(
+ "{}: {} swap failed ({e:#}), reclaiming",
+ self.name,
+ pin.filename()
+ );
+ self.reclaim_link(pin)?;
+ }
+ }
+ Ok(())
+ }
+
+ fn pin_programs(&self, bpf: &mut aya::Ebpf, set: &Path) -> anyhow::Result<()> {
+ let dir = self.prog_dir_in(set);
+ std::fs::create_dir_all(&dir)
+ .with_context(|| format!("{}: create the program directory", self.name))?;
+ for &(direction, _) in self.programs {
+ self.loaded(bpf, direction)?
+ .pin(dir.join(direction.as_str()))
+ .with_context(|| format!("{}: pin program {}", self.name, direction.as_str()))?;
+ }
+ Ok(())
+ }
+
+ // load the object with fresh maps under `staging`, have `fill` write them, then put the
+ // staged set in the live one's place and move the links over. That is one rename, an
+ // exchange when a live set is there, so a plug never finds the maps of one set next to the
+ // programs of the other, and a failure before it leaves the live set untouched. The retired
+ // set and the directories of other schemas go last, their programs stay alive through the
+ // links until each is swapped
+ fn replace_maps(
+ &self,
+ fill: impl FnOnce(&Staged<'_>) -> anyhow::Result<()>,
+ ) -> anyhow::Result<()> {
+ let others = self.other_schemas()?;
+ // programs pinned under a newer schema or by a newer version of the crate mean a newer
+ // build runs here and this process is the stale one, so it leaves the state alone
+ if let Some(newer) = self.newer_schema_installed()? {
+ anyhow::bail!(
+ "{}: pinned state is of schema {newer}, newer than this build's {}",
+ self.name,
+ self.schema_version
+ );
+ }
+ if let Some(newer) = self.newer_version_installed()? {
+ anyhow::bail!(
+ "{}: pinned programs are of version {}, newer than this build's {}",
+ self.name,
+ version_text(newer),
+ version_text(self.version)
+ );
+ }
+ std::fs::create_dir_all(self.links_dir())
+ .with_context(|| format!("{}: create the links directory", self.name))?;
+ let staging = self.staging_dir();
+ let live = self.live_dir();
+ // a run that died may have left links on the set it retired, so first make sure every
+ // link has the live set, then clear what it left behind. Without a live set the links
+ // are another schema's, whose set stays until the end
+ if staging.exists() && live.exists() {
+ self.repair_links()?;
+ }
+ remove_dir_if_present(&staging)
+ .with_context(|| format!("{}: remove the previous staging directory", self.name))?;
+ let staged = self.stage(&staging, fill).and_then(|bpf| {
+ if live.exists() {
+ renameat2(None, &staging, None, &live, RenameFlags::RENAME_EXCHANGE).with_context(
+ || format!("{}: exchange the staged set with the live one", self.name),
+ )?;
+ } else {
+ std::fs::rename(&staging, &live)
+ .with_context(|| format!("{}: move the staged set into place", self.name))?;
+ }
+ Ok(bpf)
+ });
+ let mut bpf = match staged {
+ Ok(bpf) => bpf,
+ Err(e) => {
+ // the pins would keep the fresh maps alive
+ if let Err(err) = std::fs::remove_dir_all(&staging) {
+ log::warn!("{}: remove the staged set: {err:#}", self.name);
+ }
+ return Err(e);
+ }
+ };
+ self.remove_foreign_pins()?;
+ self.swap_links(&mut bpf)?;
+ if let Err(e) = remove_dir_if_present(&staging) {
+ log::warn!("{}: remove the retired set: {e:#}", self.name);
+ }
+ for (dir, _) in others {
+ if let Err(e) = std::fs::remove_dir_all(&dir) {
+ log::warn!(
+ "{}: remove old pinned state {}: {e:#}",
+ self.name,
+ dir.display()
+ );
+ }
+ }
+ Ok(())
+ }
+
+ // the fresh set under `staging`, its maps written by `fill` and its programs pinned
+ fn stage(
+ &self,
+ staging: &Path,
+ fill: impl FnOnce(&Staged<'_>) -> anyhow::Result<()>,
+ ) -> anyhow::Result<aya::Ebpf> {
+ std::fs::create_dir_all(staging)
+ .with_context(|| format!("{}: create the staging directory", self.name))?;
+ let mut bpf = self.load(staging)?;
+ fill(&Staged { programs: self })?;
+ self.pin_programs(&mut bpf, staging)?;
+ Ok(bpf)
+ }
+
+ // one direction's program out of the loaded object, resolved once by the load loop
+ fn loaded<'b>(
+ &self,
+ bpf: &'b mut aya::Ebpf,
+ dir: Direction,
+ ) -> anyhow::Result<&'b mut SchedClassifier> {
+ let name = self
+ .program_name(dir)
+ .expect("only directions with a program are loaded");
+ let prog = bpf
+ .program_mut(name)
+ .expect("every program was resolved by the load loop")
+ .try_into()?;
+ Ok(prog)
+ }
+
+ fn live_links(&self) -> anyhow::Result<Vec<tc::LinkPin>> {
+ tc::read_pinned_links(&self.links_dir())
+ }
+
+ // a pin this build cannot read or has no program for is not its state. It would keep a link
+ // running with nothing to update it, so it goes with the next replacement or its repair
+ fn remove_foreign_pins(&self) -> anyhow::Result<()> {
+ for entry in std::fs::read_dir(self.links_dir())? {
+ let entry = entry?;
+ let name = entry.file_name();
+ let ours = name
+ .to_str()
+ .and_then(tc::LinkPin::parse)
+ .is_some_and(|pin| self.program_name(pin.dir).is_some());
+ if ours {
+ continue;
+ }
+ let name = name.to_string_lossy();
+ log::warn!("{}: removing pin {name} this build cannot use", self.name);
+ let path = entry.path();
+ if let Err(e) = tc::detach_pinned_link(&path) {
+ log::warn!("{}: detach {name}: {e:#}", self.name);
+ let _ = std::fs::remove_file(&path);
+ }
+ }
+ Ok(())
+ }
+
+ fn detach_named(
+ &self,
+ name: &str,
+ keep: impl Fn(&tc::LinkPin) -> bool,
+ ) -> anyhow::Result<Vec<u32>> {
+ let mut ifindexes = Vec::new();
+ for pin in self.live_links()? {
+ if pin.name != name || keep(&pin) {
+ continue;
+ }
+ self.reclaim_link(&pin)?;
+ if !ifindexes.contains(&pin.ifindex) {
+ ifindexes.push(pin.ifindex);
+ }
+ }
+ Ok(ifindexes)
+ }
+
+ // unpin a link. A pin that cannot be opened stays, a failed open says nothing about the link
+ // behind it
+ fn reclaim_link(&self, pin: &tc::LinkPin) -> anyhow::Result<()> {
+ let path = self.link_pin_path(pin);
+ match tc::detach_pinned_link(&path) {
+ Ok(()) => Ok(()),
+ // the other side of a lost race, another attach or unplug took the pin first
+ Err(tc::Error::PinGone) => {
+ log::debug!(
+ "{}: reclaim {}: pin gone already",
+ self.name,
+ pin.filename()
+ );
+ Ok(())
+ }
+ Err(e) => Err(e).with_context(|| format!("{}: unpin {}", self.name, pin.filename())),
+ }
+ }
+
+ fn reconcile(&self, desired: &HashSet<u32>) -> anyhow::Result<()> {
+ // opened before anything is detached, so a missing program fails the full apply before it
+ // changes anything
+ let mut progs = Vec::new();
+ for &(dir, _) in self.programs {
+ progs.push((dir, self.program(dir)?));
+ }
+ let live = self.live_links()?;
+ let mut attached: HashSet<(u32, Direction)> = HashSet::new();
+
+ for pin in &live {
+ // a pin whose index now belongs to another interface links nothing wanted
+ let alive = tc::iface_name(pin.ifindex)?.is_some_and(|name| name == pin.name);
+ if alive && desired.contains(&pin.ifindex) {
+ attached.insert((pin.ifindex, pin.dir));
+ continue;
+ }
+ log::info!("{}: detach {}", self.name, pin.filename());
+ if let Err(e) = self.reclaim_link(pin) {
+ log::warn!("{e:#}");
+ }
+ }
+
+ let mut failed = 0usize;
+ for (dir, prog) in &mut progs {
+ let dir = *dir;
+ for &ifindex in desired {
+ if attached.contains(&(ifindex, dir)) {
+ continue;
+ }
+ let Some(name) = tc::iface_name(ifindex)? else {
+ log::info!("{}: {ifindex} vanished before the attach", self.name);
+ continue;
+ };
+ let pin = tc::LinkPin { ifindex, name, dir };
+ match tc::attach_and_pin(prog, &pin, &self.link_pin_path(&pin)) {
+ Ok(()) => {}
+ Err(tc::Error::IfaceGone) => {
+ log::info!("{}: {ifindex} vanished before the attach", self.name);
+ }
+ Err(e) => {
+ log::error!("{}: attach {}: {e:#}", self.name, pin.filename());
+ failed += 1;
+ }
+ }
+ }
+ }
+ if failed > 0 {
+ anyhow::bail!("{}: {failed} link(s) failed to attach", self.name);
+ }
+ Ok(())
+ }
+
+ fn attach_iface(&self, ifindex: u32) -> anyhow::Result<()> {
+ // opened for every direction ahead of the pin checks, so a missing program fails here,
+ // never counts as a dead link and leaves no direction attached alone
+ let mut progs = Vec::new();
+ for &(dir, _) in self.programs {
+ progs.push((dir, self.program(dir)?));
+ }
+ let Some(name) = tc::iface_name(ifindex)? else {
+ log::info!("{}: {ifindex} vanished before the attach", self.name);
+ return Ok(());
+ };
+ let live = self.live_links()?;
+ for (dir, prog) in &mut progs {
+ let dir = *dir;
+ let pin = tc::LinkPin {
+ ifindex,
+ name: name.clone(),
+ dir,
+ };
+ let path = self.link_pin_path(&pin);
+ for stale in live.iter().filter(|p| p.ifindex == ifindex && p.dir == dir) {
+ if stale.name == name {
+ let link = match tc::open_pinned_link(&path) {
+ Ok(link) => link,
+ // the pin went away under the swap, another plug of the same interface
+ // is rebuilding it and the attach below finds its link
+ Err(tc::Error::PinGone) => {
+ log::debug!(
+ "{}: {} pin taken by another attach",
+ self.name,
+ path.display()
+ );
+ continue;
+ }
+ Err(e) => {
+ return Err(e).with_context(|| {
+ format!("{}: open {}", self.name, pin.filename())
+ });
+ }
+ };
+ if let Err(e) = tc::swap_link(prog, link) {
+ log::warn!(
+ "{}: {} swap failed ({e:#}), rebuilding",
+ self.name,
+ pin.filename()
+ );
+ self.reclaim_link(stale)?;
+ continue;
+ }
+ } else {
+ // the index was reused since, the old link is not this interface's and does
+ // not stand in the way of its own
+ if let Err(e) = self.reclaim_link(stale) {
+ log::warn!("{e:#}");
+ }
+ }
+ }
+ if path.exists() {
+ continue;
+ }
+ match tc::attach_and_pin(prog, &pin, &path) {
+ Ok(()) => {}
+ Err(tc::Error::IfaceGone) => {
+ log::info!("{}: {ifindex} vanished before the attach", self.name);
+ }
+ Err(tc::Error::AlreadyAttached) => {
+ log::debug!("{}: {} attached concurrently", self.name, pin.filename());
+ }
+ Err(e) => {
+ return Err(e)
+ .with_context(|| format!("{}: attach {}", self.name, pin.filename()));
+ }
+ }
+ }
+ Ok(())
+ }
+}
+
+const LINKS_DIR: &str = "links";
+
+fn links_dir_of(subsystem: &str) -> PathBuf {
+ PathBuf::from(PIN_ROOT).join(subsystem).join(LINKS_DIR)
+}
+
+// the name of a build's program directory, its version and fingerprint, with colons for the
+// dots bpffs refuses in a name
+fn build_dir_name(version: [u64; 3], fingerprint: u64) -> String {
+ format!(
+ "{}:{}:{}-{fingerprint:016x}",
+ version[0], version[1], version[2]
+ )
+}
+
+fn parse_build_dir_name(name: &str) -> Option<([u64; 3], u64)> {
+ let (version, fingerprint) = name.split_once('-')?;
+ let mut parts = version.split(':').map(|part| part.parse::<u64>().ok());
+ let version = [parts.next()??, parts.next()??, parts.next()??];
+ if parts.next().is_some() || fingerprint.len() != 16 {
+ return None;
+ }
+ Some((version, u64::from_str_radix(fingerprint, 16).ok()?))
+}
+
+fn version_text(version: [u64; 3]) -> String {
+ format!("{}.{}.{}", version[0], version[1], version[2])
+}
+
+fn schema_live_dir(schema_root: &Path) -> PathBuf {
+ schema_root.join("live")
+}
+
+fn schema_progs_dir(schema_root: &Path) -> PathBuf {
+ schema_live_dir(schema_root).join("prog")
+}
+
+fn remove_dir_if_present(dir: &Path) -> std::io::Result<()> {
+ match std::fs::remove_dir_all(dir) {
+ Err(e) if e.kind() != ErrorKind::NotFound => Err(e),
+ _ => Ok(()),
+ }
+}
+
+// a program pin is a file under a build directory. A run killed between creating that
+// directory and pinning leaves it empty. Like the links, a directory that cannot be read may
+// hold anything, a missing one is empty
+fn any_program_pinned(progs_dir: &Path) -> bool {
+ let non_empty = |dir: &Path| match std::fs::read_dir(dir) {
+ Ok(mut entries) => entries.next().is_some(),
+ Err(e) => e.kind() != ErrorKind::NotFound,
+ };
+ match std::fs::read_dir(progs_dir) {
+ Ok(fingerprints) => fingerprints.into_iter().any(|fp| match fp {
+ Ok(fp) => non_empty(&fp.path()),
+ Err(_) => true,
+ }),
+ Err(e) => e.kind() != ErrorKind::NotFound,
+ }
+}
+
+/// Marker for a guard holding the apply lock shared.
+pub struct Shared;
+
+/// Marker for a guard holding the apply lock exclusively.
+pub struct Exclusive;
+
+/// The apply lock of one subsystem, held for as long as the guard lives. Replacing the maps and
+/// matching the links to the desired set rewrite pinned state and exist only on the
+/// [`Exclusive`] guard, so nothing runs them next to shared holders. A shared holder adds or
+/// drops its own link and writes map entries that stand on their own, each write atomic in the
+/// kernel. A write that depends on what was read before it takes the exclusive lock, or the
+/// kernel's conditional write. A shared holder cannot upgrade, one that finds nothing current
+/// leaves bringing the set up to a full apply under the exclusive lock.
+pub struct ApplyLock<'a, M> {
+ programs: &'a TcPrograms,
+ _lock: Flock<File>,
+ _mode: PhantomData<M>,
+}
+
+impl<M> ApplyLock<'_, M> {
+ /// Whether this build's programs and maps are pinned and no newer schema is. A shared holder
+ /// checks this before it writes, since only a full apply brings the pins up and an older build
+ /// stays out of a newer schema. Under the lock the program and map pins do not change, only
+ /// links come and go, so the answer holds for the holder's steps.
+ pub fn is_current(&self) -> bool {
+ self.programs.is_current()
+ }
+
+ /// Whether a replacement stopped halfway. Its staged set or the directories of other schemas
+ /// are still there, so the links may still run the programs it retired.
+ pub fn interrupted(&self) -> bool {
+ self.programs.interrupted()
+ }
+
+ /// Open one of this build's live maps by name, [`NotLoaded`] when it is not pinned.
+ pub fn hash_map<K: aya::Pod, V: aya::Pod>(
+ &self,
+ name: &str,
+ ) -> anyhow::Result<MapHandle<'_, K, V>> {
+ Ok(MapHandle {
+ map: self.programs.hash_map(name)?,
+ _opened_through: PhantomData,
+ })
+ }
+
+ /// Attach the programs to a single interface, leaving the others alone.
+ ///
+ /// If a pin already exists, swap the program in place with no traffic gap. A pin that cannot
+ /// be swapped is reclaimed and the interface attached fresh. Two concurrent attaches of the
+ /// same interface run under the shared lock, the one that loses the race finds the other's
+ /// link in place and is done. A failed attach returns the error, [`NotLoaded`] when nothing
+ /// is pinned yet, and the caller decides what an interface without the program means for it.
+ pub fn attach_iface(&self, ifindex: u32) -> anyhow::Result<()> {
+ self.programs.attach_iface(ifindex)
+ }
+
+ /// Detach the programs from the interface of that name, even when the interface is gone
+ /// already. A pin for which `keep` returns true stays, such as one of a newer interface under
+ /// the same name. Returns the indexes the detached links had.
+ pub fn detach_named(
+ &self,
+ name: &str,
+ keep: impl Fn(&tc::LinkPin) -> bool,
+ ) -> anyhow::Result<Vec<u32>> {
+ self.programs.detach_named(name, keep)
+ }
+
+ /// The links of this subsystem as their pins name them. A shared holder sees a snapshot,
+ /// another attach may add to it meanwhile. The name is not unique. A leftover pin of an
+ /// interface that went away and the pin of its successor under the name share it, only the
+ /// index tells them apart.
+ pub fn attached(&self) -> anyhow::Result<Vec<tc::LinkPin>> {
+ self.programs.live_links()
+ }
+}
+
+impl ApplyLock<'_, Exclusive> {
+ /// Bring this build's programs and maps up, or replace the content of the maps when they are
+ /// up already, without a program ever reading a mix of old and new entries. A first install,
+ /// a new build and a schema change are all this one call. `fill` writes the whole desired
+ /// content into fresh maps, and only once it returned do the programs holding them take over
+ /// the links, one link at a time. A failure inside `fill`, a refused entry included, leaves
+ /// what is pinned untouched. Nothing written into the live maps carries over, a caller that
+ /// wants some of it reads it through [`hash_map`](Self::hash_map) before the call, which
+ /// answers [`NotLoaded`] when nothing of this build is pinned. Pinned state of a newer schema
+ /// or of a newer version of the subsystem's crate is refused.
+ ///
+ /// Takes the guard exclusively, a map handle open across the exchange would write the
+ /// retired map.
+ pub fn replace_maps(
+ &mut self,
+ fill: impl FnOnce(&Staged<'_>) -> anyhow::Result<()>,
+ ) -> anyhow::Result<()> {
+ self.programs.replace_maps(fill)
+ }
+
+ /// Make the attached set match `desired`. Moving links onto new programs is
+ /// [`replace_maps`](Self::replace_maps)'s job. The live set cannot change under it, every
+ /// other writer waits on the exclusive lock. What may go is decided from the kernel's state.
+ ///
+ /// An interface that failed to attach fails the apply once every other was tried, since the
+ /// subsystem does not work on an interface without its program. One that vanished since it
+ /// was named is no failure, and a failed detach only leaves a program running longer and is
+ /// logged.
+ pub fn reconcile(&self, desired: &HashSet<u32>) -> anyhow::Result<()> {
+ self.programs.reconcile(desired)
+ }
+
+ /// Drop the pinned state of this subsystem, links included. Leaving links attached would keep
+ /// the programs running with nothing left to update them. The lock under `/run` stays,
+ /// unlinking it under a waiter would let that waiter run unserialized. Takes the guard
+ /// exclusively like [`replace_maps`](Self::replace_maps).
+ pub fn clear(&mut self) -> anyhow::Result<()> {
+ self.programs.tear_down()
+ }
+}
+
+/// The fresh maps of a [`replace_maps`](ApplyLock::replace_maps), seen only by the fill that
+/// writes them.
+pub struct Staged<'a> {
+ programs: &'a TcPrograms,
+}
+
+impl Staged<'_> {
+ /// Open one of the staged maps by name, empty until the fill writes it.
+ pub fn hash_map<K: aya::Pod, V: aya::Pod>(
+ &self,
+ name: &str,
+ ) -> anyhow::Result<MapHandle<'_, K, V>> {
+ Ok(MapHandle {
+ map: self
+ .programs
+ .hash_map_in(&self.programs.staging_dir(), name)?,
+ _opened_through: PhantomData,
+ })
+ }
+}
+
+/// A pinned hash map. It cannot outlive the lock or the fill it was opened under, so every write
+/// to it runs under the lock and goes into the maps the programs use.
+pub struct MapHandle<'a, K, V> {
+ map: aya::maps::HashMap<aya::maps::MapData, K, V>,
+ _opened_through: PhantomData<&'a ()>,
+}
+
+impl<K, V> Deref for MapHandle<'_, K, V> {
+ type Target = aya::maps::HashMap<aya::maps::MapData, K, V>;
+
+ fn deref(&self) -> &Self::Target {
+ &self.map
+ }
+}
+
+impl<K, V> DerefMut for MapHandle<'_, K, V> {
+ fn deref_mut(&mut self) -> &mut Self::Target {
+ &mut self.map
+ }
+}
diff --git a/proxmox-ebpf/src/tc.rs b/proxmox-ebpf/src/tc.rs
index 1217703e8906..09a02b2947cc 100644
--- a/proxmox-ebpf/src/tc.rs
+++ b/proxmox-ebpf/src/tc.rs
@@ -10,14 +10,14 @@ use aya::{
programs::{
ProgramError, SchedClassifier, TcAttachType,
links::{FdLink, LinkError, LinkOrder, PinnedLink},
- tc::TcAttachOptions,
+ tc::{SchedClassifierLink, TcAttachOptions},
},
};
/// Why a link step did not go through, with the kernel's own answer where it gives one, so a
/// caller can tell a lost race or a vanished interface from a real failure.
#[derive(Debug)]
-pub enum Error {
+pub(crate) enum Error {
/// A link of the program is in place already, another attach of the interface won the race.
AlreadyAttached,
/// The interface is gone, its index names no netdev anymore.
@@ -108,8 +108,11 @@ pub enum Direction {
}
impl Direction {
+ /// Both hooks.
+ pub(crate) const ALL: [Self; 2] = [Self::Ingress, Self::Egress];
+
/// The lowercase name, as it stands in pin file names.
- pub fn as_str(self) -> &'static str {
+ pub const fn as_str(self) -> &'static str {
match self {
Self::Ingress => "ingress",
Self::Egress => "egress",
@@ -182,7 +185,7 @@ impl LinkPin {
/// verdict ends the chain right there. So a subsystem's program returns `TC_ACT_UNSPEC` for
/// whatever it leaves alone. Programs of several subsystems run in attach order, each new link
/// goes last.
-pub fn attach_and_pin(
+pub(crate) fn attach_and_pin(
prog: &mut SchedClassifier,
pin: &LinkPin,
pin_path: &Path,
@@ -210,18 +213,23 @@ pub fn attach_and_pin(
Ok(())
}
-/// Rebind a pinned link to `prog` via LINK_UPDATE. Atomic, traffic sees no detach/reattach gap.
-pub fn swap_pinned_link(prog: &mut SchedClassifier, pin_path: &Path) -> Result<(), Error> {
- let pinned = PinnedLink::from_pin(pin_path)?;
- let fd_link: FdLink = pinned.into();
- let link = fd_link.try_into()?;
+pub(crate) fn open_pinned_link(pin_path: &Path) -> Result<SchedClassifierLink, Error> {
+ let fd_link: FdLink = PinnedLink::from_pin(pin_path)?.into();
+ Ok(fd_link.try_into()?)
+}
+
+/// Rebind `link` to `prog` via LINK_UPDATE. Atomic, traffic sees no detach/reattach gap.
+pub(crate) fn swap_link(
+ prog: &mut SchedClassifier,
+ link: SchedClassifierLink,
+) -> Result<(), Error> {
let new_id = prog.attach_to_link(link)?;
// take the handle out of aya's internal tracking, we have the pin
let _ = prog.take_link(new_id)?;
Ok(())
}
-pub fn detach_pinned_link(pin_path: &Path) -> Result<(), Error> {
+pub(crate) fn detach_pinned_link(pin_path: &Path) -> Result<(), Error> {
let pinned = PinnedLink::from_pin(pin_path)?;
let _fd_link = pinned.unpin()?;
Ok(())
@@ -229,7 +237,7 @@ pub fn detach_pinned_link(pin_path: &Path) -> Result<(), Error> {
/// The name of an interface by index, none when it is gone. Any other failure says nothing
/// about the interface and is not taken as its absence, so the lookup reads errno itself.
-pub fn iface_name(ifindex: u32) -> anyhow::Result<Option<String>> {
+pub(crate) fn iface_name(ifindex: u32) -> anyhow::Result<Option<String>> {
let mut buf = [0u8; nix::libc::IF_NAMESIZE];
// SAFETY: the buffer holds IF_NAMESIZE bytes, the most the call writes
let name = unsafe { nix::libc::if_indextoname(ifindex, buf.as_mut_ptr().cast()) };
@@ -247,7 +255,7 @@ pub fn iface_name(ifindex: u32) -> anyhow::Result<Option<String>> {
/// The pins in `links_dir`, see [`LinkPin`], none for a missing directory. A file that is not a
/// pin is skipped.
-pub fn read_pinned_links(links_dir: &Path) -> anyhow::Result<Vec<LinkPin>> {
+pub(crate) fn read_pinned_links(links_dir: &Path) -> anyhow::Result<Vec<LinkPin>> {
let mut out = Vec::new();
let dir = match std::fs::read_dir(links_dir) {
Ok(d) => d,
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH proxmox v8 4/6] ebpf: subsystem: place subsystem links in a fixed order
2026-11-04 17:29 [PATCH proxmox v8 0/6] add proxmox-ebpf library Hannes Laimer
` (2 preceding siblings ...)
2026-11-04 17:29 ` [PATCH proxmox v8 3/6] ebpf: subsystem: add the shared tc program handling Hannes Laimer
@ 2026-11-04 17:29 ` Hannes Laimer
2026-11-06 13:30 ` Wolfgang Bumiller
2026-11-04 17:29 ` [PATCH proxmox v8 5/6] ebpf: tests: add a native harness for the BPF C programs Hannes Laimer
2026-11-04 17:29 ` [PATCH proxmox v8 6/6] ebpf: debian: package the crate as a Rust library Hannes Laimer
5 siblings, 1 reply; 9+ messages in thread
From: Hannes Laimer @ 2026-11-04 17:29 UTC (permalink / raw)
To: pve-devel
tcx runs the programs on an interface in the order their links were
attached, so without a fixed order the subsystem that sees a packet
first is whichever attached there first. The run order is a list in
the lib. An attach places its link relative to the links the other
subsystems hold on that interface, found through their pins, under
one lock for all subsystems so two attaches to one interface see each
other. Swaps keep a link's slot.
A name missing from the list fails the build. A full apply writes
its list to the file under /run when an older crate wrote the file,
and adds the names the file lacks otherwise. Every attach places by
that file. So the newest crate decides the order, a build can place
its link against a subsystem it was not built with, and attached
links stay where they are.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
proxmox-ebpf/src/subsystem.rs | 443 +++++++++++++++++++++++++++++++++-
proxmox-ebpf/src/tc.rs | 14 +-
2 files changed, 445 insertions(+), 12 deletions(-)
diff --git a/proxmox-ebpf/src/subsystem.rs b/proxmox-ebpf/src/subsystem.rs
index 39272060cd4e..274c11fdb22a 100644
--- a/proxmox-ebpf/src/subsystem.rs
+++ b/proxmox-ebpf/src/subsystem.rs
@@ -15,10 +15,16 @@
//! single-interface step shared. The holder reads the input of its change under the lock and
//! writes before it lets go, so a full apply and every step reach the kernel in the order they
//! read their input. Two steps of one interface do not order each other.
+//!
+//! The programs of different subsystems on one interface run in the order of [`RUN_ORDER`]. A
+//! full apply writes the list to a file under `/run`, and an attach places its link among the
+//! links the other subsystems hold on that interface by the list written last, so a build places
+//! right against subsystems it was not built with. It finds those links through their pins and
+//! does that under a lock all subsystems share, so each attach sees the links pinned before it.
use std::{
collections::HashSet,
fs::File,
- io::ErrorKind,
+ io::{ErrorKind, Write},
marker::PhantomData,
ops::{Deref, DerefMut},
os::unix::fs::{DirBuilderExt, OpenOptionsExt},
@@ -27,7 +33,10 @@ use std::{
};
use anyhow::Context;
-use aya::{EbpfLoader, programs::SchedClassifier};
+use aya::{
+ EbpfLoader,
+ programs::{SchedClassifier, links::LinkOrder},
+};
use nix::{
errno::Errno,
fcntl::{Flock, FlockArg, RenameFlags, renameat2},
@@ -37,6 +46,39 @@ use crate::tc::{self, Direction};
const PIN_ROOT: &str = "/sys/fs/bpf/proxmox-ebpf";
const RUN_ROOT: &str = "/run/proxmox-ebpf";
+const ORDER_FILE: &str = "order";
+
+/// The subsystems in the order their programs run on an interface, the first runs first. A name
+/// missing from the list fails the build. The newest build of the crate on a host decides the
+/// order, a new name takes effect with its next full apply, a moved or removed one for the links
+/// attached after that, since already attached links stay where they are.
+pub const RUN_ORDER: &[&str] = &[];
+
+const fn check_listed(name: &str) {
+ let mut i = 0;
+ while i < RUN_ORDER.len() {
+ if str_eq(RUN_ORDER[i], name) {
+ return;
+ }
+ i += 1;
+ }
+ panic!("subsystem not in RUN_ORDER");
+}
+
+const fn str_eq(a: &str, b: &str) -> bool {
+ let (a, b) = (a.as_bytes(), b.as_bytes());
+ if a.len() != b.len() {
+ return false;
+ }
+ let mut i = 0;
+ while i < a.len() {
+ if a[i] != b[i] {
+ return false;
+ }
+ i += 1;
+ }
+ true
+}
// FNV-1a over the object, the program selection and the map names, so the same object with
// another program chosen for a direction is another build
@@ -81,7 +123,10 @@ const fn version_part(s: &str) -> u64 {
}
/// A subsystem's [`TcPrograms`] from its name. The object is the build's `<name>.bpf.o`, the
-/// fingerprint follows from the bytes and the version is the crate's own.
+/// fingerprint follows from the bytes and the selection and the version is the crate's own.
+/// `programs` names the object's program per direction, one
+/// entry each, `maps` names its pinned maps, and `schema_version` goes up with any change to a
+/// map's key or value layout, so no build writes into maps of another layout.
#[macro_export]
macro_rules! tc_programs {
(
@@ -211,6 +256,7 @@ impl TcPrograms {
schema_version: u32,
version: [&'static str; 3],
) -> Self {
+ check_listed(name);
Self {
name,
obj,
@@ -239,7 +285,7 @@ impl TcPrograms {
self.schema_root().join("staging")
}
fn links_dir(&self) -> PathBuf {
- links_dir_of(self.name)
+ self.pin_root().join(LINKS_DIR)
}
fn link_pin_path(&self, pin: &tc::LinkPin) -> PathBuf {
self.links_dir().join(pin.filename())
@@ -362,6 +408,115 @@ impl TcPrograms {
})
}
+ // one lock for every subsystem. An attach holds it from reading the list and the neighbors to
+ // the pin of its link, so two attaches to one interface see each other, and the list is
+ // written under it
+ fn attach_lock(&self) -> anyhow::Result<Flock<File>> {
+ private_dir(Path::new(RUN_ROOT))
+ .with_context(|| format!("{}: create the run directory {RUN_ROOT}", self.name))?;
+ let path = Path::new(RUN_ROOT).join("attach.lock");
+ flock_within(&path, true, "attach lock", self.name)
+ }
+
+ // the links the other subsystems hold on the interface and direction of `pin`, with their
+ // places in `order`. A pin has the same file name in every subsystem, so finding one is a stat
+ // of that name in each of their links directories, which are never listed. A subsystem the
+ // list does not name cannot be placed against
+ fn neighbors(
+ &self,
+ pin: &tc::LinkPin,
+ order: &[String],
+ ) -> anyhow::Result<Vec<(usize, PathBuf)>> {
+ let mut found = Vec::new();
+ for (root, name) in
+ subsystem_roots().with_context(|| format!("{}: list the subsystems", self.name))?
+ {
+ if name == self.name {
+ continue;
+ }
+ let path = root.join(LINKS_DIR).join(pin.filename());
+ if !path.exists() {
+ continue;
+ }
+ match order.iter().position(|listed| *listed == name) {
+ Some(place) => found.push((place, path)),
+ None => log::warn!(
+ "{}: {name} is not in the order list, not placed against it",
+ self.name
+ ),
+ }
+ }
+ Ok(found)
+ }
+
+ // attach `prog` for `pin` in its place among the other subsystems' links, see `RUN_ORDER`,
+ // and pin it under `pin_path`
+ fn attach_link(
+ &self,
+ prog: &mut SchedClassifier,
+ pin: &tc::LinkPin,
+ pin_path: &Path,
+ ) -> Result<(), tc::Error> {
+ let _lock = self.attach_lock()?;
+ let (_, order) = read_order()?;
+ // under the lock pins only go away, other attaches wait on it. So a neighbor that cannot
+ // be placed against is dropped and the attach tries the rest without listing them again
+ let mut neighbors = self.neighbors(pin, &order)?;
+ let rank = match order.iter().position(|listed| listed.as_str() == self.name) {
+ Some(rank) => rank,
+ // only a list written by a build that dropped this subsystem lacks it
+ None => {
+ log::warn!("{}: not in the order list, attached last", self.name);
+ neighbors.clear();
+ 0
+ }
+ };
+ loop {
+ let (index, before) = match place(rank, &neighbors) {
+ Place::Before(i) => (Some(i), true),
+ Place::After(i) => (Some(i), false),
+ Place::Last => (None, false),
+ };
+ // the order holds only the neighbor's fd, the link stays open until the attach is done
+ let neighbor = match index {
+ Some(i) => match tc::open_pinned_link(&neighbors[i].1) {
+ Ok(link) => Some(link),
+ // unpinned since the lookup, its subsystem detached it
+ Err(tc::Error::PinGone) => {
+ neighbors.remove(i);
+ continue;
+ }
+ Err(e) => return Err(e),
+ },
+ None => None,
+ };
+ let placement = match (&neighbor, before) {
+ (Some(link), true) => LinkOrder::before_link(link)?,
+ (Some(link), false) => LinkOrder::after_link(link)?,
+ (None, _) => LinkOrder::last(),
+ };
+ let attached = tc::attach_and_pin(prog, pin, pin_path, placement);
+ let Some(i) = index else {
+ return attached;
+ };
+ match attached {
+ // a neighbor's pin may name a link whose interface is gone under a reused index,
+ // nothing can be placed against that one until its subsystem reclaims it. The
+ // program is better attached out of order than not at all
+ Err(tc::Error::NotAttached) => {
+ log::warn!(
+ "{}: {} not placed against {}, that link is not attached",
+ self.name,
+ pin.filename(),
+ neighbors[i].1.display()
+ );
+ neighbors.remove(i);
+ }
+ result => return result,
+ }
+ }
+ }
+
/// Whether a link of the interface name is pinned, read without the lock. Most interfaces
/// never had a link and their unplug must not wait behind a full apply. A miss is a snapshot,
/// a pin arriving after it is another attach's. A failed listing counts as a hit.
@@ -451,6 +606,29 @@ impl TcPrograms {
.with_context(|| format!("{}: remove the pinned state", self.name))
}
+ // the list every attach places by, see `next_order`
+ fn write_order(&self) -> anyhow::Result<()> {
+ let _lock = self.attach_lock()?;
+ let Some((version, names)) = next_order(&read_order()?, self.version, RUN_ORDER) else {
+ return Ok(());
+ };
+ let path = Path::new(RUN_ROOT).join(ORDER_FILE);
+ let tmp = path.with_extension("tmp");
+ let mut file = File::options()
+ .write(true)
+ .create(true)
+ .truncate(true)
+ .mode(0o600)
+ .open(&tmp)
+ .with_context(|| format!("{}: write the order list", self.name))?;
+ let content = format!("{}\n{}\n", version_text(version), names.join("\n"));
+ file.write_all(content.as_bytes())
+ .with_context(|| format!("{}: write the order list", self.name))?;
+ std::fs::rename(&tmp, &path)
+ .with_context(|| format!("{}: move the order list into place", self.name))?;
+ Ok(())
+ }
+
// this build's object with all of its programs loaded and its maps created and pinned under
// `set`. Everything is loaded before any link is touched, so a verifier rejection of one
// direction leaves the old state whole
@@ -552,7 +730,6 @@ impl TcPrograms {
&self,
fill: impl FnOnce(&Staged<'_>) -> anyhow::Result<()>,
) -> anyhow::Result<()> {
- let others = self.other_schemas()?;
// programs pinned under a newer schema or by a newer version of the crate mean a newer
// build runs here and this process is the stale one, so it leaves the state alone
if let Some(newer) = self.newer_schema_installed()? {
@@ -570,6 +747,7 @@ impl TcPrograms {
version_text(self.version)
);
}
+ let others = self.other_schemas()?;
std::fs::create_dir_all(self.links_dir())
.with_context(|| format!("{}: create the links directory", self.name))?;
let staging = self.staging_dir();
@@ -716,6 +894,7 @@ impl TcPrograms {
}
fn reconcile(&self, desired: &HashSet<u32>) -> anyhow::Result<()> {
+ self.write_order()?;
// opened before anything is detached, so a missing program fails the full apply before it
// changes anything
let mut progs = Vec::new();
@@ -750,7 +929,7 @@ impl TcPrograms {
continue;
};
let pin = tc::LinkPin { ifindex, name, dir };
- match tc::attach_and_pin(prog, &pin, &self.link_pin_path(&pin)) {
+ match self.attach_link(prog, &pin, &self.link_pin_path(&pin)) {
Ok(()) => {}
Err(tc::Error::IfaceGone) => {
log::info!("{}: {ifindex} vanished before the attach", self.name);
@@ -828,7 +1007,7 @@ impl TcPrograms {
if path.exists() {
continue;
}
- match tc::attach_and_pin(prog, &pin, &path) {
+ match self.attach_link(prog, &pin, &path) {
Ok(()) => {}
Err(tc::Error::IfaceGone) => {
log::info!("{}: {ifindex} vanished before the attach", self.name);
@@ -848,8 +1027,94 @@ impl TcPrograms {
const LINKS_DIR: &str = "links";
-fn links_dir_of(subsystem: &str) -> PathBuf {
- PathBuf::from(PIN_ROOT).join(subsystem).join(LINKS_DIR)
+// the subsystem directories under the pin root, each with its name
+fn subsystem_roots() -> std::io::Result<Vec<(PathBuf, String)>> {
+ let entries = match std::fs::read_dir(PIN_ROOT) {
+ Ok(entries) => entries,
+ Err(e) if e.kind() == ErrorKind::NotFound => return Ok(Vec::new()),
+ Err(e) => return Err(e),
+ };
+ let mut roots = Vec::new();
+ for entry in entries {
+ let entry = entry?;
+ if !entry.file_type()?.is_dir() {
+ continue;
+ }
+ roots.push((
+ entry.path(),
+ entry.file_name().to_string_lossy().into_owned(),
+ ));
+ }
+ Ok(roots)
+}
+
+// the list under /run with the version of the crate that wrote it, empty and oldest until a full
+// apply wrote one
+fn read_order() -> anyhow::Result<([u64; 3], Vec<String>)> {
+ match std::fs::read_to_string(Path::new(RUN_ROOT).join(ORDER_FILE)) {
+ Ok(content) => Ok(parse_order(&content)),
+ Err(e) if e.kind() == ErrorKind::NotFound => Ok(([0, 0, 0], Vec::new())),
+ Err(e) => Err(e).context("read the order list"),
+ }
+}
+
+// the version on the first line, then one name per line. A file without a version line is an
+// older crate's and counts as oldest
+fn parse_order(content: &str) -> ([u64; 3], Vec<String>) {
+ let mut lines = content.lines().peekable();
+ let version = lines
+ .peek()
+ .and_then(|first| parse_version(first))
+ .inspect(|_| {
+ lines.next();
+ })
+ .unwrap_or([0, 0, 0]);
+ (version, lines.map(str::to_owned).collect())
+}
+
+fn parse_version(text: &str) -> Option<[u64; 3]> {
+ let mut parts = text.split('.').map(|part| part.parse::<u64>().ok());
+ let version = [parts.next()??, parts.next()??, parts.next()??];
+ parts.next().is_none().then_some(version)
+}
+
+// what a full apply writes, or nothing. The newest crate decides the order. A file written by
+// the same or a newer crate keeps its order and only gets the names it lacks. A file written by
+// an older crate is replaced by `list`, with the names only the file has kept in, each after
+// the name it followed in the file
+fn next_order(
+ written: &([u64; 3], Vec<String>),
+ version: [u64; 3],
+ list: &[&str],
+) -> Option<([u64; 3], Vec<String>)> {
+ let (written_version, names) = written;
+ if *written_version < version {
+ let base: Vec<String> = list.iter().map(|name| name.to_string()).collect();
+ let extra: Vec<&str> = names.iter().map(String::as_str).collect();
+ return Some((version, merge_order(&base, &extra).unwrap_or(base)));
+ }
+ merge_order(names, list).map(|merged| (*written_version, merged))
+}
+
+// the written list with the names of `list` it lacks added, each after the listed name it
+// follows, or none when it lacks none. The written names keep their order and a name the list
+// dropped stays, so a move or a removal waits for the reboot
+fn merge_order(written: &[String], list: &[&str]) -> Option<Vec<String>> {
+ let mut merged: Vec<String> = written.to_vec();
+ let mut added = false;
+ for (i, name) in list.iter().enumerate() {
+ if merged.iter().any(|m| m == name) {
+ continue;
+ }
+ let after = list[..i]
+ .iter()
+ .rev()
+ .find_map(|before| merged.iter().position(|m| m == before));
+ let at = after.map_or(0, |pos| pos + 1);
+ merged.insert(at, name.to_string());
+ added = true;
+ }
+ added.then_some(merged)
}
// the name of a build's program directory, its version and fingerprint, with colons for the
@@ -890,6 +1155,31 @@ fn remove_dir_if_present(dir: &Path) -> std::io::Result<()> {
}
}
+// where a new link goes among the neighbors' links, the index names the neighbor
+enum Place {
+ Before(usize),
+ After(usize),
+ Last,
+}
+
+// before the earliest neighbor that runs later, otherwise after the latest one that runs
+// earlier. Each attach inserts into a chain sorted this way, so the chain stays sorted
+fn place(rank: usize, neighbors: &[(usize, PathBuf)]) -> Place {
+ let ranked = neighbors.iter().map(|(r, _)| *r).enumerate();
+ let later = ranked
+ .clone()
+ .filter(|&(_, r)| r > rank)
+ .min_by_key(|&(_, r)| r);
+ if let Some((i, _)) = later {
+ return Place::Before(i);
+ }
+ let earlier = ranked.filter(|&(_, r)| r < rank).max_by_key(|&(_, r)| r);
+ match earlier {
+ Some((i, _)) => Place::After(i),
+ None => Place::Last,
+ }
+}
+
// a program pin is a file under a build directory. A run killed between creating that
// directory and pinning leaves it empty. Like the links, a directory that cannot be read may
// hold anything, a missing one is empty
@@ -1065,3 +1355,138 @@ impl<K, V> DerefMut for MapHandle<'_, K, V> {
&mut self.map
}
}
+
+#[cfg(test)]
+mod test {
+ use super::*;
+
+ fn placed(rank: usize, ranks: &[usize]) -> String {
+ let neighbors: Vec<(usize, PathBuf)> = ranks
+ .iter()
+ .map(|&r| (r, PathBuf::from(format!("link-{r}"))))
+ .collect();
+ match place(rank, &neighbors) {
+ Place::Before(i) => format!("before {}", neighbors[i].1.display()),
+ Place::After(i) => format!("after {}", neighbors[i].1.display()),
+ Place::Last => "last".into(),
+ }
+ }
+
+ #[test]
+ fn a_link_goes_before_the_earliest_later_neighbor() {
+ assert_eq!(placed(1, &[3, 2]), "before link-2");
+ assert_eq!(placed(1, &[0, 3, 2]), "before link-2");
+ }
+
+ #[test]
+ fn a_link_goes_after_the_latest_earlier_neighbor() {
+ assert_eq!(placed(2, &[0, 1]), "after link-1");
+ assert_eq!(placed(3, &[1, 0]), "after link-1");
+ }
+
+ #[test]
+ fn a_link_without_neighbors_goes_last() {
+ assert_eq!(placed(1, &[]), "last");
+ }
+
+ #[test]
+ fn a_fingerprint_changes_with_the_program_selection() {
+ let obj = b"the same object";
+ let a = build_fingerprint(obj, &[(Direction::Ingress, "a")], &["m"]);
+ assert_ne!(
+ a,
+ build_fingerprint(obj, &[(Direction::Egress, "a")], &["m"])
+ );
+ assert_ne!(
+ a,
+ build_fingerprint(obj, &[(Direction::Ingress, "b")], &["m"])
+ );
+ assert_ne!(
+ a,
+ build_fingerprint(obj, &[(Direction::Ingress, "a")], &["n"])
+ );
+ assert_eq!(
+ a,
+ build_fingerprint(obj, &[(Direction::Ingress, "a")], &["m"])
+ );
+ }
+
+ #[test]
+ fn a_build_directory_name_round_trips() {
+ let name = build_dir_name([0, 9, 12], 0x0123456789abcdef);
+ assert_eq!(name, "0:9:12-0123456789abcdef");
+ assert_eq!(
+ parse_build_dir_name(&name),
+ Some(([0, 9, 12], 0x0123456789abcdef))
+ );
+ assert_eq!(parse_build_dir_name("0123456789abcdef"), None);
+ assert_eq!(parse_build_dir_name("0.9.12-0123456789abcdef"), None);
+ assert!(parse_build_dir_name("0:10:0-0123456789abcdef") > parse_build_dir_name(&name));
+ }
+
+ fn written(names: &[&str]) -> Vec<String> {
+ names.iter().map(|&n| n.to_owned()).collect()
+ }
+
+ #[test]
+ fn a_written_list_with_every_name_stays_whatever_its_order() {
+ assert_eq!(merge_order(&written(&["a", "b"]), &["a", "b"]), None);
+ assert_eq!(merge_order(&written(&["a", "b"]), &["b", "a"]), None);
+ assert_eq!(merge_order(&written(&["b", "a", "x"]), &["a", "b"]), None);
+ }
+
+ #[test]
+ fn the_order_file_starts_with_the_version_of_its_writer() {
+ assert_eq!(
+ parse_order("0.1.2\na\nb\n"),
+ ([0, 1, 2], written(&["a", "b"]))
+ );
+ assert_eq!(parse_order("a\nb\n"), ([0, 0, 0], written(&["a", "b"])));
+ assert_eq!(parse_order(""), ([0, 0, 0], written(&[])));
+ }
+
+ #[test]
+ fn the_newest_crate_decides_the_order() {
+ let file = ([0, 1, 0], written(&["a", "b", "z"]));
+ // the same or a newer crate only adds what the file lacks
+ assert_eq!(next_order(&file, [0, 1, 0], &["b", "a"]), None);
+ assert_eq!(
+ next_order(&file, [0, 0, 9], &["x", "a"]),
+ Some(([0, 1, 0], written(&["x", "a", "b", "z"])))
+ );
+ // a newer crate writes its own order, the names only the file has follow theirs
+ assert_eq!(
+ next_order(&file, [0, 2, 0], &["b", "a"]),
+ Some(([0, 2, 0], written(&["b", "z", "a"])))
+ );
+ assert_eq!(
+ next_order(&([0, 0, 0], written(&[])), [0, 2, 0], &["a"]),
+ Some(([0, 2, 0], written(&["a"])))
+ );
+ }
+
+ #[test]
+ fn a_new_name_goes_in_after_the_listed_name_it_follows() {
+ assert_eq!(
+ merge_order(&written(&[]), &["a", "b"]),
+ Some(written(&["a", "b"]))
+ );
+ assert_eq!(
+ merge_order(&written(&["a", "b"]), &["x", "a", "b"]),
+ Some(written(&["x", "a", "b"]))
+ );
+ assert_eq!(
+ merge_order(&written(&["a", "b"]), &["a", "x", "y", "b"]),
+ Some(written(&["a", "x", "y", "b"]))
+ );
+ // the written names keep their order, a dropped one stays
+ assert_eq!(
+ merge_order(&written(&["a", "b"]), &["b", "x", "a"]),
+ Some(written(&["a", "b", "x"]))
+ );
+ assert_eq!(
+ merge_order(&written(&["a", "b", "c"]), &["a", "x", "c"]),
+ Some(written(&["a", "x", "b", "c"]))
+ );
+ }
+}
diff --git a/proxmox-ebpf/src/tc.rs b/proxmox-ebpf/src/tc.rs
index 09a02b2947cc..3c6ea54908d0 100644
--- a/proxmox-ebpf/src/tc.rs
+++ b/proxmox-ebpf/src/tc.rs
@@ -24,6 +24,9 @@ pub(crate) enum Error {
IfaceGone,
/// The pin was gone by the time it was opened or removed, another attach or unplug took it.
PinGone,
+ /// The link the call referred to is not attached to the interface, the neighbor of a
+ /// placement or the link of a swap.
+ NotAttached,
Other(anyhow::Error),
}
@@ -33,6 +36,7 @@ impl std::fmt::Display for Error {
Self::AlreadyAttached => f.write_str("link already in place"),
Self::IfaceGone => f.write_str("interface is gone"),
Self::PinGone => f.write_str("pin is gone"),
+ Self::NotAttached => f.write_str("link not attached"),
Self::Other(e) if f.alternate() => write!(f, "{e:#}"),
Self::Other(e) => write!(f, "{e}"),
}
@@ -61,6 +65,9 @@ impl From<ProgramError> for Error {
ProgramError::SyscallError(s) if s.io_error.kind() == ErrorKind::AlreadyExists => {
Self::AlreadyAttached
}
+ ProgramError::SyscallError(s) if s.io_error.kind() == ErrorKind::NotFound => {
+ Self::NotAttached
+ }
e => Self::Other(e.into()),
}
}
@@ -183,18 +190,19 @@ impl LinkPin {
/// filters on the interface's qdiscs, such as the `ingress` qdisc the rate limiter installs.
/// `TC_ACT_UNSPEC` hands a packet on to the next program and then to the filters. Any other
/// verdict ends the chain right there. So a subsystem's program returns `TC_ACT_UNSPEC` for
-/// whatever it leaves alone. Programs of several subsystems run in attach order, each new link
-/// goes last.
+/// whatever it leaves alone. `order` is the link's place among the links already there, see
+/// [`RUN_ORDER`](crate::subsystem::RUN_ORDER).
pub(crate) fn attach_and_pin(
prog: &mut SchedClassifier,
pin: &LinkPin,
pin_path: &Path,
+ order: LinkOrder,
) -> Result<(), Error> {
let link_id = prog
.attach_with_options(
&pin.name,
pin.dir.aya_type(),
- TcAttachOptions::TcxOrder(LinkOrder::last()),
+ TcAttachOptions::TcxOrder(order),
)
.map_err(|e| match Error::from(e) {
// the interface can go away between the name lookup and the attach
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH proxmox v8 5/6] ebpf: tests: add a native harness for the BPF C programs
2026-11-04 17:29 [PATCH proxmox v8 0/6] add proxmox-ebpf library Hannes Laimer
` (3 preceding siblings ...)
2026-11-04 17:29 ` [PATCH proxmox v8 4/6] ebpf: subsystem: place subsystem links in a fixed order Hannes Laimer
@ 2026-11-04 17:29 ` Hannes Laimer
2026-11-04 17:29 ` [PATCH proxmox v8 6/6] ebpf: debian: package the crate as a Rust library Hannes Laimer
5 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2026-11-04 17:29 UTC (permalink / raw)
To: pve-devel
The programs' packet parsing and rewriting is plain C whose behavior
does not depend on the compilation target. So the build compiles each
one a second time natively, against shim headers that turn the
helpers into plain extern functions. A test hands a program a crafted
frame and asserts on the rewritten bytes.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
proxmox-ebpf/build.rs | 85 ++++++++
proxmox-ebpf/src/bpf-shim/bpf/bpf_helpers.h | 34 ++++
proxmox-ebpf/src/bpf-shim/bpf_debug.h | 10 +
proxmox-ebpf/src/bpf-shim/linux/bpf.h | 25 +++
proxmox-ebpf/tests/bpf/fixture.bpf.c | 37 ++++
proxmox-ebpf/tests/common/mod.rs | 212 ++++++++++++++++++++
proxmox-ebpf/tests/harness.rs | 84 ++++++++
7 files changed, 487 insertions(+)
create mode 100644 proxmox-ebpf/src/bpf-shim/bpf/bpf_helpers.h
create mode 100644 proxmox-ebpf/src/bpf-shim/bpf_debug.h
create mode 100644 proxmox-ebpf/src/bpf-shim/linux/bpf.h
create mode 100644 proxmox-ebpf/tests/bpf/fixture.bpf.c
create mode 100644 proxmox-ebpf/tests/common/mod.rs
create mode 100644 proxmox-ebpf/tests/harness.rs
diff --git a/proxmox-ebpf/build.rs b/proxmox-ebpf/build.rs
index 9cb143d955a8..e6630ec01d1d 100644
--- a/proxmox-ebpf/build.rs
+++ b/proxmox-ebpf/build.rs
@@ -18,6 +18,9 @@ fn main() {
println!("cargo:rerun-if-changed=build.rs");
println!("cargo:rerun-if-env-changed=CARGO_FEATURE_BPF_DEBUG");
+ let shim_include = src_dir.join("bpf-shim");
+ let mut native_objs = Vec::new();
+
for sub_entry in std::fs::read_dir(&src_dir).expect("read src/") {
let sub_path = sub_entry.expect("read src/ entry").path();
let bpf_dir = sub_path.join("bpf");
@@ -46,8 +49,90 @@ fn main() {
continue;
}
compile_bpf(&path, &bpf_dir, &global_include, &out_dir, bpf_debug);
+ native_objs.push(compile_native(
+ &path,
+ &bpf_dir,
+ &global_include,
+ &shim_include,
+ &out_dir,
+ ));
}
}
+
+ // the harness's own program, compiled both ways like a subsystem's, so every build runs the
+ // BPF compile and the shim through clang and the harness tests run a program without a
+ // subsystem
+ let fixture_dir = manifest_dir.join("tests").join("bpf");
+ println!("cargo:rerun-if-changed={}", fixture_dir.display());
+ for entry in std::fs::read_dir(&fixture_dir).expect("read tests/bpf") {
+ let path = entry.expect("read tests/bpf entry").path();
+ let is_program = path
+ .file_name()
+ .and_then(|s| s.to_str())
+ .is_some_and(|s| s.ends_with(".bpf.c"));
+ if is_program {
+ compile_bpf(&path, &fixture_dir, &global_include, &out_dir, bpf_debug);
+ native_objs.push(compile_native(
+ &path,
+ &fixture_dir,
+ &global_include,
+ &shim_include,
+ &out_dir,
+ ));
+ }
+ }
+
+ // archived so a member is only linked when a test references its symbols, the regular
+ // build neither grows nor needs the mocked helpers
+ let archive = out_dir.join("libbpf_native.a");
+ let _ = std::fs::remove_file(&archive);
+ let status = Command::new("ar")
+ .arg("rcs")
+ .arg(&archive)
+ .args(&native_objs)
+ .status()
+ .expect("failed to invoke ar");
+ assert!(
+ status.success(),
+ "ar failed to create {}",
+ archive.display()
+ );
+ println!("cargo:rustc-link-search=native={}", out_dir.display());
+ println!("cargo:rustc-link-lib=static=bpf_native");
+}
+
+// native build against the shim headers, so tests call the program as a plain function
+fn compile_native(
+ src: &Path,
+ local_include: &Path,
+ global_include: &Path,
+ shim_include: &Path,
+ out_dir: &Path,
+) -> PathBuf {
+ let stem = src.file_stem().unwrap().to_str().unwrap();
+ let obj_path = out_dir.join(format!("{stem}.native.o"));
+
+ let status = Command::new(CLANG)
+ .args(["-O2", "-g", "-Wall", "-fPIC"])
+ // shim first, it shadows linux/bpf.h and bpf_helpers.h
+ .arg("-I")
+ .arg(shim_include)
+ .arg("-I")
+ .arg(global_include)
+ .arg("-I")
+ .arg(local_include)
+ .arg("-c")
+ .arg(src)
+ .arg("-o")
+ .arg(&obj_path)
+ .status()
+ .unwrap_or_else(|e| panic!("failed to invoke {CLANG}: {e}"));
+ assert!(
+ status.success(),
+ "clang failed to compile {} natively",
+ src.display()
+ );
+ obj_path
}
fn multiarch() -> String {
diff --git a/proxmox-ebpf/src/bpf-shim/bpf/bpf_helpers.h b/proxmox-ebpf/src/bpf-shim/bpf/bpf_helpers.h
new file mode 100644
index 000000000000..b4bcfdf5717f
--- /dev/null
+++ b/proxmox-ebpf/src/bpf-shim/bpf/bpf_helpers.h
@@ -0,0 +1,34 @@
+#ifndef PROXMOX_EBPF_SHIM_BPF_HELPERS_H
+#define PROXMOX_EBPF_SHIM_BPF_HELPERS_H
+
+// Native stand-in for <bpf/bpf_helpers.h>. The helpers become plain extern
+// functions the test harness provides. Sections reduce to weak linkage so the
+// program objects can be linked into one test binary. Weak symbols of one
+// name link silently, so program and map names must stay distinct across
+// subsystems. The map definition macros keep their libbpf shapes, which are
+// plain C already.
+
+#include <linux/types.h>
+
+#define SEC(name) __attribute__((weak))
+#ifndef __always_inline
+#define __always_inline inline __attribute__((always_inline))
+#endif
+#define __uint(name, val) int(*name)[val]
+#define __type(name, val) typeof(val) *name
+
+#ifndef barrier_var
+#define barrier_var(var) asm volatile("" : "+r"(var))
+#endif
+
+// the value is irrelevant natively
+#define LIBBPF_PIN_BY_NAME 1
+
+extern void *bpf_map_lookup_elem(void *map, const void *key);
+extern long bpf_skb_load_bytes(const void *skb, __u32 offset, void *to, __u32 len);
+extern long bpf_skb_store_bytes(void *skb, __u32 offset, const void *from, __u32 len, __u64 flags);
+extern long bpf_skb_pull_data(void *skb, __u32 len);
+extern long bpf_skb_change_tail(void *skb, __u32 new_len, __u64 flags);
+extern long bpf_redirect(__u32 ifindex, __u64 flags);
+
+#endif
diff --git a/proxmox-ebpf/src/bpf-shim/bpf_debug.h b/proxmox-ebpf/src/bpf-shim/bpf_debug.h
new file mode 100644
index 000000000000..a7e5eac0d3b3
--- /dev/null
+++ b/proxmox-ebpf/src/bpf-shim/bpf_debug.h
@@ -0,0 +1,10 @@
+#ifndef PROXMOX_EBPF_SHIM_BPF_DEBUG_H
+#define PROXMOX_EBPF_SHIM_BPF_DEBUG_H
+
+// Native stand-in for bpf_debug.h, debug printing is a no-op in tests.
+
+#define DBG(...) \
+ do { \
+ } while (0)
+
+#endif
diff --git a/proxmox-ebpf/src/bpf-shim/linux/bpf.h b/proxmox-ebpf/src/bpf-shim/linux/bpf.h
new file mode 100644
index 000000000000..ff259c329522
--- /dev/null
+++ b/proxmox-ebpf/src/bpf-shim/linux/bpf.h
@@ -0,0 +1,25 @@
+#ifndef PROXMOX_EBPF_SHIM_LINUX_BPF_H
+#define PROXMOX_EBPF_SHIM_LINUX_BPF_H
+
+// Native stand-in for <linux/bpf.h>, only what the programs touch. The other UAPI headers
+// compile natively as they are.
+
+#include <linux/types.h>
+
+// data and data_end are wide enough for native pointers, keep in sync with SkBuff in
+// tests/common/mod.rs
+struct __sk_buff {
+ unsigned long data;
+ unsigned long data_end;
+ __u32 len;
+ __u32 ifindex;
+ __u32 mark;
+};
+
+// map-definition constants, the values are irrelevant natively
+enum {
+ BPF_MAP_TYPE_HASH = 1,
+};
+#define BPF_F_NO_PREALLOC 1
+
+#endif
diff --git a/proxmox-ebpf/tests/bpf/fixture.bpf.c b/proxmox-ebpf/tests/bpf/fixture.bpf.c
new file mode 100644
index 000000000000..67b4fd4a38ad
--- /dev/null
+++ b/proxmox-ebpf/tests/bpf/fixture.bpf.c
@@ -0,0 +1,37 @@
+// The harness's own program, compiled like a subsystem's, its BPF object is not used. It touches
+// what a subsystem program touches, the skb fields, the helpers and a map, so the shim is run by
+// the harness tests themselves.
+#include <linux/bpf.h>
+#include <linux/pkt_cls.h>
+#include <bpf/bpf_endian.h>
+#include <bpf/bpf_helpers.h>
+
+struct {
+ __uint(type, BPF_MAP_TYPE_HASH);
+ __uint(max_entries, 16);
+ __type(key, __u32);
+ __type(value, __u8);
+} fixture_stamps SEC(".maps");
+
+// writes the interface's stamp and the frame length over the first two bytes and keeps the
+// bytes they replaced in the mark, accepts a frame of an interface without a stamp as it is
+SEC("tc")
+int fixture(struct __sk_buff *skb)
+{
+ void *data = (void *)(long)skb->data;
+ void *data_end = (void *)(long)skb->data_end;
+ if (data + 2 > data_end)
+ return TC_ACT_OK;
+ __u8 *stamp = bpf_map_lookup_elem(&fixture_stamps, &skb->ifindex);
+ if (!stamp)
+ return TC_ACT_OK;
+ __u8 bytes[2];
+ if (bpf_skb_load_bytes(skb, 0, bytes, sizeof(bytes)))
+ return TC_ACT_OK;
+ skb->mark = bpf_ntohs(*(__be16 *)bytes);
+ bytes[0] = *stamp;
+ bytes[1] = skb->len;
+ if (bpf_skb_store_bytes(skb, 0, bytes, sizeof(bytes), 0))
+ return TC_ACT_OK;
+ return TC_ACT_UNSPEC;
+}
diff --git a/proxmox-ebpf/tests/common/mod.rs b/proxmox-ebpf/tests/common/mod.rs
new file mode 100644
index 000000000000..ad03710fb787
--- /dev/null
+++ b/proxmox-ebpf/tests/common/mod.rs
@@ -0,0 +1,212 @@
+//! Native harness for the subsystems' BPF C programs. A test declares a program's symbol and
+//! calls it like a plain function on a fake skb, with the helpers as bounds-checked functions
+//! over its buffer and a per-thread map registry. Nothing here touches the kernel. A test of
+//! the pinned state or the links needs root and a kernel, and runs as an e2e test on a node.
+
+// every test crate compiles the harness and uses its own part of it
+#![allow(dead_code)]
+
+// the archive is linked through the lib, a test crate that never uses the lib would drop it from
+// the link together with the program symbols
+use proxmox_ebpf as _;
+
+use std::cell::RefCell;
+use std::collections::HashMap;
+use std::ffi::{c_int, c_long, c_void};
+
+pub const TC_ACT_UNSPEC: c_int = -1;
+pub const TC_ACT_OK: c_int = 0;
+pub const TC_ACT_REDIRECT: c_int = 7;
+
+pub const PKT_CAP: usize = 2048;
+
+/// Keep in sync with struct __sk_buff in src/bpf-shim/linux/bpf.h.
+#[repr(C)]
+pub struct SkBuff {
+ pub data: usize,
+ pub data_end: usize,
+ pub len: u32,
+ pub ifindex: u32,
+ pub mark: u32,
+}
+
+/// The skb handed to a program plus the buffer behind it. The helpers get the skb pointer and
+/// cast back to this, so the skb must stay the first field.
+#[repr(C)]
+pub struct TestSkb {
+ skb: SkBuff,
+ buf: Box<[u8; PKT_CAP]>,
+}
+
+impl TestSkb {
+ pub fn new(packet: &[u8], ifindex: u32) -> Self {
+ assert!(packet.len() <= PKT_CAP);
+ let mut buf = Box::new([0u8; PKT_CAP]);
+ buf[..packet.len()].copy_from_slice(packet);
+ let mut t = TestSkb {
+ skb: SkBuff {
+ data: 0,
+ data_end: 0,
+ len: packet.len() as u32,
+ ifindex,
+ mark: 0,
+ },
+ buf,
+ };
+ t.sync();
+ t
+ }
+
+ fn sync(&mut self) {
+ let base = self.buf.as_ptr() as usize;
+ self.skb.data = base;
+ self.skb.data_end = base + self.skb.len as usize;
+ }
+
+ pub fn packet(&self) -> &[u8] {
+ &self.buf[..self.skb.len as usize]
+ }
+
+ pub fn mark(&self) -> u32 {
+ self.skb.mark
+ }
+
+ pub fn run(&mut self, prog: unsafe extern "C" fn(*mut SkBuff) -> c_int) -> c_int {
+ REDIRECTED.with(|r| *r.borrow_mut() = None);
+ unsafe { prog((self as *mut TestSkb).cast()) }
+ }
+}
+
+struct MockMap {
+ key_size: usize,
+ entries: HashMap<Vec<u8>, Box<[u8]>>,
+}
+
+thread_local! {
+ static MAPS: RefCell<HashMap<usize, MockMap>> = RefCell::new(HashMap::new());
+ static REDIRECTED: RefCell<Option<(u32, u64)>> = const { RefCell::new(None) };
+}
+
+pub fn register_map(map: *const c_void, key_size: usize) {
+ MAPS.with(|maps| {
+ maps.borrow_mut().insert(
+ map as usize,
+ MockMap {
+ key_size,
+ entries: HashMap::new(),
+ },
+ )
+ });
+}
+
+pub fn map_insert(map: *const c_void, key: &[u8], value: &[u8]) {
+ MAPS.with(|maps| {
+ let mut maps = maps.borrow_mut();
+ let m = maps.get_mut(&(map as usize)).expect("map not registered");
+ assert_eq!(key.len(), m.key_size);
+ m.entries.insert(key.to_vec(), value.into());
+ });
+}
+
+/// The ifindex of the redirect the last run issued, if any.
+pub fn redirected() -> Option<u32> {
+ REDIRECTED.with(|r| r.borrow().map(|(ifindex, _)| ifindex))
+}
+
+/// The flags of the redirect the last run issued, zero sends out of the interface.
+pub fn redirect_flags() -> Option<u64> {
+ REDIRECTED.with(|r| r.borrow().map(|(_, flags)| flags))
+}
+
+unsafe fn testskb<'a>(skb: *mut c_void) -> &'a mut TestSkb {
+ unsafe { &mut *(skb as *mut TestSkb) }
+}
+
+#[unsafe(no_mangle)]
+pub extern "C" fn bpf_map_lookup_elem(map: *mut c_void, key: *const c_void) -> *mut c_void {
+ MAPS.with(|maps| {
+ let mut maps = maps.borrow_mut();
+ let Some(m) = maps.get_mut(&(map as usize)) else {
+ panic!("lookup on unregistered map");
+ };
+ let key = unsafe { std::slice::from_raw_parts(key as *const u8, m.key_size) };
+ match m.entries.get_mut(key) {
+ // the box's heap allocation stays put while the registry holds it
+ Some(v) => v.as_mut_ptr() as *mut c_void,
+ None => std::ptr::null_mut(),
+ }
+ })
+}
+
+const EFAULT: c_long = nix::libc::EFAULT as c_long;
+const ENOMEM: c_long = nix::libc::ENOMEM as c_long;
+const EINVAL: c_long = nix::libc::EINVAL as c_long;
+
+#[unsafe(no_mangle)]
+pub extern "C" fn bpf_skb_load_bytes(
+ skb: *mut c_void,
+ offset: u32,
+ to: *mut c_void,
+ len: u32,
+) -> c_long {
+ let t = unsafe { testskb(skb) };
+ let (offset, len) = (offset as usize, len as usize);
+ if offset + len > t.skb.len as usize {
+ // the kernel clears the destination of a failed read
+ unsafe { std::ptr::write_bytes(to as *mut u8, 0, len) };
+ return -EFAULT;
+ }
+ unsafe { std::ptr::copy_nonoverlapping(t.buf.as_ptr().add(offset), to as *mut u8, len) };
+ 0
+}
+
+#[unsafe(no_mangle)]
+pub extern "C" fn bpf_skb_store_bytes(
+ skb: *mut c_void,
+ offset: u32,
+ from: *const c_void,
+ len: u32,
+ _flags: u64,
+) -> c_long {
+ let t = unsafe { testskb(skb) };
+ let (offset, len) = (offset as usize, len as usize);
+ if offset + len > t.skb.len as usize {
+ return -EFAULT;
+ }
+ unsafe {
+ std::ptr::copy_nonoverlapping(from as *const u8, t.buf.as_mut_ptr().add(offset), len)
+ };
+ 0
+}
+
+#[unsafe(no_mangle)]
+pub extern "C" fn bpf_skb_pull_data(skb: *mut c_void, len: u32) -> c_long {
+ // the buffer is always linear, pulling within it is a no-op
+ let t = unsafe { testskb(skb) };
+ if len > t.skb.len { -ENOMEM } else { 0 }
+}
+
+#[unsafe(no_mangle)]
+pub extern "C" fn bpf_skb_change_tail(skb: *mut c_void, new_len: u32, _flags: u64) -> c_long {
+ let t = unsafe { testskb(skb) };
+ // the kernel refuses to cut into the headers it knows, which is the link header here.
+ // Growing stops at the harness buffer
+ let min_len = 14;
+ if new_len < min_len || new_len as usize > PKT_CAP {
+ return -EINVAL;
+ }
+ // like the kernel, grown room reads as zeros
+ let old = t.skb.len as usize;
+ if new_len as usize > old {
+ t.buf[old..new_len as usize].fill(0);
+ }
+ t.skb.len = new_len;
+ t.sync();
+ 0
+}
+
+#[unsafe(no_mangle)]
+pub extern "C" fn bpf_redirect(ifindex: u32, flags: u64) -> c_long {
+ REDIRECTED.with(|r| *r.borrow_mut() = Some((ifindex, flags)));
+ TC_ACT_REDIRECT as c_long
+}
diff --git a/proxmox-ebpf/tests/harness.rs b/proxmox-ebpf/tests/harness.rs
new file mode 100644
index 000000000000..470e0a1cd88b
--- /dev/null
+++ b/proxmox-ebpf/tests/harness.rs
@@ -0,0 +1,84 @@
+//! The harness's own tests, the frame, the helpers, the map registry and a program built
+//! against the shim.
+mod common;
+
+use std::ffi::{c_int, c_void};
+
+use common::*;
+
+unsafe extern "C" {
+ fn fixture(skb: *mut SkBuff) -> c_int;
+ static fixture_stamps: c_void;
+}
+
+unsafe extern "C" fn pass(_skb: *mut SkBuff) -> c_int {
+ TC_ACT_UNSPEC
+}
+
+unsafe extern "C" fn accept(_skb: *mut SkBuff) -> c_int {
+ TC_ACT_OK
+}
+
+#[test]
+fn skb_round_trips_the_frame() {
+ let frame = [1u8, 2, 3, 4];
+ let mut skb = TestSkb::new(&frame, 7);
+ assert_eq!(skb.run(pass), TC_ACT_UNSPEC);
+ assert_eq!(skb.run(accept), TC_ACT_OK);
+ assert_eq!(skb.packet(), &frame);
+ assert_eq!(redirected(), None);
+ assert_eq!(redirect_flags(), None);
+}
+
+#[test]
+fn helpers_read_and_write_the_frame() {
+ let mut skb = TestSkb::new(&[1, 2, 3, 4, 5, 6], 7);
+ let raw = (&mut skb as *mut TestSkb).cast::<c_void>();
+ let mut out = [0u8; 2];
+ assert_eq!(bpf_skb_load_bytes(raw, 2, out.as_mut_ptr().cast(), 2), 0);
+ assert_eq!(out, [3, 4]);
+ assert_eq!(
+ bpf_skb_store_bytes(raw, 4, [9u8, 9].as_ptr().cast(), 2, 0),
+ 0
+ );
+ assert_eq!(skb.packet(), &[1, 2, 3, 4, 9, 9]);
+ // past the frame, the kernel clears the destination on a failed load
+ assert_ne!(bpf_skb_load_bytes(raw, 5, out.as_mut_ptr().cast(), 2), 0);
+ assert_eq!(out, [0, 0]);
+ assert_ne!(
+ bpf_skb_store_bytes(raw, 5, [9u8, 9].as_ptr().cast(), 2, 0),
+ 0
+ );
+ assert_eq!(skb.packet(), &[1, 2, 3, 4, 9, 9]);
+}
+
+#[test]
+fn maps_answer_registered_entries() {
+ // any address serves as a map's identity, the program hands the map's address to the helper
+ let map = 0x1000 as *const c_void;
+ register_map(map, 4);
+ map_insert(map, &7u32.to_ne_bytes(), &[9, 8, 7]);
+ let key = 7u32.to_ne_bytes();
+ let hit = bpf_map_lookup_elem(map.cast_mut(), key.as_ptr().cast());
+ assert!(!hit.is_null());
+ assert_eq!(
+ unsafe { std::slice::from_raw_parts(hit as *const u8, 3) },
+ &[9, 8, 7]
+ );
+ let key = 8u32.to_ne_bytes();
+ assert!(bpf_map_lookup_elem(map.cast_mut(), key.as_ptr().cast()).is_null());
+}
+
+#[test]
+fn a_program_runs_against_the_shim() {
+ let stamps = &raw const fixture_stamps;
+ register_map(stamps, 4);
+ map_insert(stamps, &7u32.to_ne_bytes(), &[0xab]);
+ let mut skb = TestSkb::new(&[0x12, 0x34, 5, 6], 9);
+ assert_eq!(skb.run(fixture), TC_ACT_OK);
+ assert_eq!(skb.packet(), &[0x12, 0x34, 5, 6]);
+ let mut skb = TestSkb::new(&[0x12, 0x34, 5, 6], 7);
+ assert_eq!(skb.run(fixture), TC_ACT_UNSPEC);
+ assert_eq!(skb.packet(), &[0xab, 4, 5, 6]);
+ assert_eq!(skb.mark(), 0x1234);
+}
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH proxmox v8 6/6] ebpf: debian: package the crate as a Rust library
2026-11-04 17:29 [PATCH proxmox v8 0/6] add proxmox-ebpf library Hannes Laimer
` (4 preceding siblings ...)
2026-11-04 17:29 ` [PATCH proxmox v8 5/6] ebpf: tests: add a native harness for the BPF C programs Hannes Laimer
@ 2026-11-04 17:29 ` Hannes Laimer
5 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2026-11-04 17:29 UTC (permalink / raw)
To: pve-devel
The overlay in debian/ builds it through debcargo, like the other Rust
crates. The BPF programs are compiled by the build script, so building
against the crate pulls in clang-21, the libbpf headers and the
kernel's UAPI headers as well.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
proxmox-ebpf/Cargo.toml | 1 +
proxmox-ebpf/build.rs | 6 ++++
proxmox-ebpf/debian/changelog | 5 ++++
proxmox-ebpf/debian/control | 50 +++++++++++++++++++++++++++++++
proxmox-ebpf/debian/copyright | 18 +++++++++++
proxmox-ebpf/debian/debcargo.toml | 13 ++++++++
6 files changed, 93 insertions(+)
create mode 100644 proxmox-ebpf/debian/changelog
create mode 100644 proxmox-ebpf/debian/control
create mode 100644 proxmox-ebpf/debian/copyright
create mode 100644 proxmox-ebpf/debian/debcargo.toml
diff --git a/proxmox-ebpf/Cargo.toml b/proxmox-ebpf/Cargo.toml
index 4cfd0b3258ed..d4607d83e6ff 100644
--- a/proxmox-ebpf/Cargo.toml
+++ b/proxmox-ebpf/Cargo.toml
@@ -1,5 +1,6 @@
[package]
name = "proxmox-ebpf"
+description = "Proxmox VE eBPF"
version = "0.1.0"
authors.workspace = true
edition.workspace = true
diff --git a/proxmox-ebpf/build.rs b/proxmox-ebpf/build.rs
index e6630ec01d1d..5ac1df60bbcd 100644
--- a/proxmox-ebpf/build.rs
+++ b/proxmox-ebpf/build.rs
@@ -99,6 +99,12 @@ fn main() {
);
println!("cargo:rustc-link-search=native={}", out_dir.display());
println!("cargo:rustc-link-lib=static=bpf_native");
+ // the archive holds code compiled from the crate's C sources, dh-cargo lists that in the
+ // Built-Using of a package linking it
+ println!(
+ "dh-cargo:deb-built-using=bpf_native=1={}",
+ manifest_dir.display()
+ );
}
// native build against the shim headers, so tests call the program as a plain function
diff --git a/proxmox-ebpf/debian/changelog b/proxmox-ebpf/debian/changelog
new file mode 100644
index 000000000000..c816e8ecbfaa
--- /dev/null
+++ b/proxmox-ebpf/debian/changelog
@@ -0,0 +1,5 @@
+rust-proxmox-ebpf (0.1.0-1) trixie; urgency=medium
+
+ * Initial release.
+
+ -- Proxmox Support Team <support@proxmox.com> Wed, 02 Sep 2026 10:00:00 +0200
diff --git a/proxmox-ebpf/debian/control b/proxmox-ebpf/debian/control
new file mode 100644
index 000000000000..d51e260ef386
--- /dev/null
+++ b/proxmox-ebpf/debian/control
@@ -0,0 +1,50 @@
+Source: rust-proxmox-ebpf
+Section: rust
+Priority: optional
+Build-Depends: debhelper-compat (= 13),
+ dh-sequence-cargo
+Build-Depends-Arch: cargo:native <!nocheck>,
+ rustc:native (>= 1.88) <!nocheck>,
+ libstd-rust-dev <!nocheck>,
+ librust-anyhow-1+default-dev <!nocheck>,
+ librust-aya-0.13+default-dev <!nocheck>,
+ librust-log-0.4+default-dev (>= 0.4.17-~~) <!nocheck>,
+ librust-nix-0.29+default-dev <!nocheck>,
+ librust-nix-0.29+fs-dev <!nocheck>,
+ clang-21 <!nocheck>,
+ libbpf-dev <!nocheck>,
+ proxmox-kernel-libc-dev <!nocheck>
+Maintainer: Proxmox Support Team <support@proxmox.com>
+Standards-Version: 4.7.2
+Vcs-Git: git://git.proxmox.com/git/proxmox.git
+Vcs-Browser: https://git.proxmox.com/?p=proxmox.git
+Homepage: https://proxmox.com
+X-Cargo-Crate: proxmox-ebpf
+
+Package: librust-proxmox-ebpf-dev
+Architecture: any
+Multi-Arch: same
+Depends:
+ ${misc:Depends},
+ librust-anyhow-1+default-dev,
+ librust-aya-0.13+default-dev,
+ librust-log-0.4+default-dev (>= 0.4.17-~~),
+ librust-nix-0.29+default-dev,
+ librust-nix-0.29+fs-dev,
+ clang-21,
+ libbpf-dev,
+ proxmox-kernel-libc-dev
+Provides:
+ librust-proxmox-ebpf+bpf-debug-dev (= ${binary:Version}),
+ librust-proxmox-ebpf+default-dev (= ${binary:Version}),
+ librust-proxmox-ebpf-0-dev (= ${binary:Version}),
+ librust-proxmox-ebpf-0+bpf-debug-dev (= ${binary:Version}),
+ librust-proxmox-ebpf-0+default-dev (= ${binary:Version}),
+ librust-proxmox-ebpf-0.1-dev (= ${binary:Version}),
+ librust-proxmox-ebpf-0.1+bpf-debug-dev (= ${binary:Version}),
+ librust-proxmox-ebpf-0.1+default-dev (= ${binary:Version}),
+ librust-proxmox-ebpf-0.1.0-dev (= ${binary:Version}),
+ librust-proxmox-ebpf-0.1.0+bpf-debug-dev (= ${binary:Version}),
+ librust-proxmox-ebpf-0.1.0+default-dev (= ${binary:Version})
+Description: Proxmox VE eBPF - Rust source code
+ Source code for Debianized Rust crate "proxmox-ebpf"
diff --git a/proxmox-ebpf/debian/copyright b/proxmox-ebpf/debian/copyright
new file mode 100644
index 000000000000..01138fa0482f
--- /dev/null
+++ b/proxmox-ebpf/debian/copyright
@@ -0,0 +1,18 @@
+Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
+
+Files:
+ *
+Copyright: 2026 Proxmox Server Solutions GmbH <support@proxmox.com>
+License: AGPL-3.0-or-later
+ This program is free software: you can redistribute it and/or modify it under
+ the terms of the GNU Affero General Public License as published by the Free
+ Software Foundation, either version 3 of the License, or (at your option) any
+ later version.
+ .
+ This program is distributed in the hope that it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+ details.
+ .
+ You should have received a copy of the GNU Affero General Public License along
+ with this program. If not, see <https://www.gnu.org/licenses/>.
diff --git a/proxmox-ebpf/debian/debcargo.toml b/proxmox-ebpf/debian/debcargo.toml
new file mode 100644
index 000000000000..04e8c8f52d4f
--- /dev/null
+++ b/proxmox-ebpf/debian/debcargo.toml
@@ -0,0 +1,13 @@
+overlay = "."
+crate_src_path = ".."
+maintainer = "Proxmox Support Team <support@proxmox.com>"
+
+# the BPF programs and the harness's own are C sources of this crate, not vendored code
+whitelist = ["src/*/bpf/*.c", "tests/bpf/*.c"]
+
+[source]
+vcs_git = "git://git.proxmox.com/git/proxmox.git"
+vcs_browser = "https://git.proxmox.com/?p=proxmox.git"
+
+[packages.lib]
+depends = ["clang-21", "libbpf-dev", "proxmox-kernel-libc-dev"]
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH proxmox v8 3/6] ebpf: subsystem: add the shared tc program handling
2026-11-04 17:29 ` [PATCH proxmox v8 3/6] ebpf: subsystem: add the shared tc program handling Hannes Laimer
@ 2026-11-05 16:20 ` Wolfgang Bumiller
0 siblings, 0 replies; 9+ messages in thread
From: Wolfgang Bumiller @ 2026-11-05 16:20 UTC (permalink / raw)
To: Hannes Laimer; +Cc: pve-devel
On Wed, Nov 04, 2026 at 06:29:15PM +0100, Hannes Laimer wrote:
> A subsystem owns tc classifier programs and their maps, attached per
> interface and pinned in bpffs under the object's fingerprint, the
> crate's version and the schema version. So state survives between
> the one-shot calls of its consumer, re-running never interrupts
> traffic, and an older build does not install over a newer one.
>
> Changes run under one lock per subsystem, taken before the change
> reads its input, so they reach the kernel in the order they read it.
>
> A subsystem writes its maps entry by entry through the live maps,
> or hands over the whole content. For the latter, and whenever its
> own programs and maps are not pinned yet, the object is loaded with
> fresh maps in a staging directory, the content is written there, and
> only then do the links move onto the new programs, one link update
> each, with the maps coming along since a program holds its maps. A
> write that stops halfway, such as an insert the kernel cannot
> allocate for, leaves the live maps whole instead of a mix of old and
> new entries. The pins of maps and programs switch in one rename, so a
> plug always finds the maps its program reads.
>
> A subsystem describes its programs with a macro, from its name, the
> program names, the maps and the schema version. The object is found
> by the subsystem's name, and its fingerprint is derived from its
> bytes.
(Maybe also describe the created directory tree here in the commit
message. IMO the cover letter did a better job of explaining this commit
than the message above 🤷)
>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> proxmox-ebpf/src/lib.rs | 1 +
> proxmox-ebpf/src/subsystem.rs | 1067 +++++++++++++++++++++++++++++++++
> proxmox-ebpf/src/tc.rs | 32 +-
> 3 files changed, 1088 insertions(+), 12 deletions(-)
> create mode 100644 proxmox-ebpf/src/subsystem.rs
>
> diff --git a/proxmox-ebpf/src/lib.rs b/proxmox-ebpf/src/lib.rs
> index 6281a024fd2b..ab0763ab3d3d 100644
> --- a/proxmox-ebpf/src/lib.rs
> +++ b/proxmox-ebpf/src/lib.rs
> @@ -2,4 +2,5 @@
> //! 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.
>
> +pub mod subsystem;
> pub mod tc;
> diff --git a/proxmox-ebpf/src/subsystem.rs b/proxmox-ebpf/src/subsystem.rs
> new file mode 100644
> index 000000000000..39272060cd4e
> --- /dev/null
> +++ b/proxmox-ebpf/src/subsystem.rs
> @@ -0,0 +1,1067 @@
> +//! The shared subsystem code. A subsystem has one tc program per direction it declares, with
> +//! their maps and links. They are pinned under `/sys/fs/bpf/proxmox-ebpf/<name>/`, the links in
> +//! `links/` and the maps and programs under `<schema>/live/`. The programs are pinned by the
> +//! fingerprint of their object and selection and the version of the crate they were built into,
> +//! so the pinned state itself says which build it belongs to, and a build does not install over
> +//! a newer one.
> +//!
> +//! The loaded BPF stays in the kernel between invocations. [`ApplyLock::replace_maps`] loads the
> +//! object with fresh maps and moves the links onto it, a full apply calls it when what is pinned
> +//! is not this build's, when the last replacement stopped halfway, or whenever it wants the
> +//! content of its maps switched as a whole. Every other step works on what is pinned. Each
> +//! subsystem owns one [`TcPrograms`].
> +//!
> +//! Every change runs under the subsystem's apply lock, a full apply exclusively and a
> +//! single-interface step shared. The holder reads the input of its change under the lock and
> +//! writes before it lets go, so a full apply and every step reach the kernel in the order they
> +//! read their input. Two steps of one interface do not order each other.
Maybe add this (and verify, and fill missing things) to the TcPrograms
type doc comments - I tried to recreate this from the code…
// /sys/fs/bpf/proxmox-epbf PIN_ROOT
// └─ <TcPrograms::name> TcPrograms::pin_root()
// ├─ <schema_version> TcPrograms::schema_root()
// │ ├─ live/ TcPrograms::live_dir()
// │ │ └─ prog/ schema_progs_dir
// │ │ └─ <crate-version>-<program-fingerprint>/
// │ │ └─ <direction> BPF program
// │ └─ staging/ TcPrograms::staging_dir()
// └─ link/ TcPrograms::links_dir()
// └─ <LinkPin::filename()> TcPrograms::link_pin_path()
> +use std::{
> + collections::HashSet,
> + fs::File,
> + io::ErrorKind,
> + marker::PhantomData,
> + ops::{Deref, DerefMut},
> + os::unix::fs::{DirBuilderExt, OpenOptionsExt},
> + path::{Path, PathBuf},
> + time::{Duration, Instant},
> +};
> +
> +use anyhow::Context;
> +use aya::{EbpfLoader, programs::SchedClassifier};
> +use nix::{
> + errno::Errno,
> + fcntl::{Flock, FlockArg, RenameFlags, renameat2},
> +};
> +
> +use crate::tc::{self, Direction};
> +
> +const PIN_ROOT: &str = "/sys/fs/bpf/proxmox-ebpf";
> +const RUN_ROOT: &str = "/run/proxmox-ebpf";
> +
> +// FNV-1a over the object, the program selection and the map names, so the same object with
> +// another program chosen for a direction is another build
> +const fn build_fingerprint(obj: &[u8], programs: &[(Direction, &str)], maps: &[&str]) -> u64 {
> + let mut hash = fnv1a(0xcbf29ce484222325, obj);
> + let mut i = 0;
> + while i < programs.len() {
> + hash = fnv1a(hash, programs[i].0.as_str().as_bytes());
> + hash = fnv1a(hash, programs[i].1.as_bytes());
> + i += 1;
> + }
> + let mut i = 0;
> + while i < maps.len() {
> + hash = fnv1a(hash, maps[i].as_bytes());
> + i += 1;
> + }
> + hash
> +}
> +
> +// one field, ended by a zero byte so two fields cannot read as one
> +const fn fnv1a(mut hash: u64, bytes: &[u8]) -> u64 {
> + let mut i = 0;
> + while i < bytes.len() {
> + hash ^= bytes[i] as u64;
> + hash = hash.wrapping_mul(0x100000001b3);
> + i += 1;
> + }
> + hash.wrapping_mul(0x100000001b3)
> +}
> +
> +// one number of a cargo version as the environment hands it to the macro, digits only
> +const fn version_part(s: &str) -> u64 {
> + let bytes = s.as_bytes();
> + let mut n = 0u64;
> + let mut i = 0;
> + while i < bytes.len() {
> + assert!(bytes[i].is_ascii_digit(), "a cargo version part is digits");
> + n = n * 10 + (bytes[i] - b'0') as u64;
> + i += 1;
> + }
> + n
> +}
> +
> +/// A subsystem's [`TcPrograms`] from its name. The object is the build's `<name>.bpf.o`, the
> +/// fingerprint follows from the bytes and the version is the crate's own.
> +#[macro_export]
> +macro_rules! tc_programs {
> + (
> + name: $name:literal,
> + programs: $programs:expr,
> + maps: $maps:expr,
> + schema_version: $schema:expr $(,)?
> + ) => {
> + // built at compile time wherever the macro is used, a let binding included
> + const {
> + const OBJ: &[u8] =
> + ::aya::include_bytes_aligned!(concat!(env!("OUT_DIR"), "/", $name, ".bpf.o"));
> + $crate::subsystem::TcPrograms::new(
> + $name,
> + OBJ,
> + $programs,
> + $maps,
> + $schema,
> + [
> + env!("CARGO_PKG_VERSION_MAJOR"),
> + env!("CARGO_PKG_VERSION_MINOR"),
> + env!("CARGO_PKG_VERSION_PATCH"),
> + ],
> + )
> + }
> + };
> +}
> +
> +const LOCK_WAIT: Duration = Duration::from_secs(10);
> +const LOCK_POLL: Duration = Duration::from_millis(10);
> +
> +// flock takes a read-only descriptor, so nobody but root gets to open the lock files
> +fn private_dir(dir: &Path) -> std::io::Result<()> {
> + std::fs::DirBuilder::new()
> + .recursive(true)
> + .mode(0o700)
> + .create(dir)
> +}
> +
> +// flock has no timeout, so the wait retries a nonblocking flock with a sleep in between.
> +//
> +// not proxmox-sys lock because: blocks and gives up on a timer signal, and under pve-rs any signal
> +// perl handles cuts the blocked wait short the same way, since perl installs its handlers without
> +// restart
Have you actually tested if perl installs a handler for this particular
signal?
And even if it does, the perlmod entry point, can just temporarily
replace it.
pve-rs will still need to call `setup_timeout_handler()` in its init
once.
Busy-loops are always wrong.
> +fn flock_within(
> + path: &Path,
> + exclusive: bool,
> + what: &str,
> + subsystem: &str,
> +) -> anyhow::Result<Flock<File>> {
^ We normally have `anyhow::Error` imported.
Use `Result<…, Error>`.
> + let mut file = File::options()
> + .create(true)
> + .truncate(false)
> + .write(true)
> + .mode(0o600)
> + .open(path)
> + .with_context(|| format!("{subsystem}: open {what} {}", path.display()))?;
> + let arg = if exclusive {
> + FlockArg::LockExclusiveNonblock
> + } else {
> + FlockArg::LockSharedNonblock
> + };
> + let deadline = Instant::now() + LOCK_WAIT;
> + loop {
> + match Flock::lock(file, arg) {
> + Ok(lock) => return Ok(lock),
> + Err((returned, Errno::EWOULDBLOCK)) if Instant::now() < deadline => {
> + file = returned;
> + std::thread::sleep(LOCK_POLL);
> + }
> + Err((_, Errno::EWOULDBLOCK)) => anyhow::bail!(
> + "{subsystem}: {what} still held after {}s",
> + LOCK_WAIT.as_secs()
> + ),
> + Err((_, e)) => {
> + return Err(e)
> + .with_context(|| format!("{subsystem}: take {what} {}", path.display()));
> + }
> + }
> + }
> +}
> +
> +/// A step that needs the programs pinned found none. Only [`ApplyLock::replace_maps`] under the
> +/// exclusive lock brings them up.
> +#[derive(Debug)]
> +pub struct NotLoaded;
> +
> +impl std::fmt::Display for NotLoaded {
> + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> + f.write_str("programs not loaded")
> + }
> +}
> +
> +impl std::error::Error for NotLoaded {}
> +
> +/// A subsystem's description of its tc programs, written with the `tc_programs!` macro. The
> +/// loaded programs live pinned in the kernel and are reached through those pins.
> +pub struct TcPrograms {
> + /// The subsystem's name, its directory under the pin root.
> + name: &'static str,
> + /// The embedded BPF object.
> + obj: &'static [u8],
> + /// FNV-1a of `obj`, `programs` and `maps`, the programs are pinned under it.
> + fingerprint: u64,
> + /// The version of the crate the object was built into, pinned next to the fingerprint. A
> + /// build does not install over a higher one.
> + version: [u64; 3],
Let's have types for fingerprint and version.
(Even if just newtypes, `Fingerprint(u64)`, `(Version([u64; 3]))`)
> + /// The directions the object carries a program for, each with that program's name in the
> + /// object. One entry per direction.
> + programs: &'static [(Direction, &'static str)],
> + /// The object's pinned maps. A missing one means the state is not current.
> + maps: &'static [&'static str],
> + /// Bumped with any change to a map's key or value layout. Each schema has its own live set,
> + /// so a step never writes into maps of another layout, and a build leaves the state of a
> + /// newer schema alone.
> + schema_version: u32,
> +}
> +
> +impl TcPrograms {
> + /// What [`tc_programs!`](crate::tc_programs) expands to, with the fingerprint derived and the
> + /// version parsed at compile time.
> + pub const fn new(
> + name: &'static str,
> + obj: &'static [u8],
> + programs: &'static [(Direction, &'static str)],
> + maps: &'static [&'static str],
> + schema_version: u32,
> + version: [&'static str; 3],
> + ) -> Self {
> + Self {
> + name,
> + obj,
> + fingerprint: build_fingerprint(obj, programs, maps),
> + version: [
> + version_part(version[0]),
> + version_part(version[1]),
> + version_part(version[2]),
> + ],
> + programs,
> + maps,
> + schema_version,
> + }
> + }
> +
> + fn pin_root(&self) -> PathBuf {
> + PathBuf::from(PIN_ROOT).join(self.name)
> + }
These are all sticking together a bit too much…
> + fn schema_root(&self) -> PathBuf {
> + self.pin_root().join(self.schema_version.to_string())
> + }
> + fn live_dir(&self) -> PathBuf {
> + schema_live_dir(&self.schema_root())
> + }
> + fn staging_dir(&self) -> PathBuf {
> + self.schema_root().join("staging")
> + }
> + fn links_dir(&self) -> PathBuf {
> + links_dir_of(self.name)
> + }
> + fn link_pin_path(&self, pin: &tc::LinkPin) -> PathBuf {
> + self.links_dir().join(pin.filename())
> + }
Somehow I'm not convinced all the path helpers are actually *helping* here.
At least without the tree being described anywhere first… (as I
mentioned at the top).
> + // the build directory of a set of maps and programs, the live one or a staged one
> + fn prog_dir_in(&self, set: &Path) -> PathBuf {
why not call it build_dir_in
Maybe drop the helper, it's used once. Document what the path looks like
in `pin_programs()` (its only user).
> + set.join("prog")
> + .join(build_dir_name(self.version, self.fingerprint))
> + }
> + // the builds with programs pinned in the live set, each with its directory
> + fn pinned_builds(&self) -> anyhow::Result<Vec<(PathBuf, [u64; 3], u64)>> {
> + let entries = match std::fs::read_dir(schema_progs_dir(&self.schema_root())) {
> + Ok(entries) => entries,
> + Err(e) if e.kind() == ErrorKind::NotFound => return Ok(Vec::new()),
> + Err(e) => return Err(e.into()),
> + };
> + let mut builds = Vec::new();
> + for entry in entries {
> + let entry = entry?;
> + if let Some((version, fingerprint)) =
> + entry.file_name().to_str().and_then(parse_build_dir_name)
> + {
> + builds.push((entry.path(), version, fingerprint));
> + }
> + }
> + Ok(builds)
> + }
> + // the directory this build's programs are pinned in, named by whichever version installed
> + // them, a release with the same object installs nothing
Why is the last part of the sentence *here* in particular?
> + fn own_prog_dir(&self) -> anyhow::Result<Option<PathBuf>> {
> + Ok(self
> + .pinned_builds()?
> + .into_iter()
> + .find(|&(_, _, fingerprint)| fingerprint == self.fingerprint)
> + .map(|(dir, _, _)| dir))
> + }
> + fn newer_version_installed(&self) -> anyhow::Result<Option<[u64; 3]>> {
> + Ok(self
> + .pinned_builds()?
> + .into_iter()
> + .map(|(_, version, _)| version)
> + .filter(|&version| version > self.version)
> + .max())
> + }
> + fn program_name(&self, dir: Direction) -> Option<&'static str> {
> + self.programs
> + .iter()
> + .find(|&&(d, _)| d == dir)
> + .map(|&(_, name)| name)
> + }
> + fn run_dir(&self) -> PathBuf {
> + PathBuf::from(RUN_ROOT).join(self.name)
> + }
> + fn hash_map<K: aya::Pod, V: aya::Pod>(
> + &self,
> + name: &str,
> + ) -> anyhow::Result<aya::maps::HashMap<aya::maps::MapData, K, V>> {
> + self.hash_map_in(&self.live_dir(), name)
> + }
> +
> + fn hash_map_in<K: aya::Pod, V: aya::Pod>(
> + &self,
> + set: &Path,
> + name: &str,
> + ) -> anyhow::Result<aya::maps::HashMap<aya::maps::MapData, K, V>> {
> + let path = set.join(name);
> + if !path.exists() {
> + // a set comes up with every map the object pins, so an existing set lacks only a map
> + // the object never had under that name
> + if set.exists() {
> + anyhow::bail!("{}: no map {name} pinned in {}", self.name, set.display());
> + }
> + return Err(NotLoaded.into());
> + }
> + let map = aya::maps::MapData::from_pin(path)
> + .with_context(|| format!("{}: open pinned map {name}", self.name))?;
> + Ok(aya::maps::HashMap::try_from(aya::maps::Map::HashMap(map))?)
> + }
> +
> + // open one direction's pinned program, NotLoaded when it is not pinned
> + fn program(&self, dir: Direction) -> anyhow::Result<SchedClassifier> {
> + let path = self.own_prog_dir()?.map(|d| d.join(dir.as_str()));
> + let Some(path) = path.filter(|p| p.exists()) else {
> + return Err(NotLoaded.into());
> + };
^ This map/filter/let-else expression is cumbersome to read.
How about
let path = self.own_prog_dir()?.ok_or(NotLoaded)?.join(dir.as_str());
if !path.exists() {
return Err(NotLoaded.into());
}
> + SchedClassifier::from_pin(path)
> + .with_context(|| format!("{}: open pinned program {}", self.name, dir.as_str()))
> + }
> +
> + // one direction's program pinned in the live set, by whichever build pinned it there
> + fn live_program(&self, dir: Direction) -> anyhow::Result<Option<SchedClassifier>> {
> + for (build, _, _) in self.pinned_builds()? {
> + let path = build.join(dir.as_str());
> + if !path.exists() {
> + continue;
> + }
> + let prog = SchedClassifier::from_pin(&path).with_context(|| {
> + format!("{}: open pinned program {}", self.name, path.display())
> + })?;
> + return Ok(Some(prog));
So this just returns the first one it can open? But they're not sorted
and there could potentially be multiple entries? Shouldn't we sort by
version first? If not, a comment might be warranted.
> + }
> + Ok(None)
> + }
> +
> + // the apply lock under /run. A full apply and a teardown take it exclusively, a
> + // single-interface step shared. A wait times out instead of hanging. Shared holders do not
> + // yield to a waiting exclusive one, so a node attaching interfaces without pause can keep a
> + // full apply waiting until it times out. Every write to the pinned state is a method of the
> + // returned guard, so a step cannot run outside the lock
> + fn lock<M>(&self, exclusive: bool) -> anyhow::Result<ApplyLock<'_, M>> {
> + let dir = self.run_dir();
> + private_dir(&dir).with_context(|| {
> + format!("{}: create the run directory {}", self.name, dir.display())
> + })?;
> + let lock = flock_within(&dir.join("lock"), exclusive, "apply lock", self.name)?;
> + Ok(ApplyLock {
> + programs: self,
> + _lock: lock,
> + _mode: PhantomData,
> + })
> + }
> +
> + /// Whether a link of the interface name is pinned, read without the lock. Most interfaces
> + /// never had a link and their unplug must not wait behind a full apply. A miss is a snapshot,
What does "A miss is a snapshot" mean?
> + /// a pin arriving after it is another attach's. A failed listing counts as a hit.
And what does the first part mean? And why the last part?
> + pub fn link_pinned(&self, name: &str) -> bool {
> + self.live_links()
> + .map_or(true, |links| links.iter().any(|pin| pin.name == name))
> + }
> +
> + /// Take the apply lock shared, for a single-interface step.
> + pub fn lock_shared(&self) -> anyhow::Result<ApplyLock<'_, Shared>> {
> + self.lock(false)
> + }
> +
> + /// Take the apply lock exclusively, for a full apply or a teardown.
> + pub fn lock_exclusive(&self) -> anyhow::Result<ApplyLock<'_, Exclusive>> {
> + self.lock(true)
> + }
> +
> + fn is_current(&self) -> bool {
> + let programs_pinned = match self.own_prog_dir() {
> + Ok(Some(dir)) => self
> + .programs
> + .iter()
> + .all(|&(d, _)| dir.join(d.as_str()).exists()),
> + _ => false,
> + };
> + programs_pinned
> + && self.maps_pinned()
> + // a failed listing cannot rule a newer schema out
> + && matches!(self.newer_schema_installed(), Ok(None))
> + }
> +
> + fn maps_pinned(&self) -> bool {
> + self.maps
> + .iter()
> + .all(|map| self.live_dir().join(map).exists())
> + }
> +
> + // the directories under the pin root other than this build's schema and the links. One whose
> + // name is not a version counts as well and goes with the old schemas
> + fn other_schemas(&self) -> anyhow::Result<Vec<(PathBuf, Option<u32>)>> {
> + let entries = match std::fs::read_dir(self.pin_root()) {
> + Ok(entries) => entries,
> + Err(e) if e.kind() == ErrorKind::NotFound => return Ok(Vec::new()),
> + Err(e) => return Err(e.into()),
> + };
> + let mut others = Vec::new();
> + for entry in entries {
> + let entry = entry?;
> + if !entry.file_type()?.is_dir() || entry.file_name() == LINKS_DIR {
> + continue;
> + }
> + let schema = entry
> + .file_name()
> + .to_str()
> + .and_then(|s| s.parse::<u32>().ok());
> + if schema != Some(self.schema_version) {
> + others.push((entry.path(), schema));
> + }
> + }
> + Ok(others)
> + }
> +
> + fn newer_schema_installed(&self) -> anyhow::Result<Option<u32>> {
> + let newer = self
> + .other_schemas()?
> + .iter()
> + .filter(|(dir, _)| any_program_pinned(&schema_progs_dir(dir)))
> + .filter_map(|(_, schema)| *schema)
> + .find(|&s| s > self.schema_version);
> + Ok(newer)
> + }
> +
> + fn interrupted(&self) -> bool {
> + let other_pinned = match self.other_schemas() {
> + Ok(others) => others
> + .iter()
> + .any(|(dir, _)| any_program_pinned(&schema_progs_dir(dir))),
> + // a failed listing cannot rule a leftover out
> + Err(_) => true,
> + };
> + self.staging_dir().exists() || other_pinned
Mh, if the staging dir is removed last on apply, doing that check here
first might be more efficient. If it's true, we skip the readdir which
is arguably more expensive.
> + }
> +
> + fn tear_down(&self) -> anyhow::Result<()> {
> + remove_dir_if_present(&self.pin_root())
> + .with_context(|| format!("{}: remove the pinned state", self.name))
> + }
> +
> + // this build's object with all of its programs loaded and its maps created and pinned under
> + // `set`. Everything is loaded before any link is touched, so a verifier rejection of one
> + // direction leaves the old state whole
> + fn load(&self, set: &Path) -> anyhow::Result<aya::Ebpf> {
> + let mut bpf = EbpfLoader::new()
> + .map_pin_path(set)
> + .load(self.obj)
> + .with_context(|| format!("{}: load the object", self.name))?;
> + for &(_, name) in self.programs {
> + let prog: &mut SchedClassifier = bpf
> + .program_mut(name)
> + .with_context(|| format!("{}: program {name} not found in object", self.name))?
> + .try_into()?;
> + prog.load()
> + .with_context(|| format!("{}: load program {name}", self.name))?;
> + }
> + Ok(bpf)
> + }
> +
> + // move every pinned link onto the loaded programs
> + fn swap_links(&self, bpf: &mut aya::Ebpf) -> anyhow::Result<()> {
> + let live = self.live_links()?;
> + for &(dir, _) in self.programs {
> + self.swap_links_onto(dir, self.loaded(bpf, dir)?, &live)?;
> + }
> + Ok(())
> + }
> +
> + // put the live set on every link, a link on it already stays as it is, and detach the links
> + // of a direction the live set has no program for, they run a retired program. The live set
> + // decides, not this build's directions, since a run that died before its exchange left the
> + // old set live, and that keeps its links until the exchange
> + fn repair_links(&self) -> anyhow::Result<()> {
> + let live = self.live_links()?;
> + for dir in Direction::ALL {
> + match self.live_program(dir)? {
> + Some(mut prog) => self.swap_links_onto(dir, &mut prog, &live)?,
> + None => {
> + for pin in live.iter().filter(|pin| pin.dir == dir) {
> + self.reclaim_link(pin)?;
> + }
> + }
> + }
> + }
> + Ok(())
> + }
> +
> + // the links of one direction onto `prog`. One that cannot be swapped is unpinned, the next
> + // full apply attaches it again if it is still wanted
> + fn swap_links_onto(
> + &self,
> + dir: Direction,
> + prog: &mut SchedClassifier,
> + links: &[tc::LinkPin],
> + ) -> anyhow::Result<()> {
> + for pin in links.iter().filter(|pin| pin.dir == dir) {
> + let path = self.link_pin_path(pin);
> + // a pin that does not open says nothing about the link, one that opens and cannot be
> + // updated is a link the kernel let go
> + let link = match tc::open_pinned_link(&path) {
> + Ok(link) => link,
> + Err(tc::Error::PinGone) => continue,
> + Err(e) => {
> + return Err(e)
> + .with_context(|| format!("{}: open {}", self.name, pin.filename()));
> + }
> + };
> + if let Err(e) = tc::swap_link(prog, link) {
> + log::warn!(
> + "{}: {} swap failed ({e:#}), reclaiming",
> + self.name,
> + pin.filename()
> + );
> + self.reclaim_link(pin)?;
> + }
> + }
> + Ok(())
> + }
> +
> + fn pin_programs(&self, bpf: &mut aya::Ebpf, set: &Path) -> anyhow::Result<()> {
> + let dir = self.prog_dir_in(set);
> + std::fs::create_dir_all(&dir)
> + .with_context(|| format!("{}: create the program directory", self.name))?;
> + for &(direction, _) in self.programs {
> + self.loaded(bpf, direction)?
> + .pin(dir.join(direction.as_str()))
> + .with_context(|| format!("{}: pin program {}", self.name, direction.as_str()))?;
> + }
> + Ok(())
> + }
> +
> + // load the object with fresh maps under `staging`, have `fill` write them, then put the
> + // staged set in the live one's place and move the links over. That is one rename, an
> + // exchange when a live set is there, so a plug never finds the maps of one set next to the
> + // programs of the other, and a failure before it leaves the live set untouched. The retired
> + // set and the directories of other schemas go last, their programs stay alive through the
> + // links until each is swapped
> + fn replace_maps(
> + &self,
> + fill: impl FnOnce(&Staged<'_>) -> anyhow::Result<()>,
> + ) -> anyhow::Result<()> {
> + let others = self.other_schemas()?;
> + // programs pinned under a newer schema or by a newer version of the crate mean a newer
> + // build runs here and this process is the stale one, so it leaves the state alone
> + if let Some(newer) = self.newer_schema_installed()? {
> + anyhow::bail!(
> + "{}: pinned state is of schema {newer}, newer than this build's {}",
> + self.name,
> + self.schema_version
> + );
> + }
> + if let Some(newer) = self.newer_version_installed()? {
> + anyhow::bail!(
> + "{}: pinned programs are of version {}, newer than this build's {}",
> + self.name,
> + version_text(newer),
> + version_text(self.version)
> + );
> + }
> + std::fs::create_dir_all(self.links_dir())
> + .with_context(|| format!("{}: create the links directory", self.name))?;
> + let staging = self.staging_dir();
> + let live = self.live_dir();
> + // a run that died may have left links on the set it retired, so first make sure every
> + // link has the live set, then clear what it left behind. Without a live set the links
> + // are another schema's, whose set stays until the end
> + if staging.exists() && live.exists() {
> + self.repair_links()?;
> + }
> + remove_dir_if_present(&staging)
> + .with_context(|| format!("{}: remove the previous staging directory", self.name))?;
> + let staged = self.stage(&staging, fill).and_then(|bpf| {
> + if live.exists() {
^ Just do the `renameat2` call below and the `else` path on `ENOENT`.
(The ENOENT won't refer to the staged *source* unless there's external
interference, in which case the `rename` will fail the same way (either
way, it's no worse than the TOCTOU from `live.exists()`. Generally,
calls to `Path::exists()` are rarely useful.))
> + renameat2(None, &staging, None, &live, RenameFlags::RENAME_EXCHANGE).with_context(
> + || format!("{}: exchange the staged set with the live one", self.name),
> + )?;
> + } else {
> + std::fs::rename(&staging, &live)
> + .with_context(|| format!("{}: move the staged set into place", self.name))?;
> + }
> + Ok(bpf)
> + });
> + let mut bpf = match staged {
> + Ok(bpf) => bpf,
> + Err(e) => {
> + // the pins would keep the fresh maps alive
> + if let Err(err) = std::fs::remove_dir_all(&staging) {
> + log::warn!("{}: remove the staged set: {err:#}", self.name);
> + }
> + return Err(e);
> + }
> + };
> + self.remove_foreign_pins()?;
> + self.swap_links(&mut bpf)?;
> + if let Err(e) = remove_dir_if_present(&staging) {
> + log::warn!("{}: remove the retired set: {e:#}", self.name);
> + }
> + for (dir, _) in others {
> + if let Err(e) = std::fs::remove_dir_all(&dir) {
> + log::warn!(
> + "{}: remove old pinned state {}: {e:#}",
> + self.name,
> + dir.display()
> + );
> + }
> + }
> + Ok(())
> + }
> +
> + // the fresh set under `staging`, its maps written by `fill` and its programs pinned
> + fn stage(
> + &self,
> + staging: &Path,
> + fill: impl FnOnce(&Staged<'_>) -> anyhow::Result<()>,
> + ) -> anyhow::Result<aya::Ebpf> {
> + std::fs::create_dir_all(staging)
> + .with_context(|| format!("{}: create the staging directory", self.name))?;
> + let mut bpf = self.load(staging)?;
> + fill(&Staged { programs: self })?;
> + self.pin_programs(&mut bpf, staging)?;
> + Ok(bpf)
> + }
> +
> + // one direction's program out of the loaded object, resolved once by the load loop
> + fn loaded<'b>(
> + &self,
> + bpf: &'b mut aya::Ebpf,
> + dir: Direction,
> + ) -> anyhow::Result<&'b mut SchedClassifier> {
> + let name = self
> + .program_name(dir)
> + .expect("only directions with a program are loaded");
> + let prog = bpf
> + .program_mut(name)
> + .expect("every program was resolved by the load loop")
> + .try_into()?;
> + Ok(prog)
> + }
> +
> + fn live_links(&self) -> anyhow::Result<Vec<tc::LinkPin>> {
> + tc::read_pinned_links(&self.links_dir())
> + }
> +
> + // a pin this build cannot read or has no program for is not its state. It would keep a link
> + // running with nothing to update it, so it goes with the next replacement or its repair
> + fn remove_foreign_pins(&self) -> anyhow::Result<()> {
> + for entry in std::fs::read_dir(self.links_dir())? {
> + let entry = entry?;
> + let name = entry.file_name();
> + let ours = name
> + .to_str()
> + .and_then(tc::LinkPin::parse)
> + .is_some_and(|pin| self.program_name(pin.dir).is_some());
> + if ours {
> + continue;
> + }
> + let name = name.to_string_lossy();
> + log::warn!("{}: removing pin {name} this build cannot use", self.name);
> + let path = entry.path();
> + if let Err(e) = tc::detach_pinned_link(&path) {
> + log::warn!("{}: detach {name}: {e:#}", self.name);
> + let _ = std::fs::remove_file(&path);
> + }
> + }
> + Ok(())
> + }
> +
> + fn detach_named(
> + &self,
> + name: &str,
> + keep: impl Fn(&tc::LinkPin) -> bool,
> + ) -> anyhow::Result<Vec<u32>> {
> + let mut ifindexes = Vec::new();
> + for pin in self.live_links()? {
> + if pin.name != name || keep(&pin) {
> + continue;
> + }
> + self.reclaim_link(&pin)?;
> + if !ifindexes.contains(&pin.ifindex) {
> + ifindexes.push(pin.ifindex);
> + }
> + }
> + Ok(ifindexes)
> + }
> +
> + // unpin a link. A pin that cannot be opened stays, a failed open says nothing about the link
"Unpin" vs "reclaim"? :/
> + // behind it
> + fn reclaim_link(&self, pin: &tc::LinkPin) -> anyhow::Result<()> {
> + let path = self.link_pin_path(pin);
> + match tc::detach_pinned_link(&path) {
And the operation is called detach.
I'm getting confused by the wording in this series.
> + Ok(()) => Ok(()),
> + // the other side of a lost race, another attach or unplug took the pin first
> + Err(tc::Error::PinGone) => {
> + log::debug!(
> + "{}: reclaim {}: pin gone already",
> + self.name,
> + pin.filename()
> + );
> + Ok(())
> + }
> + Err(e) => Err(e).with_context(|| format!("{}: unpin {}", self.name, pin.filename())),
> + }
> + }
> +
> + fn reconcile(&self, desired: &HashSet<u32>) -> anyhow::Result<()> {
> + // opened before anything is detached, so a missing program fails the full apply before it
> + // changes anything
> + let mut progs = Vec::new();
> + for &(dir, _) in self.programs {
> + progs.push((dir, self.program(dir)?));
> + }
> + let live = self.live_links()?;
> + let mut attached: HashSet<(u32, Direction)> = HashSet::new();
> +
> + for pin in &live {
> + // a pin whose index now belongs to another interface links nothing wanted
> + let alive = tc::iface_name(pin.ifindex)?.is_some_and(|name| name == pin.name);
> + if alive && desired.contains(&pin.ifindex) {
> + attached.insert((pin.ifindex, pin.dir));
> + continue;
> + }
> + log::info!("{}: detach {}", self.name, pin.filename());
> + if let Err(e) = self.reclaim_link(pin) {
> + log::warn!("{e:#}");
> + }
> + }
> +
> + let mut failed = 0usize;
> + for (dir, prog) in &mut progs {
> + let dir = *dir;
> + for &ifindex in desired {
> + if attached.contains(&(ifindex, dir)) {
> + continue;
> + }
> + let Some(name) = tc::iface_name(ifindex)? else {
> + log::info!("{}: {ifindex} vanished before the attach", self.name);
> + continue;
> + };
> + let pin = tc::LinkPin { ifindex, name, dir };
> + match tc::attach_and_pin(prog, &pin, &self.link_pin_path(&pin)) {
> + Ok(()) => {}
> + Err(tc::Error::IfaceGone) => {
> + log::info!("{}: {ifindex} vanished before the attach", self.name);
> + }
> + Err(e) => {
> + log::error!("{}: attach {}: {e:#}", self.name, pin.filename());
> + failed += 1;
> + }
> + }
> + }
> + }
> + if failed > 0 {
> + anyhow::bail!("{}: {failed} link(s) failed to attach", self.name);
> + }
> + Ok(())
> + }
> +
> + fn attach_iface(&self, ifindex: u32) -> anyhow::Result<()> {
> + // opened for every direction ahead of the pin checks, so a missing program fails here,
> + // never counts as a dead link and leaves no direction attached alone
> + let mut progs = Vec::new();
> + for &(dir, _) in self.programs {
> + progs.push((dir, self.program(dir)?));
> + }
> + let Some(name) = tc::iface_name(ifindex)? else {
> + log::info!("{}: {ifindex} vanished before the attach", self.name);
> + return Ok(());
> + };
> + let live = self.live_links()?;
> + for (dir, prog) in &mut progs {
> + let dir = *dir;
> + let pin = tc::LinkPin {
> + ifindex,
> + name: name.clone(),
> + dir,
> + };
> + let path = self.link_pin_path(&pin);
> + for stale in live.iter().filter(|p| p.ifindex == ifindex && p.dir == dir) {
> + if stale.name == name {
> + let link = match tc::open_pinned_link(&path) {
> + Ok(link) => link,
> + // the pin went away under the swap, another plug of the same interface
> + // is rebuilding it and the attach below finds its link
> + Err(tc::Error::PinGone) => {
> + log::debug!(
> + "{}: {} pin taken by another attach",
> + self.name,
> + path.display()
> + );
> + continue;
> + }
> + Err(e) => {
> + return Err(e).with_context(|| {
> + format!("{}: open {}", self.name, pin.filename())
> + });
> + }
> + };
> + if let Err(e) = tc::swap_link(prog, link) {
> + log::warn!(
> + "{}: {} swap failed ({e:#}), rebuilding",
> + self.name,
> + pin.filename()
> + );
> + self.reclaim_link(stale)?;
> + continue;
> + }
> + } else {
> + // the index was reused since, the old link is not this interface's and does
> + // not stand in the way of its own
> + if let Err(e) = self.reclaim_link(stale) {
> + log::warn!("{e:#}");
> + }
> + }
> + }
> + if path.exists() {
> + continue;
> + }
> + match tc::attach_and_pin(prog, &pin, &path) {
> + Ok(()) => {}
> + Err(tc::Error::IfaceGone) => {
> + log::info!("{}: {ifindex} vanished before the attach", self.name);
> + }
> + Err(tc::Error::AlreadyAttached) => {
> + log::debug!("{}: {} attached concurrently", self.name, pin.filename());
> + }
> + Err(e) => {
> + return Err(e)
> + .with_context(|| format!("{}: attach {}", self.name, pin.filename()));
> + }
> + }
> + }
> + Ok(())
> + }
> +}
> +
> +const LINKS_DIR: &str = "links";
> +
> +fn links_dir_of(subsystem: &str) -> PathBuf {
> + PathBuf::from(PIN_ROOT).join(subsystem).join(LINKS_DIR)
> +}
> +
> +// the name of a build's program directory, its version and fingerprint, with colons for the
> +// dots bpffs refuses in a name
> +fn build_dir_name(version: [u64; 3], fingerprint: u64) -> String {
> + format!(
> + "{}:{}:{}-{fingerprint:016x}",
> + version[0], version[1], version[2]
> + )
> +}
> +
> +fn parse_build_dir_name(name: &str) -> Option<([u64; 3], u64)> {
> + let (version, fingerprint) = name.split_once('-')?;
> + let mut parts = version.split(':').map(|part| part.parse::<u64>().ok());
> + let version = [parts.next()??, parts.next()??, parts.next()??];
> + if parts.next().is_some() || fingerprint.len() != 16 {
> + return None;
> + }
> + Some((version, u64::from_str_radix(fingerprint, 16).ok()?))
> +}
> +
> +fn version_text(version: [u64; 3]) -> String {
> + format!("{}.{}.{}", version[0], version[1], version[2])
> +}
> +
> +fn schema_live_dir(schema_root: &Path) -> PathBuf {
> + schema_root.join("live")
> +}
> +
> +fn schema_progs_dir(schema_root: &Path) -> PathBuf {
> + schema_live_dir(schema_root).join("prog")
> +}
^ The above 2 path helpers are outliers in that they aren't inside
`TcPrograms`. It gets a bit difficult to keep track of all the path
helpers.
Also, `TcPrograms::live_dir()` now calls out to this extra function
above, but could just be inline like the rest of them, as both of these
are only used inside `impl TcPrograms`.
> +
> +fn remove_dir_if_present(dir: &Path) -> std::io::Result<()> {
> + match std::fs::remove_dir_all(dir) {
> + Err(e) if e.kind() != ErrorKind::NotFound => Err(e),
> + _ => Ok(()),
> + }
> +}
> +
> +// a program pin is a file under a build directory. A run killed between creating that
> +// directory and pinning leaves it empty. Like the links, a directory that cannot be read may
> +// hold anything, a missing one is empty
> +fn any_program_pinned(progs_dir: &Path) -> bool {
> + let non_empty = |dir: &Path| match std::fs::read_dir(dir) {
> + Ok(mut entries) => entries.next().is_some(),
> + Err(e) => e.kind() != ErrorKind::NotFound,
> + };
> + match std::fs::read_dir(progs_dir) {
> + Ok(fingerprints) => fingerprints.into_iter().any(|fp| match fp {
> + Ok(fp) => non_empty(&fp.path()),
> + Err(_) => true,
> + }),
> + Err(e) => e.kind() != ErrorKind::NotFound,
> + }
> +}
> +
> +/// Marker for a guard holding the apply lock shared.
> +pub struct Shared;
> +
> +/// Marker for a guard holding the apply lock exclusively.
> +pub struct Exclusive;
> +
> +/// The apply lock of one subsystem, held for as long as the guard lives. Replacing the maps and
"held for as long as the guard lives" is superfluous.
> +/// matching the links to the desired set rewrite pinned state and exist only on the
> +/// [`Exclusive`] guard, so nothing runs them next to shared holders. A shared holder adds or
Weirdly worded and already seen from the impl blocks (which can also
have doc comments themselves FYI. Rustdoc shows them as separate blocks,
too.
> +/// drops its own link and writes map entries that stand on their own, each write atomic in the
*ugh*
> +/// kernel. A write that depends on what was read before it takes the exclusive lock, or the
> +/// kernel's conditional write. A shared holder cannot upgrade, one that finds nothing current
What conditional write is this referring to?
> +/// leaves bringing the set up to a full apply under the exclusive lock.
> +pub struct ApplyLock<'a, M> {
> + programs: &'a TcPrograms,
> + _lock: Flock<File>,
> + _mode: PhantomData<M>,
> +}
> +
> +impl<M> ApplyLock<'_, M> {
> + /// Whether this build's programs and maps are pinned and no newer schema is. A shared holder
> + /// checks this before it writes, since only a full apply brings the pins up and an older build
> + /// stays out of a newer schema. Under the lock the program and map pins do not change, only
> + /// links come and go, so the answer holds for the holder's steps.
> + pub fn is_current(&self) -> bool {
> + self.programs.is_current()
> + }
> +
> + /// Whether a replacement stopped halfway. Its staged set or the directories of other schemas
> + /// are still there, so the links may still run the programs it retired.
> + pub fn interrupted(&self) -> bool {
> + self.programs.interrupted()
> + }
> +
> + /// Open one of this build's live maps by name, [`NotLoaded`] when it is not pinned.
> + pub fn hash_map<K: aya::Pod, V: aya::Pod>(
> + &self,
> + name: &str,
> + ) -> anyhow::Result<MapHandle<'_, K, V>> {
> + Ok(MapHandle {
> + map: self.programs.hash_map(name)?,
> + _opened_through: PhantomData,
> + })
> + }
> +
> + /// Attach the programs to a single interface, leaving the others alone.
> + ///
> + /// If a pin already exists, swap the program in place with no traffic gap. A pin that cannot
> + /// be swapped is reclaimed and the interface attached fresh. Two concurrent attaches of the
> + /// same interface run under the shared lock, the one that loses the race finds the other's
> + /// link in place and is done. A failed attach returns the error, [`NotLoaded`] when nothing
> + /// is pinned yet, and the caller decides what an interface without the program means for it.
> + pub fn attach_iface(&self, ifindex: u32) -> anyhow::Result<()> {
> + self.programs.attach_iface(ifindex)
> + }
> +
> + /// Detach the programs from the interface of that name, even when the interface is gone
> + /// already. A pin for which `keep` returns true stays, such as one of a newer interface under
> + /// the same name. Returns the indexes the detached links had.
> + pub fn detach_named(
> + &self,
> + name: &str,
> + keep: impl Fn(&tc::LinkPin) -> bool,
> + ) -> anyhow::Result<Vec<u32>> {
> + self.programs.detach_named(name, keep)
> + }
> +
> + /// The links of this subsystem as their pins name them. A shared holder sees a snapshot,
> + /// another attach may add to it meanwhile. The name is not unique. A leftover pin of an
> + /// interface that went away and the pin of its successor under the name share it, only the
> + /// index tells them apart.
> + pub fn attached(&self) -> anyhow::Result<Vec<tc::LinkPin>> {
> + self.programs.live_links()
> + }
> +}
> +
> +impl ApplyLock<'_, Exclusive> {
> + /// Bring this build's programs and maps up, or replace the content of the maps when they are
> + /// up already, without a program ever reading a mix of old and new entries. A first install,
> + /// a new build and a schema change are all this one call. `fill` writes the whole desired
> + /// content into fresh maps, and only once it returned do the programs holding them take over
> + /// the links, one link at a time. A failure inside `fill`, a refused entry included, leaves
> + /// what is pinned untouched. Nothing written into the live maps carries over, a caller that
> + /// wants some of it reads it through [`hash_map`](Self::hash_map) before the call, which
> + /// answers [`NotLoaded`] when nothing of this build is pinned. Pinned state of a newer schema
> + /// or of a newer version of the subsystem's crate is refused.
This IMO does not properly describe what fill is *supposed* to do. But
it does claim to state what it "does".
> + ///
> + /// Takes the guard exclusively, a map handle open across the exchange would write the
> + /// retired map.
> + pub fn replace_maps(
> + &mut self,
> + fill: impl FnOnce(&Staged<'_>) -> anyhow::Result<()>,
> + ) -> anyhow::Result<()> {
> + self.programs.replace_maps(fill)
> + }
> +
> + /// Make the attached set match `desired`. Moving links onto new programs is
> + /// [`replace_maps`](Self::replace_maps)'s job. The live set cannot change under it, every
> + /// other writer waits on the exclusive lock. What may go is decided from the kernel's state.
Most of this paragraph does not seem to belong here?
The parameter is not really explained (and this is a public API not
called from anywhere in here).
> + ///
> + /// An interface that failed to attach fails the apply once every other was tried, since the
> + /// subsystem does not work on an interface without its program. One that vanished since it
> + /// was named is no failure, and a failed detach only leaves a program running longer and is
> + /// logged.
> + pub fn reconcile(&self, desired: &HashSet<u32>) -> anyhow::Result<()> {
> + self.programs.reconcile(desired)
> + }
> +
> + /// Drop the pinned state of this subsystem, links included. Leaving links attached would keep
> + /// the programs running with nothing left to update them. The lock under `/run` stays,
> + /// unlinking it under a waiter would let that waiter run unserialized. Takes the guard
> + /// exclusively like [`replace_maps`](Self::replace_maps).
> + pub fn clear(&mut self) -> anyhow::Result<()> {
> + self.programs.tear_down()
> + }
> +}
> +
> +/// The fresh maps of a [`replace_maps`](ApplyLock::replace_maps), seen only by the fill that
> +/// writes them.
> +pub struct Staged<'a> {
> + programs: &'a TcPrograms,
> +}
> +
> +impl Staged<'_> {
> + /// Open one of the staged maps by name, empty until the fill writes it.
> + pub fn hash_map<K: aya::Pod, V: aya::Pod>(
> + &self,
> + name: &str,
> + ) -> anyhow::Result<MapHandle<'_, K, V>> {
> + Ok(MapHandle {
> + map: self
> + .programs
> + .hash_map_in(&self.programs.staging_dir(), name)?,
> + _opened_through: PhantomData,
> + })
> + }
> +}
> +
> +/// A pinned hash map. It cannot outlive the lock or the fill it was opened under, so every write
> +/// to it runs under the lock and goes into the maps the programs use.
> +pub struct MapHandle<'a, K, V> {
> + map: aya::maps::HashMap<aya::maps::MapData, K, V>,
> + _opened_through: PhantomData<&'a ()>,
> +}
> +
> +impl<K, V> Deref for MapHandle<'_, K, V> {
> + type Target = aya::maps::HashMap<aya::maps::MapData, K, V>;
> +
> + fn deref(&self) -> &Self::Target {
> + &self.map
> + }
> +}
> +
> +impl<K, V> DerefMut for MapHandle<'_, K, V> {
> + fn deref_mut(&mut self) -> &mut Self::Target {
> + &mut self.map
> + }
> +}
> diff --git a/proxmox-ebpf/src/tc.rs b/proxmox-ebpf/src/tc.rs
> index 1217703e8906..09a02b2947cc 100644
> --- a/proxmox-ebpf/src/tc.rs
> +++ b/proxmox-ebpf/src/tc.rs
> @@ -10,14 +10,14 @@ use aya::{
> programs::{
> ProgramError, SchedClassifier, TcAttachType,
> links::{FdLink, LinkError, LinkOrder, PinnedLink},
> - tc::TcAttachOptions,
> + tc::{SchedClassifierLink, TcAttachOptions},
> },
> };
>
> /// Why a link step did not go through, with the kernel's own answer where it gives one, so a
> /// caller can tell a lost race or a vanished interface from a real failure.
> #[derive(Debug)]
> -pub enum Error {
> +pub(crate) enum Error {
> /// A link of the program is in place already, another attach of the interface won the race.
> AlreadyAttached,
> /// The interface is gone, its index names no netdev anymore.
> @@ -108,8 +108,11 @@ pub enum Direction {
> }
>
> impl Direction {
> + /// Both hooks.
> + pub(crate) const ALL: [Self; 2] = [Self::Ingress, Self::Egress];
> +
> /// The lowercase name, as it stands in pin file names.
> - pub fn as_str(self) -> &'static str {
> + pub const fn as_str(self) -> &'static str {
> match self {
> Self::Ingress => "ingress",
> Self::Egress => "egress",
> @@ -182,7 +185,7 @@ impl LinkPin {
> /// verdict ends the chain right there. So a subsystem's program returns `TC_ACT_UNSPEC` for
> /// whatever it leaves alone. Programs of several subsystems run in attach order, each new link
> /// goes last.
> -pub fn attach_and_pin(
> +pub(crate) fn attach_and_pin(
> prog: &mut SchedClassifier,
> pin: &LinkPin,
> pin_path: &Path,
> @@ -210,18 +213,23 @@ pub fn attach_and_pin(
> Ok(())
> }
>
> -/// Rebind a pinned link to `prog` via LINK_UPDATE. Atomic, traffic sees no detach/reattach gap.
> -pub fn swap_pinned_link(prog: &mut SchedClassifier, pin_path: &Path) -> Result<(), Error> {
> - let pinned = PinnedLink::from_pin(pin_path)?;
> - let fd_link: FdLink = pinned.into();
> - let link = fd_link.try_into()?;
> +pub(crate) fn open_pinned_link(pin_path: &Path) -> Result<SchedClassifierLink, Error> {
> + let fd_link: FdLink = PinnedLink::from_pin(pin_path)?.into();
> + Ok(fd_link.try_into()?)
> +}
> +
> +/// Rebind `link` to `prog` via LINK_UPDATE. Atomic, traffic sees no detach/reattach gap.
> +pub(crate) fn swap_link(
> + prog: &mut SchedClassifier,
> + link: SchedClassifierLink,
> +) -> Result<(), Error> {
> let new_id = prog.attach_to_link(link)?;
> // take the handle out of aya's internal tracking, we have the pin
> let _ = prog.take_link(new_id)?;
> Ok(())
> }
>
> -pub fn detach_pinned_link(pin_path: &Path) -> Result<(), Error> {
> +pub(crate) fn detach_pinned_link(pin_path: &Path) -> Result<(), Error> {
> let pinned = PinnedLink::from_pin(pin_path)?;
> let _fd_link = pinned.unpin()?;
> Ok(())
> @@ -229,7 +237,7 @@ pub fn detach_pinned_link(pin_path: &Path) -> Result<(), Error> {
>
> /// The name of an interface by index, none when it is gone. Any other failure says nothing
> /// about the interface and is not taken as its absence, so the lookup reads errno itself.
> -pub fn iface_name(ifindex: u32) -> anyhow::Result<Option<String>> {
> +pub(crate) fn iface_name(ifindex: u32) -> anyhow::Result<Option<String>> {
> let mut buf = [0u8; nix::libc::IF_NAMESIZE];
> // SAFETY: the buffer holds IF_NAMESIZE bytes, the most the call writes
> let name = unsafe { nix::libc::if_indextoname(ifindex, buf.as_mut_ptr().cast()) };
> @@ -247,7 +255,7 @@ pub fn iface_name(ifindex: u32) -> anyhow::Result<Option<String>> {
>
> /// The pins in `links_dir`, see [`LinkPin`], none for a missing directory. A file that is not a
> /// pin is skipped.
> -pub fn read_pinned_links(links_dir: &Path) -> anyhow::Result<Vec<LinkPin>> {
> +pub(crate) fn read_pinned_links(links_dir: &Path) -> anyhow::Result<Vec<LinkPin>> {
> let mut out = Vec::new();
> let dir = match std::fs::read_dir(links_dir) {
> Ok(d) => d,
> --
> 2.47.3
>
>
>
>
>
--
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH proxmox v8 4/6] ebpf: subsystem: place subsystem links in a fixed order
2026-11-04 17:29 ` [PATCH proxmox v8 4/6] ebpf: subsystem: place subsystem links in a fixed order Hannes Laimer
@ 2026-11-06 13:30 ` Wolfgang Bumiller
0 siblings, 0 replies; 9+ messages in thread
From: Wolfgang Bumiller @ 2026-11-06 13:30 UTC (permalink / raw)
To: Hannes Laimer; +Cc: pve-devel
🥗
On Wed, Nov 04, 2026 at 06:29:16PM +0100, Hannes Laimer wrote:
> tcx runs the programs on an interface in the order their links were
> attached, so without a fixed order the subsystem that sees a packet
> first is whichever attached there first. The run order is a list in
> the lib. An attach places its link relative to the links the other
> subsystems hold on that interface, found through their pins, under
> one lock for all subsystems so two attaches to one interface see each
> other. Swaps keep a link's slot.
>
> A name missing from the list fails the build. A full apply writes
> its list to the file under /run when an older crate wrote the file,
> and adds the names the file lacks otherwise. Every attach places by
> that file. So the newest crate decides the order, a build can place
> its link against a subsystem it was not built with, and attached
Maybe introduce somewhere specifically what you refer to with "build"
somewhere, and "subsystem", and "link" vs "pin" vs "LinkPin" vs
"interface"… The comments in this series are often not very helpful with
their language.
Also "place against a subsystem"?
> links stay where they are.
>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> proxmox-ebpf/src/subsystem.rs | 443 +++++++++++++++++++++++++++++++++-
> proxmox-ebpf/src/tc.rs | 14 +-
> 2 files changed, 445 insertions(+), 12 deletions(-)
>
> diff --git a/proxmox-ebpf/src/subsystem.rs b/proxmox-ebpf/src/subsystem.rs
> index 39272060cd4e..274c11fdb22a 100644
> --- a/proxmox-ebpf/src/subsystem.rs
> +++ b/proxmox-ebpf/src/subsystem.rs
> @@ -15,10 +15,16 @@
> //! single-interface step shared. The holder reads the input of its change under the lock and
> //! writes before it lets go, so a full apply and every step reach the kernel in the order they
> //! read their input. Two steps of one interface do not order each other.
> +//!
> +//! The programs of different subsystems on one interface run in the order of [`RUN_ORDER`]. A
> +//! full apply writes the list to a file under `/run`, and an attach places its link among the
> +//! links the other subsystems hold on that interface by the list written last, so a build places
> +//! right against subsystems it was not built with. It finds those links through their pins and
> +//! does that under a lock all subsystems share, so each attach sees the links pinned before it.
> use std::{
> collections::HashSet,
> fs::File,
> - io::ErrorKind,
> + io::{ErrorKind, Write},
> marker::PhantomData,
> ops::{Deref, DerefMut},
> os::unix::fs::{DirBuilderExt, OpenOptionsExt},
> @@ -27,7 +33,10 @@ use std::{
> };
>
> use anyhow::Context;
> -use aya::{EbpfLoader, programs::SchedClassifier};
> +use aya::{
> + EbpfLoader,
> + programs::{SchedClassifier, links::LinkOrder},
> +};
> use nix::{
> errno::Errno,
> fcntl::{Flock, FlockArg, RenameFlags, renameat2},
> @@ -37,6 +46,39 @@ use crate::tc::{self, Direction};
>
> const PIN_ROOT: &str = "/sys/fs/bpf/proxmox-ebpf";
> const RUN_ROOT: &str = "/run/proxmox-ebpf";
> +const ORDER_FILE: &str = "order";
> +
> +/// The subsystems in the order their programs run on an interface, the first runs first. A name
> +/// missing from the list fails the build. The newest build of the crate on a host decides the
> +/// order, a new name takes effect with its next full apply, a moved or removed one for the links
> +/// attached after that, since already attached links stay where they are.
> +pub const RUN_ORDER: &[&str] = &[];
> +
> +const fn check_listed(name: &str) {
> + let mut i = 0;
> + while i < RUN_ORDER.len() {
> + if str_eq(RUN_ORDER[i], name) {
> + return;
> + }
> + i += 1;
> + }
> + panic!("subsystem not in RUN_ORDER");
> +}
> +
> +const fn str_eq(a: &str, b: &str) -> bool {
Use `proxmox_const_utils::byte_string_eq()`.
Any time a const fn helper is added, check the const-utils crate, and
consider adding something there.
> + let (a, b) = (a.as_bytes(), b.as_bytes());
> + if a.len() != b.len() {
> + return false;
> + }
> + let mut i = 0;
> + while i < a.len() {
> + if a[i] != b[i] {
> + return false;
> + }
> + i += 1;
> + }
> + true
> +}
>
> // FNV-1a over the object, the program selection and the map names, so the same object with
> // another program chosen for a direction is another build
> @@ -81,7 +123,10 @@ const fn version_part(s: &str) -> u64 {
> }
>
> /// A subsystem's [`TcPrograms`] from its name. The object is the build's `<name>.bpf.o`, the
> -/// fingerprint follows from the bytes and the version is the crate's own.
> +/// fingerprint follows from the bytes and the selection and the version is the crate's own.
> +/// `programs` names the object's program per direction, one
> +/// entry each, `maps` names its pinned maps, and `schema_version` goes up with any change to a
> +/// map's key or value layout, so no build writes into maps of another layout.
> #[macro_export]
> macro_rules! tc_programs {
> (
> @@ -211,6 +256,7 @@ impl TcPrograms {
> schema_version: u32,
> version: [&'static str; 3],
> ) -> Self {
> + check_listed(name);
> Self {
> name,
> obj,
> @@ -239,7 +285,7 @@ impl TcPrograms {
> self.schema_root().join("staging")
> }
> fn links_dir(&self) -> PathBuf {
> - links_dir_of(self.name)
> + self.pin_root().join(LINKS_DIR)
> }
> fn link_pin_path(&self, pin: &tc::LinkPin) -> PathBuf {
> self.links_dir().join(pin.filename())
> @@ -362,6 +408,115 @@ impl TcPrograms {
> })
> }
>
> + // one lock for every subsystem. An attach holds it from reading the list and the neighbors to
> + // the pin of its link, so two attaches to one interface see each other, and the list is
> + // written under it
> + fn attach_lock(&self) -> anyhow::Result<Flock<File>> {
> + private_dir(Path::new(RUN_ROOT))
> + .with_context(|| format!("{}: create the run directory {RUN_ROOT}", self.name))?;
> + let path = Path::new(RUN_ROOT).join("attach.lock");
> + flock_within(&path, true, "attach lock", self.name)
> + }
> +
> + // the links the other subsystems hold on the interface and direction of `pin`, with their
> + // places in `order`. A pin has the same file name in every subsystem, so finding one is a stat
> + // of that name in each of their links directories, which are never listed. A subsystem the
> + // list does not name cannot be placed against
> + fn neighbors(
> + &self,
> + pin: &tc::LinkPin,
> + order: &[String],
> + ) -> anyhow::Result<Vec<(usize, PathBuf)>> {
> + let mut found = Vec::new();
> + for (root, name) in
> + subsystem_roots().with_context(|| format!("{}: list the subsystems", self.name))?
> + {
> + if name == self.name {
> + continue;
> + }
> + let path = root.join(LINKS_DIR).join(pin.filename());
> + if !path.exists() {
> + continue;
> + }
> + match order.iter().position(|listed| *listed == name) {
> + Some(place) => found.push((place, path)),
> + None => log::warn!(
> + "{}: {name} is not in the order list, not placed against it",
> + self.name
> + ),
> + }
> + }
> + Ok(found)
> + }
> +
> + // attach `prog` for `pin` in its place among the other subsystems' links, see `RUN_ORDER`,
> + // and pin it under `pin_path`
> + fn attach_link(
> + &self,
> + prog: &mut SchedClassifier,
> + pin: &tc::LinkPin,
> + pin_path: &Path,
> + ) -> Result<(), tc::Error> {
> + let _lock = self.attach_lock()?;
> + let (_, order) = read_order()?;
> + // under the lock pins only go away, other attaches wait on it. So a neighbor that cannot
> + // be placed against is dropped and the attach tries the rest without listing them again
> + let mut neighbors = self.neighbors(pin, &order)?;
> + let rank = match order.iter().position(|listed| listed.as_str() == self.name) {
> + Some(rank) => rank,
> + // only a list written by a build that dropped this subsystem lacks it
> + None => {
> + log::warn!("{}: not in the order list, attached last", self.name);
> + neighbors.clear();
> + 0
> + }
> + };
> + loop {
> + let (index, before) = match place(rank, &neighbors) {
> + Place::Before(i) => (Some(i), true),
> + Place::After(i) => (Some(i), false),
> + Place::Last => (None, false),
> + };
> + // the order holds only the neighbor's fd, the link stays open until the attach is done
> + let neighbor = match index {
> + Some(i) => match tc::open_pinned_link(&neighbors[i].1) {
> + Ok(link) => Some(link),
> + // unpinned since the lookup, its subsystem detached it
> + Err(tc::Error::PinGone) => {
> + neighbors.remove(i);
> + continue;
> + }
> + Err(e) => return Err(e),
> + },
> + None => None,
> + };
> + let placement = match (&neighbor, before) {
> + (Some(link), true) => LinkOrder::before_link(link)?,
> + (Some(link), false) => LinkOrder::after_link(link)?,
> + (None, _) => LinkOrder::last(),
> + };
> + let attached = tc::attach_and_pin(prog, pin, pin_path, placement);
> + let Some(i) = index else {
> + return attached;
> + };
> + match attached {
> + // a neighbor's pin may name a link whose interface is gone under a reused index,
> + // nothing can be placed against that one until its subsystem reclaims it. The
> + // program is better attached out of order than not at all
> + Err(tc::Error::NotAttached) => {
> + log::warn!(
> + "{}: {} not placed against {}, that link is not attached",
> + self.name,
> + pin.filename(),
> + neighbors[i].1.display()
> + );
> + neighbors.remove(i);
> + }
> + result => return result,
> + }
> + }
> + }
> +
> /// Whether a link of the interface name is pinned, read without the lock. Most interfaces
> /// never had a link and their unplug must not wait behind a full apply. A miss is a snapshot,
> /// a pin arriving after it is another attach's. A failed listing counts as a hit.
> @@ -451,6 +606,29 @@ impl TcPrograms {
> .with_context(|| format!("{}: remove the pinned state", self.name))
> }
>
> + // the list every attach places by, see `next_order`
> + fn write_order(&self) -> anyhow::Result<()> {
> + let _lock = self.attach_lock()?;
> + let Some((version, names)) = next_order(&read_order()?, self.version, RUN_ORDER) else {
> + return Ok(());
> + };
> + let path = Path::new(RUN_ROOT).join(ORDER_FILE);
> + let tmp = path.with_extension("tmp");
> + let mut file = File::options()
> + .write(true)
> + .create(true)
> + .truncate(true)
> + .mode(0o600)
> + .open(&tmp)
> + .with_context(|| format!("{}: write the order list", self.name))?;
> + let content = format!("{}\n{}\n", version_text(version), names.join("\n"));
> + file.write_all(content.as_bytes())
> + .with_context(|| format!("{}: write the order list", self.name))?;
> + std::fs::rename(&tmp, &path)
> + .with_context(|| format!("{}: move the order list into place", self.name))?;
> + Ok(())
> + }
> +
> // this build's object with all of its programs loaded and its maps created and pinned under
> // `set`. Everything is loaded before any link is touched, so a verifier rejection of one
> // direction leaves the old state whole
> @@ -552,7 +730,6 @@ impl TcPrograms {
> &self,
> fill: impl FnOnce(&Staged<'_>) -> anyhow::Result<()>,
> ) -> anyhow::Result<()> {
> - let others = self.other_schemas()?;
> // programs pinned under a newer schema or by a newer version of the crate mean a newer
> // build runs here and this process is the stale one, so it leaves the state alone
> if let Some(newer) = self.newer_schema_installed()? {
> @@ -570,6 +747,7 @@ impl TcPrograms {
> version_text(self.version)
> );
> }
> + let others = self.other_schemas()?;
> std::fs::create_dir_all(self.links_dir())
> .with_context(|| format!("{}: create the links directory", self.name))?;
> let staging = self.staging_dir();
> @@ -716,6 +894,7 @@ impl TcPrograms {
> }
>
> fn reconcile(&self, desired: &HashSet<u32>) -> anyhow::Result<()> {
> + self.write_order()?;
> // opened before anything is detached, so a missing program fails the full apply before it
> // changes anything
> let mut progs = Vec::new();
> @@ -750,7 +929,7 @@ impl TcPrograms {
> continue;
> };
> let pin = tc::LinkPin { ifindex, name, dir };
> - match tc::attach_and_pin(prog, &pin, &self.link_pin_path(&pin)) {
> + match self.attach_link(prog, &pin, &self.link_pin_path(&pin)) {
> Ok(()) => {}
> Err(tc::Error::IfaceGone) => {
> log::info!("{}: {ifindex} vanished before the attach", self.name);
> @@ -828,7 +1007,7 @@ impl TcPrograms {
> if path.exists() {
> continue;
> }
> - match tc::attach_and_pin(prog, &pin, &path) {
> + match self.attach_link(prog, &pin, &path) {
> Ok(()) => {}
> Err(tc::Error::IfaceGone) => {
> log::info!("{}: {ifindex} vanished before the attach", self.name);
> @@ -848,8 +1027,94 @@ impl TcPrograms {
>
> const LINKS_DIR: &str = "links";
>
> -fn links_dir_of(subsystem: &str) -> PathBuf {
> - PathBuf::from(PIN_ROOT).join(subsystem).join(LINKS_DIR)
> +// the subsystem directories under the pin root, each with its name
> +fn subsystem_roots() -> std::io::Result<Vec<(PathBuf, String)>> {
> + let entries = match std::fs::read_dir(PIN_ROOT) {
> + Ok(entries) => entries,
> + Err(e) if e.kind() == ErrorKind::NotFound => return Ok(Vec::new()),
> + Err(e) => return Err(e),
> + };
> + let mut roots = Vec::new();
> + for entry in entries {
> + let entry = entry?;
> + if !entry.file_type()?.is_dir() {
> + continue;
> + }
> + roots.push((
> + entry.path(),
> + entry.file_name().to_string_lossy().into_owned(),
> + ));
> + }
> + Ok(roots)
> +}
> +
> +// the list under /run with the version of the crate that wrote it, empty and oldest until a full
> +// apply wrote one
> +fn read_order() -> anyhow::Result<([u64; 3], Vec<String>)> {
> + match std::fs::read_to_string(Path::new(RUN_ROOT).join(ORDER_FILE)) {
> + Ok(content) => Ok(parse_order(&content)),
> + Err(e) if e.kind() == ErrorKind::NotFound => Ok(([0, 0, 0], Vec::new())),
> + Err(e) => Err(e).context("read the order list"),
> + }
> +}
> +
> +// the version on the first line, then one name per line. A file without a version line is an
> +// older crate's and counts as oldest
We never had an older crate, so it should just be an error.
> +fn parse_order(content: &str) -> ([u64; 3], Vec<String>) {
> + let mut lines = content.lines().peekable();
> + let version = lines
> + .peek()
> + .and_then(|first| parse_version(first))
> + .inspect(|_| {
> + lines.next();
> + })
> + .unwrap_or([0, 0, 0]);
> + (version, lines.map(str::to_owned).collect())
> +}
> +
> +fn parse_version(text: &str) -> Option<[u64; 3]> {
> + let mut parts = text.split('.').map(|part| part.parse::<u64>().ok());
> + let version = [parts.next()??, parts.next()??, parts.next()??];
> + parts.next().is_none().then_some(version)
> +}
> +
> +// what a full apply writes, or nothing. The newest crate decides the order. A file written by
> +// the same or a newer crate keeps its order and only gets the names it lacks. A file written by
> +// an older crate is replaced by `list`, with the names only the file has kept in, each after
> +// the name it followed in the file
I'm not sure what that last part is supposed to tell me.
Also, is there an explanation somewhere *why* we do it this way?
And if I actually change `RUN_ORDER` but the same names are in there as
before, the order is not changed at all?
> +fn next_order(
> + written: &([u64; 3], Vec<String>),
> + version: [u64; 3],
> + list: &[&str],
And what "written" and "list" represent is also not clear.
> +) -> Option<([u64; 3], Vec<String>)> {
> + let (written_version, names) = written;
> + if *written_version < version {
> + let base: Vec<String> = list.iter().map(|name| name.to_string()).collect();
> + let extra: Vec<&str> = names.iter().map(String::as_str).collect();
We go from `written` -> `names` -> `extra` here. (Why 3 different words?)
And `list` becomes `base`.
> + return Some((version, merge_order(&base, &extra).unwrap_or(base)));
So now from the perspective of the merge call, `base` is then named
`written`, and `extra` is named `list`.
I get that if we want to retain things from something old, we swap the
parameters, but you do know rust allows you to just reuse a name with
`let x = foo(x);` :S
So anyway, what we do here is not *quite* just
(a.version < b.version)
? (b.version, merge(b, a))
: (a.version, merge(a, b))
but the difference is just
(a.version < b.version)
? (b.version, merge(b, a) or b) // written was older so we want to reapply everything
: (a.version, merge(a, b)) // we do nothing if written was newer and already contained all entries
^ And are my comments there above correct? If so, maybe write it that way
instead of, well, the current comments.
written -> current_state?
list -> run_order (as that's the const's name)?
> + }
> + merge_order(names, list).map(|merged| (*written_version, merged))
> +}
> +
> +// the written list with the names of `list` it lacks added, each after the listed name it
> +// follows, or none when it lacks none. The written names keep their order and a name the list
> +// dropped stays, so a move or a removal waits for the reboot
> +fn merge_order(written: &[String], list: &[&str]) -> Option<Vec<String>> {
> + let mut merged: Vec<String> = written.to_vec();
`written` already could be `Vec<String>`, there's only 1 user above and
one invocation could move, the other could just `.clone()`.
> + let mut added = false;
> + for (i, name) in list.iter().enumerate() {
> + if merged.iter().any(|m| m == name) {
> + continue;
> + }
> + let after = list[..i]
> + .iter()
> + .rev()
> + .find_map(|before| merged.iter().position(|m| m == before));
> + let at = after.map_or(0, |pos| pos + 1);
> + merged.insert(at, name.to_string());
> + added = true;
> + }
> + added.then_some(merged)
> }
>
> // the name of a build's program directory, its version and fingerprint, with colons for the
> @@ -890,6 +1155,31 @@ fn remove_dir_if_present(dir: &Path) -> std::io::Result<()> {
> }
> }
>
> +// where a new link goes among the neighbors' links, the index names the neighbor
> +enum Place {
> + Before(usize),
> + After(usize),
> + Last,
> +}
> +
> +// before the earliest neighbor that runs later, otherwise after the latest one that runs
> +// earlier. Each attach inserts into a chain sorted this way, so the chain stays sorted
> +fn place(rank: usize, neighbors: &[(usize, PathBuf)]) -> Place {
> + let ranked = neighbors.iter().map(|(r, _)| *r).enumerate();
> + let later = ranked
> + .clone()
> + .filter(|&(_, r)| r > rank)
> + .min_by_key(|&(_, r)| r);
> + if let Some((i, _)) = later {
> + return Place::Before(i);
> + }
> + let earlier = ranked.filter(|&(_, r)| r < rank).max_by_key(|&(_, r)| r);
> + match earlier {
> + Some((i, _)) => Place::After(i),
> + None => Place::Last,
> + }
> +}
> +
> // a program pin is a file under a build directory. A run killed between creating that
> // directory and pinning leaves it empty. Like the links, a directory that cannot be read may
> // hold anything, a missing one is empty
> @@ -1065,3 +1355,138 @@ impl<K, V> DerefMut for MapHandle<'_, K, V> {
> &mut self.map
> }
> }
> +
> +#[cfg(test)]
> +mod test {
> + use super::*;
> +
> + fn placed(rank: usize, ranks: &[usize]) -> String {
> + let neighbors: Vec<(usize, PathBuf)> = ranks
> + .iter()
> + .map(|&r| (r, PathBuf::from(format!("link-{r}"))))
> + .collect();
> + match place(rank, &neighbors) {
> + Place::Before(i) => format!("before {}", neighbors[i].1.display()),
> + Place::After(i) => format!("after {}", neighbors[i].1.display()),
> + Place::Last => "last".into(),
> + }
> + }
> +
> + #[test]
> + fn a_link_goes_before_the_earliest_later_neighbor() {
> + assert_eq!(placed(1, &[3, 2]), "before link-2");
> + assert_eq!(placed(1, &[0, 3, 2]), "before link-2");
> + }
> +
> + #[test]
> + fn a_link_goes_after_the_latest_earlier_neighbor() {
> + assert_eq!(placed(2, &[0, 1]), "after link-1");
> + assert_eq!(placed(3, &[1, 0]), "after link-1");
> + }
> +
> + #[test]
> + fn a_link_without_neighbors_goes_last() {
> + assert_eq!(placed(1, &[]), "last");
> + }
> +
> + #[test]
> + fn a_fingerprint_changes_with_the_program_selection() {
> + let obj = b"the same object";
> + let a = build_fingerprint(obj, &[(Direction::Ingress, "a")], &["m"]);
> + assert_ne!(
> + a,
> + build_fingerprint(obj, &[(Direction::Egress, "a")], &["m"])
> + );
> + assert_ne!(
> + a,
> + build_fingerprint(obj, &[(Direction::Ingress, "b")], &["m"])
> + );
> + assert_ne!(
> + a,
> + build_fingerprint(obj, &[(Direction::Ingress, "a")], &["n"])
> + );
> + assert_eq!(
> + a,
> + build_fingerprint(obj, &[(Direction::Ingress, "a")], &["m"])
> + );
> + }
> +
> + #[test]
> + fn a_build_directory_name_round_trips() {
> + let name = build_dir_name([0, 9, 12], 0x0123456789abcdef);
> + assert_eq!(name, "0:9:12-0123456789abcdef");
> + assert_eq!(
> + parse_build_dir_name(&name),
> + Some(([0, 9, 12], 0x0123456789abcdef))
> + );
> + assert_eq!(parse_build_dir_name("0123456789abcdef"), None);
> + assert_eq!(parse_build_dir_name("0.9.12-0123456789abcdef"), None);
> + assert!(parse_build_dir_name("0:10:0-0123456789abcdef") > parse_build_dir_name(&name));
> + }
> +
> + fn written(names: &[&str]) -> Vec<String> {
> + names.iter().map(|&n| n.to_owned()).collect()
> + }
> +
> + #[test]
> + fn a_written_list_with_every_name_stays_whatever_its_order() {
> + assert_eq!(merge_order(&written(&["a", "b"]), &["a", "b"]), None);
> + assert_eq!(merge_order(&written(&["a", "b"]), &["b", "a"]), None);
> + assert_eq!(merge_order(&written(&["b", "a", "x"]), &["a", "b"]), None);
> + }
> +
> + #[test]
> + fn the_order_file_starts_with_the_version_of_its_writer() {
> + assert_eq!(
> + parse_order("0.1.2\na\nb\n"),
> + ([0, 1, 2], written(&["a", "b"]))
> + );
> + assert_eq!(parse_order("a\nb\n"), ([0, 0, 0], written(&["a", "b"])));
> + assert_eq!(parse_order(""), ([0, 0, 0], written(&[])));
> + }
> +
> + #[test]
> + fn the_newest_crate_decides_the_order() {
> + let file = ([0, 1, 0], written(&["a", "b", "z"]));
> + // the same or a newer crate only adds what the file lacks
> + assert_eq!(next_order(&file, [0, 1, 0], &["b", "a"]), None);
> + assert_eq!(
> + next_order(&file, [0, 0, 9], &["x", "a"]),
> + Some(([0, 1, 0], written(&["x", "a", "b", "z"])))
> + );
> + // a newer crate writes its own order, the names only the file has follow theirs
> + assert_eq!(
> + next_order(&file, [0, 2, 0], &["b", "a"]),
> + Some(([0, 2, 0], written(&["b", "z", "a"])))
> + );
> + assert_eq!(
> + next_order(&([0, 0, 0], written(&[])), [0, 2, 0], &["a"]),
> + Some(([0, 2, 0], written(&["a"])))
> + );
> + }
> +
> + #[test]
> + fn a_new_name_goes_in_after_the_listed_name_it_follows() {
> + assert_eq!(
> + merge_order(&written(&[]), &["a", "b"]),
> + Some(written(&["a", "b"]))
> + );
> + assert_eq!(
> + merge_order(&written(&["a", "b"]), &["x", "a", "b"]),
> + Some(written(&["x", "a", "b"]))
> + );
> + assert_eq!(
> + merge_order(&written(&["a", "b"]), &["a", "x", "y", "b"]),
> + Some(written(&["a", "x", "y", "b"]))
> + );
> + // the written names keep their order, a dropped one stays
> + assert_eq!(
> + merge_order(&written(&["a", "b"]), &["b", "x", "a"]),
> + Some(written(&["a", "b", "x"]))
> + );
> + assert_eq!(
> + merge_order(&written(&["a", "b", "c"]), &["a", "x", "c"]),
> + Some(written(&["a", "x", "b", "c"]))
> + );
> + }
> +}
> diff --git a/proxmox-ebpf/src/tc.rs b/proxmox-ebpf/src/tc.rs
> index 09a02b2947cc..3c6ea54908d0 100644
> --- a/proxmox-ebpf/src/tc.rs
> +++ b/proxmox-ebpf/src/tc.rs
> @@ -24,6 +24,9 @@ pub(crate) enum Error {
> IfaceGone,
> /// The pin was gone by the time it was opened or removed, another attach or unplug took it.
> PinGone,
> + /// The link the call referred to is not attached to the interface, the neighbor of a
> + /// placement or the link of a swap.
> + NotAttached,
> Other(anyhow::Error),
> }
>
> @@ -33,6 +36,7 @@ impl std::fmt::Display for Error {
> Self::AlreadyAttached => f.write_str("link already in place"),
> Self::IfaceGone => f.write_str("interface is gone"),
> Self::PinGone => f.write_str("pin is gone"),
> + Self::NotAttached => f.write_str("link not attached"),
> Self::Other(e) if f.alternate() => write!(f, "{e:#}"),
> Self::Other(e) => write!(f, "{e}"),
> }
> @@ -61,6 +65,9 @@ impl From<ProgramError> for Error {
> ProgramError::SyscallError(s) if s.io_error.kind() == ErrorKind::AlreadyExists => {
> Self::AlreadyAttached
> }
> + ProgramError::SyscallError(s) if s.io_error.kind() == ErrorKind::NotFound => {
> + Self::NotAttached
> + }
> e => Self::Other(e.into()),
> }
> }
> @@ -183,18 +190,19 @@ impl LinkPin {
> /// filters on the interface's qdiscs, such as the `ingress` qdisc the rate limiter installs.
> /// `TC_ACT_UNSPEC` hands a packet on to the next program and then to the filters. Any other
> /// verdict ends the chain right there. So a subsystem's program returns `TC_ACT_UNSPEC` for
> -/// whatever it leaves alone. Programs of several subsystems run in attach order, each new link
> -/// goes last.
> +/// whatever it leaves alone. `order` is the link's place among the links already there, see
> +/// [`RUN_ORDER`](crate::subsystem::RUN_ORDER).
> pub(crate) fn attach_and_pin(
> prog: &mut SchedClassifier,
> pin: &LinkPin,
> pin_path: &Path,
> + order: LinkOrder,
> ) -> Result<(), Error> {
> let link_id = prog
> .attach_with_options(
> &pin.name,
> pin.dir.aya_type(),
> - TcAttachOptions::TcxOrder(LinkOrder::last()),
> + TcAttachOptions::TcxOrder(order),
> )
> .map_err(|e| match Error::from(e) {
> // the interface can go away between the name lookup and the attach
> --
> 2.47.3
>
>
>
>
>
--
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-11-06 13:30 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-04 17:29 [PATCH proxmox v8 0/6] add proxmox-ebpf library Hannes Laimer
2026-11-04 17:29 ` [PATCH proxmox v8 1/6] ebpf: add the crate with its BPF build Hannes Laimer
2026-11-04 17:29 ` [PATCH proxmox v8 2/6] ebpf: tc: add the link plumbing Hannes Laimer
2026-11-04 17:29 ` [PATCH proxmox v8 3/6] ebpf: subsystem: add the shared tc program handling Hannes Laimer
2026-11-05 16:20 ` Wolfgang Bumiller
2026-11-04 17:29 ` [PATCH proxmox v8 4/6] ebpf: subsystem: place subsystem links in a fixed order Hannes Laimer
2026-11-06 13:30 ` Wolfgang Bumiller
2026-11-04 17:29 ` [PATCH proxmox v8 5/6] ebpf: tests: add a native harness for the BPF C programs Hannes Laimer
2026-11-04 17:29 ` [PATCH proxmox v8 6/6] ebpf: debian: package the crate as a Rust library Hannes Laimer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.