[PATCH bpf 0/4] Fix caching of BTF for kfuncs in the verifier

[PATCH bpf 0/4] Fix caching of BTF for kfuncs in the verifier

[Toke Høiland-Jørgensen]

When playing around with defining kfuncs in some custom modules, we
noticed that if a BPF program calls two functions with the same
signature in two different modules, the function from the wrong module
may sometimes end up being called. Whether this happens depends on the
order of the calls in the BPF program, which turns out to be due to the
use of sort() inside __find_kfunc_desc_btf() in the verifier code.

This series contains a fix for the issue (first patch), and a selftest
to trigger it (last patch). The two middle commits refactor some of the
selftest code to better handle building and loading multiple kernel
modules as part of the testing. See the individual patch descriptions
for more details.

---
Simon Sundberg (2):
      selftests/bpf: Provide a generic [un]load_module helper
      selftests/bpf: Add test for kfunc module order

Toke Høiland-Jørgensen (2):
      bpf: fix kfunc btf caching for modules
      selftests/bpf: Consolidate kernel modules into common directory

 kernel/bpf/verifier.c                              |  8 +++-
 tools/testing/selftests/bpf/Makefile               | 35 +++++++-------
 .../testing/selftests/bpf/bpf_test_no_cfi/Makefile | 19 --------
 tools/testing/selftests/bpf/bpf_testmod/Makefile   | 20 --------
 .../testing/selftests/bpf/prog_tests/core_reloc.c  |  2 +-
 .../selftests/bpf/prog_tests/kfunc_module_order.c  | 55 ++++++++++++++++++++++
 tools/testing/selftests/bpf/progs/bad_struct_ops.c |  2 +-
 tools/testing/selftests/bpf/progs/cb_refs.c        |  2 +-
 tools/testing/selftests/bpf/progs/epilogue_exit.c  |  4 +-
 .../selftests/bpf/progs/epilogue_tailcall.c        |  4 +-
 tools/testing/selftests/bpf/progs/iters_testmod.c  |  2 +-
 tools/testing/selftests/bpf/progs/jit_probe_mem.c  |  2 +-
 .../selftests/bpf/progs/kfunc_call_destructive.c   |  2 +-
 .../testing/selftests/bpf/progs/kfunc_call_fail.c  |  2 +-
 .../testing/selftests/bpf/progs/kfunc_call_race.c  |  2 +-
 .../testing/selftests/bpf/progs/kfunc_call_test.c  |  2 +-
 .../selftests/bpf/progs/kfunc_call_test_subprog.c  |  2 +-
 .../selftests/bpf/progs/kfunc_module_order.c       | 30 ++++++++++++
 .../testing/selftests/bpf/progs/local_kptr_stash.c |  2 +-
 tools/testing/selftests/bpf/progs/map_kptr.c       |  2 +-
 tools/testing/selftests/bpf/progs/map_kptr_fail.c  |  2 +-
 tools/testing/selftests/bpf/progs/missed_kprobe.c  |  2 +-
 .../selftests/bpf/progs/missed_kprobe_recursion.c  |  2 +-
 tools/testing/selftests/bpf/progs/nested_acquire.c |  2 +-
 tools/testing/selftests/bpf/progs/pro_epilogue.c   |  4 +-
 .../selftests/bpf/progs/pro_epilogue_goto_start.c  |  4 +-
 tools/testing/selftests/bpf/progs/sock_addr_kern.c |  2 +-
 .../selftests/bpf/progs/struct_ops_detach.c        |  2 +-
 .../selftests/bpf/progs/struct_ops_forgotten_cb.c  |  2 +-
 .../selftests/bpf/progs/struct_ops_maybe_null.c    |  2 +-
 .../bpf/progs/struct_ops_maybe_null_fail.c         |  2 +-
 .../selftests/bpf/progs/struct_ops_module.c        |  2 +-
 .../selftests/bpf/progs/struct_ops_multi_pages.c   |  2 +-
 .../selftests/bpf/progs/struct_ops_nulled_out_cb.c |  2 +-
 .../bpf/progs/test_kfunc_param_nullable.c          |  2 +-
 .../selftests/bpf/progs/test_module_attach.c       |  2 +-
 .../selftests/bpf/progs/test_tp_btf_nullable.c     |  2 +-
 .../testing/selftests/bpf/progs/unsupported_ops.c  |  2 +-
 tools/testing/selftests/bpf/progs/wq.c             |  2 +-
 tools/testing/selftests/bpf/progs/wq_failures.c    |  2 +-
 .../bpf/{bpf_testmod => test_kmods}/.gitignore     |  0
 tools/testing/selftests/bpf/test_kmods/Makefile    | 26 ++++++++++
 .../selftests/bpf/test_kmods/bpf_test_modorder_x.c | 39 +++++++++++++++
 .../selftests/bpf/test_kmods/bpf_test_modorder_y.c | 39 +++++++++++++++
 .../bpf_test_no_cfi.c                              |  0
 .../bpf_testmod-events.h                           |  0
 .../bpf/{bpf_testmod => test_kmods}/bpf_testmod.c  |  0
 .../bpf/{bpf_testmod => test_kmods}/bpf_testmod.h  |  0
 .../bpf_testmod_kfunc.h                            |  0
 tools/testing/selftests/bpf/testing_helpers.c      | 34 ++++++++-----
 tools/testing/selftests/bpf/testing_helpers.h      |  2 +
 51 files changed, 275 insertions(+), 108 deletions(-)
---
base-commit: bcd28cfd04ebd3f871443e4746e511147686e517
change-id: 20241008-fix-kfunc-btf-caching-for-modules-b62603484ffb

Best regards,
-- 
Toke Høiland-Jørgensen <toke@redhat.com>

[PATCH bpf 1/4] bpf: fix kfunc btf caching for modules

[Toke Høiland-Jørgensen]

The verifier contains a cache for looking up module BTF objects when
calling kfuncs defined in modules. This cache uses a 'struct
bpf_kfunc_btf_tab', which contains a sorted list of BTF objects that
were already seen in the current verifier run, and the BTF objects are
looked up by the offset stored in the relocated call instruction using
bsearch().

The first time a given offset is seen, the module BTF is loaded from the
file descriptor passed in by libbpf, and stored into the cache. However,
there's a bug in the code storing the new entry: it stores a pointer to
the new cache entry, then calls sort() to keep the cache sorted for the
next lookup using bsearch(), and then returns the entry that was just
stored through the stored pointer. However, because sort() modifies the
list of entries in place *by value*, the stored pointer may no longer
point to the right entry, in which case the wrong BTF object will be
returned.

The end result of this is an intermittent bug where, if a BPF program
calls two functions with the same signature in two different modules,
the function from the wrong module may sometimes end up being called.
Whether this happens depends on the order of the calls in the BPF
program (as that affects whether sort() reorders the array of BTF
objects), making it especially hard to track down. Simon, credited as
reporter below, spent significant effort analysing and creating a
reproducer for this issue. The reproducer is added as a selftest in a
subsequent patch.

The fix is straight forward: simply don't use the stored pointer after
calling sort(). Since we already have an on-stack pointer to the BTF
object itself at the point where the function return, just use that, and
populate it from the cache entry in the branch where the lookup
succeeds.

Fixes: 2357672c54c3 ("bpf: Introduce BPF support for kernel module function calls")
Reported-by: Simon Sundberg <simon.sundberg@kau.se>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 kernel/bpf/verifier.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 434de48cd24bd8d9fb008e4a1e9e0ab4d75ef90a..98d866ba90bf92e3666fb9a07b36f48d452779c6 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2750,10 +2750,16 @@ static struct btf *__find_kfunc_desc_btf(struct bpf_verifier_env *env,
 		b->module = mod;
 		b->offset = offset;
 
+		/* sort() reorders entries by value, so b may no longer point
+		 * to the right entry after this
+		 */
 		sort(tab->descs, tab->nr_descs, sizeof(tab->descs[0]),
 		     kfunc_btf_cmp_by_off, NULL);
+	} else {
+		btf = b->btf;
 	}
-	return b->btf;
+
+	return btf;
 }
 
 void bpf_free_kfunc_btf_tab(struct bpf_kfunc_btf_tab *tab)

-- 
2.47.0

[PATCH bpf 2/4] selftests/bpf: Consolidate kernel modules into common directory

[Toke Høiland-Jørgensen]

The selftests build two kernel modules (bpf_testmod.ko and
bpf_test_no_cfi.ko) which use copy-pasted Makefile targets. This is a
bit messy, and doesn't scale so well when we add more modules, so let's
consolidate these rules into a single rule generated for each module
name, and move the module sources into a single directory.

To avoid parallel builds of the different modules stepping on each
other's toes during the 'modpost' phase of the Kbuild 'make modules', we
create a single target for all the defined modules, which contains the
recursive 'make' call into the modules directory. The Makefile in the
subdirectory building the modules is modified to also touch a
'modules.built' file, which we can add as a dependency on the top-level
selftests Makefile, thus ensuring that the modules are always rebuilt if
any of the dependencies in the selftests change.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 tools/testing/selftests/bpf/Makefile               | 34 ++++++++++------------
 .../testing/selftests/bpf/bpf_test_no_cfi/Makefile | 19 ------------
 tools/testing/selftests/bpf/bpf_testmod/Makefile   | 20 -------------
 .../testing/selftests/bpf/prog_tests/core_reloc.c  |  2 +-
 tools/testing/selftests/bpf/progs/bad_struct_ops.c |  2 +-
 tools/testing/selftests/bpf/progs/cb_refs.c        |  2 +-
 tools/testing/selftests/bpf/progs/epilogue_exit.c  |  4 +--
 .../selftests/bpf/progs/epilogue_tailcall.c        |  4 +--
 tools/testing/selftests/bpf/progs/iters_testmod.c  |  2 +-
 tools/testing/selftests/bpf/progs/jit_probe_mem.c  |  2 +-
 .../selftests/bpf/progs/kfunc_call_destructive.c   |  2 +-
 .../testing/selftests/bpf/progs/kfunc_call_fail.c  |  2 +-
 .../testing/selftests/bpf/progs/kfunc_call_race.c  |  2 +-
 .../testing/selftests/bpf/progs/kfunc_call_test.c  |  2 +-
 .../selftests/bpf/progs/kfunc_call_test_subprog.c  |  2 +-
 .../testing/selftests/bpf/progs/local_kptr_stash.c |  2 +-
 tools/testing/selftests/bpf/progs/map_kptr.c       |  2 +-
 tools/testing/selftests/bpf/progs/map_kptr_fail.c  |  2 +-
 tools/testing/selftests/bpf/progs/missed_kprobe.c  |  2 +-
 .../selftests/bpf/progs/missed_kprobe_recursion.c  |  2 +-
 tools/testing/selftests/bpf/progs/nested_acquire.c |  2 +-
 tools/testing/selftests/bpf/progs/pro_epilogue.c   |  4 +--
 .../selftests/bpf/progs/pro_epilogue_goto_start.c  |  4 +--
 tools/testing/selftests/bpf/progs/sock_addr_kern.c |  2 +-
 .../selftests/bpf/progs/struct_ops_detach.c        |  2 +-
 .../selftests/bpf/progs/struct_ops_forgotten_cb.c  |  2 +-
 .../selftests/bpf/progs/struct_ops_maybe_null.c    |  2 +-
 .../bpf/progs/struct_ops_maybe_null_fail.c         |  2 +-
 .../selftests/bpf/progs/struct_ops_module.c        |  2 +-
 .../selftests/bpf/progs/struct_ops_multi_pages.c   |  2 +-
 .../selftests/bpf/progs/struct_ops_nulled_out_cb.c |  2 +-
 .../bpf/progs/test_kfunc_param_nullable.c          |  2 +-
 .../selftests/bpf/progs/test_module_attach.c       |  2 +-
 .../selftests/bpf/progs/test_tp_btf_nullable.c     |  2 +-
 .../testing/selftests/bpf/progs/unsupported_ops.c  |  2 +-
 tools/testing/selftests/bpf/progs/wq.c             |  2 +-
 tools/testing/selftests/bpf/progs/wq_failures.c    |  2 +-
 .../bpf/{bpf_testmod => test_kmods}/.gitignore     |  0
 tools/testing/selftests/bpf/test_kmods/Makefile    | 25 ++++++++++++++++
 .../bpf_test_no_cfi.c                              |  0
 .../bpf_testmod-events.h                           |  0
 .../bpf/{bpf_testmod => test_kmods}/bpf_testmod.c  |  0
 .../bpf/{bpf_testmod => test_kmods}/bpf_testmod.h  |  0
 .../bpf_testmod_kfunc.h                            |  0
 44 files changed, 79 insertions(+), 95 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index f04af11df8eb5a1cecd75a4864d45c669433df61..4bde8d57a4e940f4672c6723d05da0c9fd8b62e6 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -152,12 +152,14 @@ TEST_PROGS_EXTENDED := with_addr.sh \
 	with_tunnels.sh ima_setup.sh verify_sig_setup.sh \
 	test_xdp_vlan.sh test_bpftool.py
 
+TEST_KMODS := bpf_testmod.ko bpf_test_no_cfi.ko
+
 # Compile but not part of 'make run_tests'
 TEST_GEN_PROGS_EXTENDED = \
 	flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
-	test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
+	test_lirc_mode2_user xdping test_cpp runqslower bench \
 	xskxceiver xdp_redirect_multi xdp_synproxy veristat xdp_hw_metadata \
-	xdp_features bpf_test_no_cfi.ko
+	xdp_features
 
 TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi
 
@@ -172,8 +174,9 @@ override define CLEAN
 	$(Q)$(RM) -r $(TEST_GEN_PROGS)
 	$(Q)$(RM) -r $(TEST_GEN_PROGS_EXTENDED)
 	$(Q)$(RM) -r $(TEST_GEN_FILES)
+	$(Q)$(RM) -r $(TEST_KMODS)
 	$(Q)$(RM) -r $(EXTRA_CLEAN)
-	$(Q)$(MAKE) -C bpf_testmod clean
+	$(Q)$(MAKE) -C test_kmods clean
 	$(Q)$(MAKE) docs-clean
 endef
 
@@ -239,7 +242,7 @@ endif
 # to build individual tests.
 # NOTE: Semicolon at the end is critical to override lib.mk's default static
 # rule for binaries.
-$(notdir $(TEST_GEN_PROGS)						\
+$(notdir $(TEST_GEN_PROGS) $(TEST_KMODS)				\
 	 $(TEST_GEN_PROGS_EXTENDED)): %: $(OUTPUT)/% ;
 
 # sort removes libbpf duplicates when not cross-building
@@ -291,17 +294,13 @@ $(OUTPUT)/sign-file: ../../../../scripts/sign-file.c
 		  $< -o $@ \
 		  $(shell $(PKG_CONFIG) --libs libcrypto 2> /dev/null || echo -lcrypto)
 
-$(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(RESOLVE_BTFIDS) $(wildcard bpf_testmod/Makefile bpf_testmod/*.[ch])
-	$(call msg,MOD,,$@)
-	$(Q)$(RM) bpf_testmod/bpf_testmod.ko # force re-compilation
-	$(Q)$(MAKE) $(submake_extras) RESOLVE_BTFIDS=$(RESOLVE_BTFIDS) -C bpf_testmod
-	$(Q)cp bpf_testmod/bpf_testmod.ko $@
+test_kmods/modules.built: $(VMLINUX_BTF) $(RESOLVE_BTFIDS) $(wildcard test_kmods/Makefile test_kmods/*.[ch])
+	$(Q)$(RM) test_kmods/*.ko # force re-compilation
+	$(Q)$(MAKE) $(submake_extras) RESOLVE_BTFIDS=$(RESOLVE_BTFIDS) -C test_kmods
 
-$(OUTPUT)/bpf_test_no_cfi.ko: $(VMLINUX_BTF) $(RESOLVE_BTFIDS) $(wildcard bpf_test_no_cfi/Makefile bpf_test_no_cfi/*.[ch])
+$(addprefix $(OUTPUT)/, $(TEST_KMODS)): test_kmods/modules.built
 	$(call msg,MOD,,$@)
-	$(Q)$(RM) bpf_test_no_cfi/bpf_test_no_cfi.ko # force re-compilation
-	$(Q)$(MAKE) $(submake_extras) RESOLVE_BTFIDS=$(RESOLVE_BTFIDS) -C bpf_test_no_cfi
-	$(Q)cp bpf_test_no_cfi/bpf_test_no_cfi.ko $@
+	$(Q)cp test_kmods/$(@F) $@
 
 DEFAULT_BPFTOOL := $(HOST_SCRATCH_DIR)/sbin/bpftool
 ifneq ($(CROSS_COMPILE),)
@@ -720,13 +719,13 @@ TRUNNER_EXTRA_SOURCES := test_progs.c		\
 			 json_writer.c 		\
 			 flow_dissector_load.h	\
 			 ip_check_defrag_frags.h
-TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko	\
-		       $(OUTPUT)/bpf_test_no_cfi.ko			\
+TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read				\
 		       $(OUTPUT)/liburandom_read.so			\
 		       $(OUTPUT)/xdp_synproxy				\
 		       $(OUTPUT)/sign-file				\
 		       $(OUTPUT)/uprobe_multi				\
-		       ima_setup.sh 					\
+		       $(addprefix $(OUTPUT)/, $(TEST_KMODS))		\
+		       ima_setup.sh					\
 		       verify_sig_setup.sh				\
 		       $(wildcard progs/btf_dump_test_case_*.c)		\
 		       $(wildcard progs/*.bpf.o)
@@ -854,8 +853,7 @@ EXTRA_CLEAN := $(SCRATCH_DIR) $(HOST_SCRATCH_DIR)			\
 	prog_tests/tests.h map_tests/tests.h verifier/tests.h		\
 	feature bpftool 						\
 	$(addprefix $(OUTPUT)/,*.o *.d *.skel.h *.lskel.h *.subskel.h	\
-			       no_alu32 cpuv4 bpf_gcc bpf_testmod.ko	\
-			       bpf_test_no_cfi.ko			\
+			       no_alu32 cpuv4 bpf_gcc $(TEST_KMODS)	\
 			       liburandom_read.so)			\
 	$(OUTPUT)/FEATURE-DUMP.selftests
 
diff --git a/tools/testing/selftests/bpf/bpf_test_no_cfi/Makefile b/tools/testing/selftests/bpf/bpf_test_no_cfi/Makefile
deleted file mode 100644
index ed5143b79edf790b5d4e7213507110e2c6fb4886..0000000000000000000000000000000000000000
--- a/tools/testing/selftests/bpf/bpf_test_no_cfi/Makefile
+++ /dev/null
@@ -1,19 +0,0 @@
-BPF_TEST_NO_CFI_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
-KDIR ?= $(abspath $(BPF_TEST_NO_CFI_DIR)/../../../../..)
-
-ifeq ($(V),1)
-Q =
-else
-Q = @
-endif
-
-MODULES = bpf_test_no_cfi.ko
-
-obj-m += bpf_test_no_cfi.o
-
-all:
-	+$(Q)make -C $(KDIR) M=$(BPF_TEST_NO_CFI_DIR) modules
-
-clean:
-	+$(Q)make -C $(KDIR) M=$(BPF_TEST_NO_CFI_DIR) clean
-
diff --git a/tools/testing/selftests/bpf/bpf_testmod/Makefile b/tools/testing/selftests/bpf/bpf_testmod/Makefile
deleted file mode 100644
index 15cb36c4483ac3b970d93d2e4467cd290917c361..0000000000000000000000000000000000000000
--- a/tools/testing/selftests/bpf/bpf_testmod/Makefile
+++ /dev/null
@@ -1,20 +0,0 @@
-BPF_TESTMOD_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
-KDIR ?= $(abspath $(BPF_TESTMOD_DIR)/../../../../..)
-
-ifeq ($(V),1)
-Q =
-else
-Q = @
-endif
-
-MODULES = bpf_testmod.ko
-
-obj-m += bpf_testmod.o
-CFLAGS_bpf_testmod.o = -I$(src)
-
-all:
-	+$(Q)make -C $(KDIR) M=$(BPF_TESTMOD_DIR) modules
-
-clean:
-	+$(Q)make -C $(KDIR) M=$(BPF_TESTMOD_DIR) clean
-
diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
index 26019313e1fc20e6beabde1e0827ee74daa17c69..ef880c7f7373650ef00b36520bf7b508b90e1efd 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -2,7 +2,7 @@
 #define _GNU_SOURCE
 #include <test_progs.h>
 #include "progs/core_reloc_types.h"
-#include "bpf_testmod/bpf_testmod.h"
+#include "test_kmods/bpf_testmod.h"
 #include <linux/limits.h>
 #include <sys/mman.h>
 #include <sys/syscall.h>
diff --git a/tools/testing/selftests/bpf/progs/bad_struct_ops.c b/tools/testing/selftests/bpf/progs/bad_struct_ops.c
index b7e175cd0af0cbc51a16a7d695b52828c40ad0a0..b3f77b4561c8281363b5a083368e5f12dc13d758 100644
--- a/tools/testing/selftests/bpf/progs/bad_struct_ops.c
+++ b/tools/testing/selftests/bpf/progs/bad_struct_ops.c
@@ -3,7 +3,7 @@
 #include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
-#include "../bpf_testmod/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/cb_refs.c b/tools/testing/selftests/bpf/progs/cb_refs.c
index 56c764df8196793155d69967ca1c4a28099d2540..5d6fc7f01ebb69e26351f3811ac1551a9e72a414 100644
--- a/tools/testing/selftests/bpf/progs/cb_refs.c
+++ b/tools/testing/selftests/bpf/progs/cb_refs.c
@@ -2,7 +2,7 @@
 #include <vmlinux.h>
 #include <bpf/bpf_tracing.h>
 #include <bpf/bpf_helpers.h>
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 struct map_value {
 	struct prog_test_ref_kfunc __kptr *ptr;
diff --git a/tools/testing/selftests/bpf/progs/epilogue_exit.c b/tools/testing/selftests/bpf/progs/epilogue_exit.c
index 33d3a57bee903ea40aa009cef9b69c354fd6e2c2..35fec7c75bef9a59998ee9fe985ab64b63c2e133 100644
--- a/tools/testing/selftests/bpf/progs/epilogue_exit.c
+++ b/tools/testing/selftests/bpf/progs/epilogue_exit.c
@@ -4,8 +4,8 @@
 #include <vmlinux.h>
 #include <bpf/bpf_tracing.h>
 #include "bpf_misc.h"
-#include "../bpf_testmod/bpf_testmod.h"
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/epilogue_tailcall.c b/tools/testing/selftests/bpf/progs/epilogue_tailcall.c
index 7275dd594de0636e7e5d6460bc7813d24539149d..153514691ba4bc5ce24f81f51d23bffdd9773214 100644
--- a/tools/testing/selftests/bpf/progs/epilogue_tailcall.c
+++ b/tools/testing/selftests/bpf/progs/epilogue_tailcall.c
@@ -4,8 +4,8 @@
 #include <vmlinux.h>
 #include <bpf/bpf_tracing.h>
 #include "bpf_misc.h"
-#include "../bpf_testmod/bpf_testmod.h"
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/iters_testmod.c b/tools/testing/selftests/bpf/progs/iters_testmod.c
index df1d3db60b1b12c36c9d56b0ade990f94f95fca6..9e4b45201e692721ddda6a25a866f44ac05fe6a0 100644
--- a/tools/testing/selftests/bpf/progs/iters_testmod.c
+++ b/tools/testing/selftests/bpf/progs/iters_testmod.c
@@ -4,7 +4,7 @@
 #include "bpf_experimental.h"
 #include <bpf/bpf_helpers.h>
 #include "bpf_misc.h"
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/jit_probe_mem.c b/tools/testing/selftests/bpf/progs/jit_probe_mem.c
index f9789e66829732f174892f4ed85a6db9687f6bf2..82190d79de375642d09416dd971cf2c84ccfcd11 100644
--- a/tools/testing/selftests/bpf/progs/jit_probe_mem.c
+++ b/tools/testing/selftests/bpf/progs/jit_probe_mem.c
@@ -3,7 +3,7 @@
 #include <vmlinux.h>
 #include <bpf/bpf_tracing.h>
 #include <bpf/bpf_helpers.h>
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 static struct prog_test_ref_kfunc __kptr *v;
 long total_sum = -1;
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c b/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c
index 7632d9ecb253ba37d3e85a97cc6a19adc1b08112..b9670e9a6e3df53d1178eb05b437f298ac314fd2 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 SEC("tc")
 int kfunc_destructive_test(void)
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_fail.c b/tools/testing/selftests/bpf/progs/kfunc_call_fail.c
index 08fae306539c591d89201ec10b9ae7b258eab8a5..a1963497f0bff99a21c305a861ddcc35d16c7d66 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_fail.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_fail.c
@@ -2,7 +2,7 @@
 /* Copyright (c) 2021 Facebook */
 #include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 struct syscall_test_args {
 	__u8 data[16];
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_race.c b/tools/testing/selftests/bpf/progs/kfunc_call_race.c
index d532af07decf954ca168335446daf1a3e34c81b6..48f64827cd934d678ff5ced4369408187df0233e 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_race.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_race.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 SEC("tc")
 int kfunc_call_fail(struct __sk_buff *ctx)
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test.c b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
index f502f755f56793d7c0bfa241f0cb79539e1960c4..8b86113a0126162d7ce891ee213927b4e778f612 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_test.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
@@ -2,7 +2,7 @@
 /* Copyright (c) 2021 Facebook */
 #include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 SEC("tc")
 int kfunc_call_test4(struct __sk_buff *skb)
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c b/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
index 2380c75e74ce2c8e9914e4304cbddbcdccd54d3c..8e150e85b50d286d34c7e569cfadfc294cc52dd3 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2021 Facebook */
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 extern const int bpf_prog_active __ksym;
 int active_res = -1;
diff --git a/tools/testing/selftests/bpf/progs/local_kptr_stash.c b/tools/testing/selftests/bpf/progs/local_kptr_stash.c
index b092a72b2c9df567d8cec37bb9d8645310383670..d736506a4c807feb842dbc749a76c6c51b3270a6 100644
--- a/tools/testing/selftests/bpf/progs/local_kptr_stash.c
+++ b/tools/testing/selftests/bpf/progs/local_kptr_stash.c
@@ -6,7 +6,7 @@
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_core_read.h>
 #include "../bpf_experimental.h"
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 struct plain_local;
 
diff --git a/tools/testing/selftests/bpf/progs/map_kptr.c b/tools/testing/selftests/bpf/progs/map_kptr.c
index ab0ce1d01a4a714c6271e36e8831fc5ffadbb553..edaba481db9d83fb47b255c90369f8dde5448449 100644
--- a/tools/testing/selftests/bpf/progs/map_kptr.c
+++ b/tools/testing/selftests/bpf/progs/map_kptr.c
@@ -2,7 +2,7 @@
 #include <vmlinux.h>
 #include <bpf/bpf_tracing.h>
 #include <bpf/bpf_helpers.h>
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 struct map_value {
 	struct prog_test_ref_kfunc __kptr_untrusted *unref_ptr;
diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
index 450bb373b179fc91b028568ee6d787d85147ecbb..c2a6bd392e480e2d32659e94c6904d26ad1ebafe 100644
--- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
@@ -4,7 +4,7 @@
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_core_read.h>
 #include "bpf_misc.h"
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 struct map_value {
 	char buf[8];
diff --git a/tools/testing/selftests/bpf/progs/missed_kprobe.c b/tools/testing/selftests/bpf/progs/missed_kprobe.c
index 7f9ef701f5dee65eb71a4c5236bdf6e570e4ed50..51a4fe64c9175f6aa6f4e0210cb1bbf74b8e4f37 100644
--- a/tools/testing/selftests/bpf/progs/missed_kprobe.c
+++ b/tools/testing/selftests/bpf/progs/missed_kprobe.c
@@ -2,7 +2,7 @@
 #include "vmlinux.h"
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/missed_kprobe_recursion.c b/tools/testing/selftests/bpf/progs/missed_kprobe_recursion.c
index 8ea71cbd6c45160c5c58d61592967489ba9df4ba..c4bf679a987634c711970cf01ce0b041f3f05697 100644
--- a/tools/testing/selftests/bpf/progs/missed_kprobe_recursion.c
+++ b/tools/testing/selftests/bpf/progs/missed_kprobe_recursion.c
@@ -2,7 +2,7 @@
 #include "vmlinux.h"
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/nested_acquire.c b/tools/testing/selftests/bpf/progs/nested_acquire.c
index 8e521a21d995957cd42216418c2607c91c2d5b5f..49ad7b9adf566fc608502649ae5a2885a61f50e6 100644
--- a/tools/testing/selftests/bpf/progs/nested_acquire.c
+++ b/tools/testing/selftests/bpf/progs/nested_acquire.c
@@ -4,7 +4,7 @@
 #include <bpf/bpf_tracing.h>
 #include <bpf/bpf_helpers.h>
 #include "bpf_misc.h"
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/pro_epilogue.c b/tools/testing/selftests/bpf/progs/pro_epilogue.c
index 44bc3f06b4b612b5520f4cec07c61700beb7949d..d97d6e07ef5c156d1c94c6d7b199c50e3ac293e6 100644
--- a/tools/testing/selftests/bpf/progs/pro_epilogue.c
+++ b/tools/testing/selftests/bpf/progs/pro_epilogue.c
@@ -4,8 +4,8 @@
 #include <vmlinux.h>
 #include <bpf/bpf_tracing.h>
 #include "bpf_misc.h"
-#include "../bpf_testmod/bpf_testmod.h"
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/pro_epilogue_goto_start.c b/tools/testing/selftests/bpf/progs/pro_epilogue_goto_start.c
index 3529e53be35501b19a47626ee5c50895a02e67f1..6048d79be48bfce0555827b8d5ca6678617d3050 100644
--- a/tools/testing/selftests/bpf/progs/pro_epilogue_goto_start.c
+++ b/tools/testing/selftests/bpf/progs/pro_epilogue_goto_start.c
@@ -4,8 +4,8 @@
 #include <vmlinux.h>
 #include <bpf/bpf_tracing.h>
 #include "bpf_misc.h"
-#include "../bpf_testmod/bpf_testmod.h"
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/sock_addr_kern.c b/tools/testing/selftests/bpf/progs/sock_addr_kern.c
index 8386bb15ccdc19728cba21cff5f96403903aff7d..84ad515eafd6c84e2762af06de0e598ede70435b 100644
--- a/tools/testing/selftests/bpf/progs/sock_addr_kern.c
+++ b/tools/testing/selftests/bpf/progs/sock_addr_kern.c
@@ -2,7 +2,7 @@
 /* Copyright (c) 2024 Google LLC */
 #include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 SEC("syscall")
 int init_sock(struct init_sock_args *args)
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_detach.c b/tools/testing/selftests/bpf/progs/struct_ops_detach.c
index 56b787a8987610ffe776238801937890244f6184..3b6e6891dd8551c1413b84d1ebac4dde3b357757 100644
--- a/tools/testing/selftests/bpf/progs/struct_ops_detach.c
+++ b/tools/testing/selftests/bpf/progs/struct_ops_detach.c
@@ -2,7 +2,7 @@
 /* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
 #include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
-#include "../bpf_testmod/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_forgotten_cb.c b/tools/testing/selftests/bpf/progs/struct_ops_forgotten_cb.c
index 3c822103bd405b693373824b006c239a85b3a6fa..d8cc99f5c2e2f9832cdc0da18df00a445cc6be40 100644
--- a/tools/testing/selftests/bpf/progs/struct_ops_forgotten_cb.c
+++ b/tools/testing/selftests/bpf/progs/struct_ops_forgotten_cb.c
@@ -2,7 +2,7 @@
 /* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
 #include <vmlinux.h>
 #include <bpf/bpf_tracing.h>
-#include "../bpf_testmod/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_maybe_null.c b/tools/testing/selftests/bpf/progs/struct_ops_maybe_null.c
index b450f72e744ad22a78d98bc3b539527abc7e01c6..ccab3935aa425ec8ff9c7d0aa4cb6ce77fc547f7 100644
--- a/tools/testing/selftests/bpf/progs/struct_ops_maybe_null.c
+++ b/tools/testing/selftests/bpf/progs/struct_ops_maybe_null.c
@@ -2,7 +2,7 @@
 /* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
 #include <vmlinux.h>
 #include <bpf/bpf_tracing.h>
-#include "../bpf_testmod/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_maybe_null_fail.c b/tools/testing/selftests/bpf/progs/struct_ops_maybe_null_fail.c
index 6283099ec383a38bb11c47c6c8f9d51c6dd220e0..8b5515f4f724e8f12fd8f170acbdcd5fb11cb96b 100644
--- a/tools/testing/selftests/bpf/progs/struct_ops_maybe_null_fail.c
+++ b/tools/testing/selftests/bpf/progs/struct_ops_maybe_null_fail.c
@@ -2,7 +2,7 @@
 /* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
 #include <vmlinux.h>
 #include <bpf/bpf_tracing.h>
-#include "../bpf_testmod/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_module.c b/tools/testing/selftests/bpf/progs/struct_ops_module.c
index 4c56d4a9d9f410b42430bd58cd4a0011047e58c7..71c420c3a5a6c7ad8fca16fd3e6432b36027811c 100644
--- a/tools/testing/selftests/bpf/progs/struct_ops_module.c
+++ b/tools/testing/selftests/bpf/progs/struct_ops_module.c
@@ -3,7 +3,7 @@
 #include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
-#include "../bpf_testmod/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_multi_pages.c b/tools/testing/selftests/bpf/progs/struct_ops_multi_pages.c
index 9efcc6e4d3566d9814933d0d9a3568a3f10d41b7..5b23ea817f1f90d9db1333e00031bdb669509029 100644
--- a/tools/testing/selftests/bpf/progs/struct_ops_multi_pages.c
+++ b/tools/testing/selftests/bpf/progs/struct_ops_multi_pages.c
@@ -3,7 +3,7 @@
 #include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
-#include "../bpf_testmod/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_nulled_out_cb.c b/tools/testing/selftests/bpf/progs/struct_ops_nulled_out_cb.c
index fa2021388485d67192e8b60f29803f206cda7521..5d0937fa07be20face0300cfebacd387f79ac66f 100644
--- a/tools/testing/selftests/bpf/progs/struct_ops_nulled_out_cb.c
+++ b/tools/testing/selftests/bpf/progs/struct_ops_nulled_out_cb.c
@@ -2,7 +2,7 @@
 /* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
 #include <vmlinux.h>
 #include <bpf/bpf_tracing.h>
-#include "../bpf_testmod/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/test_kfunc_param_nullable.c b/tools/testing/selftests/bpf/progs/test_kfunc_param_nullable.c
index 7ac7e1de34d811865ca187a200bc6f213d482511..0ad1bf1ede8dc7f83f2e922d918ce40d196b64a4 100644
--- a/tools/testing/selftests/bpf/progs/test_kfunc_param_nullable.c
+++ b/tools/testing/selftests/bpf/progs/test_kfunc_param_nullable.c
@@ -4,7 +4,7 @@
 #include <bpf/bpf_helpers.h>
 #include "bpf_misc.h"
 #include "bpf_kfuncs.h"
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 SEC("tc")
 int kfunc_dynptr_nullable_test1(struct __sk_buff *skb)
diff --git a/tools/testing/selftests/bpf/progs/test_module_attach.c b/tools/testing/selftests/bpf/progs/test_module_attach.c
index cc1a012d038f1ddca8c982a88b9ef15802f39610..fb07f5773888b05f0c653bc168769c85c99811fb 100644
--- a/tools/testing/selftests/bpf/progs/test_module_attach.c
+++ b/tools/testing/selftests/bpf/progs/test_module_attach.c
@@ -5,7 +5,7 @@
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
 #include <bpf/bpf_core_read.h>
-#include "../bpf_testmod/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod.h"
 
 __u32 raw_tp_read_sz = 0;
 
diff --git a/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c b/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c
index bba3e37f749b866b9d1af47958ed88b9193cab30..39ff06f2c834ae28081da7dae23755d5a8699e83 100644
--- a/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c
+++ b/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c
@@ -3,7 +3,7 @@
 #include "vmlinux.h"
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
-#include "../bpf_testmod/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod.h"
 #include "bpf_misc.h"
 
 SEC("tp_btf/bpf_testmod_test_nullable_bare")
diff --git a/tools/testing/selftests/bpf/progs/unsupported_ops.c b/tools/testing/selftests/bpf/progs/unsupported_ops.c
index 9180365a3568f8f5d04682704ae08588bcdc2025..8aa2e0dd624e4b8d83a9377ae3b4adfd62bc350b 100644
--- a/tools/testing/selftests/bpf/progs/unsupported_ops.c
+++ b/tools/testing/selftests/bpf/progs/unsupported_ops.c
@@ -4,7 +4,7 @@
 #include <vmlinux.h>
 #include <bpf/bpf_tracing.h>
 #include "bpf_misc.h"
-#include "../bpf_testmod/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/wq.c b/tools/testing/selftests/bpf/progs/wq.c
index f8d3ae0c29aeb3da58c604f6a2eb6aeb3c90e365..2f1ba08c293e26c48e0866ea00ff73b8517faab3 100644
--- a/tools/testing/selftests/bpf/progs/wq.c
+++ b/tools/testing/selftests/bpf/progs/wq.c
@@ -5,7 +5,7 @@
 #include "bpf_experimental.h"
 #include <bpf/bpf_helpers.h>
 #include "bpf_misc.h"
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/wq_failures.c b/tools/testing/selftests/bpf/progs/wq_failures.c
index 25b51a72fe0fe6d0f1253e22e204aaf6c1946fc6..4240211a19001fbbe0860da761121917a91049f5 100644
--- a/tools/testing/selftests/bpf/progs/wq_failures.c
+++ b/tools/testing/selftests/bpf/progs/wq_failures.c
@@ -5,7 +5,7 @@
 #include "bpf_experimental.h"
 #include <bpf/bpf_helpers.h>
 #include "bpf_misc.h"
-#include "../bpf_testmod/bpf_testmod_kfunc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/bpf_testmod/.gitignore b/tools/testing/selftests/bpf/test_kmods/.gitignore
similarity index 100%
rename from tools/testing/selftests/bpf/bpf_testmod/.gitignore
rename to tools/testing/selftests/bpf/test_kmods/.gitignore
diff --git a/tools/testing/selftests/bpf/test_kmods/Makefile b/tools/testing/selftests/bpf/test_kmods/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..393f407f35baf7e2b657b5d7910a6ffdecb35910
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_kmods/Makefile
@@ -0,0 +1,25 @@
+TEST_KMOD_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
+KDIR ?= $(abspath $(TEST_KMOD_DIR)/../../../../..)
+
+ifeq ($(V),1)
+Q =
+else
+Q = @
+endif
+
+MODULES = bpf_testmod.ko bpf_test_no_cfi.ko
+
+$(foreach m,$(MODULES),$(eval obj-m += $(m:.ko=.o)))
+
+CFLAGS_bpf_testmod.o = -I$(src)
+
+all: modules.built
+
+modules.built: *.[ch]
+	+$(Q)make -C $(KDIR) M=$(TEST_KMOD_DIR) modules
+	touch modules.built
+
+clean:
+	+$(Q)make -C $(KDIR) M=$(TEST_KMOD_DIR) clean
+	rm -f modules.built
+
diff --git a/tools/testing/selftests/bpf/bpf_test_no_cfi/bpf_test_no_cfi.c b/tools/testing/selftests/bpf/test_kmods/bpf_test_no_cfi.c
similarity index 100%
rename from tools/testing/selftests/bpf/bpf_test_no_cfi/bpf_test_no_cfi.c
rename to tools/testing/selftests/bpf/test_kmods/bpf_test_no_cfi.c
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
similarity index 100%
rename from tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
rename to tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
similarity index 100%
rename from tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
rename to tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.h
similarity index 100%
rename from tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
rename to tools/testing/selftests/bpf/test_kmods/bpf_testmod.h
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
similarity index 100%
rename from tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
rename to tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h

-- 
2.47.0

[PATCH bpf 3/4] selftests/bpf: Provide a generic [un]load_module helper

[Toke Høiland-Jørgensen]

From: Simon Sundberg <simon.sundberg@kau.se>

Generalize the previous [un]load_bpf_testmod() helpers (in
testing_helpers.c) to the more generic [un]load_module(), which can
load an arbitrary kernel module by name. This allows future selftests
to more easily load custom kernel modules other than bpf_testmod.ko.
Refactor [un]load_bpf_testmod() to wrap this new helper.

Signed-off-by: Simon Sundberg <simon.sundberg@kau.se>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 tools/testing/selftests/bpf/testing_helpers.c | 34 +++++++++++++++++----------
 tools/testing/selftests/bpf/testing_helpers.h |  2 ++
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
index d3c3c3a24150f99abd13ecb7d7b11d8f7351560d..5e9f16683be5460b1a295fb9754df761cbd090ea 100644
--- a/tools/testing/selftests/bpf/testing_helpers.c
+++ b/tools/testing/selftests/bpf/testing_helpers.c
@@ -367,7 +367,7 @@ int delete_module(const char *name, int flags)
 	return syscall(__NR_delete_module, name, flags);
 }
 
-int unload_bpf_testmod(bool verbose)
+int unload_module(const char *name, bool verbose)
 {
 	int ret, cnt = 0;
 
@@ -375,11 +375,11 @@ int unload_bpf_testmod(bool verbose)
 		fprintf(stdout, "Failed to trigger kernel-side RCU sync!\n");
 
 	for (;;) {
-		ret = delete_module("bpf_testmod", 0);
+		ret = delete_module(name, 0);
 		if (!ret || errno != EAGAIN)
 			break;
 		if (++cnt > 10000) {
-			fprintf(stdout, "Unload of bpf_testmod timed out\n");
+			fprintf(stdout, "Unload of %s timed out\n", name);
 			break;
 		}
 		usleep(100);
@@ -388,41 +388,51 @@ int unload_bpf_testmod(bool verbose)
 	if (ret) {
 		if (errno == ENOENT) {
 			if (verbose)
-				fprintf(stdout, "bpf_testmod.ko is already unloaded.\n");
+				fprintf(stdout, "%s.ko is already unloaded.\n", name);
 			return -1;
 		}
-		fprintf(stdout, "Failed to unload bpf_testmod.ko from kernel: %d\n", -errno);
+		fprintf(stdout, "Failed to unload %s.ko from kernel: %d\n", name, -errno);
 		return -1;
 	}
 	if (verbose)
-		fprintf(stdout, "Successfully unloaded bpf_testmod.ko.\n");
+		fprintf(stdout, "Successfully unloaded %s.ko.\n", name);
 	return 0;
 }
 
-int load_bpf_testmod(bool verbose)
+int load_module(const char *path, bool verbose)
 {
 	int fd;
 
 	if (verbose)
-		fprintf(stdout, "Loading bpf_testmod.ko...\n");
+		fprintf(stdout, "Loading %s...\n", path);
 
-	fd = open("bpf_testmod.ko", O_RDONLY);
+	fd = open(path, O_RDONLY);
 	if (fd < 0) {
-		fprintf(stdout, "Can't find bpf_testmod.ko kernel module: %d\n", -errno);
+		fprintf(stdout, "Can't find %s kernel module: %d\n", path, -errno);
 		return -ENOENT;
 	}
 	if (finit_module(fd, "", 0)) {
-		fprintf(stdout, "Failed to load bpf_testmod.ko into the kernel: %d\n", -errno);
+		fprintf(stdout, "Failed to load %s into the kernel: %d\n", path, -errno);
 		close(fd);
 		return -EINVAL;
 	}
 	close(fd);
 
 	if (verbose)
-		fprintf(stdout, "Successfully loaded bpf_testmod.ko.\n");
+		fprintf(stdout, "Successfully loaded %s.\n", path);
 	return 0;
 }
 
+int unload_bpf_testmod(bool verbose)
+{
+	return unload_module("bpf_testmod", verbose);
+}
+
+int load_bpf_testmod(bool verbose)
+{
+	return load_module("bpf_testmod.ko", verbose);
+}
+
 /*
  * Trigger synchronize_rcu() in kernel.
  */
diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h
index d55f6ab124338ccab33bc120ca7e3baa18264aea..46d7f7089f636b0d2476859fd0fa5e1c4b305419 100644
--- a/tools/testing/selftests/bpf/testing_helpers.h
+++ b/tools/testing/selftests/bpf/testing_helpers.h
@@ -38,6 +38,8 @@ int unload_bpf_testmod(bool verbose);
 int kern_sync_rcu(void);
 int finit_module(int fd, const char *param_values, int flags);
 int delete_module(const char *name, int flags);
+int load_module(const char *path, bool verbose);
+int unload_module(const char *name, bool verbose);
 
 static inline __u64 get_time_ns(void)
 {

-- 
2.47.0

[PATCH bpf 4/4] selftests/bpf: Add test for kfunc module order

[Toke Høiland-Jørgensen]

From: Simon Sundberg <simon.sundberg@kau.se>

Add a test case for kfuncs from multiple external modules, checking
that the correct kfuncs are called regardless of which order they're
called in. Specifically, check that calling the kfuncs in an order
different from the one the modules' BTF are loaded in works.

Signed-off-by: Simon Sundberg <simon.sundberg@kau.se>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 tools/testing/selftests/bpf/Makefile               |  3 +-
 .../selftests/bpf/prog_tests/kfunc_module_order.c  | 55 ++++++++++++++++++++++
 .../selftests/bpf/progs/kfunc_module_order.c       | 30 ++++++++++++
 tools/testing/selftests/bpf/test_kmods/Makefile    |  3 +-
 .../selftests/bpf/test_kmods/bpf_test_modorder_x.c | 39 +++++++++++++++
 .../selftests/bpf/test_kmods/bpf_test_modorder_y.c | 39 +++++++++++++++
 6 files changed, 167 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 4bde8d57a4e940f4672c6723d05da0c9fd8b62e6..1e3e05d1ab804b585c3083e97c0f3d2c2a6655b8 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -152,7 +152,8 @@ TEST_PROGS_EXTENDED := with_addr.sh \
 	with_tunnels.sh ima_setup.sh verify_sig_setup.sh \
 	test_xdp_vlan.sh test_bpftool.py
 
-TEST_KMODS := bpf_testmod.ko bpf_test_no_cfi.ko
+TEST_KMODS := bpf_testmod.ko bpf_test_no_cfi.ko bpf_test_modorder_x.ko \
+	bpf_test_modorder_y.ko
 
 # Compile but not part of 'make run_tests'
 TEST_GEN_PROGS_EXTENDED = \
diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_module_order.c b/tools/testing/selftests/bpf/prog_tests/kfunc_module_order.c
new file mode 100644
index 0000000000000000000000000000000000000000..45872bac024246a3f77c636bd840543d50f6adc4
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/kfunc_module_order.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <testing_helpers.h>
+
+#include "kfunc_module_order.skel.h"
+
+static int test_run_prog(const struct bpf_program *prog,
+			 struct bpf_test_run_opts *opts, int expect_val)
+{
+	int err;
+
+	err = bpf_prog_test_run_opts(bpf_program__fd(prog), opts);
+	if (!ASSERT_OK(err, "bpf_prog_test_run_opts"))
+		return err;
+
+	if (!ASSERT_EQ((int)opts->retval, expect_val, bpf_program__name(prog)))
+		return -EINVAL;
+
+	return 0;
+}
+
+void test_kfunc_module_order(void)
+{
+	struct kfunc_module_order *skel;
+	char pkt_data[64] = {};
+	int err = 0;
+
+	DECLARE_LIBBPF_OPTS(bpf_test_run_opts, test_opts, .data_in = pkt_data,
+			    .data_size_in = sizeof(pkt_data));
+
+	err = load_module("bpf_test_modorder_x.ko",
+			  env_verbosity > VERBOSE_NONE);
+	if (!ASSERT_OK(err, "load bpf_test_modorder_x.ko"))
+		return;
+
+	err = load_module("bpf_test_modorder_y.ko",
+			  env_verbosity > VERBOSE_NONE);
+	if (!ASSERT_OK(err, "load bpf_test_modorder_y.ko"))
+		goto exit_modx;
+
+	skel = kfunc_module_order__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "kfunc_module_order__open_and_load()")) {
+		err = -EINVAL;
+		goto exit_mods;
+	}
+
+	test_run_prog(skel->progs.call_kfunc_xy, &test_opts, 0);
+	test_run_prog(skel->progs.call_kfunc_yx, &test_opts, 0);
+
+	kfunc_module_order__destroy(skel);
+exit_mods:
+	unload_module("bpf_test_modorder_y", env_verbosity > VERBOSE_NONE);
+exit_modx:
+	unload_module("bpf_test_modorder_x", env_verbosity > VERBOSE_NONE);
+}
diff --git a/tools/testing/selftests/bpf/progs/kfunc_module_order.c b/tools/testing/selftests/bpf/progs/kfunc_module_order.c
new file mode 100644
index 0000000000000000000000000000000000000000..76003d04c95f4eaef4c9f3ec640a0da2a20253e2
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/kfunc_module_order.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+extern int bpf_test_modorder_retx(void) __ksym;
+extern int bpf_test_modorder_rety(void) __ksym;
+
+SEC("classifier")
+int call_kfunc_xy(struct __sk_buff *skb)
+{
+	int ret1, ret2;
+
+	ret1 = bpf_test_modorder_retx();
+	ret2 = bpf_test_modorder_rety();
+
+	return ret1 == 'x' && ret2 == 'y' ? 0 : -1;
+}
+
+SEC("classifier")
+int call_kfunc_yx(struct __sk_buff *skb)
+{
+	int ret1, ret2;
+
+	ret1 = bpf_test_modorder_rety();
+	ret2 = bpf_test_modorder_retx();
+
+	return ret1 == 'y' && ret2 == 'x' ? 0 : -1;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_kmods/Makefile b/tools/testing/selftests/bpf/test_kmods/Makefile
index 393f407f35baf7e2b657b5d7910a6ffdecb35910..5057c6aacc58be8182be8884854eba846789a2a8 100644
--- a/tools/testing/selftests/bpf/test_kmods/Makefile
+++ b/tools/testing/selftests/bpf/test_kmods/Makefile
@@ -7,7 +7,8 @@ else
 Q = @
 endif
 
-MODULES = bpf_testmod.ko bpf_test_no_cfi.ko
+MODULES = bpf_testmod.ko bpf_test_no_cfi.ko bpf_test_modorder_x.ko \
+	bpf_test_modorder_y.ko
 
 $(foreach m,$(MODULES),$(eval obj-m += $(m:.ko=.o)))
 
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_test_modorder_x.c b/tools/testing/selftests/bpf/test_kmods/bpf_test_modorder_x.c
new file mode 100644
index 0000000000000000000000000000000000000000..0cc747fa912fcd5b6738af15dc1b8dfb88c33f6b
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_test_modorder_x.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <linux/btf.h>
+#include <linux/module.h>
+#include <linux/init.h>
+
+__bpf_kfunc_start_defs();
+
+__bpf_kfunc int bpf_test_modorder_retx(void)
+{
+	return 'x';
+}
+
+__bpf_kfunc_end_defs();
+
+BTF_KFUNCS_START(bpf_test_modorder_kfunc_x_ids)
+BTF_ID_FLAGS(func, bpf_test_modorder_retx);
+BTF_KFUNCS_END(bpf_test_modorder_kfunc_x_ids)
+
+static const struct btf_kfunc_id_set bpf_test_modorder_x_set = {
+	.owner = THIS_MODULE,
+	.set = &bpf_test_modorder_kfunc_x_ids,
+};
+
+static int __init bpf_test_modorder_x_init(void)
+{
+	return register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS,
+					 &bpf_test_modorder_x_set);
+}
+
+static void __exit bpf_test_modorder_x_exit(void)
+{
+}
+
+module_init(bpf_test_modorder_x_init);
+module_exit(bpf_test_modorder_x_exit);
+
+MODULE_DESCRIPTION("BPF selftest ordertest module X");
+MODULE_LICENSE("GPL");
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_test_modorder_y.c b/tools/testing/selftests/bpf/test_kmods/bpf_test_modorder_y.c
new file mode 100644
index 0000000000000000000000000000000000000000..c627ee085d1305af98c5d7f66d99dcfbf98dc4e1
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_test_modorder_y.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <linux/btf.h>
+#include <linux/module.h>
+#include <linux/init.h>
+
+__bpf_kfunc_start_defs();
+
+__bpf_kfunc int bpf_test_modorder_rety(void)
+{
+	return 'y';
+}
+
+__bpf_kfunc_end_defs();
+
+BTF_KFUNCS_START(bpf_test_modorder_kfunc_y_ids)
+BTF_ID_FLAGS(func, bpf_test_modorder_rety);
+BTF_KFUNCS_END(bpf_test_modorder_kfunc_y_ids)
+
+static const struct btf_kfunc_id_set bpf_test_modorder_y_set = {
+	.owner = THIS_MODULE,
+	.set = &bpf_test_modorder_kfunc_y_ids,
+};
+
+static int __init bpf_test_modorder_y_init(void)
+{
+	return register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS,
+					 &bpf_test_modorder_y_set);
+}
+
+static void __exit bpf_test_modorder_y_exit(void)
+{
+}
+
+module_init(bpf_test_modorder_y_init);
+module_exit(bpf_test_modorder_y_exit);
+
+MODULE_DESCRIPTION("BPF selftest ordertest module Y");
+MODULE_LICENSE("GPL");

-- 
2.47.0

Re: [PATCH bpf 1/4] bpf: fix kfunc btf caching for modules

[Jiri Olsa]

On Tue, Oct 08, 2024 at 12:35:16PM +0200, Toke Høiland-Jørgensen wrote:
> The verifier contains a cache for looking up module BTF objects when
> calling kfuncs defined in modules. This cache uses a 'struct
> bpf_kfunc_btf_tab', which contains a sorted list of BTF objects that
> were already seen in the current verifier run, and the BTF objects are
> looked up by the offset stored in the relocated call instruction using
> bsearch().
> 
> The first time a given offset is seen, the module BTF is loaded from the
> file descriptor passed in by libbpf, and stored into the cache. However,
> there's a bug in the code storing the new entry: it stores a pointer to
> the new cache entry, then calls sort() to keep the cache sorted for the
> next lookup using bsearch(), and then returns the entry that was just
> stored through the stored pointer. However, because sort() modifies the
> list of entries in place *by value*, the stored pointer may no longer
> point to the right entry, in which case the wrong BTF object will be
> returned.
> 
> The end result of this is an intermittent bug where, if a BPF program
> calls two functions with the same signature in two different modules,
> the function from the wrong module may sometimes end up being called.
> Whether this happens depends on the order of the calls in the BPF
> program (as that affects whether sort() reorders the array of BTF
> objects), making it especially hard to track down. Simon, credited as
> reporter below, spent significant effort analysing and creating a
> reproducer for this issue. The reproducer is added as a selftest in a
> subsequent patch.
> 
> The fix is straight forward: simply don't use the stored pointer after
> calling sort(). Since we already have an on-stack pointer to the BTF
> object itself at the point where the function return, just use that, and
> populate it from the cache entry in the branch where the lookup
> succeeds.
> 
> Fixes: 2357672c54c3 ("bpf: Introduce BPF support for kernel module function calls")
> Reported-by: Simon Sundberg <simon.sundberg@kau.se>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>

nice catch

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> ---
>  kernel/bpf/verifier.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 434de48cd24bd8d9fb008e4a1e9e0ab4d75ef90a..98d866ba90bf92e3666fb9a07b36f48d452779c6 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2750,10 +2750,16 @@ static struct btf *__find_kfunc_desc_btf(struct bpf_verifier_env *env,
>  		b->module = mod;
>  		b->offset = offset;
>  
> +		/* sort() reorders entries by value, so b may no longer point
> +		 * to the right entry after this
> +		 */
>  		sort(tab->descs, tab->nr_descs, sizeof(tab->descs[0]),
>  		     kfunc_btf_cmp_by_off, NULL);
> +	} else {
> +		btf = b->btf;
>  	}
> -	return b->btf;
> +
> +	return btf;
>  }
>  
>  void bpf_free_kfunc_btf_tab(struct bpf_kfunc_btf_tab *tab)
> 
> -- 
> 2.47.0
> 

Re: [PATCH bpf 2/4] selftests/bpf: Consolidate kernel modules into common directory

[Jiri Olsa]

On Tue, Oct 08, 2024 at 12:35:17PM +0200, Toke Høiland-Jørgensen wrote:

SNIP

> diff --git a/tools/testing/selftests/bpf/bpf_testmod/.gitignore b/tools/testing/selftests/bpf/test_kmods/.gitignore
> similarity index 100%
> rename from tools/testing/selftests/bpf/bpf_testmod/.gitignore
> rename to tools/testing/selftests/bpf/test_kmods/.gitignore
> diff --git a/tools/testing/selftests/bpf/test_kmods/Makefile b/tools/testing/selftests/bpf/test_kmods/Makefile
> new file mode 100644
> index 0000000000000000000000000000000000000000..393f407f35baf7e2b657b5d7910a6ffdecb35910
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_kmods/Makefile
> @@ -0,0 +1,25 @@
> +TEST_KMOD_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
> +KDIR ?= $(abspath $(TEST_KMOD_DIR)/../../../../..)
> +
> +ifeq ($(V),1)
> +Q =
> +else
> +Q = @
> +endif
> +
> +MODULES = bpf_testmod.ko bpf_test_no_cfi.ko
> +
> +$(foreach m,$(MODULES),$(eval obj-m += $(m:.ko=.o)))
> +
> +CFLAGS_bpf_testmod.o = -I$(src)
> +
> +all: modules.built
> +
> +modules.built: *.[ch]

curious, the top Makefile already checks for test_kmods/*.[ch], do we need *.[ch] ?

jirka

> +	+$(Q)make -C $(KDIR) M=$(TEST_KMOD_DIR) modules
> +	touch modules.built
> +
> +clean:
> +	+$(Q)make -C $(KDIR) M=$(TEST_KMOD_DIR) clean
> +	rm -f modules.built
> +
> diff --git a/tools/testing/selftests/bpf/bpf_test_no_cfi/bpf_test_no_cfi.c b/tools/testing/selftests/bpf/test_kmods/bpf_test_no_cfi.c
> similarity index 100%
> rename from tools/testing/selftests/bpf/bpf_test_no_cfi/bpf_test_no_cfi.c
> rename to tools/testing/selftests/bpf/test_kmods/bpf_test_no_cfi.c
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
> similarity index 100%
> rename from tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
> rename to tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> similarity index 100%
> rename from tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> rename to tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.h
> similarity index 100%
> rename from tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
> rename to tools/testing/selftests/bpf/test_kmods/bpf_testmod.h
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
> similarity index 100%
> rename from tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> rename to tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
> 
> -- 
> 2.47.0
> 

Re: [PATCH bpf 4/4] selftests/bpf: Add test for kfunc module order

[Jiri Olsa]

On Tue, Oct 08, 2024 at 12:35:19PM +0200, Toke Høiland-Jørgensen wrote:

SNIP

> +static int test_run_prog(const struct bpf_program *prog,
> +			 struct bpf_test_run_opts *opts, int expect_val)
> +{
> +	int err;
> +
> +	err = bpf_prog_test_run_opts(bpf_program__fd(prog), opts);
> +	if (!ASSERT_OK(err, "bpf_prog_test_run_opts"))
> +		return err;
> +
> +	if (!ASSERT_EQ((int)opts->retval, expect_val, bpf_program__name(prog)))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +void test_kfunc_module_order(void)
> +{
> +	struct kfunc_module_order *skel;
> +	char pkt_data[64] = {};
> +	int err = 0;
> +
> +	DECLARE_LIBBPF_OPTS(bpf_test_run_opts, test_opts, .data_in = pkt_data,
> +			    .data_size_in = sizeof(pkt_data));
> +
> +	err = load_module("bpf_test_modorder_x.ko",
> +			  env_verbosity > VERBOSE_NONE);
> +	if (!ASSERT_OK(err, "load bpf_test_modorder_x.ko"))
> +		return;
> +
> +	err = load_module("bpf_test_modorder_y.ko",
> +			  env_verbosity > VERBOSE_NONE);
> +	if (!ASSERT_OK(err, "load bpf_test_modorder_y.ko"))
> +		goto exit_modx;
> +
> +	skel = kfunc_module_order__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "kfunc_module_order__open_and_load()")) {
> +		err = -EINVAL;
> +		goto exit_mods;
> +	}
> +
> +	test_run_prog(skel->progs.call_kfunc_xy, &test_opts, 0);
> +	test_run_prog(skel->progs.call_kfunc_yx, &test_opts, 0);

nit, no need to pass expect_val, it's always 0

jirka

Re: [PATCH bpf 3/4] selftests/bpf: Provide a generic [un]load_module helper

[Jiri Olsa]

On Tue, Oct 08, 2024 at 12:35:18PM +0200, Toke Høiland-Jørgensen wrote:
> From: Simon Sundberg <simon.sundberg@kau.se>
> 
> Generalize the previous [un]load_bpf_testmod() helpers (in
> testing_helpers.c) to the more generic [un]load_module(), which can
> load an arbitrary kernel module by name. This allows future selftests
> to more easily load custom kernel modules other than bpf_testmod.ko.
> Refactor [un]load_bpf_testmod() to wrap this new helper.
> 
> Signed-off-by: Simon Sundberg <simon.sundberg@kau.se>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> ---
>  tools/testing/selftests/bpf/testing_helpers.c | 34 +++++++++++++++++----------
>  tools/testing/selftests/bpf/testing_helpers.h |  2 ++
>  2 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
> index d3c3c3a24150f99abd13ecb7d7b11d8f7351560d..5e9f16683be5460b1a295fb9754df761cbd090ea 100644
> --- a/tools/testing/selftests/bpf/testing_helpers.c
> +++ b/tools/testing/selftests/bpf/testing_helpers.c
> @@ -367,7 +367,7 @@ int delete_module(const char *name, int flags)
>  	return syscall(__NR_delete_module, name, flags);
>  }
>  
> -int unload_bpf_testmod(bool verbose)
> +int unload_module(const char *name, bool verbose)
>  {
>  	int ret, cnt = 0;
>  
> @@ -375,11 +375,11 @@ int unload_bpf_testmod(bool verbose)
>  		fprintf(stdout, "Failed to trigger kernel-side RCU sync!\n");
>  
>  	for (;;) {
> -		ret = delete_module("bpf_testmod", 0);
> +		ret = delete_module(name, 0);
>  		if (!ret || errno != EAGAIN)
>  			break;
>  		if (++cnt > 10000) {
> -			fprintf(stdout, "Unload of bpf_testmod timed out\n");
> +			fprintf(stdout, "Unload of %s timed out\n", name);
>  			break;
>  		}
>  		usleep(100);
> @@ -388,41 +388,51 @@ int unload_bpf_testmod(bool verbose)
>  	if (ret) {
>  		if (errno == ENOENT) {
>  			if (verbose)
> -				fprintf(stdout, "bpf_testmod.ko is already unloaded.\n");
> +				fprintf(stdout, "%s.ko is already unloaded.\n", name);
>  			return -1;
>  		}
> -		fprintf(stdout, "Failed to unload bpf_testmod.ko from kernel: %d\n", -errno);
> +		fprintf(stdout, "Failed to unload %s.ko from kernel: %d\n", name, -errno);
>  		return -1;
>  	}
>  	if (verbose)
> -		fprintf(stdout, "Successfully unloaded bpf_testmod.ko.\n");
> +		fprintf(stdout, "Successfully unloaded %s.ko.\n", name);
>  	return 0;
>  }
>  
> -int load_bpf_testmod(bool verbose)
> +int load_module(const char *path, bool verbose)
>  {
>  	int fd;
>  
>  	if (verbose)
> -		fprintf(stdout, "Loading bpf_testmod.ko...\n");
> +		fprintf(stdout, "Loading %s...\n", path);
>  
> -	fd = open("bpf_testmod.ko", O_RDONLY);
> +	fd = open(path, O_RDONLY);
>  	if (fd < 0) {
> -		fprintf(stdout, "Can't find bpf_testmod.ko kernel module: %d\n", -errno);
> +		fprintf(stdout, "Can't find %s kernel module: %d\n", path, -errno);
>  		return -ENOENT;
>  	}
>  	if (finit_module(fd, "", 0)) {
> -		fprintf(stdout, "Failed to load bpf_testmod.ko into the kernel: %d\n", -errno);
> +		fprintf(stdout, "Failed to load %s into the kernel: %d\n", path, -errno);
>  		close(fd);
>  		return -EINVAL;
>  	}
>  	close(fd);
>  
>  	if (verbose)
> -		fprintf(stdout, "Successfully loaded bpf_testmod.ko.\n");
> +		fprintf(stdout, "Successfully loaded %s.\n", path);
>  	return 0;
>  }
>  
> +int unload_bpf_testmod(bool verbose)
> +{
> +	return unload_module("bpf_testmod", verbose);
> +}
> +
> +int load_bpf_testmod(bool verbose)
> +{
> +	return load_module("bpf_testmod.ko", verbose);
> +}
> +
>  /*
>   * Trigger synchronize_rcu() in kernel.
>   */
> diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h
> index d55f6ab124338ccab33bc120ca7e3baa18264aea..46d7f7089f636b0d2476859fd0fa5e1c4b305419 100644
> --- a/tools/testing/selftests/bpf/testing_helpers.h
> +++ b/tools/testing/selftests/bpf/testing_helpers.h
> @@ -38,6 +38,8 @@ int unload_bpf_testmod(bool verbose);
>  int kern_sync_rcu(void);
>  int finit_module(int fd, const char *param_values, int flags);
>  int delete_module(const char *name, int flags);
> +int load_module(const char *path, bool verbose);
> +int unload_module(const char *name, bool verbose);
>  
>  static inline __u64 get_time_ns(void)
>  {
> 
> -- 
> 2.47.0
> 

Re: [PATCH bpf 2/4] selftests/bpf: Consolidate kernel modules into common directory

[Toke Høiland-Jørgensen]

Jiri Olsa <olsajiri@gmail.com> writes:

> On Tue, Oct 08, 2024 at 12:35:17PM +0200, Toke Høiland-Jørgensen wrote:
>
> SNIP
>
>> diff --git a/tools/testing/selftests/bpf/bpf_testmod/.gitignore b/tools/testing/selftests/bpf/test_kmods/.gitignore
>> similarity index 100%
>> rename from tools/testing/selftests/bpf/bpf_testmod/.gitignore
>> rename to tools/testing/selftests/bpf/test_kmods/.gitignore
>> diff --git a/tools/testing/selftests/bpf/test_kmods/Makefile b/tools/testing/selftests/bpf/test_kmods/Makefile
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..393f407f35baf7e2b657b5d7910a6ffdecb35910
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/test_kmods/Makefile
>> @@ -0,0 +1,25 @@
>> +TEST_KMOD_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
>> +KDIR ?= $(abspath $(TEST_KMOD_DIR)/../../../../..)
>> +
>> +ifeq ($(V),1)
>> +Q =
>> +else
>> +Q = @
>> +endif
>> +
>> +MODULES = bpf_testmod.ko bpf_test_no_cfi.ko
>> +
>> +$(foreach m,$(MODULES),$(eval obj-m += $(m:.ko=.o)))
>> +
>> +CFLAGS_bpf_testmod.o = -I$(src)
>> +
>> +all: modules.built
>> +
>> +modules.built: *.[ch]
>
> curious, the top Makefile already checks for test_kmods/*.[ch], do we
> need *.[ch] ?

Not really for building from the top-level Makefile, that is for running
'make' inside the subdir, in case anyone tries that. Don't feel strongly
about it, so can remove it if you prefer?

-Toke

Re: [PATCH bpf 4/4] selftests/bpf: Add test for kfunc module order

[Toke Høiland-Jørgensen]

Jiri Olsa <olsajiri@gmail.com> writes:

> On Tue, Oct 08, 2024 at 12:35:19PM +0200, Toke Høiland-Jørgensen wrote:
>
> SNIP
>
>> +static int test_run_prog(const struct bpf_program *prog,
>> +			 struct bpf_test_run_opts *opts, int expect_val)
>> +{
>> +	int err;
>> +
>> +	err = bpf_prog_test_run_opts(bpf_program__fd(prog), opts);
>> +	if (!ASSERT_OK(err, "bpf_prog_test_run_opts"))
>> +		return err;
>> +
>> +	if (!ASSERT_EQ((int)opts->retval, expect_val, bpf_program__name(prog)))
>> +		return -EINVAL;
>> +
>> +	return 0;
>> +}
>> +
>> +void test_kfunc_module_order(void)
>> +{
>> +	struct kfunc_module_order *skel;
>> +	char pkt_data[64] = {};
>> +	int err = 0;
>> +
>> +	DECLARE_LIBBPF_OPTS(bpf_test_run_opts, test_opts, .data_in = pkt_data,
>> +			    .data_size_in = sizeof(pkt_data));
>> +
>> +	err = load_module("bpf_test_modorder_x.ko",
>> +			  env_verbosity > VERBOSE_NONE);
>> +	if (!ASSERT_OK(err, "load bpf_test_modorder_x.ko"))
>> +		return;
>> +
>> +	err = load_module("bpf_test_modorder_y.ko",
>> +			  env_verbosity > VERBOSE_NONE);
>> +	if (!ASSERT_OK(err, "load bpf_test_modorder_y.ko"))
>> +		goto exit_modx;
>> +
>> +	skel = kfunc_module_order__open_and_load();
>> +	if (!ASSERT_OK_PTR(skel, "kfunc_module_order__open_and_load()")) {
>> +		err = -EINVAL;
>> +		goto exit_mods;
>> +	}
>> +
>> +	test_run_prog(skel->progs.call_kfunc_xy, &test_opts, 0);
>> +	test_run_prog(skel->progs.call_kfunc_yx, &test_opts, 0);
>
> nit, no need to pass expect_val, it's always 0

Sure, can get rid of that...

-Toke

Re: [PATCH bpf 2/4] selftests/bpf: Consolidate kernel modules into common directory

[Jiri Olsa]

On Tue, Oct 08, 2024 at 07:55:13PM +0200, Toke Høiland-Jørgensen wrote:
> Jiri Olsa <olsajiri@gmail.com> writes:
> 
> > On Tue, Oct 08, 2024 at 12:35:17PM +0200, Toke Høiland-Jørgensen wrote:
> >
> > SNIP
> >
> >> diff --git a/tools/testing/selftests/bpf/bpf_testmod/.gitignore b/tools/testing/selftests/bpf/test_kmods/.gitignore
> >> similarity index 100%
> >> rename from tools/testing/selftests/bpf/bpf_testmod/.gitignore
> >> rename to tools/testing/selftests/bpf/test_kmods/.gitignore
> >> diff --git a/tools/testing/selftests/bpf/test_kmods/Makefile b/tools/testing/selftests/bpf/test_kmods/Makefile
> >> new file mode 100644
> >> index 0000000000000000000000000000000000000000..393f407f35baf7e2b657b5d7910a6ffdecb35910
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/bpf/test_kmods/Makefile
> >> @@ -0,0 +1,25 @@
> >> +TEST_KMOD_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
> >> +KDIR ?= $(abspath $(TEST_KMOD_DIR)/../../../../..)
> >> +
> >> +ifeq ($(V),1)
> >> +Q =
> >> +else
> >> +Q = @
> >> +endif
> >> +
> >> +MODULES = bpf_testmod.ko bpf_test_no_cfi.ko
> >> +
> >> +$(foreach m,$(MODULES),$(eval obj-m += $(m:.ko=.o)))
> >> +
> >> +CFLAGS_bpf_testmod.o = -I$(src)
> >> +
> >> +all: modules.built
> >> +
> >> +modules.built: *.[ch]
> >
> > curious, the top Makefile already checks for test_kmods/*.[ch], do we
> > need *.[ch] ?
> 
> Not really for building from the top-level Makefile, that is for running
> 'make' inside the subdir, in case anyone tries that. Don't feel strongly
> about it, so can remove it if you prefer?

no strong feelings either ;-) I was just wondering what was the purpose

thanks,
jirka

Re: [PATCH bpf 2/4] selftests/bpf: Consolidate kernel modules into common directory

[Toke Høiland-Jørgensen]

Jiri Olsa <olsajiri@gmail.com> writes:

> On Tue, Oct 08, 2024 at 07:55:13PM +0200, Toke Høiland-Jørgensen wrote:
>> Jiri Olsa <olsajiri@gmail.com> writes:
>> 
>> > On Tue, Oct 08, 2024 at 12:35:17PM +0200, Toke Høiland-Jørgensen wrote:
>> >
>> > SNIP
>> >
>> >> diff --git a/tools/testing/selftests/bpf/bpf_testmod/.gitignore b/tools/testing/selftests/bpf/test_kmods/.gitignore
>> >> similarity index 100%
>> >> rename from tools/testing/selftests/bpf/bpf_testmod/.gitignore
>> >> rename to tools/testing/selftests/bpf/test_kmods/.gitignore
>> >> diff --git a/tools/testing/selftests/bpf/test_kmods/Makefile b/tools/testing/selftests/bpf/test_kmods/Makefile
>> >> new file mode 100644
>> >> index 0000000000000000000000000000000000000000..393f407f35baf7e2b657b5d7910a6ffdecb35910
>> >> --- /dev/null
>> >> +++ b/tools/testing/selftests/bpf/test_kmods/Makefile
>> >> @@ -0,0 +1,25 @@
>> >> +TEST_KMOD_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
>> >> +KDIR ?= $(abspath $(TEST_KMOD_DIR)/../../../../..)
>> >> +
>> >> +ifeq ($(V),1)
>> >> +Q =
>> >> +else
>> >> +Q = @
>> >> +endif
>> >> +
>> >> +MODULES = bpf_testmod.ko bpf_test_no_cfi.ko
>> >> +
>> >> +$(foreach m,$(MODULES),$(eval obj-m += $(m:.ko=.o)))
>> >> +
>> >> +CFLAGS_bpf_testmod.o = -I$(src)
>> >> +
>> >> +all: modules.built
>> >> +
>> >> +modules.built: *.[ch]
>> >
>> > curious, the top Makefile already checks for test_kmods/*.[ch], do we
>> > need *.[ch] ?
>> 
>> Not really for building from the top-level Makefile, that is for running
>> 'make' inside the subdir, in case anyone tries that. Don't feel strongly
>> about it, so can remove it if you prefer?
>
> no strong feelings either ;-) I was just wondering what was the
> purpose

Actually, removing it means the .ko files may not be rebuilt. But it's
not enough with just that dependency either; will give it another try
and see if I can avoid a .PHONY (or just add that if not)...

-Toke

Re: [PATCH bpf 1/4] bpf: fix kfunc btf caching for modules

[Kumar Kartikeya Dwivedi]

On Tue, 8 Oct 2024 at 12:35, Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> The verifier contains a cache for looking up module BTF objects when
> calling kfuncs defined in modules. This cache uses a 'struct
> bpf_kfunc_btf_tab', which contains a sorted list of BTF objects that
> were already seen in the current verifier run, and the BTF objects are
> looked up by the offset stored in the relocated call instruction using
> bsearch().
>
> The first time a given offset is seen, the module BTF is loaded from the
> file descriptor passed in by libbpf, and stored into the cache. However,
> there's a bug in the code storing the new entry: it stores a pointer to
> the new cache entry, then calls sort() to keep the cache sorted for the
> next lookup using bsearch(), and then returns the entry that was just
> stored through the stored pointer. However, because sort() modifies the
> list of entries in place *by value*, the stored pointer may no longer
> point to the right entry, in which case the wrong BTF object will be
> returned.
>
> The end result of this is an intermittent bug where, if a BPF program
> calls two functions with the same signature in two different modules,
> the function from the wrong module may sometimes end up being called.
> Whether this happens depends on the order of the calls in the BPF
> program (as that affects whether sort() reorders the array of BTF
> objects), making it especially hard to track down. Simon, credited as
> reporter below, spent significant effort analysing and creating a
> reproducer for this issue. The reproducer is added as a selftest in a
> subsequent patch.
>
> The fix is straight forward: simply don't use the stored pointer after
> calling sort(). Since we already have an on-stack pointer to the BTF
> object itself at the point where the function return, just use that, and
> populate it from the cache entry in the branch where the lookup
> succeeds.
>
> Fixes: 2357672c54c3 ("bpf: Introduce BPF support for kernel module function calls")
> Reported-by: Simon Sundberg <simon.sundberg@kau.se>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---

Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>

Re: [PATCH bpf 3/4] selftests/bpf: Provide a generic [un]load_module helper

[Kumar Kartikeya Dwivedi]

On Tue, 8 Oct 2024 at 12:35, Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> From: Simon Sundberg <simon.sundberg@kau.se>
>
> Generalize the previous [un]load_bpf_testmod() helpers (in
> testing_helpers.c) to the more generic [un]load_module(), which can
> load an arbitrary kernel module by name. This allows future selftests
> to more easily load custom kernel modules other than bpf_testmod.ko.
> Refactor [un]load_bpf_testmod() to wrap this new helper.
>
> Signed-off-by: Simon Sundberg <simon.sundberg@kau.se>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---

Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>

Re: [PATCH bpf 2/4] selftests/bpf: Consolidate kernel modules into common directory

[Alexei Starovoitov]

On Tue, Oct 8, 2024 at 3:35 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> The selftests build two kernel modules (bpf_testmod.ko and
> bpf_test_no_cfi.ko) which use copy-pasted Makefile targets. This is a
> bit messy, and doesn't scale so well when we add more modules, so let's
> consolidate these rules into a single rule generated for each module
> name, and move the module sources into a single directory.
>
> To avoid parallel builds of the different modules stepping on each
> other's toes during the 'modpost' phase of the Kbuild 'make modules', we
> create a single target for all the defined modules, which contains the
> recursive 'make' call into the modules directory. The Makefile in the
> subdirectory building the modules is modified to also touch a
> 'modules.built' file, which we can add as a dependency on the top-level
> selftests Makefile, thus ensuring that the modules are always rebuilt if
> any of the dependencies in the selftests change.

Nice cleanup, but looks unrelated to the fix and hence
not a bpf material.
Why combine them?

Re: [PATCH bpf 4/4] selftests/bpf: Add test for kfunc module order

[Kumar Kartikeya Dwivedi]

On Tue, 8 Oct 2024 at 12:35, Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> From: Simon Sundberg <simon.sundberg@kau.se>
>
> Add a test case for kfuncs from multiple external modules, checking
> that the correct kfuncs are called regardless of which order they're
> called in. Specifically, check that calling the kfuncs in an order
> different from the one the modules' BTF are loaded in works.
>
> Signed-off-by: Simon Sundberg <simon.sundberg@kau.se>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---

Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>

Re: [PATCH bpf 2/4] selftests/bpf: Consolidate kernel modules into common directory

[Toke Høiland-Jørgensen]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Tue, Oct 8, 2024 at 3:35 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> The selftests build two kernel modules (bpf_testmod.ko and
>> bpf_test_no_cfi.ko) which use copy-pasted Makefile targets. This is a
>> bit messy, and doesn't scale so well when we add more modules, so let's
>> consolidate these rules into a single rule generated for each module
>> name, and move the module sources into a single directory.
>>
>> To avoid parallel builds of the different modules stepping on each
>> other's toes during the 'modpost' phase of the Kbuild 'make modules', we
>> create a single target for all the defined modules, which contains the
>> recursive 'make' call into the modules directory. The Makefile in the
>> subdirectory building the modules is modified to also touch a
>> 'modules.built' file, which we can add as a dependency on the top-level
>> selftests Makefile, thus ensuring that the modules are always rebuilt if
>> any of the dependencies in the selftests change.
>
> Nice cleanup, but looks unrelated to the fix and hence
> not a bpf material.
> Why combine them?

Because the selftest adds two more kernel modules to the selftest build,
so we'd have to add two more directories with a single module in each
and copy-pasted Makefile rules. It seemed simpler to just refactor the
build of the two existing modules first, after which adding the two new
modules means just dropping two more source files into the modules
directory.

I guess we could technically do the single-directory-per-module, and
then send this patch as a follow-up once bpf gets merged back into
bpf-next, but it seems a bit of a hassle, TBH. WDYT?

-Toke

Re: [PATCH bpf 2/4] selftests/bpf: Consolidate kernel modules into common directory

[Alexei Starovoitov]

On Wed, Oct 9, 2024 at 12:39 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>
> > On Tue, Oct 8, 2024 at 3:35 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >>
> >> The selftests build two kernel modules (bpf_testmod.ko and
> >> bpf_test_no_cfi.ko) which use copy-pasted Makefile targets. This is a
> >> bit messy, and doesn't scale so well when we add more modules, so let's
> >> consolidate these rules into a single rule generated for each module
> >> name, and move the module sources into a single directory.
> >>
> >> To avoid parallel builds of the different modules stepping on each
> >> other's toes during the 'modpost' phase of the Kbuild 'make modules', we
> >> create a single target for all the defined modules, which contains the
> >> recursive 'make' call into the modules directory. The Makefile in the
> >> subdirectory building the modules is modified to also touch a
> >> 'modules.built' file, which we can add as a dependency on the top-level
> >> selftests Makefile, thus ensuring that the modules are always rebuilt if
> >> any of the dependencies in the selftests change.
> >
> > Nice cleanup, but looks unrelated to the fix and hence
> > not a bpf material.
> > Why combine them?
>
> Because the selftest adds two more kernel modules to the selftest build,
> so we'd have to add two more directories with a single module in each
> and copy-pasted Makefile rules. It seemed simpler to just refactor the
> build of the two existing modules first, after which adding the two new
> modules means just dropping two more source files into the modules
> directory.
>
> I guess we could technically do the single-directory-per-module, and
> then send this patch as a follow-up once bpf gets merged back into
> bpf-next, but it seems a bit of a hassle, TBH. WDYT?

The way it is right it's certainly not going into bpf tree.
So if you don't want to split then the whole thing is bpf-next then.

Re: [PATCH bpf 2/4] selftests/bpf: Consolidate kernel modules into common directory

[Toke Høiland-Jørgensen]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Wed, Oct 9, 2024 at 12:39 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>>
>> > On Tue, Oct 8, 2024 at 3:35 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>> >>
>> >> The selftests build two kernel modules (bpf_testmod.ko and
>> >> bpf_test_no_cfi.ko) which use copy-pasted Makefile targets. This is a
>> >> bit messy, and doesn't scale so well when we add more modules, so let's
>> >> consolidate these rules into a single rule generated for each module
>> >> name, and move the module sources into a single directory.
>> >>
>> >> To avoid parallel builds of the different modules stepping on each
>> >> other's toes during the 'modpost' phase of the Kbuild 'make modules', we
>> >> create a single target for all the defined modules, which contains the
>> >> recursive 'make' call into the modules directory. The Makefile in the
>> >> subdirectory building the modules is modified to also touch a
>> >> 'modules.built' file, which we can add as a dependency on the top-level
>> >> selftests Makefile, thus ensuring that the modules are always rebuilt if
>> >> any of the dependencies in the selftests change.
>> >
>> > Nice cleanup, but looks unrelated to the fix and hence
>> > not a bpf material.
>> > Why combine them?
>>
>> Because the selftest adds two more kernel modules to the selftest build,
>> so we'd have to add two more directories with a single module in each
>> and copy-pasted Makefile rules. It seemed simpler to just refactor the
>> build of the two existing modules first, after which adding the two new
>> modules means just dropping two more source files into the modules
>> directory.
>>
>> I guess we could technically do the single-directory-per-module, and
>> then send this patch as a follow-up once bpf gets merged back into
>> bpf-next, but it seems a bit of a hassle, TBH. WDYT?
>
> The way it is right it's certainly not going into bpf tree.
> So if you don't want to split then the whole thing is bpf-next then.

ACK. Will see how cumbersome it is to split, and otherwise resubmit the
whole thing against bpf-next.

-Toke