[RFC bpf-next 0/3] libbpf: Add support for aliased BPF programs

[RFC bpf-next 0/3] libbpf: Add support for aliased BPF programs

[Viktor Malik]

TL;DR

This adds libbpf support for creating multiple BPF programs having the
same instructions using symbol aliases.

Context
=======

bpftrace has so-called "wildcarded" probes which allow to attach the
same program to multple different attach points. For k(u)probes, this is
easy to do as we can leverage k(u)probe_multi, however, other program
types (fentry/fexit, tracepoints) don't have such features.

Currently, what bpftrace does is that it creates a copy of the program
for each attach point. This naturally results in a lot of redundant code
in the produced BPF object.

Proposal
========

One way to address this problem would be to use *symbol aliases*. In
short, they allow to have multiple symbol table entries for the same
address. In bpftrace, we would create them using llvm::GlobalAlias. In
C, it can be achieved using compiler __attribute__((alias(...))):

    int BPF_PROG(prog)
    {
        [...]
    }
    int prog_alias() __attribute__((alias("prog")));

When calling bpf_object__open, libbpf is currently able to discover all
the programs and internally does a separate copy of the instructions for
each aliased program. What libbpf cannot do, is perform relocations b/c
it assumes that each instruction belongs to a single program only. The
second patch of this series changes relocation collection such that it
records relocations for each aliased program. With that, bpftrace can
emit just one copy of the full program and an alias for each target
attach point.

For example, considering the following bpftrace script collecting the
number of hits of each VFS function using fentry over a one second
period:

    $ bpftrace -e 'kfunc:vfs_* { @[func] = count() } i:s:1 { exit() }'
    [...]

this change will allow to reduce the size of the in-memory BPF object
that bpftrace generates from 60K to 9K.

For reference, the bpftrace PoC is in [1].

The advantage of this change is that for BPF objects without aliases, it
doesn't introduce any overhead.

Limitations
===========

Unfortunately, the second patch of the series is only sufficient for
bpftrace approach. When using bpftool to generate BPF objects, such as
is done in selftests, it turns out that libbpf linker cannot handle
aliased programs. The reason is that Clang doesn't emit BTF for the
aliased symbols which the linker doesn't expect. This is resolved by the
first patch of the series which improves searching BTF entries for
symbols such that an entry for a different symbol located at the same
address can be used. This, however, requires an additional lookup in the
symbol table during BTF id search in find_glob_sym_btf, increasing the
complexity of the method.

To quantify the impact of this, I ran some simple benchmarks.
The overhead naturally grows with the number of programs in the object.
I took one BPF object from selftests (dynptr_fail.bpf.o) with many (72)
BPF programs inside and extracted some stats from calling

    $ bpftool gen object dynptr_fail.bpf.linked.o dynptr_fail.bpf.o

Here are the stats:

    master:
        0.015922 +- 0.000251 seconds time elapsed  ( +-  1.58% )
        10 calls of find_sym_by_name

    patchset:
        0.020365 +- 0.000261 seconds time elapsed  ( +-  1.28% )
        8382 calls of find_sym_by_name

The overhead is there, however, the linker is still fast for quite a
large number of BPF programs so it would probably cause a noticeable
slowdown only for BPF objects containing thousands of programs.

Another limitation is that aliases cannot be used in combination with
section name-based auto-attachment as the program only exists in a
single section.

Alternative solutions
=====================

As an alternative, bpftrace could use a similar approach to what
retsnoop [2] does: load the program just once and clone it for each
match using manual call of bpf_prog_load. This is certainly feasible but
requires each tool which wants to do this to reimplement the logic.

This patch series is an alternative to retsnoop's approach which I think
is easier to use from tools' perspective so I'm posting this RFC to see
what people think about it.

Viktor

[1] https://github.com/viktormalik/bpftrace/tree/alias-expansion
[2] https://github.com/anakryiko/retsnoop/blob/master/src/mass_attacher.c#L1096

Viktor Malik (3):
  libbpf: Support aliased symbols in linker
  libbpf: Handle relocations in aliased symbols
  selftests/bpf: Add tests for aliased programs

 tools/lib/bpf/libbpf.c                        | 143 ++++++++++--------
 tools/lib/bpf/linker.c                        |  68 +++++----
 .../selftests/bpf/prog_tests/fentry_alias.c   |  41 +++++
 .../selftests/bpf/progs/fentry_alias.c        |  83 ++++++++++
 4 files changed, 246 insertions(+), 89 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/fentry_alias.c
 create mode 100644 tools/testing/selftests/bpf/progs/fentry_alias.c

-- 
2.46.0

[RFC bpf-next 1/3] libbpf: Support aliased symbols in linker

[Viktor Malik]

It is possible to create multiple BPF programs sharing the same
instructions using the compiler `__attribute__((alias("...")))`:

    int BPF_PROG(prog)
    {
        [...]
    }
    int prog_alias() __attribute__((alias("prog")));

This may be convenient when creating multiple programs with the same
instruction set attached to different events (such as bpftrace does).

One problem in this situation is that Clang doesn't generate a BTF entry
for `prog_alias` which makes libbpf linker fail when processing such a
BPF object.

This commits adds support for that by finding another symbol at the same
address for which a BTF entry exists and using that entry in the linker.
This allows to use the linker (e.g. via `bpftool gen object ...`) on BPF
objects containing aliases.

Note that this won't be sufficient for most programs as we also need to
add support for handling relocations in the aliased programs. This will
be added by the following commit.

Signed-off-by: Viktor Malik <vmalik@redhat.com>
---
 tools/lib/bpf/linker.c | 68 +++++++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 30 deletions(-)

diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
index 9cd3d4109788..5ebc9ff1246e 100644
--- a/tools/lib/bpf/linker.c
+++ b/tools/lib/bpf/linker.c
@@ -1688,6 +1688,34 @@ static bool btf_is_non_static(const struct btf_type *t)
 	       || (btf_is_func(t) && btf_func_linkage(t) != BTF_FUNC_STATIC);
 }
 
+static Elf64_Sym *find_sym_by_name(struct src_obj *obj, size_t sec_idx,
+				   int sym_type, const char *sym_name)
+{
+	struct src_sec *symtab = &obj->secs[obj->symtab_sec_idx];
+	Elf64_Sym *sym = symtab->data->d_buf;
+	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
+	int str_sec_idx = symtab->shdr->sh_link;
+	const char *name;
+
+	for (i = 0; i < n; i++, sym++) {
+		if (sym->st_shndx != sec_idx)
+			continue;
+		if (ELF64_ST_TYPE(sym->st_info) != sym_type)
+			continue;
+
+		name = elf_strptr(obj->elf, str_sec_idx, sym->st_name);
+		if (!name)
+			return NULL;
+
+		if (strcmp(sym_name, name) != 0)
+			continue;
+
+		return sym;
+	}
+
+	return NULL;
+}
+
 static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sym_name,
 			     int *out_btf_sec_id, int *out_btf_id)
 {
@@ -1695,6 +1723,7 @@ static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sy
 	const struct btf_type *t;
 	const struct btf_var_secinfo *vi;
 	const char *name;
+	Elf64_Sym *s;
 
 	if (!obj->btf) {
 		pr_warn("failed to find BTF info for object '%s'\n", obj->filename);
@@ -1710,8 +1739,15 @@ static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sy
 		 */
 		if (btf_is_non_static(t)) {
 			name = btf__str_by_offset(obj->btf, t->name_off);
-			if (strcmp(name, sym_name) != 0)
-				continue;
+			if (strcmp(name, sym_name) != 0) {
+				/* the symbol that we look for may not have BTF as it may
+				 * be an alias of another symbol; we check if this is
+				 * the original symbol and if so, we use its BTF id
+				 */
+				s = find_sym_by_name(obj, sym->st_shndx, STT_FUNC, name);
+				if (!s || s->st_value != sym->st_value)
+					continue;
+			}
 
 			/* remember and still try to find DATASEC */
 			btf_id = i;
@@ -2132,34 +2168,6 @@ static int linker_append_elf_relos(struct bpf_linker *linker, struct src_obj *ob
 	return 0;
 }
 
-static Elf64_Sym *find_sym_by_name(struct src_obj *obj, size_t sec_idx,
-				   int sym_type, const char *sym_name)
-{
-	struct src_sec *symtab = &obj->secs[obj->symtab_sec_idx];
-	Elf64_Sym *sym = symtab->data->d_buf;
-	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
-	int str_sec_idx = symtab->shdr->sh_link;
-	const char *name;
-
-	for (i = 0; i < n; i++, sym++) {
-		if (sym->st_shndx != sec_idx)
-			continue;
-		if (ELF64_ST_TYPE(sym->st_info) != sym_type)
-			continue;
-
-		name = elf_strptr(obj->elf, str_sec_idx, sym->st_name);
-		if (!name)
-			return NULL;
-
-		if (strcmp(sym_name, name) != 0)
-			continue;
-
-		return sym;
-	}
-
-	return NULL;
-}
-
 static int linker_fixup_btf(struct src_obj *obj)
 {
 	const char *sec_name;
-- 
2.46.0

[RFC bpf-next 2/3] libbpf: Handle relocations in aliased symbols

[Viktor Malik]

It is possible to create multiple BPF programs sharing the same
instructions using the compiler `__attribute__((alias("...")))`:

    int BPF_PROG(prog)
    {
        [...]
    }
    int prog_alias() __attribute__((alias("prog")));

The problem is that libbpf currently assumes that there may not be
multiple BPF programs sharing the same instruction set in a BPF object
and therefore records relocations for a single (arbitrarily chosen)
program only.

This commit adds libbpf support for aliased programs by recording
relocations for all the aliased programs. The core of the change is
updating find_prog_by_sec_insn() to return a sequence of programs
instead of one and updating its users (where relevant) to perform
relocation recording for all of the returned programs. For BPF object
not containing aliased symbols, this works exactly as before without
additional overhead.

Currently, the main user of this feature is bpftrace which allows to
attach the same BPF program to many attach points. While this is simple
for k(u)probes using k(u)probe_multi, other program types (fentry/fexit,
(raw) tracepoints) do not have such features and currently require the
BPF object to contain a copy of the program for each attach point.

For example, considering the following bpftrace script collecting the
number of hits of each VFS function using fentry over a one second
period:

    $ bpftrace -e 'kfunc:vfs_* { @[func] = count() } i:s:1 { exit() }'
    [...]

this change will allow to reduce the size of the in-memory BPF object
that bpftrace generates from 60K to 9K.

Signed-off-by: Viktor Malik <vmalik@redhat.com>
---
 tools/lib/bpf/libbpf.c | 143 ++++++++++++++++++++++++-----------------
 1 file changed, 84 insertions(+), 59 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e55353887439..91afacede687 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4612,14 +4612,15 @@ static bool prog_contains_insn(const struct bpf_program *prog, size_t insn_idx)
 	       insn_idx < prog->sec_insn_off + prog->sec_insn_cnt;
 }
 
-static struct bpf_program *find_prog_by_sec_insn(const struct bpf_object *obj,
-						 size_t sec_idx, size_t insn_idx)
+static int find_progs_by_sec_insn(const struct bpf_object *obj,
+				     size_t sec_idx, size_t insn_idx,
+				     struct bpf_program **progs)
 {
-	int l = 0, r = obj->nr_programs - 1, m;
+	int l = 0, r = obj->nr_programs - 1, m, nr_progs = 0;
 	struct bpf_program *prog;
 
 	if (!obj->nr_programs)
-		return NULL;
+		return 0;
 
 	while (l < r) {
 		m = l + (r - l + 1) / 2;
@@ -4631,13 +4632,20 @@ static struct bpf_program *find_prog_by_sec_insn(const struct bpf_object *obj,
 		else
 			r = m - 1;
 	}
-	/* matching program could be at index l, but it still might be the
-	 * wrong one, so we need to double check conditions for the last time
+	/* there may be multiple (aliasing) programs sharing the same instructions,
+	 * index l will contain the last one so we search for the first one,
+	 * set *progs to point to it, and return the number of matching programs
+	 *
+	 * note that there may be no matching programs so we may return 0 here
 	 */
 	prog = &obj->programs[l];
-	if (prog->sec_idx == sec_idx && prog_contains_insn(prog, insn_idx))
-		return prog;
-	return NULL;
+	while (l >= 0 && prog->sec_idx == sec_idx && prog_contains_insn(prog, insn_idx)) {
+		prog = &obj->programs[--l];
+		nr_progs++;
+	}
+	if (nr_progs)
+		*progs = &obj->programs[l + 1];
+	return nr_progs;
 }
 
 static int
@@ -4647,7 +4655,7 @@ bpf_object__collect_prog_relos(struct bpf_object *obj, Elf64_Shdr *shdr, Elf_Dat
 	size_t sec_idx = shdr->sh_info, sym_idx;
 	struct bpf_program *prog;
 	struct reloc_desc *relos;
-	int err, i, nrels;
+	int err, i, j, nrels, nr_progs;
 	const char *sym_name;
 	__u32 insn_idx;
 	Elf_Scn *scn;
@@ -4715,27 +4723,33 @@ bpf_object__collect_prog_relos(struct bpf_object *obj, Elf64_Shdr *shdr, Elf_Dat
 		pr_debug("sec '%s': relo #%d: insn #%u against '%s'\n",
 			 relo_sec_name, i, insn_idx, sym_name);
 
-		prog = find_prog_by_sec_insn(obj, sec_idx, insn_idx);
-		if (!prog) {
+		nr_progs = find_progs_by_sec_insn(obj, sec_idx, insn_idx, &prog);
+		if (!nr_progs) {
 			pr_debug("sec '%s': relo #%d: couldn't find program in section '%s' for insn #%u, probably overridden weak function, skipping...\n",
 				relo_sec_name, i, sec_name, insn_idx);
 			continue;
 		}
 
-		relos = libbpf_reallocarray(prog->reloc_desc,
-					    prog->nr_reloc + 1, sizeof(*relos));
-		if (!relos)
-			return -ENOMEM;
-		prog->reloc_desc = relos;
-
-		/* adjust insn_idx to local BPF program frame of reference */
+		/* adjust insn_idx to local BPF program frame of reference
+		 * we can just do it once as all progs have the same sec_insn_off
+		 */
 		insn_idx -= prog->sec_insn_off;
-		err = bpf_program__record_reloc(prog, &relos[prog->nr_reloc],
-						insn_idx, sym_name, sym, rel);
-		if (err)
-			return err;
 
-		prog->nr_reloc++;
+		for (j = 0; j < nr_progs; j++, prog++) {
+			relos = libbpf_reallocarray(prog->reloc_desc,
+						    prog->nr_reloc + 1, sizeof(*relos));
+			if (!relos)
+				return -ENOMEM;
+			prog->reloc_desc = relos;
+
+			/* adjust insn_idx to local BPF program frame of reference */
+			err = bpf_program__record_reloc(prog, &relos[prog->nr_reloc],
+							insn_idx, sym_name, sym, rel);
+			if (err)
+				return err;
+
+			prog->nr_reloc++;
+		}
 	}
 	return 0;
 }
@@ -5861,7 +5875,7 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
 	struct bpf_program *prog;
 	struct bpf_insn *insn;
 	const char *sec_name;
-	int i, err = 0, insn_idx, sec_idx, sec_num;
+	int i, j, err = 0, insn_idx, sec_idx, sec_num, nr_progs;
 
 	if (obj->btf_ext->core_relo_info.len == 0)
 		return 0;
@@ -5899,8 +5913,8 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
 			if (rec->insn_off % BPF_INSN_SZ)
 				return -EINVAL;
 			insn_idx = rec->insn_off / BPF_INSN_SZ;
-			prog = find_prog_by_sec_insn(obj, sec_idx, insn_idx);
-			if (!prog) {
+			nr_progs = find_progs_by_sec_insn(obj, sec_idx, insn_idx, &prog);
+			if (!nr_progs) {
 				/* When __weak subprog is "overridden" by another instance
 				 * of the subprog from a different object file, linker still
 				 * appends all the .BTF.ext info that used to belong to that
@@ -5913,43 +5927,48 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
 					 sec_name, i, insn_idx);
 				continue;
 			}
-			/* no need to apply CO-RE relocation if the program is
-			 * not going to be loaded
-			 */
-			if (!prog->autoload)
-				continue;
 
 			/* adjust insn_idx from section frame of reference to the local
 			 * program's frame of reference; (sub-)program code is not yet
-			 * relocated, so it's enough to just subtract in-section offset
+			 * relocated, so it's enough to just subtract in-section offset;
+			 * we can just do it once as all progs have the same sec_insn_off
 			 */
 			insn_idx = insn_idx - prog->sec_insn_off;
 			if (insn_idx >= prog->insns_cnt)
 				return -EINVAL;
-			insn = &prog->insns[insn_idx];
 
-			err = record_relo_core(prog, rec, insn_idx);
-			if (err) {
-				pr_warn("prog '%s': relo #%d: failed to record relocation: %d\n",
-					prog->name, i, err);
-				goto out;
-			}
+			for (j = 0; j < nr_progs; j++, prog++) {
+				/* no need to apply CO-RE relocation if the program is
+				 * not going to be loaded
+				 */
+				if (!prog->autoload)
+					continue;
 
-			if (prog->obj->gen_loader)
-				continue;
+				insn = &prog->insns[insn_idx];
 
-			err = bpf_core_resolve_relo(prog, rec, i, obj->btf, cand_cache, &targ_res);
-			if (err) {
-				pr_warn("prog '%s': relo #%d: failed to relocate: %d\n",
-					prog->name, i, err);
-				goto out;
-			}
+				err = record_relo_core(prog, rec, insn_idx);
+				if (err) {
+					pr_warn("prog '%s': relo #%d: failed to record relocation: %d\n",
+						prog->name, i, err);
+					goto out;
+				}
 
-			err = bpf_core_patch_insn(prog->name, insn, insn_idx, rec, i, &targ_res);
-			if (err) {
-				pr_warn("prog '%s': relo #%d: failed to patch insn #%u: %d\n",
-					prog->name, i, insn_idx, err);
-				goto out;
+				if (prog->obj->gen_loader)
+					continue;
+
+				err = bpf_core_resolve_relo(prog, rec, i, obj->btf, cand_cache, &targ_res);
+				if (err) {
+					pr_warn("prog '%s': relo #%d: failed to relocate: %d\n",
+						prog->name, i, err);
+					goto out;
+				}
+
+				err = bpf_core_patch_insn(prog->name, insn, insn_idx, rec, i, &targ_res);
+				if (err) {
+					pr_warn("prog '%s': relo #%d: failed to patch insn #%u: %d\n",
+						prog->name, i, insn_idx, err);
+					goto out;
+				}
 			}
 		}
 	}
@@ -6350,7 +6369,7 @@ bpf_object__reloc_code(struct bpf_object *obj, struct bpf_program *main_prog,
 	struct bpf_program *subprog;
 	struct reloc_desc *relo;
 	struct bpf_insn *insn;
-	int err;
+	int err, nr_subprogs;
 
 	err = reloc_prog_func_and_line_info(obj, main_prog, prog);
 	if (err)
@@ -6406,12 +6425,15 @@ bpf_object__reloc_code(struct bpf_object *obj, struct bpf_program *main_prog,
 		}
 
 		/* we enforce that sub-programs should be in .text section */
-		subprog = find_prog_by_sec_insn(obj, obj->efile.text_shndx, sub_insn_idx);
-		if (!subprog) {
+		nr_subprogs = find_progs_by_sec_insn(obj, obj->efile.text_shndx, sub_insn_idx, &subprog);
+		if (!nr_subprogs) {
 			pr_warn("prog '%s': no .text section found yet sub-program call exists\n",
 				prog->name);
 			return -LIBBPF_ERRNO__RELOC;
 		}
+		/* there may be multiple aliasing subprogs but it doesn't matter
+		 * which one we use here as they must all have the same insns
+		 */
 
 		/* if it's the first call instruction calling into this
 		 * subprogram (meaning this subprog hasn't been processed
@@ -9725,7 +9747,7 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj,
 	__u32 member_idx;
 	Elf64_Sym *sym;
 	Elf64_Rel *rel;
-	int i, nrels;
+	int i, nrels, nr_progs;
 
 	btf = obj->btf;
 	nrels = shdr->sh_size / shdr->sh_entsize;
@@ -9789,12 +9811,15 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj,
 			return -EINVAL;
 		}
 
-		prog = find_prog_by_sec_insn(obj, shdr_idx, insn_idx);
-		if (!prog) {
+		nr_progs = find_progs_by_sec_insn(obj, shdr_idx, insn_idx, &prog);
+		if (!nr_progs) {
 			pr_warn("struct_ops reloc %s: cannot find prog at shdr_idx %u to relocate func ptr %s\n",
 				map->name, shdr_idx, name);
 			return -EINVAL;
 		}
+		/* there may be multiple aliasing progs but it doesn't matter
+		 * which one we use here as they must all have the same insns
+		 */
 
 		/* prevent the use of BPF prog with invalid type */
 		if (prog->type != BPF_PROG_TYPE_STRUCT_OPS) {
-- 
2.46.0

[RFC bpf-next 3/3] selftests/bpf: Add tests for aliased programs

[Viktor Malik]

The previous two commits added libbpf support for BPF objects containing
multiple programs sharing the same instructions using the compiler
`__attribute__((alias("...")))`.

This adds tests for such objects. The tests check that different kinds
of relocations (namely for global vars, maps, subprogs, and CO-RE) are
correctly performed in both the original and the aliasing BPF program
and that both programs hit when the target event occurs.

Signed-off-by: Viktor Malik <vmalik@redhat.com>
---
 .../selftests/bpf/prog_tests/fentry_alias.c   | 41 +++++++++
 .../selftests/bpf/progs/fentry_alias.c        | 83 +++++++++++++++++++
 2 files changed, 124 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/fentry_alias.c
 create mode 100644 tools/testing/selftests/bpf/progs/fentry_alias.c

diff --git a/tools/testing/selftests/bpf/prog_tests/fentry_alias.c b/tools/testing/selftests/bpf/prog_tests/fentry_alias.c
new file mode 100644
index 000000000000..6b9c6895b374
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/fentry_alias.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include "fentry_alias.skel.h"
+
+void test_fentry_alias(void)
+{
+	struct fentry_alias *fentry_skel = NULL;
+	int err, prog_fd;
+	__u64 key = 0, val;
+
+	LIBBPF_OPTS(bpf_test_run_opts, opts);
+
+	fentry_skel = fentry_alias__open_and_load();
+	if (!ASSERT_OK_PTR(fentry_skel, "fentry_skel_load"))
+		goto cleanup;
+
+	err = fentry_alias__attach(fentry_skel);
+	if (!ASSERT_OK(err, "fentry_attach"))
+		goto cleanup;
+
+	fentry_skel->bss->real_pid = getpid();
+
+	prog_fd = bpf_program__fd(fentry_skel->progs.test1);
+	err = bpf_prog_test_run_opts(prog_fd, &opts);
+
+	ASSERT_EQ(fentry_skel->bss->test1_hit_cnt, 2,
+		  "fentry_alias_test1_result");
+
+	err = bpf_map__lookup_elem(fentry_skel->maps.map, &key, sizeof(key),
+				   &val, sizeof(val), 0);
+	ASSERT_OK(err, "hashmap lookup");
+	ASSERT_EQ(val, 2, "fentry_alias_test2_result");
+
+	ASSERT_EQ(fentry_skel->bss->test3_hit_cnt, 2,
+		  "fentry_alias_test3_result");
+
+	ASSERT_EQ(fentry_skel->bss->test4_hit_cnt, 2,
+		  "fentry_alias_test4_result");
+cleanup:
+	fentry_alias__destroy(fentry_skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/fentry_alias.c b/tools/testing/selftests/bpf/progs/fentry_alias.c
new file mode 100644
index 000000000000..5d7b492c7f95
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/fentry_alias.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_core_read.h>
+
+char _license[] SEC("license") = "GPL";
+
+struct task_struct {
+	int tgid;
+} __attribute__((preserve_access_index));
+
+int real_pid = 0;
+
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(max_entries, 1);
+	__type(key, __u64);
+	__type(value, __u64);
+} map SEC(".maps");
+
+/* test1 - glob var relocations */
+__u64 test1_hit_cnt = 0;
+
+SEC("fentry/bpf_fentry_test1")
+int BPF_PROG(test1, int a)
+{
+	test1_hit_cnt++;
+	return 0;
+}
+
+int test1_alias(int a) __attribute__((alias("test1")));
+
+/* test2 - map relocations */
+SEC("fentry/bpf_fentry_test1")
+int BPF_PROG(test2, int a)
+{
+	__u64 key = 0, *value, new_value;
+
+	value = bpf_map_lookup_elem(&map, &key);
+	new_value = value ? *value + 1 : 1;
+	bpf_map_update_elem(&map, &key, &new_value, 0);
+	return 0;
+}
+
+int test2_alias(int a) __attribute__((alias("test2")));
+
+/* test3 - subprog relocations */
+__u64 test3_hit_cnt = 0;
+
+static __noinline void test3_subprog(void)
+{
+	test3_hit_cnt++;
+}
+
+SEC("fentry/bpf_fentry_test1")
+int BPF_PROG(test3, int a)
+{
+	test3_subprog();
+	return 0;
+}
+
+int test3_alias(int a) __attribute__((alias("test3")));
+
+/* test4 - CO-RE relocations */
+__u64 test4_hit_cnt = 0;
+
+SEC("fentry/bpf_fentry_test1")
+int BPF_PROG(test4, int a)
+{
+	struct task_struct *task;
+	int pid;
+
+	task = (void *)bpf_get_current_task();
+	pid = BPF_CORE_READ(task, tgid);
+
+	if (pid == real_pid)
+		test4_hit_cnt++;
+
+	return 0;
+}
+
+int test4_alias(int a) __attribute__((alias("test4")));
-- 
2.46.0

Re: [RFC bpf-next 0/3] libbpf: Add support for aliased BPF programs

[Alan Maguire]

On 02/09/2024 07:58, Viktor Malik wrote:
> TL;DR
> 
> This adds libbpf support for creating multiple BPF programs having the
> same instructions using symbol aliases.
> 
> Context
> =======
> 
> bpftrace has so-called "wildcarded" probes which allow to attach the
> same program to multple different attach points. For k(u)probes, this is
> easy to do as we can leverage k(u)probe_multi, however, other program
> types (fentry/fexit, tracepoints) don't have such features.
> 
> Currently, what bpftrace does is that it creates a copy of the program
> for each attach point. This naturally results in a lot of redundant code
> in the produced BPF object.
> 
> Proposal
> ========
> 
> One way to address this problem would be to use *symbol aliases*. In
> short, they allow to have multiple symbol table entries for the same
> address. In bpftrace, we would create them using llvm::GlobalAlias. In
> C, it can be achieved using compiler __attribute__((alias(...))):
> 
>     int BPF_PROG(prog)
>     {
>         [...]
>     }
>     int prog_alias() __attribute__((alias("prog")));
> 
> When calling bpf_object__open, libbpf is currently able to discover all
> the programs and internally does a separate copy of the instructions for
> each aliased program. What libbpf cannot do, is perform relocations b/c
> it assumes that each instruction belongs to a single program only. The
> second patch of this series changes relocation collection such that it
> records relocations for each aliased program. With that, bpftrace can
> emit just one copy of the full program and an alias for each target
> attach point.
> 
> For example, considering the following bpftrace script collecting the
> number of hits of each VFS function using fentry over a one second
> period:
> 
>     $ bpftrace -e 'kfunc:vfs_* { @[func] = count() } i:s:1 { exit() }'
>     [...]
> 
> this change will allow to reduce the size of the in-memory BPF object
> that bpftrace generates from 60K to 9K.
> 
> For reference, the bpftrace PoC is in [1].
> 
> The advantage of this change is that for BPF objects without aliases, it
> doesn't introduce any overhead.
> 

A few high-level questions - apologies in advance if I'm missing the
point here.

Could bpftrace use program linking to solve this issue instead? So we'd
have separate progs for the various attach points associated with vfs_*
functions, but they would all call the same global function. That
_should_ reduce the memory footprint of the object I think - or are
there issues with doing that? I also wonder if aliasing helps memory
footprint fully, especially if we end up with separate copies of the
program for relocation purposes; won't we have separate copies in-kernel
then too? So I _think_ the memory utilization you're concerned about is
not what's running in the kernel, but the BPF object representation in
bpftrace; is that right?

Thanks!

Alan

Re: [RFC bpf-next 0/3] libbpf: Add support for aliased BPF programs

[Viktor Malik]

On 9/2/24 19:01, Alan Maguire wrote:
> On 02/09/2024 07:58, Viktor Malik wrote:
>> TL;DR
>>
>> This adds libbpf support for creating multiple BPF programs having the
>> same instructions using symbol aliases.
>>
>> Context
>> =======
>>
>> bpftrace has so-called "wildcarded" probes which allow to attach the
>> same program to multple different attach points. For k(u)probes, this is
>> easy to do as we can leverage k(u)probe_multi, however, other program
>> types (fentry/fexit, tracepoints) don't have such features.
>>
>> Currently, what bpftrace does is that it creates a copy of the program
>> for each attach point. This naturally results in a lot of redundant code
>> in the produced BPF object.
>>
>> Proposal
>> ========
>>
>> One way to address this problem would be to use *symbol aliases*. In
>> short, they allow to have multiple symbol table entries for the same
>> address. In bpftrace, we would create them using llvm::GlobalAlias. In
>> C, it can be achieved using compiler __attribute__((alias(...))):
>>
>>     int BPF_PROG(prog)
>>     {
>>         [...]
>>     }
>>     int prog_alias() __attribute__((alias("prog")));
>>
>> When calling bpf_object__open, libbpf is currently able to discover all
>> the programs and internally does a separate copy of the instructions for
>> each aliased program. What libbpf cannot do, is perform relocations b/c
>> it assumes that each instruction belongs to a single program only. The
>> second patch of this series changes relocation collection such that it
>> records relocations for each aliased program. With that, bpftrace can
>> emit just one copy of the full program and an alias for each target
>> attach point.
>>
>> For example, considering the following bpftrace script collecting the
>> number of hits of each VFS function using fentry over a one second
>> period:
>>
>>     $ bpftrace -e 'kfunc:vfs_* { @[func] = count() } i:s:1 { exit() }'
>>     [...]
>>
>> this change will allow to reduce the size of the in-memory BPF object
>> that bpftrace generates from 60K to 9K.
>>
>> For reference, the bpftrace PoC is in [1].
>>
>> The advantage of this change is that for BPF objects without aliases, it
>> doesn't introduce any overhead.
>>
> 
> A few high-level questions - apologies in advance if I'm missing the
> point here.
> 
> Could bpftrace use program linking to solve this issue instead? So we'd
> have separate progs for the various attach points associated with vfs_*
> functions, but they would all call the same global function. That
> _should_ reduce the memory footprint of the object I think - or are
> there issues with doing that? 

That's a good suggestion, thanks! We added subprograms to bpftrace only
relatively recently so I didn't really think about this option. I'll
definitely give it a try as it could be even more efficient.

> I also wonder if aliasing helps memory
> footprint fully, especially if we end up with separate copies of the
> program for relocation purposes; won't we have separate copies in-kernel
> then too? So I _think_ the memory utilization you're concerned about is
> not what's running in the kernel, but the BPF object representation in
> bpftrace; is that right?

Yes, that is correct. libbpf will create a copy of the program for each
symbol in PROGBITS section that it discovers (including aliases) and the
copies will be loaded into kernel.

It's mainly the footprint of the BPF object produced by bpftrace that I
was concerned about. (The reason is that we work on ahead-of-time
compilation so it will directly affect the size of the pre-compiled
binaries). But the above solution using global subprograms should reduce
the in-kernel footprint, too, so I'll try to add it and see if it would
work for bpftrace.

Thanks!
Viktor

> 
> Thanks!
> 
> Alan
> 

Re: [RFC bpf-next 1/3] libbpf: Support aliased symbols in linker

[Jiri Olsa]

On Mon, Sep 02, 2024 at 08:58:01AM +0200, Viktor Malik wrote:
> It is possible to create multiple BPF programs sharing the same
> instructions using the compiler `__attribute__((alias("...")))`:
> 
>     int BPF_PROG(prog)
>     {
>         [...]
>     }
>     int prog_alias() __attribute__((alias("prog")));
> 
> This may be convenient when creating multiple programs with the same
> instruction set attached to different events (such as bpftrace does).
> 
> One problem in this situation is that Clang doesn't generate a BTF entry
> for `prog_alias` which makes libbpf linker fail when processing such a
> BPF object.

this might not solve all the issues, but could we change pahole to
generate BTF FUNC for alias function symbols?

jirka

> 
> This commits adds support for that by finding another symbol at the same
> address for which a BTF entry exists and using that entry in the linker.
> This allows to use the linker (e.g. via `bpftool gen object ...`) on BPF
> objects containing aliases.
> 
> Note that this won't be sufficient for most programs as we also need to
> add support for handling relocations in the aliased programs. This will
> be added by the following commit.
> 
> Signed-off-by: Viktor Malik <vmalik@redhat.com>
> ---
>  tools/lib/bpf/linker.c | 68 +++++++++++++++++++++++-------------------
>  1 file changed, 38 insertions(+), 30 deletions(-)
> 
> diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
> index 9cd3d4109788..5ebc9ff1246e 100644
> --- a/tools/lib/bpf/linker.c
> +++ b/tools/lib/bpf/linker.c
> @@ -1688,6 +1688,34 @@ static bool btf_is_non_static(const struct btf_type *t)
>  	       || (btf_is_func(t) && btf_func_linkage(t) != BTF_FUNC_STATIC);
>  }
>  
> +static Elf64_Sym *find_sym_by_name(struct src_obj *obj, size_t sec_idx,
> +				   int sym_type, const char *sym_name)
> +{
> +	struct src_sec *symtab = &obj->secs[obj->symtab_sec_idx];
> +	Elf64_Sym *sym = symtab->data->d_buf;
> +	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
> +	int str_sec_idx = symtab->shdr->sh_link;
> +	const char *name;
> +
> +	for (i = 0; i < n; i++, sym++) {
> +		if (sym->st_shndx != sec_idx)
> +			continue;
> +		if (ELF64_ST_TYPE(sym->st_info) != sym_type)
> +			continue;
> +
> +		name = elf_strptr(obj->elf, str_sec_idx, sym->st_name);
> +		if (!name)
> +			return NULL;
> +
> +		if (strcmp(sym_name, name) != 0)
> +			continue;
> +
> +		return sym;
> +	}
> +
> +	return NULL;
> +}
> +
>  static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sym_name,
>  			     int *out_btf_sec_id, int *out_btf_id)
>  {
> @@ -1695,6 +1723,7 @@ static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sy
>  	const struct btf_type *t;
>  	const struct btf_var_secinfo *vi;
>  	const char *name;
> +	Elf64_Sym *s;
>  
>  	if (!obj->btf) {
>  		pr_warn("failed to find BTF info for object '%s'\n", obj->filename);
> @@ -1710,8 +1739,15 @@ static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sy
>  		 */
>  		if (btf_is_non_static(t)) {
>  			name = btf__str_by_offset(obj->btf, t->name_off);
> -			if (strcmp(name, sym_name) != 0)
> -				continue;
> +			if (strcmp(name, sym_name) != 0) {
> +				/* the symbol that we look for may not have BTF as it may
> +				 * be an alias of another symbol; we check if this is
> +				 * the original symbol and if so, we use its BTF id
> +				 */
> +				s = find_sym_by_name(obj, sym->st_shndx, STT_FUNC, name);
> +				if (!s || s->st_value != sym->st_value)
> +					continue;
> +			}
>  
>  			/* remember and still try to find DATASEC */
>  			btf_id = i;
> @@ -2132,34 +2168,6 @@ static int linker_append_elf_relos(struct bpf_linker *linker, struct src_obj *ob
>  	return 0;
>  }
>  
> -static Elf64_Sym *find_sym_by_name(struct src_obj *obj, size_t sec_idx,
> -				   int sym_type, const char *sym_name)
> -{
> -	struct src_sec *symtab = &obj->secs[obj->symtab_sec_idx];
> -	Elf64_Sym *sym = symtab->data->d_buf;
> -	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
> -	int str_sec_idx = symtab->shdr->sh_link;
> -	const char *name;
> -
> -	for (i = 0; i < n; i++, sym++) {
> -		if (sym->st_shndx != sec_idx)
> -			continue;
> -		if (ELF64_ST_TYPE(sym->st_info) != sym_type)
> -			continue;
> -
> -		name = elf_strptr(obj->elf, str_sec_idx, sym->st_name);
> -		if (!name)
> -			return NULL;
> -
> -		if (strcmp(sym_name, name) != 0)
> -			continue;
> -
> -		return sym;
> -	}
> -
> -	return NULL;
> -}
> -
>  static int linker_fixup_btf(struct src_obj *obj)
>  {
>  	const char *sec_name;
> -- 
> 2.46.0
> 

Re: [RFC bpf-next 1/3] libbpf: Support aliased symbols in linker

[Viktor Malik]

On 9/3/24 13:16, Jiri Olsa wrote:
> On Mon, Sep 02, 2024 at 08:58:01AM +0200, Viktor Malik wrote:
>> It is possible to create multiple BPF programs sharing the same
>> instructions using the compiler `__attribute__((alias("...")))`:
>>
>>     int BPF_PROG(prog)
>>     {
>>         [...]
>>     }
>>     int prog_alias() __attribute__((alias("prog")));
>>
>> This may be convenient when creating multiple programs with the same
>> instruction set attached to different events (such as bpftrace does).
>>
>> One problem in this situation is that Clang doesn't generate a BTF entry
>> for `prog_alias` which makes libbpf linker fail when processing such a
>> BPF object.
> 
> this might not solve all the issues, but could we change pahole to
> generate BTF FUNC for alias function symbols?

I don't think that would work here. First, we don't usually run pahole
when building BPF objects, it's Clang which generates BTF for the "bpf"
target directly. Second, AFAIK, pahole converts DWARF to BTF and
compilers don't generate DWARF entries for alias function symbols either.

Viktor

> 
> jirka
> 
>>
>> This commits adds support for that by finding another symbol at the same
>> address for which a BTF entry exists and using that entry in the linker.
>> This allows to use the linker (e.g. via `bpftool gen object ...`) on BPF
>> objects containing aliases.
>>
>> Note that this won't be sufficient for most programs as we also need to
>> add support for handling relocations in the aliased programs. This will
>> be added by the following commit.
>>
>> Signed-off-by: Viktor Malik <vmalik@redhat.com>
>> ---
>>  tools/lib/bpf/linker.c | 68 +++++++++++++++++++++++-------------------
>>  1 file changed, 38 insertions(+), 30 deletions(-)
>>
>> diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
>> index 9cd3d4109788..5ebc9ff1246e 100644
>> --- a/tools/lib/bpf/linker.c
>> +++ b/tools/lib/bpf/linker.c
>> @@ -1688,6 +1688,34 @@ static bool btf_is_non_static(const struct btf_type *t)
>>  	       || (btf_is_func(t) && btf_func_linkage(t) != BTF_FUNC_STATIC);
>>  }
>>  
>> +static Elf64_Sym *find_sym_by_name(struct src_obj *obj, size_t sec_idx,
>> +				   int sym_type, const char *sym_name)
>> +{
>> +	struct src_sec *symtab = &obj->secs[obj->symtab_sec_idx];
>> +	Elf64_Sym *sym = symtab->data->d_buf;
>> +	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
>> +	int str_sec_idx = symtab->shdr->sh_link;
>> +	const char *name;
>> +
>> +	for (i = 0; i < n; i++, sym++) {
>> +		if (sym->st_shndx != sec_idx)
>> +			continue;
>> +		if (ELF64_ST_TYPE(sym->st_info) != sym_type)
>> +			continue;
>> +
>> +		name = elf_strptr(obj->elf, str_sec_idx, sym->st_name);
>> +		if (!name)
>> +			return NULL;
>> +
>> +		if (strcmp(sym_name, name) != 0)
>> +			continue;
>> +
>> +		return sym;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>>  static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sym_name,
>>  			     int *out_btf_sec_id, int *out_btf_id)
>>  {
>> @@ -1695,6 +1723,7 @@ static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sy
>>  	const struct btf_type *t;
>>  	const struct btf_var_secinfo *vi;
>>  	const char *name;
>> +	Elf64_Sym *s;
>>  
>>  	if (!obj->btf) {
>>  		pr_warn("failed to find BTF info for object '%s'\n", obj->filename);
>> @@ -1710,8 +1739,15 @@ static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sy
>>  		 */
>>  		if (btf_is_non_static(t)) {
>>  			name = btf__str_by_offset(obj->btf, t->name_off);
>> -			if (strcmp(name, sym_name) != 0)
>> -				continue;
>> +			if (strcmp(name, sym_name) != 0) {
>> +				/* the symbol that we look for may not have BTF as it may
>> +				 * be an alias of another symbol; we check if this is
>> +				 * the original symbol and if so, we use its BTF id
>> +				 */
>> +				s = find_sym_by_name(obj, sym->st_shndx, STT_FUNC, name);
>> +				if (!s || s->st_value != sym->st_value)
>> +					continue;
>> +			}
>>  
>>  			/* remember and still try to find DATASEC */
>>  			btf_id = i;
>> @@ -2132,34 +2168,6 @@ static int linker_append_elf_relos(struct bpf_linker *linker, struct src_obj *ob
>>  	return 0;
>>  }
>>  
>> -static Elf64_Sym *find_sym_by_name(struct src_obj *obj, size_t sec_idx,
>> -				   int sym_type, const char *sym_name)
>> -{
>> -	struct src_sec *symtab = &obj->secs[obj->symtab_sec_idx];
>> -	Elf64_Sym *sym = symtab->data->d_buf;
>> -	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
>> -	int str_sec_idx = symtab->shdr->sh_link;
>> -	const char *name;
>> -
>> -	for (i = 0; i < n; i++, sym++) {
>> -		if (sym->st_shndx != sec_idx)
>> -			continue;
>> -		if (ELF64_ST_TYPE(sym->st_info) != sym_type)
>> -			continue;
>> -
>> -		name = elf_strptr(obj->elf, str_sec_idx, sym->st_name);
>> -		if (!name)
>> -			return NULL;
>> -
>> -		if (strcmp(sym_name, name) != 0)
>> -			continue;
>> -
>> -		return sym;
>> -	}
>> -
>> -	return NULL;
>> -}
>> -
>>  static int linker_fixup_btf(struct src_obj *obj)
>>  {
>>  	const char *sec_name;
>> -- 
>> 2.46.0
>>
> 

Re: [RFC bpf-next 1/3] libbpf: Support aliased symbols in linker

[Arnaldo Carvalho de Melo]

On Tue, Sep 03, 2024 at 03:08:25PM +0200, Viktor Malik wrote:
> On 9/3/24 13:16, Jiri Olsa wrote:
> > On Mon, Sep 02, 2024 at 08:58:01AM +0200, Viktor Malik wrote:
> >> It is possible to create multiple BPF programs sharing the same
> >> instructions using the compiler `__attribute__((alias("...")))`:
> >>
> >>     int BPF_PROG(prog)
> >>     {
> >>         [...]
> >>     }
> >>     int prog_alias() __attribute__((alias("prog")));
> >>
> >> This may be convenient when creating multiple programs with the same
> >> instruction set attached to different events (such as bpftrace does).
> >>
> >> One problem in this situation is that Clang doesn't generate a BTF entry
> >> for `prog_alias` which makes libbpf linker fail when processing such a
> >> BPF object.
> > 
> > this might not solve all the issues, but could we change pahole to
> > generate BTF FUNC for alias function symbols?
> 
> I don't think that would work here. First, we don't usually run pahole
> when building BPF objects, it's Clang which generates BTF for the "bpf"
> target directly. Second, AFAIK, pahole converts DWARF to BTF and
> compilers don't generate DWARF entries for alias function symbols either.

So, pahole adds BTF info that doesn't come from DWARF, and it could read
the BTF generated by clang for the bpf target and ammend/augment/add to
it if that would allow us to have something we need and that isn't
currently (or planned) to be supported by clang.

I.e. we have a BTF loader, we could pair it with the BPF encoder, in
addition to the usual DWARF Loader + BPF encoder as what the loaders do
is to generate internal representation that is then consumed by the
pretty printer or the BTF encoder.

In this case the BTF encoder would use what is the internal
representation, that it doesn't even know came from a BPF object
generated by clang's BPF target, and would add this extra aliases, if we
can get it from somewhere else.

- Arnaldo
 
> Viktor
> 
> > 
> > jirka
> > 
> >>
> >> This commits adds support for that by finding another symbol at the same
> >> address for which a BTF entry exists and using that entry in the linker.
> >> This allows to use the linker (e.g. via `bpftool gen object ...`) on BPF
> >> objects containing aliases.
> >>
> >> Note that this won't be sufficient for most programs as we also need to
> >> add support for handling relocations in the aliased programs. This will
> >> be added by the following commit.
> >>
> >> Signed-off-by: Viktor Malik <vmalik@redhat.com>
> >> ---
> >>  tools/lib/bpf/linker.c | 68 +++++++++++++++++++++++-------------------
> >>  1 file changed, 38 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
> >> index 9cd3d4109788..5ebc9ff1246e 100644
> >> --- a/tools/lib/bpf/linker.c
> >> +++ b/tools/lib/bpf/linker.c
> >> @@ -1688,6 +1688,34 @@ static bool btf_is_non_static(const struct btf_type *t)
> >>  	       || (btf_is_func(t) && btf_func_linkage(t) != BTF_FUNC_STATIC);
> >>  }
> >>  
> >> +static Elf64_Sym *find_sym_by_name(struct src_obj *obj, size_t sec_idx,
> >> +				   int sym_type, const char *sym_name)
> >> +{
> >> +	struct src_sec *symtab = &obj->secs[obj->symtab_sec_idx];
> >> +	Elf64_Sym *sym = symtab->data->d_buf;
> >> +	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
> >> +	int str_sec_idx = symtab->shdr->sh_link;
> >> +	const char *name;
> >> +
> >> +	for (i = 0; i < n; i++, sym++) {
> >> +		if (sym->st_shndx != sec_idx)
> >> +			continue;
> >> +		if (ELF64_ST_TYPE(sym->st_info) != sym_type)
> >> +			continue;
> >> +
> >> +		name = elf_strptr(obj->elf, str_sec_idx, sym->st_name);
> >> +		if (!name)
> >> +			return NULL;
> >> +
> >> +		if (strcmp(sym_name, name) != 0)
> >> +			continue;
> >> +
> >> +		return sym;
> >> +	}
> >> +
> >> +	return NULL;
> >> +}
> >> +
> >>  static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sym_name,
> >>  			     int *out_btf_sec_id, int *out_btf_id)
> >>  {
> >> @@ -1695,6 +1723,7 @@ static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sy
> >>  	const struct btf_type *t;
> >>  	const struct btf_var_secinfo *vi;
> >>  	const char *name;
> >> +	Elf64_Sym *s;
> >>  
> >>  	if (!obj->btf) {
> >>  		pr_warn("failed to find BTF info for object '%s'\n", obj->filename);
> >> @@ -1710,8 +1739,15 @@ static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sy
> >>  		 */
> >>  		if (btf_is_non_static(t)) {
> >>  			name = btf__str_by_offset(obj->btf, t->name_off);
> >> -			if (strcmp(name, sym_name) != 0)
> >> -				continue;
> >> +			if (strcmp(name, sym_name) != 0) {
> >> +				/* the symbol that we look for may not have BTF as it may
> >> +				 * be an alias of another symbol; we check if this is
> >> +				 * the original symbol and if so, we use its BTF id
> >> +				 */
> >> +				s = find_sym_by_name(obj, sym->st_shndx, STT_FUNC, name);
> >> +				if (!s || s->st_value != sym->st_value)
> >> +					continue;
> >> +			}
> >>  
> >>  			/* remember and still try to find DATASEC */
> >>  			btf_id = i;
> >> @@ -2132,34 +2168,6 @@ static int linker_append_elf_relos(struct bpf_linker *linker, struct src_obj *ob
> >>  	return 0;
> >>  }
> >>  
> >> -static Elf64_Sym *find_sym_by_name(struct src_obj *obj, size_t sec_idx,
> >> -				   int sym_type, const char *sym_name)
> >> -{
> >> -	struct src_sec *symtab = &obj->secs[obj->symtab_sec_idx];
> >> -	Elf64_Sym *sym = symtab->data->d_buf;
> >> -	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
> >> -	int str_sec_idx = symtab->shdr->sh_link;
> >> -	const char *name;
> >> -
> >> -	for (i = 0; i < n; i++, sym++) {
> >> -		if (sym->st_shndx != sec_idx)
> >> -			continue;
> >> -		if (ELF64_ST_TYPE(sym->st_info) != sym_type)
> >> -			continue;
> >> -
> >> -		name = elf_strptr(obj->elf, str_sec_idx, sym->st_name);
> >> -		if (!name)
> >> -			return NULL;
> >> -
> >> -		if (strcmp(sym_name, name) != 0)
> >> -			continue;
> >> -
> >> -		return sym;
> >> -	}
> >> -
> >> -	return NULL;
> >> -}
> >> -
> >>  static int linker_fixup_btf(struct src_obj *obj)
> >>  {
> >>  	const char *sec_name;
> >> -- 
> >> 2.46.0
> >>
> > 

Re: [RFC bpf-next 1/3] libbpf: Support aliased symbols in linker

[Jiri Olsa]

On Tue, Sep 03, 2024 at 03:08:25PM +0200, Viktor Malik wrote:
> On 9/3/24 13:16, Jiri Olsa wrote:
> > On Mon, Sep 02, 2024 at 08:58:01AM +0200, Viktor Malik wrote:
> >> It is possible to create multiple BPF programs sharing the same
> >> instructions using the compiler `__attribute__((alias("...")))`:
> >>
> >>     int BPF_PROG(prog)
> >>     {
> >>         [...]
> >>     }
> >>     int prog_alias() __attribute__((alias("prog")));
> >>
> >> This may be convenient when creating multiple programs with the same
> >> instruction set attached to different events (such as bpftrace does).
> >>
> >> One problem in this situation is that Clang doesn't generate a BTF entry
> >> for `prog_alias` which makes libbpf linker fail when processing such a
> >> BPF object.
> > 
> > this might not solve all the issues, but could we change pahole to
> > generate BTF FUNC for alias function symbols?
> 
> I don't think that would work here. First, we don't usually run pahole
> when building BPF objects, it's Clang which generates BTF for the "bpf"
> target directly. Second, AFAIK, pahole converts DWARF to BTF and
> compilers don't generate DWARF entries for alias function symbols either.

ah ok, sry I misunderstood the purpose.. it's about the bpf code
generated in bpftrace

jirka

Re: [RFC bpf-next 0/3] libbpf: Add support for aliased BPF programs

[Alexei Starovoitov]

On Mon, Sep 2, 2024 at 10:57 PM Viktor Malik <vmalik@redhat.com> wrote:
>
> On 9/2/24 19:01, Alan Maguire wrote:
> > On 02/09/2024 07:58, Viktor Malik wrote:
> >> TL;DR
> >>
> >> This adds libbpf support for creating multiple BPF programs having the
> >> same instructions using symbol aliases.
> >>
> >> Context
> >> =======
> >>
> >> bpftrace has so-called "wildcarded" probes which allow to attach the
> >> same program to multple different attach points. For k(u)probes, this is
> >> easy to do as we can leverage k(u)probe_multi, however, other program
> >> types (fentry/fexit, tracepoints) don't have such features.
> >>
> >> Currently, what bpftrace does is that it creates a copy of the program
> >> for each attach point. This naturally results in a lot of redundant code
> >> in the produced BPF object.
> >>
> >> Proposal
> >> ========
> >>
> >> One way to address this problem would be to use *symbol aliases*. In
> >> short, they allow to have multiple symbol table entries for the same
> >> address. In bpftrace, we would create them using llvm::GlobalAlias. In
> >> C, it can be achieved using compiler __attribute__((alias(...))):
> >>
> >>     int BPF_PROG(prog)
> >>     {
> >>         [...]
> >>     }
> >>     int prog_alias() __attribute__((alias("prog")));
> >>
> >> When calling bpf_object__open, libbpf is currently able to discover all
> >> the programs and internally does a separate copy of the instructions for
> >> each aliased program. What libbpf cannot do, is perform relocations b/c
> >> it assumes that each instruction belongs to a single program only. The
> >> second patch of this series changes relocation collection such that it
> >> records relocations for each aliased program. With that, bpftrace can
> >> emit just one copy of the full program and an alias for each target
> >> attach point.
> >>
> >> For example, considering the following bpftrace script collecting the
> >> number of hits of each VFS function using fentry over a one second
> >> period:
> >>
> >>     $ bpftrace -e 'kfunc:vfs_* { @[func] = count() } i:s:1 { exit() }'
> >>     [...]
> >>
> >> this change will allow to reduce the size of the in-memory BPF object
> >> that bpftrace generates from 60K to 9K.
> >>
> >> For reference, the bpftrace PoC is in [1].
> >>
> >> The advantage of this change is that for BPF objects without aliases, it
> >> doesn't introduce any overhead.
> >>
> >
> > A few high-level questions - apologies in advance if I'm missing the
> > point here.
> >
> > Could bpftrace use program linking to solve this issue instead? So we'd
> > have separate progs for the various attach points associated with vfs_*
> > functions, but they would all call the same global function. That
> > _should_ reduce the memory footprint of the object I think - or are
> > there issues with doing that?
>
> That's a good suggestion, thanks! We added subprograms to bpftrace only
> relatively recently so I didn't really think about this option. I'll
> definitely give it a try as it could be even more efficient.
>
> > I also wonder if aliasing helps memory
> > footprint fully, especially if we end up with separate copies of the
> > program for relocation purposes; won't we have separate copies in-kernel
> > then too? So I _think_ the memory utilization you're concerned about is
> > not what's running in the kernel, but the BPF object representation in
> > bpftrace; is that right?
>
> Yes, that is correct. libbpf will create a copy of the program for each
> symbol in PROGBITS section that it discovers (including aliases) and the
> copies will be loaded into kernel.
>
> It's mainly the footprint of the BPF object produced by bpftrace that I
> was concerned about. (The reason is that we work on ahead-of-time
> compilation so it will directly affect the size of the pre-compiled
> binaries). But the above solution using global subprograms should reduce
> the in-kernel footprint, too, so I'll try to add it and see if it would
> work for bpftrace.

I think it's a half solution, since prog will be duplicated anyway and
loaded multiple times into the kernel.

I think it's better to revive this patch sets:

v1:
https://lore.kernel.org/bpf/20240220035105.34626-1-dongmenglong.8@bytedance.com/

v2:
https://lore.kernel.org/bpf/20240311093526.1010158-1-dongmenglong.8@bytedance.com/

Unfortunately it went off rails, because we went deep into trying
to save bpf trampoline memory too.

I think it can still land.
We can load fentry program once and attach it in multiple places
with many bpf links.
That is still worth doing.

I think it should solve retsnoop and bpftrace use cases.
Not perfect, since there will be many trampolines, but still an improvement.

Re: [RFC bpf-next 1/3] libbpf: Support aliased symbols in linker

[Viktor Malik]

On 9/3/24 16:08, Arnaldo Carvalho de Melo wrote:
> On Tue, Sep 03, 2024 at 03:08:25PM +0200, Viktor Malik wrote:
>> On 9/3/24 13:16, Jiri Olsa wrote:
>>> On Mon, Sep 02, 2024 at 08:58:01AM +0200, Viktor Malik wrote:
>>>> It is possible to create multiple BPF programs sharing the same
>>>> instructions using the compiler `__attribute__((alias("...")))`:
>>>>
>>>>     int BPF_PROG(prog)
>>>>     {
>>>>         [...]
>>>>     }
>>>>     int prog_alias() __attribute__((alias("prog")));
>>>>
>>>> This may be convenient when creating multiple programs with the same
>>>> instruction set attached to different events (such as bpftrace does).
>>>>
>>>> One problem in this situation is that Clang doesn't generate a BTF entry
>>>> for `prog_alias` which makes libbpf linker fail when processing such a
>>>> BPF object.
>>>
>>> this might not solve all the issues, but could we change pahole to
>>> generate BTF FUNC for alias function symbols?
>>
>> I don't think that would work here. First, we don't usually run pahole
>> when building BPF objects, it's Clang which generates BTF for the "bpf"
>> target directly. Second, AFAIK, pahole converts DWARF to BTF and
>> compilers don't generate DWARF entries for alias function symbols either.
> 
> So, pahole adds BTF info that doesn't come from DWARF, and it could read
> the BTF generated by clang for the bpf target and ammend/augment/add to
> it if that would allow us to have something we need and that isn't
> currently (or planned) to be supported by clang.
> 
> I.e. we have a BTF loader, we could pair it with the BPF encoder, in
> addition to the usual DWARF Loader + BPF encoder as what the loaders do
> is to generate internal representation that is then consumed by the
> pretty printer or the BTF encoder.
> 
> In this case the BTF encoder would use what is the internal
> representation, that it doesn't even know came from a BPF object
> generated by clang's BPF target, and would add this extra aliases, if we
> can get it from somewhere else.

Yes, that would be an option. We should be able to get the info about
aliases from the symbol table.

The problem here is that it would require an extra step of running
pahole when compiling BPF objects. Which should be ok for bpftrace,
though, as we control how we create BPF objects and can easily add the
extra step.

Viktor

> 
> - Arnaldo
>  
>> Viktor
>>
>>>
>>> jirka
>>>
>>>>
>>>> This commits adds support for that by finding another symbol at the same
>>>> address for which a BTF entry exists and using that entry in the linker.
>>>> This allows to use the linker (e.g. via `bpftool gen object ...`) on BPF
>>>> objects containing aliases.
>>>>
>>>> Note that this won't be sufficient for most programs as we also need to
>>>> add support for handling relocations in the aliased programs. This will
>>>> be added by the following commit.
>>>>
>>>> Signed-off-by: Viktor Malik <vmalik@redhat.com>
>>>> ---
>>>>  tools/lib/bpf/linker.c | 68 +++++++++++++++++++++++-------------------
>>>>  1 file changed, 38 insertions(+), 30 deletions(-)
>>>>
>>>> diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
>>>> index 9cd3d4109788..5ebc9ff1246e 100644
>>>> --- a/tools/lib/bpf/linker.c
>>>> +++ b/tools/lib/bpf/linker.c
>>>> @@ -1688,6 +1688,34 @@ static bool btf_is_non_static(const struct btf_type *t)
>>>>  	       || (btf_is_func(t) && btf_func_linkage(t) != BTF_FUNC_STATIC);
>>>>  }
>>>>  
>>>> +static Elf64_Sym *find_sym_by_name(struct src_obj *obj, size_t sec_idx,
>>>> +				   int sym_type, const char *sym_name)
>>>> +{
>>>> +	struct src_sec *symtab = &obj->secs[obj->symtab_sec_idx];
>>>> +	Elf64_Sym *sym = symtab->data->d_buf;
>>>> +	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
>>>> +	int str_sec_idx = symtab->shdr->sh_link;
>>>> +	const char *name;
>>>> +
>>>> +	for (i = 0; i < n; i++, sym++) {
>>>> +		if (sym->st_shndx != sec_idx)
>>>> +			continue;
>>>> +		if (ELF64_ST_TYPE(sym->st_info) != sym_type)
>>>> +			continue;
>>>> +
>>>> +		name = elf_strptr(obj->elf, str_sec_idx, sym->st_name);
>>>> +		if (!name)
>>>> +			return NULL;
>>>> +
>>>> +		if (strcmp(sym_name, name) != 0)
>>>> +			continue;
>>>> +
>>>> +		return sym;
>>>> +	}
>>>> +
>>>> +	return NULL;
>>>> +}
>>>> +
>>>>  static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sym_name,
>>>>  			     int *out_btf_sec_id, int *out_btf_id)
>>>>  {
>>>> @@ -1695,6 +1723,7 @@ static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sy
>>>>  	const struct btf_type *t;
>>>>  	const struct btf_var_secinfo *vi;
>>>>  	const char *name;
>>>> +	Elf64_Sym *s;
>>>>  
>>>>  	if (!obj->btf) {
>>>>  		pr_warn("failed to find BTF info for object '%s'\n", obj->filename);
>>>> @@ -1710,8 +1739,15 @@ static int find_glob_sym_btf(struct src_obj *obj, Elf64_Sym *sym, const char *sy
>>>>  		 */
>>>>  		if (btf_is_non_static(t)) {
>>>>  			name = btf__str_by_offset(obj->btf, t->name_off);
>>>> -			if (strcmp(name, sym_name) != 0)
>>>> -				continue;
>>>> +			if (strcmp(name, sym_name) != 0) {
>>>> +				/* the symbol that we look for may not have BTF as it may
>>>> +				 * be an alias of another symbol; we check if this is
>>>> +				 * the original symbol and if so, we use its BTF id
>>>> +				 */
>>>> +				s = find_sym_by_name(obj, sym->st_shndx, STT_FUNC, name);
>>>> +				if (!s || s->st_value != sym->st_value)
>>>> +					continue;
>>>> +			}
>>>>  
>>>>  			/* remember and still try to find DATASEC */
>>>>  			btf_id = i;
>>>> @@ -2132,34 +2168,6 @@ static int linker_append_elf_relos(struct bpf_linker *linker, struct src_obj *ob
>>>>  	return 0;
>>>>  }
>>>>  
>>>> -static Elf64_Sym *find_sym_by_name(struct src_obj *obj, size_t sec_idx,
>>>> -				   int sym_type, const char *sym_name)
>>>> -{
>>>> -	struct src_sec *symtab = &obj->secs[obj->symtab_sec_idx];
>>>> -	Elf64_Sym *sym = symtab->data->d_buf;
>>>> -	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
>>>> -	int str_sec_idx = symtab->shdr->sh_link;
>>>> -	const char *name;
>>>> -
>>>> -	for (i = 0; i < n; i++, sym++) {
>>>> -		if (sym->st_shndx != sec_idx)
>>>> -			continue;
>>>> -		if (ELF64_ST_TYPE(sym->st_info) != sym_type)
>>>> -			continue;
>>>> -
>>>> -		name = elf_strptr(obj->elf, str_sec_idx, sym->st_name);
>>>> -		if (!name)
>>>> -			return NULL;
>>>> -
>>>> -		if (strcmp(sym_name, name) != 0)
>>>> -			continue;
>>>> -
>>>> -		return sym;
>>>> -	}
>>>> -
>>>> -	return NULL;
>>>> -}
>>>> -
>>>>  static int linker_fixup_btf(struct src_obj *obj)
>>>>  {
>>>>  	const char *sec_name;
>>>> -- 
>>>> 2.46.0
>>>>
>>>
> 

Re: [RFC bpf-next 0/3] libbpf: Add support for aliased BPF programs

[Andrii Nakryiko]

On Tue, Sep 3, 2024 at 1:19 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Sep 2, 2024 at 10:57 PM Viktor Malik <vmalik@redhat.com> wrote:
> >
> > On 9/2/24 19:01, Alan Maguire wrote:
> > > On 02/09/2024 07:58, Viktor Malik wrote:
> > >> TL;DR
> > >>
> > >> This adds libbpf support for creating multiple BPF programs having the
> > >> same instructions using symbol aliases.
> > >>
> > >> Context
> > >> =======
> > >>
> > >> bpftrace has so-called "wildcarded" probes which allow to attach the
> > >> same program to multple different attach points. For k(u)probes, this is
> > >> easy to do as we can leverage k(u)probe_multi, however, other program
> > >> types (fentry/fexit, tracepoints) don't have such features.
> > >>
> > >> Currently, what bpftrace does is that it creates a copy of the program
> > >> for each attach point. This naturally results in a lot of redundant code
> > >> in the produced BPF object.
> > >>
> > >> Proposal
> > >> ========
> > >>
> > >> One way to address this problem would be to use *symbol aliases*. In
> > >> short, they allow to have multiple symbol table entries for the same
> > >> address. In bpftrace, we would create them using llvm::GlobalAlias. In
> > >> C, it can be achieved using compiler __attribute__((alias(...))):
> > >>
> > >>     int BPF_PROG(prog)
> > >>     {
> > >>         [...]
> > >>     }
> > >>     int prog_alias() __attribute__((alias("prog")));
> > >>
> > >> When calling bpf_object__open, libbpf is currently able to discover all
> > >> the programs and internally does a separate copy of the instructions for
> > >> each aliased program. What libbpf cannot do, is perform relocations b/c
> > >> it assumes that each instruction belongs to a single program only. The
> > >> second patch of this series changes relocation collection such that it
> > >> records relocations for each aliased program. With that, bpftrace can
> > >> emit just one copy of the full program and an alias for each target
> > >> attach point.
> > >>
> > >> For example, considering the following bpftrace script collecting the
> > >> number of hits of each VFS function using fentry over a one second
> > >> period:
> > >>
> > >>     $ bpftrace -e 'kfunc:vfs_* { @[func] = count() } i:s:1 { exit() }'
> > >>     [...]
> > >>
> > >> this change will allow to reduce the size of the in-memory BPF object
> > >> that bpftrace generates from 60K to 9K.

Tbh, I'm not too keen on adding this aliasing complication just for
this. It seems a bit too intrusive for something so obscure.

retsnoop doesn't need this and bypasses the issue by cloning with
bpf_prog_load(), can bpftrace follow the same model? If we need to add
some getters to bpf_program() to make this easier, that sounds like a
better long-term investment, API-wise.

> > >>
> > >> For reference, the bpftrace PoC is in [1].
> > >>
> > >> The advantage of this change is that for BPF objects without aliases, it
> > >> doesn't introduce any overhead.
> > >>
> > >
> > > A few high-level questions - apologies in advance if I'm missing the
> > > point here.
> > >
> > > Could bpftrace use program linking to solve this issue instead? So we'd
> > > have separate progs for the various attach points associated with vfs_*
> > > functions, but they would all call the same global function. That
> > > _should_ reduce the memory footprint of the object I think - or are
> > > there issues with doing that?
> >
> > That's a good suggestion, thanks! We added subprograms to bpftrace only
> > relatively recently so I didn't really think about this option. I'll
> > definitely give it a try as it could be even more efficient.
> >
> > > I also wonder if aliasing helps memory
> > > footprint fully, especially if we end up with separate copies of the
> > > program for relocation purposes; won't we have separate copies in-kernel
> > > then too? So I _think_ the memory utilization you're concerned about is
> > > not what's running in the kernel, but the BPF object representation in
> > > bpftrace; is that right?
> >
> > Yes, that is correct. libbpf will create a copy of the program for each
> > symbol in PROGBITS section that it discovers (including aliases) and the
> > copies will be loaded into kernel.
> >
> > It's mainly the footprint of the BPF object produced by bpftrace that I
> > was concerned about. (The reason is that we work on ahead-of-time
> > compilation so it will directly affect the size of the pre-compiled
> > binaries). But the above solution using global subprograms should reduce
> > the in-kernel footprint, too, so I'll try to add it and see if it would
> > work for bpftrace.
>
> I think it's a half solution, since prog will be duplicated anyway and
> loaded multiple times into the kernel.
>
> I think it's better to revive this patch sets:
>
> v1:
> https://lore.kernel.org/bpf/20240220035105.34626-1-dongmenglong.8@bytedance.com/
>
> v2:
> https://lore.kernel.org/bpf/20240311093526.1010158-1-dongmenglong.8@bytedance.com/
>
> Unfortunately it went off rails, because we went deep into trying
> to save bpf trampoline memory too.
>

+1 to adding multi-attach fentry, even if it will have to duplicate trampolines

> I think it can still land.
> We can load fentry program once and attach it in multiple places
> with many bpf links.
> That is still worth doing.

This part I'm confused about, but we can postpone discussion until
someone actually works on this.

>
> I think it should solve retsnoop and bpftrace use cases.
> Not perfect, since there will be many trampolines, but still an improvement.
>

retsnoop is fine already through bpf_prog_load()-based cloning, but
faster attachment for fentry/fexit would definitely help.

Re: [RFC bpf-next 0/3] libbpf: Add support for aliased BPF programs

[Viktor Malik]

On 9/4/24 21:07, Andrii Nakryiko wrote:
> On Tue, Sep 3, 2024 at 1:19 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>>
>> On Mon, Sep 2, 2024 at 10:57 PM Viktor Malik <vmalik@redhat.com> wrote:
>>>
>>> On 9/2/24 19:01, Alan Maguire wrote:
>>>> On 02/09/2024 07:58, Viktor Malik wrote:
>>>>> TL;DR
>>>>>
>>>>> This adds libbpf support for creating multiple BPF programs having the
>>>>> same instructions using symbol aliases.
>>>>>
>>>>> Context
>>>>> =======
>>>>>
>>>>> bpftrace has so-called "wildcarded" probes which allow to attach the
>>>>> same program to multple different attach points. For k(u)probes, this is
>>>>> easy to do as we can leverage k(u)probe_multi, however, other program
>>>>> types (fentry/fexit, tracepoints) don't have such features.
>>>>>
>>>>> Currently, what bpftrace does is that it creates a copy of the program
>>>>> for each attach point. This naturally results in a lot of redundant code
>>>>> in the produced BPF object.
>>>>>
>>>>> Proposal
>>>>> ========
>>>>>
>>>>> One way to address this problem would be to use *symbol aliases*. In
>>>>> short, they allow to have multiple symbol table entries for the same
>>>>> address. In bpftrace, we would create them using llvm::GlobalAlias. In
>>>>> C, it can be achieved using compiler __attribute__((alias(...))):
>>>>>
>>>>>     int BPF_PROG(prog)
>>>>>     {
>>>>>         [...]
>>>>>     }
>>>>>     int prog_alias() __attribute__((alias("prog")));
>>>>>
>>>>> When calling bpf_object__open, libbpf is currently able to discover all
>>>>> the programs and internally does a separate copy of the instructions for
>>>>> each aliased program. What libbpf cannot do, is perform relocations b/c
>>>>> it assumes that each instruction belongs to a single program only. The
>>>>> second patch of this series changes relocation collection such that it
>>>>> records relocations for each aliased program. With that, bpftrace can
>>>>> emit just one copy of the full program and an alias for each target
>>>>> attach point.
>>>>>
>>>>> For example, considering the following bpftrace script collecting the
>>>>> number of hits of each VFS function using fentry over a one second
>>>>> period:
>>>>>
>>>>>     $ bpftrace -e 'kfunc:vfs_* { @[func] = count() } i:s:1 { exit() }'
>>>>>     [...]
>>>>>
>>>>> this change will allow to reduce the size of the in-memory BPF object
>>>>> that bpftrace generates from 60K to 9K.
> 
> Tbh, I'm not too keen on adding this aliasing complication just for
> this. It seems a bit too intrusive for something so obscure.
> 
> retsnoop doesn't need this and bypasses the issue by cloning with
> bpf_prog_load(), can bpftrace follow the same model? If we need to add
> some getters to bpf_program() to make this easier, that sounds like a
> better long-term investment, API-wise.

Yes, as I already mentioned, it should be possible to use cloning via
bpf_prog_load(). The aliasing approach just seemed simpler from tool's
perspective - you just emit one alias for each clone and libbpf takes
care of the rest. But I admit that I anticipated the change to be much
simpler than it turned out to be.

Let me try add the cloning and I'll see if we could add something to
libbpf to make it simpler.

> 
>>>>>
>>>>> For reference, the bpftrace PoC is in [1].
>>>>>
>>>>> The advantage of this change is that for BPF objects without aliases, it
>>>>> doesn't introduce any overhead.
>>>>>
>>>>
>>>> A few high-level questions - apologies in advance if I'm missing the
>>>> point here.
>>>>
>>>> Could bpftrace use program linking to solve this issue instead? So we'd
>>>> have separate progs for the various attach points associated with vfs_*
>>>> functions, but they would all call the same global function. That
>>>> _should_ reduce the memory footprint of the object I think - or are
>>>> there issues with doing that?
>>>
>>> That's a good suggestion, thanks! We added subprograms to bpftrace only
>>> relatively recently so I didn't really think about this option. I'll
>>> definitely give it a try as it could be even more efficient.
>>>
>>>> I also wonder if aliasing helps memory
>>>> footprint fully, especially if we end up with separate copies of the
>>>> program for relocation purposes; won't we have separate copies in-kernel
>>>> then too? So I _think_ the memory utilization you're concerned about is
>>>> not what's running in the kernel, but the BPF object representation in
>>>> bpftrace; is that right?
>>>
>>> Yes, that is correct. libbpf will create a copy of the program for each
>>> symbol in PROGBITS section that it discovers (including aliases) and the
>>> copies will be loaded into kernel.
>>>
>>> It's mainly the footprint of the BPF object produced by bpftrace that I
>>> was concerned about. (The reason is that we work on ahead-of-time
>>> compilation so it will directly affect the size of the pre-compiled
>>> binaries). But the above solution using global subprograms should reduce
>>> the in-kernel footprint, too, so I'll try to add it and see if it would
>>> work for bpftrace.
>>
>> I think it's a half solution, since prog will be duplicated anyway and
>> loaded multiple times into the kernel.
>>
>> I think it's better to revive this patch sets:
>>
>> v1:
>> https://lore.kernel.org/bpf/20240220035105.34626-1-dongmenglong.8@bytedance.com/
>>
>> v2:
>> https://lore.kernel.org/bpf/20240311093526.1010158-1-dongmenglong.8@bytedance.com/
>>
>> Unfortunately it went off rails, because we went deep into trying
>> to save bpf trampoline memory too.
>>
> 
> +1 to adding multi-attach fentry, even if it will have to duplicate trampolines

That would resolve fentry but the same problem still exists for (raw)
tracepoints. In bpftrace, you may want to do something like
`tracepoint:syscalls:sys_enter_* { ... }` and at the moment, we still
need to the cloning in such case.

But still, it does solve a part of the problem and could be useful for
bpftrace to reduce the memory footprint for wildcarded fentry probes.
I'll try to find some time to give this a shot (unless someone beats me
to it).

Viktor

> 
>> I think it can still land.
>> We can load fentry program once and attach it in multiple places
>> with many bpf links.
>> That is still worth doing.
> 
> This part I'm confused about, but we can postpone discussion until
> someone actually works on this.
> 
>>
>> I think it should solve retsnoop and bpftrace use cases.
>> Not perfect, since there will be many trampolines, but still an improvement.
>>
> 
> retsnoop is fine already through bpf_prog_load()-based cloning, but
> faster attachment for fentry/fexit would definitely help.
> 

Re: [RFC bpf-next 0/3] libbpf: Add support for aliased BPF programs

[Alexei Starovoitov]

On Thu, Sep 5, 2024 at 10:04 PM Viktor Malik <vmalik@redhat.com> wrote:
>
> >
> > +1 to adding multi-attach fentry, even if it will have to duplicate trampolines
>
> That would resolve fentry but the same problem still exists for (raw)
> tracepoints. In bpftrace, you may want to do something like
> `tracepoint:syscalls:sys_enter_* { ... }` and at the moment, we still
> need to the cloning in such case.

Multi-tracepoint support in the kernel would be nice too.
Probably much easier to add than multi-fentry.

> But still, it does solve a part of the problem and could be useful for
> bpftrace to reduce the memory footprint for wildcarded fentry probes.
> I'll try to find some time to give this a shot (unless someone beats me
> to it).

Pls go ahead. I'm not aware of anyone working on it.

Re: [RFC bpf-next 0/3] libbpf: Add support for aliased BPF programs

[Andrii Nakryiko]

On Thu, Sep 5, 2024 at 10:04 PM Viktor Malik <vmalik@redhat.com> wrote:
>
> On 9/4/24 21:07, Andrii Nakryiko wrote:
> > On Tue, Sep 3, 2024 at 1:19 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> >>
> >> On Mon, Sep 2, 2024 at 10:57 PM Viktor Malik <vmalik@redhat.com> wrote:
> >>>
> >>> On 9/2/24 19:01, Alan Maguire wrote:
> >>>> On 02/09/2024 07:58, Viktor Malik wrote:
> >>>>> TL;DR
> >>>>>
> >>>>> This adds libbpf support for creating multiple BPF programs having the
> >>>>> same instructions using symbol aliases.
> >>>>>
> >>>>> Context
> >>>>> =======
> >>>>>
> >>>>> bpftrace has so-called "wildcarded" probes which allow to attach the
> >>>>> same program to multple different attach points. For k(u)probes, this is
> >>>>> easy to do as we can leverage k(u)probe_multi, however, other program
> >>>>> types (fentry/fexit, tracepoints) don't have such features.
> >>>>>
> >>>>> Currently, what bpftrace does is that it creates a copy of the program
> >>>>> for each attach point. This naturally results in a lot of redundant code
> >>>>> in the produced BPF object.
> >>>>>
> >>>>> Proposal
> >>>>> ========
> >>>>>
> >>>>> One way to address this problem would be to use *symbol aliases*. In
> >>>>> short, they allow to have multiple symbol table entries for the same
> >>>>> address. In bpftrace, we would create them using llvm::GlobalAlias. In
> >>>>> C, it can be achieved using compiler __attribute__((alias(...))):
> >>>>>
> >>>>>     int BPF_PROG(prog)
> >>>>>     {
> >>>>>         [...]
> >>>>>     }
> >>>>>     int prog_alias() __attribute__((alias("prog")));
> >>>>>
> >>>>> When calling bpf_object__open, libbpf is currently able to discover all
> >>>>> the programs and internally does a separate copy of the instructions for
> >>>>> each aliased program. What libbpf cannot do, is perform relocations b/c
> >>>>> it assumes that each instruction belongs to a single program only. The
> >>>>> second patch of this series changes relocation collection such that it
> >>>>> records relocations for each aliased program. With that, bpftrace can
> >>>>> emit just one copy of the full program and an alias for each target
> >>>>> attach point.
> >>>>>
> >>>>> For example, considering the following bpftrace script collecting the
> >>>>> number of hits of each VFS function using fentry over a one second
> >>>>> period:
> >>>>>
> >>>>>     $ bpftrace -e 'kfunc:vfs_* { @[func] = count() } i:s:1 { exit() }'
> >>>>>     [...]
> >>>>>
> >>>>> this change will allow to reduce the size of the in-memory BPF object
> >>>>> that bpftrace generates from 60K to 9K.
> >
> > Tbh, I'm not too keen on adding this aliasing complication just for
> > this. It seems a bit too intrusive for something so obscure.
> >
> > retsnoop doesn't need this and bypasses the issue by cloning with
> > bpf_prog_load(), can bpftrace follow the same model? If we need to add
> > some getters to bpf_program() to make this easier, that sounds like a
> > better long-term investment, API-wise.
>
> Yes, as I already mentioned, it should be possible to use cloning via
> bpf_prog_load(). The aliasing approach just seemed simpler from tool's
> perspective - you just emit one alias for each clone and libbpf takes
> care of the rest. But I admit that I anticipated the change to be much
> simpler than it turned out to be.
>
> Let me try add the cloning and I'll see if we could add something to
> libbpf to make it simpler.
>
> >
> >>>>>
> >>>>> For reference, the bpftrace PoC is in [1].
> >>>>>
> >>>>> The advantage of this change is that for BPF objects without aliases, it
> >>>>> doesn't introduce any overhead.
> >>>>>
> >>>>
> >>>> A few high-level questions - apologies in advance if I'm missing the
> >>>> point here.
> >>>>
> >>>> Could bpftrace use program linking to solve this issue instead? So we'd
> >>>> have separate progs for the various attach points associated with vfs_*
> >>>> functions, but they would all call the same global function. That
> >>>> _should_ reduce the memory footprint of the object I think - or are
> >>>> there issues with doing that?
> >>>
> >>> That's a good suggestion, thanks! We added subprograms to bpftrace only
> >>> relatively recently so I didn't really think about this option. I'll
> >>> definitely give it a try as it could be even more efficient.
> >>>
> >>>> I also wonder if aliasing helps memory
> >>>> footprint fully, especially if we end up with separate copies of the
> >>>> program for relocation purposes; won't we have separate copies in-kernel
> >>>> then too? So I _think_ the memory utilization you're concerned about is
> >>>> not what's running in the kernel, but the BPF object representation in
> >>>> bpftrace; is that right?
> >>>
> >>> Yes, that is correct. libbpf will create a copy of the program for each
> >>> symbol in PROGBITS section that it discovers (including aliases) and the
> >>> copies will be loaded into kernel.
> >>>
> >>> It's mainly the footprint of the BPF object produced by bpftrace that I
> >>> was concerned about. (The reason is that we work on ahead-of-time
> >>> compilation so it will directly affect the size of the pre-compiled
> >>> binaries). But the above solution using global subprograms should reduce
> >>> the in-kernel footprint, too, so I'll try to add it and see if it would
> >>> work for bpftrace.
> >>
> >> I think it's a half solution, since prog will be duplicated anyway and
> >> loaded multiple times into the kernel.
> >>
> >> I think it's better to revive this patch sets:
> >>
> >> v1:
> >> https://lore.kernel.org/bpf/20240220035105.34626-1-dongmenglong.8@bytedance.com/
> >>
> >> v2:
> >> https://lore.kernel.org/bpf/20240311093526.1010158-1-dongmenglong.8@bytedance.com/
> >>
> >> Unfortunately it went off rails, because we went deep into trying
> >> to save bpf trampoline memory too.
> >>
> >
> > +1 to adding multi-attach fentry, even if it will have to duplicate trampolines
>
> That would resolve fentry but the same problem still exists for (raw)
> tracepoints. In bpftrace, you may want to do something like
> `tracepoint:syscalls:sys_enter_* { ... }` and at the moment, we still
> need to the cloning in such case.

There are two problems for fentry: loading and attaching/detaching.
Given how fentry programs need to provide different attach_btf_id
during *load*, the load is the challenging part that requires cloning.

Then there is an attachment/detachment, which is just slow, but not
problematic logistically. Multi-fentry would solve/mitigate both.

But for tracepoints the load problem doesn't exist. You can load one
single instance of a program, and then attach multiple times to
different tracepoints. So cloning is not needed for tracepoints *at
all*.

If you want to know which tracepoint exactly you are processing at
runtime, use BPF cookie.

If you mean BTF-enabled raw tracepoints, then it's a separate thing
(and yes, there is a similar problem to fentry, of course).

But particularly for syscalls:sys_enter_*, I suspect the best solution
would be to attach generic sys_enter raw tracepoint and filter in
bpftrace by syscall number. Then it would be easy load, fast attach,
and most probably not worse runtime performance.

>
> But still, it does solve a part of the problem and could be useful for
> bpftrace to reduce the memory footprint for wildcarded fentry probes.
> I'll try to find some time to give this a shot (unless someone beats me
> to it).
>
> Viktor
>
> >
> >> I think it can still land.
> >> We can load fentry program once and attach it in multiple places
> >> with many bpf links.
> >> That is still worth doing.
> >
> > This part I'm confused about, but we can postpone discussion until
> > someone actually works on this.
> >
> >>
> >> I think it should solve retsnoop and bpftrace use cases.
> >> Not perfect, since there will be many trampolines, but still an improvement.
> >>
> >
> > retsnoop is fine already through bpf_prog_load()-based cloning, but
> > faster attachment for fentry/fexit would definitely help.
> >
>