[PATCH bpf-next v2 0/2] perf: stop using deprecated bpf APIs

[PATCH bpf-next v2 0/2] perf: stop using deprecated bpf APIs

[Christy Lee]

libbpf's bpf_prog_load() and bpf__object_next() APIs are deprecated.
remove perf's usage of these deprecated functions. After this patch
set, the only remaining libbpf deprecated APIs in perf would be
bpf_program__set_prep() and bpf_program__nth_fd().

Changelog:
----------
v1 -> v2:
https://lore.kernel.org/all/20211216222108.110518-1-christylee@fb.com/

Patch 1/2:
Added missing commit message

Patch 2/2:
Added more details to commit message and added steps to reproduce
original test case.

Christy Lee (2):
  perf: stop using deprecated bpf_prog_load() API
  perf: stop using deprecated bpf__object_next() API

 tools/perf/tests/bpf.c       | 14 ++-----
 tools/perf/util/bpf-loader.c | 72 +++++++++++++++++++++++++++---------
 tools/perf/util/bpf-loader.h |  1 +
 3 files changed, 59 insertions(+), 28 deletions(-)

--
2.30.2

[PATCH bpf-next v2 1/2] perf: stop using deprecated bpf_prog_load() API

[Christy Lee]

bpf_prog_load() API is deprecated, remove perf's usage of the deprecated
function.

Signed-off-by: Christy Lee <christylee@fb.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/perf/tests/bpf.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
index 573490530194..57b9591f7cbb 100644
--- a/tools/perf/tests/bpf.c
+++ b/tools/perf/tests/bpf.c
@@ -281,8 +281,8 @@ static int __test__bpf(int idx)
 
 static int check_env(void)
 {
+	LIBBPF_OPTS(bpf_prog_load_opts, opts);
 	int err;
-	unsigned int kver_int;
 	char license[] = "GPL";
 
 	struct bpf_insn insns[] = {
@@ -290,19 +290,13 @@ static int check_env(void)
 		BPF_EXIT_INSN(),
 	};
 
-	err = fetch_kernel_version(&kver_int, NULL, 0);
+	err = fetch_kernel_version(&opts.kern_version, NULL, 0);
 	if (err) {
 		pr_debug("Unable to get kernel version\n");
 		return err;
 	}
-
-/* temporarily disable libbpf deprecation warnings */
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-	err = bpf_load_program(BPF_PROG_TYPE_KPROBE, insns,
-			       ARRAY_SIZE(insns),
-			       license, kver_int, NULL, 0);
-#pragma GCC diagnostic pop
+	err = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, license, insns,
+			    ARRAY_SIZE(insns), &opts);
 	if (err < 0) {
 		pr_err("Missing basic BPF support, skip this test: %s\n",
 		       strerror(errno));
-- 
2.30.2

Re: [PATCH bpf-next v2 1/2] perf: stop using deprecated bpf_prog_load() API

[Andrii Nakryiko]

On Thu, Jan 6, 2022 at 12:03 PM Christy Lee <christylee@fb.com> wrote:
>
> bpf_prog_load() API is deprecated, remove perf's usage of the deprecated
> function.
>
> Signed-off-by: Christy Lee <christylee@fb.com>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  tools/perf/tests/bpf.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> index 573490530194..57b9591f7cbb 100644
> --- a/tools/perf/tests/bpf.c
> +++ b/tools/perf/tests/bpf.c
> @@ -281,8 +281,8 @@ static int __test__bpf(int idx)
>
>  static int check_env(void)
>  {
> +       LIBBPF_OPTS(bpf_prog_load_opts, opts);
>         int err;
> -       unsigned int kver_int;
>         char license[] = "GPL";
>
>         struct bpf_insn insns[] = {
> @@ -290,19 +290,13 @@ static int check_env(void)
>                 BPF_EXIT_INSN(),
>         };
>
> -       err = fetch_kernel_version(&kver_int, NULL, 0);
> +       err = fetch_kernel_version(&opts.kern_version, NULL, 0);
>         if (err) {
>                 pr_debug("Unable to get kernel version\n");
>                 return err;
>         }
> -
> -/* temporarily disable libbpf deprecation warnings */
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> -       err = bpf_load_program(BPF_PROG_TYPE_KPROBE, insns,
> -                              ARRAY_SIZE(insns),
> -                              license, kver_int, NULL, 0);
> -#pragma GCC diagnostic pop
> +       err = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, license, insns,
> +                           ARRAY_SIZE(insns), &opts);

Christy, you should probably define __weak bpf_prog_load
implementation that will re-route to bpf_load_program? See how it was
done for other APIs (e.g., btf__load_from_kernel_by_id).

Arnaldo, more generally, what would be the plan for libbpf 1.0 at
which point those deprecated APIs (e.g., bpf_load_program) are going
to be removed completely? Do you have some control over the minimal
libbpf version for cases when you'd like to use libbpf as a shared
library?


>         if (err < 0) {
>                 pr_err("Missing basic BPF support, skip this test: %s\n",
>                        strerror(errno));
> --
> 2.30.2
>

[PATCH bpf-next v2 2/2] perf: stop using deprecated bpf__object_next() API

[Christy Lee]

Previously libbpf maintained a list of bpf_objects, and bpf_objects
can be added to the list via bpf__object__next() API. Libbpf has
deprecated the ability to keep track of object list inside libbpf,
so we need to hoist the tracking logic to perf.

Committer note:

This is tested by following the committer's note in the original commit
"aa3abf30bb28addcf593578d37447d42e3f65fc3".

I ran 'perf test -v LLVM' and used it's output to generate a script for
compiling the perf test object:

--------------------------------------------------
$ cat ~/bin/hello-ebpf
INPUT_FILE=/tmp/test.c
OUTPUT_FILE=/tmp/test.o

export KBUILD_DIR=/lib/modules/5.12.0-0_fbk2_3390_g7ecb4ac46d7f/build
export NR_CPUS=56
export LINUX_VERSION_CODE=0x50c00
export CLANG_EXEC=/data/users/christylee/devtools/llvm/latest/bin/clang
export CLANG_OPTIONS=-xc
export KERNEL_INC_OPTIONS="KERNEL_INC_OPTIONS= -nostdinc \
-isystem /usr/lib/gcc/x86_64-redhat-linux/8/include -I./arch/x86/include \
-I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi \
-I./arch/x86/include/generated/uapi -I./include/uapi \
-I./include/generated/uapi -include ./include/linux/compiler-version.h \
-include ./include/linux/kconfig.h"
export PERF_BPF_INC_OPTIONS=-I/usr/lib/perf/include/bpf
export WORKING_DIR=/lib/modules/5.12.0-0_fbk2_3390_g7ecb4ac46d7f/build
export CLANG_SOURCE=-

rm -f $OUTPUT_FILE
cat $INPUT_FILE | /data/users/christylee/devtools/llvm/latest/bin/clang \
-D__KERNEL__ -D__NR_CPUS__=56 -DLINUX_VERSION_CODE=0x50c00 -xc  \
-I/usr/lib/perf/include/bpf -nostdinc \
-isystem /usr/lib/gcc/x86_64-redhat-linux/8/include -I./arch/x86/include \
-I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi \
-I./arch/x86/include/generated/uapi -I./include/uapi \
-I./include/generated/uapi -include ./include/linux/compiler-version.h \
-include ./include/linux/kconfig.h -Wno-unused-value -Wno-pointer-sign \
-working-directory /lib/modules/5.12.0-0_fbk2_3390_g7ecb4ac46d7f/build \
-c - -target bpf -O2 -o $OUTPUT_FILE
--------------------------------------------------

I then wrote and compiled a script that ask to get asks to put a probe
at a function that
does not exists in the kernel, it errors out as expected:

$ cat /tmp/test.c
__attribute__((section("probe_point=not_exist"), used))
int probe_point(void *ctx) {
    return 0;
}
char _license[] __attribute__((section("license"), used)) = "GPL";
int _version __attribute__((section("version"), used)) = 0x40100;

$ cd ~/bin && ./hello-ebpf
$ ./perf record --event /tmp/test.o sleep 1

Using perf wrapper that supports hot-text. Try perf.real if you
encounter any issues.
Probe point 'not_exist' not found.
event syntax error: '/tmp/test.o'
                     \___ You need to check probing points in BPF file

(add -v to see detail)
Run 'perf list' for a list of valid events

 Usage: perf record [<options>] [<command>]
    or: perf record [<options>] -- <command> [<options>]

    -e, --event <event>   event selector. use 'perf list' to list
available events

---------------------------------------------------

Next I changed the attribute to something that exists in the kernel.
As expected, it errors out
with permission problem:
$ cat /tmp/test.c
__attribute__((section("probe_point=kernel_execve"), used))
int probe_point(void *ctx) {
    return 0;
}
char _license[] __attribute__((section("license"), used)) = "GPL";
int _version __attribute__((section("version"), used)) = 0x40100;

$ grep kernel_execve /proc/kallsyms
ffffffff812dc210 T kernel_execve

$ cd ~/bin && ./hello-ebpf
$ ./perf record --event /tmp/test.o sleep 1

Using perf wrapper that supports hot-text. Try perf.real if you
encounter any issues.
Failed to open kprobe_events: Permission denied
event syntax error: '/tmp/test.o'
                     \___ You need to be root

(add -v to see detail)
Run 'perf list' for a list of valid events

 Usage: perf record [<options>] [<command>]
    or: perf record [<options>] -- <command> [<options>]

    -e, --event <event>   event selector. use 'perf list' to list
available events

---------------------------------------------------

Reran as root, see that the probe point worked as intended.

$ sudo -i
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.018 MB perf.data ]
perf_bpf_probe:probe_point
perf_bpf_probe:probe_point: type: 2, size: 128, config: 0x8e9, \
{ sample_period, sample_freq }: 1, \
sample_type: IP|TID|TIME|CPU|PERIOD|RAW, read_format: ID, disabled: 1, \
inherit: 1, mmap: 1, comm: 1, enable_on_exec: 1, task: 1, \
sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, \
bpf_event: 1

---------------------------------------------------

Signed-off-by: Christy Lee <christylee@fb.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/perf/util/bpf-loader.c | 72 +++++++++++++++++++++++++++---------
 tools/perf/util/bpf-loader.h |  1 +
 2 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 528aeb0ab79d..9e3988fd719a 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -29,9 +29,6 @@
 
 #include <internal/xyarray.h>
 
-/* temporarily disable libbpf deprecation warnings */
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-
 static int libbpf_perf_print(enum libbpf_print_level level __attribute__((unused)),
 			      const char *fmt, va_list args)
 {
@@ -49,6 +46,36 @@ struct bpf_prog_priv {
 	int *type_mapping;
 };
 
+struct bpf_perf_object {
+	struct bpf_object *obj;
+	struct list_head list;
+};
+
+static LIST_HEAD(bpf_objects_list);
+
+struct bpf_perf_object *bpf_perf_object__next(struct bpf_perf_object *prev)
+{
+	struct bpf_perf_object *next;
+
+	if (!prev)
+		next = list_first_entry(&bpf_objects_list,
+					struct bpf_perf_object, list);
+	else
+		next = list_next_entry(prev, list);
+
+	/* Empty list is noticed here so don't need checking on entry. */
+	if (&next->list == &bpf_objects_list)
+		return NULL;
+
+	return next;
+}
+
+#define bpf_perf_object__for_each(perf_obj, tmp, obj)                          \
+	for ((perf_obj) = bpf_perf_object__next(NULL),                         \
+	    (tmp) = bpf_perf_object__next(perf_obj), (obj) = NULL;             \
+	     (perf_obj) != NULL; (perf_obj) = (tmp),                           \
+	    (tmp) = bpf_perf_object__next(tmp), (obj) = (perf_obj)->obj)
+
 static bool libbpf_initialized;
 
 struct bpf_object *
@@ -113,9 +140,10 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
 
 void bpf__clear(void)
 {
-	struct bpf_object *obj, *tmp;
+	struct bpf_perf_object *perf_obj, *tmp;
+	struct bpf_object *obj;
 
-	bpf_object__for_each_safe(obj, tmp) {
+	bpf_perf_object__for_each(perf_obj, tmp, obj) {
 		bpf__unprobe(obj);
 		bpf_object__close(obj);
 	}
@@ -621,8 +649,12 @@ static int hook_load_preprocessor(struct bpf_program *prog)
 	if (err)
 		return err;
 
+/* temporarily disable libbpf deprecation warnings */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 	err = bpf_program__set_prep(prog, priv->nr_types,
 				    preproc_gen_prologue);
+#pragma GCC diagnostic pop
 	return err;
 }
 
@@ -776,7 +808,11 @@ int bpf__foreach_event(struct bpf_object *obj,
 			if (priv->need_prologue) {
 				int type = priv->type_mapping[i];
 
+/* temporarily disable libbpf deprecation warnings */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 				fd = bpf_program__nth_fd(prog, type);
+#pragma GCC diagnostic pop
 			} else {
 				fd = bpf_program__fd(prog);
 			}
@@ -1498,10 +1534,11 @@ apply_obj_config_object(struct bpf_object *obj)
 
 int bpf__apply_obj_config(void)
 {
-	struct bpf_object *obj, *tmp;
+	struct bpf_perf_object *perf_obj, *tmp;
+	struct bpf_object *obj;
 	int err;
 
-	bpf_object__for_each_safe(obj, tmp) {
+	bpf_perf_object__for_each(perf_obj, tmp, obj) {
 		err = apply_obj_config_object(obj);
 		if (err)
 			return err;
@@ -1510,26 +1547,25 @@ int bpf__apply_obj_config(void)
 	return 0;
 }
 
-#define bpf__for_each_map(pos, obj, objtmp)	\
-	bpf_object__for_each_safe(obj, objtmp)	\
-		bpf_object__for_each_map(pos, obj)
+#define bpf__perf_for_each_map(perf_obj, tmp, obj, map)                        \
+	bpf_perf_object__for_each(perf_obj, tmp, obj)                          \
+		bpf_object__for_each_map(map, obj)
 
-#define bpf__for_each_map_named(pos, obj, objtmp, name)	\
-	bpf__for_each_map(pos, obj, objtmp) 		\
-		if (bpf_map__name(pos) && 		\
-			(strcmp(name, 			\
-				bpf_map__name(pos)) == 0))
+#define bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name)            \
+	bpf__perf_for_each_map(perf_obj, tmp, obj, map)                        \
+		if (bpf_map__name(map) && (strcmp(name, bpf_map__name(map)) == 0))
 
 struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name)
 {
 	struct bpf_map_priv *tmpl_priv = NULL;
-	struct bpf_object *obj, *tmp;
+	struct bpf_perf_object *perf_obj, *tmp;
+	struct bpf_object *obj;
 	struct evsel *evsel = NULL;
 	struct bpf_map *map;
 	int err;
 	bool need_init = false;
 
-	bpf__for_each_map_named(map, obj, tmp, name) {
+	bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name) {
 		struct bpf_map_priv *priv = bpf_map__priv(map);
 
 		if (IS_ERR(priv))
@@ -1565,7 +1601,7 @@ struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name)
 		evsel = evlist__last(evlist);
 	}
 
-	bpf__for_each_map_named(map, obj, tmp, name) {
+	bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name) {
 		struct bpf_map_priv *priv = bpf_map__priv(map);
 
 		if (IS_ERR(priv))
diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
index 5d1c725cea29..95262b7e936f 100644
--- a/tools/perf/util/bpf-loader.h
+++ b/tools/perf/util/bpf-loader.h
@@ -53,6 +53,7 @@ typedef int (*bpf_prog_iter_callback_t)(const char *group, const char *event,
 
 #ifdef HAVE_LIBBPF_SUPPORT
 struct bpf_object *bpf__prepare_load(const char *filename, bool source);
+struct bpf_perf_object *bpf_perf_object__next(struct bpf_perf_object *prev);
 int bpf__strerror_prepare_load(const char *filename, bool source,
 			       int err, char *buf, size_t size);
 
-- 
2.30.2

Re: [PATCH bpf-next v2 2/2] perf: stop using deprecated bpf__object_next() API

[Yonghong Song]

On 1/6/22 12:00 PM, Christy Lee wrote:
> Previously libbpf maintained a list of bpf_objects, and bpf_objects
> can be added to the list via bpf__object__next() API. Libbpf has
> deprecated the ability to keep track of object list inside libbpf,
> so we need to hoist the tracking logic to perf.
> 
> Committer note:
> 
> This is tested by following the committer's note in the original commit
> "aa3abf30bb28addcf593578d37447d42e3f65fc3".
> 
> I ran 'perf test -v LLVM' and used it's output to generate a script for
> compiling the perf test object:
> 
> --------------------------------------------------
> $ cat ~/bin/hello-ebpf
> INPUT_FILE=/tmp/test.c
> OUTPUT_FILE=/tmp/test.o
> 
> export KBUILD_DIR=/lib/modules/5.12.0-0_fbk2_3390_g7ecb4ac46d7f/build
> export NR_CPUS=56
> export LINUX_VERSION_CODE=0x50c00
> export CLANG_EXEC=/data/users/christylee/devtools/llvm/latest/bin/clang
> export CLANG_OPTIONS=-xc
> export KERNEL_INC_OPTIONS="KERNEL_INC_OPTIONS= -nostdinc \
> -isystem /usr/lib/gcc/x86_64-redhat-linux/8/include -I./arch/x86/include \
> -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi \
> -I./arch/x86/include/generated/uapi -I./include/uapi \
> -I./include/generated/uapi -include ./include/linux/compiler-version.h \
> -include ./include/linux/kconfig.h"
> export PERF_BPF_INC_OPTIONS=-I/usr/lib/perf/include/bpf
> export WORKING_DIR=/lib/modules/5.12.0-0_fbk2_3390_g7ecb4ac46d7f/build
> export CLANG_SOURCE=-
> 
> rm -f $OUTPUT_FILE
> cat $INPUT_FILE | /data/users/christylee/devtools/llvm/latest/bin/clang \
> -D__KERNEL__ -D__NR_CPUS__=56 -DLINUX_VERSION_CODE=0x50c00 -xc  \
> -I/usr/lib/perf/include/bpf -nostdinc \
> -isystem /usr/lib/gcc/x86_64-redhat-linux/8/include -I./arch/x86/include \
> -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi \
> -I./arch/x86/include/generated/uapi -I./include/uapi \
> -I./include/generated/uapi -include ./include/linux/compiler-version.h \
> -include ./include/linux/kconfig.h -Wno-unused-value -Wno-pointer-sign \
> -working-directory /lib/modules/5.12.0-0_fbk2_3390_g7ecb4ac46d7f/build \
> -c - -target bpf -O2 -o $OUTPUT_FILE
> --------------------------------------------------
> 
> I then wrote and compiled a script that ask to get asks to put a probe
> at a function that
> does not exists in the kernel, it errors out as expected:
> 
> $ cat /tmp/test.c
> __attribute__((section("probe_point=not_exist"), used))
> int probe_point(void *ctx) {
>      return 0;
> }
> char _license[] __attribute__((section("license"), used)) = "GPL";
> int _version __attribute__((section("version"), used)) = 0x40100;
> 
> $ cd ~/bin && ./hello-ebpf
> $ ./perf record --event /tmp/test.o sleep 1
> 
> Using perf wrapper that supports hot-text. Try perf.real if you
> encounter any issues.

The above doesn't exist in upstream perf. It would be good to just 
demonstrate with upstream built perf. Otherwise, people will confuse
what 'perf' we are talking about here.

The same for another 'perf record' below.

> Probe point 'not_exist' not found.
> event syntax error: '/tmp/test.o'
>                       \___ You need to check probing points in BPF file
> 
> (add -v to see detail)
> Run 'perf list' for a list of valid events
> 
>   Usage: perf record [<options>] [<command>]
>      or: perf record [<options>] -- <command> [<options>]
> 
>      -e, --event <event>   event selector. use 'perf list' to list
> available events
> 
> ---------------------------------------------------
> 
> Next I changed the attribute to something that exists in the kernel.
> As expected, it errors out
> with permission problem:
> $ cat /tmp/test.c
> __attribute__((section("probe_point=kernel_execve"), used))
> int probe_point(void *ctx) {
>      return 0;
> }
> char _license[] __attribute__((section("license"), used)) = "GPL";
> int _version __attribute__((section("version"), used)) = 0x40100;
> 
> $ grep kernel_execve /proc/kallsyms
> ffffffff812dc210 T kernel_execve
> 
> $ cd ~/bin && ./hello-ebpf
> $ ./perf record --event /tmp/test.o sleep 1
> 
> Using perf wrapper that supports hot-text. Try perf.real if you
> encounter any issues.

here.

> Failed to open kprobe_events: Permission denied
> event syntax error: '/tmp/test.o'
>                       \___ You need to be root
> 
> (add -v to see detail)
> Run 'perf list' for a list of valid events
> 
>   Usage: perf record [<options>] [<command>]
>      or: perf record [<options>] -- <command> [<options>]
> 
>      -e, --event <event>   event selector. use 'perf list' to list
> available events
> 
[...]