From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 81AD91FF09C for ; Mon, 19 Oct 2026 16:24:31 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 6F01521609; Mon, 19 Oct 2026 16:24:26 +0200 (CEST) From: Hannes Laimer To: pve-devel@lists.proxmox.com Subject: [PATCH proxmox-ebpf v4 1/5] tc: add the link plumbing Date: Mon, 19 Oct 2026 16:24:08 +0200 Message-ID: <20261019142413.2255644-2-h.laimer@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261019142413.2255644-1-h.laimer@proxmox.com> References: <20261019142413.2255644-1-h.laimer@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1792419860360 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.493 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: W26AT7WF7NLSGF2USJKMVWOHRJSBCVFR X-Message-ID-Hash: W26AT7WF7NLSGF2USJKMVWOHRJSBCVFR X-MailFrom: h.laimer@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- src/lib.rs | 2 + src/tc.rs | 303 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 305 insertions(+) create mode 100644 src/tc.rs diff --git a/src/lib.rs b/src/lib.rs index 244f44c..8d4420a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,2 +1,4 @@ //! 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/src/tc.rs b/src/tc.rs new file mode 100644 index 0000000..05d823e --- /dev/null +++ b/src/tc.rs @@ -0,0 +1,303 @@ +//! 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, str::FromStr}; + +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 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 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 the path taken by another attach, the link is there either way +impl From 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 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 for Error { + fn from(e: std::io::Error) -> Self { + if e.kind() == ErrorKind::NotFound { + Self::PinGone + } else { + Self::Other(e.into()) + } + } +} + +#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)] +pub enum Direction { + Ingress, + Egress, +} + +impl Direction { + pub fn as_str(self) -> &'static str { + match self { + Self::Ingress => "ingress", + Self::Egress => "egress", + } + } + pub fn aya_type(self) -> TcAttachType { + match self { + Self::Ingress => TcAttachType::Ingress, + Self::Egress => TcAttachType::Egress, + } + } +} + +impl FromStr for Direction { + type Err = (); + fn from_str(s: &str) -> Result { + match s { + "ingress" => Ok(Self::Ingress), + "egress" => Ok(Self::Egress), + _ => Err(()), + } + } +} + +/// 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 { + pub ifindex: u32, + pub name: String, + pub dir: Direction, +} + +impl LinkPin { + 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 { + let mut parts = filename.splitn(3, ':'); + let ifindex = parts.next()?.parse().ok()?; + let dir = parts.next()?.parse().ok()?; + 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()?; + // 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> { + 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 with a warning. +pub fn read_pinned_links(links_dir: &Path) -> anyhow::Result> { + 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::warn!( + "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