Re: [PATCH 5/5] selftests/bpf: Test two-way arena access with two guests

[bot+bpf-ci]

> selftests/bpf: Test two-way arena access with two guests

TL;DR:
- Unconditional -lzstd in the new Makefile link rules may break the whole
  selftests/bpf build where libzstd is missing
- arena_kvm.py doesn't use ksft.py reporting (minor; results still counted)

> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index a22be7efd1fa..29fffa7857c1 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile

[ ... ]

> @@ -94,6 +100,25 @@ BPFTOOLDIR := $(TOOLSDIR)/bpf/bpftool
>  HOST_BPFOBJ := $(HOST_BUILD_DIR)/libbpf/libbpf.a
>  BPF_TARGET_ENDIAN:=$(if $(IS_LITTLE_ENDIAN),--target=bpfel,--target=bpfeb)
>  
> +# These helpers are installed beside arena_kvm.py, including for OUTPUT=.

[ ... ]

> +$(OUTPUT)/arena_kvm_guest-init: arena_kvm_guest.c arena_kvm_shared.h \
> +		$(BPFOBJ) | $(OUTPUT)
> +	$(call msg,BINARY,,$@)
> +	$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $< $(BPFOBJ) $(LDLIBS) -lzstd -o $@
> +
> +$(OUTPUT)/arena_kvm_host-runner: arena_kvm_host.c arena_kvm_shared.h \
> +		$(BPFOBJ) | $(OUTPUT)
> +	$(call msg,BINARY,,$@)
> +	$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $< $(BPFOBJ) $(LDLIBS) -lzstd -o $@
> +
>  NON_CHECK_FEAT_TARGETS := clean docs-clean emit_tests
>  CHECK_FEAT := $(filter-out $(NON_CHECK_FEAT_TARGETS),$(or $(MAKECMDGOALS), "none"))
>  ifneq ($(CHECK_FEAT),)

Is it intended to pass -lzstd unconditionally in these two link rules?

Neither arena_kvm_guest.c nor arena_kvm_host.c includes a zstd header or
calls into zstd, and no other userspace link line in selftests/bpf uses
-lzstd.  test_progs, for example, links with plain $(LDLIBS), which is
$(LIBELF_LIBS) -lz -lrt -lpthread.

Elsewhere in tools/, libzstd is optional.  bpftool only adds it when the
feature-libelf-zstd probe passes, and libelf builds without zstd are a
supported configuration.

Both binaries are in TEST_GEN_FILES without a CLANG_CPUV4 or feature guard,
and these link rules do not use $(call skip_on_fail,...).  On a host or
sysroot that has libelf and zlib development files but no linkable libzstd,
wouldn't 'make -C tools/testing/selftests/bpf' now fail with "cannot find
-lzstd", even with BPF_STRICT_BUILD=0?  That would block all of
selftests/bpf, not just this test.

Would dropping -lzstd be enough, since a dynamic libelf pulls in its own
dependencies?  If it is needed for a static libelf, it could be made
conditional the same way bpftool does it.

> diff --git a/tools/testing/selftests/bpf/arena_kvm.py b/tools/testing/selftests/bpf/arena_kvm.py
> new file mode 100755
> index 000000000000..552bfcfb80ac
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/arena_kvm.py
> @@ -0,0 +1,448 @@

[ ... ]

> +KSFT_SKIP = 4

[ ... ]

> +        print(f"OK: two guests exchanged BPF values through arena slices {offsets}")

[ ... ]

> +def skip(reason):
> +    print(f"SKIP: {reason}")
> +    return KSFT_SKIP

[ ... ]

> +if __name__ == "__main__":

[ ... ]

> +        stop_children()
> +    sys.exit(status)

A subsystem guideline flags this as potentially concerning: a test that
prints its own ad hoc result text and exits directly, instead of going
through the kselftest reporting helpers.

arena_kvm.py defines its own KSFT_SKIP, prints hand-written "SKIP: ..." and
"OK: ..." lines, and then calls sys.exit(status) directly.
tools/testing/selftests/kselftest/ksft.py provides KSFT_SKIP,
test_result_pass(), test_result_skip(), test_result_fail(), exit_pass() and
exit_fail(), and none of them are used here, so the output is not KTAP.

Should this script report through ksft.py instead?

For context, when the kselftest runner executes a TEST_PROGS entry, it
prefixes the script's output with '# ' and turns the exit code into the TAP
result line (exit 4 becomes '# SKIP'), so the one result this script
produces is still counted correctly.  The other TEST_PROGS scripts in this
directory (test_kmod.sh, test_xsk.sh, test_xdp_features.sh) also print their
own results.  The guideline text is written about C test binaries and does
not name the Python helper.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38093513563