patches and low-level development discussion
 help / color / mirror / code / Atom feed
From: Yureka <yuka@yuka.dev>
To: Demi Marie Obenour <demiobenour@gmail.com>
Cc: devel@spectrum-os.org
Subject: [PATCH] tools: add xdp-forwarder
Date: Sun, 7 Sep 2025 22:28:05 +0200	[thread overview]
Message-ID: <9e9e071f-d999-476d-895a-9128dd36a0a5@yuka.dev> (raw)
In-Reply-To: <c77568fd-0d94-48f1-978f-7fb7fac02393@gmail.com>


On 9/7/25 21:25, Demi Marie Obenour wrote:
> 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: [...]

Not upstreamable


>
>> +/* 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): [...]

Not upstreamable

> 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
you're not sure this would be what?
>
>> +/*
>> + * 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.
Will do.
>
>> +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 mostly got inspiration from the systemd meson files. I'm not very 
familiar with meson, but I will try to add this.

>
> 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
it makes the [... what]?
>
>> +
>> + 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.
Thanks, will fix.
>
>> + 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;
> }
The "< 0" check is strictly an error check. In success cases, 
vlan_tag_pop returns exactly the VLAN ID.
>
>> + 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);
> }

I'm not sure what you intend with this. If there is no significant flaws 
in the upstream code, why should we write our own version?



  reply	other threads:[~2025-09-07 20:28 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
2025-09-07 20:28   ` Yureka [this message]
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=9e9e071f-d999-476d-895a-9128dd36a0a5@yuka.dev \
    --to=yuka@yuka.dev \
    --cc=demiobenour@gmail.com \
    --cc=devel@spectrum-os.org \
    /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).