Date:   Sat, 15 Dec 2018 00:56:29 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Quentin Monnet <quentin.monnet@...ronome.com>,
        Alexei Starovoitov <ast@...nel.org>
Cc:     netdev@...r.kernel.org, oss-drivers@...ronome.com,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Stanislav Fomichev <sdf@...gle.com>
Subject: Re: [PATCH bpf-next 3/8] tools: bpftool: add probes for kernel
 configuration options

On 12/13/2018 01:19 PM, Quentin Monnet wrote:
> Add probes to dump a number of options set (or not set) for compiling
> the kernel image. These parameters provide information about what BPF
> components should be available on the system. A number of them are not
> directly related to eBPF, but are in fact used in the kernel as
> conditions on which to compile, or not to compile, some of the eBPF
> helper functions.
> 
> Sample output:
> 
>     # bpftool feature probe kernel
>     Scanning system configuration...
>     ...
>     CONFIG_BPF is set to y
>     CONFIG_BPF_SYSCALL is set to y
>     CONFIG_HAVE_EBPF_JIT is set to y
>     ...
> 
>     # bpftool --pretty --json feature probe kernel
>     {
>         "system_config": {
>             ...
>             "CONFIG_BPF": "y",
>             "CONFIG_BPF_SYSCALL": "y",
>             "CONFIG_HAVE_EBPF_JIT": "y",
>             ...
>         }
>     }
> 
>     # bpftool feature probe kernel macros prefix BPFTOOL_
>     /*** System configuration ***/
>     ...
>     #define BPFTOOL_CONFIG_BPF y
>     #define BPFTOOL_CONFIG_BPF_SYSCALL y
>     #define BPFTOOL_CONFIG_HAVE_EBPF_JIT y
>     ...

Looks reasonable. I think as a user next question that would
follow-up from it would be whether this set of config means
that e.g. requirements for XDP, cgroups bpf, tracing or xyz is
fulfilled. Perhaps it makes sense to split the options[] into
base_options[], bpf_trace_options[], bpf_tc_options[] etc such
that it might become obvious that base_options[] + bpf_tc_options[]
are supported and thus cls_bpf could be used. I'd see this part
here in general more as giving a hint to the user in that some
basic assumptions could be made and providing some info on the
misc ones on what might potentially be missing. Though more
concrete info would come from the actual helper / map / prog
type probing.

> Signed-off-by: Quentin Monnet <quentin.monnet@...ronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
> ---
>  tools/bpf/bpftool/feature.c | 144 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 144 insertions(+)
> 
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index 9fa7016c7d21..4a3a45f44162 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -52,6 +52,37 @@ print_bool_feature(const char *feat_name, const char *define_name,
>  		printf("%s is %savailable\n", plain_name, res ? "" : "NOT ");
>  }
>  
> +static void
> +print_kernel_option(const char *name, const char *value,
> +		    const char *define_prefix)
> +{
> +	char *endptr;
> +	int res;
> +
> +	if (json_output) {
> +		if (!value) {
> +			jsonw_null_field(json_wtr, name);
> +			return;
> +		}
> +		errno = 0;
> +		res = strtol(value, &endptr, 0);
> +		if (!errno && *endptr == '\n')
> +			jsonw_int_field(json_wtr, name, res);
> +		else
> +			jsonw_string_field(json_wtr, name, value);
> +	} else if (define_prefix) {
> +		if (value)
> +			printf("#define %s%s %s\n", define_prefix, name, value);
> +		else
> +			printf("#define %sNO_%s\n", define_prefix, name);
> +	} else {
> +		if (value)
> +			printf("%s is set to %s\n", name, value);
> +		else
> +			printf("%s is not set\n", name);
> +	}
> +}
> +
>  static void
>  print_start_section(const char *json_title, const char *define_comment,
>  		    const char *plain_title, const char *define_prefix)
> @@ -298,6 +329,118 @@ static void probe_jit_kallsyms(const char *define_prefix)
>  	}
>  }
>  
> +static char *get_kernel_config_option(FILE *fd, const char *option)
> +{
> +	size_t line_n = 0, optlen = strlen(option);
> +	char *res, *strval, *line = NULL;
> +	ssize_t n;
> +
> +	rewind(fd);
> +	while ((n = getline(&line, &line_n, fd)) > 0) {
> +		if (strncmp(line, option, optlen))
> +			continue;
> +		/* Check we have at least '=', value, and '\n' */
> +		if (strlen(line) < optlen + 3)
> +			continue;
> +		if (*(line + optlen) != '=')
> +			continue;
> +
> +		/* Trim ending '\n' */
> +		line[strlen(line) - 1] = '\0';
> +
> +		/* Copy and return config option value */
> +		strval = line + optlen + 1;
> +		res = strdup(strval);
> +		free(line);
> +		return res;
> +	}
> +	free(line);
> +
> +	return NULL;
> +}
> +
> +static void probe_kernel_image_config(const char *define_prefix)
> +{
> +	const char * const options[] = {
> +		"CONFIG_BPF",
> +		"CONFIG_BPF_SYSCALL",
> +		"CONFIG_HAVE_EBPF_JIT",
> +		"CONFIG_BPF_JIT",
> +		"CONFIG_BPF_JIT_ALWAYS_ON",
> +		"CONFIG_NET",
> +		"CONFIG_XDP_SOCKETS",
> +		"CONFIG_CGROUPS",
> +		"CONFIG_CGROUP_BPF",
> +		"CONFIG_CGROUP_NET_CLASSID",
> +		"CONFIG_BPF_EVENTS",
> +		"CONFIG_LWTUNNEL_BPF",
> +		"CONFIG_NET_ACT_BPF",
> +		"CONFIG_NET_CLS_ACT",
> +		"CONFIG_NET_CLS_BPF",
> +		"CONFIG_NET_SCH_INGRESS",
> +		"CONFIG_XFRM",
> +		"CONFIG_SOCK_CGROUP_DATA",
> +		"CONFIG_IP_ROUTE_CLASSID",
> +		"CONFIG_IPV6_SEG6_BPF",
> +		"CONFIG_FUNCTION_ERROR_INJECTION",
> +		"CONFIG_BPF_KPROBE_OVERRIDE",
> +		"CONFIG_BPF_LIRC_MODE2",
> +		"CONFIG_NETFILTER_XT_MATCH_BPF",
> +		"CONFIG_TEST_BPF",
> +		"CONFIG_BPFILTER",
> +		"CONFIG_BPFILTER_UMH",
> +		"CONFIG_BPF_STREAM_PARSER",
> +	};
> +	char *value, *buf = NULL;
> +	struct utsname utsn;
> +	char path[PATH_MAX];
> +	size_t i, n;
> +	ssize_t ret;
> +	FILE *fd;
> +
> +	if (uname(&utsn))
> +		goto no_config;
> +
> +	snprintf(path, sizeof(path), "/boot/config-%s", utsn.release);
> +
> +	fd = fopen(path, "r");
> +	if (!fd && errno == ENOENT) {
> +		/* Sometimes config is at /proc/config */
> +		fd = fopen("/proc/config", "r");
> +	}
> +	if (!fd) {
> +		p_err("can't open kernel config file: %s", strerror(errno));
> +		goto no_config;
> +	}
> +	/* Sanity checks */
> +	ret = getline(&buf, &n, fd);
> +	ret = getline(&buf, &n, fd);
> +	if (!buf || !ret) {
> +		p_err("can't read from kernel config file: %s",
> +		      strerror(errno));
> +		free(buf);
> +		goto no_config;
> +	}
> +	if (strcmp(buf, "# Automatically generated file; DO NOT EDIT.\n")) {
> +		p_err("can't find correct kernel config file");
> +		free(buf);
> +		goto no_config;
> +	}
> +	free(buf);
> +
> +	for (i = 0; i < ARRAY_SIZE(options); i++) {
> +		value = get_kernel_config_option(fd, options[i]);
> +		print_kernel_option(options[i], value, define_prefix);
> +		free(value);
> +	}
> +	fclose(fd);
> +	return;
> +
> +no_config:
> +	for (i = 0; i < ARRAY_SIZE(options); i++)
> +		print_kernel_option(options[i], NULL, define_prefix);
> +}
> +
>  static int probe_kernel_version(const char *define_prefix)
>  {
>  	int version, subversion, patchlevel, code = 0;
> @@ -402,6 +545,7 @@ static int do_probe(int argc, char **argv)
>  		} else {
>  			p_info("/* procfs not mounted, skipping related probes */");
>  		}
> +		probe_kernel_image_config(define_prefix);
>  		if (json_output)
>  			jsonw_end_object(json_wtr);
>  		else
> 

Powered by blists - more mailing lists