Re: [RFC PATCH bpf-next v2 3/9] tools: bpftool: add probes for kernel configuration options

[Stanislav Fomichev <sdf@fomichev.me>]

On 12/20, Quentin Monnet wrote:
> 2018-12-20 09:40 UTC-0800 ~ Stanislav Fomichev <sdf@fomichev.me>
> > On 12/20, 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",
> > >              ...
> > >          }
> > >      }
> > > 
> > > v2:
> > > - Remove C-style macros output from this patch.
> > > - NOT addressed: grouping of those config options into subsections
> > >    (I don't see an easy way of grouping them at the moment, please see
> > >    also the discussion on v1 thread).
> > > 
> > > Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> > > Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > ---
> > >   tools/bpf/bpftool/feature.c | 137 ++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 137 insertions(+)
> 
> > > +static void probe_kernel_image_config(void)
> > > +{
> > > +	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");
> > I wonder whether the order here should be reversed?
> > 1. try /proc/config.gz (CONFIG_IKCONFIG_PROC)
> > 2. if not avail, try /boot/config-$(uname -r)
> > 
> > Because, at the end, /proc/config.gz is the real source of truth (if
> > available).
> > 
> > What is /proc/config btw? I can see only /proc/config.gz being exported.
> 
> Hi Stanislav,
> 
> Cilium checks for /proc/config (apparently this is where CoreOS puts its
> config file), /proc/config.gz and /boot/config-$(uname -r) [0]. I took
> inspiration on that but didn't implement the /proc/config.gz file, because
> it would require linking with the libz and I'm not sure this is worth it.
> Could be added as a follow-up if necessary.
Makes sense, then maybe add a comment? So future readers know
that /proc/config.gz is explicitly missing, /proc/config comes from coreos,
and on the other distros we expect /boot/config-$(uname -r).

I'm mainly wondering because my arch box doesn't have
/boot/config-$(uname -r) and has only /proc/config.gz :-) Sigh..
> 
> [0] https://github.com/cilium/cilium/blob/master/bpf/run_probes.sh#L42