patches and low-level development discussion
 help / color / mirror / code / Atom feed
From: Demi Marie Obenour <demiobenour@gmail.com>
To: Yureka Lilian <yureka@cyberchaos.dev>, devel@spectrum-os.org
Subject: Re: [PATCH] tools: add xdp-forwarder
Date: Sun, 7 Sep 2025 15:25:43 -0400	[thread overview]
Message-ID: <c77568fd-0d94-48f1-978f-7fb7fac02393@gmail.com> (raw)
In-Reply-To: <20250906141228.2357630-1-yureka@cyberchaos.dev>


[-- Attachment #1.1.1: Type: text/plain, Size: 32483 bytes --]

On 9/6/25 10:12, Yureka Lilian wrote:
> The xdp-forwarder's purpose is implementing the functionality needed
> within the net-vm (a VM running the Linux drivers for any physical
> interfaces on the spectrum system).
> 
> In the future, the net-vm will load the included XDP programs on the
> passed-through physical interfaces as well as the downstream virtio
> interface going into the router (recognized by its special MAC address).
> 
> The net-vm needs to multiplex between the physical interfaces, as there
> might be several interfaces in the same IOMMU-group.
> 
> For this, the XDP program loaded on the physical interfaces
> (`prog_physical.o`) applies a VLAN tag corresponding to the interface id
> and redirects the packets to the router interface (identified by the
> `router_iface` bpf map). In the other direction the XDP program loaded on
> the router interface (`prog_router.o`) removes one layer of VLAN tagging
> and redirects the packets to the interface read from the VLAN tag.
> 
> The helper program `set_router_iface` is used to update the `router_iface`
> bpf map to point to the interface passed as argument to the program.
> 
> Signed-off-by: Yureka Lilian <yureka@cyberchaos.dev>
> ---
>  pkgs/default.nix                              |   5 +
>  tools/default.nix                             |  15 +-
>  tools/meson.build                             |   5 +
>  tools/meson_options.txt                       |   4 +
>  tools/xdp-forwarder/include/parsing_helpers.h | 273 ++++++++++++++++++
>  tools/xdp-forwarder/include/rewrite_helpers.h | 145 ++++++++++
>  tools/xdp-forwarder/meson.build               |  38 +++
>  tools/xdp-forwarder/prog_physical.c           |  37 +++
>  tools/xdp-forwarder/prog_router.c             |  43 +++
>  tools/xdp-forwarder/set_router_iface.c        |  29 ++
>  10 files changed, 591 insertions(+), 3 deletions(-)
>  create mode 100644 tools/xdp-forwarder/include/parsing_helpers.h
>  create mode 100644 tools/xdp-forwarder/include/rewrite_helpers.h
>  create mode 100644 tools/xdp-forwarder/meson.build
>  create mode 100644 tools/xdp-forwarder/prog_physical.c
>  create mode 100644 tools/xdp-forwarder/prog_router.c
>  create mode 100644 tools/xdp-forwarder/set_router_iface.c
> 
> diff --git a/pkgs/default.nix b/pkgs/default.nix
> index 3b81339..76b2a5c 100644
> --- a/pkgs/default.nix
> +++ b/pkgs/default.nix
> @@ -1,4 +1,5 @@
>  # SPDX-FileCopyrightText: 2023-2024 Alyssa Ross <hi@alyssa.is>
> +# SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev>
>  # SPDX-License-Identifier: MIT
>  
>  { ... } @ args:
> @@ -42,6 +43,10 @@ let
>        guestSupport = false;
>        hostSupport = true;
>      };
> +    spectrum-driver-tools = self.callSpectrumPackage ../tools {
> +      guestSupport = false;
> +      driverSupport = true;
> +    };
>      xdg-desktop-portal-spectrum-host =
>        self.callSpectrumPackage ../tools/xdg-desktop-portal-spectrum-host {};
>  
> diff --git a/tools/default.nix b/tools/default.nix
> index 95d76a1..e664f47 100644
> --- a/tools/default.nix
> +++ b/tools/default.nix
> @@ -1,13 +1,16 @@
>  # SPDX-License-Identifier: MIT
>  # SPDX-FileCopyrightText: 2022-2025 Alyssa Ross <hi@alyssa.is>
> +# SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev>
>  
>  import ../lib/call-package.nix (
>  { src, lib, stdenv, fetchCrate, fetchurl, runCommand, buildPackages
>  , meson, ninja, pkg-config, rustc
>  , clang-tools, clippy
>  , dbus
> +, clang, libbpf
>  , guestSupport ? true
>  , hostSupport ? false
> +, driverSupport ? false
>  }:
>  
>  let
> @@ -70,15 +73,18 @@ stdenv.mkDerivation (finalAttrs: {
>        ./lsvm
>        ./start-vmm
>        ./subprojects
> +    ] ++ lib.optionals driverSupport [
> +      ./xdp-forwarder
>      ]));
>    };
>    sourceRoot = "source/tools";
>  
>    depsBuildBuild = lib.optionals hostSupport [ buildPackages.stdenv.cc ];
>    nativeBuildInputs = [ meson ninja ]
> -    ++ lib.optionals guestSupport [ pkg-config ]
> -    ++ lib.optionals hostSupport [ rustc ];
> -  buildInputs = lib.optionals guestSupport [ dbus ];
> +    ++ lib.optionals (guestSupport || driverSupport) [ pkg-config ]
> +    ++ lib.optionals hostSupport [ rustc ]
> +    ++ lib.optionals driverSupport [ clang ];
> +  buildInputs = lib.optionals guestSupport [ dbus ] ++ lib.optionals driverSupport [ libbpf ];
>  
>    postPatch = lib.optionals hostSupport (lib.concatMapStringsSep "\n" (crate: ''
>      mkdir -p subprojects/packagecache
> @@ -88,12 +94,15 @@ stdenv.mkDerivation (finalAttrs: {
>    mesonFlags = [
>      (lib.mesonBool "guest" guestSupport)
>      (lib.mesonBool "host" hostSupport)
> +    (lib.mesonBool "driver" driverSupport)
>      "-Dhostfsrootdir=/run/virtiofs/virtiofs0"
>      "-Dtests=false"
>      "-Dunwind=false"
>      "-Dwerror=true"
>    ];
>  
> +  hardeningDisable = lib.optionals driverSupport [ "zerocallusedregs" ];
> +
>    passthru.tests = {
>      clang-tidy = finalAttrs.finalPackage.overrideAttrs (
>        { name, src, nativeBuildInputs ? [], ... }:
> diff --git a/tools/meson.build b/tools/meson.build
> index 9cebd03..e49f27c 100644
> --- a/tools/meson.build
> +++ b/tools/meson.build
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: EUPL-1.2+
>  # SPDX-FileCopyrightText: 2024 Alyssa Ross <hi@alyssa.is>
> +# SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev>
>  
>  project('spectrum-tools', 'c',
>    default_options : {
> @@ -26,3 +27,7 @@ endif
>  if get_option('guest')
>    subdir('xdg-desktop-portal-spectrum')
>  endif
> +
> +if get_option('driver')
> +  subdir('xdp-forwarder')
> +endif
> diff --git a/tools/meson_options.txt b/tools/meson_options.txt
> index 4af0031..887e388 100644
> --- a/tools/meson_options.txt
> +++ b/tools/meson_options.txt
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: EUPL-1.2+
>  # SPDX-FileCopyrightText: 2022-2024 Alyssa Ross <hi@alyssa.is>
> +# SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev>
>  
>  option('host', type : 'boolean', value : false,
>    description : 'Build tools for the Spectrum host')
> @@ -7,6 +8,9 @@ option('host', type : 'boolean', value : false,
>  option('guest', type : 'boolean',
>    description : 'Build tools for Spectrum guests')
>  
> +option('driver', type : 'boolean',
> +  description : 'Build tools for Spectrum driver VMs')
> +
>  option('hostfsrootdir', type : 'string', value : '/run/host',
>    description : 'Path where the virtio-fs provided by the host will be mounted')
>  
> diff --git a/tools/xdp-forwarder/include/parsing_helpers.h b/tools/xdp-forwarder/include/parsing_helpers.h
> new file mode 100644
> index 0000000..3d240cd
> --- /dev/null
> +++ b/tools/xdp-forwarder/include/parsing_helpers.h
> @@ -0,0 +1,273 @@
> +/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-clause) */
> +/* Vendored from https://github.com/xdp-project/xdp-tutorial/blob/d3d3eed6ea9a63d1302bfa8b5a8e93862bfe11f0/common/parsing_helpers.h */
> +/*
> + * This file contains parsing functions that are used in the packetXX XDP
> + * programs. The functions are marked as __always_inline, and fully defined in
> + * this header file to be included in the BPF program.
> + *
> + * Each helper parses a packet header, including doing bounds checking, and
> + * returns the type of its contents if successful, and -1 otherwise.
> + *
> + * For Ethernet and IP headers, the content type is the type of the payload
> + * (h_proto for Ethernet, nexthdr for IPv6), for ICMP it is the ICMP type field.
> + * All return values are in host byte order.
> + *
> + * The versions of the functions included here are slightly expanded versions of
> + * the functions in the packet01 lesson. For instance, the Ethernet header
> + * parsing has support for parsing VLAN tags.
> + */
> +
> +#ifndef __PARSING_HELPERS_H
> +#define __PARSING_HELPERS_H
> +
> +#include <stddef.h>
> +#include <linux/if_ether.h>
> +#include <linux/if_packet.h>
> +#include <linux/ip.h>
> +#include <linux/ipv6.h>
> +#include <linux/icmp.h>
> +#include <linux/icmpv6.h>
> +#include <linux/udp.h>
> +#include <linux/tcp.h>
> +
> +/* Header cursor to keep track of current parsing position */
> +struct hdr_cursor {
> +	void *pos;
> +};
> +
> +/*
> + *	struct vlan_hdr - vlan header
> + *	@h_vlan_TCI: priority and VLAN ID
> + *	@h_vlan_encapsulated_proto: packet type ID or len
> + */
> +struct vlan_hdr {
> +	__be16 h_vlan_TCI;
> +	__be16 h_vlan_encapsulated_proto;
> +};
> +
> +/*
> + * Struct icmphdr_common represents the common part of the icmphdr and icmp6hdr
> + * structures.
> + */
> +struct icmphdr_common {
> +	__u8 type;
> +	__u8 code;
> +	__sum16 cksum;
> +};
> +
> +/* Allow users of header file to redefine VLAN max depth */
> +#ifndef VLAN_MAX_DEPTH
> +#define VLAN_MAX_DEPTH 2
> +#endif
> +
> +#define VLAN_VID_MASK           0x0fff /* VLAN Identifier */
> +/* Struct for collecting VLANs after parsing via parse_ethhdr_vlan */
> +struct collect_vlans {
> +	__u16 id[VLAN_MAX_DEPTH];
> +};
> +
> +static __always_inline int proto_is_vlan(__u16 h_proto)
> +{
> +	return !!(h_proto == bpf_htons(ETH_P_8021Q) ||
> +	          h_proto == bpf_htons(ETH_P_8021AD));
> +}
> +
> +/* Notice, parse_ethhdr() will skip VLAN tags, by advancing nh->pos and returns
> + * next header EtherType, BUT the ethhdr pointer supplied still points to the
> + * Ethernet header. Thus, caller can look at eth->h_proto to see if this was a
> + * VLAN tagged packet.
> + */
> +static __always_inline int parse_ethhdr_vlan(struct hdr_cursor *nh,
> +                                             void *data_end,
> +                                             struct ethhdr **ethhdr,
> +                                             struct collect_vlans *vlans)
> +{
> +	struct ethhdr *eth = nh->pos;
> +	int hdrsize = sizeof(*eth);
> +	struct vlan_hdr *vlh;
> +	__u16 h_proto;
> +	int i;
> +
> +	/* Byte-count bounds check; check if current pointer + size of header
> +	 * is after data_end.
> +	 */
> +	if (nh->pos + hdrsize > data_end)
> +		return -1;
> +
> +	nh->pos += hdrsize;
> +	*ethhdr = eth;
> +	vlh = nh->pos;
> +	h_proto = eth->h_proto;
> +
> +	/* Use loop unrolling to avoid the verifier restriction on loops;
> +	 * support up to VLAN_MAX_DEPTH layers of VLAN encapsulation.
> +	 */
> +	#pragma unroll
> +	for (i = 0; i < VLAN_MAX_DEPTH; i++) {
> +		if (!proto_is_vlan(h_proto))
> +			break;
> +
> +		if (vlh + 1 > data_end)
> +			break;
> +
> +		h_proto = vlh->h_vlan_encapsulated_proto;
> +		if (vlans) /* collect VLAN ids */
> +			vlans->id[i] =
> +				(bpf_ntohs(vlh->h_vlan_TCI) & VLAN_VID_MASK);
> +
> +		vlh++;
> +	}
> +
> +	nh->pos = vlh;
> +	return h_proto; /* network-byte-order */
> +}
> +
> +static __always_inline int parse_ethhdr(struct hdr_cursor *nh,
> +                                        void *data_end,
> +                                        struct ethhdr **ethhdr)
> +{
> +	/* Expect compiler removes the code that collects VLAN ids */
> +	return parse_ethhdr_vlan(nh, data_end, ethhdr, NULL);
> +}
> +
> +static __always_inline int parse_ip6hdr(struct hdr_cursor *nh,
> +                                        void *data_end,
> +                                        struct ipv6hdr **ip6hdr)
> +{
> +	struct ipv6hdr *ip6h = nh->pos;
> +
> +	/* Pointer-arithmetic bounds check; pointer +1 points to after end of
> +	 * thing being pointed to. We will be using this style in the remainder
> +	 * of the tutorial.
> +	 */
> +	if (ip6h + 1 > data_end)
> +		return -1;
> +
> +	nh->pos = ip6h + 1;
> +	*ip6hdr = ip6h;
> +
> +	return ip6h->nexthdr;
> +}
> +
> +static __always_inline int parse_iphdr(struct hdr_cursor *nh,
> +                                       void *data_end,
> +                                       struct iphdr **iphdr)
> +{
> +	struct iphdr *iph = nh->pos;
> +	int hdrsize;
> +
> +	if (iph + 1 > data_end)
> +		return -1;
> +
> +	hdrsize = iph->ihl * 4;
> +	/* Sanity check packet field is valid */
> +	if(hdrsize < sizeof(*iph))
> +		return -1;
> +
> +	/* Variable-length IPv4 header, need to use byte-based arithmetic */
> +	if (nh->pos + hdrsize > data_end)
> +		return -1;
> +
> +	nh->pos += hdrsize;
> +	*iphdr = iph;
> +
> +	return iph->protocol;
> +}
> +
> +static __always_inline int parse_icmp6hdr(struct hdr_cursor *nh,
> +                                          void *data_end,
> +                                          struct icmp6hdr **icmp6hdr)
> +{
> +	struct icmp6hdr *icmp6h = nh->pos;
> +
> +	if (icmp6h + 1 > data_end)
> +		return -1;
> +
> +	nh->pos   = icmp6h + 1;
> +	*icmp6hdr = icmp6h;
> +
> +	return icmp6h->icmp6_type;
> +}
> +
> +static __always_inline int parse_icmphdr(struct hdr_cursor *nh,
> +                                         void *data_end,
> +                                         struct icmphdr **icmphdr)
> +{
> +	struct icmphdr *icmph = nh->pos;
> +
> +	if (icmph + 1 > data_end)
> +		return -1;
> +
> +	nh->pos  = icmph + 1;
> +	*icmphdr = icmph;
> +
> +	return icmph->type;
> +}
> +
> +static __always_inline int parse_icmphdr_common(struct hdr_cursor *nh,
> +                                                void *data_end,
> +                                                struct icmphdr_common **icmphdr)
> +{
> +	struct icmphdr_common *h = nh->pos;
> +
> +	if (h + 1 > data_end)
> +		return -1;
> +
> +	nh->pos  = h + 1;
> +	*icmphdr = h;
> +
> +	return h->type;
> +}
> +
> +/*
> + * parse_udphdr: parse the udp header and return the length of the udp payload
> + */
> +static __always_inline int parse_udphdr(struct hdr_cursor *nh,
> +                                        void *data_end,
> +                                        struct udphdr **udphdr)
> +{
> +	int len;
> +	struct udphdr *h = nh->pos;
> +
> +	if (h + 1 > data_end)
> +		return -1;
> +
> +	nh->pos  = h + 1;
> +	*udphdr = h;
> +
> +	len = bpf_ntohs(h->len) - sizeof(struct udphdr);
> +	if (len < 0)
> +		return -1;
> +
> +	return len;
> +}
> +
> +/*
> + * parse_tcphdr: parse and return the length of the tcp header
> + */
> +static __always_inline int parse_tcphdr(struct hdr_cursor *nh,
> +                                        void *data_end,
> +                                        struct tcphdr **tcphdr)
> +{
> +	int len;
> +	struct tcphdr *h = nh->pos;
> +
> +	if (h + 1 > data_end)
> +		return -1;
> +
> +	len = h->doff * 4;
> +	/* Sanity check packet field is valid */
> +	if(len < sizeof(*h))
> +		return -1;
> +
> +	/* Variable-length TCP header, need to use byte-based arithmetic */
> +	if (nh->pos + len > data_end)
> +		return -1;
> +
> +	nh->pos += len;
> +	*tcphdr = h;
> +
> +	return len;
> +}
> +
> +#endif /* __PARSING_HELPERS_H */
> diff --git a/tools/xdp-forwarder/include/rewrite_helpers.h b/tools/xdp-forwarder/include/rewrite_helpers.h
> new file mode 100644
> index 0000000..2deae9a
> --- /dev/null
> +++ b/tools/xdp-forwarder/include/rewrite_helpers.h
> @@ -0,0 +1,145 @@
> +/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-clause) */
> +/* Vendored from https://github.com/xdp-project/xdp-tutorial/blob/d3d3eed6ea9a63d1302bfa8b5a8e93862bfe11f0/common/rewrite_helpers.h */
> +/*
> + * This file contains functions that are used in the packetXX XDP programs to
> + * manipulate on packets data. The functions are marked as __always_inline, and
> + * fully defined in this header file to be included in the BPF program.
> + */
> +
> +#ifndef __REWRITE_HELPERS_H
> +#define __REWRITE_HELPERS_H
> +
> +#include <linux/bpf.h>
> +#include <linux/ip.h>
> +#include <linux/ipv6.h>
> +#include <linux/if_ether.h>
> +
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_endian.h>
> +
> +/* Pops the outermost VLAN tag off the packet. Returns the popped VLAN ID on
> + * success or negative errno on failure.
> + */
> +static __always_inline int vlan_tag_pop(struct xdp_md *ctx, struct ethhdr *eth)
> +{
> +	void *data_end = (void *)(long)ctx->data_end;
> +	struct ethhdr eth_cpy;
> +	struct vlan_hdr *vlh;
> +	__be16 h_proto;
> +	int vlid;
> +
> +	if (!proto_is_vlan(eth->h_proto))
> +		return -1;
> +
> +	/* Careful with the parenthesis here */
> +	vlh = (void *)(eth + 1);
> +
> +	/* Still need to do bounds checking */
> +	if (vlh + 1 > data_end)
> +		return -1;
> +
> +	/* Save vlan ID for returning, h_proto for updating Ethernet header */
> +	vlid = bpf_ntohs(vlh->h_vlan_TCI);
> +	h_proto = vlh->h_vlan_encapsulated_proto;
> +
> +	/* Make a copy of the outer Ethernet header before we cut it off */
> +	__builtin_memcpy(&eth_cpy, eth, sizeof(eth_cpy));
> +
> +	/* Actually adjust the head pointer */
> +	if (bpf_xdp_adjust_head(ctx, (int)sizeof(*vlh)))
> +		return -1;
> +
> +	/* Need to re-evaluate data *and* data_end and do new bounds checking
> +	 * after adjusting head
> +	 */
> +	eth = (void *)(long)ctx->data;
> +	data_end = (void *)(long)ctx->data_end;
> +	if (eth + 1 > data_end)
> +		return -1;
> +
> +	/* Copy back the old Ethernet header and update the proto type */
> +	__builtin_memcpy(eth, &eth_cpy, sizeof(*eth));
> +	eth->h_proto = h_proto;
> +
> +	return vlid;
> +}

In Spectrum, eth is always the packet data, so this should be able to be
simplified to:

/* Pops the outermost VLAN tag off the packet. Returns the popped VLAN ID on
 * success or negative errno on failure.
 */
static __always_inline int vlan_tag_pop(struct xdp_md *ctx)
{

	char *data_end = (void *)(long)ctx->data_end;
	struct ethhdr *eth = (void *)(long)ctx->data;
	struct vlan_hdr *vlh;
	int vlid;

	/* Careful with the parenthesis here */
	vlh = (void *)(eth + 1);

	/* Still need to do bounds checking */
	if (vlh + 1 > data_end)
		return -1;

	if (!proto_is_vlan(eth->h_proto))
		return -1;

	/* Save vlan ID for returning */
	vlid = bpf_ntohs(vlh->h_vlan_TCI);

	/* Move the source and destination MAC addresses */
	__builtin_memmove(data + sizeof(*vlh), data,
	                  offsetof(struct ethhdr, h_proto));

	/* Actually adjust the head pointer */
	if (bpf_xdp_adjust_head(ctx, (int)sizeof(*vlh)))
		return -1;

	return vlid;
}

> +/* Pushes a new VLAN tag after the Ethernet header. Returns 0 on success,
> + * -1 on failure.
> + */
> +static __always_inline int vlan_tag_push(struct xdp_md *ctx,
> +                                         struct ethhdr *eth, int vlid)
> +{
> +	void *data_end = (void *)(long)ctx->data_end;
> +	struct ethhdr eth_cpy;
> +	struct vlan_hdr *vlh;
> +
> +	/* First copy the original Ethernet header */
> +	__builtin_memcpy(&eth_cpy, eth, sizeof(eth_cpy));
> +
> +	/* Then add space in front of the packet */
> +	if (bpf_xdp_adjust_head(ctx, 0 - (int)sizeof(*vlh)))
> +		return -1;
> +
> +	/* Need to re-evaluate data_end and data after head adjustment, and
> +	 * bounds check, even though we know there is enough space (as we
> +	 * increased it).
> +	 */
> +	data_end = (void *)(long)ctx->data_end;
> +	eth = (void *)(long)ctx->data;
> +
> +	if (eth + 1 > data_end)
> +		return -1;
> +
> +	/* Copy back Ethernet header in the right place, populate VLAN tag with
> +	 * ID and proto, and set outer Ethernet header to VLAN type.
> +	 */
> +	__builtin_memcpy(eth, &eth_cpy, sizeof(*eth));
> +
> +	vlh = (void *)(eth + 1);
> +
> +	if (vlh + 1 > data_end)
> +		return -1;
> +
> +	vlh->h_vlan_TCI = bpf_htons(vlid);
> +	vlh->h_vlan_encapsulated_proto = eth->h_proto;
> +
> +	eth->h_proto = bpf_htons(ETH_P_8021Q);
> +	return 0;
> +}

In the Spectrum use-case (where the Ethernet header is always the one in
the packet) this should be able to be simplified to (not tested):

/* Pushes a new VLAN tag after the Ethernet header. Returns 0 on success,
 * -1 on failure.
 */
static __always_inline int vlan_tag_push(struct xdp_md *ctx, int vlid)
{
	struct ethhdr *eth;
	__u16 *h_vlan_TCI;
	void *data_end;

	/* Add space in front of the packet */
	if (bpf_xdp_adjust_head(ctx, -(int)sizeof(struct vlan_hdr)))
		return -1;

	/* Need to refetch eth.  Also need to bounds check even
	 * though we know there is enough space (as we increased it).
	 */
	data_end = (void *)(long)ctx->data_end;
	eth = (void *)(long)ctx->data;
	h_vlan_TCI = (__u16 *)(eth + 1);
	if (h_vlan_TCI + 1 > data_end)
		return -1;

	/* Move source and destination MAC addresses, populate VLAN tag
	 * with ID and proto, and set outer Ethernet header to VLAN type.
	 */
	__builtin_memmove(eth, eth + sizeof(struct vlan_hdr),
	                  offsetof(struct ethhdr, h_proto));
	eth->h_proto = bpf_htons(ETH_P_8021Q);
	*h_vlan_TCI = bpf_htons(vlid);
	return 0;
}

This also avoids needing the 'eth' parameter, as it is now assigned in the
function body before being read from.  I'm not sure this would be

> +/*
> + * Swaps destination and source MAC addresses inside an Ethernet header
> + */
> +static __always_inline void swap_src_dst_mac(struct ethhdr *eth)
> +{
> +	__u8 h_tmp[ETH_ALEN];
> +
> +	__builtin_memcpy(h_tmp, eth->h_source, ETH_ALEN);
> +	__builtin_memcpy(eth->h_source, eth->h_dest, ETH_ALEN);
> +	__builtin_memcpy(eth->h_dest, h_tmp, ETH_ALEN);
> +}
> +
> +/*
> + * Swaps destination and source IPv6 addresses inside an IPv6 header
> + */
> +static __always_inline void swap_src_dst_ipv6(struct ipv6hdr *ipv6)
> +{
> +	struct in6_addr tmp = ipv6->saddr;
> +
> +	ipv6->saddr = ipv6->daddr;
> +	ipv6->daddr = tmp;
> +}
> +
> +/*
> + * Swaps destination and source IPv4 addresses inside an IPv4 header
> + */
> +static __always_inline void swap_src_dst_ipv4(struct iphdr *iphdr)
> +{
> +	__be32 tmp = iphdr->saddr;
> +
> +	iphdr->saddr = iphdr->daddr;
> +	iphdr->daddr = tmp;
> +}
> +
> +#endif /* __REWRITE_HELPERS_H */
> diff --git a/tools/xdp-forwarder/meson.build b/tools/xdp-forwarder/meson.build
> new file mode 100644
> index 0000000..7e60c11
> --- /dev/null
> +++ b/tools/xdp-forwarder/meson.build
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: EUPL-1.2+
> +# SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev>
> +
> +libbpf = dependency('libbpf', version : '1.6.2')
> +
> +executable('set_router_iface', 'set_router_iface.c',
> +  dependencies : libbpf,
> +  install : true)
> +
> +clang = find_program('clang')
> +
> +bpf_o_cmd = [
> +  clang.full_path(),
> +  '-fno-stack-protector',
> +  '-O2',
> +  '-target', 'bpf',
> +  '-I', meson.current_source_dir() + '/include',
> +  '-g',
> +  '-c',
> +  '@INPUT@',
> +  '-o',
> +  '@OUTPUT@',
> +]

I recommend adding -fno-strict-overflow and -fno-strict-aliasing
here.  I don't know if those are implied by -target bpf, but better
safe than sorry.

> +prog_router_o = custom_target(
> +  input : 'prog_router.c',
> +  output : 'prog_router.o',
> +  command : bpf_o_cmd,
> +  install: true,
> +  install_dir: 'lib/xdp')
> +
> +prog_physical_o = custom_target(
> +  input : 'prog_physical.c',
> +  output : 'prog_physical.o',
> +  command : bpf_o_cmd,
> +  install: true,
> +  install_dir: 'lib/xdp')

Meson allows providing a dependency file so that the
target is remade if any of the headers are changed.
Clang is able to produce such a file (-MD -MP -MF file).

I also recommend compiling with lots of warnings enabled
(perhaps -Wall -Wextra).

> diff --git a/tools/xdp-forwarder/prog_physical.c b/tools/xdp-forwarder/prog_physical.c
> new file mode 100644
> index 0000000..04b5131
> --- /dev/null
> +++ b/tools/xdp-forwarder/prog_physical.c
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: EUPL-1.2+
> +// SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev>
> +
> +#define VLAN_MAX_DEPTH 1
> +
> +#include <linux/bpf.h>
> +#include <bpf/bpf_endian.h>
> +#include "parsing_helpers.h"
> +#include "rewrite_helpers.h"
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_DEVMAP);
> +	__type(key, int);
> +	__type(value, int);
> +	__uint(max_entries, 1);
> +	__uint(pinning, LIBBPF_PIN_BY_NAME);
> +} router_iface SEC(".maps");
> +
> +SEC("xdp")
> +int physical(struct xdp_md *ctx)
> +{
> +	void *data_end = (void *)(long)ctx->data_end;
> +	void *data = (void *)(long)ctx->data;

Would it make sense to use char * here?  I know that Linux uses
arithmetic on void *, but it makes the

> +
> +	struct hdr_cursor nh;
> +	nh.pos = data;
> +
> +	struct ethhdr *eth;
> +	if (parse_ethhdr(&nh, data_end, &eth) < 0)
> +		return XDP_DROP;> +
> +
> +	if (ctx->ingress_ifindex < 1 || ctx->ingress_ifindex > 4096)
> +		return XDP_DROP;
> +
> +	vlan_tag_push(ctx, eth, ctx->ingress_ifindex);

Looks like a missing check for the return value.

> +	return bpf_redirect_map(&router_iface, 0, 0);
> +}
> diff --git a/tools/xdp-forwarder/prog_router.c b/tools/xdp-forwarder/prog_router.c
> new file mode 100644
> index 0000000..fe6a6b5
> --- /dev/null
> +++ b/tools/xdp-forwarder/prog_router.c
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: EUPL-1.2+
> +// SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev>
> +
> +#define VLAN_MAX_DEPTH 1
> +
> +#include <linux/bpf.h>
> +#include <bpf/bpf_endian.h>
> +#include "parsing_helpers.h"
> +#include "rewrite_helpers.h"
> +
> +// The map is actually not used by this program, but just included
> +// to keep the reference-counted pin alive before any physical interfaces
> +// are added.
> +struct {
> +	__uint(type, BPF_MAP_TYPE_DEVMAP);
> +	__type(key, int);
> +	__type(value, int);
> +	__uint(max_entries, 1);
> +	__uint(pinning, LIBBPF_PIN_BY_NAME);
> +} router_iface SEC(".maps");
> +
> +
> +SEC("xdp")
> +int router(struct xdp_md *ctx)
> +{
> +	void *data_end = (void *)(long)ctx->data_end;
> +	void *data = (void *)(long)ctx->data;
> +
> +	struct hdr_cursor nh;
> +	nh.pos = data;
> +
> +	struct ethhdr *eth;
> +	int r;
> +	if (r = parse_ethhdr(&nh, data_end, &eth) < 0)
> +		return XDP_DROP;

Did you mean

	if ((r = parse_ethhdr(&nh, data_end, &eth)) < 0)
		return XDP_DROP;

?

> +	int vlid = vlan_tag_pop(ctx, eth);
> +	if (vlid < 0) {
> +		return XDP_DROP;
> +	}

I don’t think Spectrum uses bits other than the VLAN ID,
and VLAN 0 is reserved, so perhaps:

	if (vlid < 1 || vlid > 4096) {
		return XDP_DROP;
	}

> +	return bpf_redirect(vlid, 0);
> +}
> diff --git a/tools/xdp-forwarder/set_router_iface.c b/tools/xdp-forwarder/set_router_iface.c
> new file mode 100644
> index 0000000..32835f9
> --- /dev/null
> +++ b/tools/xdp-forwarder/set_router_iface.c
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: EUPL-1.2+
> +// SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev>
> +
> +#include <stdio.h>
> +#include <net/if.h>
> +#include <bpf/bpf.h>
> +#include <err.h>
> +
> +int main(int argc, char **argv)
> +{
> +	if (argc < 2) {
> +		err(EXIT_FAILURE, "missing interface name");
> +	}
> +
> +	int router_idx = if_nametoindex(argv[1]);
> +	if (router_idx <= 0) {
> +		err(EXIT_FAILURE, "error getting router interface");
> +	}
> +
> +	int map_fd = bpf_obj_get("/sys/fs/bpf/router_iface");
> +	if (map_fd < 0) {
> +		err(EXIT_FAILURE, "failed to open bpf map");
> +	}
> +
> +	int id = 0;
> +	if (bpf_map_update_elem(map_fd, &id, &router_idx, 0) < 0) {
> +	        err(EXIT_FAILURE, "failed to update bpf map");
> +	}
> +}

I simplified the code into the following (only compile-tested):

/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-clause) */
/*
 * This file contains parsing functions that can be used in eXDP programs. The
 * functions are marked as __always_inline, and fully defined in this header
 * file to be included in the BPF program.
 *
 * Each helper parses a packet header, including doing bounds checking, and
 * returns the type of its contents if successful, and -1 otherwise.
 *
 * For Ethernet and IP headers, the content type is the type of the payload
 * (h_proto for Ethernet, nexthdr for IPv6), for ICMP it is the ICMP type field.
 * All return values are in host byte order.
 */

#include <stddef.h>
#include <linux/if_ether.h>
#include <linux/if_packet.h>
#include <linux/bpf.h>
#include <bpf/bpf_endian.h>
#include <bpf/bpf_helpers.h>

/*
 *	struct vlan_hdr - vlan header
 *	@h_vlan_TCI: priority and VLAN ID
 *	@h_vlan_encapsulated_proto: packet type ID or len
 */
struct vlan_hdr {
	__be16	h_vlan_TCI;
	__be16	h_vlan_encapsulated_proto;
} __attribute__((__packed__));

struct eth_vlan_hdr {
	struct ethhdr eth;
	struct vlan_hdr vlan;
} __attribute__((__packed__));

static __always_inline int proto_is_vlan(__u16 h_proto)
{
	return !!(h_proto == bpf_htons(ETH_P_8021Q) ||
		  h_proto == bpf_htons(ETH_P_8021AD));
}

/* Pushes a new VLAN tag after the Ethernet header. Returns 0 on success,
 * -1 on failure.
 */
static __always_inline int vlan_tag_push(struct xdp_md *ctx, int vlid)
{
	struct eth_vlan_hdr *hdr;
	char *data_end;
	char *data;

	/* Check for valid Ethernet frame */
	if ((unsigned long)ctx->data + ETH_ZLEN > (unsigned long)ctx->data_end)
		return -1;

	/* Add space in front of the packet */
	if (bpf_xdp_adjust_head(ctx, -(int)sizeof(struct vlan_hdr)))
		return -1;

	/* Must re-fetch header pointers */
	data_end = (char *)(long)ctx->data_end;
	data = (char *)(long)ctx->data;

	/* Verifier requires this (redundant) bounds check */
	if (data + ETH_ZLEN + sizeof(struct vlan_hdr) > data_end)
		return -1;

	/* Move source and destination MAC addresses, populate VLAN tag
	 * with ID and proto, and set outer Ethernet header to VLAN type.
	 */
	__builtin_memmove(data, data + sizeof(struct vlan_hdr),
	                  offsetof(struct ethhdr, h_proto));
	hdr = (struct eth_vlan_hdr *)data;
	/* TODO: should this be ETH_P_8021AD? */
	hdr->eth.h_proto = bpf_htons(ETH_P_8021Q);
	hdr->vlan.h_vlan_TCI = bpf_htons(vlid);
	return 0;
}

/* Pops the outermost VLAN tag off the packet. Returns the popped VLAN ID on
 * success or negative errno on failure.
 */
static __always_inline int vlan_tag_pop(struct xdp_md *ctx)
{

	char *data_end = (void *)(long)ctx->data_end;
	struct ethhdr *eth = (void *)(long)ctx->data;
	struct vlan_hdr *vlh;
	int vlid;

	/* Check that the produced Ethernet frame will be long enough */
	if ((unsigned long)eth + ETH_ZLEN + sizeof(*vlh) >
	    (unsigned long)data_end)
		return -1;

	/* Careful with the parenthesis here */
	vlh = (void *)(eth + 1);

	if (!proto_is_vlan(eth->h_proto))
		return -1;

	/* Save vlan ID for returning */
	vlid = bpf_ntohs(vlh->h_vlan_TCI);

	/* Move the source and destination MAC addresses.
	 * This moves the Ethernet header by 4 bytes, so
	 * be sure to cast to char * before doing pointer
	 * arithmetic.
	 */
	__builtin_memmove((char *)eth + sizeof(*vlh), eth,
	                  offsetof(struct ethhdr, h_proto));

	/* Actually adjust the head pointer */
	if (bpf_xdp_adjust_head(ctx, (int)sizeof(*vlh)))
		return -1;

	return vlid;
}

// The maps are actually not used by this program, but just included
// to keep the reference-counted pin alive before any physical interfaces
// are added.
struct {
	__uint(type, BPF_MAP_TYPE_DEVMAP);
	__type(key, int);
	__type(value, int);
	__uint(max_entries, 1);
	__uint(pinning, LIBBPF_PIN_BY_NAME);
} physical_iface SEC(".maps");

struct {
	__uint(type, BPF_MAP_TYPE_DEVMAP);
	__type(key, int);
	__type(value, int);
	__uint(max_entries, 1);
	__uint(pinning, LIBBPF_PIN_BY_NAME);
} router_iface SEC(".maps");

SEC("xdp")
int physical(struct xdp_md *ctx)
{
	if (ctx->ingress_ifindex < 1 || ctx->ingress_ifindex > 4096)
		return XDP_DROP;

	if (vlan_tag_push(ctx, ctx->ingress_ifindex))
		return XDP_DROP;

	return bpf_redirect_map(&physical_iface, 0, 0);
}

SEC("xdp")
int router(struct xdp_md *ctx)
{
	int vlid = vlan_tag_pop(ctx);

	/* Negative VLAN IDs indicate errors, and 0 is reserved and invalid.
	 * Values above 4096 indicate that the priority field is used, which
	 * isn't suppported out of simplicity.
	 */
	if (vlid < 1 || vlid > 4096)
		return XDP_DROP;

	return bpf_redirect(vlid, 0);
}
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2025-09-07 19:26 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-06 14:12 [PATCH] tools: add xdp-forwarder Yureka Lilian
2025-09-07 19:25 ` Demi Marie Obenour [this message]
2025-09-07 20:28   ` Yureka
2025-09-07 21:30     ` Demi Marie Obenour
2025-09-08  7:56 ` Alyssa Ross

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=c77568fd-0d94-48f1-978f-7fb7fac02393@gmail.com \
    --to=demiobenour@gmail.com \
    --cc=devel@spectrum-os.org \
    --cc=yureka@cyberchaos.dev \
    /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.
Code repositories for project(s) associated with this public inbox

	https://spectrum-os.org/git/crosvm
	https://spectrum-os.org/git/doc
	https://spectrum-os.org/git/mktuntap
	https://spectrum-os.org/git/nixpkgs
	https://spectrum-os.org/git/spectrum
	https://spectrum-os.org/git/ucspi-vsock
	https://spectrum-os.org/git/www

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).