[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b8bff20c-20e5-cbf0-583a-588bfe8daa45@netronome.com>
Date: Thu, 20 Dec 2018 18:14:03 +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: [RFC PATCH bpf-next v2 6/9] tools: bpftool: add probes for eBPF
helper functions
2018-12-20 09:53 UTC-0800 ~ Stanislav Fomichev <sdf@...ichev.me>
> On 12/20, Quentin Monnet wrote:
>> Similarly to what was done for program types and map types, add a set of
>> probes to test the availability of the different eBPF helper functions
>> on the current system.
>>
>> Each known helper is tested with all program types supported by the
>> system, in order to establish a compatibility matrix. Output is provided
>> as a list of compatible program types, for each helper.
>>
>> Sample output:
>>
>> # bpftool feature probe kernel
>> ...
>> Scanning eBPF helper functions...
>> ...
>> eBPF helper bpf_skb_change_head supported for program types: \
>> lwt_xmit sk_skb
>> eBPF helper bpf_xdp_adjust_head supported for program types: xdp
>> eBPF helper bpf_probe_read_str supported for program types: \
>> kprobe tracepoint perf_event raw_tracepoint
>> ...
>>
>> # bpftool --json --pretty feature probe kernel
>> {
>> ...
>> "helpers": {
>> ...
>> "bpf_skb_change_head_compat_list": ["lwt_xmit","sk_skb"
>> ],
>> "bpf_xdp_adjust_head_compat_list": ["xdp"
>> ],
>> "bpf_probe_read_str_compat_list": ["kprobe","tracepoint", \
>> "perf_event","raw_tracepoint"
>> ],
>> ...
>> }
>> }
>>
>> v2:
>> - Move probes from bpftool to libbpf.
>> - Test all program types for each helper, print a list of working prog
>> types for each helper.
>> - Fall back on include/uapi/linux/bpf.h for names and ids of helpers.
>> - Remove C-style macros output from this patch.
>>
>> Signed-off-by: Quentin Monnet <quentin.monnet@...ronome.com>
>> ---
>> .../bpftool/Documentation/bpftool-feature.rst | 4 ++
>> tools/bpf/bpftool/feature.c | 51 +++++++++++++++
>> tools/lib/bpf/libbpf.h | 2 +
>> tools/lib/bpf/libbpf.map | 1 +
>> tools/lib/bpf/libbpf_probes.c | 63 +++++++++++++++++++
>> 5 files changed, 121 insertions(+)
>>
>> diff --git a/tools/bpf/bpftool/Documentation/bpftool-feature.rst b/tools/bpf/bpftool/Documentation/bpftool-feature.rst
>> index 40ac13c0b782..255e3b3629a0 100644
>> --- a/tools/bpf/bpftool/Documentation/bpftool-feature.rst
>> +++ b/tools/bpf/bpftool/Documentation/bpftool-feature.rst
>> @@ -30,6 +30,10 @@ DESCRIPTION
>>
>> Keyword **kernel** can be omitted.
>>
>> + Note that when probed, some eBPF helpers (e.g.
>> + **bpf_trace_printk**\ () or **bpf_probe_write_user**\ ()) may
>> + print warnings to kernel logs.
>> +
>> **bpftool feature help**
>> Print short help message.
>>
>> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
>> index 3c44953ded5a..bc483823014b 100644
>> --- a/tools/bpf/bpftool/feature.c
>> +++ b/tools/bpf/bpftool/feature.c
>> @@ -25,6 +25,11 @@ enum probe_component {
>> COMPONENT_KERNEL,
>> };
>>
>> +#define BPF_HELPER_MAKE_ENTRY(name) [BPF_FUNC_ ## name] = "bpf_" # name
>> +static const char * const helper_name[] = {
>> + __BPF_FUNC_MAPPER(BPF_HELPER_MAKE_ENTRY)
>> +};
>> +
>> /* Miscellaneous utility functions */
>>
>> static bool check_procfs(void)
>> @@ -418,6 +423,45 @@ static void probe_map_type(enum bpf_map_type map_type)
>> print_bool_feature(feat_name, plain_desc, res);
>> }
>>
>> +static void
>> +probe_helper(__u32 id, const char *name, int kernel_version,
>> + bool *supported_types)
>> +{
>> + char feat_name[128], plain_desc[128];
>> + unsigned int i;
>> +
>> + sprintf(feat_name, "%s_compat_list", name);
>> + sprintf(plain_desc,
>> + "eBPF helper %s supported for program types:",
>> + name);
>> +
>> + if (json_output) {
>> + jsonw_name(json_wtr, feat_name);
>> + jsonw_start_array(json_wtr);
>> + } else {
>> + printf("%s", plain_desc);
>> + }
>> +
>> + for (i = BPF_PROG_TYPE_UNSPEC + 1;
>> + i < ARRAY_SIZE(prog_type_name); i++) {
>> + if (!supported_types[i])
>> + continue;
>> +
>> + if (!bpf_probe_helper(id, i, kernel_version, 0))
>> + continue;
>> +
>> + if (json_output)
>> + jsonw_string(json_wtr, prog_type_name[i]);
>> + else
>> + printf(" %s", prog_type_name[i]);
>> + }
>> +
>> + if (json_output)
>> + jsonw_end_array(json_wtr);
>> + else
>> + printf("\n");
>> +}
>> +
>> static int do_probe(int argc, char **argv)
>> {
>> enum probe_component target = COMPONENT_UNSPEC;
>> @@ -494,6 +538,13 @@ static int do_probe(int argc, char **argv)
>> for (i = BPF_MAP_TYPE_UNSPEC + 1; i < map_type_name_size; i++)
>> probe_map_type(i);
>>
>> + print_end_then_start_section("helpers",
>> + "Scanning eBPF helper functions...");
>> +
>> + for (i = 1; i < ARRAY_SIZE(helper_name); i++)
>> + probe_helper(i, helper_name[i], kernel_version,
>> + supported_types);
>> +
>> exit_close_json:
>> if (json_output) {
>> /* End current "section" of probes */
>> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
>> index 202c1ee5c579..ab5b40af5fae 100644
>> --- a/tools/lib/bpf/libbpf.h
>> +++ b/tools/lib/bpf/libbpf.h
>> @@ -361,6 +361,8 @@ bpf_prog_linfo__lfind(const struct bpf_prog_linfo *prog_linfo,
>> LIBBPF_API bool bpf_probe_prog_type(enum bpf_prog_type prog_type,
>> int kernel_version, __u32 ifindex);
>> LIBBPF_API bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex);
>> +LIBBPF_API bool bpf_probe_helper(__u32 id, enum bpf_prog_type prog_type,
>> + int kernel_version, __u32 ifindex);
>>
>> #ifdef __cplusplus
>> } /* extern "C" */
>> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
>> index c08f4c726e8e..67e51b2becec 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_helper;
>> bpf_probe_map_type;
>> bpf_probe_prog_type;
>> bpf_prog_attach;
>> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
>> index 796fe1e66169..19aa062f33b5 100644
>> --- a/tools/lib/bpf/libbpf_probes.c
>> +++ b/tools/lib/bpf/libbpf_probes.c
>> @@ -3,7 +3,11 @@
>> /* Copyright (c) 2018 Netronome Systems, Inc. */
>>
>> #include <errno.h>
>> +#include <fcntl.h>
>> +#include <string.h>
>> +#include <stdlib.h>
>> #include <unistd.h>
>> +#include <net/if.h>
>>
>> #include <linux/filter.h>
>> #include <linux/kernel.h>
>> @@ -11,6 +15,37 @@
>> #include "bpf.h"
>> #include "libbpf.h"
>>
>> +static bool grep(const char *buffer, const char *pattern)
>> +{
>> + return !!strstr(buffer, pattern);
>> +}
>> +
>> +static int get_vendor_id(int ifindex)
>> +{
>> + char ifname[IF_NAMESIZE], path[64], buf[8];
>> + ssize_t len;
>> + int fd;
>> +
>> + if (!if_indextoname(ifindex, ifname))
>> + return -1;
>> +
>> + snprintf(path, sizeof(path), "/sys/class/net/%s/device/vendor", ifname);
>> +
>> + fd = open(path, O_RDONLY);
>> + if (fd < 0)
>> + return -1;
>> +
>> + len = read(fd, buf, sizeof(buf));
>> + close(fd);
>> + if (len < 0)
>> + return -1;
>> + if (len >= (ssize_t)sizeof(buf))
>> + return -1;
>> + buf[len] = '\0';
>> +
>> + return strtol(buf, NULL, 0);
>> +}
>> +
>> 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,
>> @@ -112,3 +147,31 @@ bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex)
>>
>> return fd >= 0;
>> }
>> +
>> +bool bpf_probe_helper(__u32 id, enum bpf_prog_type prog_type,
>> + int kernel_version, __u32 ifindex)
>> +{
>> + struct bpf_insn insns[2] = {
>> + BPF_EMIT_CALL(id),
>> + BPF_EXIT_INSN()
>> + };
>> + char buf[4096] = {};
>> + bool res;
>> +
>> + prog_load(prog_type, insns, ARRAY_SIZE(insns), kernel_version,
>> + buf, sizeof(buf), ifindex);
>> + res = !grep(buf, "invalid func ") && !grep(buf, "unknown func ");
> Why not look at errno directly?
Because your program can be rejected for a whole lot of reasons here,
since we do not make any effort on ensuring the call to the helper would
be valid. So if I remember correctly, -EINVAL would be returned if the
helper is not supported, or if the number of arguments is incorrect, or
if the arguments are not of the correct type, or...
So instead I'm looking for those error messages in the log buffer, which
seems much more reliable in this case.
Powered by blists - more mailing lists