Hi Martin,
Thank you for your reply!
On 06/07/2024 01:10, Martin KaFai Lau wrote:
On 7/4/24 3:48 AM, Matthieu Baerts wrote:
diff --git a/tools/testing/selftests/bpf/Makefile
b/tools/testing/
selftests/bpf/Makefile
index e0b3887b3d2d..204269d0b5b8 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -144,7 +144,7 @@ TEST_GEN_PROGS_EXTENDED =
test_skb_cgroup_id_user \
flow_dissector_load test_flow_dissector
test_tcp_check_syncookie_user \
test_lirc_mode2_user xdping test_cpp runqslower bench
bpf_testmod.ko \
xskxceiver xdp_redirect_multi xdp_synproxy veristat
xdp_hw_metadata \
- xdp_features bpf_test_no_cfi.ko
+ xdp_features bpf_test_no_cfi.ko mptcp_pm_nl_ctl
On the BPF CI, we have such errors:
mptcp_pm_nl_ctl.c:20:10: fatal error: 'linux/mptcp.h' file
not found
20 | #include "linux/mptcp.h"
| ^~~~~~~~~~~~~~~
On my side, I don't have any issue, because the compiler uses the
mptcp.h file from the system: /usr/include/linux/mptcp.h
I suppose that's not OK on the BPF CI, as it looks like it
doesn't have
this file there, probably because it still uses Ubuntu 20.04 as
base,
which doesn't include this file in the linux-libc-dev package.
When I look at how this 'mptcp_pm_nl_ctl' tool -- and all the
other
programs from that list -- is compiled (V=1), I see that the
following
"-I" options are given:
-I${PWD}/tools/testing/selftests/bpf
-I${BUILD}//tools/include
-I${BUILD}/include/generated
-I${PWD}/tools/lib
-I${PWD}/tools/include
-I${PWD}/tools/include/uapi
-I${BUILD}/
It will then not look at -I${PWD}/usr/include or the directory
generated
with:
make headers_install INSTALL_HDR_PATH=(...)
It sounds like the tools/testing/selftests/net/mptcp/Makefile is
looking
at this include path, so it works?
Yes it does work.
iiu the bpf/Makefile correctly, it has the bpftool "make" compiled
and
installed at tools/testing/selftests/bpf/tools/sbin/. May be
directly
compile the pm_nl_ctl by "make tools/testing/selftests/net/mptcp/"?
That could be an alternative, I didn't know it would be OK to add
such
dependence, good idea.
I guess that's why people have duplicated files in
'tools/include/uapi',
but I also understood from Jakub that it is not a good idea to
continue
to do so.
What would be the best solution to avoid a copy? A symlink still
looks
like a workaround.
In the other selftests, KHDR_INCLUDES is used to be able to
include the
path containing the UAPI headers. So if someone built the headers
in a
Meaning KHDR_INCLUDES should be used and -
I${PWD}/tools/include/uapi can
be retired?
That's the idea, yes, for "userspace programs". I mean: for BPF
programs
requiring vmlinux.h (BPF_CFLAGS), I guess you will still need the
bpf.h
file from tools/include/uapi, no?
I haven't looked into the details. I quickly tried but it
fails in my environment.
Do you not have issues because some files have something like:
#include <uapi/linux/(...).h>
On my side, I had a working version using this patch:
diff --git a/tools/testing/selftests/bpf/Makefile
b/tools/testing/selftests/bpf/Makefile
index 7c5827d20c2e..112f14d40852 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -37,7 +37,7 @@ CFLAGS += -g $(OPT_FLAGS) -rdynamic \
-Wall -Werror -fno-omit-frame-pointer \
$(GENFLAGS) $(SAN_CFLAGS) $(LIBELF_CFLAGS) \
-I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
- -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT)
+ -I$(TOOLSINCDIR) $(KHDR_INCLUDES) -I$(OUTPUT)
LDFLAGS += $(SAN_LDFLAGS)
LDLIBS += $(LIBELF_LIBS) -lz -lrt -lpthread
But only after having removed these extra 'uapi/':
$ git grep -l '<uapi/' -- tools/testing/selftests/bpf | \
xargs sed -i 's|#include <uapi/|#include <|g'
Is it not OK for you like that?
