[PATCH bpf-next v1] bpf/selftests: improve arg parsing in test_verifier

* [PATCH bpf-next v1] bpf/selftests: improve arg parsing in test_verifier
@ 2023-09-25 23:37 Tony Ambardar
  2023-09-26 12:40 ` Jiri Olsa
  2023-09-26 12:54 ` Jiri Olsa
  0 siblings, 2 replies; 3+ messages in thread
From: Tony Ambardar @ 2023-09-25 23:37 UTC (permalink / raw)
  To: bpf, linux-kselftest
  Cc: Tony Ambardar, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Shuah Khan

Current test_verifier provides little feedback or argument validation,
instead silently falling back to running all tests in case of user error
or even expected use cases. Trying to do manual exploratory testing,
switching between kernel versions (e.g. with varying tests), or working
around problematic tests (e.g. kernel hangs/crashes) can be a frustrating
experience.

Rework argument parsing to be more robust and strict, and provide basic
help on errors. Clamp test ranges to valid values and add an option to
list available built-in tests ("-l"). Default "test_verifier" behaviour
(run all tests) is unchanged and backwards-compatible. Updated examples:

     $ test_verifier die die die     # previously ran all tests
     Usage: test_verifier -l | [-v|-vv] [<tst_lo> [<tst_hi>]]

     $ test_verifier 700 9999        # runs test subset from 700 to end

Signed-off-by: Tony Ambardar <Tony.Ambardar@gmail.com>
---
 tools/testing/selftests/bpf/test_verifier.c | 54 ++++++++++++---------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 98107e0452d3..3712b5363f60 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -10,9 +10,11 @@
 #include <endian.h>
 #include <asm/types.h>
 #include <linux/types.h>
+#include <linux/minmax.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <ctype.h>
 #include <unistd.h>
 #include <errno.h>
 #include <string.h>
@@ -1848,36 +1850,40 @@ int main(int argc, char **argv)
 {
 	unsigned int from = 0, to = ARRAY_SIZE(tests);
 	bool unpriv = !is_admin();
-	int arg = 1;
-
-	if (argc > 1 && strcmp(argv[1], "-v") == 0) {
+	int i, arg = 1;
+
+	while (argc > 1 && *argv[arg] == '-') {
+		if (strcmp(argv[arg], "-l") == 0) {
+			for (i = from; i < to; i++)
+				printf("#%d %s\n", i, tests[i].descr);
+			return EXIT_SUCCESS;
+		} else if (strcmp(argv[arg], "-v") == 0) {
+			verbose = true;
+			verif_log_level = 1;
+		} else if (strcmp(argv[arg], "-vv") == 0) {
+			verbose = true;
+			verif_log_level = 2;
+		} else
+			goto out_help;
 		arg++;
-		verbose = true;
-		verif_log_level = 1;
 		argc--;
 	}
-	if (argc > 1 && strcmp(argv[1], "-vv") == 0) {
-		arg++;
-		verbose = true;
-		verif_log_level = 2;
-		argc--;
-	}
-
-	if (argc == 3) {
-		unsigned int l = atoi(argv[arg]);
-		unsigned int u = atoi(argv[arg + 1]);
 
-		if (l < to && u < to) {
-			from = l;
-			to   = u + 1;
-		}
-	} else if (argc == 2) {
-		unsigned int t = atoi(argv[arg]);
+	for (i = 1; i <= 2 && argc > 1; i++, arg++, argc--) {
+		unsigned int t = min(atoi(argv[arg]), ARRAY_SIZE(tests) - 1);
 
-		if (t < to) {
+		if (!isdigit(*argv[arg]))
+			goto out_help;
+		if (i == 1)
 			from = t;
-			to   = t + 1;
-		}
+		to = t + 1;
+	}
+
+	if (argc > 1) {
+out_help:
+		printf("Usage: %s -l | [-v|-vv] [<tst_lo> [<tst_hi>]]\n",
+		       argv[0]);
+		return EXIT_FAILURE;
 	}
 
 	unpriv_disabled = get_unpriv_disabled();
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread