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