On Thu, 2025-01-02 at 19:24 +0100, Jose E. Marchesi wrote:
[...]
> IMO the BPP selftest (and BPF programs in general) must not include host
> glibc headers at all, regardless of what BPF compiler is used. The
> glibc headers installed in the host are tailored to some particular
> architecture, be it x86_64 or whatever, not necessarily compatible with
> what the compilers assume for the BPF target.
>
> This particular case shows the problem well: all the glibc headers
> included by that BPF selftest assume that `long' is 32 bits, not 64
> bits, because x86_64 is not defined. This conflicts with both clang's
> and GCC's assumption that in BPF a `long' is 64 bits. This may or may
> not be a problem, depending on whether the BPF program uses the stuff
> defined in the headers and how it uses it. Had you be using an arm or
> sparc host instead of x86_64, you may be including macros and stuff that
> assume chars are unsigned. But chars are signed in bpf.
This makes sense, but might cause some friction.
The following glibc headers are included directly from selftests:
- errno.h
- features.h
- inttypes.h
- limits.h
- netinet/in.h
- netinet/udp.h
- sched.h
- stdint.h
- stdlib.h
- string.h
- sys/socket.h
- sys/types.h
- time.h
- unistd.h
However, removing includes for these headers does not help the test in
question, because some linux UAPI headers include libc headers when exported:
In file included from /usr/include/netinet/udp.h:51,
from progs/test_cls_redirect_dynptr.c:20:
/home/eddy/work/tmp/gccbpf/lib/gcc/bpf-unknown-none/15.0.0/include/stdint.h:43:24: error: conflicting types for ‘int64_t’; have ‘long int’
43 | typedef __INT64_TYPE__ int64_t;
| ^~~~~~~
In file included from /usr/include/sys/types.h:155,
from /usr/include/bits/socket.h:29,
from /usr/include/sys/socket.h:33,
from /usr/include/linux/if.h:28,
from /usr/include/linux/icmp.h:23,
from progs/test_cls_redirect_dynptr.c:12:
/usr/include/bits/stdint-intn.h:27:19: note: previous declaration of ‘int64_t’ with type ‘int64_t’ {aka ‘long long int’}
27 | typedef __int64_t int64_t;
| ^~~~~~~
On my system (Fedora 41) the linux/{icmp,if}.h UAPI headers are
provided by kernel-headers package, sys/socket.h is provided by
glibc-devel package.
The UAPI headers have two modes depending whether __KERNEL__ is
defined. When used during kernel build the __KERNEL__ is defined and
there are no outside references. When exported for packages like
kernel-headers (via 'make headers' target) the __KERNEL__ is not
defined and there are some references to libc includes
(in fact, references to '#ifdef __KERNEL__' blocks are cut out during
headers export).
E.g. here is a fragment of linux/if.h, when viewed from kernel source:
#ifndef _LINUX_IF_H
#define _LINUX_IF_H
#include <linux/libc-compat.h> /* for compatibility with glibc */
#include <linux/types.h> /* for "__kernel_caddr_t" et al */
#include <linux/socket.h> /* for "struct sockaddr" et al */
#include <linux/compiler.h> /* for "__user" et al */
#ifndef __KERNEL__
#include <sys/socket.h> /* for struct sockaddr. */
#endif
And here is the same fragment as part of the kernel-headers package
(/usr/include/linux/if.h):
#ifndef _LINUX_IF_H
#define _LINUX_IF_H
#include <linux/libc-compat.h> /* for compatibility with glibc */
#include <linux/types.h> /* for "__kernel_caddr_t" et al */
#include <linux/socket.h> /* for "struct sockaddr" et al */
/* for "__user" et al */
#include <sys/socket.h> /* for struct sockaddr. */
As far as I understand, the idea right now is that BPF users can
install the kernel-headers package (or its equivalent) and start
hacking. If we declare that this is no longer a blessed way, people
would need to switch to packages like kernel-devel that provide full
set of kernel headers for use with dkms etc, e.g. for my system the
if.h would be located here:
/usr/src/kernels/6.12.6-200.fc41.x86_64/include/uapi/linux/if.h .
To me this seems logical, however potentially such change might have
implications for existing BPF code-base.
