public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Hannes Laimer <h.laimer@proxmox.com>
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	[thread overview]
Message-ID: <20261110135005.2847847-3-h.laimer@proxmox.com> (raw)
In-Reply-To: <20261110135005.2847847-1-h.laimer@proxmox.com>

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 <h.laimer@proxmox.com>
---
 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<anyhow::Error> 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<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 means there is a link of this subsystem pinned at the path already, so
+// the interface already has a link
+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.
+#[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<Self> {
+        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/<subsystem>/links/<ifindex>:<direction>:<name>
+/// ```
+///
+/// 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<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 };
+        // 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<SchedClassifierLink, Error> {
+    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::<u32>(),
+        )
+    };
+    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<Option<String>, 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/<subsystem>/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<Vec<LinkPin>, 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





  parent reply	other threads:[~2026-11-10 13:50 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-10 13:50 [PATCH proxmox v9 0/5] add proxmox-ebpf library Hannes Laimer
2026-11-10 13:50 ` [PATCH proxmox v9 1/5] ebpf: init crate for the code shared by subsystems Hannes Laimer
2026-11-10 13:50 ` Hannes Laimer [this message]
2026-11-10 13:50 ` [PATCH proxmox v9 3/5] ebpf: subsystem: add the shared tc program handling Hannes Laimer
2026-11-10 13:50 ` [PATCH proxmox v9 4/5] ebpf: subsystem: place subsystem links in a fixed order Hannes Laimer
2026-11-10 13:50 ` [PATCH proxmox v9 5/5] ebpf: debian: package the crate as a Rust library Hannes Laimer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261110135005.2847847-3-h.laimer@proxmox.com \
    --to=h.laimer@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal