[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8ecf0c62-b773-eb8f-d668-ab1e6bb6bae4@netronome.com>
Date: Thu, 20 Dec 2018 18:21:55 +0000
From: Quentin Monnet <quentin.monnet@...ronome.com>
To: Stanislav Fomichev <sdf@...ichev.me>
Cc: Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>, 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: [oss-drivers] Re: [RFC PATCH bpf-next v2 4/9] tools: bpftool: add
probes for eBPF program types
2018-12-20 10:17 UTC-0800 ~ Stanislav Fomichev <sdf@...ichev.me>
> On 12/20, Quentin Monnet wrote:
>> 2018-12-20 09:45 UTC-0800 ~ Stanislav Fomichev <sdf@...ichev.me>
>>> On 12/20, Quentin Monnet wrote:
>>>> Introduce probes for supported BPF program types in libbpf, and call it
>>>> from bpftool to test what types are available on the system. The probe
>>>> simply consists in loading a very simple program of that type and see if
>>>> the verifier complains or not.
>>>>
>>>> Sample output:
>>>>
>>>> # bpftool feature probe kernel
>>>> ...
>>>> Scanning eBPF program types...
>>>> eBPF program_type socket_filter is available
>>>> eBPF program_type kprobe is available
>>>> eBPF program_type sched_cls is available
>>>> ...
>>>>
>>>> # bpftool --json --pretty feature probe kernel
>>>> {
>>>> ...
>>>> "program_types": {
>>>> "have_socket_filter_prog_type": true,
>>>> "have_kprobe_prog_type": true,
>>>> "have_sched_cls_prog_type": true,
>>>> ...
>>>> }
>>>> }
>>>>
>>>> v2:
>>>> - Move probes from bpftool to libbpf.
>>>> - Remove C-style macros output from this patch.
>>>>
>>>> Signed-off-by: Quentin Monnet <quentin.monnet@...ronome.com>
>>>> ---
>>>> tools/bpf/bpftool/feature.c | 53 +++++++++++++++++++++++++++++++--
>>>> tools/lib/bpf/Build | 2 +-
>>>> tools/lib/bpf/libbpf.h | 6 ++++
>>>> tools/lib/bpf/libbpf.map | 1 +
>>>> tools/lib/bpf/libbpf_probes.c | 55 +++++++++++++++++++++++++++++++++++
>>>> 5 files changed, 114 insertions(+), 3 deletions(-)
>>>> create mode 100644 tools/lib/bpf/libbpf_probes.c
>>>>
>>>> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
>>>> index 238d7b80f426..3ba0a0a5904c 100644
>>>> --- a/tools/bpf/bpftool/feature.c
>>>> +++ b/tools/bpf/bpftool/feature.c
>>>> @@ -1,6 +1,7 @@
>>>> // SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> /* Copyright (c) 2018 Netronome Systems, Inc. */
>>>> +#include <ctype.h>
>>>> #include <errno.h>
>>>> #include <string.h>
>>>> #include <unistd.h>
>>>> @@ -11,6 +12,7 @@
>>>> #include <linux/limits.h>
>>>> #include <bpf.h>
>>>> +#include <libbpf.h>
>>>> #include "main.h"
>>>> @@ -83,6 +85,17 @@ print_start_section(const char *json_title, const char *plain_title)
>>>> }
>>>> }
>>>> +static void
>>>> +print_end_then_start_section(const char *json_title, const char *plain_title)
>>>> +{
>>>> + if (json_output)
>>>> + jsonw_end_object(json_wtr);
>>>> + else
>>>> + printf("\n");
>>>> +
>>>> + print_start_section(json_title, plain_title);
>>>> +}
>>>> +
>>>> /* Probing functions */
>>>> static int read_procfs(const char *path)
>>>> @@ -361,9 +374,36 @@ static bool probe_bpf_syscall(void)
>>>> return res;
>>>> }
>>>> +static void
>>>> +probe_prog_type(enum bpf_prog_type prog_type, int kernel_version,
>>>> + bool *supported_types)
>>>> +{
>>>> + const char *plain_comment = "eBPF program_type ";
>>>> + char feat_name[128], plain_desc[128];
>>>> + size_t maxlen;
>>>> + bool res;
>>>> +
>>>> + res = bpf_probe_prog_type(prog_type, kernel_version, 0);
>>>> +
>>>> + supported_types[prog_type] |= res;
>>>> +
>>>> + maxlen = sizeof(plain_desc) - strlen(plain_comment) - 1;
>>>> + if (strlen(prog_type_name[prog_type]) > maxlen) {
>>>> + p_info("program type name too long");
>>>> + return;
>>>> + }
>>>> +
>>>> + sprintf(feat_name, "have_%s_prog_type", prog_type_name[prog_type]);
>>>> + sprintf(plain_desc, "%s%s", plain_comment, prog_type_name[prog_type]);
>>>> + print_bool_feature(feat_name, plain_desc, res);
>>>> +}
>>>> +
>>>> static int do_probe(int argc, char **argv)
>>>> {
>>>> enum probe_component target = COMPONENT_UNSPEC;
>>>> + bool supported_types[128] = {};
>>>> + int kernel_version;
>>>> + unsigned int i;
>>>> /* Detection assumes user has sufficient privileges (CAP_SYS_ADMIN).
>>>> * Let's approximate, and restrict usage to root user only.
>>>> @@ -417,9 +457,18 @@ static int do_probe(int argc, char **argv)
>>>> print_start_section("syscall_config",
>>>> "Scanning system call and kernel version...");
>>>> - probe_kernel_version();
>>>> - probe_bpf_syscall();
>>>> + kernel_version = probe_kernel_version();
>>>> + if (!probe_bpf_syscall())
>>>> + /* bpf() syscall unavailable, don't probe other BPF features */
>>>> + goto exit_close_json;
>>>> +
>>>> + print_end_then_start_section("program_types",
>>>> + "Scanning eBPF program types...");
>>>> +
>>>> + for (i = BPF_PROG_TYPE_UNSPEC + 1; i < ARRAY_SIZE(prog_type_name); i++)
>>>> + probe_prog_type(i, kernel_version, supported_types);
>>>> +exit_close_json:
>>>> if (json_output) {
>>>> /* End current "section" of probes */
>>>> jsonw_end_object(json_wtr);
>>>> diff --git a/tools/lib/bpf/Build b/tools/lib/bpf/Build
>>>> index 197b40f5b5c6..bfd9bfc82c3b 100644
>>>> --- a/tools/lib/bpf/Build
>>>> +++ b/tools/lib/bpf/Build
>>>> @@ -1 +1 @@
>>>> -libbpf-y := libbpf.o bpf.o nlattr.o btf.o libbpf_errno.o str_error.o netlink.o bpf_prog_linfo.o
>>>> +libbpf-y := libbpf.o bpf.o nlattr.o btf.o libbpf_errno.o str_error.o netlink.o bpf_prog_linfo.o libbpf_probes.o
>>>> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
>>>> index 5f68d7b75215..f4bb2764ca9a 100644
>>>> --- a/tools/lib/bpf/libbpf.h
>>>> +++ b/tools/lib/bpf/libbpf.h
>>>> @@ -355,6 +355,12 @@ LIBBPF_API const struct bpf_line_info *
>>>> bpf_prog_linfo__lfind(const struct bpf_prog_linfo *prog_linfo,
>>>> __u32 insn_off, __u32 nr_skip);
>>>> +/*
>>>> + * Probe for supported system features
>>>> + */
>>>> +LIBBPF_API bool bpf_probe_prog_type(enum bpf_prog_type prog_type,
>>>> + int kernel_version, __u32 ifindex);
>>>> +
>>>> #ifdef __cplusplus
>>>> } /* extern "C" */
>>>> #endif
>>>> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
>>>> index cd02cd4e2cc3..6355e4c80a86 100644
>>>> --- a/tools/lib/bpf/libbpf.map
>>>> +++ b/tools/lib/bpf/libbpf.map
>>>> @@ -56,6 +56,7 @@ LIBBPF_0.0.1 {
>>>> bpf_object__unpin_maps;
>>>> bpf_object__unpin_programs;
>>>> bpf_perf_event_read_simple;
>>>> + bpf_probe_prog_type;
>>>> bpf_prog_attach;
>>>> bpf_prog_detach;
>>>> bpf_prog_detach2;
>>>> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
>>>> new file mode 100644
>>>> index 000000000000..2c5e0cdc9f2f
>>>> --- /dev/null
>>>> +++ b/tools/lib/bpf/libbpf_probes.c
>>>> @@ -0,0 +1,55 @@
>>>> +// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
>>>> +
>>>> +/* Copyright (c) 2018 Netronome Systems, Inc. */
>>>> +
>>>> +#include <errno.h>
>>>> +#include <unistd.h>
>>>> +
>>>> +#include <linux/filter.h>
>>>> +#include <linux/kernel.h>
>>>> +
>>>> +#include "bpf.h"
>>>> +#include "libbpf.h"
>>>> +
>>>> +static void
>>>> +prog_load(enum bpf_prog_type prog_type, const struct bpf_insn *insns,
>>>> + size_t insns_cnt, int kernel_version, char *buf, size_t buf_len,
>>>> + __u32 ifindex)
>>>> +{
>>>> + struct bpf_load_program_attr xattr = {};
>>>> + int fd;
>>>> +
>>>> + /* Some prog type require an expected_attach_type */
>>>> + if (prog_type == BPF_PROG_TYPE_CGROUP_SOCK_ADDR)
>>>> + xattr.expected_attach_type = BPF_CGROUP_INET4_CONNECT;
>>>> +
>>>> + xattr.prog_type = prog_type;
>>>> + xattr.insns = insns;
>>>> + xattr.insns_cnt = insns_cnt;
>>>> + xattr.license = "GPL";
>>>> + xattr.kern_version = kernel_version;
>>>> + xattr.prog_ifindex = ifindex;
>>>> +
>>>> + fd = bpf_load_program_xattr(&xattr, buf, buf_len);
>>>> + if (fd >= 0)
>>>> + close(fd);
>>>> +}
>>>> +
>>>> +bool bpf_probe_prog_type(enum bpf_prog_type prog_type, int kernel_version,
>>> Do we really need this kernel_version argument? Isn't it going away in
>>> the future (I saw a patch from Daniel that drops kernel check under
>>> sys_bpf). Going forward, does it make sense to have it at the API
>>> level?
>>
>> It does go away and should be left aside for new applications. The objective
>> with bpftool is to be able to probe systems possibly running older kernels
>> though, and the only way to probe for kprobes support on anything older than
>> a 4.21 is to keep the version number.
> But isn't kernel_version irrelevant for probing? For probing, we always want
> to have kernel_version == <running kernel version>. So I don't
> understand why libbpf user should care about the version.
Oh, do you mean it should be collected in libbpf instead of bpftool and
not passed by the caller? Sorry I didn't understand in the first place.
If so, you're absolutely right, I should fix that, thanks!
Powered by blists - more mailing lists