by Namhyung Kim

[permalink] [raw]

Subject: Re: [RFC PATCH 04/22] perf tools: Add new 'perf bpf' command.

Hi,

On Thu, Apr 30, 2015 at 10:52:27AM +0000, Wang Nan wrote:
> Adding new 'perf bpf' command to provide eBPF program loading and
> management support.
>
> Signed-off-by: Wang Nan <[email protected]>
> ---
> tools/perf/Build | 1 +
> tools/perf/Documentation/perf-bpf.txt | 18 ++++++++++
> tools/perf/builtin-bpf.c | 63 +++++++++++++++++++++++++++++++++++
> tools/perf/builtin.h | 1 +
> tools/perf/perf.c | 1 +
> tools/perf/util/Build | 1 +
> tools/perf/util/bpf-loader.c | 35 +++++++++++++++++++
> tools/perf/util/bpf-loader.h | 21 ++++++++++++
> 8 files changed, 141 insertions(+)
> create mode 100644 tools/perf/Documentation/perf-bpf.txt
> create mode 100644 tools/perf/builtin-bpf.c
> create mode 100644 tools/perf/util/bpf-loader.c
> create mode 100644 tools/perf/util/bpf-loader.h
>
> diff --git a/tools/perf/Build b/tools/perf/Build
> index b77370e..c69f0c1 100644
> --- a/tools/perf/Build
> +++ b/tools/perf/Build
> @@ -19,6 +19,7 @@ perf-y += builtin-kvm.o
> perf-y += builtin-inject.o
> perf-y += builtin-mem.o
> perf-y += builtin-data.o
> +perf-y += builtin-bpf.o
>
> perf-$(CONFIG_AUDIT) += builtin-trace.o
> perf-$(CONFIG_LIBELF) += builtin-probe.o
> diff --git a/tools/perf/Documentation/perf-bpf.txt b/tools/perf/Documentation/perf-bpf.txt
> new file mode 100644
> index 0000000..634d588
> --- /dev/null
> +++ b/tools/perf/Documentation/perf-bpf.txt
> @@ -0,0 +1,18 @@
> +perf-bpf(1)
> +==============
> +
> +NAME
> +----
> +perf-bpf - loads eBPF programs into kernel.
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'perf bpf' [<common options>] <bpfprogram.o>",
> +
> +DESCRIPTION
> +-----------
> +Loading eBPF programs into kernel.
> +
> +OPTIONS
> +-------
> diff --git a/tools/perf/builtin-bpf.c b/tools/perf/builtin-bpf.c
> new file mode 100644
> index 0000000..0fc7a82
> --- /dev/null
> +++ b/tools/perf/builtin-bpf.c
> @@ -0,0 +1,63 @@
> +/*
> + * buildin-bpf.c

s/buildin/builtn/

> + *
> + * Buildin bpf command: Load bpf and attach bpf programs onto kprobes.

ditto.

> + */
> +#include "builtin.h"
> +#include "perf.h"
> +#include "debug.h"
> +#include "parse-options.h"
> +#include "bpf-loader.h"
> +
> +static const char *bpf_usage[] = {
> + "perf bpf [<options>] <bpfobj>",
> + NULL
> +};
> +
> +static void print_usage(void)
> +{
> + printf("Usage:\n");
> + printf("\t%s\n\n", bpf_usage[0]);
> +}

Why not using usage_with_options() for this?

Thanks,
Namhyung

> +
> +struct option __bpf_options[] = {
> + OPT_INCR('v', "verbose", &verbose, "be more verbose"),
> + OPT_END()
> +};
> +
> +struct option *bpf_options = __bpf_options;
> +
> +int cmd_bpf(int argc, const char **argv,
> + const char *prefix __maybe_unused)
> +{
> + int err;
> + const char **pfn;
> +
> + if (argc < 2)
> + goto usage;
> +
> + argc = parse_options(argc, argv, bpf_options, bpf_usage,
> + PARSE_OPT_STOP_AT_NON_OPTION);
> + if (argc < 1)
> + goto usage;
> +
> + pfn = argv;
> + while (*pfn != NULL) {
> + const char *fn = *pfn++;
> +
> + err = bpf__load(fn);
> + if (err) {
> + pr_err("bpf: load bpf program from %s: result: %d\n",
> + fn, err);
> + break;
> + }
> + }
> +
> + if (!err)
> + bpf__run();
> + return err;
> +usage:
> + print_usage();
> + return -1;
> +}
> +

2015-05-11 06:42:16

by Namhyung Kim

[permalink] [raw]

Subject: Re: [RFC PATCH 09/22] perf bpf: collect map definitions.

On Thu, Apr 30, 2015 at 10:52:32AM +0000, Wang Nan wrote:
> If maps are used by eBPF programs, corresponding object file(s) should
> contain a section named 'map'. Which contains map definitions, one for
> each map to describe its format. 'struct perf_bpf_map_def' is
> introduced as part of protocol between perf and eBPF programs. All map
> definitions are copied to obj->maps.
>
> bpf.h is introduced for common bpf operations.
>
> Signed-off-by: Wang Nan <[email protected]>
> ---
> tools/perf/util/bpf-loader.c | 31 +++++++++++++++++++++++++++++++
> tools/perf/util/bpf-loader.h | 3 +++
> 2 files changed, 34 insertions(+)
>
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index 296fb06..bf3b793 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -65,6 +65,8 @@ static void bpf_obj_close(struct bpf_obj *obj)
>
> if (obj->path)
> free(obj->path);
> + if (obj->maps)
> + free(obj->maps);
> free(obj);
> }
>
> @@ -183,6 +185,32 @@ static int bpf_obj_kver_init(struct bpf_obj *obj,
> return 0;
> }
>
> +static int bpf_obj_maps_init(struct bpf_obj *obj, void *data,
> + size_t size)
> +{
> + size_t map_def_sz = sizeof(struct bpf_map_def);
> + int nr_maps = size / map_def_sz;
> +
> + if (nr_maps == 0) {
> + pr_debug("bpf: %s doesn't need map definition\n",
> + obj->path);
> + return 0;
> + }
> +
> + obj->maps = malloc(nr_maps * map_def_sz);
> + if (!obj->maps) {
> + pr_err("bpf: malloc maps failed: %s\n", obj->path);
> + return -ENOMEM;
> + }
> +
> + obj->nr_maps = nr_maps;
> + memcpy(obj->maps, data, nr_maps * map_def_sz);

Doesn't it need to swap the data as it's binary?

Thanks,
Namhyung

> + pr_debug("bpf: %d map%s in %s\n", nr_maps,
> + nr_maps > 1 ? "s" : "",
> + obj->path);
> + return 0;
> +}
> +
> static int bpf_obj_elf_collect(struct bpf_obj *obj)
> {
> Elf *elf = obj->elf.elf;
> @@ -237,6 +265,9 @@ static int bpf_obj_elf_collect(struct bpf_obj *obj)
> else if (strcmp(name, "version") == 0)
> err = bpf_obj_kver_init(obj, data->d_buf,
> data->d_size);
> + else if (strcmp(name, "maps") == 0)
> + err = bpf_obj_maps_init(obj, data->d_buf,
> + data->d_size);
> if (err)
> goto out;
> }
> diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
> index e1d5c42..6c5c8d6 100644
> --- a/tools/perf/util/bpf-loader.h
> +++ b/tools/perf/util/bpf-loader.h
> @@ -14,6 +14,7 @@
> #include "perf.h"
> #include "symbol.h"
> #include "probe-event.h"
> +#include "bpf.h"
>
> int bpf__load(const char *path);
> int bpf__run(void);
> @@ -25,6 +26,8 @@ struct bpf_obj {
> bool needs_swap;
> char license[64];
> u32 kern_version;
> + struct bpf_map_def *maps;
> + size_t nr_maps;
>
> /*
> * Information when doing elf related work. Only valid if fd
> --
> 1.8.3.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/