* [PATCH v4 0/2] Fix build of forwarder, then improve it [not found] <20251008-fix-forwarder-build-v3-1-705d1636d4f1@gmail.com> @ 2025-10-21 19:27 ` Demi Marie Obenour 2025-10-21 19:27 ` [PATCH v4 1/2] tools/xdp-forwarder: Do not include libc headers in eBPF programs Demi Marie Obenour 2025-10-21 19:27 ` [PATCH v4 2/2] tools/xdp-forwarder: Simplify forwarder programs Demi Marie Obenour 0 siblings, 2 replies; 5+ messages in thread From: Demi Marie Obenour @ 2025-10-21 19:27 UTC (permalink / raw) To: Spectrum OS Development; +Cc: Demi Marie Obenour, Alyssa Ross The first patch fixes the build of the XDP forwarder and ensures it does not include any libc headers. The second patch is a significant cleanup and also should improve performance a tiny amount. Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com> --- Changes in v4: - Use find_program instead of passing clang path as Meson option. - Use path to Linux kernel package instead of path to include directory. - Split the patch into two parts: a minimal one that just fixes the build, and a bigger one that refactors the BPF programs. - Link to v3: https://spectrum-os.org/lists/archives/spectrum-devel/20251008-fix-forwarder-build-v3-1-a93e5156fb6a@gmail.com Changes in v3: - Fix build. - Clean up XDP programs. - Link to v2: https://spectrum-os.org/lists/archives/spectrum-devel/20251006-fix-forwarder-build-v2-1-6fdd7e05cb14@gmail.com Changes in v2: - Rewrite the programs to not use the helpers. - This doesn't build. Link to v1: https://spectrum-os.org/lists/archives/spectrum-devel/20251003-fix-forwarder-build-v1-1-856b78ae5656@gmail.com --- Demi Marie Obenour (2): tools/xdp-forwarder: Do not include libc headers in eBPF programs tools/xdp-forwarder: Simplify forwarder programs tools/default.nix | 11 +- tools/meson.options | 4 + tools/xdp-forwarder/helpers.h | 54 +++++++ tools/xdp-forwarder/meson.build | 11 +- tools/xdp-forwarder/parsing_helpers.h | 274 ---------------------------------- tools/xdp-forwarder/prog_physical.c | 52 ++++--- tools/xdp-forwarder/prog_router.c | 48 ++++-- tools/xdp-forwarder/rewrite_helpers.h | 146 ------------------ 8 files changed, 138 insertions(+), 462 deletions(-) --- base-commit: c5d5786d3dc938af0b279c542d1e43bce381b4b9 change-id: 20251003-fix-forwarder-build-2889ea9aec91 -- Sincerely, Demi Marie Obenour (she/her/hers) ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 1/2] tools/xdp-forwarder: Do not include libc headers in eBPF programs 2025-10-21 19:27 ` [PATCH v4 0/2] Fix build of forwarder, then improve it Demi Marie Obenour @ 2025-10-21 19:27 ` Demi Marie Obenour 2025-10-24 20:35 ` Alyssa Ross 2025-10-21 19:27 ` [PATCH v4 2/2] tools/xdp-forwarder: Simplify forwarder programs Demi Marie Obenour 1 sibling, 1 reply; 5+ messages in thread From: Demi Marie Obenour @ 2025-10-21 19:27 UTC (permalink / raw) To: Spectrum OS Development; +Cc: Demi Marie Obenour, Alyssa Ross The build happened to work on arm64 because the glibc arm64 headers don't support multilib. On x86_64, glibc headers assume that BPF is a 32-bit platform (because __x86_64__ isn't defined) and fail to find the 32-bit headers. This is not a glibc bug. Rather, BPF programs should not be including glibc headers. Most Linux headers are not trivial to include in BPF programs. The version of the headers meant for userspace use do include glibc headers, and that isn't supported in BPF. The version meant for building kernel modules does not, but using it requires much more complicated build system. Solve this problem by only including headers intended for use in BPF programs. These headers include declarations explicitly intended for use in BPF programs, so if they do pull in libc headers that is a bug. Nix's wrapped clang would pull in libc headers automatically and is not suitable when a target is specified explicitly. Therefore, use an unwrapped clang. Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com> --- To check that the program does not include libc headers, one can add #include <stdio.h> (or any other libc header) and check that one gets an appropriate error. Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com> --- tools/default.nix | 11 +++---- tools/meson.options | 4 +++ tools/xdp-forwarder/meson.build | 11 +++++-- tools/xdp-forwarder/parsing_helpers.h | 57 ----------------------------------- 4 files changed, 18 insertions(+), 65 deletions(-) diff --git a/tools/default.nix b/tools/default.nix index 2c6846c80073e7b64fb7a19488103f6cf97a4420..f4febc9aba394fad85f20a51bb33e6b3b9224bab 100644 --- a/tools/default.nix +++ b/tools/default.nix @@ -6,7 +6,7 @@ import ../lib/call-package.nix ( { src, lib, stdenv, fetchCrate, fetchurl, runCommand, buildPackages , meson, ninja, pkg-config, rustc , clang-tools, clippy, jq -, dbus +, dbus, linuxHeaders # clang 19 (current nixpkgs default) is too old to support -fwrapv-pointer , clang_21, libbpf , buildSupport ? false @@ -87,8 +87,8 @@ stdenv.mkDerivation (finalAttrs: { nativeBuildInputs = [ meson ninja ] ++ lib.optionals (appSupport || driverSupport) [ pkg-config ] ++ lib.optionals hostSupport [ rustc ] - ++ lib.optionals driverSupport [ clang_21 ]; - buildInputs = lib.optionals appSupport [ dbus ] ++ lib.optionals driverSupport [ libbpf ]; + ++ lib.optionals driverSupport [ clang_21.cc ]; + buildInputs = lib.optionals appSupport [ dbus ] ++ lib.optionals driverSupport [ libbpf linuxHeaders ]; postPatch = lib.optionals hostSupport (lib.concatMapStringsSep "\n" (crate: '' mkdir -p subprojects/packagecache @@ -104,11 +104,10 @@ stdenv.mkDerivation (finalAttrs: { "-Dtests=false" "-Dunwind=false" "-Dwerror=true" + ] ++ lib.optionals driverSupport [ + "-Dlinux-headers=${linuxHeaders}" ]; - # Not supported for target bpf - hardeningDisable = lib.optionals driverSupport [ "zerocallusedregs" ]; - passthru.tests = { clang-tidy = finalAttrs.finalPackage.overrideAttrs ( { name, src, nativeBuildInputs ? [], ... }: diff --git a/tools/meson.options b/tools/meson.options index 301efb9f677fdec57c8491fd6a6868f2d35cb076..2077cdeb33d6b962107b46733f855172ecfc499d 100644 --- a/tools/meson.options +++ b/tools/meson.options @@ -13,6 +13,10 @@ option('driver', type : 'boolean', value : false, option('hostfsrootdir', type : 'string', value : '/run/host', description : 'Path where the virtio-fs provided by the host will be mounted') +option('linux-headers', + type : 'string', + description : 'Path to Linux kernel package') + option('tests', type : 'boolean', description : 'Build tests') diff --git a/tools/xdp-forwarder/meson.build b/tools/xdp-forwarder/meson.build index b73130eb27b8000a102b0a8847ecb06b93a955d2..501540af4d3e786774cedc1bb092c9887a2f37d8 100644 --- a/tools/xdp-forwarder/meson.build +++ b/tools/xdp-forwarder/meson.build @@ -11,8 +11,13 @@ executable('set-router-iface', 'set_router_iface.c', clang = find_program('clang', native : true) +linux_headers_path = get_option('linux-headers') +if linux_headers_path == '' + error('Linux header path must be provided to build XDP forwarder') +endif + bpf_o_cmd = [ - clang.full_path(), + clang, '-fno-stack-protector', '-fno-strict-aliasing', '-fwrapv', '-fwrapv-pointer', @@ -22,10 +27,12 @@ bpf_o_cmd = [ '-Wno-sign-compare', '-O2', '-target', 'bpf', - '-I', meson.current_source_dir() + '/include', '-g', '-c', + '-std=gnu23', '-o', '@OUTPUT@', + '-I', libbpf.get_variable('includedir'), + '-I', linux_headers_path + '/include', '-MD', '-MQ', '@OUTPUT', '-MF', '@DEPFILE@', diff --git a/tools/xdp-forwarder/parsing_helpers.h b/tools/xdp-forwarder/parsing_helpers.h index da099346008bd58485af8308feb4d3391ceef8f5..1ea822100fdb9a75c2d28d34d93e6bb2b5d3ae26 100644 --- a/tools/xdp-forwarder/parsing_helpers.h +++ b/tools/xdp-forwarder/parsing_helpers.h @@ -26,8 +26,6 @@ #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> @@ -46,16 +44,6 @@ struct vlan_hdr { __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 @@ -175,51 +163,6 @@ static __always_inline int parse_iphdr(struct hdr_cursor *nh, 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 */ -- 2.51.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/2] tools/xdp-forwarder: Do not include libc headers in eBPF programs 2025-10-21 19:27 ` [PATCH v4 1/2] tools/xdp-forwarder: Do not include libc headers in eBPF programs Demi Marie Obenour @ 2025-10-24 20:35 ` Alyssa Ross 0 siblings, 0 replies; 5+ messages in thread From: Alyssa Ross @ 2025-10-24 20:35 UTC (permalink / raw) To: Demi Marie Obenour, Spectrum OS Development Cc: Demi Marie Obenour, Alyssa Ross This patch has been committed as db54efac07deee32f39bdec4e4b8b73674df18c7, which can be viewed online at https://spectrum-os.org/git/spectrum/commit/?id=db54efac07deee32f39bdec4e4b8b73674df18c7. This is an automated message. Send comments/questions/requests to: Alyssa Ross <hi@alyssa.is> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 2/2] tools/xdp-forwarder: Simplify forwarder programs 2025-10-21 19:27 ` [PATCH v4 0/2] Fix build of forwarder, then improve it Demi Marie Obenour 2025-10-21 19:27 ` [PATCH v4 1/2] tools/xdp-forwarder: Do not include libc headers in eBPF programs Demi Marie Obenour @ 2025-10-21 19:27 ` Demi Marie Obenour 2025-10-25 11:17 ` Alyssa Ross 1 sibling, 1 reply; 5+ messages in thread From: Demi Marie Obenour @ 2025-10-21 19:27 UTC (permalink / raw) To: Spectrum OS Development; +Cc: Demi Marie Obenour, Alyssa Ross The generic functions from the example header file are far more complex than necessary. They do unnecessary bounds, reload pointers more than necessary, and make unnecessary copies of packet data. Use simpler functions that pop and push vlan tags in the most straightforward way possible. These functions also check for invalid Ethernet headers, so there is no need for the calling code to parse the headers. The vendored code is no longer needed and is thus removed. Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com> --- tools/xdp-forwarder/helpers.h | 54 +++++++++ tools/xdp-forwarder/parsing_helpers.h | 217 ---------------------------------- tools/xdp-forwarder/prog_physical.c | 52 +++++--- tools/xdp-forwarder/prog_router.c | 48 +++++--- tools/xdp-forwarder/rewrite_helpers.h | 146 ----------------------- 5 files changed, 120 insertions(+), 397 deletions(-) diff --git a/tools/xdp-forwarder/helpers.h b/tools/xdp-forwarder/helpers.h new file mode 100644 index 0000000000000000000000000000000000000000..f9bcbf48a230bd374d1035a0f9da8d04dc301505 --- /dev/null +++ b/tools/xdp-forwarder/helpers.h @@ -0,0 +1,54 @@ +/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */ +/* SPDX-FileCopyrightText: 2021 The xdp-tutorial Authors */ +// SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev> +// SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com> +/* + * This file contains a helper for validating a VLAN tag, + * as well as structs used by both BPF programs. + */ + +#ifndef HELPERS_H +#define HELPERS_H + +#include <linux/bpf.h> +#include <bpf/bpf_endian.h> +#include <bpf/bpf_helpers.h> + +#define VLAN_ETHTYPE 0x8100 /* Ethertype for tagged frames */ + +struct ethhdr { + struct { + __u8 destination_mac[6]; + __u8 source_mac[6]; + } mac_addresses; + __be16 h_proto; +}; + +struct vlan_hdr { + __be16 h_vlan_TCI; + __be16 h_vlan_encapsulated_proto; +}; + +struct maybe_tagged_ethhdr { + union { + struct { + struct ethhdr eth; + struct vlan_hdr vlan; + } tagged; + struct { + struct vlan_hdr pad; + struct ethhdr eth; + } untagged; + }; +}; + +// The router doesn't support the PCP and DEI bits +// and they are not part of the VLAN tag. +// Therefore, ensure they are unset. +// Also reject VLAN 0, which is reserved. +static __always_inline bool vlan_tag_is_valid(__u32 tag) +{ + return tag >= 1 && tag <= 0x0FFF; +} + +#endif /* HELPERS_H */ diff --git a/tools/xdp-forwarder/parsing_helpers.h b/tools/xdp-forwarder/parsing_helpers.h deleted file mode 100644 index 1ea822100fdb9a75c2d28d34d93e6bb2b5d3ae26..0000000000000000000000000000000000000000 --- a/tools/xdp-forwarder/parsing_helpers.h +++ /dev/null @@ -1,217 +0,0 @@ -/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */ -/* SPDX-FileCopyrightText: 2021 The xdp-tutorial Authors */ -/* 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/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; -}; - -/* 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; -} - -/* - * 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/prog_physical.c b/tools/xdp-forwarder/prog_physical.c index 2b70654ebe21cb2504e6d26ca3b93cb4b62efb0f..0fdf0a8c3b09cc34d325b7a71f915b4f39e3bd86 100644 --- a/tools/xdp-forwarder/prog_physical.c +++ b/tools/xdp-forwarder/prog_physical.c @@ -1,12 +1,9 @@ -// SPDX-License-Identifier: EUPL-1.2+ +// SPDX-License-Identifier: EUPL-1.2+ AND (GPL-2.0-or-later OR BSD-2-Clause) +// SPDX-FileCopyrightText: 2021 The xdp-tutorial Authors // SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev> +// SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com> -#define VLAN_MAX_DEPTH 1 - -#include <linux/bpf.h> -#include <bpf/bpf_endian.h> -#include "parsing_helpers.h" -#include "rewrite_helpers.h" +#include "helpers.h" struct { __uint(type, BPF_MAP_TYPE_DEVMAP); @@ -16,24 +13,43 @@ struct { __uint(pinning, LIBBPF_PIN_BY_NAME); } router_iface SEC(".maps"); +static __always_inline bool vlan_tag_push(struct xdp_md *ctx, __u16 tag) +{ + struct maybe_tagged_ethhdr *hdr; + + // Add extra space at the front of the packet. + // Doing this first avoids reloading pointers and + // extra bounds checks later. + if (bpf_xdp_adjust_head(ctx, -(int)sizeof(hdr->untagged.pad))) + return false; + + hdr = (void *)(long)ctx->data; + if (hdr + 1 > (void *)(long)ctx->data_end) + return false; + + // Move the MAC addresses. + // Ethertype is already in the correct position. + __builtin_memmove(&hdr->tagged.eth.mac_addresses, + &hdr->untagged.eth.mac_addresses, + sizeof(hdr->tagged.eth.mac_addresses)); + + // Set the VLAN ID and the Ethertype of the frame. + hdr->tagged.vlan.h_vlan_TCI = bpf_htons((__u16)tag); + hdr->tagged.eth.h_proto = bpf_htons(VLAN_ETHTYPE); + return true; +} + 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, ð) < 0) - return XDP_DROP; + __u32 ingress_ifindex = ctx->ingress_ifindex; - if (ctx->ingress_ifindex < 1 || ctx->ingress_ifindex > VLAN_VID_MASK) + if (!vlan_tag_is_valid(ingress_ifindex)) return XDP_DROP; - if (vlan_tag_push(ctx, eth, ctx->ingress_ifindex) < 0) + if (!vlan_tag_push(ctx, (__u16)ingress_ifindex)) return XDP_DROP; + // Redirect to the router interface. return bpf_redirect_map(&router_iface, 0, 0); } diff --git a/tools/xdp-forwarder/prog_router.c b/tools/xdp-forwarder/prog_router.c index 5b0c8fa5c55e365c08fdb1be8f0613bdc67d8649..c69cb62b5c225a739e3d95f69b5094e76b576425 100644 --- a/tools/xdp-forwarder/prog_router.c +++ b/tools/xdp-forwarder/prog_router.c @@ -1,12 +1,9 @@ -// SPDX-License-Identifier: EUPL-1.2+ +// SPDX-License-Identifier: EUPL-1.2+ AND (GPL-2.0-or-later OR BSD-2-Clause) +// SPDX-FileCopyrightText: 2021 The xdp-tutorial Authors // SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev> +// SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com> -#define VLAN_MAX_DEPTH 1 - -#include <linux/bpf.h> -#include <bpf/bpf_endian.h> -#include "parsing_helpers.h" -#include "rewrite_helpers.h" +#include "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 @@ -19,23 +16,42 @@ struct { __uint(pinning, LIBBPF_PIN_BY_NAME); } router_iface SEC(".maps"); +static __always_inline bool vlan_tag_pop(struct xdp_md *ctx, __u16 *tag) +{ + struct maybe_tagged_ethhdr *hdr = (void *)(long)ctx->data; + if (hdr + 1 > (void *)(long)ctx->data_end) + return false; + + // 0x8A88 is also valid but the router does not + // use it. It's meant for service provider VLANs. + if (hdr->tagged.eth.h_proto != bpf_htons(VLAN_ETHTYPE)) + return false; + + *tag = bpf_ntohs(hdr->tagged.vlan.h_vlan_TCI); + + // Move the MAC addresses. + // Ethertype is already in correct position. + __builtin_memmove(&hdr->untagged.eth.mac_addresses, + &hdr->tagged.eth.mac_addresses, + sizeof(hdr->tagged.eth.mac_addresses)); + + // Move the head pointer to the new Ethernet header. + // Doing this last avoids needing to reload pointers + // and add extra bounds checks earlier. + return !bpf_xdp_adjust_head(ctx, (int)sizeof(hdr->untagged.pad)); +} 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; + __u16 vlid; - struct ethhdr *eth; - if (parse_ethhdr(&nh, data_end, ð) < 0) + if (!vlan_tag_pop(ctx, &vlid)) return XDP_DROP; - int vlid = vlan_tag_pop(ctx, eth); - if (vlid < 0) + if (!vlan_tag_is_valid(vlid)) return XDP_DROP; + // Redirect to the correct physical interface. return bpf_redirect(vlid, 0); } diff --git a/tools/xdp-forwarder/rewrite_helpers.h b/tools/xdp-forwarder/rewrite_helpers.h deleted file mode 100644 index 71aa23e038cc450087c1db20c00cbdbe1616b0a1..0000000000000000000000000000000000000000 --- a/tools/xdp-forwarder/rewrite_helpers.h +++ /dev/null @@ -1,146 +0,0 @@ -/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */ -/* SPDX-FileCopyrightText: 2019 The xdp-tutorial Authors */ -/* 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(ð_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, ð_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; - struct vlan_hdr *vlh; - - /* First copy the original Ethernet header */ - __builtin_memcpy(ð_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, ð_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; -} - -/* - * 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 */ -- 2.51.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4 2/2] tools/xdp-forwarder: Simplify forwarder programs 2025-10-21 19:27 ` [PATCH v4 2/2] tools/xdp-forwarder: Simplify forwarder programs Demi Marie Obenour @ 2025-10-25 11:17 ` Alyssa Ross 0 siblings, 0 replies; 5+ messages in thread From: Alyssa Ross @ 2025-10-25 11:17 UTC (permalink / raw) To: Yureka Lilian; +Cc: Spectrum OS Development, Demi Marie Obenour [-- Attachment #1: Type: text/plain, Size: 20547 bytes --] Demi Marie Obenour <demiobenour@gmail.com> writes: > The generic functions from the example header file are far more complex > than necessary. They do unnecessary bounds, reload pointers more than > necessary, and make unnecessary copies of packet data. Use simpler > functions that pop and push vlan tags in the most straightforward way > possible. These functions also check for invalid Ethernet headers, so > there is no need for the calling code to parse the headers. > > The vendored code is no longer needed and is thus removed. > > Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com> > --- > tools/xdp-forwarder/helpers.h | 54 +++++++++ > tools/xdp-forwarder/parsing_helpers.h | 217 ---------------------------------- > tools/xdp-forwarder/prog_physical.c | 52 +++++--- > tools/xdp-forwarder/prog_router.c | 48 +++++--- > tools/xdp-forwarder/rewrite_helpers.h | 146 ----------------------- > 5 files changed, 120 insertions(+), 397 deletions(-) Adding Yureka for this one. Yureka, I'm less keen on vendoring the headers from the tutorial since we discovered that they're designed to be built in an environment that's subtly broken. See e.g. [1]. If we can get better code by refactoring code originating in the headers from the tutorial headers, I think it's worth doing, but I haven't reviewed these changes in particular since you'll have a better picture of that than me. [1]: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/FLBBMJ2GTD4SCGTIMDAT6GQDXJMDFGVH/ > diff --git a/tools/xdp-forwarder/helpers.h b/tools/xdp-forwarder/helpers.h > new file mode 100644 > index 0000000000000000000000000000000000000000..f9bcbf48a230bd374d1035a0f9da8d04dc301505 > --- /dev/null > +++ b/tools/xdp-forwarder/helpers.h > @@ -0,0 +1,54 @@ > +/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */ > +/* SPDX-FileCopyrightText: 2021 The xdp-tutorial Authors */ > +// SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev> > +// SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com> > +/* > + * This file contains a helper for validating a VLAN tag, > + * as well as structs used by both BPF programs. > + */ > + > +#ifndef HELPERS_H > +#define HELPERS_H > + > +#include <linux/bpf.h> > +#include <bpf/bpf_endian.h> > +#include <bpf/bpf_helpers.h> > + > +#define VLAN_ETHTYPE 0x8100 /* Ethertype for tagged frames */ > + > +struct ethhdr { > + struct { > + __u8 destination_mac[6]; > + __u8 source_mac[6]; > + } mac_addresses; > + __be16 h_proto; > +}; > + > +struct vlan_hdr { > + __be16 h_vlan_TCI; > + __be16 h_vlan_encapsulated_proto; > +}; > + > +struct maybe_tagged_ethhdr { > + union { > + struct { > + struct ethhdr eth; > + struct vlan_hdr vlan; > + } tagged; > + struct { > + struct vlan_hdr pad; > + struct ethhdr eth; > + } untagged; > + }; > +}; > + > +// The router doesn't support the PCP and DEI bits > +// and they are not part of the VLAN tag. > +// Therefore, ensure they are unset. > +// Also reject VLAN 0, which is reserved. > +static __always_inline bool vlan_tag_is_valid(__u32 tag) > +{ > + return tag >= 1 && tag <= 0x0FFF; > +} > + > +#endif /* HELPERS_H */ > diff --git a/tools/xdp-forwarder/parsing_helpers.h b/tools/xdp-forwarder/parsing_helpers.h > deleted file mode 100644 > index 1ea822100fdb9a75c2d28d34d93e6bb2b5d3ae26..0000000000000000000000000000000000000000 > --- a/tools/xdp-forwarder/parsing_helpers.h > +++ /dev/null > @@ -1,217 +0,0 @@ > -/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */ > -/* SPDX-FileCopyrightText: 2021 The xdp-tutorial Authors */ > -/* 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/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; > -}; > - > -/* 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; > -} > - > -/* > - * 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/prog_physical.c b/tools/xdp-forwarder/prog_physical.c > index 2b70654ebe21cb2504e6d26ca3b93cb4b62efb0f..0fdf0a8c3b09cc34d325b7a71f915b4f39e3bd86 100644 > --- a/tools/xdp-forwarder/prog_physical.c > +++ b/tools/xdp-forwarder/prog_physical.c > @@ -1,12 +1,9 @@ > -// SPDX-License-Identifier: EUPL-1.2+ > +// SPDX-License-Identifier: EUPL-1.2+ AND (GPL-2.0-or-later OR BSD-2-Clause) > +// SPDX-FileCopyrightText: 2021 The xdp-tutorial Authors > // SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev> > +// SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com> > > -#define VLAN_MAX_DEPTH 1 > - > -#include <linux/bpf.h> > -#include <bpf/bpf_endian.h> > -#include "parsing_helpers.h" > -#include "rewrite_helpers.h" > +#include "helpers.h" > > struct { > __uint(type, BPF_MAP_TYPE_DEVMAP); > @@ -16,24 +13,43 @@ struct { > __uint(pinning, LIBBPF_PIN_BY_NAME); > } router_iface SEC(".maps"); > > +static __always_inline bool vlan_tag_push(struct xdp_md *ctx, __u16 tag) > +{ > + struct maybe_tagged_ethhdr *hdr; > + > + // Add extra space at the front of the packet. > + // Doing this first avoids reloading pointers and > + // extra bounds checks later. > + if (bpf_xdp_adjust_head(ctx, -(int)sizeof(hdr->untagged.pad))) > + return false; > + > + hdr = (void *)(long)ctx->data; > + if (hdr + 1 > (void *)(long)ctx->data_end) > + return false; > + > + // Move the MAC addresses. > + // Ethertype is already in the correct position. > + __builtin_memmove(&hdr->tagged.eth.mac_addresses, > + &hdr->untagged.eth.mac_addresses, > + sizeof(hdr->tagged.eth.mac_addresses)); > + > + // Set the VLAN ID and the Ethertype of the frame. > + hdr->tagged.vlan.h_vlan_TCI = bpf_htons((__u16)tag); > + hdr->tagged.eth.h_proto = bpf_htons(VLAN_ETHTYPE); > + return true; > +} > + > 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, ð) < 0) > - return XDP_DROP; > + __u32 ingress_ifindex = ctx->ingress_ifindex; > > - if (ctx->ingress_ifindex < 1 || ctx->ingress_ifindex > VLAN_VID_MASK) > + if (!vlan_tag_is_valid(ingress_ifindex)) > return XDP_DROP; > > - if (vlan_tag_push(ctx, eth, ctx->ingress_ifindex) < 0) > + if (!vlan_tag_push(ctx, (__u16)ingress_ifindex)) > return XDP_DROP; > > + // Redirect to the router interface. > return bpf_redirect_map(&router_iface, 0, 0); > } > diff --git a/tools/xdp-forwarder/prog_router.c b/tools/xdp-forwarder/prog_router.c > index 5b0c8fa5c55e365c08fdb1be8f0613bdc67d8649..c69cb62b5c225a739e3d95f69b5094e76b576425 100644 > --- a/tools/xdp-forwarder/prog_router.c > +++ b/tools/xdp-forwarder/prog_router.c > @@ -1,12 +1,9 @@ > -// SPDX-License-Identifier: EUPL-1.2+ > +// SPDX-License-Identifier: EUPL-1.2+ AND (GPL-2.0-or-later OR BSD-2-Clause) > +// SPDX-FileCopyrightText: 2021 The xdp-tutorial Authors > // SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev> > +// SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com> > > -#define VLAN_MAX_DEPTH 1 > - > -#include <linux/bpf.h> > -#include <bpf/bpf_endian.h> > -#include "parsing_helpers.h" > -#include "rewrite_helpers.h" > +#include "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 > @@ -19,23 +16,42 @@ struct { > __uint(pinning, LIBBPF_PIN_BY_NAME); > } router_iface SEC(".maps"); > > +static __always_inline bool vlan_tag_pop(struct xdp_md *ctx, __u16 *tag) > +{ > + struct maybe_tagged_ethhdr *hdr = (void *)(long)ctx->data; > + if (hdr + 1 > (void *)(long)ctx->data_end) > + return false; > + > + // 0x8A88 is also valid but the router does not > + // use it. It's meant for service provider VLANs. > + if (hdr->tagged.eth.h_proto != bpf_htons(VLAN_ETHTYPE)) > + return false; > + > + *tag = bpf_ntohs(hdr->tagged.vlan.h_vlan_TCI); > + > + // Move the MAC addresses. > + // Ethertype is already in correct position. > + __builtin_memmove(&hdr->untagged.eth.mac_addresses, > + &hdr->tagged.eth.mac_addresses, > + sizeof(hdr->tagged.eth.mac_addresses)); > + > + // Move the head pointer to the new Ethernet header. > + // Doing this last avoids needing to reload pointers > + // and add extra bounds checks earlier. > + return !bpf_xdp_adjust_head(ctx, (int)sizeof(hdr->untagged.pad)); > +} > > 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; > + __u16 vlid; > > - struct ethhdr *eth; > - if (parse_ethhdr(&nh, data_end, ð) < 0) > + if (!vlan_tag_pop(ctx, &vlid)) > return XDP_DROP; > > - int vlid = vlan_tag_pop(ctx, eth); > - if (vlid < 0) > + if (!vlan_tag_is_valid(vlid)) > return XDP_DROP; > > + // Redirect to the correct physical interface. > return bpf_redirect(vlid, 0); > } > diff --git a/tools/xdp-forwarder/rewrite_helpers.h b/tools/xdp-forwarder/rewrite_helpers.h > deleted file mode 100644 > index 71aa23e038cc450087c1db20c00cbdbe1616b0a1..0000000000000000000000000000000000000000 > --- a/tools/xdp-forwarder/rewrite_helpers.h > +++ /dev/null > @@ -1,146 +0,0 @@ > -/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */ > -/* SPDX-FileCopyrightText: 2019 The xdp-tutorial Authors */ > -/* 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(ð_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, ð_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; > - struct vlan_hdr *vlh; > - > - /* First copy the original Ethernet header */ > - __builtin_memcpy(ð_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, ð_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; > -} > - > -/* > - * 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 */ > > -- > 2.51.1 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 227 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-10-25 11:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20251008-fix-forwarder-build-v3-1-705d1636d4f1@gmail.com>
2025-10-21 19:27 ` [PATCH v4 0/2] Fix build of forwarder, then improve it Demi Marie Obenour
2025-10-21 19:27 ` [PATCH v4 1/2] tools/xdp-forwarder: Do not include libc headers in eBPF programs Demi Marie Obenour
2025-10-24 20:35 ` Alyssa Ross
2025-10-21 19:27 ` [PATCH v4 2/2] tools/xdp-forwarder: Simplify forwarder programs Demi Marie Obenour
2025-10-25 11:17 ` Alyssa Ross
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).