netdev - Re: [PATCH net-next RFC] selftests/net: integrate packetdrill with ksft

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240902094612.6d40a914@kernel.org>
Date: Mon, 2 Sep 2024 09:46:12 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
 davem@...emloft.net, edumazet@...gle.com, ncardwell@...gle.com,
 shuah@...nel.org, linux-kselftest@...r.kernel.org, fw@...len.de, Willem de
 Bruijn <willemb@...gle.com>, "Matthieu Baerts (NGI0)" <matttbe@...nel.org>,
 martineau@...nel.org
Subject: Re: [PATCH net-next RFC] selftests/net: integrate packetdrill with
 ksft

On Sun, 01 Sep 2024 17:15:38 -0400 Willem de Bruijn wrote:
> Changing kselftests to preserve directories turns out to be trivial.
> Patch inline below.
> 
> But, existing TARGETS of course then start failing. Because they
> depend on existing rsync without -R. In (at least) two ways:
> 
> amd-pstate fails because its TEST_FILES has includes from other
> directories and it expects those files to land in the directory
> with tests.
> 
> x86 prefixes all its output with $(OUTPUT) to form absolute paths,
> which also creates absolute paths in kselftest-list.txt.
> 
> These two are examples, not necessarily the one instances of those
> patterns. So switching to preserving directories for existing targets
> like TEST_FILES seems intractable.

I wonder how many of the targets actually need this behavior (it's
intentionally useful to them) vs they grew to depend on it accidentally.

> Plan B is to add a new TEST_PROGS_RECURSE, analogous to how
> TEST_INCLUDES extended TEST_FILES with optional path preservation.
> That is not much more complex.

Alternative would be to allow opt-in (diff at the end), I'm personally
biased against yet another TEST_* group because it's hard enough already
to make sense of the existing ones. Maybe it's better for the test to
"declare":

PRESERVE_TEST_DIRS


---
 tools/testing/selftests/lib.mk | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index d6edcfcb5be8..7be9bd583642 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -51,6 +51,12 @@ else
 CC := $(CROSS_COMPILE)gcc
 endif # LLVM
 
+ifeq ($(PRESERVE_TEST_DIRS),)
+RSYNC_INSTALL_FLAGS=-a --copy-unsafe-links
+else
+RSYNC_INSTALL_FLAGS=-aR --copy-unsafe-links
+endif
+
 ifeq (0,$(MAKELEVEL))
     ifeq ($(OUTPUT),)
 	OUTPUT := $(shell pwd)
@@ -150,12 +156,12 @@ endif
 
 define INSTALL_SINGLE_RULE
 	$(if $(INSTALL_LIST),@mkdir -p $(INSTALL_PATH))
-	$(if $(INSTALL_LIST),rsync -a --copy-unsafe-links $(INSTALL_LIST) $(INSTALL_PATH)/)
+	$(if $(INSTALL_LIST),rsync $(RSYNC_INSTALL_FLAGS) $(INSTALL_LIST) $(INSTALL_PATH)/)
 endef
 
 define INSTALL_MODS_RULE
 	$(if $(INSTALL_LIST),@mkdir -p $(INSTALL_PATH)/$(INSTALL_LIST))
-	$(if $(INSTALL_LIST),rsync -a --copy-unsafe-links $(INSTALL_LIST)/*.ko $(INSTALL_PATH)/$(INSTALL_LIST))
+	$(if $(INSTALL_LIST),rsync $(RSYNC_INSTALL_FLAGS) $(INSTALL_LIST)/*.ko $(INSTALL_PATH)/$(INSTALL_LIST))
 endef
 
 define INSTALL_RULE
@@ -179,10 +185,17 @@ else
 endif
 
 emit_tests:
+ifeq ($(PRESERVE_TEST_DIRS),)
 	for TEST in $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS); do \
 		BASENAME_TEST=`basename $$TEST`;	\
 		echo "$(COLLECTION):$$BASENAME_TEST";	\
 	done
+else
+	for TEST in $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS); do \
+		BASENAME_TEST=$$TEST;	\
+		echo "$(COLLECTION):$$TEST";	\
+	done
+endif
 
 # define if isn't already. It is undefined in make O= case.
 ifeq ($(RM),)
-- 
2.46.0