selftest/bpf/benchs: Enhance argp parsing

To parse command line the bench utility uses the argp_parse() function. This
function takes as an argument a parent 'struct argp' structure which defines
common command line options and an array of children 'struct argp' structures
which defines additional command line options for particular benchmarks. This
implementation doesn't allow benchmarks to share option names, e.g., if two
benchmarks want to use, say, the --option option, then only one of them will
succeed (the first one encountered in the array).  This will be convenient if
same option names could be used in different benchmarks (with the same
semantics, e.g., --nr_loops=N).

Fix this by calling the argp_parse() function twice. The first call is the same
as it was before, with all children argps, and helps to find the benchmark name
and to print a combined help message if anything is wrong.  Given the name, we
can call the argp_parse the second time, but now the children array points only
to a correct benchmark thus always calling the correct parsers. (If there's no
a specific list of arguments, then only one call to argp_parse will be done.)

Signed-off-by: Anton Protopopov <aspsk@isovalent.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/20230213091519.1202813-4-aspsk@isovalent.com

diff --git a/tools/testing/selftests/bpf/bench.c b/tools/testing/selftests/bpf/bench.c
index c1f20a1474624131a9f23d8903f2ed6e240149ee..12c3b3ab84aab2c7f06749c4dc1658fa55a8ff30 100644 (file)
--- a/tools/testing/selftests/bpf/bench.c
+++ b/tools/testing/selftests/bpf/bench.c
@@ -287,10 +287,11 @@ static const struct argp_child bench_parsers[] = {
        {},
 };
 
+/* Make pos_args global, so that we can run argp_parse twice, if necessary */
+static int pos_args;
+
 static error_t parse_arg(int key, char *arg, struct argp_state *state)
 {
-       static int pos_args;
-
        switch (key) {
        case 'v':
                env.verbose = true;
@@ -359,7 +360,7 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
        return 0;
 }
 
-static void parse_cmdline_args(int argc, char **argv)
+static void parse_cmdline_args_init(int argc, char **argv)
 {
        static const struct argp argp = {
                .options = opts,
@@ -369,9 +370,25 @@ static void parse_cmdline_args(int argc, char **argv)
        };
        if (argp_parse(&argp, argc, argv, 0, NULL, NULL))
                exit(1);
-       if (!env.list && !env.bench_name) {
-               argp_help(&argp, stderr, ARGP_HELP_DOC, "bench");
-               exit(1);
+}
+
+static void parse_cmdline_args_final(int argc, char **argv)
+{
+       struct argp_child bench_parsers[2] = {};
+       const struct argp argp = {
+               .options = opts,
+               .parser = parse_arg,
+               .doc = argp_program_doc,
+               .children = bench_parsers,
+       };
+
+       /* Parse arguments the second time with the correct set of parsers */
+       if (bench->argp) {
+               bench_parsers[0].argp = bench->argp;
+               bench_parsers[0].header = bench->name;
+               pos_args = 0;
+               if (argp_parse(&argp, argc, argv, 0, NULL, NULL))
+                       exit(1);
        }
 }
 
@@ -531,15 +548,14 @@ static const struct bench *benchs[] = {
        &bench_local_storage_tasks_trace,
 };
 
-static void setup_benchmark()
+static void find_benchmark(void)
 {
-       int i, err;
+       int i;
 
        if (!env.bench_name) {
                fprintf(stderr, "benchmark name is not specified\n");
                exit(1);
        }
-
        for (i = 0; i < ARRAY_SIZE(benchs); i++) {
                if (strcmp(benchs[i]->name, env.bench_name) == 0) {
                        bench = benchs[i];
@@ -550,6 +566,11 @@ static void setup_benchmark()
                fprintf(stderr, "benchmark '%s' not found\n", env.bench_name);
                exit(1);
        }
+}
+
+static void setup_benchmark(void)
+{
+       int i, err;
 
        printf("Setting up benchmark '%s'...\n", bench->name);
 
@@ -621,7 +642,7 @@ static void collect_measurements(long delta_ns) {
 
 int main(int argc, char **argv)
 {
-       parse_cmdline_args(argc, argv);
+       parse_cmdline_args_init(argc, argv);
 
        if (env.list) {
                int i;
@@ -633,6 +654,9 @@ int main(int argc, char **argv)
                return 0;
        }
 
+       find_benchmark();
+       parse_cmdline_args_final(argc, argv);
+
        setup_benchmark();
 
        setup_timer();

diff --git a/tools/testing/selftests/bpf/bench.h b/tools/testing/selftests/bpf/bench.h
index d748255877e2d24852074d24d21ebb779a68a576..3c8afa0131a3aabe184835448dde367275f634a8 100644 (file)
--- a/tools/testing/selftests/bpf/bench.h
+++ b/tools/testing/selftests/bpf/bench.h
@@ -47,6 +47,7 @@ struct bench_res {
 
 struct bench {
        const char *name;
+       const struct argp *argp;
        void (*validate)(void);
        void (*setup)(void);
        void *(*producer_thread)(void *ctx);

diff --git a/tools/testing/selftests/bpf/benchs/bench_bloom_filter_map.c b/tools/testing/selftests/bpf/benchs/bench_bloom_filter_map.c
index 5bcb8a8cdeb20479aeb6b4a0c4d4ac4d5b0beaf5..7c8ccc1083138f9d30283bae1d8f9a48823e1ba1 100644 (file)
--- a/tools/testing/selftests/bpf/benchs/bench_bloom_filter_map.c
+++ b/tools/testing/selftests/bpf/benchs/bench_bloom_filter_map.c
@@ -428,6 +428,7 @@ static void *consumer(void *input)
 
 const struct bench bench_bloom_lookup = {
        .name = "bloom-lookup",
+       .argp = &bench_bloom_map_argp,
        .validate = validate,
        .setup = bloom_lookup_setup,
        .producer_thread = producer,
@@ -439,6 +440,7 @@ const struct bench bench_bloom_lookup = {
 
 const struct bench bench_bloom_update = {
        .name = "bloom-update",
+       .argp = &bench_bloom_map_argp,
        .validate = validate,
        .setup = bloom_update_setup,
        .producer_thread = producer,
@@ -450,6 +452,7 @@ const struct bench bench_bloom_update = {
 
 const struct bench bench_bloom_false_positive = {
        .name = "bloom-false-positive",
+       .argp = &bench_bloom_map_argp,
        .validate = validate,
        .setup = false_positive_setup,
        .producer_thread = producer,
@@ -461,6 +464,7 @@ const struct bench bench_bloom_false_positive = {
 
 const struct bench bench_hashmap_without_bloom = {
        .name = "hashmap-without-bloom",
+       .argp = &bench_bloom_map_argp,
        .validate = validate,
        .setup = hashmap_no_bloom_setup,
        .producer_thread = producer,
@@ -472,6 +476,7 @@ const struct bench bench_hashmap_without_bloom = {
 
 const struct bench bench_hashmap_with_bloom = {
        .name = "hashmap-with-bloom",
+       .argp = &bench_bloom_map_argp,
        .validate = validate,
        .setup = hashmap_with_bloom_setup,
        .producer_thread = producer,

diff --git a/tools/testing/selftests/bpf/benchs/bench_bpf_loop.c b/tools/testing/selftests/bpf/benchs/bench_bpf_loop.c
index d0a6572bfab61d6fce755645f4634ac2ad3a875e..d8a0394e10b15fecc351ac247ee5f22445701b86 100644 (file)
--- a/tools/testing/selftests/bpf/benchs/bench_bpf_loop.c
+++ b/tools/testing/selftests/bpf/benchs/bench_bpf_loop.c
@@ -95,6 +95,7 @@ static void setup(void)
 
 const struct bench bench_bpf_loop = {
        .name = "bpf-loop",
+       .argp = &bench_bpf_loop_argp,
        .validate = validate,
        .setup = setup,
        .producer_thread = producer,

diff --git a/tools/testing/selftests/bpf/benchs/bench_local_storage.c b/tools/testing/selftests/bpf/benchs/bench_local_storage.c
index 5a378c84e81f645fad5e5220181c120efa290f1f..d4b2817306d42ab70cb6662edac74a7a81659d6b 100644 (file)
--- a/tools/testing/selftests/bpf/benchs/bench_local_storage.c
+++ b/tools/testing/selftests/bpf/benchs/bench_local_storage.c
@@ -255,6 +255,7 @@ static void *producer(void *input)
  */
 const struct bench bench_local_storage_cache_seq_get = {
        .name = "local-storage-cache-seq-get",
+       .argp = &bench_local_storage_argp,
        .validate = validate,
        .setup = local_storage_cache_get_setup,
        .producer_thread = producer,
@@ -266,6 +267,7 @@ const struct bench bench_local_storage_cache_seq_get = {
 
 const struct bench bench_local_storage_cache_interleaved_get = {
        .name = "local-storage-cache-int-get",
+       .argp = &bench_local_storage_argp,
        .validate = validate,
        .setup = local_storage_cache_get_interleaved_setup,
        .producer_thread = producer,
@@ -277,6 +279,7 @@ const struct bench bench_local_storage_cache_interleaved_get = {
 
 const struct bench bench_local_storage_cache_hashmap_control = {
        .name = "local-storage-cache-hashmap-control",
+       .argp = &bench_local_storage_argp,
        .validate = validate,
        .setup = hashmap_setup,
        .producer_thread = producer,

diff --git a/tools/testing/selftests/bpf/benchs/bench_local_storage_rcu_tasks_trace.c b/tools/testing/selftests/bpf/benchs/bench_local_storage_rcu_tasks_trace.c
index 43f109d9313029c4e68afc166fee1f075f7739e9..4f9401ecf09ce725fd4407373e6041e344717a01 100644 (file)
--- a/tools/testing/selftests/bpf/benchs/bench_local_storage_rcu_tasks_trace.c
+++ b/tools/testing/selftests/bpf/benchs/bench_local_storage_rcu_tasks_trace.c
@@ -271,6 +271,7 @@ static void report_final(struct bench_res res[], int res_cnt)
  */
 const struct bench bench_local_storage_tasks_trace = {
        .name = "local-storage-tasks-trace",
+       .argp = &bench_local_storage_rcu_tasks_trace_argp,
        .validate = validate,
        .setup = local_storage_tasks_trace_setup,
        .producer_thread = producer,

diff --git a/tools/testing/selftests/bpf/benchs/bench_ringbufs.c b/tools/testing/selftests/bpf/benchs/bench_ringbufs.c
index c2554f9695ffbbc9560e7ce3882394062ab1b27c..fc91fdac4faa5eb8bcfdfef61aff346804641c09 100644 (file)
--- a/tools/testing/selftests/bpf/benchs/bench_ringbufs.c
+++ b/tools/testing/selftests/bpf/benchs/bench_ringbufs.c
@@ -518,6 +518,7 @@ static void *perfbuf_custom_consumer(void *input)
 
 const struct bench bench_rb_libbpf = {
        .name = "rb-libbpf",
+       .argp = &bench_ringbufs_argp,
        .validate = bufs_validate,
        .setup = ringbuf_libbpf_setup,
        .producer_thread = bufs_sample_producer,
@@ -529,6 +530,7 @@ const struct bench bench_rb_libbpf = {
 
 const struct bench bench_rb_custom = {
        .name = "rb-custom",
+       .argp = &bench_ringbufs_argp,
        .validate = bufs_validate,
        .setup = ringbuf_custom_setup,
        .producer_thread = bufs_sample_producer,
@@ -540,6 +542,7 @@ const struct bench bench_rb_custom = {
 
 const struct bench bench_pb_libbpf = {
        .name = "pb-libbpf",
+       .argp = &bench_ringbufs_argp,
        .validate = bufs_validate,
        .setup = perfbuf_libbpf_setup,
        .producer_thread = bufs_sample_producer,
@@ -551,6 +554,7 @@ const struct bench bench_pb_libbpf = {
 
 const struct bench bench_pb_custom = {
        .name = "pb-custom",
+       .argp = &bench_ringbufs_argp,
        .validate = bufs_validate,
        .setup = perfbuf_libbpf_setup,
        .producer_thread = bufs_sample_producer,

diff --git a/tools/testing/selftests/bpf/benchs/bench_strncmp.c b/tools/testing/selftests/bpf/benchs/bench_strncmp.c
index 494b591c028970b1247921d3ae003cced1930811..d3fad2ba6916fd91f079fa768121604dfe644fba 100644 (file)
--- a/tools/testing/selftests/bpf/benchs/bench_strncmp.c
+++ b/tools/testing/selftests/bpf/benchs/bench_strncmp.c
@@ -140,6 +140,7 @@ static void strncmp_measure(struct bench_res *res)
 
 const struct bench bench_strncmp_no_helper = {
        .name = "strncmp-no-helper",
+       .argp = &bench_strncmp_argp,
        .validate = strncmp_validate,
        .setup = strncmp_no_helper_setup,
        .producer_thread = strncmp_producer,
@@ -151,6 +152,7 @@ const struct bench bench_strncmp_no_helper = {
 
 const struct bench bench_strncmp_helper = {
        .name = "strncmp-helper",
+       .argp = &bench_strncmp_argp,
        .validate = strncmp_validate,
        .setup = strncmp_helper_setup,
        .producer_thread = strncmp_producer,