patches and low-level development discussion
 help / color / mirror / code / Atom feed
From: Demi Marie Obenour <demiobenour@gmail.com>
To: Spectrum OS Development <devel@spectrum-os.org>,
	Yureka Lilian <yuka@yuka.dev>
Subject: Re: [DO_NOT_APPLY v3 2/3] integrate xdp-forwarder into net-vm
Date: Sat, 6 Sep 2025 13:07:12 -0400	[thread overview]
Message-ID: <292b0238-9fae-4266-bc0e-a3b007cfa05d@gmail.com> (raw)
In-Reply-To: <20250901201248.19794-2-yureka@cyberchaos.dev>


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

On 9/1/25 16:12, Yureka Lilian wrote:
> 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/prog_physical.c           |  37 +++
>  tools/xdp-forwarder/prog_router.c             |  43 +++
>  tools/xdp-forwarder/set_router_iface.c        |  32 ++
>  9 files changed, 556 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/prog_physical.c
>  create mode 100644 tools/xdp-forwarder/prog_router.c
>  create mode 100644 tools/xdp-forwarder/set_router_iface.c

I have quite a few comments here, but they are almost all about
the vendored helper code, not your code.  It's not written the
way I would write it, and the code for pushing and popping VLAN
tags is very slightly suboptimal.  Also, while it is correct in
the kernel/BPF context it uses practices which would be bad, if
not insecure, in userspace.

> 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;
> +};

It's better to use `unsigned char *` or `uint8_t *` here.  I believe that
clang can be made to issue a warning about arithmetic on `void *`.

> +/*
> + *	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));
> +}

Presumably the router only uses one of these tags, as using both would be
pointless complexity.  There is no need to interoperate with third-party
code that assumes that only the innermost tag uses ETH_P_8021Q.  Therefore,
one of these can be removed.

> +/* 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.
> + */

This function is only ever called with VLAN_MAX_DEPTH == 1, so it is significant
overkill for Spectrum's needs.

> +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;

In C, pointer arithmetic must produce a pointer that is either inside the
bounds of an allocation, or one past the end.  This means that this is
undefined behavior if nh->pos points to less than hdrsize bytes of memory.
To avoid this, it is better to write the code as follows *if* data_end is
known to be at least nh->pos, or if (as here) hdrsize is signed and known
to be non-negative:

	if (data_end - nh->pos < hdrsize)
		return -1;

If hdrsize is unsigned and might exceed PTRDIFF_MAX, the following is
needed:

	if ((data_end >= nh->pos) ||
	    (size_t)(data_end - nh->pos) < hdrsize)
		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;

Generally this is an antipattern, as it can cause (potentially exploitable)
overflow or wraparound if vlh is an integer, or undefined behavior if vlh
is a past-the-end pointer.  Subtracting sizeof(*vlh) from data_end will
also be undefined behavior if the frame is too short.  I recommend

		if (((const char *)data_end - (const char *)vlh)
		    <= (ptrdiff_t)sizeof(*vlh))
			break;

which is uglier, but safe.  This can be wrapped in a macro if you wish,
such as:

#define INC_WOULD_BE_OOB(ptr, end) \
	((const char *)(end) - (const char *)(ptr) < (ptrdiff_t)sizeof(*(ptr)))

		if (INC_WOULD_BE_OOB(vlh, data_end))
			break;

In the Linux kernel (specifically), one can get away with this because:

- Linux compiles with -fno-strict-overflow, so out-of-bounds pointer arithmetic
  is not undefined behavior.
- Linux reserves the top 4096 bytes of the address space for error pointers,
  so there will never be a valid pointer to this memory.
- sizeof(*vlh) < 4096 so if vlh is not an error pointer, vlh + 1 will not
  wrap around.

The same arguments all apply to BPF provided one compiles with -fno-strict-overflow
and (possibly) -fno-strict-aliasing.  The Linux kernel version is much easier to
read, which I suspect is why it gets used.  In userspace, one (sadly) needs to
use the uglier versions.

> +		h_proto = vlh->h_vlan_encapsulated_proto;
> +		if (vlans) /* collect VLAN ids */
> +			vlans->id[i] =
> +				(bpf_ntohs(vlh->h_vlan_TCI) & VLAN_VID_MASK);

Spectrum only uses parse_ethhdr(), which passes NULL for the
`vlans` argument.  Therefore, this branch is unreachable.

> +		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;

Only safe in kernel/BPF.

> +	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;

Only safe in kernel/BPF.

> +	hdrsize = iph->ihl * 4;
> +	/* Sanity check packet field is valid */
> +	if(hdrsize < sizeof(*iph))
> +		return -1;

This is normally a very bad pattern because of integer overflow and/or
wraparound, but in this case iph->ihl is a 4-bit bitfield and so
iph->ihl * 4 is no more than 15 * 4 == 60.

> +	/* Variable-length IPv4 header, need to use byte-based arithmetic */
> +	if (nh->pos + hdrsize > data_end)
> +		return -1;

See above: this is UB in userspace but safe in the kernel/BPF
with the correct compiler flags.  If hdrsize could exceed 4095
it would not be, but it cannot.

> +	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;

Only safe in kernel/BPF.

> +	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;

Only safe in kernel/BPF.

> +	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;

Only safe in kernel/BPF.

> +	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;

Only safe in kernel/BPF.

> +	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;

Only safe in kernel/BPF.

> +	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;

Only safe in kernel/BPF.

> +	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;

Unless the verifier requires otherwise, this only needs to be 12
bytes as the Ethertype is not restored, only the source and
destination MAC addresses.

> +	struct vlan_hdr *vlh;
> +	__be16 h_proto;
> +	int vlid;> +
> +	if (!proto_is_vlan(eth->h_proto))
> +		return -1;

Only safe because the caller did a bounds check.

> +	/* Careful with the parenthesis here */
> +	vlh = (void *)(eth + 1);
> +
> +	/* Still need to do bounds checking */
> +	if (vlh + 1 > data_end)
> +		return -1;

Only safe in kernel/BPF.

> +	/* 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;
> +}
> +
> +/* 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;

Again, this only needs to be 12 bytes, not 14, as the old Ethertype
is unchanged.  The only exception is if not doing this causes the BPF
verifier to complain.

> +	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;

Only safe in kernel/BPF.

> +	/* 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;

Only safe in kernel/BPF.

> +	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;
> +}
> +
> +/*
> + * 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/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;
> +
> +	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);
> +	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;
> +
> +	int vlid = vlan_tag_pop(ctx, eth);
> +	if (vlid < 0) {
> +		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..f1a2bac
> --- /dev/null
> +++ b/tools/xdp-forwarder/set_router_iface.c
> @@ -0,0 +1,32 @@
> +// 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>
> +
> +int main(int argc, char **argv)
> +{
> +	if (argc < 2) {
> +		fprintf(stderr, "missing interface name\n");
> +		return 1;
> +	}
> +
> +	int router_idx = if_nametoindex(argv[1]);
> +	if (router_idx <= 0) {
> +		perror("error getting router interface");
> +		return 1;
> +	}
> +
> +	int map_fd = bpf_obj_get("/sys/fs/bpf/router_iface");
> +	if (map_fd < 0) {
> +		perror("failed to open bpf map");
> +		return 1;
> +	}
> +
> +	int id = 0;
> +	if (bpf_map_update_elem(map_fd, &id, &router_idx, 0) < 0) {
> +		perror("failed to update bpf map");
> +		return 1;
> +	}
> +}-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

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

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

  parent reply	other threads:[~2025-09-06 17:07 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-01 20:12 [DO_NOT_APPLY v3 0/3] xdp-forwarder Yureka Lilian
2025-09-01 20:12 ` [DO_NOT_APPLY v3 1/3] add xdp-forwarder Yureka Lilian
2025-09-06 12:05   ` Alyssa Ross
     [not found]     ` <08193621-065b-466f-8703-5ae5ecf71788@yuka.dev>
2025-09-06 16:13       ` Alyssa Ross
2025-09-06 17:07   ` Demi Marie Obenour [this message]
2025-09-07  0:08     ` [DO_NOT_APPLY v3 2/3] integrate xdp-forwarder into net-vm Yureka
2025-09-01 20:12 ` Yureka Lilian
2025-09-06 16:08   ` Alyssa Ross
2025-09-01 20:12 ` [DO_NOT_APPLY v3 3/3] docs/architecture: add paragraph about networking Yureka Lilian
2025-09-08 10:05   ` 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=292b0238-9fae-4266-bc0e-a3b007cfa05d@gmail.com \
    --to=demiobenour@gmail.com \
    --cc=devel@spectrum-os.org \
    --cc=yuka@yuka.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).