Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 1 · Low: 1
- [Medium] Incomplete fix: the new guard `ifeq ($(LLVM)$(CC),cc)` in
tools/testing/selftests/drivers/net/hw/Makefile only overrides CC when…
- [Low] Missing `Fixes:` tag.
--
selftests: drv-net: Use cross-compilation environment for the io_uring check
This sets CC to $(CROSS_COMPILE)gcc in the drivers/net/hw selftests
Makefile before the liburing feature probe runs, so that the probe uses the
target toolchain instead of the host compiler. The override is applied only
when make's CC is still the built-in default and LLVM is unset.
No Link: tag present in the commit header.
> diff --git a/tools/testing/selftests/drivers/net/hw/Makefile
> b/tools/testing/selftests/drivers/net/hw/Makefile
> index 6105be8e590f7..f9458dc6c1508 100644
> --- a/tools/testing/selftests/drivers/net/hw/Makefile
> +++ b/tools/testing/selftests/drivers/net/hw/Makefile
> @@ -1,5 +1,10 @@
> # SPDX-License-Identifier: GPL-2.0+ OR MIT
>
> +# Set CC for the io_uring check
> +ifeq ($(LLVM)$(CC),cc)
> +CC := $(CROSS_COMPILE)gcc
> +endif
[Severity: Medium]
Does this condition cover the case where CC comes from the environment
rather than from make's built-in default?
With something like:
export CC=gcc-13
make -C tools/testing/selftests/drivers/net/hw
CROSS_COMPILE=aarch64-linux-gnu-
$(LLVM)$(CC) expands to gcc-13, so the override is skipped and the probe
below runs the host gcc-13. Later, lib.mk does:
tools/testing/selftests/lib.mk:
ifneq ($(LLVM),)
CC := $(CLANG) $(CLANG_FLAGS) -fintegrated-as
else
CC := $(CROSS_COMPILE)gcc
endif # LLVM
Since that is a := assignment in a makefile, it overrides the
environment-origin CC, so the compiler that actually links iou-zcrx is not
the one that was probed.
Would an origin test such as:
ifeq ($(origin CC),default)
or mirroring lib.mk's own ifneq ($(LLVM),) ... else CC := $(CROSS_COMPILE)gcc
structure (as tools/testing/selftests/arm64/mte/Makefile does) make both
sides agree, and also close the LLVM case noted in the commit message?
[Severity: Low]
Should this carry a Fixes: tag?
git blame on the baseline attributes the whole probe block, including the
$(CC) -luring invocation and the COND_GEN_FILES += iou-zcrx gate, to:
f81171fecd0c ("selftests: hw-net: auto-disable building the iouring C code")
Before that commit there was no CC-dependent probe in this Makefile. Adding
the tag would let backporters pick this up into trees that already carry
f81171fecd0c.
>
> # Check if io_uring supports zero-copy receive
> HAS_IOURING_ZCRX := $(shell \
> echo -e '#include <liburing.h>\n' \
[Severity: Medium]
Related to the CC question above: the probe invokes only $(CC) -luring and
passes none of CFLAGS/CPPFLAGS/LDFLAGS/USERCFLAGS/USERLDFLAGS, while the
generic C rule in lib.mk does pass them when linking $(OUTPUT)/iou-zcrx.
Can a target liburing that is only reachable through user-supplied
-I/-L/sysroot flags stay invisible to this probe?
The two failure modes both come from the probe answering for a different
toolchain than the build:
ifeq ($(HAS_IOURING_ZCRX),y)
$(OUTPUT)/iou-zcrx: LDLIBS += -luring
endif
Host-only liburing gives y and the cross link of iou-zcrx then fails on
-luring, taking the whole drivers/net/hw build with it. Target-only liburing
gives an empty value and the test is dropped with the warning "excluding
iouring tests, liburing not installed or too old".
[ ... ]
--
Sashiko AI review ·
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907161438.755125-1-maxime.chevallier%40bootlin.com
