From 53df63ccdc0258118e53089197d0428c5330cc9c Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Mon, 13 Sep 2021 18:47:30 -0700 Subject: [PATCH 1/4] selftests/bpf: Update selftests to always provide "struct_ops" SEC Update struct_ops selftests to always specify "struct_ops" section prefix. Libbpf will require a proper BPF program type set in the next patch, so this prevents tests breaking. Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Acked-by: Martin KaFai Lau Link: https://lore.kernel.org/bpf/20210914014733.2768-2-andrii@kernel.org --- tools/testing/selftests/bpf/progs/bpf_cubic.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/tools/testing/selftests/bpf/progs/bpf_cubic.c b/tools/testing/selftests/bpf/progs/bpf_cubic.c index f62df4d023f9..d9660e7200e2 100644 --- a/tools/testing/selftests/bpf/progs/bpf_cubic.c +++ b/tools/testing/selftests/bpf/progs/bpf_cubic.c @@ -169,11 +169,7 @@ static __always_inline void bictcp_hystart_reset(struct sock *sk) ca->sample_cnt = 0; } -/* "struct_ops/" prefix is not a requirement - * It will be recognized as BPF_PROG_TYPE_STRUCT_OPS - * as long as it is used in one of the func ptr - * under SEC(".struct_ops"). - */ +/* "struct_ops/" prefix is a requirement */ SEC("struct_ops/bpf_cubic_init") void BPF_PROG(bpf_cubic_init, struct sock *sk) { @@ -188,10 +184,8 @@ void BPF_PROG(bpf_cubic_init, struct sock *sk) tcp_sk(sk)->snd_ssthresh = initial_ssthresh; } -/* No prefix in SEC will also work. - * The remaining tcp-cubic functions have an easier way. - */ -SEC("no-sec-prefix-bictcp_cwnd_event") +/* "struct_ops" prefix is a requirement */ +SEC("struct_ops/bpf_cubic_cwnd_event") void BPF_PROG(bpf_cubic_cwnd_event, struct sock *sk, enum tcp_ca_event event) { if (event == CA_EVENT_TX_START) { From 91b4d1d1d54431c72f3a7ff034f30a635f787426 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Mon, 13 Sep 2021 18:47:31 -0700 Subject: [PATCH 2/4] libbpf: Ensure BPF prog types are set before relocations Refactor bpf_object__open() sequencing to perform BPF program type detection based on SEC() definitions before we get to relocations collection. This allows to have more information about BPF program by the time we get to, say, struct_ops relocation gathering. This, subsequently, simplifies struct_ops logic and removes the need to perform extra find_sec_def() resolution. With this patch libbpf will require all struct_ops BPF programs to be marked with SEC("struct_ops") or SEC("struct_ops/xxx") annotations. Real-world applications are already doing that through something like selftests's BPF_STRUCT_OPS() macro. This change streamlines libbpf's internal handling of SEC() definitions and is in the sprit of upcoming libbpf-1.0 section strictness changes ([0]). [0] https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#stricter-and-more-uniform-bpf-program-section-name-sec-handling Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Acked-by: Martin KaFai Lau Link: https://lore.kernel.org/bpf/20210914014733.2768-3-andrii@kernel.org --- tools/lib/bpf/libbpf.c | 93 +++++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 42 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 9d99183bffba..6abfbf283092 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -6373,12 +6373,37 @@ bpf_object__load_progs(struct bpf_object *obj, int log_level) static const struct bpf_sec_def *find_sec_def(const char *sec_name); +static int bpf_object_init_progs(struct bpf_object *obj, const struct bpf_object_open_opts *opts) +{ + struct bpf_program *prog; + + bpf_object__for_each_program(prog, obj) { + prog->sec_def = find_sec_def(prog->sec_name); + if (!prog->sec_def) { + /* couldn't guess, but user might manually specify */ + pr_debug("prog '%s': unrecognized ELF section name '%s'\n", + prog->name, prog->sec_name); + continue; + } + + if (prog->sec_def->is_sleepable) + prog->prog_flags |= BPF_F_SLEEPABLE; + bpf_program__set_type(prog, prog->sec_def->prog_type); + bpf_program__set_expected_attach_type(prog, prog->sec_def->expected_attach_type); + + if (prog->sec_def->prog_type == BPF_PROG_TYPE_TRACING || + prog->sec_def->prog_type == BPF_PROG_TYPE_EXT) + prog->attach_prog_fd = OPTS_GET(opts, attach_prog_fd, 0); + } + + return 0; +} + static struct bpf_object * __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz, const struct bpf_object_open_opts *opts) { const char *obj_name, *kconfig, *btf_tmp_path; - struct bpf_program *prog; struct bpf_object *obj; char tmp_name[64]; int err; @@ -6436,31 +6461,13 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz, err = err ? : bpf_object__collect_externs(obj); err = err ? : bpf_object__finalize_btf(obj); err = err ? : bpf_object__init_maps(obj, opts); + err = err ? : bpf_object_init_progs(obj, opts); err = err ? : bpf_object__collect_relos(obj); if (err) goto out; + bpf_object__elf_finish(obj); - bpf_object__for_each_program(prog, obj) { - prog->sec_def = find_sec_def(prog->sec_name); - if (!prog->sec_def) { - /* couldn't guess, but user might manually specify */ - pr_debug("prog '%s': unrecognized ELF section name '%s'\n", - prog->name, prog->sec_name); - continue; - } - - if (prog->sec_def->is_sleepable) - prog->prog_flags |= BPF_F_SLEEPABLE; - bpf_program__set_type(prog, prog->sec_def->prog_type); - bpf_program__set_expected_attach_type(prog, - prog->sec_def->expected_attach_type); - - if (prog->sec_def->prog_type == BPF_PROG_TYPE_TRACING || - prog->sec_def->prog_type == BPF_PROG_TYPE_EXT) - prog->attach_prog_fd = OPTS_GET(opts, attach_prog_fd, 0); - } - return obj; out: bpf_object__close(obj); @@ -8250,35 +8257,37 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj, return -EINVAL; } - if (prog->type == BPF_PROG_TYPE_UNSPEC) { - const struct bpf_sec_def *sec_def; + /* prevent the use of BPF prog with invalid type */ + if (prog->type != BPF_PROG_TYPE_STRUCT_OPS) { + pr_warn("struct_ops reloc %s: prog %s is not struct_ops BPF program\n", + map->name, prog->name); + return -EINVAL; + } - sec_def = find_sec_def(prog->sec_name); - if (sec_def && - sec_def->prog_type != BPF_PROG_TYPE_STRUCT_OPS) { - /* for pr_warn */ - prog->type = sec_def->prog_type; - goto invalid_prog; - } - - prog->type = BPF_PROG_TYPE_STRUCT_OPS; + /* if we haven't yet processed this BPF program, record proper + * attach_btf_id and member_idx + */ + if (!prog->attach_btf_id) { prog->attach_btf_id = st_ops->type_id; prog->expected_attach_type = member_idx; - } else if (prog->type != BPF_PROG_TYPE_STRUCT_OPS || - prog->attach_btf_id != st_ops->type_id || - prog->expected_attach_type != member_idx) { - goto invalid_prog; } + + /* struct_ops BPF prog can be re-used between multiple + * .struct_ops as long as it's the same struct_ops struct + * definition and the same function pointer field + */ + if (prog->attach_btf_id != st_ops->type_id || + prog->expected_attach_type != member_idx) { + pr_warn("struct_ops reloc %s: cannot use prog %s in sec %s with type %u attach_btf_id %u expected_attach_type %u for func ptr %s\n", + map->name, prog->name, prog->sec_name, prog->type, + prog->attach_btf_id, prog->expected_attach_type, name); + return -EINVAL; + } + st_ops->progs[member_idx] = prog; } return 0; - -invalid_prog: - pr_warn("struct_ops reloc %s: cannot use prog %s in sec %s with type %u attach_btf_id %u expected_attach_type %u for func ptr %s\n", - map->name, prog->name, prog->sec_name, prog->type, - prog->attach_btf_id, prog->expected_attach_type, name); - return -EINVAL; } #define BTF_TRACE_PREFIX "btf_trace_" From 5532dfd42e4846e84d346a6dfe01e477e35baa65 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Mon, 13 Sep 2021 18:47:32 -0700 Subject: [PATCH 3/4] libbpf: Simplify BPF program auto-attach code Remove the need to explicitly pass bpf_sec_def for auto-attachable BPF programs, as it is already recorded at bpf_object__open() time for all recognized type of BPF programs. This further reduces number of explicit calls to find_sec_def(), simplifying further refactorings. No functional changes are done by this patch. Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Acked-by: Martin KaFai Lau Link: https://lore.kernel.org/bpf/20210914014733.2768-4-andrii@kernel.org --- tools/lib/bpf/libbpf.c | 61 +++++++++++++++--------------------------- 1 file changed, 22 insertions(+), 39 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 6abfbf283092..e473fbd2b361 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -218,8 +218,7 @@ struct reloc_desc { struct bpf_sec_def; -typedef struct bpf_link *(*attach_fn_t)(const struct bpf_sec_def *sec, - struct bpf_program *prog); +typedef struct bpf_link *(*attach_fn_t)(struct bpf_program *prog); struct bpf_sec_def { const char *sec; @@ -7920,18 +7919,12 @@ void bpf_program__set_expected_attach_type(struct bpf_program *prog, __VA_ARGS__ \ } -static struct bpf_link *attach_kprobe(const struct bpf_sec_def *sec, - struct bpf_program *prog); -static struct bpf_link *attach_tp(const struct bpf_sec_def *sec, - struct bpf_program *prog); -static struct bpf_link *attach_raw_tp(const struct bpf_sec_def *sec, - struct bpf_program *prog); -static struct bpf_link *attach_trace(const struct bpf_sec_def *sec, - struct bpf_program *prog); -static struct bpf_link *attach_lsm(const struct bpf_sec_def *sec, - struct bpf_program *prog); -static struct bpf_link *attach_iter(const struct bpf_sec_def *sec, - struct bpf_program *prog); +static struct bpf_link *attach_kprobe(struct bpf_program *prog); +static struct bpf_link *attach_tp(struct bpf_program *prog); +static struct bpf_link *attach_raw_tp(struct bpf_program *prog); +static struct bpf_link *attach_trace(struct bpf_program *prog); +static struct bpf_link *attach_lsm(struct bpf_program *prog); +static struct bpf_link *attach_iter(struct bpf_program *prog); static const struct bpf_sec_def section_defs[] = { BPF_PROG_SEC("socket", BPF_PROG_TYPE_SOCKET_FILTER), @@ -9391,8 +9384,7 @@ struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog, return bpf_program__attach_kprobe_opts(prog, func_name, &opts); } -static struct bpf_link *attach_kprobe(const struct bpf_sec_def *sec, - struct bpf_program *prog) +static struct bpf_link *attach_kprobe(struct bpf_program *prog) { DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, opts); unsigned long offset = 0; @@ -9401,8 +9393,8 @@ static struct bpf_link *attach_kprobe(const struct bpf_sec_def *sec, char *func; int n, err; - func_name = prog->sec_name + sec->len; - opts.retprobe = strcmp(sec->sec, "kretprobe/") == 0; + func_name = prog->sec_name + prog->sec_def->len; + opts.retprobe = strcmp(prog->sec_def->sec, "kretprobe/") == 0; n = sscanf(func_name, "%m[a-zA-Z0-9_.]+%li", &func, &offset); if (n < 1) { @@ -9565,8 +9557,7 @@ struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog, return bpf_program__attach_tracepoint_opts(prog, tp_category, tp_name, NULL); } -static struct bpf_link *attach_tp(const struct bpf_sec_def *sec, - struct bpf_program *prog) +static struct bpf_link *attach_tp(struct bpf_program *prog) { char *sec_name, *tp_cat, *tp_name; struct bpf_link *link; @@ -9576,7 +9567,7 @@ static struct bpf_link *attach_tp(const struct bpf_sec_def *sec, return libbpf_err_ptr(-ENOMEM); /* extract "tp//" */ - tp_cat = sec_name + sec->len; + tp_cat = sec_name + prog->sec_def->len; tp_name = strchr(tp_cat, '/'); if (!tp_name) { free(sec_name); @@ -9620,10 +9611,9 @@ struct bpf_link *bpf_program__attach_raw_tracepoint(struct bpf_program *prog, return link; } -static struct bpf_link *attach_raw_tp(const struct bpf_sec_def *sec, - struct bpf_program *prog) +static struct bpf_link *attach_raw_tp(struct bpf_program *prog) { - const char *tp_name = prog->sec_name + sec->len; + const char *tp_name = prog->sec_name + prog->sec_def->len; return bpf_program__attach_raw_tracepoint(prog, tp_name); } @@ -9668,14 +9658,12 @@ struct bpf_link *bpf_program__attach_lsm(struct bpf_program *prog) return bpf_program__attach_btf_id(prog); } -static struct bpf_link *attach_trace(const struct bpf_sec_def *sec, - struct bpf_program *prog) +static struct bpf_link *attach_trace(struct bpf_program *prog) { return bpf_program__attach_trace(prog); } -static struct bpf_link *attach_lsm(const struct bpf_sec_def *sec, - struct bpf_program *prog) +static struct bpf_link *attach_lsm(struct bpf_program *prog) { return bpf_program__attach_lsm(prog); } @@ -9806,21 +9794,17 @@ bpf_program__attach_iter(struct bpf_program *prog, return link; } -static struct bpf_link *attach_iter(const struct bpf_sec_def *sec, - struct bpf_program *prog) +static struct bpf_link *attach_iter(struct bpf_program *prog) { return bpf_program__attach_iter(prog, NULL); } struct bpf_link *bpf_program__attach(struct bpf_program *prog) { - const struct bpf_sec_def *sec_def; - - sec_def = find_sec_def(prog->sec_name); - if (!sec_def || !sec_def->attach_fn) + if (!prog->sec_def || !prog->sec_def->attach_fn) return libbpf_err_ptr(-ESRCH); - return sec_def->attach_fn(sec_def, prog); + return prog->sec_def->attach_fn(prog); } static int bpf_link__detach_struct_ops(struct bpf_link *link) @@ -10899,16 +10883,15 @@ int bpf_object__attach_skeleton(struct bpf_object_skeleton *s) for (i = 0; i < s->prog_cnt; i++) { struct bpf_program *prog = *s->progs[i].prog; struct bpf_link **link = s->progs[i].link; - const struct bpf_sec_def *sec_def; if (!prog->load) continue; - sec_def = find_sec_def(prog->sec_name); - if (!sec_def || !sec_def->attach_fn) + /* auto-attaching not supported for this program */ + if (!prog->sec_def || !prog->sec_def->attach_fn) continue; - *link = sec_def->attach_fn(sec_def, prog); + *link = prog->sec_def->attach_fn(prog); err = libbpf_get_error(*link); if (err) { pr_warn("failed to auto-attach program '%s': %d\n", From b6291a6f30d35bd4459dc35aac2f30669a4356ac Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Mon, 13 Sep 2021 18:47:33 -0700 Subject: [PATCH 4/4] libbpf: Minimize explicit iterator of section definition array Remove almost all the code that explicitly iterated BPF program section definitions in favor of using find_sec_def(). The only remaining user of section_defs is libbpf_get_type_names that has to iterate all of them to construct its result. Having one internal API entry point for section definitions will simplify further refactorings around libbpf's program section definitions parsing. Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Acked-by: Martin KaFai Lau Link: https://lore.kernel.org/bpf/20210914014733.2768-5-andrii@kernel.org --- tools/lib/bpf/libbpf.c | 46 +++++++++++++++++------------------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index e473fbd2b361..62ed073ea944 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -8438,22 +8438,13 @@ static int libbpf_find_attach_btf_id(struct bpf_program *prog, int *btf_obj_fd, __u32 attach_prog_fd = prog->attach_prog_fd; const char *name = prog->sec_name, *attach_name; const struct bpf_sec_def *sec = NULL; - int i, err = 0; + int err = 0; if (!name) return -EINVAL; - for (i = 0; i < ARRAY_SIZE(section_defs); i++) { - if (!section_defs[i].is_attach_btf) - continue; - if (strncmp(name, section_defs[i].sec, section_defs[i].len)) - continue; - - sec = §ion_defs[i]; - break; - } - - if (!sec) { + sec = find_sec_def(name); + if (!sec || !sec->is_attach_btf) { pr_warn("failed to identify BTF ID based on ELF section name '%s'\n", name); return -ESRCH; } @@ -8491,27 +8482,28 @@ int libbpf_attach_type_by_name(const char *name, enum bpf_attach_type *attach_type) { char *type_names; - int i; + const struct bpf_sec_def *sec_def; if (!name) return libbpf_err(-EINVAL); - for (i = 0; i < ARRAY_SIZE(section_defs); i++) { - if (strncmp(name, section_defs[i].sec, section_defs[i].len)) - continue; - if (!section_defs[i].is_attachable) - return libbpf_err(-EINVAL); - *attach_type = section_defs[i].expected_attach_type; - return 0; - } - pr_debug("failed to guess attach type based on ELF section name '%s'\n", name); - type_names = libbpf_get_type_names(true); - if (type_names != NULL) { - pr_debug("attachable section(type) names are:%s\n", type_names); - free(type_names); + sec_def = find_sec_def(name); + if (!sec_def) { + pr_debug("failed to guess attach type based on ELF section name '%s'\n", name); + type_names = libbpf_get_type_names(true); + if (type_names != NULL) { + pr_debug("attachable section(type) names are:%s\n", type_names); + free(type_names); + } + + return libbpf_err(-EINVAL); } - return libbpf_err(-EINVAL); + if (!sec_def->is_attachable) + return libbpf_err(-EINVAL); + + *attach_type = sec_def->expected_attach_type; + return 0; } int bpf_map__fd(const struct bpf_map *map)