Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 1 · Low: 0
- [Medium] Incomplete fix:
`tools/testing/selftests/drivers/net/netdevsim/Makefile` is the only…
--
selftests/drivers/net: include lib/sh/*.sh
This adds $(wildcard ../../net/lib/sh/*.sh) to TEST_INCLUDES in the
drivers/net selftest Makefiles so that net/lib/sh/defer.sh gets installed
by "make install", and sorts the TEST_INCLUDES lists while at it.
> diff --git a/tools/testing/selftests/drivers/net/Makefile
> b/tools/testing/selftests/drivers/net/Makefile
> index d5bf4cb638a8c..ab39c2e508f0a 100644
> --- a/tools/testing/selftests/drivers/net/Makefile
> +++ b/tools/testing/selftests/drivers/net/Makefile
> @@ -3,6 +3,7 @@ CFLAGS += $(KHDR_INCLUDES)
>
> TEST_INCLUDES := $(wildcard lib/py/*.py) \
> $(wildcard lib/sh/*.sh) \
> + $(wildcard ../../net/lib/sh/*.sh) \
> ../../net/lib.sh \
>
> TEST_GEN_FILES := \
[Severity: Medium]
Was drivers/net/netdevsim intentionally left out? It is the only Makefile
under tools/testing/selftests/drivers/net/ that this patch does not touch,
and it defines no TEST_INCLUDES at all:
tools/testing/selftests/drivers/net/netdevsim/Makefile:
TEST_PROGS := \
devlink.sh \
...
peer.sh \
...
# end of TEST_PROGS
TEST_FILES := \
ethtool-common.sh
# end of TEST_FILES
include ../../../lib.mk
Its installed scripts do consume the net shell libraries. peer.sh does:
lib_dir=$(dirname $0)/../../../net
source $lib_dir/lib.sh
and devlink.sh, devlink_in_netns.sh, devlink_trap.sh, fib.sh,
fib_notifications.sh, hw_stats_l3.sh, nexthop.sh and psample.sh do:
lib_dir=$(dirname $0)/../../../net/forwarding
source $lib_dir/lib.sh
which reaches net/lib.sh, and that unconditionally sources defer.sh:
tools/testing/selftests/net/lib.sh:
net_dir=$(dirname "$(readlink -e "${BASH_SOURCE[0]}")")
source "$net_dir/lib/sh/defer.sh"
There is also no automatic fallback for this target, since the top-level
Makefile only matches the exact strings net, drivers/net and
drivers/net/hw:
tools/testing/selftests/Makefile:
# Networking tests want the net/lib target, include it automatically
ifneq ($(filter net drivers/net drivers/net/hw,$(TARGETS)),)
ifeq ($(filter net/lib,$(TARGETS)),)
INSTALL_DEP_TARGETS := net/lib
endif
endif
So does the failure quoted in the changelog still reproduce with:
make INSTALL_PATH=/tmp/kself TARGETS=drivers/net/netdevsim \
-C tools/testing/selftests install
The commit message says the patch "adds a wildcard to include all shell
files for drivers/net tests that consume net lib.sh" — should that claim be
narrowed, or netdevsim covered too?
Note the gap in netdevsim is wider than defer.sh: adding only
$(wildcard ../../../net/lib/sh/*.sh) would not be enough, since
../../../net/lib.sh and ../../../net/forwarding/lib.sh are not installed
either (plus devlink_lib.sh, fib_offload_lib.sh and tc_common.sh, which
several of those scripts source).
For reference, drivers/net/dsa is also not in the default TARGETS list and
this patch does cover it, and drivers/net/{mlxsw,ocelot,microchip} have no
Makefile so they are not installable targets at all.
--
Sashiko AI review ·
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907-selftest_lib_defer-v1-1-8af94645aaa3%40kylinos.cn
