Re: [PATCH net] selftests/drivers/net: include lib/sh/*.sh

[netdev-bot+sashiko@kernel.org]

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