Re: [PATCH v3] tools/xdp-forwarder: Do not include libc headers in eBPF programs

[Alyssa Ross <hi@alyssa.is>]

[-- Attachment #1: Type: text/plain, Size: 4633 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> 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.
> This also allows using an unwrapped clang.  Nix's wrapped clang is not
> suitable when a target is explicitly specified, so this is another a
> bug fix.
>
> This prevents vendoring the example headers, so replace them with a
> header that only includes common functionality common to both eBPF
> programs.  A single struct including both Ethernet II and 802.1Q headers
> is used, massively simplifying the code.

Could we put that cleanup into a separate patch, and keep this patch as
the smallest change possible necessary to fix the build?

> diff --git a/tools/default.nix b/tools/default.nix
> index 2c6846c80073e7b64fb7a19488103f6cf97a4420..8fd3491010303f981dac80cd6581614d594f993c 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
> @@ -88,7 +88,7 @@ stdenv.mkDerivation (finalAttrs: {
>      ++ lib.optionals (appSupport || driverSupport) [ pkg-config ]
>      ++ lib.optionals hostSupport [ rustc ]
>      ++ lib.optionals driverSupport [ clang_21 ];
> -  buildInputs = lib.optionals appSupport [ dbus ] ++ lib.optionals driverSupport [ libbpf ];
> +  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,11 @@ stdenv.mkDerivation (finalAttrs: {
>      "-Dtests=false"
>      "-Dunwind=false"
>      "-Dwerror=true"
> +  ] ++ lib.optionals driverSupport [
> +    "-Dclang=${clang_21.cc}/bin/clang"

llvmPackages_21.clang-unwrapped would prevent having to go through the
unwrapped compiler.

> +    "-Dlinux-headers=${linuxHeaders}/include"
>    ];
>  
> -  # 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..2aa82a3188f2912487d075c8f320a63c14878f91 100644
> --- a/tools/meson.options
> +++ b/tools/meson.options
> @@ -13,6 +13,13 @@ 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('clang', type : 'string',
> +  description : 'Path to clang')
> +
> +option('linux-headers',
> +  type : 'string',
> +  description : 'Path to Linux kernel headers')

Let's make this the package prefix, rather than the include directory.

> +
>  option('tests',
>    type : 'boolean',
>    description : 'Build tests')
> diff --git a/tools/xdp-forwarder/meson.build b/tools/xdp-forwarder/meson.build
> index b73130eb27b8000a102b0a8847ecb06b93a955d2..dec75c55884c4317af747b9a12e8f791d09d0a19 100644
> --- a/tools/xdp-forwarder/meson.build
> +++ b/tools/xdp-forwarder/meson.build
> @@ -9,10 +9,18 @@ executable('set-router-iface', 'set_router_iface.c',
>    dependencies : libbpf,
>    install : true)
>  
> -clang = find_program('clang', native : true)
> +clang_path = get_option('clang')
> +if clang_path == ''
> +  error('clang must be provided to build XDP forwarder')
> +endif

What was wrong with find_program()?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]