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 657B21FF0B4 for ; Tue, 10 Nov 2026 14:50:43 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id ECEBB2166E; Tue, 10 Nov 2026 14:50:35 +0100 (CET) From: Hannes Laimer To: pve-devel@lists.proxmox.com Subject: [PATCH proxmox v9 2/5] ebpf: tc: add the link plumbing Date: Tue, 10 Nov 2026 14:50:02 +0100 Message-ID: <20261110135005.2847847-3-h.laimer@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261110135005.2847847-1-h.laimer@proxmox.com> References: <20261110135005.2847847-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: 1794318619883 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.296 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: CQNJANN753FJINS3O5KMGXIK3CCPEXDV X-Message-ID-Hash: CQNJANN753FJINS3O5KMGXIK3CCPEXDV 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 points the pinned link at a new program, so replacing a program does not interrupt traffic. The caller decides what a failure means for the interface. So a lost race against another attach, an interface that is gone and a pin that is gone each return their own error. Signed-off-by: Hannes Laimer --- proxmox-ebpf/src/lib.rs | 2 + proxmox-ebpf/src/tc.rs | 355 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 357 insertions(+) create mode 100644 proxmox-ebpf/src/tc.rs diff --git a/proxmox-ebpf/src/lib.rs b/proxmox-ebpf/src/lib.rs index 22e094233bed..b9babe319a41 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))] //! The shared code of eBPF subsystems, one subsystem per concern. Nothing here runs on its own, //! a subsystem describes its tc programs and calls into this crate. + +pub mod tc; diff --git a/proxmox-ebpf/src/tc.rs b/proxmox-ebpf/src/tc.rs new file mode 100644 index 000000000000..d99f705bfa88 --- /dev/null +++ b/proxmox-ebpf/src/tc.rs @@ -0,0 +1,355 @@ +//! TC link plumbing shared by all subsystems. A subsystem's programs attach as tcx links (kernel +//! 6.6 and newer), and [`LinkPin`] names their pins in bpffs. tcx runs its programs on the netdev +//! hook ahead of the filters on the interface's qdiscs, such as an `ingress` qdisc. +//! `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. + +use std::{io::ErrorKind, os::fd::AsRawFd, path::Path}; + +use anyhow::Context; +use aya::{ + pin::PinError, + programs::{ + MultiProgLink, ProgramError, SchedClassifier, TcAttachType, + links::{FdLink, LinkError, LinkOrder, PinnedLink}, + tc::{SchedClassifierLink, TcAttachOptions}, + }, +}; + +/// Why a link operation failed, 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. + AlreadyAttached, + /// The interface is gone, either no device under that ifindex exists, or the device at that + /// index has another name now. This can happen on a rename or a reused index. + IfaceGone, + /// The pin was gone by the time it was opened or removed. + 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 not allowing 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 means there is a link of this subsystem pinned at the path already, so +// the interface already has a link +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()) + } + } +} + +/// The tc hook a program attaches to. +#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)] +pub enum Direction { + Ingress, + Egress, +} + +impl Direction { + /// The lowercase name, as it stands in pin file names. + pub const fn as_str(self) -> &'static str { + match self { + Self::Ingress => "ingress", + Self::Egress => "egress", + } + } + pub(crate) fn aya_type(self) -> TcAttachType { + match self { + Self::Ingress => TcAttachType::Ingress, + Self::Egress => TcAttachType::Egress, + } + } + fn parse(s: &str) -> Option { + match s { + "ingress" => Some(Self::Ingress), + "egress" => Some(Self::Egress), + _ => None, + } + } +} + +/// One pinned link. Its filename contains the information this struct holds +/// +/// ```text +/// /sys/fs/bpf/proxmox-ebpf//links/:: +/// ``` +/// +/// The kernel does not allow ':' in interface names, so they are safe to use as a separator here. A +/// dot inside the interface name is also written as a ':', because the kernel does not allow a dot +/// in the filename in bpffs. We use this information to find links whose underlying device +/// disappeared, and to detect if an ifindex is reused. +#[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('.', ":") + ) + } + + /// Parses the filename. Names that are not parsable return [`Option::None`]. We did not create + /// them, so we ignore them. + pub fn parse(filename: &str) -> Option { + 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 }; + // we only allow interface names the kernel would allow, and that could exist as part of the + // link filename in the bpffs + let valid_name = !pin.name.is_empty() + && pin.name.len() < nix::libc::IF_NAMESIZE + && pin.name != "." + && pin.name != ".." + && !pin + .name + .chars() + .any(|c| (c.is_whitespace() && c.is_ascii()) || 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`. +/// 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(()) +} + +pub fn open_pinned_link(pin_path: &Path) -> Result { + let fd_link: FdLink = PinnedLink::from_pin(pin_path)?.into(); + Ok(fd_link.try_into()?) +} + +/// Update the program a link points to to `prog`. This is done with LINK_UPDATE, which is atomic, +/// and swaps the old program for `prog` without a single packet passing without a program. +pub 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(()) +} + +// `BPF_LINK_DETACH` in the kernel's `enum bpf_cmd`, aya has no call for it. +// FIXME: use aya's `PinnedLink::unpin` once the 6.14 kernel is EOL, from 6.18 on and in the stable +// kernels with the backport, unpinning and closing the link releases it right away +// commit 4f375ade6aa9 ("bpf: Avoid RCU context warning when unpinning htab with internal structs") +const BPF_LINK_DETACH: nix::libc::c_long = 34; + +/// Detach the link pinned at `pin_path` and remove its pin. The link is out of the interface's +/// chain when this returns. +/// +/// Removing the pin alone leaves the detach to the kernel. Kernels before 6.18 without the +/// backport of 4f375ade6aa9 only get to that after an RCU grace period, and an attach of the same +/// program in between is refused with `EEXIST`, since the old link still holds its place. +pub fn detach_pinned_link(pin_path: &Path) -> Result<(), Error> { + let link = open_pinned_link(pin_path)?; + // the `link_detach` member of the kernel's `union bpf_attr` is the link's fd and nothing else + let link_fd = link.fd()?.as_raw_fd() as u32; + // SAFETY: the call reads the four bytes of `link_fd`, which stays valid until it returns + let ret = unsafe { + nix::libc::syscall( + nix::libc::SYS_bpf, + BPF_LINK_DETACH, + &link_fd as *const u32, + std::mem::size_of::(), + ) + }; + if ret < 0 { + let err = anyhow::Error::new(std::io::Error::last_os_error()); + return Err(Error::Other(err.context("detach the link"))); + } + std::fs::remove_file(pin_path)?; + Ok(()) +} + +/// The name of an interface by index, none when it is gone. Any other failure returns `Err`. +pub fn iface_name(ifindex: u32) -> Result, anyhow::Error> { + let mut buf = [0u8; nix::libc::IF_NAMESIZE]; + // nix's if_indextoname takes only -1 as its error, so a NULL from libc comes back as `Ok` with + // an empty name + // 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())) +} + +/// Lists all link pins under +/// +/// ```text +/// /sys/fs/bpf/proxmox-ebpf//links/ +/// ``` +/// +/// The found link files are parsed into [`LinkPin`] as described there. Filenames we cannot parse +/// are ignored, we only care about the ones we place and manage. +pub fn read_pinned_links(links_dir: &Path) -> Result, anyhow::Error> { + 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