[PATCH bpf-next v1 00/19] selftests/bpf: Improve libc portability / musl support (part 1)
Hello all,
This series includes the bulk of libc-related compile fixes accumulated to support systems using musl, with smaller numbers to follow. These patches are simple and straightforward, and the series has been tested with the kernel-patches/bpf CI and locally using mips64el-gcc/musl-libc and QEMU with an OpenWrt rootfs.
The patches address a few general categories of libc portability issues:
- missing, redundant or incorrect include headers - disabled GNU header extensions (i.e. missing #define _GNU_SOURCE) - issues with types and casting
Feedback and suggestions for improvement are welcome!
Thanks, Tony
Tony Ambardar (19): selftests/bpf: Use pid_t consistently in test_progs.c selftests/bpf: Fix compile error from rlim_t in sk_storage_map.c selftests/bpf: Fix error compiling bpf_iter_setsockopt.c with musl libc selftests/bpf: Drop unneeded include in unpriv_helpers.c selftests/bpf: Drop unneeded include in sk_lookup.c selftests/bpf: Drop unneeded include in flow_dissector.c selftests/bpf: Fix missing ARRAY_SIZE() definition in bench.c selftests/bpf: Fix missing UINT_MAX definitions in benchmarks selftests/bpf: Fix missing BUILD_BUG_ON() declaration selftests/bpf: Fix include of <sys/fcntl.h> selftests/bpf: Fix compiling parse_tcp_hdr_opt.c with musl-libc selftests/bpf: Fix compiling kfree_skb.c with musl-libc selftests/bpf: Fix compiling flow_dissector.c with musl-libc selftests/bpf: Fix compiling tcp_rtt.c with musl-libc selftests/bpf: Fix compiling core_reloc.c with musl-libc selftests/bpf: Fix errors compiling lwt_redirect.c with musl libc selftests/bpf: Fix errors compiling decap_sanity.c with musl libc selftests/bpf: Fix errors compiling crypto_sanity.c with musl libc selftests/bpf: Fix errors compiling cg_storage_multi.h with musl libc
tools/testing/selftests/bpf/bench.c | 1 + tools/testing/selftests/bpf/bench.h | 1 + tools/testing/selftests/bpf/map_tests/sk_storage_map.c | 2 +- tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c | 2 +- tools/testing/selftests/bpf/prog_tests/core_reloc.c | 1 + tools/testing/selftests/bpf/prog_tests/crypto_sanity.c | 1 - tools/testing/selftests/bpf/prog_tests/decap_sanity.c | 1 - tools/testing/selftests/bpf/prog_tests/flow_dissector.c | 2 +- tools/testing/selftests/bpf/prog_tests/kfree_skb.c | 1 + tools/testing/selftests/bpf/prog_tests/lwt_redirect.c | 1 - tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c | 2 +- tools/testing/selftests/bpf/prog_tests/parse_tcp_hdr_opt.c | 1 + tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 1 - tools/testing/selftests/bpf/prog_tests/tcp_rtt.c | 1 + tools/testing/selftests/bpf/prog_tests/user_ringbuf.c | 1 + tools/testing/selftests/bpf/progs/cg_storage_multi.h | 2 -- tools/testing/selftests/bpf/test_progs.c | 2 +- tools/testing/selftests/bpf/unpriv_helpers.c | 1 - 18 files changed, 12 insertions(+), 12 deletions(-)
Use pid_t rather than __pid_t when allocating memory for 'worker_pids' in 'struct test_env', as this is its declared type and also avoids compile errors seen building against musl libc on mipsel64:
test_progs.c:1738:49: error: '__pid_t' undeclared (first use in this function); did you mean 'pid_t'? 1738 | env.worker_pids = calloc(sizeof(__pid_t), env.workers); | ^~~~~~~ | pid_t test_progs.c:1738:49: note: each undeclared identifier is reported only once for each function it appears in
Fixes: 91b2c0afd00c ("selftests/bpf: Add parallelism to test_progs") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/test_progs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index 1677f6f1eaae..091b49bf671a 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -1738,7 +1738,7 @@ int main(int argc, char **argv) /* launch workers if requested */ env.worker_id = -1; /* main process */ if (env.workers) { - env.worker_pids = calloc(sizeof(__pid_t), env.workers); + env.worker_pids = calloc(sizeof(pid_t), env.workers); env.worker_socks = calloc(sizeof(int), env.workers); if (env.debug) fprintf(stdout, "Launching %d workers.\n", env.workers);
On Mon, 2024-07-22 at 22:54 -0700, Tony Ambardar wrote:
Use pid_t rather than __pid_t when allocating memory for 'worker_pids' in 'struct test_env', as this is its declared type and also avoids compile errors seen building against musl libc on mipsel64:
test_progs.c:1738:49: error: '__pid_t' undeclared (first use in this function); did you mean 'pid_t'? 1738 | env.worker_pids = calloc(sizeof(__pid_t), env.workers); | ^~~~~~~ | pid_t test_progs.c:1738:49: note: each undeclared identifier is reported only once for each function it appears in
Fixes: 91b2c0afd00c ("selftests/bpf: Add parallelism to test_progs") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com
tools/testing/selftests/bpf/test_progs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index 1677f6f1eaae..091b49bf671a 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -1738,7 +1738,7 @@ int main(int argc, char **argv) /* launch workers if requested */ env.worker_id = -1; /* main process */ if (env.workers) {
env.worker_pids = calloc(sizeof(__pid_t),env.workers);
env.worker_pids = calloc(sizeof(pid_t),env.workers);
Yes, "pid_t" is much better. "worker_pids" dose define as "pid_t" in test_progs.h:
struct test_env { ... ... pid_t *worker_pids; /* array of worker pids */ ... ... };
Acked-by: Geliang Tang geliang@kernel.org
env.worker_socks = calloc(sizeof(int), env.workers); if (env.debug) fprintf(stdout, "Launching %d workers.\n", env.workers);
Cast 'rlim_t' argument to match expected type of printf() format and avoid compile errors seen building for mips64el/musl-libc:
In file included from map_tests/sk_storage_map.c:20: map_tests/sk_storage_map.c: In function 'test_sk_storage_map_stress_free': map_tests/sk_storage_map.c:414:56: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'rlim_t' {aka 'long long unsigned int'} [-Werror=format=] 414 | CHECK(err, "setrlimit(RLIMIT_NOFILE)", "rlim_new:%lu errno:%d", | ^~~~~~~~~~~~~~~~~~~~~~~ 415 | rlim_new.rlim_cur, errno); | ~~~~~~~~~~~~~~~~~ | | | rlim_t {aka long long unsigned int} ./test_maps.h:12:24: note: in definition of macro 'CHECK' 12 | printf(format); \ | ^~~~~~ map_tests/sk_storage_map.c:414:68: note: format string is defined here 414 | CHECK(err, "setrlimit(RLIMIT_NOFILE)", "rlim_new:%lu errno:%d", | ~~^ | | | long unsigned int | %llu cc1: all warnings being treated as errors
Fixes: 51a0e301a563 ("bpf: Add BPF_MAP_TYPE_SK_STORAGE test to test_maps") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/map_tests/sk_storage_map.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/map_tests/sk_storage_map.c b/tools/testing/selftests/bpf/map_tests/sk_storage_map.c index 18405c3b7cee..af10c309359a 100644 --- a/tools/testing/selftests/bpf/map_tests/sk_storage_map.c +++ b/tools/testing/selftests/bpf/map_tests/sk_storage_map.c @@ -412,7 +412,7 @@ static void test_sk_storage_map_stress_free(void) rlim_new.rlim_max = rlim_new.rlim_cur + 128; err = setrlimit(RLIMIT_NOFILE, &rlim_new); CHECK(err, "setrlimit(RLIMIT_NOFILE)", "rlim_new:%lu errno:%d", - rlim_new.rlim_cur, errno); + (unsigned long) rlim_new.rlim_cur, errno); }
err = do_sk_storage_map_stress_free();
Existing code calls getsockname() with a 'struct sockaddr_in6 *' argument where a 'struct sockaddr *' argument is declared, yielding compile errors when building for mips64el/musl-libc:
bpf_iter_setsockopt.c: In function 'get_local_port': bpf_iter_setsockopt.c:98:30: error: passing argument 2 of 'getsockname' from incompatible pointer type [-Werror=incompatible-pointer-types] 98 | if (!getsockname(fd, &addr, &addrlen)) | ^~~~~ | | | struct sockaddr_in6 * In file included from .../netinet/in.h:10, from .../arpa/inet.h:9, from ./test_progs.h:17, from bpf_iter_setsockopt.c:5: .../sys/socket.h:391:23: note: expected 'struct sockaddr * restrict' but argument is of type 'struct sockaddr_in6 *' 391 | int getsockname (int, struct sockaddr *__restrict, socklen_t *__restrict); | ^ cc1: all warnings being treated as errors
This compiled under glibc only because the argument is declared to be a "funky" transparent union which includes both types above. Explicitly cast the argument to allow compiling for both musl and glibc.
Fixes: eed92afdd14c ("bpf: selftest: Test batching and bpf_(get|set)sockopt in bpf tcp iter") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c index b52ff8ce34db..35363b104dd2 100644 --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c @@ -95,7 +95,7 @@ static unsigned short get_local_port(int fd) struct sockaddr_in6 addr; socklen_t addrlen = sizeof(addr);
- if (!getsockname(fd, &addr, &addrlen)) + if (!getsockname(fd, (struct sockaddr *) &addr, &addrlen)) return ntohs(addr.sin6_port);
return 0;
On Mon, 2024-07-22 at 22:54 -0700, Tony Ambardar wrote:
Existing code calls getsockname() with a 'struct sockaddr_in6 *' argument where a 'struct sockaddr *' argument is declared, yielding compile errors when building for mips64el/musl-libc:
bpf_iter_setsockopt.c: In function 'get_local_port': bpf_iter_setsockopt.c:98:30: error: passing argument 2 of 'getsockname' from incompatible pointer type [-Werror=incompatible- pointer-types] 98 | if (!getsockname(fd, &addr, &addrlen)) | ^~~~~ | | | struct sockaddr_in6 * In file included from .../netinet/in.h:10, from .../arpa/inet.h:9, from ./test_progs.h:17, from bpf_iter_setsockopt.c:5: .../sys/socket.h:391:23: note: expected 'struct sockaddr * restrict' but argument is of type 'struct sockaddr_in6 *' 391 | int getsockname (int, struct sockaddr *__restrict, socklen_t *__restrict); | ^ cc1: all warnings being treated as errors
This compiled under glibc only because the argument is declared to be a "funky" transparent union which includes both types above. Explicitly cast the argument to allow compiling for both musl and glibc.
Fixes: eed92afdd14c ("bpf: selftest: Test batching and bpf_(get|set)sockopt in bpf tcp iter") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com
tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c index b52ff8ce34db..35363b104dd2 100644 --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c @@ -95,7 +95,7 @@ static unsigned short get_local_port(int fd) struct sockaddr_in6 addr; socklen_t addrlen = sizeof(addr);
- if (!getsockname(fd, &addr, &addrlen))
- if (!getsockname(fd, (struct sockaddr *) &addr, &addrlen))
nit: Generally, a space is not required when casting:
(struct sockaddr *)&addr not
(struct sockaddr *) &addr.
See here:
$ grep -r "struct sockaddr *) &" tools/testing/selftests/bpf/prog_tests | wc -l 1 $ grep -r "struct sockaddr *)&" tools/testing/selftests/bpf/prog_tests | wc -l 33
Except this, LGTM.
Acked-by: Geliang Tang geliang@kernel.org
return ntohs(addr.sin6_port); return 0;
The addition of general support for unprivileged tests in test_loader.c breaks building test_verifier on non-glibc (e.g. musl) systems, due to the inclusion of glibc extension '<error.h>' in 'unpriv_helpers.c'. However, the header is actually not needed, so remove it to restore building.
Fixes: 1d56ade032a4 ("selftests/bpf: Unprivileged tests for test_loader.c") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/unpriv_helpers.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/unpriv_helpers.c b/tools/testing/selftests/bpf/unpriv_helpers.c index b6d016461fb0..220f6a963813 100644 --- a/tools/testing/selftests/bpf/unpriv_helpers.c +++ b/tools/testing/selftests/bpf/unpriv_helpers.c @@ -2,7 +2,6 @@
#include <stdbool.h> #include <stdlib.h> -#include <error.h> #include <stdio.h> #include <string.h> #include <unistd.h>
Test prog sk_lookup.c includes glibc extension '<error.h>' and fails to build for non-glibc (i.e. musl) systems. However, the header is actually not needed, so remove it to allow more portable compilation.
Fixes: 0ab5539f8584 ("selftests/bpf: Tests for BPF_SK_LOOKUP attach point") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c index ae87c00867ba..dcb2f62cdec6 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c @@ -18,7 +18,6 @@ #include <arpa/inet.h> #include <assert.h> #include <errno.h> -#include <error.h> #include <fcntl.h> #include <sched.h> #include <stdio.h>
Test prog flow_dissector.c includes glibc extension '<error.h>' and fails to build for non-glibc (i.e. musl) systems. However, the header is actually not needed, so remove it to allow more portable compilation.
Fixes: 0905beec9f52 ("selftests/bpf: run flow dissector tests in skb-less mode") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/prog_tests/flow_dissector.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/flow_dissector.c b/tools/testing/selftests/bpf/prog_tests/flow_dissector.c index 9e5f38739104..9625e6d21791 100644 --- a/tools/testing/selftests/bpf/prog_tests/flow_dissector.c +++ b/tools/testing/selftests/bpf/prog_tests/flow_dissector.c @@ -1,7 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 #include <test_progs.h> #include <network_helpers.h> -#include <error.h> #include <linux/if_tun.h> #include <sys/uio.h>
Add a "bpf_util.h" include to avoid the following error seen compiling for mips64el with musl libc:
bench.c: In function 'find_benchmark': bench.c:590:25: error: implicit declaration of function 'ARRAY_SIZE' [-Werror=implicit-function-declaration] 590 | for (i = 0; i < ARRAY_SIZE(benchs); i++) { | ^~~~~~~~~~ cc1: all warnings being treated as errors
Fixes: 8e7c2a023ac0 ("selftests/bpf: Add benchmark runner infrastructure") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/bench.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/bench.c b/tools/testing/selftests/bpf/bench.c index 627b74ae041b..90dc3aca32bd 100644 --- a/tools/testing/selftests/bpf/bench.c +++ b/tools/testing/selftests/bpf/bench.c @@ -10,6 +10,7 @@ #include <sys/sysinfo.h> #include <signal.h> #include "bench.h" +#include "bpf_util.h" #include "testing_helpers.h"
struct env env = {
On Mon, 2024-07-22 at 22:54 -0700, Tony Ambardar wrote:
Add a "bpf_util.h" include to avoid the following error seen compiling for mips64el with musl libc:
bench.c: In function 'find_benchmark': bench.c:590:25: error: implicit declaration of function 'ARRAY_SIZE' [-Werror=implicit-function-declaration] 590 | for (i = 0; i < ARRAY_SIZE(benchs); i++) { | ^~~~~~~~~~ cc1: all warnings being treated as errors
I'm curious why this error doesn't occur on other platforms. ARRAY_SIZE is actually defined in linux/kernel.h (tools/include/linux/kernel.h). I think you should find out why this file is not included on your platform.
Fixes: 8e7c2a023ac0 ("selftests/bpf: Add benchmark runner infrastructure") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com
tools/testing/selftests/bpf/bench.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/bench.c b/tools/testing/selftests/bpf/bench.c index 627b74ae041b..90dc3aca32bd 100644 --- a/tools/testing/selftests/bpf/bench.c +++ b/tools/testing/selftests/bpf/bench.c @@ -10,6 +10,7 @@ #include <sys/sysinfo.h> #include <signal.h> #include "bench.h" +#include "bpf_util.h" #include "testing_helpers.h" struct env env = {
On Wed, Jul 24, 2024 at 03:08:31PM +0800, Geliang Tang wrote:
On Mon, 2024-07-22 at 22:54 -0700, Tony Ambardar wrote:
Add a "bpf_util.h" include to avoid the following error seen compiling for mips64el with musl libc:
bench.c: In function 'find_benchmark': bench.c:590:25: error: implicit declaration of function 'ARRAY_SIZE' [-Werror=implicit-function-declaration] 590 | for (i = 0; i < ARRAY_SIZE(benchs); i++) { | ^~~~~~~~~~ cc1: all warnings being treated as errors
I'm curious why this error doesn't occur on other platforms. ARRAY_SIZE is actually defined in linux/kernel.h (tools/include/linux/kernel.h). I think you should find out why this file is not included on your platform.
The surprising reason is that it compiles on glibc systems by accident, due to bench.c using get_nprocs() and needing to include <sys/sysinfo.h>. On musl systems this header defines 'struct sysinfo', but the glibc header version pulls in <linux/kernel.h> for this struct, which in turn drags in ARRAY_SIZE and the rest of the kitchen sink. So not very portable...
Fixes: 8e7c2a023ac0 ("selftests/bpf: Add benchmark runner infrastructure") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com
tools/testing/selftests/bpf/bench.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/bench.c b/tools/testing/selftests/bpf/bench.c index 627b74ae041b..90dc3aca32bd 100644 --- a/tools/testing/selftests/bpf/bench.c +++ b/tools/testing/selftests/bpf/bench.c @@ -10,6 +10,7 @@ #include <sys/sysinfo.h> #include <signal.h> #include "bench.h" +#include "bpf_util.h" #include "testing_helpers.h" struct env env = {
Include <limits.h> in 'bench.h' to provide a UINT_MAX definition and avoid multiple compile errors against mips64el/musl-libc like:
benchs/bench_local_storage.c: In function 'parse_arg': benchs/bench_local_storage.c:40:38: error: 'UINT_MAX' undeclared (first use in this function) 40 | if (ret < 1 || ret > UINT_MAX) { | ^~~~~~~~ benchs/bench_local_storage.c:11:1: note: 'UINT_MAX' is defined in header '<limits.h>'; did you forget to '#include <limits.h>'? 10 | #include <test_btf.h> +++ |+#include <limits.h> 11 |
seen with bench_local_storage.c, bench_local_storage_rcu_tasks_trace.c, and bench_bpf_hashmap_lookup.c.
Fixes: 73087489250d ("selftests/bpf: Add benchmark for local_storage get") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/bench.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/bench.h b/tools/testing/selftests/bpf/bench.h index 68180d8f8558..005c401b3e22 100644 --- a/tools/testing/selftests/bpf/bench.h +++ b/tools/testing/selftests/bpf/bench.h @@ -10,6 +10,7 @@ #include <math.h> #include <time.h> #include <sys/syscall.h> +#include <limits.h>
struct cpu_set { bool *cpus;
Explicitly include '<linux/build_bug.h>' to fix errors seen compiling with gcc targeting mips64el/musl-libc:
user_ringbuf.c: In function 'test_user_ringbuf_loop': user_ringbuf.c:426:9: error: implicit declaration of function 'BUILD_BUG_ON' [-Werror=implicit-function-declaration] 426 | BUILD_BUG_ON(total_samples <= c_max_entries); | ^~~~~~~~~~~~ cc1: all warnings being treated as errors
Fixes: e5a9df51c746 ("selftests/bpf: Add selftests validating the user ringbuf") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/prog_tests/user_ringbuf.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c index e51721df14fc..dfff6feac12c 100644 --- a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c +++ b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c @@ -4,6 +4,7 @@ #define _GNU_SOURCE #include <linux/compiler.h> #include <linux/ring_buffer.h> +#include <linux/build_bug.h> #include <pthread.h> #include <stdio.h> #include <stdlib.h>
Update ns_current_pid_tgid.c to use '#include <fcntl.h>' and avoid compile error against mips64el/musl libc:
In file included from .../prog_tests/ns_current_pid_tgid.c:14: .../include/sys/fcntl.h:1:2: error: #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h> [-Werror=cpp] 1 | #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h> | ^~~~~~~ cc1: all warnings being treated as errors
Fixes: 09c02d553c49 ("bpf, selftests: Fold test_current_pid_tgid_new_ns into test_progs.") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c index e72d75d6baa7..c29787e092d6 100644 --- a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c +++ b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c @@ -11,7 +11,7 @@ #include <sched.h> #include <sys/wait.h> #include <sys/mount.h> -#include <sys/fcntl.h> +#include <fcntl.h> #include "network_helpers.h"
#define STACK_SIZE (1024 * 1024)
The GNU version of 'struct tcphdr', with members 'doff' and 'urg_ptr', is not exposed by musl headers unless _GNU_SOURCE is defined.
Add this definition to fix errors seen compiling for mips64el/musl-libc:
parse_tcp_hdr_opt.c:18:21: error: 'struct tcphdr' has no member named 'urg_ptr' 18 | .pk6_v6.tcp.urg_ptr = 123, | ^~~~~~~ parse_tcp_hdr_opt.c:19:21: error: 'struct tcphdr' has no member named 'doff' 19 | .pk6_v6.tcp.doff = 9, /* 16 bytes of options */ | ^~~~
Fixes: cfa7b011894d ("selftests/bpf: tests for using dynptrs to parse skb and xdp buffers") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/prog_tests/parse_tcp_hdr_opt.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/parse_tcp_hdr_opt.c b/tools/testing/selftests/bpf/prog_tests/parse_tcp_hdr_opt.c index daa952711d8f..e9c07d561ded 100644 --- a/tools/testing/selftests/bpf/prog_tests/parse_tcp_hdr_opt.c +++ b/tools/testing/selftests/bpf/prog_tests/parse_tcp_hdr_opt.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE #include <test_progs.h> #include <network_helpers.h> #include "test_parse_tcp_hdr_opt.skel.h"
The GNU version of 'struct tcphdr' with member 'doff' is not exposed by musl headers unless _GNU_SOURCE is defined. Add this definition to fix errors seen compiling for mips64el/musl-libc:
In file included from kfree_skb.c:2: kfree_skb.c: In function 'on_sample': kfree_skb.c:45:30: error: 'struct tcphdr' has no member named 'doff' 45 | if (CHECK(pkt_v6->tcp.doff != 5, "check_tcp", | ^
Fixes: 580d656d80cf ("selftests/bpf: Add kfree_skb raw_tp test") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/prog_tests/kfree_skb.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/kfree_skb.c b/tools/testing/selftests/bpf/prog_tests/kfree_skb.c index c07991544a78..34f8822fd221 100644 --- a/tools/testing/selftests/bpf/prog_tests/kfree_skb.c +++ b/tools/testing/selftests/bpf/prog_tests/kfree_skb.c @@ -1,4 +1,5 @@ // SPDX-License-Identifier: GPL-2.0 +#define _GNU_SOURCE #include <test_progs.h> #include <network_helpers.h> #include "kfree_skb.skel.h"
The GNU version of 'struct tcphdr' has members 'doff', 'source' and 'dest', which are not exposed by musl libc headers unless _GNU_SOURCE is defined.
Add this definition to fix errors seen compiling for mips64el/musl-libc:
flow_dissector.c:118:30: error: 'struct tcphdr' has no member named 'doff' 118 | .tcp.doff = 5, | ^~~~ flow_dissector.c:119:30: error: 'struct tcphdr' has no member named 'source' 119 | .tcp.source = 80, | ^~~~~~ flow_dissector.c:120:30: error: 'struct tcphdr' has no member named 'dest' 120 | .tcp.dest = 8080, | ^~~~
Fixes: ae173a915785 ("selftests/bpf: support BPF_FLOW_DISSECTOR_F_PARSE_1ST_FRAG") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/prog_tests/flow_dissector.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/flow_dissector.c b/tools/testing/selftests/bpf/prog_tests/flow_dissector.c index 9625e6d21791..3171047414a7 100644 --- a/tools/testing/selftests/bpf/prog_tests/flow_dissector.c +++ b/tools/testing/selftests/bpf/prog_tests/flow_dissector.c @@ -1,4 +1,5 @@ // SPDX-License-Identifier: GPL-2.0 +#define _GNU_SOURCE #include <test_progs.h> #include <network_helpers.h> #include <linux/if_tun.h>
The GNU version of 'struct tcp_info' in 'netinet/tcp.h' is not exposed by musl headers unless _GNU_SOURCE is defined.
Add this definition to fix errors seen compiling for mips64el/musl-libc:
tcp_rtt.c: In function 'wait_for_ack': tcp_rtt.c:24:25: error: storage size of 'info' isn't known 24 | struct tcp_info info; | ^~~~ tcp_rtt.c:24:25: error: unused variable 'info' [-Werror=unused-variable] cc1: all warnings being treated as errors
Fixes: 1f4f80fed217 ("selftests/bpf: test_progs: convert test_tcp_rtt") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/prog_tests/tcp_rtt.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/tcp_rtt.c b/tools/testing/selftests/bpf/prog_tests/tcp_rtt.c index f2b99d95d916..c38784c1c066 100644 --- a/tools/testing/selftests/bpf/prog_tests/tcp_rtt.c +++ b/tools/testing/selftests/bpf/prog_tests/tcp_rtt.c @@ -1,4 +1,5 @@ // SPDX-License-Identifier: GPL-2.0 +#define _GNU_SOURCE #include <test_progs.h> #include "cgroup_helpers.h" #include "network_helpers.h"
The type 'loff_t' is a GNU extension and not exposed by the musl 'fcntl.h' header unless _GNU_SOURCE is defined. Add this definition to fix errors seen compiling for mips64el/musl-libc:
In file included from tools/testing/selftests/bpf/prog_tests/core_reloc.c:4: ./bpf_testmod/bpf_testmod.h:10:9: error: unknown type name 'loff_t' 10 | loff_t off; | ^~~~~~ ./bpf_testmod/bpf_testmod.h:16:9: error: unknown type name 'loff_t' 16 | loff_t off; | ^~~~~~
Fixes: 6bcd39d366b6 ("selftests/bpf: Add CO-RE relocs selftest relying on kernel module BTF") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/prog_tests/core_reloc.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c index 47f42e680105..26019313e1fc 100644 --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c @@ -1,4 +1,5 @@ // SPDX-License-Identifier: GPL-2.0 +#define _GNU_SOURCE #include <test_progs.h> #include "progs/core_reloc_types.h" #include "bpf_testmod/bpf_testmod.h"
Remove a redundant include of '<linux/icmp.h>' which is already provided in 'lwt_helpers.h'. This avoids errors seen compiling for mips64el/musl-libc:
In file included from .../arpa/inet.h:9, from lwt_redirect.c:51: .../netinet/in.h:23:8: error: redefinition of 'struct in6_addr' 23 | struct in6_addr { | ^~~~~~~~ In file included from .../linux/icmp.h:24, from lwt_redirect.c:50: .../linux/in6.h:33:8: note: originally defined here 33 | struct in6_addr { | ^~~~~~~~ .../netinet/in.h:34:8: error: redefinition of 'struct sockaddr_in6' 34 | struct sockaddr_in6 { | ^~~~~~~~~~~~ .../linux/in6.h:50:8: note: originally defined here 50 | struct sockaddr_in6 { | ^~~~~~~~~~~~ .../netinet/in.h:42:8: error: redefinition of 'struct ipv6_mreq' 42 | struct ipv6_mreq { | ^~~~~~~~~ .../linux/in6.h:60:8: note: originally defined here 60 | struct ipv6_mreq { | ^~~~~~~~~
Fixes: 43a7c3ef8a15 ("selftests/bpf: Add lwt_xmit tests for BPF_REDIRECT") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/prog_tests/lwt_redirect.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/lwt_redirect.c b/tools/testing/selftests/bpf/prog_tests/lwt_redirect.c index 835a1d756c16..b6e8d822e8e9 100644 --- a/tools/testing/selftests/bpf/prog_tests/lwt_redirect.c +++ b/tools/testing/selftests/bpf/prog_tests/lwt_redirect.c @@ -47,7 +47,6 @@ #include <linux/if_ether.h> #include <linux/if_packet.h> #include <linux/if_tun.h> -#include <linux/icmp.h> #include <arpa/inet.h> #include <unistd.h> #include <errno.h>
Remove a redundant include of '<linux/in6.h>', whose needed definitions are already provided by 'test_progs.h'. This avoids errors seen compiling for mips64el/musl-libc:
In file included from .../arpa/inet.h:9, from ./test_progs.h:17, from prog_tests/decap_sanity.c:9: .../netinet/in.h:23:8: error: redefinition of 'struct in6_addr' 23 | struct in6_addr { | ^~~~~~~~ In file included from decap_sanity.c:7: .../linux/in6.h:33:8: note: originally defined here 33 | struct in6_addr { | ^~~~~~~~ .../netinet/in.h:34:8: error: redefinition of 'struct sockaddr_in6' 34 | struct sockaddr_in6 { | ^~~~~~~~~~~~ .../linux/in6.h:50:8: note: originally defined here 50 | struct sockaddr_in6 { | ^~~~~~~~~~~~ .../netinet/in.h:42:8: error: redefinition of 'struct ipv6_mreq' 42 | struct ipv6_mreq { | ^~~~~~~~~ .../linux/in6.h:60:8: note: originally defined here 60 | struct ipv6_mreq { | ^~~~~~~~~
Fixes: 70a00e2f1dba ("selftests/bpf: Test bpf_skb_adjust_room on CHECKSUM_PARTIAL") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/prog_tests/decap_sanity.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/decap_sanity.c b/tools/testing/selftests/bpf/prog_tests/decap_sanity.c index dcb9e5070cc3..d79f398ec6b7 100644 --- a/tools/testing/selftests/bpf/prog_tests/decap_sanity.c +++ b/tools/testing/selftests/bpf/prog_tests/decap_sanity.c @@ -4,7 +4,6 @@ #include <sys/types.h> #include <sys/socket.h> #include <net/if.h> -#include <linux/in6.h>
#include "test_progs.h" #include "network_helpers.h"
Remove a redundant include of '<linux/in6.h>', whose needed definitions are already provided by 'test_progs.h'. This avoids errors seen compiling for mips64el/musl-libc:
In file included from .../arpa/inet.h:9, from ./test_progs.h:17, from prog_tests/crypto_sanity.c:10: .../netinet/in.h:23:8: error: redefinition of 'struct in6_addr' 23 | struct in6_addr { | ^~~~~~~~ In file included from crypto_sanity.c:7: .../linux/in6.h:33:8: note: originally defined here 33 | struct in6_addr { | ^~~~~~~~ .../netinet/in.h:34:8: error: redefinition of 'struct sockaddr_in6' 34 | struct sockaddr_in6 { | ^~~~~~~~~~~~ .../linux/in6.h:50:8: note: originally defined here 50 | struct sockaddr_in6 { | ^~~~~~~~~~~~ .../netinet/in.h:42:8: error: redefinition of 'struct ipv6_mreq' 42 | struct ipv6_mreq { | ^~~~~~~~~ .../linux/in6.h:60:8: note: originally defined here 60 | struct ipv6_mreq { | ^~~~~~~~~
Fixes: 91541ab192fc ("selftests: bpf: crypto skcipher algo selftests") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/prog_tests/crypto_sanity.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c b/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c index b1a3a49a822a..42bd07f7218d 100644 --- a/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c +++ b/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c @@ -4,7 +4,6 @@ #include <sys/types.h> #include <sys/socket.h> #include <net/if.h> -#include <linux/in6.h> #include <linux/if_alg.h>
#include "test_progs.h"
On 23/07/2024 06:54, Tony Ambardar wrote:
Remove a redundant include of '<linux/in6.h>', whose needed definitions are already provided by 'test_progs.h'. This avoids errors seen compiling for mips64el/musl-libc:
In file included from .../arpa/inet.h:9, from ./test_progs.h:17, from prog_tests/crypto_sanity.c:10: .../netinet/in.h:23:8: error: redefinition of 'struct in6_addr' 23 | struct in6_addr { | ^~~~~~~~ In file included from crypto_sanity.c:7: .../linux/in6.h:33:8: note: originally defined here 33 | struct in6_addr { | ^~~~~~~~ .../netinet/in.h:34:8: error: redefinition of 'struct sockaddr_in6' 34 | struct sockaddr_in6 { | ^~~~~~~~~~~~ .../linux/in6.h:50:8: note: originally defined here 50 | struct sockaddr_in6 { | ^~~~~~~~~~~~ .../netinet/in.h:42:8: error: redefinition of 'struct ipv6_mreq' 42 | struct ipv6_mreq { | ^~~~~~~~~ .../linux/in6.h:60:8: note: originally defined here 60 | struct ipv6_mreq { | ^~~~~~~~~
Fixes: 91541ab192fc ("selftests: bpf: crypto skcipher algo selftests") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com
tools/testing/selftests/bpf/prog_tests/crypto_sanity.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c b/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c index b1a3a49a822a..42bd07f7218d 100644 --- a/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c +++ b/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c @@ -4,7 +4,6 @@ #include <sys/types.h> #include <sys/socket.h> #include <net/if.h> -#include <linux/in6.h> #include <linux/if_alg.h> #include "test_progs.h"
Reviewed-by: Vadim Fedorenko vadim.fedorenko@linux.dev
Remove a redundant include of '<asm/types.h>', whose needed definitions are already included (via '<linux/types.h>') in cg_storage_multi_egress_only.c, cg_storage_multi_isolated.c, and cg_storage_multi_shared.c. This avoids redefinition errors seen compiling for mips64el/musl-libc like:
In file included from progs/cg_storage_multi_egress_only.c:13: In file included from progs/cg_storage_multi.h:6: In file included from /usr/mips64el-linux-gnuabi64/include/asm/types.h:23: /usr/include/asm-generic/int-l64.h:29:25: error: typedef redefinition with different types ('long' vs 'long long') 29 | typedef __signed__ long __s64; | ^ /usr/include/asm-generic/int-ll64.h:30:44: note: previous definition is here 30 | __extension__ typedef __signed__ long long __s64; | ^
Fixes: 9e5bd1f7633b ("selftests/bpf: Test CGROUP_STORAGE map can't be used by multiple progs") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com --- tools/testing/selftests/bpf/progs/cg_storage_multi.h | 2 -- 1 file changed, 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/cg_storage_multi.h b/tools/testing/selftests/bpf/progs/cg_storage_multi.h index a0778fe7857a..41d59f0ee606 100644 --- a/tools/testing/selftests/bpf/progs/cg_storage_multi.h +++ b/tools/testing/selftests/bpf/progs/cg_storage_multi.h @@ -3,8 +3,6 @@ #ifndef __PROGS_CG_STORAGE_MULTI_H #define __PROGS_CG_STORAGE_MULTI_H
-#include <asm/types.h> - struct cgroup_value { __u32 egress_pkts; __u32 ingress_pkts;
On Mon, Jul 22, 2024 at 10:56 PM Tony Ambardar tony.ambardar@gmail.com wrote:
Remove a redundant include of '<asm/types.h>', whose needed definitions are already included (via '<linux/types.h>') in cg_storage_multi_egress_only.c, cg_storage_multi_isolated.c, and cg_storage_multi_shared.c. This avoids redefinition errors seen compiling for mips64el/musl-libc like:
In file included from progs/cg_storage_multi_egress_only.c:13: In file included from progs/cg_storage_multi.h:6: In file included from /usr/mips64el-linux-gnuabi64/include/asm/types.h:23: /usr/include/asm-generic/int-l64.h:29:25: error: typedef redefinition with different types ('long' vs 'long long') 29 | typedef __signed__ long __s64; | ^ /usr/include/asm-generic/int-ll64.h:30:44: note: previous definition is here 30 | __extension__ typedef __signed__ long long __s64; | ^
Fixes: 9e5bd1f7633b ("selftests/bpf: Test CGROUP_STORAGE map can't be used by multiple progs") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com
tools/testing/selftests/bpf/progs/cg_storage_multi.h | 2 -- 1 file changed, 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/cg_storage_multi.h b/tools/testing/selftests/bpf/progs/cg_storage_multi.h index a0778fe7857a..41d59f0ee606 100644 --- a/tools/testing/selftests/bpf/progs/cg_storage_multi.h +++ b/tools/testing/selftests/bpf/progs/cg_storage_multi.h @@ -3,8 +3,6 @@ #ifndef __PROGS_CG_STORAGE_MULTI_H #define __PROGS_CG_STORAGE_MULTI_H
-#include <asm/types.h>
struct cgroup_value { __u32 egress_pkts; __u32 ingress_pkts; -- 2.34.1
Hmm, some linter checks prefer headers themselves include everything they use. This header uses __u32 and after this patch it would include no headers. Would it be okay to include <linux/types.h> or we don't care?
YiFei Zhu
On Tue, Jul 23, 2024 at 3:35 PM YiFei Zhu zhuyifei@google.com wrote:
On Mon, Jul 22, 2024 at 10:56 PM Tony Ambardar tony.ambardar@gmail.com wrote:
Remove a redundant include of '<asm/types.h>', whose needed definitions are already included (via '<linux/types.h>') in cg_storage_multi_egress_only.c, cg_storage_multi_isolated.c, and cg_storage_multi_shared.c. This avoids redefinition errors seen compiling for mips64el/musl-libc like:
In file included from progs/cg_storage_multi_egress_only.c:13: In file included from progs/cg_storage_multi.h:6: In file included from /usr/mips64el-linux-gnuabi64/include/asm/types.h:23: /usr/include/asm-generic/int-l64.h:29:25: error: typedef redefinition with different types ('long' vs 'long long') 29 | typedef __signed__ long __s64; | ^ /usr/include/asm-generic/int-ll64.h:30:44: note: previous definition is here 30 | __extension__ typedef __signed__ long long __s64; | ^
Fixes: 9e5bd1f7633b ("selftests/bpf: Test CGROUP_STORAGE map can't be used by multiple progs") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com
tools/testing/selftests/bpf/progs/cg_storage_multi.h | 2 -- 1 file changed, 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/cg_storage_multi.h b/tools/testing/selftests/bpf/progs/cg_storage_multi.h index a0778fe7857a..41d59f0ee606 100644 --- a/tools/testing/selftests/bpf/progs/cg_storage_multi.h +++ b/tools/testing/selftests/bpf/progs/cg_storage_multi.h @@ -3,8 +3,6 @@ #ifndef __PROGS_CG_STORAGE_MULTI_H #define __PROGS_CG_STORAGE_MULTI_H
-#include <asm/types.h>
struct cgroup_value { __u32 egress_pkts; __u32 ingress_pkts; -- 2.34.1
Hmm, some linter checks prefer headers themselves include everything they use. This header uses __u32 and after this patch it would include no headers. Would it be okay to include <linux/types.h> or we don't care?
This header seems to be included both from user space code and BPF code, so it probably is for the better to not include specific headers and rely on respective code to provide "basic" type definitions like __u32. (For BPF side they might be coming from vmlinux.h, for example, which won't be compatible with anything included in user space code).
YiFei Zhu
On Tue, Jul 23, 2024 at 03:35:22PM -0700, YiFei Zhu wrote:
On Mon, Jul 22, 2024 at 10:56 PM Tony Ambardar tony.ambardar@gmail.com wrote:
Remove a redundant include of '<asm/types.h>', whose needed definitions are already included (via '<linux/types.h>') in cg_storage_multi_egress_only.c, cg_storage_multi_isolated.c, and cg_storage_multi_shared.c. This avoids redefinition errors seen compiling for mips64el/musl-libc like:
In file included from progs/cg_storage_multi_egress_only.c:13: In file included from progs/cg_storage_multi.h:6: In file included from /usr/mips64el-linux-gnuabi64/include/asm/types.h:23: /usr/include/asm-generic/int-l64.h:29:25: error: typedef redefinition with different types ('long' vs 'long long') 29 | typedef __signed__ long __s64; | ^ /usr/include/asm-generic/int-ll64.h:30:44: note: previous definition is here 30 | __extension__ typedef __signed__ long long __s64; | ^
Fixes: 9e5bd1f7633b ("selftests/bpf: Test CGROUP_STORAGE map can't be used by multiple progs") Signed-off-by: Tony Ambardar tony.ambardar@gmail.com
tools/testing/selftests/bpf/progs/cg_storage_multi.h | 2 -- 1 file changed, 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/cg_storage_multi.h b/tools/testing/selftests/bpf/progs/cg_storage_multi.h index a0778fe7857a..41d59f0ee606 100644 --- a/tools/testing/selftests/bpf/progs/cg_storage_multi.h +++ b/tools/testing/selftests/bpf/progs/cg_storage_multi.h @@ -3,8 +3,6 @@ #ifndef __PROGS_CG_STORAGE_MULTI_H #define __PROGS_CG_STORAGE_MULTI_H
-#include <asm/types.h>
struct cgroup_value { __u32 egress_pkts; __u32 ingress_pkts; -- 2.34.1
Hmm, some linter checks prefer headers themselves include everything they use. This header uses __u32 and after this patch it would include no headers. Would it be okay to include <linux/types.h> or we don't care?
YiFei Zhu
Good point, I agree even the readability suffers without headers. Replacing <asm/types.h> with <linux/types.h> makes more sense, and guards in the latter avoid the conflicts noted above. I'll queue this change for a v2. Appreciate the feedback!
Cheers, Tony Ambardar
On Mon, Jul 22, 2024 at 10:55 PM Tony Ambardar tony.ambardar@gmail.com wrote:
Hello all,
This series includes the bulk of libc-related compile fixes accumulated to support systems using musl, with smaller numbers to follow. These patches are simple and straightforward, and the series has been tested with the kernel-patches/bpf CI and locally using mips64el-gcc/musl-libc and QEMU with an OpenWrt rootfs.
The patches address a few general categories of libc portability issues:
- missing, redundant or incorrect include headers
- disabled GNU header extensions (i.e. missing #define _GNU_SOURCE)
- issues with types and casting
Feedback and suggestions for improvement are welcome!
Thanks, Tony
Tony Ambardar (19): selftests/bpf: Use pid_t consistently in test_progs.c selftests/bpf: Fix compile error from rlim_t in sk_storage_map.c selftests/bpf: Fix error compiling bpf_iter_setsockopt.c with musl libc selftests/bpf: Drop unneeded include in unpriv_helpers.c selftests/bpf: Drop unneeded include in sk_lookup.c selftests/bpf: Drop unneeded include in flow_dissector.c
I squashed the above three patches into one, as they are the same kind of fix with the same reasoning. The rest at least have a specific error example, so I didn't touch them.
But generally speaking, I think it's fair to combine the same kind of fixes across a few files (for the future).
Applied to bpf-next, thanks.
selftests/bpf: Fix missing ARRAY_SIZE() definition in bench.c selftests/bpf: Fix missing UINT_MAX definitions in benchmarks selftests/bpf: Fix missing BUILD_BUG_ON() declaration selftests/bpf: Fix include of <sys/fcntl.h> selftests/bpf: Fix compiling parse_tcp_hdr_opt.c with musl-libc selftests/bpf: Fix compiling kfree_skb.c with musl-libc selftests/bpf: Fix compiling flow_dissector.c with musl-libc selftests/bpf: Fix compiling tcp_rtt.c with musl-libc selftests/bpf: Fix compiling core_reloc.c with musl-libc selftests/bpf: Fix errors compiling lwt_redirect.c with musl libc selftests/bpf: Fix errors compiling decap_sanity.c with musl libc selftests/bpf: Fix errors compiling crypto_sanity.c with musl libc selftests/bpf: Fix errors compiling cg_storage_multi.h with musl libc
tools/testing/selftests/bpf/bench.c | 1 + tools/testing/selftests/bpf/bench.h | 1 + tools/testing/selftests/bpf/map_tests/sk_storage_map.c | 2 +- tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c | 2 +- tools/testing/selftests/bpf/prog_tests/core_reloc.c | 1 + tools/testing/selftests/bpf/prog_tests/crypto_sanity.c | 1 - tools/testing/selftests/bpf/prog_tests/decap_sanity.c | 1 - tools/testing/selftests/bpf/prog_tests/flow_dissector.c | 2 +- tools/testing/selftests/bpf/prog_tests/kfree_skb.c | 1 + tools/testing/selftests/bpf/prog_tests/lwt_redirect.c | 1 - tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c | 2 +- tools/testing/selftests/bpf/prog_tests/parse_tcp_hdr_opt.c | 1 + tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 1 - tools/testing/selftests/bpf/prog_tests/tcp_rtt.c | 1 + tools/testing/selftests/bpf/prog_tests/user_ringbuf.c | 1 + tools/testing/selftests/bpf/progs/cg_storage_multi.h | 2 -- tools/testing/selftests/bpf/test_progs.c | 2 +- tools/testing/selftests/bpf/unpriv_helpers.c | 1 - 18 files changed, 12 insertions(+), 12 deletions(-)
-- 2.34.1
Hi Andrii,
On Wed, Jul 24, 2024 at 04:52:57PM -0700, Andrii Nakryiko wrote:
On Mon, Jul 22, 2024 at 10:55 PM Tony Ambardar tony.ambardar@gmail.com wrote:
[...]
Tony Ambardar (19): selftests/bpf: Use pid_t consistently in test_progs.c selftests/bpf: Fix compile error from rlim_t in sk_storage_map.c selftests/bpf: Fix error compiling bpf_iter_setsockopt.c with musl libc selftests/bpf: Drop unneeded include in unpriv_helpers.c selftests/bpf: Drop unneeded include in sk_lookup.c selftests/bpf: Drop unneeded include in flow_dissector.c
I squashed the above three patches into one, as they are the same kind of fix with the same reasoning. The rest at least have a specific error example, so I didn't touch them.
But generally speaking, I think it's fair to combine the same kind of fixes across a few files (for the future).
Thanks for clarifying this and reviewing. My concern earlier was that squashing "Fixes:" tags could hinder backporting. Is that a non-issue?
Applied to bpf-next, thanks.
And thanks to Geliang Tang, YiFei Zhu, and Vadim Fedorenko for their feedback.
Cheers, Tony Ambardar
[...]
On Wed, Jul 24, 2024 at 5:28 PM Tony Ambardar tony.ambardar@gmail.com wrote:
Hi Andrii,
On Wed, Jul 24, 2024 at 04:52:57PM -0700, Andrii Nakryiko wrote:
On Mon, Jul 22, 2024 at 10:55 PM Tony Ambardar tony.ambardar@gmail.com wrote:
[...]
Tony Ambardar (19): selftests/bpf: Use pid_t consistently in test_progs.c selftests/bpf: Fix compile error from rlim_t in sk_storage_map.c selftests/bpf: Fix error compiling bpf_iter_setsockopt.c with musl libc selftests/bpf: Drop unneeded include in unpriv_helpers.c selftests/bpf: Drop unneeded include in sk_lookup.c selftests/bpf: Drop unneeded include in flow_dissector.c
I squashed the above three patches into one, as they are the same kind of fix with the same reasoning. The rest at least have a specific error example, so I didn't touch them.
But generally speaking, I think it's fair to combine the same kind of fixes across a few files (for the future).
Thanks for clarifying this and reviewing. My concern earlier was that squashing "Fixes:" tags could hinder backporting. Is that a non-issue?
The fixes are small, so probably not. It's also fixes for selftests, so I'm not even sure how much that matters (for backporting).
Applied to bpf-next, thanks.
And thanks to Geliang Tang, YiFei Zhu, and Vadim Fedorenko for their feedback.
Cheers, Tony Ambardar
[...]
Hello:
This series was applied to bpf/bpf-next.git (master) by Andrii Nakryiko andrii@kernel.org:
On Mon, 22 Jul 2024 22:54:27 -0700 you wrote:
Hello all,
This series includes the bulk of libc-related compile fixes accumulated to support systems using musl, with smaller numbers to follow. These patches are simple and straightforward, and the series has been tested with the kernel-patches/bpf CI and locally using mips64el-gcc/musl-libc and QEMU with an OpenWrt rootfs.
[...]
Here is the summary with links: - [bpf-next,v1,01/19] selftests/bpf: Use pid_t consistently in test_progs.c https://git.kernel.org/bpf/bpf-next/c/afd8169d2724 - [bpf-next,v1,02/19] selftests/bpf: Fix compile error from rlim_t in sk_storage_map.c https://git.kernel.org/bpf/bpf-next/c/bb3b965ee3a0 - [bpf-next,v1,03/19] selftests/bpf: Fix error compiling bpf_iter_setsockopt.c with musl libc https://git.kernel.org/bpf/bpf-next/c/6c3a808171a2 - [bpf-next,v1,04/19] selftests/bpf: Drop unneeded include in unpriv_helpers.c https://git.kernel.org/bpf/bpf-next/c/80fd6c991e95 - [bpf-next,v1,05/19] selftests/bpf: Drop unneeded include in sk_lookup.c (no matching commit) - [bpf-next,v1,06/19] selftests/bpf: Drop unneeded include in flow_dissector.c (no matching commit) - [bpf-next,v1,07/19] selftests/bpf: Fix missing ARRAY_SIZE() definition in bench.c https://git.kernel.org/bpf/bpf-next/c/9dc46fdb679b - [bpf-next,v1,08/19] selftests/bpf: Fix missing UINT_MAX definitions in benchmarks https://git.kernel.org/bpf/bpf-next/c/6898506ee0ae - [bpf-next,v1,09/19] selftests/bpf: Fix missing BUILD_BUG_ON() declaration https://git.kernel.org/bpf/bpf-next/c/b855ef609329 - [bpf-next,v1,10/19] selftests/bpf: Fix include of <sys/fcntl.h> https://git.kernel.org/bpf/bpf-next/c/f9d6628b2f54 - [bpf-next,v1,11/19] selftests/bpf: Fix compiling parse_tcp_hdr_opt.c with musl-libc https://git.kernel.org/bpf/bpf-next/c/81c60e36c31b - [bpf-next,v1,12/19] selftests/bpf: Fix compiling kfree_skb.c with musl-libc https://git.kernel.org/bpf/bpf-next/c/787a2e4f1b9e - [bpf-next,v1,13/19] selftests/bpf: Fix compiling flow_dissector.c with musl-libc https://git.kernel.org/bpf/bpf-next/c/c2e6d8c605ac - [bpf-next,v1,14/19] selftests/bpf: Fix compiling tcp_rtt.c with musl-libc https://git.kernel.org/bpf/bpf-next/c/2a6a6956f616 - [bpf-next,v1,15/19] selftests/bpf: Fix compiling core_reloc.c with musl-libc https://git.kernel.org/bpf/bpf-next/c/231c5446bfbc - [bpf-next,v1,16/19] selftests/bpf: Fix errors compiling lwt_redirect.c with musl libc https://git.kernel.org/bpf/bpf-next/c/7ce34ba2b21a - [bpf-next,v1,17/19] selftests/bpf: Fix errors compiling decap_sanity.c with musl libc https://git.kernel.org/bpf/bpf-next/c/352d541fae2d - [bpf-next,v1,18/19] selftests/bpf: Fix errors compiling crypto_sanity.c with musl libc https://git.kernel.org/bpf/bpf-next/c/a88580ba22aa - [bpf-next,v1,19/19] selftests/bpf: Fix errors compiling cg_storage_multi.h with musl libc https://git.kernel.org/bpf/bpf-next/c/56b0ab53657b
You are awesome, thank you!
linux-kselftest-mirror@lists.linaro.org
-
Andrii Nakryiko -
Geliang Tang -
patchwork-bot+netdevbpf＠kernel.org -
Tony Ambardar -
Vadim Fedorenko -
YiFei Zhu