* [PATCH bpf-next 1/6] bpf: fix bpf_put_raw_tracepoint()'s use of __module_address()
2020-11-19 23:22 [PATCH bpf-next 0/6] libbpf: add support for kernel module BTF CO-RE relocations Andrii Nakryiko
@ 2020-11-19 23:22 ` Andrii Nakryiko
2020-11-20 17:55 ` Martin KaFai Lau
2020-11-24 5:49 ` Alexei Starovoitov
2020-11-19 23:22 ` [PATCH bpf-next 2/6] libbpf: add internal helper to load BTF data by FD Andrii Nakryiko
` (4 subsequent siblings)
5 siblings, 2 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2020-11-19 23:22 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team
__module_address() needs to be called with preemption disabled or with
module_mutex taken. preempt_disable() is enough for read-only uses, which is
what this fix does.
Fixes: a38d1107f937 ("bpf: support raw tracepoints in modules")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
kernel/trace/bpf_trace.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d255bc9b2bfa..bb98a377050a 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2060,7 +2060,11 @@ struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char *name)
void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp)
{
- struct module *mod = __module_address((unsigned long)btp);
+ struct module *mod;
+
+ preempt_disable();
+ mod = __module_address((unsigned long)btp);
+ preempt_enable();
if (mod)
module_put(mod);
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next 1/6] bpf: fix bpf_put_raw_tracepoint()'s use of __module_address()
2020-11-19 23:22 ` [PATCH bpf-next 1/6] bpf: fix bpf_put_raw_tracepoint()'s use of __module_address() Andrii Nakryiko
@ 2020-11-20 17:55 ` Martin KaFai Lau
2020-11-24 5:49 ` Alexei Starovoitov
1 sibling, 0 replies; 21+ messages in thread
From: Martin KaFai Lau @ 2020-11-20 17:55 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, netdev, ast, daniel, kernel-team
On Thu, Nov 19, 2020 at 03:22:39PM -0800, Andrii Nakryiko wrote:
> __module_address() needs to be called with preemption disabled or with
> module_mutex taken. preempt_disable() is enough for read-only uses, which is
> what this fix does.
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next 1/6] bpf: fix bpf_put_raw_tracepoint()'s use of __module_address()
2020-11-19 23:22 ` [PATCH bpf-next 1/6] bpf: fix bpf_put_raw_tracepoint()'s use of __module_address() Andrii Nakryiko
2020-11-20 17:55 ` Martin KaFai Lau
@ 2020-11-24 5:49 ` Alexei Starovoitov
2020-11-24 7:49 ` Andrii Nakryiko
1 sibling, 1 reply; 21+ messages in thread
From: Alexei Starovoitov @ 2020-11-24 5:49 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, netdev, ast, daniel, kernel-team
On Thu, Nov 19, 2020 at 03:22:39PM -0800, Andrii Nakryiko wrote:
> __module_address() needs to be called with preemption disabled or with
> module_mutex taken. preempt_disable() is enough for read-only uses, which is
> what this fix does.
>
> Fixes: a38d1107f937 ("bpf: support raw tracepoints in modules")
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> kernel/trace/bpf_trace.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index d255bc9b2bfa..bb98a377050a 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2060,7 +2060,11 @@ struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char *name)
>
> void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp)
> {
> - struct module *mod = __module_address((unsigned long)btp);
> + struct module *mod;
> +
> + preempt_disable();
> + mod = __module_address((unsigned long)btp);
> + preempt_enable();
>
> if (mod)
> module_put(mod);
I don't understand why 'mod' cannot become dangling pointer after preempt_enable().
Either it needs a comment explaining why it's ok or module_put() should
be in preempt disabled section.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next 1/6] bpf: fix bpf_put_raw_tracepoint()'s use of __module_address()
2020-11-24 5:49 ` Alexei Starovoitov
@ 2020-11-24 7:49 ` Andrii Nakryiko
0 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2020-11-24 7:49 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
On Mon, Nov 23, 2020 at 9:49 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Nov 19, 2020 at 03:22:39PM -0800, Andrii Nakryiko wrote:
> > __module_address() needs to be called with preemption disabled or with
> > module_mutex taken. preempt_disable() is enough for read-only uses, which is
> > what this fix does.
> >
> > Fixes: a38d1107f937 ("bpf: support raw tracepoints in modules")
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> > kernel/trace/bpf_trace.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index d255bc9b2bfa..bb98a377050a 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2060,7 +2060,11 @@ struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char *name)
> >
> > void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp)
> > {
> > - struct module *mod = __module_address((unsigned long)btp);
> > + struct module *mod;
> > +
> > + preempt_disable();
> > + mod = __module_address((unsigned long)btp);
> > + preempt_enable();
> >
> > if (mod)
> > module_put(mod);
>
> I don't understand why 'mod' cannot become dangling pointer after preempt_enable().
> Either it needs a comment explaining why it's ok or module_put() should
> be in preempt disabled section.
Yeah, I think it can, assuming the kernel module can be unloaded
despite non-zero refcnt (probably happens with force unload?). I'll
drop the `if (mod)` part (module_put() checks that internally) and
will move module_put(mod) inside the preempt disable/enable region.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH bpf-next 2/6] libbpf: add internal helper to load BTF data by FD
2020-11-19 23:22 [PATCH bpf-next 0/6] libbpf: add support for kernel module BTF CO-RE relocations Andrii Nakryiko
2020-11-19 23:22 ` [PATCH bpf-next 1/6] bpf: fix bpf_put_raw_tracepoint()'s use of __module_address() Andrii Nakryiko
@ 2020-11-19 23:22 ` Andrii Nakryiko
2020-11-20 18:20 ` Martin KaFai Lau
2020-11-19 23:22 ` [PATCH bpf-next 3/6] libbpf: refactor CO-RE relocs to not assume a single BTF object Andrii Nakryiko
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Andrii Nakryiko @ 2020-11-19 23:22 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team
Add a btf_get_from_fd() helper, which constructs struct btf from in-kernel BTF
data by FD. This is used for loading module BTFs.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/btf.c | 61 +++++++++++++++++++--------------
tools/lib/bpf/libbpf_internal.h | 1 +
2 files changed, 36 insertions(+), 26 deletions(-)
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 8ff46cd30ca1..541696d9aed6 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -1318,35 +1318,27 @@ const char *btf__name_by_offset(const struct btf *btf, __u32 offset)
return btf__str_by_offset(btf, offset);
}
-int btf__get_from_id(__u32 id, struct btf **btf)
+struct btf *btf_get_from_fd(int btf_fd, struct btf *base_btf)
{
- struct bpf_btf_info btf_info = { 0 };
+ struct bpf_btf_info btf_info;
__u32 len = sizeof(btf_info);
__u32 last_size;
- int btf_fd;
+ struct btf *btf;
void *ptr;
int err;
- err = 0;
- *btf = NULL;
- btf_fd = bpf_btf_get_fd_by_id(id);
- if (btf_fd < 0)
- return 0;
-
/* we won't know btf_size until we call bpf_obj_get_info_by_fd(). so
* let's start with a sane default - 4KiB here - and resize it only if
* bpf_obj_get_info_by_fd() needs a bigger buffer.
*/
- btf_info.btf_size = 4096;
- last_size = btf_info.btf_size;
+ last_size = 4096;
ptr = malloc(last_size);
- if (!ptr) {
- err = -ENOMEM;
- goto exit_free;
- }
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
- memset(ptr, 0, last_size);
+ memset(&btf_info, 0, sizeof(btf_info));
btf_info.btf = ptr_to_u64(ptr);
+ btf_info.btf_size = last_size;
err = bpf_obj_get_info_by_fd(btf_fd, &btf_info, &len);
if (!err && btf_info.btf_size > last_size) {
@@ -1355,31 +1347,48 @@ int btf__get_from_id(__u32 id, struct btf **btf)
last_size = btf_info.btf_size;
temp_ptr = realloc(ptr, last_size);
if (!temp_ptr) {
- err = -ENOMEM;
+ btf = ERR_PTR(-ENOMEM);
goto exit_free;
}
ptr = temp_ptr;
- memset(ptr, 0, last_size);
+
+ len = sizeof(btf_info);
+ memset(&btf_info, 0, sizeof(btf_info));
btf_info.btf = ptr_to_u64(ptr);
+ btf_info.btf_size = last_size;
+
err = bpf_obj_get_info_by_fd(btf_fd, &btf_info, &len);
}
if (err || btf_info.btf_size > last_size) {
- err = errno;
+ btf = err ? ERR_PTR(-errno) : ERR_PTR(-E2BIG);
goto exit_free;
}
- *btf = btf__new((__u8 *)(long)btf_info.btf, btf_info.btf_size);
- if (IS_ERR(*btf)) {
- err = PTR_ERR(*btf);
- *btf = NULL;
- }
+ btf = btf_new(ptr, btf_info.btf_size, base_btf);
exit_free:
- close(btf_fd);
free(ptr);
+ return btf;
+}
- return err;
+int btf__get_from_id(__u32 id, struct btf **btf)
+{
+ struct btf *res;
+ int btf_fd;
+
+ *btf = NULL;
+ btf_fd = bpf_btf_get_fd_by_id(id);
+ if (btf_fd < 0)
+ return 0;
+
+ res = btf_get_from_fd(btf_fd, NULL);
+ close(btf_fd);
+ if (IS_ERR(res))
+ return PTR_ERR(res);
+
+ *btf = res;
+ return 0;
}
int btf__get_map_kv_tids(const struct btf *btf, const char *map_name,
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index d99bc847bf84..e569ae63808e 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -155,6 +155,7 @@ int bpf_object__section_size(const struct bpf_object *obj, const char *name,
__u32 *size);
int bpf_object__variable_offset(const struct bpf_object *obj, const char *name,
__u32 *off);
+struct btf *btf_get_from_fd(int btf_fd, struct btf *base_btf);
struct btf_ext_info {
/*
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next 2/6] libbpf: add internal helper to load BTF data by FD
2020-11-19 23:22 ` [PATCH bpf-next 2/6] libbpf: add internal helper to load BTF data by FD Andrii Nakryiko
@ 2020-11-20 18:20 ` Martin KaFai Lau
2020-11-20 18:25 ` Andrii Nakryiko
0 siblings, 1 reply; 21+ messages in thread
From: Martin KaFai Lau @ 2020-11-20 18:20 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, netdev, ast, daniel, kernel-team
On Thu, Nov 19, 2020 at 03:22:40PM -0800, Andrii Nakryiko wrote:
[ ... ]
> +int btf__get_from_id(__u32 id, struct btf **btf)
> +{
> + struct btf *res;
> + int btf_fd;
> +
> + *btf = NULL;
> + btf_fd = bpf_btf_get_fd_by_id(id);
> + if (btf_fd < 0)
> + return 0;
It should return an error.
> +
> + res = btf_get_from_fd(btf_fd, NULL);
> + close(btf_fd);
> + if (IS_ERR(res))
> + return PTR_ERR(res);
> +
> + *btf = res;
> + return 0;
> }
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next 2/6] libbpf: add internal helper to load BTF data by FD
2020-11-20 18:20 ` Martin KaFai Lau
@ 2020-11-20 18:25 ` Andrii Nakryiko
0 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2020-11-20 18:25 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
On Fri, Nov 20, 2020 at 10:20 AM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Thu, Nov 19, 2020 at 03:22:40PM -0800, Andrii Nakryiko wrote:
> [ ... ]
>
> > +int btf__get_from_id(__u32 id, struct btf **btf)
> > +{
> > + struct btf *res;
> > + int btf_fd;
> > +
> > + *btf = NULL;
> > + btf_fd = bpf_btf_get_fd_by_id(id);
> > + if (btf_fd < 0)
> > + return 0;
> It should return an error.
>
That would break the original behavior with (ret == 0 && btf == NULL),
but I think it's more consistent, so I'll fix this and will add Fixes:
tag.
> > +
> > + res = btf_get_from_fd(btf_fd, NULL);
> > + close(btf_fd);
> > + if (IS_ERR(res))
> > + return PTR_ERR(res);
> > +
> > + *btf = res;
> > + return 0;
> > }
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH bpf-next 3/6] libbpf: refactor CO-RE relocs to not assume a single BTF object
2020-11-19 23:22 [PATCH bpf-next 0/6] libbpf: add support for kernel module BTF CO-RE relocations Andrii Nakryiko
2020-11-19 23:22 ` [PATCH bpf-next 1/6] bpf: fix bpf_put_raw_tracepoint()'s use of __module_address() Andrii Nakryiko
2020-11-19 23:22 ` [PATCH bpf-next 2/6] libbpf: add internal helper to load BTF data by FD Andrii Nakryiko
@ 2020-11-19 23:22 ` Andrii Nakryiko
2020-11-20 22:11 ` Martin KaFai Lau
2020-11-19 23:22 ` [PATCH bpf-next 4/6] libbpf: add kernel module BTF support for CO-RE relocations Andrii Nakryiko
` (2 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Andrii Nakryiko @ 2020-11-19 23:22 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team
Refactor CO-RE relocation candidate search to not expect a single BTF, rather
return all candidate types with their corresponding BTF objects. This will
allow to extend CO-RE relocations to accommodate kernel module BTFs.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf.c | 187 ++++++++++++++++++++++++-----------------
1 file changed, 111 insertions(+), 76 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 313034117070..c4a49e8eb7b5 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -462,11 +462,14 @@ struct bpf_object {
struct list_head list;
struct btf *btf;
+ struct btf_ext *btf_ext;
+
/* Parse and load BTF vmlinux if any of the programs in the object need
* it at load time.
*/
struct btf *btf_vmlinux;
- struct btf_ext *btf_ext;
+ /* vmlinux BTF override for CO-RE relocations */
+ struct btf *btf_vmlinux_override;
void *priv;
bpf_object_clear_priv_t clear_priv;
@@ -4600,46 +4603,43 @@ static size_t bpf_core_essential_name_len(const char *name)
return n;
}
-/* dynamically sized list of type IDs */
-struct ids_vec {
- __u32 *data;
+struct core_cand
+{
+ const struct btf *btf;
+ const struct btf_type *t;
+ const char *name;
+ __u32 id;
+};
+
+/* dynamically sized list of type IDs and its associated struct btf */
+struct core_cand_list {
+ struct core_cand *cands;
int len;
};
-static void bpf_core_free_cands(struct ids_vec *cand_ids)
+static void bpf_core_free_cands(struct core_cand_list *cands)
{
- free(cand_ids->data);
- free(cand_ids);
+ free(cands->cands);
+ free(cands);
}
-static struct ids_vec *bpf_core_find_cands(const struct btf *local_btf,
- __u32 local_type_id,
- const struct btf *targ_btf)
+static int bpf_core_add_cands(struct core_cand *local_cand,
+ size_t local_essent_len,
+ const struct btf *targ_btf,
+ const char *targ_btf_name,
+ int targ_start_id,
+ struct core_cand_list *cands)
{
- size_t local_essent_len, targ_essent_len;
- const char *local_name, *targ_name;
- const struct btf_type *t, *local_t;
- struct ids_vec *cand_ids;
- __u32 *new_ids;
- int i, err, n;
-
- local_t = btf__type_by_id(local_btf, local_type_id);
- if (!local_t)
- return ERR_PTR(-EINVAL);
-
- local_name = btf__name_by_offset(local_btf, local_t->name_off);
- if (str_is_empty(local_name))
- return ERR_PTR(-EINVAL);
- local_essent_len = bpf_core_essential_name_len(local_name);
-
- cand_ids = calloc(1, sizeof(*cand_ids));
- if (!cand_ids)
- return ERR_PTR(-ENOMEM);
+ struct core_cand *new_cands, *cand;
+ const struct btf_type *t;
+ const char *targ_name;
+ size_t targ_essent_len;
+ int n, i;
n = btf__get_nr_types(targ_btf);
- for (i = 1; i <= n; i++) {
+ for (i = targ_start_id; i <= n; i++) {
t = btf__type_by_id(targ_btf, i);
- if (btf_kind(t) != btf_kind(local_t))
+ if (btf_kind(t) != btf_kind(local_cand->t))
continue;
targ_name = btf__name_by_offset(targ_btf, t->name_off);
@@ -4650,25 +4650,62 @@ static struct ids_vec *bpf_core_find_cands(const struct btf *local_btf,
if (targ_essent_len != local_essent_len)
continue;
- if (strncmp(local_name, targ_name, local_essent_len) == 0) {
- pr_debug("CO-RE relocating [%d] %s %s: found target candidate [%d] %s %s\n",
- local_type_id, btf_kind_str(local_t),
- local_name, i, btf_kind_str(t), targ_name);
- new_ids = libbpf_reallocarray(cand_ids->data,
- cand_ids->len + 1,
- sizeof(*cand_ids->data));
- if (!new_ids) {
- err = -ENOMEM;
- goto err_out;
- }
- cand_ids->data = new_ids;
- cand_ids->data[cand_ids->len++] = i;
- }
+ if (strncmp(local_cand->name, targ_name, local_essent_len) != 0)
+ continue;
+
+ pr_debug("CO-RE relocating [%d] %s %s: found target candidate [%d] %s %s in [%s]\n",
+ local_cand->id, btf_kind_str(local_cand->t),
+ local_cand->name, i, btf_kind_str(t), targ_name,
+ targ_btf_name);
+ new_cands = libbpf_reallocarray(cands->cands, cands->len + 1,
+ sizeof(*cands->cands));
+ if (!new_cands)
+ return -ENOMEM;
+
+ cand = &new_cands[cands->len];
+ cand->btf = targ_btf;
+ cand->t = t;
+ cand->name = targ_name;
+ cand->id = i;
+
+ cands->cands = new_cands;
+ cands->len++;
}
- return cand_ids;
-err_out:
- bpf_core_free_cands(cand_ids);
- return ERR_PTR(err);
+ return 0;
+}
+
+static struct core_cand_list *
+bpf_core_find_cands(struct bpf_object *obj, const struct btf *local_btf, __u32 local_type_id)
+{
+ struct core_cand local_cand = {};
+ struct core_cand_list *cands;
+ size_t local_essent_len;
+ int err;
+
+ local_cand.btf = local_btf;
+ local_cand.t = btf__type_by_id(local_btf, local_type_id);
+ if (!local_cand.t)
+ return ERR_PTR(-EINVAL);
+
+ local_cand.name = btf__name_by_offset(local_btf, local_cand.t->name_off);
+ if (str_is_empty(local_cand.name))
+ return ERR_PTR(-EINVAL);
+ local_essent_len = bpf_core_essential_name_len(local_cand.name);
+
+ cands = calloc(1, sizeof(*cands));
+ if (!cands)
+ return ERR_PTR(-ENOMEM);
+
+ /* Attempt to find target candidates in vmlinux BTF first */
+ err = bpf_core_add_cands(&local_cand, local_essent_len,
+ obj->btf_vmlinux_override ?: obj->btf_vmlinux,
+ "vmlinux", 1, cands);
+ if (err) {
+ bpf_core_free_cands(cands);
+ return ERR_PTR(err);
+ }
+
+ return cands;
}
/* Check two types for compatibility for the purpose of field access
@@ -5661,7 +5698,6 @@ static int bpf_core_apply_relo(struct bpf_program *prog,
const struct bpf_core_relo *relo,
int relo_idx,
const struct btf *local_btf,
- const struct btf *targ_btf,
struct hashmap *cand_cache)
{
struct bpf_core_spec local_spec, cand_spec, targ_spec = {};
@@ -5669,8 +5705,8 @@ static int bpf_core_apply_relo(struct bpf_program *prog,
struct bpf_core_relo_res cand_res, targ_res;
const struct btf_type *local_type;
const char *local_name;
- struct ids_vec *cand_ids;
- __u32 local_id, cand_id;
+ struct core_cand_list *cands = NULL;
+ __u32 local_id;
const char *spec_str;
int i, j, err;
@@ -5717,24 +5753,24 @@ static int bpf_core_apply_relo(struct bpf_program *prog,
return -EOPNOTSUPP;
}
- if (!hashmap__find(cand_cache, type_key, (void **)&cand_ids)) {
- cand_ids = bpf_core_find_cands(local_btf, local_id, targ_btf);
- if (IS_ERR(cand_ids)) {
+ if (!hashmap__find(cand_cache, type_key, (void **)&cands)) {
+ cands = bpf_core_find_cands(prog->obj, local_btf, local_id);
+ if (IS_ERR(cands)) {
pr_warn("prog '%s': relo #%d: target candidate search failed for [%d] %s %s: %ld",
prog->name, relo_idx, local_id, btf_kind_str(local_type),
- local_name, PTR_ERR(cand_ids));
- return PTR_ERR(cand_ids);
+ local_name, PTR_ERR(cands));
+ return PTR_ERR(cands);
}
- err = hashmap__set(cand_cache, type_key, cand_ids, NULL, NULL);
+ err = hashmap__set(cand_cache, type_key, cands, NULL, NULL);
if (err) {
- bpf_core_free_cands(cand_ids);
+ bpf_core_free_cands(cands);
return err;
}
}
- for (i = 0, j = 0; i < cand_ids->len; i++) {
- cand_id = cand_ids->data[i];
- err = bpf_core_spec_match(&local_spec, targ_btf, cand_id, &cand_spec);
+ for (i = 0, j = 0; i < cands->len; i++) {
+ err = bpf_core_spec_match(&local_spec, cands->cands[i].btf,
+ cands->cands[i].id, &cand_spec);
if (err < 0) {
pr_warn("prog '%s': relo #%d: error matching candidate #%d ",
prog->name, relo_idx, i);
@@ -5778,7 +5814,7 @@ static int bpf_core_apply_relo(struct bpf_program *prog,
return -EINVAL;
}
- cand_ids->data[j++] = cand_spec.root_type_id;
+ cands->cands[j++] = cands->cands[i];
}
/*
@@ -5790,7 +5826,7 @@ static int bpf_core_apply_relo(struct bpf_program *prog,
* depending on relo's kind.
*/
if (j > 0)
- cand_ids->len = j;
+ cands->len = j;
/*
* If no candidates were found, it might be both a programmer error,
@@ -5834,20 +5870,19 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
struct hashmap_entry *entry;
struct hashmap *cand_cache = NULL;
struct bpf_program *prog;
- struct btf *targ_btf;
const char *sec_name;
int i, err = 0, insn_idx, sec_idx;
if (obj->btf_ext->core_relo_info.len == 0)
return 0;
- if (targ_btf_path)
- targ_btf = btf__parse(targ_btf_path, NULL);
- else
- targ_btf = obj->btf_vmlinux;
- if (IS_ERR_OR_NULL(targ_btf)) {
- pr_warn("failed to get target BTF: %ld\n", PTR_ERR(targ_btf));
- return PTR_ERR(targ_btf);
+ if (targ_btf_path) {
+ obj->btf_vmlinux_override = btf__parse(targ_btf_path, NULL);
+ if (IS_ERR_OR_NULL(obj->btf_vmlinux_override)) {
+ err = PTR_ERR(obj->btf_vmlinux_override);
+ pr_warn("failed to parse target BTF: %d\n", err);
+ return err;
+ }
}
cand_cache = hashmap__new(bpf_core_hash_fn, bpf_core_equal_fn, NULL);
@@ -5899,8 +5934,7 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
if (!prog->load)
continue;
- err = bpf_core_apply_relo(prog, rec, i, obj->btf,
- targ_btf, cand_cache);
+ err = bpf_core_apply_relo(prog, rec, i, obj->btf, cand_cache);
if (err) {
pr_warn("prog '%s': relo #%d: failed to relocate: %d\n",
prog->name, i, err);
@@ -5911,8 +5945,9 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
out:
/* obj->btf_vmlinux is freed at the end of object load phase */
- if (targ_btf != obj->btf_vmlinux)
- btf__free(targ_btf);
+ btf__free(obj->btf_vmlinux_override);
+ obj->btf_vmlinux_override = NULL;
+
if (!IS_ERR_OR_NULL(cand_cache)) {
hashmap__for_each_entry(cand_cache, entry, i) {
bpf_core_free_cands(entry->value);
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH bpf-next 4/6] libbpf: add kernel module BTF support for CO-RE relocations
2020-11-19 23:22 [PATCH bpf-next 0/6] libbpf: add support for kernel module BTF CO-RE relocations Andrii Nakryiko
` (2 preceding siblings ...)
2020-11-19 23:22 ` [PATCH bpf-next 3/6] libbpf: refactor CO-RE relocs to not assume a single BTF object Andrii Nakryiko
@ 2020-11-19 23:22 ` Andrii Nakryiko
2020-11-20 0:46 ` Maciej Fijalkowski
2020-11-20 23:05 ` Martin KaFai Lau
2020-11-19 23:22 ` [PATCH bpf-next 5/6] selftests/bpf: add bpf_sidecar kernel module for testing Andrii Nakryiko
2020-11-19 23:22 ` [PATCH bpf-next 6/6] selftests/bpf: add CO-RE relocs selftest relying on kernel module BTF Andrii Nakryiko
5 siblings, 2 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2020-11-19 23:22 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team
Teach libbpf to search for candidate types for CO-RE relocations across kernel
modules BTFs, in addition to vmlinux BTF. If at least one candidate type is
found in vmlinux BTF, kernel module BTFs are not iterated. If vmlinux BTF has
no matching candidates, then find all kernel module BTFs and search for all
matching candidates across all of them.
Kernel's support for module BTFs are inferred from the support for BTF name
pointer in BPF UAPI.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf.c | 185 ++++++++++++++++++++++++++++++++++++++---
1 file changed, 172 insertions(+), 13 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index c4a49e8eb7b5..ac1ff4e7741a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -176,6 +176,8 @@ enum kern_feature_id {
FEAT_PROBE_READ_KERN,
/* BPF_PROG_BIND_MAP is supported */
FEAT_PROG_BIND_MAP,
+ /* Kernel support for module BTFs */
+ FEAT_MODULE_BTF,
__FEAT_CNT,
};
@@ -402,6 +404,12 @@ struct extern_desc {
static LIST_HEAD(bpf_objects_list);
+struct module_btf {
+ struct btf *btf;
+ char *name;
+ __u32 id;
+};
+
struct bpf_object {
char name[BPF_OBJ_NAME_LEN];
char license[64];
@@ -470,6 +478,11 @@ struct bpf_object {
struct btf *btf_vmlinux;
/* vmlinux BTF override for CO-RE relocations */
struct btf *btf_vmlinux_override;
+ /* Lazily initialized kernel module BTFs */
+ struct module_btf *btf_modules;
+ bool btf_modules_loaded;
+ size_t btf_module_cnt;
+ size_t btf_module_cap;
void *priv;
bpf_object_clear_priv_t clear_priv;
@@ -3960,6 +3973,36 @@ static int probe_prog_bind_map(void)
return ret >= 0;
}
+static int probe_module_btf(void)
+{
+ static const char strs[] = "\0int";
+ __u32 types[] = {
+ /* int */
+ BTF_TYPE_INT_ENC(1, BTF_INT_SIGNED, 0, 32, 4),
+ };
+ struct bpf_btf_info info;
+ __u32 len = sizeof(info);
+ char name[16];
+ int fd, err;
+
+ fd = libbpf__load_raw_btf((char *)types, sizeof(types), strs, sizeof(strs));
+ if (fd < 0)
+ return 0; /* BTF not supported at all */
+
+ len = sizeof(info);
+ memset(&info, 0, sizeof(info));
+ info.name = ptr_to_u64(name);
+ info.name_len = sizeof(name);
+
+ /* check that BPF_OBJ_GET_INFO_BY_FD supports specifying name pointer;
+ * kernel's module BTF support coincides with support for
+ * name/name_len fields in struct bpf_btf_info.
+ */
+ err = bpf_obj_get_info_by_fd(fd, &info, &len);
+ close(fd);
+ return !err;
+}
+
enum kern_feature_result {
FEAT_UNKNOWN = 0,
FEAT_SUPPORTED = 1,
@@ -4003,7 +4046,10 @@ static struct kern_feature_desc {
},
[FEAT_PROG_BIND_MAP] = {
"BPF_PROG_BIND_MAP support", probe_prog_bind_map,
- }
+ },
+ [FEAT_MODULE_BTF] = {
+ "module BTF support", probe_module_btf,
+ },
};
static bool kernel_supports(enum kern_feature_id feat_id)
@@ -4674,13 +4720,95 @@ static int bpf_core_add_cands(struct core_cand *local_cand,
return 0;
}
+static int load_module_btfs(struct bpf_object *obj)
+{
+ struct bpf_btf_info info;
+ struct module_btf *mod_btf;
+ struct btf *btf;
+ char name[64];
+ __u32 id, len;
+ int err, fd;
+
+ if (obj->btf_modules_loaded)
+ return 0;
+
+ /* don't do this again, even if we find no module BTFs */
+ obj->btf_modules_loaded = true;
+
+ /* kernel too old to support module BTFs */
+ if (!kernel_supports(FEAT_MODULE_BTF))
+ return 0;
+
+ while (true) {
+ err = bpf_btf_get_next_id(id, &id);
+ if (err && errno == ENOENT)
+ return 0;
+ if (err) {
+ err = -errno;
+ pr_warn("failed to iterate BTF objects: %d\n", err);
+ return err;
+ }
+
+ fd = bpf_btf_get_fd_by_id(id);
+ if (fd < 0) {
+ if (errno == ENOENT)
+ continue; /* expected race: BTF was unloaded */
+ err = -errno;
+ pr_warn("failed to get BTF object #%d FD: %d\n", id, err);
+ return err;
+ }
+
+ len = sizeof(info);
+ memset(&info, 0, sizeof(info));
+ info.name = ptr_to_u64(name);
+ info.name_len = sizeof(name);
+
+ err = bpf_obj_get_info_by_fd(fd, &info, &len);
+ if (err) {
+ err = -errno;
+ pr_warn("failed to get BTF object #%d info: %d\n", id, err);
+ return err;
+ }
+
+ /* ignore non-module BTFs */
+ if (!info.kernel_btf || strcmp(name, "vmlinux") == 0) {
+ close(fd);
+ continue;
+ }
+
+ btf = btf_get_from_fd(fd, obj->btf_vmlinux);
+ close(fd);
+ if (IS_ERR(btf)) {
+ pr_warn("failed to load module [%s]'s BTF object #%d: %ld\n",
+ name, id, PTR_ERR(btf));
+ return PTR_ERR(btf);
+ }
+
+ err = btf_ensure_mem((void **)&obj->btf_modules, &obj->btf_module_cap,
+ sizeof(*obj->btf_modules), obj->btf_module_cnt + 1);
+ if (err)
+ return err;
+
+ mod_btf = &obj->btf_modules[obj->btf_module_cnt++];
+
+ mod_btf->btf = btf;
+ mod_btf->id = id;
+ mod_btf->name = strdup(name);
+ if (!mod_btf->name)
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
static struct core_cand_list *
bpf_core_find_cands(struct bpf_object *obj, const struct btf *local_btf, __u32 local_type_id)
{
struct core_cand local_cand = {};
struct core_cand_list *cands;
+ const struct btf *main_btf;
size_t local_essent_len;
- int err;
+ int err, i;
local_cand.btf = local_btf;
local_cand.t = btf__type_by_id(local_btf, local_type_id);
@@ -4697,15 +4825,38 @@ bpf_core_find_cands(struct bpf_object *obj, const struct btf *local_btf, __u32 l
return ERR_PTR(-ENOMEM);
/* Attempt to find target candidates in vmlinux BTF first */
- err = bpf_core_add_cands(&local_cand, local_essent_len,
- obj->btf_vmlinux_override ?: obj->btf_vmlinux,
- "vmlinux", 1, cands);
- if (err) {
- bpf_core_free_cands(cands);
- return ERR_PTR(err);
+ main_btf = obj->btf_vmlinux_override ?: obj->btf_vmlinux;
+ err = bpf_core_add_cands(&local_cand, local_essent_len, main_btf, "vmlinux", 1, cands);
+ if (err)
+ goto err_out;
+
+ /* if vmlinux BTF has any candidate, don't got for module BTFs */
+ if (cands->len)
+ return cands;
+
+ /* if vmlinux BTF was overridden, don't attempt to load module BTFs */
+ if (obj->btf_vmlinux_override)
+ return cands;
+
+ /* now look through module BTFs, trying to still find candidates */
+ err = load_module_btfs(obj);
+ if (err)
+ goto err_out;
+
+ for (i = 0; i < obj->btf_module_cnt; i++) {
+ err = bpf_core_add_cands(&local_cand, local_essent_len,
+ obj->btf_modules[i].btf,
+ obj->btf_modules[i].name,
+ btf__get_nr_types(obj->btf_vmlinux) + 1,
+ cands);
+ if (err)
+ goto err_out;
}
return cands;
+err_out:
+ bpf_core_free_cands(cands);
+ return ERR_PTR(err);
}
/* Check two types for compatibility for the purpose of field access
@@ -5756,7 +5907,7 @@ static int bpf_core_apply_relo(struct bpf_program *prog,
if (!hashmap__find(cand_cache, type_key, (void **)&cands)) {
cands = bpf_core_find_cands(prog->obj, local_btf, local_id);
if (IS_ERR(cands)) {
- pr_warn("prog '%s': relo #%d: target candidate search failed for [%d] %s %s: %ld",
+ pr_warn("prog '%s': relo #%d: target candidate search failed for [%d] %s %s: %ld\n",
prog->name, relo_idx, local_id, btf_kind_str(local_type),
local_name, PTR_ERR(cands));
return PTR_ERR(cands);
@@ -5944,7 +6095,7 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
}
out:
- /* obj->btf_vmlinux is freed at the end of object load phase */
+ /* obj->btf_vmlinux and module BTFs are freed after object load */
btf__free(obj->btf_vmlinux_override);
obj->btf_vmlinux_override = NULL;
@@ -7303,6 +7454,14 @@ int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
err = err ? : bpf_object__relocate(obj, attr->target_btf_path);
err = err ? : bpf_object__load_progs(obj, attr->log_level);
+ /* clean up module BTFs */
+ for (i = 0; i < obj->btf_module_cnt; i++) {
+ btf__free(obj->btf_modules[i].btf);
+ free(obj->btf_modules[i].name);
+ }
+ free(obj->btf_modules);
+
+ /* clean up vmlinux BTF */
btf__free(obj->btf_vmlinux);
obj->btf_vmlinux = NULL;
@@ -8656,9 +8815,6 @@ static inline int __find_vmlinux_btf_id(struct btf *btf, const char *name,
else
err = btf__find_by_name_kind(btf, name, BTF_KIND_FUNC);
- if (err <= 0)
- pr_warn("%s is not found in vmlinux BTF\n", name);
-
return err;
}
@@ -8675,6 +8831,9 @@ int libbpf_find_vmlinux_btf_id(const char *name,
}
err = __find_vmlinux_btf_id(btf, name, attach_type);
+ if (err <= 0)
+ pr_warn("%s is not found in vmlinux BTF\n", name);
+
btf__free(btf);
return err;
}
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next 4/6] libbpf: add kernel module BTF support for CO-RE relocations
2020-11-19 23:22 ` [PATCH bpf-next 4/6] libbpf: add kernel module BTF support for CO-RE relocations Andrii Nakryiko
@ 2020-11-20 0:46 ` Maciej Fijalkowski
2020-11-20 1:24 ` Andrii Nakryiko
2020-11-20 23:05 ` Martin KaFai Lau
1 sibling, 1 reply; 21+ messages in thread
From: Maciej Fijalkowski @ 2020-11-20 0:46 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, netdev, ast, daniel, kernel-team
On Thu, Nov 19, 2020 at 03:22:42PM -0800, Andrii Nakryiko wrote:
> Teach libbpf to search for candidate types for CO-RE relocations across kernel
> modules BTFs, in addition to vmlinux BTF. If at least one candidate type is
> found in vmlinux BTF, kernel module BTFs are not iterated. If vmlinux BTF has
> no matching candidates, then find all kernel module BTFs and search for all
> matching candidates across all of them.
>
> Kernel's support for module BTFs are inferred from the support for BTF name
> pointer in BPF UAPI.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> tools/lib/bpf/libbpf.c | 185 ++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 172 insertions(+), 13 deletions(-)
>
[...]
> +static int probe_module_btf(void)
> +{
> + static const char strs[] = "\0int";
> + __u32 types[] = {
> + /* int */
> + BTF_TYPE_INT_ENC(1, BTF_INT_SIGNED, 0, 32, 4),
> + };
> + struct bpf_btf_info info;
> + __u32 len = sizeof(info);
> + char name[16];
> + int fd, err;
> +
> + fd = libbpf__load_raw_btf((char *)types, sizeof(types), strs, sizeof(strs));
> + if (fd < 0)
> + return 0; /* BTF not supported at all */
> +
> + len = sizeof(info);
nit: reinit of len
> + memset(&info, 0, sizeof(info));
use len in memset
> + info.name = ptr_to_u64(name);
> + info.name_len = sizeof(name);
> +
> + /* check that BPF_OBJ_GET_INFO_BY_FD supports specifying name pointer;
> + * kernel's module BTF support coincides with support for
> + * name/name_len fields in struct bpf_btf_info.
> + */
> + err = bpf_obj_get_info_by_fd(fd, &info, &len);
> + close(fd);
> + return !err;
> +}
[...]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next 4/6] libbpf: add kernel module BTF support for CO-RE relocations
2020-11-20 0:46 ` Maciej Fijalkowski
@ 2020-11-20 1:24 ` Andrii Nakryiko
2020-11-20 2:05 ` Maciej Fijalkowski
0 siblings, 1 reply; 21+ messages in thread
From: Andrii Nakryiko @ 2020-11-20 1:24 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
On Thu, Nov 19, 2020 at 4:55 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Thu, Nov 19, 2020 at 03:22:42PM -0800, Andrii Nakryiko wrote:
> > Teach libbpf to search for candidate types for CO-RE relocations across kernel
> > modules BTFs, in addition to vmlinux BTF. If at least one candidate type is
> > found in vmlinux BTF, kernel module BTFs are not iterated. If vmlinux BTF has
> > no matching candidates, then find all kernel module BTFs and search for all
> > matching candidates across all of them.
> >
> > Kernel's support for module BTFs are inferred from the support for BTF name
> > pointer in BPF UAPI.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> > tools/lib/bpf/libbpf.c | 185 ++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 172 insertions(+), 13 deletions(-)
> >
>
> [...]
>
> > +static int probe_module_btf(void)
> > +{
> > + static const char strs[] = "\0int";
> > + __u32 types[] = {
> > + /* int */
> > + BTF_TYPE_INT_ENC(1, BTF_INT_SIGNED, 0, 32, 4),
> > + };
> > + struct bpf_btf_info info;
> > + __u32 len = sizeof(info);
> > + char name[16];
> > + int fd, err;
> > +
> > + fd = libbpf__load_raw_btf((char *)types, sizeof(types), strs, sizeof(strs));
> > + if (fd < 0)
> > + return 0; /* BTF not supported at all */
> > +
> > + len = sizeof(info);
>
> nit: reinit of len
>
oops, right, I'll remove it
> > + memset(&info, 0, sizeof(info));
>
> use len in memset
why?
>
> > + info.name = ptr_to_u64(name);
> > + info.name_len = sizeof(name);
> > +
> > + /* check that BPF_OBJ_GET_INFO_BY_FD supports specifying name pointer;
> > + * kernel's module BTF support coincides with support for
> > + * name/name_len fields in struct bpf_btf_info.
> > + */
> > + err = bpf_obj_get_info_by_fd(fd, &info, &len);
> > + close(fd);
> > + return !err;
> > +}
>
> [...]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next 4/6] libbpf: add kernel module BTF support for CO-RE relocations
2020-11-20 1:24 ` Andrii Nakryiko
@ 2020-11-20 2:05 ` Maciej Fijalkowski
2020-11-20 3:31 ` Andrii Nakryiko
0 siblings, 1 reply; 21+ messages in thread
From: Maciej Fijalkowski @ 2020-11-20 2:05 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
On Thu, Nov 19, 2020 at 05:24:43PM -0800, Andrii Nakryiko wrote:
> On Thu, Nov 19, 2020 at 4:55 PM Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > On Thu, Nov 19, 2020 at 03:22:42PM -0800, Andrii Nakryiko wrote:
> > > Teach libbpf to search for candidate types for CO-RE relocations across kernel
> > > modules BTFs, in addition to vmlinux BTF. If at least one candidate type is
> > > found in vmlinux BTF, kernel module BTFs are not iterated. If vmlinux BTF has
> > > no matching candidates, then find all kernel module BTFs and search for all
> > > matching candidates across all of them.
> > >
> > > Kernel's support for module BTFs are inferred from the support for BTF name
> > > pointer in BPF UAPI.
> > >
> > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > > ---
> > > tools/lib/bpf/libbpf.c | 185 ++++++++++++++++++++++++++++++++++++++---
> > > 1 file changed, 172 insertions(+), 13 deletions(-)
> > >
> >
> > [...]
> >
> > > +static int probe_module_btf(void)
> > > +{
> > > + static const char strs[] = "\0int";
> > > + __u32 types[] = {
> > > + /* int */
> > > + BTF_TYPE_INT_ENC(1, BTF_INT_SIGNED, 0, 32, 4),
> > > + };
> > > + struct bpf_btf_info info;
> > > + __u32 len = sizeof(info);
> > > + char name[16];
> > > + int fd, err;
> > > +
> > > + fd = libbpf__load_raw_btf((char *)types, sizeof(types), strs, sizeof(strs));
> > > + if (fd < 0)
> > > + return 0; /* BTF not supported at all */
> > > +
> > > + len = sizeof(info);
> >
> > nit: reinit of len
> >
>
> oops, right, I'll remove it
>
>
> > > + memset(&info, 0, sizeof(info));
> >
> > use len in memset
>
> why?
Hm, just to make use of local var? We might argue that current version is
more readable, but then again I would question the len's existence.
Do whatever you want, these were just nits :)
>
> >
> > > + info.name = ptr_to_u64(name);
> > > + info.name_len = sizeof(name);
> > > +
> > > + /* check that BPF_OBJ_GET_INFO_BY_FD supports specifying name pointer;
> > > + * kernel's module BTF support coincides with support for
> > > + * name/name_len fields in struct bpf_btf_info.
> > > + */
> > > + err = bpf_obj_get_info_by_fd(fd, &info, &len);
> > > + close(fd);
> > > + return !err;
> > > +}
> >
> > [...]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next 4/6] libbpf: add kernel module BTF support for CO-RE relocations
2020-11-20 2:05 ` Maciej Fijalkowski
@ 2020-11-20 3:31 ` Andrii Nakryiko
0 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2020-11-20 3:31 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
On Thu, Nov 19, 2020 at 6:14 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Thu, Nov 19, 2020 at 05:24:43PM -0800, Andrii Nakryiko wrote:
> > On Thu, Nov 19, 2020 at 4:55 PM Maciej Fijalkowski
> > <maciej.fijalkowski@intel.com> wrote:
> > >
> > > On Thu, Nov 19, 2020 at 03:22:42PM -0800, Andrii Nakryiko wrote:
> > > > Teach libbpf to search for candidate types for CO-RE relocations across kernel
> > > > modules BTFs, in addition to vmlinux BTF. If at least one candidate type is
> > > > found in vmlinux BTF, kernel module BTFs are not iterated. If vmlinux BTF has
> > > > no matching candidates, then find all kernel module BTFs and search for all
> > > > matching candidates across all of them.
> > > >
> > > > Kernel's support for module BTFs are inferred from the support for BTF name
> > > > pointer in BPF UAPI.
> > > >
> > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > > > ---
> > > > tools/lib/bpf/libbpf.c | 185 ++++++++++++++++++++++++++++++++++++++---
> > > > 1 file changed, 172 insertions(+), 13 deletions(-)
> > > >
> > >
> > > [...]
> > >
> > > > +static int probe_module_btf(void)
> > > > +{
> > > > + static const char strs[] = "\0int";
> > > > + __u32 types[] = {
> > > > + /* int */
> > > > + BTF_TYPE_INT_ENC(1, BTF_INT_SIGNED, 0, 32, 4),
> > > > + };
> > > > + struct bpf_btf_info info;
> > > > + __u32 len = sizeof(info);
> > > > + char name[16];
> > > > + int fd, err;
> > > > +
> > > > + fd = libbpf__load_raw_btf((char *)types, sizeof(types), strs, sizeof(strs));
> > > > + if (fd < 0)
> > > > + return 0; /* BTF not supported at all */
> > > > +
> > > > + len = sizeof(info);
> > >
> > > nit: reinit of len
> > >
> >
> > oops, right, I'll remove it
> >
> >
> > > > + memset(&info, 0, sizeof(info));
> > >
> > > use len in memset
> >
> > why?
>
> Hm, just to make use of local var? We might argue that current version is
I agree, I think sizeof(info) is more readable. But my point is that
if you suggest something, please provide at least some argument for
why you think it's better or why existing code is worse or wrong (if
you think it is).
> more readable, but then again I would question the len's existence.
len is passed to the kernel by reference and the kernel is updating it
with the actual length it has (which could be <, ==, or > than what
the program specified). So it has to be in a variable.
>
> Do whatever you want, these were just nits :)
>
> >
> > >
> > > > + info.name = ptr_to_u64(name);
> > > > + info.name_len = sizeof(name);
> > > > +
> > > > + /* check that BPF_OBJ_GET_INFO_BY_FD supports specifying name pointer;
> > > > + * kernel's module BTF support coincides with support for
> > > > + * name/name_len fields in struct bpf_btf_info.
> > > > + */
> > > > + err = bpf_obj_get_info_by_fd(fd, &info, &len);
here -------------------------------------------------^^^^
> > > > + close(fd);
> > > > + return !err;
> > > > +}
> > >
> > > [...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next 4/6] libbpf: add kernel module BTF support for CO-RE relocations
2020-11-19 23:22 ` [PATCH bpf-next 4/6] libbpf: add kernel module BTF support for CO-RE relocations Andrii Nakryiko
2020-11-20 0:46 ` Maciej Fijalkowski
@ 2020-11-20 23:05 ` Martin KaFai Lau
2020-11-20 23:12 ` Andrii Nakryiko
1 sibling, 1 reply; 21+ messages in thread
From: Martin KaFai Lau @ 2020-11-20 23:05 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, netdev, ast, daniel, kernel-team
On Thu, Nov 19, 2020 at 03:22:42PM -0800, Andrii Nakryiko wrote:
[ ... ]
> +static int load_module_btfs(struct bpf_object *obj)
> +{
> + struct bpf_btf_info info;
> + struct module_btf *mod_btf;
> + struct btf *btf;
> + char name[64];
> + __u32 id, len;
> + int err, fd;
> +
> + if (obj->btf_modules_loaded)
> + return 0;
> +
> + /* don't do this again, even if we find no module BTFs */
> + obj->btf_modules_loaded = true;
> +
> + /* kernel too old to support module BTFs */
> + if (!kernel_supports(FEAT_MODULE_BTF))
> + return 0;
> +
> + while (true) {
> + err = bpf_btf_get_next_id(id, &id);
> + if (err && errno == ENOENT)
> + return 0;
> + if (err) {
> + err = -errno;
> + pr_warn("failed to iterate BTF objects: %d\n", err);
> + return err;
> + }
> +
> + fd = bpf_btf_get_fd_by_id(id);
> + if (fd < 0) {
> + if (errno == ENOENT)
> + continue; /* expected race: BTF was unloaded */
> + err = -errno;
> + pr_warn("failed to get BTF object #%d FD: %d\n", id, err);
> + return err;
> + }
> +
> + len = sizeof(info);
> + memset(&info, 0, sizeof(info));
> + info.name = ptr_to_u64(name);
> + info.name_len = sizeof(name);
> +
> + err = bpf_obj_get_info_by_fd(fd, &info, &len);
> + if (err) {
> + err = -errno;
> + pr_warn("failed to get BTF object #%d info: %d\n", id, err);
close(fd);
> + return err;
> + }
> +
> + /* ignore non-module BTFs */
> + if (!info.kernel_btf || strcmp(name, "vmlinux") == 0) {
> + close(fd);
> + continue;
> + }
> +
[ ... ]
> @@ -8656,9 +8815,6 @@ static inline int __find_vmlinux_btf_id(struct btf *btf, const char *name,
> else
> err = btf__find_by_name_kind(btf, name, BTF_KIND_FUNC);
>
> - if (err <= 0)
> - pr_warn("%s is not found in vmlinux BTF\n", name);
> -
> return err;
> }
>
> @@ -8675,6 +8831,9 @@ int libbpf_find_vmlinux_btf_id(const char *name,
> }
>
> err = __find_vmlinux_btf_id(btf, name, attach_type);
> + if (err <= 0)
> + pr_warn("%s is not found in vmlinux BTF\n", name);
> +
Please explain this move in the commit message.
> btf__free(btf);
> return err;
> }
> --
> 2.24.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next 4/6] libbpf: add kernel module BTF support for CO-RE relocations
2020-11-20 23:05 ` Martin KaFai Lau
@ 2020-11-20 23:12 ` Andrii Nakryiko
0 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2020-11-20 23:12 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
On Fri, Nov 20, 2020 at 3:06 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Thu, Nov 19, 2020 at 03:22:42PM -0800, Andrii Nakryiko wrote:
> [ ... ]
>
> > +static int load_module_btfs(struct bpf_object *obj)
> > +{
> > + struct bpf_btf_info info;
> > + struct module_btf *mod_btf;
> > + struct btf *btf;
> > + char name[64];
> > + __u32 id, len;
> > + int err, fd;
> > +
> > + if (obj->btf_modules_loaded)
> > + return 0;
> > +
> > + /* don't do this again, even if we find no module BTFs */
> > + obj->btf_modules_loaded = true;
> > +
> > + /* kernel too old to support module BTFs */
> > + if (!kernel_supports(FEAT_MODULE_BTF))
> > + return 0;
> > +
> > + while (true) {
> > + err = bpf_btf_get_next_id(id, &id);
> > + if (err && errno == ENOENT)
> > + return 0;
> > + if (err) {
> > + err = -errno;
> > + pr_warn("failed to iterate BTF objects: %d\n", err);
> > + return err;
> > + }
> > +
> > + fd = bpf_btf_get_fd_by_id(id);
> > + if (fd < 0) {
> > + if (errno == ENOENT)
> > + continue; /* expected race: BTF was unloaded */
> > + err = -errno;
> > + pr_warn("failed to get BTF object #%d FD: %d\n", id, err);
> > + return err;
> > + }
> > +
> > + len = sizeof(info);
> > + memset(&info, 0, sizeof(info));
> > + info.name = ptr_to_u64(name);
> > + info.name_len = sizeof(name);
> > +
> > + err = bpf_obj_get_info_by_fd(fd, &info, &len);
> > + if (err) {
> > + err = -errno;
> > + pr_warn("failed to get BTF object #%d info: %d\n", id, err);
>
> close(fd);
>
> > + return err;
> > + }
> > +
> > + /* ignore non-module BTFs */
> > + if (!info.kernel_btf || strcmp(name, "vmlinux") == 0) {
> > + close(fd);
> > + continue;
> > + }
> > +
>
> [ ... ]
>
> > @@ -8656,9 +8815,6 @@ static inline int __find_vmlinux_btf_id(struct btf *btf, const char *name,
> > else
> > err = btf__find_by_name_kind(btf, name, BTF_KIND_FUNC);
> >
> > - if (err <= 0)
> > - pr_warn("%s is not found in vmlinux BTF\n", name);
> > -
> > return err;
> > }
> >
> > @@ -8675,6 +8831,9 @@ int libbpf_find_vmlinux_btf_id(const char *name,
> > }
> >
> > err = __find_vmlinux_btf_id(btf, name, attach_type);
> > + if (err <= 0)
> > + pr_warn("%s is not found in vmlinux BTF\n", name);
> > +
> Please explain this move in the commit message.
ok, I'll add something about that. The short answer is that
__find_vmlinux_btf_id() is now expected to not find a type in vmlinux
BTF, so emitting error would be wrong. So I moved it up a level where
it's not expected.
>
> > btf__free(btf);
> > return err;
> > }
> > --
> > 2.24.1
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH bpf-next 5/6] selftests/bpf: add bpf_sidecar kernel module for testing
2020-11-19 23:22 [PATCH bpf-next 0/6] libbpf: add support for kernel module BTF CO-RE relocations Andrii Nakryiko
` (3 preceding siblings ...)
2020-11-19 23:22 ` [PATCH bpf-next 4/6] libbpf: add kernel module BTF support for CO-RE relocations Andrii Nakryiko
@ 2020-11-19 23:22 ` Andrii Nakryiko
2020-11-20 23:21 ` Martin KaFai Lau
2020-11-19 23:22 ` [PATCH bpf-next 6/6] selftests/bpf: add CO-RE relocs selftest relying on kernel module BTF Andrii Nakryiko
5 siblings, 1 reply; 21+ messages in thread
From: Andrii Nakryiko @ 2020-11-19 23:22 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team
Add bpf_sidecar module, which is conceptually out-of-tree module and provides
ways for selftests/bpf to test various kernel module-related functionality:
raw tracepoint, fentry/fexit/fmod_ret, etc. This module will be auto-loaded by
test_progs test runner and expected by some of selftests to be present and
loaded.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/testing/selftests/bpf/.gitignore | 1 +
tools/testing/selftests/bpf/Makefile | 12 +++--
.../selftests/bpf/bpf_sidecar/.gitignore | 6 +++
.../selftests/bpf/bpf_sidecar/Makefile | 20 +++++++
.../bpf/bpf_sidecar/bpf_sidecar-events.h | 36 +++++++++++++
.../selftests/bpf/bpf_sidecar/bpf_sidecar.c | 51 ++++++++++++++++++
.../selftests/bpf/bpf_sidecar/bpf_sidecar.h | 14 +++++
tools/testing/selftests/bpf/test_progs.c | 52 +++++++++++++++++++
8 files changed, 189 insertions(+), 3 deletions(-)
create mode 100644 tools/testing/selftests/bpf/bpf_sidecar/.gitignore
create mode 100644 tools/testing/selftests/bpf/bpf_sidecar/Makefile
create mode 100644 tools/testing/selftests/bpf/bpf_sidecar/bpf_sidecar-events.h
create mode 100644 tools/testing/selftests/bpf/bpf_sidecar/bpf_sidecar.c
create mode 100644 tools/testing/selftests/bpf/bpf_sidecar/bpf_sidecar.h
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 395ae040ce1f..752d8edddc66 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -35,3 +35,4 @@ test_cpp
/tools
/runqslower
/bench
+*.ko
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 3d5940cd110d..c099f8601604 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -82,7 +82,7 @@ TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
test_lirc_mode2_user xdping test_cpp runqslower bench
-TEST_CUSTOM_PROGS = urandom_read
+TEST_CUSTOM_PROGS = urandom_read bpf_sidecar.ko
# Emit succinct information message describing current building step
# $1 - generic step name (e.g., CC, LINK, etc);
@@ -104,6 +104,7 @@ OVERRIDE_TARGETS := 1
override define CLEAN
$(call msg,CLEAN)
$(Q)$(RM) -r $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) $(EXTRA_CLEAN)
+ $(Q)$(MAKE) -C bpf_sidecar clean
endef
include ../lib.mk
@@ -136,6 +137,11 @@ $(OUTPUT)/urandom_read: urandom_read.c
$(call msg,BINARY,,$@)
$(Q)$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS) -Wl,--build-id=sha1
+$(OUTPUT)/bpf_sidecar.ko: $(wildcard bpf_sidecar/Makefile bpf_sidecar/*.[ch])
+ $(call msg,MOD,,$@)
+ $(Q)$(MAKE) $(submake_extras) -C bpf_sidecar
+ $(Q)cp bpf_sidecar/bpf_sidecar.ko $@
+
$(OUTPUT)/test_stub.o: test_stub.c $(BPFOBJ)
$(call msg,CC,,$@)
$(Q)$(CC) -c $(CFLAGS) -o $@ $<
@@ -388,7 +394,7 @@ TRUNNER_BPF_PROGS_DIR := progs
TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \
network_helpers.c testing_helpers.c \
btf_helpers.c flow_dissector_load.h
-TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \
+TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_sidecar.ko \
$(wildcard progs/btf_dump_test_case_*.c)
TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE
TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS)
@@ -459,4 +465,4 @@ $(OUTPUT)/bench: $(OUTPUT)/bench.o $(OUTPUT)/testing_helpers.o \
EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) \
prog_tests/tests.h map_tests/tests.h verifier/tests.h \
feature \
- $(addprefix $(OUTPUT)/,*.o *.skel.h no_alu32 bpf_gcc)
+ $(addprefix $(OUTPUT)/,*.o *.skel.h no_alu32 bpf_gcc bpf_sidecar.ko)
diff --git a/tools/testing/selftests/bpf/bpf_sidecar/.gitignore b/tools/testing/selftests/bpf/bpf_sidecar/.gitignore
new file mode 100644
index 000000000000..ded513777281
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_sidecar/.gitignore
@@ -0,0 +1,6 @@
+*.mod
+*.mod.c
+*.o
+.ko
+/Module.symvers
+/modules.order
diff --git a/tools/testing/selftests/bpf/bpf_sidecar/Makefile b/tools/testing/selftests/bpf/bpf_sidecar/Makefile
new file mode 100644
index 000000000000..41b2339312a1
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_sidecar/Makefile
@@ -0,0 +1,20 @@
+BPF_SIDECAR_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
+KDIR ?= $(abspath $(BPF_SIDECAR_DIR)/../../../../..)
+
+ifeq ($(V),1)
+Q =
+else
+Q = @
+endif
+
+MODULES = bpf_sidecar.ko
+
+obj-m += bpf_sidecar.o
+CFLAGS_bpf_sidecar.o = -I$(src)
+
+all:
+ +$(Q)make -C $(KDIR) M=$(BPF_SIDECAR_DIR) modules
+
+clean:
+ +$(Q)make -C $(KDIR) M=$(BPF_SIDECAR_DIR) clean
+
diff --git a/tools/testing/selftests/bpf/bpf_sidecar/bpf_sidecar-events.h b/tools/testing/selftests/bpf/bpf_sidecar/bpf_sidecar-events.h
new file mode 100644
index 000000000000..433fca06a7b8
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_sidecar/bpf_sidecar-events.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2020 Facebook */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM bpf_sidecar
+
+#if !defined(_BPF_SIDECAR_EVENTS_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _BPF_SIDECAR_EVENTS_H
+
+#include <linux/tracepoint.h>
+#include "bpf_sidecar.h"
+
+TRACE_EVENT(bpf_sidecar_test_read,
+ TP_PROTO(struct task_struct *task, struct bpf_sidecar_test_read_ctx *ctx),
+ TP_ARGS(task, ctx),
+ TP_STRUCT__entry(
+ __field(pid_t, pid)
+ __array(char, comm, TASK_COMM_LEN)
+ __field(loff_t, off)
+ __field(size_t, len)
+ ),
+ TP_fast_assign(
+ __entry->pid = task->pid;
+ memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
+ __entry->off = ctx->off;
+ __entry->len = ctx->len;
+ ),
+ TP_printk("pid=%d comm=%s off=%llu len=%zu",
+ __entry->pid, __entry->comm, __entry->off, __entry->len)
+);
+
+#endif /* _BPF_SIDECAR_EVENTS_H */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#define TRACE_INCLUDE_FILE bpf_sidecar-events
+#include <trace/define_trace.h>
diff --git a/tools/testing/selftests/bpf/bpf_sidecar/bpf_sidecar.c b/tools/testing/selftests/bpf/bpf_sidecar/bpf_sidecar.c
new file mode 100644
index 000000000000..46f48c20d99b
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_sidecar/bpf_sidecar.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020 Facebook */
+#include <linux/error-injection.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/sysfs.h>
+#include <linux/tracepoint.h>
+#include "bpf_sidecar.h"
+
+#define CREATE_TRACE_POINTS
+#include "bpf_sidecar-events.h"
+
+static noinline ssize_t
+bpf_sidecar_test_read(struct file *file, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t off, size_t len)
+{
+ struct bpf_sidecar_test_read_ctx ctx = {
+ .buf = buf,
+ .off = off,
+ .len = len,
+ };
+
+ trace_bpf_sidecar_test_read(current, &ctx);
+
+ return -EIO; /* always fail */
+}
+ALLOW_ERROR_INJECTION(bpf_sidecar_test_read, ERRNO);
+
+static struct bin_attribute bin_attr_bpf_sidecar_file __ro_after_init = {
+ .attr = { .name = "bpf_sidecar", .mode = 0444, },
+ .read = bpf_sidecar_test_read,
+};
+
+static int bpf_sidecar_init(void)
+{
+ return sysfs_create_bin_file(kernel_kobj, &bin_attr_bpf_sidecar_file);
+}
+
+static void bpf_sidecar_exit(void)
+{
+ return sysfs_remove_bin_file(kernel_kobj, &bin_attr_bpf_sidecar_file);
+}
+
+module_init(bpf_sidecar_init);
+module_exit(bpf_sidecar_exit);
+
+MODULE_AUTHOR("Andrii Nakryiko");
+MODULE_DESCRIPTION("BPF selftests sidecar module");
+MODULE_LICENSE("Dual BSD/GPL");
+
diff --git a/tools/testing/selftests/bpf/bpf_sidecar/bpf_sidecar.h b/tools/testing/selftests/bpf/bpf_sidecar/bpf_sidecar.h
new file mode 100644
index 000000000000..35668d0803ff
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_sidecar/bpf_sidecar.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2020 Facebook */
+#ifndef _BPF_SIDECAR_H
+#define _BPF_SIDECAR_H
+
+#include <linux/types.h>
+
+struct bpf_sidecar_test_read_ctx {
+ char *buf;
+ loff_t off;
+ size_t len;
+};
+
+#endif /* _BPF_SIDECAR_H */
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 22943b58d752..544630202247 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -360,6 +360,56 @@ int extract_build_id(char *build_id, size_t size)
return -1;
}
+static int finit_module(int fd, const char *param_values, int flags)
+{
+ return syscall(__NR_finit_module, fd, param_values, flags);
+}
+
+static int delete_module(const char *name, int flags)
+{
+ return syscall(__NR_delete_module, name, flags);
+}
+
+void unload_bpf_sidecar_module()
+{
+ if (delete_module("bpf_sidecar", 0)) {
+ if (errno == ENOENT) {
+ if (env.verbosity > VERBOSE_NONE)
+ fprintf(stdout, "bpf_sidecar.ko is already unloaded.\n");
+ return;
+ }
+ fprintf(env.stderr, "Failed to unload bpf_sidecar.ko from kernel: %d\n", -errno);
+ exit(1);
+ }
+ if (env.verbosity > VERBOSE_NONE)
+ fprintf(stdout, "Successfully unloaded bpf_sidecar.ko.\n");
+}
+
+void load_bpf_sidecar_module()
+{
+ int fd;
+
+ /* ensure previous instance of the module is unloaded */
+ unload_bpf_sidecar_module();
+
+ if (env.verbosity > VERBOSE_NONE)
+ fprintf(stdout, "Loading bpf_sidecar.ko...\n");
+
+ fd = open("bpf_sidecar.ko", O_RDONLY);
+ if (fd < 0) {
+ fprintf(env.stderr, "Can't find bpf_sidecar.ko kernel module: %d\n", -errno);
+ exit(1);
+ }
+ if (finit_module(fd, "", 0)) {
+ fprintf(env.stderr, "Failed to load bpf_sidecar.ko into the kernel: %d\n", -errno);
+ exit(1);
+ }
+ close(fd);
+
+ if (env.verbosity > VERBOSE_NONE)
+ fprintf(stdout, "Successfully loaded bpf_sidecar.ko.\n");
+}
+
/* extern declarations for test funcs */
#define DEFINE_TEST(name) extern void test_##name(void);
#include <prog_tests/tests.h>
@@ -678,6 +728,7 @@ int main(int argc, char **argv)
save_netns();
stdio_hijack();
+ load_bpf_sidecar_module();
for (i = 0; i < prog_test_cnt; i++) {
struct prog_test_def *test = &prog_test_defs[i];
@@ -722,6 +773,7 @@ int main(int argc, char **argv)
if (test->need_cgroup_cleanup)
cleanup_cgroup_environment();
}
+ unload_bpf_sidecar_module();
stdio_restore();
if (env.get_test_cnt) {
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next 5/6] selftests/bpf: add bpf_sidecar kernel module for testing
2020-11-19 23:22 ` [PATCH bpf-next 5/6] selftests/bpf: add bpf_sidecar kernel module for testing Andrii Nakryiko
@ 2020-11-20 23:21 ` Martin KaFai Lau
0 siblings, 0 replies; 21+ messages in thread
From: Martin KaFai Lau @ 2020-11-20 23:21 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, netdev, ast, daniel, kernel-team
On Thu, Nov 19, 2020 at 03:22:43PM -0800, Andrii Nakryiko wrote:
> Add bpf_sidecar module, which is conceptually out-of-tree module and provides
> ways for selftests/bpf to test various kernel module-related functionality:
> raw tracepoint, fentry/fexit/fmod_ret, etc. This module will be auto-loaded by
> test_progs test runner and expected by some of selftests to be present and
> loaded.
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH bpf-next 6/6] selftests/bpf: add CO-RE relocs selftest relying on kernel module BTF
2020-11-19 23:22 [PATCH bpf-next 0/6] libbpf: add support for kernel module BTF CO-RE relocations Andrii Nakryiko
` (4 preceding siblings ...)
2020-11-19 23:22 ` [PATCH bpf-next 5/6] selftests/bpf: add bpf_sidecar kernel module for testing Andrii Nakryiko
@ 2020-11-19 23:22 ` Andrii Nakryiko
2020-11-20 23:27 ` Martin KaFai Lau
5 siblings, 1 reply; 21+ messages in thread
From: Andrii Nakryiko @ 2020-11-19 23:22 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team
Add a self-tests validating libbpf is able to perform CO-RE relocations
against the type defined in kernel module BTF.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
.../selftests/bpf/prog_tests/core_reloc.c | 72 ++++++++++++++++---
.../selftests/bpf/progs/core_reloc_types.h | 17 +++++
.../bpf/progs/test_core_reloc_module.c | 66 +++++++++++++++++
3 files changed, 144 insertions(+), 11 deletions(-)
create mode 100644 tools/testing/selftests/bpf/progs/test_core_reloc_module.c
diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
index 30e40ff4b0d8..b3f14e3d9077 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <test_progs.h>
#include "progs/core_reloc_types.h"
+#include "bpf_sidecar/bpf_sidecar.h"
#include <sys/mman.h>
#include <sys/syscall.h>
#include <bpf/btf.h>
@@ -9,6 +10,29 @@ static int duration = 0;
#define STRUCT_TO_CHAR_PTR(struct_name) (const char *)&(struct struct_name)
+#define MODULES_CASE(name, sec_name, tp_name) { \
+ .case_name = name, \
+ .bpf_obj_file = "test_core_reloc_module.o", \
+ .btf_src_file = NULL, /* find in kernel module BTFs */ \
+ .input = "", \
+ .input_len = 0, \
+ .output = STRUCT_TO_CHAR_PTR(core_reloc_module_output) { \
+ .read_ctx_sz = sizeof(struct bpf_sidecar_test_read_ctx),\
+ .read_ctx_exists = true, \
+ .buf_exists = true, \
+ .len_exists = true, \
+ .off_exists = true, \
+ .len = 123, \
+ .off = 0, \
+ .comm = "test_progs", \
+ .comm_len = sizeof("test_progs"), \
+ }, \
+ .output_len = sizeof(struct core_reloc_module_output), \
+ .prog_sec_name = sec_name, \
+ .raw_tp_name = tp_name, \
+ .trigger = trigger_module_test_read, \
+}
+
#define FLAVORS_DATA(struct_name) STRUCT_TO_CHAR_PTR(struct_name) { \
.a = 42, \
.b = 0xc001, \
@@ -211,7 +235,7 @@ static int duration = 0;
.output = STRUCT_TO_CHAR_PTR(core_reloc_bitfields_output) \
__VA_ARGS__, \
.output_len = sizeof(struct core_reloc_bitfields_output), \
- .direct_raw_tp = true, \
+ .prog_sec_name = "tp_btf/sys_enter", \
}
@@ -222,7 +246,7 @@ static int duration = 0;
}, { \
BITFIELDS_CASE_COMMON("test_core_reloc_bitfields_direct.o", \
"direct:", name), \
- .direct_raw_tp = true, \
+ .prog_sec_name = "tp_btf/sys_enter", \
.fails = true, \
}
@@ -309,6 +333,7 @@ static int duration = 0;
struct core_reloc_test_case;
typedef int (*setup_test_fn)(struct core_reloc_test_case *test);
+typedef int (*trigger_test_fn)(const struct core_reloc_test_case *test);
struct core_reloc_test_case {
const char *case_name;
@@ -320,8 +345,10 @@ struct core_reloc_test_case {
int output_len;
bool fails;
bool relaxed_core_relocs;
- bool direct_raw_tp;
+ const char *prog_sec_name;
+ const char *raw_tp_name;
setup_test_fn setup;
+ trigger_test_fn trigger;
};
static int find_btf_type(const struct btf *btf, const char *name, __u32 kind)
@@ -451,6 +478,23 @@ static int setup_type_id_case_failure(struct core_reloc_test_case *test)
return 0;
}
+static int trigger_module_test_read(const struct core_reloc_test_case *test)
+{
+ struct core_reloc_module_output *exp = (void *)test->output;
+ int fd, err;
+
+ fd = open("/sys/kernel/bpf_sidecar", O_RDONLY);
+ err = -errno;
+ if (CHECK(fd < 0, "sidecar_file_open", "failed: %d\n", err))
+ return err;
+
+ read(fd, NULL, exp->len); /* request expected number of bytes */
+ close(fd);
+
+ return 0;
+}
+
+
static struct core_reloc_test_case test_cases[] = {
/* validate we can find kernel image and use its BTF for relocs */
{
@@ -467,6 +511,9 @@ static struct core_reloc_test_case test_cases[] = {
.output_len = sizeof(struct core_reloc_kernel_output),
},
+ /* validate we can find kernel module BTF types for relocs/attach */
+ MODULES_CASE("module", "raw_tp/bpf_sidecar_test_read", "bpf_sidecar_test_read"),
+
/* validate BPF program can use multiple flavors to match against
* single target BTF type
*/
@@ -790,13 +837,11 @@ void test_core_reloc(void)
test_case->bpf_obj_file, PTR_ERR(obj)))
continue;
- /* for typed raw tracepoints, NULL should be specified */
- if (test_case->direct_raw_tp) {
- probe_name = "tp_btf/sys_enter";
- tp_name = NULL;
- } else {
- probe_name = "raw_tracepoint/sys_enter";
- tp_name = "sys_enter";
+ probe_name = "raw_tracepoint/sys_enter";
+ tp_name = "sys_enter";
+ if (test_case->prog_sec_name) {
+ probe_name = test_case->prog_sec_name;
+ tp_name = test_case->raw_tp_name; /* NULL for tp_btf */
}
prog = bpf_object__find_program_by_title(obj, probe_name);
@@ -837,7 +882,12 @@ void test_core_reloc(void)
goto cleanup;
/* trigger test run */
- usleep(1);
+ if (test_case->trigger) {
+ if (!ASSERT_OK(test_case->trigger(test_case), "test_trigger"))
+ goto cleanup;
+ } else {
+ usleep(1);
+ }
if (data->skip) {
test__skip();
diff --git a/tools/testing/selftests/bpf/progs/core_reloc_types.h b/tools/testing/selftests/bpf/progs/core_reloc_types.h
index e6e616cb7bc9..9a2850850121 100644
--- a/tools/testing/selftests/bpf/progs/core_reloc_types.h
+++ b/tools/testing/selftests/bpf/progs/core_reloc_types.h
@@ -15,6 +15,23 @@ struct core_reloc_kernel_output {
int comm_len;
};
+/*
+ * MODULE
+ */
+
+struct core_reloc_module_output {
+ long long len;
+ long long off;
+ int read_ctx_sz;
+ bool read_ctx_exists;
+ bool buf_exists;
+ bool len_exists;
+ bool off_exists;
+ /* we have test_progs[-flavor], so cut flavor part */
+ char comm[sizeof("test_progs")];
+ int comm_len;
+};
+
/*
* FLAVORS
*/
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_module.c b/tools/testing/selftests/bpf/progs/test_core_reloc_module.c
new file mode 100644
index 000000000000..4630301de259
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_module.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020 Facebook */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_core_read.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+struct bpf_sidecar_test_read_ctx {
+ /* field order is mixed up */
+ size_t len;
+ char *buf;
+ loff_t off;
+} __attribute__((preserve_access_index));
+
+struct {
+ char in[256];
+ char out[256];
+ bool skip;
+ uint64_t my_pid_tgid;
+} data = {};
+
+struct core_reloc_module_output {
+ long long len;
+ long long off;
+ int read_ctx_sz;
+ bool read_ctx_exists;
+ bool buf_exists;
+ bool len_exists;
+ bool off_exists;
+ /* we have test_progs[-flavor], so cut flavor part */
+ char comm[sizeof("test_progs")];
+ int comm_len;
+};
+
+SEC("raw_tp/bpf_sidecar_test_read")
+int BPF_PROG(test_core_module,
+ struct task_struct *task,
+ struct bpf_sidecar_test_read_ctx *read_ctx)
+{
+ struct core_reloc_module_output *out = (void *)&data.out;
+ __u64 pid_tgid = bpf_get_current_pid_tgid();
+ __u32 real_tgid = (__u32)(pid_tgid >> 32);
+ __u32 real_pid = (__u32)pid_tgid;
+
+ if (data.my_pid_tgid != pid_tgid)
+ return 0;
+
+ if (BPF_CORE_READ(task, pid) != real_pid || BPF_CORE_READ(task, tgid) != real_tgid)
+ return 0;
+
+ out->len = BPF_CORE_READ(read_ctx, len);
+ out->off = BPF_CORE_READ(read_ctx, off);
+
+ out->read_ctx_sz = bpf_core_type_size(struct bpf_sidecar_test_read_ctx);
+ out->read_ctx_exists = bpf_core_type_exists(struct bpf_sidecar_test_read_ctx);
+ out->buf_exists = bpf_core_field_exists(read_ctx->buf);
+ out->off_exists = bpf_core_field_exists(read_ctx->off);
+ out->len_exists = bpf_core_field_exists(read_ctx->len);
+
+ out->comm_len = BPF_CORE_READ_STR_INTO(&out->comm, task, comm);
+
+ return 0;
+}
--
2.24.1
^ permalink raw reply related [flat|nested] 21+ messages in thread