[PATCH bpf-next v1 00/19] selftests/bpf: Improve libc portability / musl support (part 1)

[PATCH bpf-next v1 00/19] selftests/bpf: Improve libc portability / musl support (part 1)

[Tony Ambardar]

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(-)

-- 
2.34.1

[PATCH bpf-next v1 01/19] selftests/bpf: Use pid_t consistently in test_progs.c

[Tony Ambardar]

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);
-- 
2.34.1

[PATCH bpf-next v1 02/19] selftests/bpf: Fix compile error from rlim_t in sk_storage_map.c

[Tony Ambardar]

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();
-- 
2.34.1

[PATCH bpf-next v1 03/19] selftests/bpf: Fix error compiling bpf_iter_setsockopt.c with musl libc

[Tony Ambardar]

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;
-- 
2.34.1

[PATCH bpf-next v1 04/19] selftests/bpf: Drop unneeded include in unpriv_helpers.c

[Tony Ambardar]

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>
-- 
2.34.1

[PATCH bpf-next v1 05/19] selftests/bpf: Drop unneeded include in sk_lookup.c

[Tony Ambardar]

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>
-- 
2.34.1

[PATCH bpf-next v1 06/19] selftests/bpf: Drop unneeded include in flow_dissector.c

[Tony Ambardar]

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>
 
-- 
2.34.1

[PATCH bpf-next v1 07/19] selftests/bpf: Fix missing ARRAY_SIZE() definition in bench.c

[Tony Ambardar]

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 = {
-- 
2.34.1

[PATCH bpf-next v1 08/19] selftests/bpf: Fix missing UINT_MAX definitions in benchmarks

[Tony Ambardar]

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;
-- 
2.34.1

[PATCH bpf-next v1 09/19] selftests/bpf: Fix missing BUILD_BUG_ON() declaration

[Tony Ambardar]

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>
-- 
2.34.1

[PATCH bpf-next v1 10/19] selftests/bpf: Fix include of <sys/fcntl.h>

[Tony Ambardar]

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)
-- 
2.34.1

[PATCH bpf-next v1 11/19] selftests/bpf: Fix compiling parse_tcp_hdr_opt.c with musl-libc

[Tony Ambardar]

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"
-- 
2.34.1

[PATCH bpf-next v1 12/19] selftests/bpf: Fix compiling kfree_skb.c with musl-libc

[Tony Ambardar]

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"
-- 
2.34.1

[PATCH bpf-next v1 13/19] selftests/bpf: Fix compiling flow_dissector.c with musl-libc

[Tony Ambardar]

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>
-- 
2.34.1

[PATCH bpf-next v1 14/19] selftests/bpf: Fix compiling tcp_rtt.c with musl-libc

[Tony Ambardar]

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"
-- 
2.34.1

[PATCH bpf-next v1 15/19] selftests/bpf: Fix compiling core_reloc.c with musl-libc

[Tony Ambardar]

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"
-- 
2.34.1

[PATCH bpf-next v1 16/19] selftests/bpf: Fix errors compiling lwt_redirect.c with musl libc

[Tony Ambardar]

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>
-- 
2.34.1

[PATCH bpf-next v1 17/19] selftests/bpf: Fix errors compiling decap_sanity.c with musl libc

[Tony Ambardar]

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"
-- 
2.34.1

[PATCH bpf-next v1 18/19] selftests/bpf: Fix errors compiling crypto_sanity.c with musl libc

[Tony Ambardar]

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"
-- 
2.34.1

[PATCH bpf-next v1 19/19] selftests/bpf: Fix errors compiling cg_storage_multi.h with musl libc

[Tony Ambardar]

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

Re: [PATCH bpf-next v1 00/19] selftests/bpf: Improve libc portability / musl support (part 1)

[bot+bpf-ci]

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

Dear patch submitter,

CI has tested the following submission:
Status:     SUCCESS
Name:       [bpf-next,v1,00/19] selftests/bpf: Improve libc portability / musl support (part 1)
Patchwork:  https://patchwork.kernel.org/project/netdevbpf/list/?series=873126&state=*
Matrix:     https://github.com/kernel-patches/bpf/actions/runs/10053605528

No further action is necessary on your part.


Please note: this email is coming from an unmonitored mailbox. If you have
questions or feedback, please reach out to the Meta Kernel CI team at
kernel-ci@meta.com.

Re: [PATCH bpf-next v1 18/19] selftests/bpf: Fix errors compiling crypto_sanity.c with musl libc

[Vadim Fedorenko]

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>

Re: [PATCH bpf-next v1 19/19] selftests/bpf: Fix errors compiling cg_storage_multi.h with musl libc

[YiFei Zhu]

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

Re: [PATCH bpf-next v1 01/19] selftests/bpf: Use pid_t consistently in test_progs.c

[Geliang Tang]

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);

Re: [PATCH bpf-next v1 03/19] selftests/bpf: Fix error compiling bpf_iter_setsockopt.c with musl libc

[Geliang Tang]

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;

Re: [PATCH bpf-next v1 07/19] selftests/bpf: Fix missing ARRAY_SIZE() definition in bench.c

[Geliang Tang]

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 = {

Re: [PATCH bpf-next v1 19/19] selftests/bpf: Fix errors compiling cg_storage_multi.h with musl libc

[Andrii Nakryiko]

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

Re: [PATCH bpf-next v1 00/19] selftests/bpf: Improve libc portability / musl support (part 1)

[Andrii Nakryiko]

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
>

Re: [PATCH bpf-next v1 19/19] selftests/bpf: Fix errors compiling cg_storage_multi.h with musl libc

[Tony Ambardar]

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

Re: [PATCH bpf-next v1 00/19] selftests/bpf: Improve libc portability / musl support (part 1)

[patchwork-bot+netdevbpf]

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH bpf-next v1 00/19] selftests/bpf: Improve libc portability / musl support (part 1)

[Tony Ambardar]

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

[...]

Re: [PATCH bpf-next v1 00/19] selftests/bpf: Improve libc portability / musl support (part 1)

[Andrii Nakryiko]

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
>
> [...]

Re: [PATCH bpf-next v1 07/19] selftests/bpf: Fix missing ARRAY_SIZE() definition in bench.c

[Tony Ambardar]

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 = {
>