The configuration of the in-kernel RV monitor is relying on global variables, some static and some extern and the argument parsing occurs there. That's not scalable especially if the configuration needs to be shared by other monitor implementations (e.g. BPF).
Move the common argument parsing in utils and use a global struct for the configuration. Implementation-specific configuration such as the reactors are still handled in the in-kernel source. Signed-off-by: Gabriele Monaco <[email protected]> --- tools/verification/rv/include/utils.h | 17 ++- tools/verification/rv/src/in_kernel.c | 176 ++++++-------------------- tools/verification/rv/src/utils.c | 95 +++++++++++++- 3 files changed, 149 insertions(+), 139 deletions(-) diff --git a/tools/verification/rv/include/utils.h b/tools/verification/rv/include/utils.h index f24ae8282bd2..61f77a72a1a2 100644 --- a/tools/verification/rv/include/utils.h +++ b/tools/verification/rv/include/utils.h @@ -1,8 +1,23 @@ // SPDX-License-Identifier: GPL-2.0 +#include <stdbool.h> + #define MAX_PATH 1024 void debug_msg(const char *fmt, ...); void err_msg(const char *fmt, ...); +void mon_usage(int exit_val, char *monitor_name, const char *fmt, ...); +int parse_arguments(char *monitor_name, int argc, char **argv); + +void ikm_usage_print_reactors(void); -extern int config_debug; +struct config { + bool debug; + bool is_container; + bool trace; + int has_id; + int my_pid; + char *initial_reactor; + char *reactor; +}; +extern struct config config; diff --git a/tools/verification/rv/src/in_kernel.c b/tools/verification/rv/src/in_kernel.c index e6dea4040f8f..90c340a73c76 100644 --- a/tools/verification/rv/src/in_kernel.c +++ b/tools/verification/rv/src/in_kernel.c @@ -4,7 +4,6 @@ * * Copyright (C) 2022 Red Hat Inc, Daniel Bristot de Oliveira <[email protected]> */ -#include <getopt.h> #include <stdlib.h> #include <stdio.h> #include <string.h> @@ -16,14 +15,6 @@ #include <utils.h> #include <rv.h> -static int config_has_id; -static int config_is_container; -static int config_my_pid; -static int config_trace; - -static char *config_initial_reactor; -static char *config_reactor; - /* * __ikm_read_enable - reads monitor's enable status * @@ -66,7 +57,7 @@ static int __ikm_find_monitor_name(char *monitor_name, char *out_name) if (!available_monitors) return -1; - config_is_container = 0; + config.is_container = 0; cursor = available_monitors; while ((line = strsep(&cursor, "\n"))) { char *colon = strchr(line, ':'); @@ -83,7 +74,7 @@ static int __ikm_find_monitor_name(char *monitor_name, char *out_name) /* If there are children, they are on the next line. */ line = strsep(&cursor, "\n"); if (line && !strncmp(line, monitor_name, len) && line[len] == ':') - config_is_container = 1; + config.is_container = 1; } found = 1; @@ -391,7 +382,7 @@ int ikm_list_monitors(char *container) static void ikm_print_header(struct trace_seq *s) { trace_seq_printf(s, "%16s-%-8s %5s %5s ", "<TASK>", "PID", "[CPU]", "TYPE"); - if (config_has_id) + if (config.has_id) trace_seq_printf(s, "%8s ", "ID"); trace_seq_printf(s, "%24s x %-24s -> %-24s %s\n", @@ -402,7 +393,7 @@ static void ikm_print_header(struct trace_seq *s) trace_seq_printf(s, "%16s %-8s %5s %5s ", " | ", " | ", " | ", " | "); - if (config_has_id) + if (config.has_id) trace_seq_printf(s, "%8s ", " | "); trace_seq_printf(s, "%24s %-24s %-24s %s\n", @@ -431,25 +422,25 @@ ikm_event_handler(struct trace_seq *s, struct tep_record *record, int val; bool missing_id; - if (config_has_id) + if (config.has_id) missing_id = tep_get_field_val(s, trace_event, "id", record, &id, 1); tep_get_common_field_val(s, trace_event, "common_pid", record, &pid, 1); - if (config_has_id && (config_my_pid == id)) + if (config.has_id && (config.my_pid == id)) return 0; - else if (config_my_pid == pid) + else if (config.my_pid == pid) return 0; tep_print_event(trace_event->tep, s, record, "%16s-%-8d [%.3d] ", TEP_PRINT_COMM, TEP_PRINT_PID, TEP_PRINT_CPU); - if (config_is_container) + if (config.is_container) tep_print_event(trace_event->tep, s, record, "%s ", TEP_PRINT_NAME); else trace_seq_printf(s, "event "); - if (config_has_id) { + if (config.has_id) { if (missing_id) /* placeholder if we are dealing with a mixed-type container*/ trace_seq_printf(s, " "); @@ -490,24 +481,24 @@ ikm_error_handler(struct trace_seq *s, struct tep_record *record, int val; bool missing_id; - if (config_has_id) + if (config.has_id) missing_id = tep_get_field_val(s, trace_event, "id", record, &id, 1); tep_get_common_field_val(s, trace_event, "common_pid", record, &pid, 1); - if (config_has_id && config_my_pid == id) + if (config.has_id && config.my_pid == id) return 0; - else if (config_my_pid == pid) + else if (config.my_pid == pid) return 0; trace_seq_printf(s, "%8lld [%03d] ", pid, cpu); - if (config_is_container) + if (config.is_container) tep_print_event(trace_event->tep, s, record, "%s ", TEP_PRINT_NAME); else trace_seq_printf(s, "error "); - if (config_has_id) { + if (config.has_id) { if (missing_id) /* placeholder if we are dealing with a mixed-type container*/ trace_seq_printf(s, " "); @@ -548,8 +539,8 @@ static int ikm_enable_trace_events(char *monitor_name, struct trace_instance *in ikm_error_handler, NULL); /* set if at least 1 monitor has id in case of a container */ - config_has_id = ikm_has_id(monitor_name); - if (config_has_id < 0) + config.has_id = ikm_has_id(monitor_name); + if (config.has_id < 0) return -1; @@ -596,7 +587,7 @@ static struct trace_instance *ikm_setup_trace_instance(char *monitor_name) struct trace_instance *inst; int retval; - if (!config_trace) + if (!config.trace) return NULL; /* alloc data */ @@ -610,7 +601,7 @@ static struct trace_instance *ikm_setup_trace_instance(char *monitor_name) if (retval) goto out_free; - if (config_is_container) + if (config.is_container) retval = ikm_enable_trace_container(monitor_name, inst); else retval = ikm_enable_trace_events(monitor_name, inst); @@ -645,7 +636,7 @@ static void ikm_destroy_trace_instance(struct trace_instance *inst) /* * ikm_usage_print_reactors - print all available reactors, one per line. */ -static void ikm_usage_print_reactors(void) +void ikm_usage_print_reactors(void) { char *reactors = tracefs_instance_file_read(NULL, "rv/available_reactors", NULL); char *start, *end; @@ -669,106 +660,6 @@ static void ikm_usage_print_reactors(void) fprintf(stderr, "\n"); } -/* - * ikm_usage - print usage - */ -static void ikm_usage(int exit_val, char *monitor_name, const char *fmt, ...) -{ - - char message[1024]; - va_list ap; - int i; - - static const char *const usage[] = { - "", - " -h/--help: print this menu and the reactor list", - " -r/--reactor 'reactor': enables the 'reactor'", - " -s/--self: when tracing (-t), also trace rv command", - " -t/--trace: trace monitor's event", - " -v/--verbose: print debug messages", - "", - NULL, - }; - - va_start(ap, fmt); - vsnprintf(message, sizeof(message), fmt, ap); - va_end(ap); - - fprintf(stderr, " %s\n", message); - - fprintf(stderr, "\n usage: rv mon %s [-h] [-q] [-r reactor] [-s] [-v]", monitor_name); - - for (i = 0; usage[i]; i++) - fprintf(stderr, "%s\n", usage[i]); - - ikm_usage_print_reactors(); - exit(exit_val); -} - -/* - * parse_arguments - parse arguments and set config - */ -static int parse_arguments(char *monitor_name, int argc, char **argv) -{ - int c, retval; - - config_my_pid = getpid(); - - while (1) { - static struct option long_options[] = { - {"help", no_argument, 0, 'h'}, - {"reactor", required_argument, 0, 'r'}, - {"self", no_argument, 0, 's'}, - {"trace", no_argument, 0, 't'}, - {"verbose", no_argument, 0, 'v'}, - {0, 0, 0, 0} - }; - - /* getopt_long stores the option index here. */ - int option_index = 0; - - c = getopt_long(argc, argv, "hr:stv", long_options, &option_index); - - /* detect the end of the options. */ - if (c == -1) - break; - - switch (c) { - case 'h': - ikm_usage(0, monitor_name, "help:"); - break; - case 'r': - config_reactor = optarg; - break; - case 's': - config_my_pid = -1; - break; - case 't': - config_trace = 1; - break; - case 'v': - config_debug = 1; - break; - } - } - - if (config_reactor) { - config_initial_reactor = ikm_get_current_reactor(monitor_name); - if (!config_initial_reactor) - ikm_usage(1, monitor_name, - "ikm: failed to read current reactor, are reactors enabled?"); - - retval = ikm_write_reactor(monitor_name, config_reactor); - if (retval <= 0) - ikm_usage(1, monitor_name, - "ikm: failed to set %s reactor, is it available?", - config_reactor); - } - - debug_msg("ikm: my pid is %d\n", config_my_pid); - - return 0; -} /** * ikm_run_monitor - apply configs and run the monitor @@ -805,9 +696,22 @@ int ikm_run_monitor(char *monitor_name, int argc, char **argv) /* we should be good to go */ retval = parse_arguments(full_name, argc, argv); if (retval) - ikm_usage(1, nested_name, "ikm: failed parsing arguments"); + mon_usage(1, nested_name, "ikm: failed parsing arguments"); + + if (config.reactor) { + config.initial_reactor = ikm_get_current_reactor(full_name); + if (!config.initial_reactor) + mon_usage(1, full_name, + "ikm: failed to read current reactor, are reactors enabled?"); + + retval = ikm_write_reactor(full_name, config.reactor); + if (retval <= 0) + mon_usage(1, full_name, + "ikm: failed to set %s reactor, is it available?", + config.reactor); + } - if (config_trace) { + if (config.trace) { inst = ikm_setup_trace_instance(nested_name); if (!inst) goto out_free_instance; @@ -817,11 +721,11 @@ int ikm_run_monitor(char *monitor_name, int argc, char **argv) if (retval < 0) goto out_free_instance; - if (config_trace) + if (config.trace) ikm_print_header(inst->seq); while (!should_stop()) { - if (config_trace) { + if (config.trace) { retval = tracefs_iterate_raw_events(inst->tep, inst->inst, NULL, @@ -840,14 +744,14 @@ int ikm_run_monitor(char *monitor_name, int argc, char **argv) ikm_disable(full_name); ikm_destroy_trace_instance(inst); - if (config_reactor && config_initial_reactor) - ikm_write_reactor(full_name, config_initial_reactor); + if (config.reactor && config.initial_reactor) + ikm_write_reactor(full_name, config.initial_reactor); return 1; out_free_instance: ikm_destroy_trace_instance(inst); - if (config_reactor && config_initial_reactor) - ikm_write_reactor(full_name, config_initial_reactor); + if (config.reactor && config.initial_reactor) + ikm_write_reactor(full_name, config.initial_reactor); return -1; } diff --git a/tools/verification/rv/src/utils.c b/tools/verification/rv/src/utils.c index 5677b439dc2f..e19dd65ec789 100644 --- a/tools/verification/rv/src/utils.c +++ b/tools/verification/rv/src/utils.c @@ -5,11 +5,14 @@ * Copyright (C) 2022 Red Hat Inc, Daniel Bristot de Oliveira <[email protected]> */ +#include <getopt.h> #include <stdarg.h> +#include <stdlib.h> #include <stdio.h> +#include <unistd.h> #include <utils.h> -int config_debug; +struct config config; #define MAX_MSG_LENGTH 1024 @@ -36,7 +39,7 @@ void debug_msg(const char *fmt, ...) char message[MAX_MSG_LENGTH]; va_list ap; - if (!config_debug) + if (!config.debug) return; va_start(ap, fmt); @@ -45,3 +48,91 @@ void debug_msg(const char *fmt, ...) fprintf(stderr, "%s", message); } + +/* + * mon_usage - print usage + */ +void mon_usage(int exit_val, char *monitor_name, const char *fmt, ...) +{ + + char message[1024]; + va_list ap; + int i; + + static const char *const usage[] = { + "", + " -h/--help: print this menu and the reactor list", + " -r/--reactor 'reactor': enables the 'reactor'", + " -s/--self: when tracing (-t), also trace rv command", + " -t/--trace: trace monitor's event", + " -v/--verbose: print debug messages", + "", + NULL, + }; + + va_start(ap, fmt); + vsnprintf(message, sizeof(message), fmt, ap); + va_end(ap); + + fprintf(stderr, " %s\n", message); + + fprintf(stderr, "\n usage: rv mon %s [-h] [-q] [-r reactor] [-s] [-v]", monitor_name); + + for (i = 0; usage[i]; i++) + fprintf(stderr, "%s\n", usage[i]); + + ikm_usage_print_reactors(); + exit(exit_val); +} + +/* + * parse_arguments - parse arguments and set config + */ +int parse_arguments(char *monitor_name, int argc, char **argv) +{ + int c; + + config.my_pid = getpid(); + + while (1) { + static struct option long_options[] = { + {"help", no_argument, 0, 'h'}, + {"reactor", required_argument, 0, 'r'}, + {"self", no_argument, 0, 's'}, + {"trace", no_argument, 0, 't'}, + {"verbose", no_argument, 0, 'v'}, + {0, 0, 0, 0} + }; + + /* getopt_long stores the option index here. */ + int option_index = 0; + + c = getopt_long(argc, argv, "hr:stv", long_options, &option_index); + + /* detect the end of the options. */ + if (c == -1) + break; + + switch (c) { + case 'h': + mon_usage(0, monitor_name, "help:"); + break; + case 'r': + config.reactor = optarg; + break; + case 's': + config.my_pid = -1; + break; + case 't': + config.trace = 1; + break; + case 'v': + config.debug = 1; + break; + } + } + + debug_msg("ikm: my pid is %d\n", config.my_pid); + + return 0; +} -- 2.55.0
