2024-08-10 07:02 UTC+0200 ~ Salvatore Bonaccorso carnil@debian.org
Hi Greg,
[adding as well people involved in the original commit and the backporting for 6.1.y branch]
On Thu, Aug 08, 2024 at 12:33:22PM +0200, Salvatore Bonaccorso wrote:
Hi Greg,
On Thu, Aug 08, 2024 at 11:11:49AM +0200, Greg Kroah-Hartman wrote:
This is the start of the stable review cycle for the 6.1.104 release. There are 86 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Sat, 10 Aug 2024 09:11:02 +0000. Anything received after that time might be too late.
Sorry for bothering you again with it (see previous comment on 6.1.103, respectively 6.1.104-rc1): bpftool still would fail to compile:
gcc -O2 -W -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wbad-function-cast -Wdeclaration-after-statement -Wformat-security -Wformat-y2k -Winit-self -Wmissing-declarations -Wmissing-prototypes -Wno-system-headers -Wold-style-definition -Wpacked -Wredundant-decls -Wstrict-prototypes -Wswitch-default -Wundef -Wwrite-strings -Wformat -Wno-type-limits -Wstrict-aliasing=3 -Wshadow -DPACKAGE='"bpftool"' -D__EXPORTED_HEADERS__ -I. -I/home/build/linux-stable-rc/tools/bpf/bpftool/libbpf/include -I/home/build/linux-stable-rc/kernel/bpf/ -I/home/build/linux-stable-rc/tools/include -I/home/build/linux-stable-rc/tools/include/uapi -DUSE_LIBCAP -DBPFTOOL_WITHOUT_SKELETONS -c -MMD prog.c -o prog.o prog.c: In function ‘load_with_options’: prog.c:1710:23: warning: implicit declaration of function ‘create_and_mount_bpffs_dir’ [-Wimplicit-function-declaration] 1710 | err = create_and_mount_bpffs_dir(pinmaps); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ gcc -O2 -W -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wbad-function-cast -Wdeclaration-after-statement -Wformat-security -Wformat-y2k -Winit-self -Wmissing-declarations -Wmissing-prototypes -Wno-system-headers -Wold-style-definition -Wpacked -Wredundant-decls -Wstrict-prototypes -Wswitch-default -Wundef -Wwrite-strings -Wformat -Wno-type-limits -Wstrict-aliasing=3 -Wshadow -DPACKAGE='"bpftool"' -D__EXPORTED_HEADERS__ -I. -I/home/build/linux-stable-rc/tools/bpf/bpftool/libbpf/include -I/home/build/linux-stable-rc/kernel/bpf/ -I/home/build/linux-stable-rc/tools/include -I/home/build/linux-stable-rc/tools/include/uapi -DUSE_LIBCAP -DBPFTOOL_WITHOUT_SKELETONS btf.o btf_dumper.o cfg.o cgroup.o common.o feature.o gen.o iter.o json_writer.o link.o main.o map.o map_perf_ring.o net.o netlink_dumper.o perf.o pids.o prog.o struct_ops.o tracelog.o xlated_dumper.o disasm.o /home/build/linux-stable-rc/tools/bpf/bpftool/libbpf/libbpf.a -lelf -lz -lcap -o bpftool /bin/ld: prog.o: in function `load_with_options': prog.c:(.text+0x2f98): undefined reference to `create_and_mount_bpffs_dir' /bin/ld: prog.c:(.text+0x2ff2): undefined reference to `create_and_mount_bpffs_dir' collect2: error: ld returned 1 exit status make[1]: *** [Makefile:216: bpftool] Error 1 make: *** [Makefile:113: bpftool] Error 2
Reverting 65dd9cbafec2f6f7908cebcab0386f750fc352af fixes the issue. In fact 65dd9cbafec2f6f7908cebcab0386f750fc352af is the only commit adding call to create_and_mount_bpffs_dir:
$ git grep create_and_mount_bpffs_dir tools/bpf/bpftool/prog.c: err = create_and_mount_bpffs_dir(pinmaps);
Just one additional note, at least 478a535ae54a ("bpftool: Mount bpffs on provided dir instead of parent dir") would be a reqisite where the code was refactored introducing create_and_mount_bpffs_dir() (but won't apply cleanly to 6.1.y). But are more requisites needed?
Should it be safest to just revert the breaking commit for the bpftool build?
Regards, Salvatore
Hi,
You should be able to fix the build by first cherry-picking commit 2a36c26fe3b8 ("bpftool: Support bpffs mountpoint as pin path for prog loadall"), and then commit 478a535ae54a ("bpftool: Mount bpffs on provided dir instead of parent dir") as you figured. Both commits have a minor conflict on tools/bpf/bpftool/struct_ops.c, which should be addressed by discarding the relevant hunk (for both commit).
Alternatively, it's also fine to revert the breaking commit. It's a quality of life improvement without which users may have to manually mount the bpffs at the location they want to pin their maps when loading multiple BPF programs with "bpftool prog loadall", in the unlikely event they're not using /sys/kernel/bpf, prior to running the bpftool command. It's not in use during the kernel build process or for the BPF selftests, so not necessary on stable branches.
I hope this helps, Quentin