* [PATCH bpf-next v4 0/3] libbpf: Add support for dynamic program attach target
@ 2020-02-17 12:42 Eelco Chaudron
2020-02-17 12:43 ` [PATCH bpf-next v4 1/3] libbpf: Bump libpf current version to v0.0.8 Eelco Chaudron
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Eelco Chaudron @ 2020-02-17 12:42 UTC (permalink / raw)
To: bpf; +Cc: davem, netdev, ast, daniel, kafai, songliubraving, yhs, andriin, toke
Currently when you want to attach a trace program to a bpf program
the section name needs to match the tracepoint/function semantics.
However the addition of the bpf_program__set_attach_target() API
allows you to specify the tracepoint/function dynamically.
The call flow would look something like this:
xdp_fd = bpf_prog_get_fd_by_id(id);
trace_obj = bpf_object__open_file("func.o", NULL);
prog = bpf_object__find_program_by_title(trace_obj,
"fentry/myfunc");
bpf_program__set_expected_attach_type(prog, BPF_TRACE_FENTRY);
bpf_program__set_attach_target(prog, xdp_fd,
"xdpfilt_blk_all");
bpf_object__load(trace_obj)
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
cked-by: Toke Høiland-Jørgensen <toke@redhat.com>
v1 -> v2: Remove requirement for attach type hint in API
v2 -> v3: Moved common warning to __find_vmlinux_btf_id, requested by Andrii
Updated the xdp_bpf2bpf test to use this new API
v4 -> v4: Split up patch, update libbpf.map version
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH bpf-next v4 1/3] libbpf: Bump libpf current version to v0.0.8
2020-02-17 12:42 [PATCH bpf-next v4 0/3] libbpf: Add support for dynamic program attach target Eelco Chaudron
@ 2020-02-17 12:43 ` Eelco Chaudron
2020-02-17 12:43 ` [PATCH bpf-next v4 2/3] libbpf: Add support for dynamic program attach target Eelco Chaudron
2020-02-17 12:43 ` [PATCH bpf-next v4 3/3] selftests/bpf: update xdp_bpf2bpf test to use new set_attach_target API Eelco Chaudron
2 siblings, 0 replies; 11+ messages in thread
From: Eelco Chaudron @ 2020-02-17 12:43 UTC (permalink / raw)
To: bpf; +Cc: davem, netdev, ast, daniel, kafai, songliubraving, yhs, andriin, toke
New development cycles starts, bump to v0.0.8.
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
tools/lib/bpf/libbpf.map | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index b035122142bb..45be19c9d752 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -235,3 +235,6 @@ LIBBPF_0.0.7 {
btf__align_of;
libbpf_find_kernel_btf;
} LIBBPF_0.0.6;
+
+LIBBPF_0.0.8 {
+} LIBBPF_0.0.7;
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH bpf-next v4 2/3] libbpf: Add support for dynamic program attach target
2020-02-17 12:42 [PATCH bpf-next v4 0/3] libbpf: Add support for dynamic program attach target Eelco Chaudron
2020-02-17 12:43 ` [PATCH bpf-next v4 1/3] libbpf: Bump libpf current version to v0.0.8 Eelco Chaudron
@ 2020-02-17 12:43 ` Eelco Chaudron
2020-02-18 16:34 ` Jakub Sitnicki
2020-02-17 12:43 ` [PATCH bpf-next v4 3/3] selftests/bpf: update xdp_bpf2bpf test to use new set_attach_target API Eelco Chaudron
2 siblings, 1 reply; 11+ messages in thread
From: Eelco Chaudron @ 2020-02-17 12:43 UTC (permalink / raw)
To: bpf; +Cc: davem, netdev, ast, daniel, kafai, songliubraving, yhs, andriin, toke
Currently when you want to attach a trace program to a bpf program
the section name needs to match the tracepoint/function semantics.
However the addition of the bpf_program__set_attach_target() API
allows you to specify the tracepoint/function dynamically.
The call flow would look something like this:
xdp_fd = bpf_prog_get_fd_by_id(id);
trace_obj = bpf_object__open_file("func.o", NULL);
prog = bpf_object__find_program_by_title(trace_obj,
"fentry/myfunc");
bpf_program__set_expected_attach_type(prog, BPF_TRACE_FENTRY);
bpf_program__set_attach_target(prog, xdp_fd,
"xdpfilt_blk_all");
bpf_object__load(trace_obj)
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
tools/lib/bpf/libbpf.c | 34 ++++++++++++++++++++++++++++++----
tools/lib/bpf/libbpf.h | 4 ++++
tools/lib/bpf/libbpf.map | 2 ++
3 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 514b1a524abb..0c25d78fb5d8 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4939,8 +4939,8 @@ int bpf_program__load(struct bpf_program *prog, char *license, __u32 kern_ver)
{
int err = 0, fd, i, btf_id;
- if (prog->type == BPF_PROG_TYPE_TRACING ||
- prog->type == BPF_PROG_TYPE_EXT) {
+ if ((prog->type == BPF_PROG_TYPE_TRACING ||
+ prog->type == BPF_PROG_TYPE_EXT) && !prog->attach_btf_id) {
btf_id = libbpf_find_attach_btf_id(prog);
if (btf_id <= 0)
return btf_id;
@@ -6583,6 +6583,9 @@ 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;
}
@@ -6655,8 +6658,6 @@ static int libbpf_find_attach_btf_id(struct bpf_program *prog)
err = __find_vmlinux_btf_id(prog->obj->btf_vmlinux,
name + section_defs[i].len,
attach_type);
- if (err <= 0)
- pr_warn("%s is not found in vmlinux BTF\n", name);
return err;
}
pr_warn("failed to identify btf_id based on ELF section name '%s'\n", name);
@@ -8132,6 +8133,31 @@ void bpf_program__bpil_offs_to_addr(struct bpf_prog_info_linear *info_linear)
}
}
+int bpf_program__set_attach_target(struct bpf_program *prog,
+ int attach_prog_fd,
+ const char *attach_func_name)
+{
+ int btf_id;
+
+ if (!prog || attach_prog_fd < 0 || !attach_func_name)
+ return -EINVAL;
+
+ if (attach_prog_fd)
+ btf_id = libbpf_find_prog_btf_id(attach_func_name,
+ attach_prog_fd);
+ else
+ btf_id = __find_vmlinux_btf_id(prog->obj->btf_vmlinux,
+ attach_func_name,
+ prog->expected_attach_type);
+
+ if (btf_id <= 0)
+ return btf_id;
+
+ prog->attach_btf_id = btf_id;
+ prog->attach_prog_fd = attach_prog_fd;
+ return 0;
+}
+
int parse_cpu_mask_str(const char *s, bool **mask, int *mask_sz)
{
int err = 0, n, len, start, end = -1;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 3fe12c9d1f92..02fc58a21a7f 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -334,6 +334,10 @@ LIBBPF_API void
bpf_program__set_expected_attach_type(struct bpf_program *prog,
enum bpf_attach_type type);
+LIBBPF_API int
+bpf_program__set_attach_target(struct bpf_program *prog, int attach_prog_fd,
+ const char *attach_func_name);
+
LIBBPF_API bool bpf_program__is_socket_filter(const struct bpf_program *prog);
LIBBPF_API bool bpf_program__is_tracepoint(const struct bpf_program *prog);
LIBBPF_API bool bpf_program__is_raw_tracepoint(const struct bpf_program *prog);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 45be19c9d752..7b014c8cdece 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -237,4 +237,6 @@ LIBBPF_0.0.7 {
} LIBBPF_0.0.6;
LIBBPF_0.0.8 {
+ global:
+ bpf_program__set_attach_target;
} LIBBPF_0.0.7;
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH bpf-next v4 3/3] selftests/bpf: update xdp_bpf2bpf test to use new set_attach_target API
2020-02-17 12:42 [PATCH bpf-next v4 0/3] libbpf: Add support for dynamic program attach target Eelco Chaudron
2020-02-17 12:43 ` [PATCH bpf-next v4 1/3] libbpf: Bump libpf current version to v0.0.8 Eelco Chaudron
2020-02-17 12:43 ` [PATCH bpf-next v4 2/3] libbpf: Add support for dynamic program attach target Eelco Chaudron
@ 2020-02-17 12:43 ` Eelco Chaudron
2020-02-18 21:21 ` Andrii Nakryiko
2 siblings, 1 reply; 11+ messages in thread
From: Eelco Chaudron @ 2020-02-17 12:43 UTC (permalink / raw)
To: bpf; +Cc: davem, netdev, ast, daniel, kafai, songliubraving, yhs, andriin, toke
Use the new bpf_program__set_attach_target() API in the xdp_bpf2bpf
selftest so it can be referenced as an example on how to use it.
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
.../testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c | 16 +++++++++++++---
.../testing/selftests/bpf/progs/test_xdp_bpf2bpf.c | 4 ++--
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c
index 6b56bdc73ebc..513fdbf02b81 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c
@@ -14,7 +14,7 @@ void test_xdp_bpf2bpf(void)
struct test_xdp *pkt_skel = NULL;
struct test_xdp_bpf2bpf *ftrace_skel = NULL;
struct vip key4 = {.protocol = 6, .family = AF_INET};
- DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts);
+ struct bpf_program *prog;
/* Load XDP program to introspect */
pkt_skel = test_xdp__open_and_load();
@@ -27,11 +27,21 @@ void test_xdp_bpf2bpf(void)
bpf_map_update_elem(map_fd, &key4, &value4, 0);
/* Load trace program */
- opts.attach_prog_fd = pkt_fd,
- ftrace_skel = test_xdp_bpf2bpf__open_opts(&opts);
+ ftrace_skel = test_xdp_bpf2bpf__open();
if (CHECK(!ftrace_skel, "__open", "ftrace skeleton failed\n"))
goto out;
+ /* Demonstrate the bpf_program__set_attach_target() API rather than
+ * the load with options, i.e. opts.attach_prog_fd.
+ */
+ prog = *ftrace_skel->skeleton->progs[0].prog;
+ bpf_program__set_expected_attach_type(prog, BPF_TRACE_FENTRY);
+ bpf_program__set_attach_target(prog, pkt_fd, "_xdp_tx_iptunnel");
+
+ prog = *ftrace_skel->skeleton->progs[1].prog;
+ bpf_program__set_expected_attach_type(prog, BPF_TRACE_FEXIT);
+ bpf_program__set_attach_target(prog, pkt_fd, "_xdp_tx_iptunnel");
+
err = test_xdp_bpf2bpf__load(ftrace_skel);
if (CHECK(err, "__load", "ftrace skeleton failed\n"))
goto out;
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c b/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c
index cb8a04ab7a78..b840fc9e3ed5 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c
@@ -28,7 +28,7 @@ struct xdp_buff {
} __attribute__((preserve_access_index));
__u64 test_result_fentry = 0;
-SEC("fentry/_xdp_tx_iptunnel")
+SEC("fentry/FUNC")
int BPF_PROG(trace_on_entry, struct xdp_buff *xdp)
{
test_result_fentry = xdp->rxq->dev->ifindex;
@@ -36,7 +36,7 @@ int BPF_PROG(trace_on_entry, struct xdp_buff *xdp)
}
__u64 test_result_fexit = 0;
-SEC("fexit/_xdp_tx_iptunnel")
+SEC("fexit/FUNC")
int BPF_PROG(trace_on_exit, struct xdp_buff *xdp, int ret)
{
test_result_fexit = ret;
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v4 2/3] libbpf: Add support for dynamic program attach target
2020-02-17 12:43 ` [PATCH bpf-next v4 2/3] libbpf: Add support for dynamic program attach target Eelco Chaudron
@ 2020-02-18 16:34 ` Jakub Sitnicki
2020-02-18 21:24 ` Andrii Nakryiko
0 siblings, 1 reply; 11+ messages in thread
From: Jakub Sitnicki @ 2020-02-18 16:34 UTC (permalink / raw)
To: Eelco Chaudron
Cc: bpf, davem, netdev, ast, daniel, kafai, songliubraving, yhs,
andriin, toke
Hey Eelco,
On Mon, Feb 17, 2020 at 12:43 PM GMT, Eelco Chaudron wrote:
> Currently when you want to attach a trace program to a bpf program
> the section name needs to match the tracepoint/function semantics.
>
> However the addition of the bpf_program__set_attach_target() API
> allows you to specify the tracepoint/function dynamically.
>
> The call flow would look something like this:
>
> xdp_fd = bpf_prog_get_fd_by_id(id);
> trace_obj = bpf_object__open_file("func.o", NULL);
> prog = bpf_object__find_program_by_title(trace_obj,
> "fentry/myfunc");
> bpf_program__set_expected_attach_type(prog, BPF_TRACE_FENTRY);
> bpf_program__set_attach_target(prog, xdp_fd,
> "xdpfilt_blk_all");
> bpf_object__load(trace_obj)
>
> Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> ---
> tools/lib/bpf/libbpf.c | 34 ++++++++++++++++++++++++++++++----
> tools/lib/bpf/libbpf.h | 4 ++++
> tools/lib/bpf/libbpf.map | 2 ++
> 3 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 514b1a524abb..0c25d78fb5d8 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
[...]
> @@ -8132,6 +8133,31 @@ void bpf_program__bpil_offs_to_addr(struct bpf_prog_info_linear *info_linear)
> }
> }
>
> +int bpf_program__set_attach_target(struct bpf_program *prog,
> + int attach_prog_fd,
> + const char *attach_func_name)
> +{
> + int btf_id;
> +
> + if (!prog || attach_prog_fd < 0 || !attach_func_name)
> + return -EINVAL;
> +
> + if (attach_prog_fd)
> + btf_id = libbpf_find_prog_btf_id(attach_func_name,
> + attach_prog_fd);
> + else
> + btf_id = __find_vmlinux_btf_id(prog->obj->btf_vmlinux,
> + attach_func_name,
> + prog->expected_attach_type);
> +
> + if (btf_id <= 0)
> + return btf_id;
Looks like we can get 0 as return value on both error and success
(below)? Is that intentional?
> +
> + prog->attach_btf_id = btf_id;
> + prog->attach_prog_fd = attach_prog_fd;
> + return 0;
> +}
> +
> int parse_cpu_mask_str(const char *s, bool **mask, int *mask_sz)
> {
> int err = 0, n, len, start, end = -1;
[...]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v4 3/3] selftests/bpf: update xdp_bpf2bpf test to use new set_attach_target API
2020-02-17 12:43 ` [PATCH bpf-next v4 3/3] selftests/bpf: update xdp_bpf2bpf test to use new set_attach_target API Eelco Chaudron
@ 2020-02-18 21:21 ` Andrii Nakryiko
2020-02-19 10:54 ` Eelco Chaudron
0 siblings, 1 reply; 11+ messages in thread
From: Andrii Nakryiko @ 2020-02-18 21:21 UTC (permalink / raw)
To: Eelco Chaudron
Cc: bpf, David S. Miller, Networking, Alexei Starovoitov,
Daniel Borkmann, Martin Lau, Song Liu, Yonghong Song,
Andrii Nakryiko, Toke Høiland-Jørgensen
On Mon, Feb 17, 2020 at 5:03 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>
> Use the new bpf_program__set_attach_target() API in the xdp_bpf2bpf
> selftest so it can be referenced as an example on how to use it.
>
>
nit: extra empty line?
> Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> ---
> .../testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c | 16 +++++++++++++---
> .../testing/selftests/bpf/progs/test_xdp_bpf2bpf.c | 4 ++--
> 2 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c
> index 6b56bdc73ebc..513fdbf02b81 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c
> @@ -14,7 +14,7 @@ void test_xdp_bpf2bpf(void)
> struct test_xdp *pkt_skel = NULL;
> struct test_xdp_bpf2bpf *ftrace_skel = NULL;
> struct vip key4 = {.protocol = 6, .family = AF_INET};
> - DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts);
> + struct bpf_program *prog;
>
> /* Load XDP program to introspect */
> pkt_skel = test_xdp__open_and_load();
> @@ -27,11 +27,21 @@ void test_xdp_bpf2bpf(void)
> bpf_map_update_elem(map_fd, &key4, &value4, 0);
>
> /* Load trace program */
> - opts.attach_prog_fd = pkt_fd,
> - ftrace_skel = test_xdp_bpf2bpf__open_opts(&opts);
> + ftrace_skel = test_xdp_bpf2bpf__open();
> if (CHECK(!ftrace_skel, "__open", "ftrace skeleton failed\n"))
> goto out;
>
> + /* Demonstrate the bpf_program__set_attach_target() API rather than
> + * the load with options, i.e. opts.attach_prog_fd.
> + */
> + prog = *ftrace_skel->skeleton->progs[0].prog;
it took me a while to understand what's going on here... :) You are
not supposed to peek into ftrace_skel->skeleton, it's an "internal"
object that's passed into libbpf.
It's better to write it as a nice and short:
prog = ftrace_skel->progs.trace_on_entry;
> + bpf_program__set_expected_attach_type(prog, BPF_TRACE_FENTRY);
> + bpf_program__set_attach_target(prog, pkt_fd, "_xdp_tx_iptunnel");
> +
> + prog = *ftrace_skel->skeleton->progs[1].prog;
same as above: ftrace_skel->progs.trace_on_exit
> + bpf_program__set_expected_attach_type(prog, BPF_TRACE_FEXIT);
> + bpf_program__set_attach_target(prog, pkt_fd, "_xdp_tx_iptunnel");
> +
> err = test_xdp_bpf2bpf__load(ftrace_skel);
> if (CHECK(err, "__load", "ftrace skeleton failed\n"))
> goto out;
> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c b/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c
> index cb8a04ab7a78..b840fc9e3ed5 100644
> --- a/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c
> +++ b/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c
> @@ -28,7 +28,7 @@ struct xdp_buff {
> } __attribute__((preserve_access_index));
>
> __u64 test_result_fentry = 0;
> -SEC("fentry/_xdp_tx_iptunnel")
> +SEC("fentry/FUNC")
> int BPF_PROG(trace_on_entry, struct xdp_buff *xdp)
> {
> test_result_fentry = xdp->rxq->dev->ifindex;
> @@ -36,7 +36,7 @@ int BPF_PROG(trace_on_entry, struct xdp_buff *xdp)
> }
>
> __u64 test_result_fexit = 0;
> -SEC("fexit/_xdp_tx_iptunnel")
> +SEC("fexit/FUNC")
> int BPF_PROG(trace_on_exit, struct xdp_buff *xdp, int ret)
> {
> test_result_fexit = ret;
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v4 2/3] libbpf: Add support for dynamic program attach target
2020-02-18 16:34 ` Jakub Sitnicki
@ 2020-02-18 21:24 ` Andrii Nakryiko
2020-02-19 11:06 ` Eelco Chaudron
0 siblings, 1 reply; 11+ messages in thread
From: Andrii Nakryiko @ 2020-02-18 21:24 UTC (permalink / raw)
To: Jakub Sitnicki
Cc: Eelco Chaudron, bpf, David S. Miller, Networking,
Alexei Starovoitov, Daniel Borkmann, Martin Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Toke Høiland-Jørgensen
On Tue, Feb 18, 2020 at 8:34 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>
> Hey Eelco,
>
> On Mon, Feb 17, 2020 at 12:43 PM GMT, Eelco Chaudron wrote:
> > Currently when you want to attach a trace program to a bpf program
> > the section name needs to match the tracepoint/function semantics.
> >
> > However the addition of the bpf_program__set_attach_target() API
> > allows you to specify the tracepoint/function dynamically.
> >
> > The call flow would look something like this:
> >
> > xdp_fd = bpf_prog_get_fd_by_id(id);
> > trace_obj = bpf_object__open_file("func.o", NULL);
> > prog = bpf_object__find_program_by_title(trace_obj,
> > "fentry/myfunc");
> > bpf_program__set_expected_attach_type(prog, BPF_TRACE_FENTRY);
> > bpf_program__set_attach_target(prog, xdp_fd,
> > "xdpfilt_blk_all");
> > bpf_object__load(trace_obj)
> >
> > Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
> > Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> > ---
> > tools/lib/bpf/libbpf.c | 34 ++++++++++++++++++++++++++++++----
> > tools/lib/bpf/libbpf.h | 4 ++++
> > tools/lib/bpf/libbpf.map | 2 ++
> > 3 files changed, 36 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 514b1a524abb..0c25d78fb5d8 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
>
> [...]
>
> > @@ -8132,6 +8133,31 @@ void bpf_program__bpil_offs_to_addr(struct bpf_prog_info_linear *info_linear)
> > }
> > }
> >
> > +int bpf_program__set_attach_target(struct bpf_program *prog,
> > + int attach_prog_fd,
> > + const char *attach_func_name)
> > +{
> > + int btf_id;
> > +
> > + if (!prog || attach_prog_fd < 0 || !attach_func_name)
> > + return -EINVAL;
> > +
> > + if (attach_prog_fd)
> > + btf_id = libbpf_find_prog_btf_id(attach_func_name,
> > + attach_prog_fd);
> > + else
> > + btf_id = __find_vmlinux_btf_id(prog->obj->btf_vmlinux,
> > + attach_func_name,
> > + prog->expected_attach_type);
> > +
> > + if (btf_id <= 0)
> > + return btf_id;
>
> Looks like we can get 0 as return value on both error and success
> (below)? Is that intentional?
Neither libbpf_find_prog_btf_id nor __find_vmlinux_btf_id are going to
return 0 on failure. But I do agree that if (btf_id < 0) check would
be better here.
With that minor nit:
Acked-by: Andrii Nakryiko <andriin@fb.com>
>
> > +
> > + prog->attach_btf_id = btf_id;
> > + prog->attach_prog_fd = attach_prog_fd;
> > + return 0;
> > +}
> > +
> > int parse_cpu_mask_str(const char *s, bool **mask, int *mask_sz)
> > {
> > int err = 0, n, len, start, end = -1;
>
> [...]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v4 3/3] selftests/bpf: update xdp_bpf2bpf test to use new set_attach_target API
2020-02-18 21:21 ` Andrii Nakryiko
@ 2020-02-19 10:54 ` Eelco Chaudron
0 siblings, 0 replies; 11+ messages in thread
From: Eelco Chaudron @ 2020-02-19 10:54 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, David S. Miller, Networking, Alexei Starovoitov,
Daniel Borkmann, Martin Lau, Song Liu, Yonghong Song,
Andrii Nakryiko, Toke Høiland-Jørgensen
On 18 Feb 2020, at 22:21, Andrii Nakryiko wrote:
> On Mon, Feb 17, 2020 at 5:03 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>>
>> Use the new bpf_program__set_attach_target() API in the xdp_bpf2bpf
>> selftest so it can be referenced as an example on how to use it.
>>
>>
>
> nit: extra empty line?
ACK <SNIP>
>> + prog = *ftrace_skel->skeleton->progs[0].prog;
>
> it took me a while to understand what's going on here... :) You are
> not supposed to peek into ftrace_skel->skeleton, it's an "internal"
> object that's passed into libbpf.
>
> It's better to write it as a nice and short:
>
> prog = ftrace_skel->progs.trace_on_entry;
>
Will change in next rev…
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v4 2/3] libbpf: Add support for dynamic program attach target
2020-02-18 21:24 ` Andrii Nakryiko
@ 2020-02-19 11:06 ` Eelco Chaudron
2020-02-19 17:41 ` Andrii Nakryiko
0 siblings, 1 reply; 11+ messages in thread
From: Eelco Chaudron @ 2020-02-19 11:06 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jakub Sitnicki, bpf, David S. Miller, Networking,
Alexei Starovoitov, Daniel Borkmann, Martin Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Toke Høiland-Jørgensen
On 18 Feb 2020, at 22:24, Andrii Nakryiko wrote:
> On Tue, Feb 18, 2020 at 8:34 AM Jakub Sitnicki <jakub@cloudflare.com>
> wrote:
>>
>> Hey Eelco,
>>
>> On Mon, Feb 17, 2020 at 12:43 PM GMT, Eelco Chaudron wrote:
>>> Currently when you want to attach a trace program to a bpf program
>>> the section name needs to match the tracepoint/function semantics.
>>>
>>> However the addition of the bpf_program__set_attach_target() API
>>> allows you to specify the tracepoint/function dynamically.
>>>
>>> The call flow would look something like this:
>>>
>>> xdp_fd = bpf_prog_get_fd_by_id(id);
>>> trace_obj = bpf_object__open_file("func.o", NULL);
>>> prog = bpf_object__find_program_by_title(trace_obj,
>>> "fentry/myfunc");
>>> bpf_program__set_expected_attach_type(prog, BPF_TRACE_FENTRY);
>>> bpf_program__set_attach_target(prog, xdp_fd,
>>> "xdpfilt_blk_all");
>>> bpf_object__load(trace_obj)
>>>
>>> Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>>> ---
>>> tools/lib/bpf/libbpf.c | 34 ++++++++++++++++++++++++++++++----
>>> tools/lib/bpf/libbpf.h | 4 ++++
>>> tools/lib/bpf/libbpf.map | 2 ++
>>> 3 files changed, 36 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>> index 514b1a524abb..0c25d78fb5d8 100644
>>> --- a/tools/lib/bpf/libbpf.c
>>> +++ b/tools/lib/bpf/libbpf.c
>>
>> [...]
>>
>>> @@ -8132,6 +8133,31 @@ void bpf_program__bpil_offs_to_addr(struct
>>> bpf_prog_info_linear *info_linear)
>>> }
>>> }
>>>
>>> +int bpf_program__set_attach_target(struct bpf_program *prog,
>>> + int attach_prog_fd,
>>> + const char *attach_func_name)
>>> +{
>>> + int btf_id;
>>> +
>>> + if (!prog || attach_prog_fd < 0 || !attach_func_name)
>>> + return -EINVAL;
>>> +
>>> + if (attach_prog_fd)
>>> + btf_id = libbpf_find_prog_btf_id(attach_func_name,
>>> + attach_prog_fd);
>>> + else
>>> + btf_id = __find_vmlinux_btf_id(prog->obj->btf_vmlinux,
>>> + attach_func_name,
>>> +
>>> prog->expected_attach_type);
>>> +
>>> + if (btf_id <= 0)
>>> + return btf_id;
>>
>> Looks like we can get 0 as return value on both error and success
>> (below)? Is that intentional?
>
> Neither libbpf_find_prog_btf_id nor __find_vmlinux_btf_id are going to
> return 0 on failure. But I do agree that if (btf_id < 0) check would
> be better here.
Is see in theory btf__find_by_name_kind() could return 0:
if (kind == BTF_KIND_UNKN || !strcmp(type_name, "void"))
return 0;
But for our case, this will not happen and is invalid, so what about
just to make sure its future proof?:
if (btf_id <= 0)
return btf_id ? btf_id : -ENOENT;
> With that minor nit:
>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
>
>>
>>> +
>>> + prog->attach_btf_id = btf_id;
>>> + prog->attach_prog_fd = attach_prog_fd;
>>> + return 0;
>>> +}
>>> +
>>> int parse_cpu_mask_str(const char *s, bool **mask, int *mask_sz)
>>> {
>>> int err = 0, n, len, start, end = -1;
>>
>> [...]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v4 2/3] libbpf: Add support for dynamic program attach target
2020-02-19 11:06 ` Eelco Chaudron
@ 2020-02-19 17:41 ` Andrii Nakryiko
2020-02-20 13:21 ` Eelco Chaudron
0 siblings, 1 reply; 11+ messages in thread
From: Andrii Nakryiko @ 2020-02-19 17:41 UTC (permalink / raw)
To: Eelco Chaudron
Cc: Jakub Sitnicki, bpf, David S. Miller, Networking,
Alexei Starovoitov, Daniel Borkmann, Martin Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Toke Høiland-Jørgensen
On Wed, Feb 19, 2020 at 3:06 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>
>
>
> On 18 Feb 2020, at 22:24, Andrii Nakryiko wrote:
>
> > On Tue, Feb 18, 2020 at 8:34 AM Jakub Sitnicki <jakub@cloudflare.com>
> > wrote:
> >>
> >> Hey Eelco,
> >>
> >> On Mon, Feb 17, 2020 at 12:43 PM GMT, Eelco Chaudron wrote:
> >>> Currently when you want to attach a trace program to a bpf program
> >>> the section name needs to match the tracepoint/function semantics.
> >>>
> >>> However the addition of the bpf_program__set_attach_target() API
> >>> allows you to specify the tracepoint/function dynamically.
> >>>
> >>> The call flow would look something like this:
> >>>
> >>> xdp_fd = bpf_prog_get_fd_by_id(id);
> >>> trace_obj = bpf_object__open_file("func.o", NULL);
> >>> prog = bpf_object__find_program_by_title(trace_obj,
> >>> "fentry/myfunc");
> >>> bpf_program__set_expected_attach_type(prog, BPF_TRACE_FENTRY);
> >>> bpf_program__set_attach_target(prog, xdp_fd,
> >>> "xdpfilt_blk_all");
> >>> bpf_object__load(trace_obj)
> >>>
> >>> Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
> >>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> >>> ---
> >>> tools/lib/bpf/libbpf.c | 34 ++++++++++++++++++++++++++++++----
> >>> tools/lib/bpf/libbpf.h | 4 ++++
> >>> tools/lib/bpf/libbpf.map | 2 ++
> >>> 3 files changed, 36 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> >>> index 514b1a524abb..0c25d78fb5d8 100644
> >>> --- a/tools/lib/bpf/libbpf.c
> >>> +++ b/tools/lib/bpf/libbpf.c
> >>
> >> [...]
> >>
> >>> @@ -8132,6 +8133,31 @@ void bpf_program__bpil_offs_to_addr(struct
> >>> bpf_prog_info_linear *info_linear)
> >>> }
> >>> }
> >>>
> >>> +int bpf_program__set_attach_target(struct bpf_program *prog,
> >>> + int attach_prog_fd,
> >>> + const char *attach_func_name)
> >>> +{
> >>> + int btf_id;
> >>> +
> >>> + if (!prog || attach_prog_fd < 0 || !attach_func_name)
> >>> + return -EINVAL;
> >>> +
> >>> + if (attach_prog_fd)
> >>> + btf_id = libbpf_find_prog_btf_id(attach_func_name,
> >>> + attach_prog_fd);
> >>> + else
> >>> + btf_id = __find_vmlinux_btf_id(prog->obj->btf_vmlinux,
> >>> + attach_func_name,
> >>> +
> >>> prog->expected_attach_type);
> >>> +
> >>> + if (btf_id <= 0)
> >>> + return btf_id;
> >>
> >> Looks like we can get 0 as return value on both error and success
> >> (below)? Is that intentional?
> >
> > Neither libbpf_find_prog_btf_id nor __find_vmlinux_btf_id are going to
> > return 0 on failure. But I do agree that if (btf_id < 0) check would
> > be better here.
>
> Is see in theory btf__find_by_name_kind() could return 0:
>
> if (kind == BTF_KIND_UNKN || !strcmp(type_name, "void"))
> return 0;
>
> But for our case, this will not happen and is invalid, so what about
> just to make sure its future proof?:
>
> if (btf_id <= 0)
> return btf_id ? btf_id : -ENOENT;
I don't see how void can be the right attach type, so I'd keep it
simple: if (btf_id < 0) return btf_id.
If it so happens that 0 is returned, it will fail at attach time anyways.
>
>
> > With that minor nit:
> >
> > Acked-by: Andrii Nakryiko <andriin@fb.com>
> >
> >>
> >>> +
> >>> + prog->attach_btf_id = btf_id;
> >>> + prog->attach_prog_fd = attach_prog_fd;
> >>> + return 0;
> >>> +}
> >>> +
> >>> int parse_cpu_mask_str(const char *s, bool **mask, int *mask_sz)
> >>> {
> >>> int err = 0, n, len, start, end = -1;
> >>
> >> [...]
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v4 2/3] libbpf: Add support for dynamic program attach target
2020-02-19 17:41 ` Andrii Nakryiko
@ 2020-02-20 13:21 ` Eelco Chaudron
0 siblings, 0 replies; 11+ messages in thread
From: Eelco Chaudron @ 2020-02-20 13:21 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jakub Sitnicki, bpf, David S. Miller, Networking,
Alexei Starovoitov, Daniel Borkmann, Martin Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Toke Høiland-Jørgensen
On 19 Feb 2020, at 18:41, Andrii Nakryiko wrote:
> On Wed, Feb 19, 2020 at 3:06 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>>
>>
>>
>> On 18 Feb 2020, at 22:24, Andrii Nakryiko wrote:
>>
>>> On Tue, Feb 18, 2020 at 8:34 AM Jakub Sitnicki <jakub@cloudflare.com>
>>> wrote:
>>>>
>>>> Hey Eelco,
>>>>
>>>> On Mon, Feb 17, 2020 at 12:43 PM GMT, Eelco Chaudron wrote:
>>>>> Currently when you want to attach a trace program to a bpf program
>>>>> the section name needs to match the tracepoint/function semantics.
>>>>>
>>>>> However the addition of the bpf_program__set_attach_target() API
>>>>> allows you to specify the tracepoint/function dynamically.
>>>>>
>>>>> The call flow would look something like this:
>>>>>
>>>>> xdp_fd = bpf_prog_get_fd_by_id(id);
>>>>> trace_obj = bpf_object__open_file("func.o", NULL);
>>>>> prog = bpf_object__find_program_by_title(trace_obj,
>>>>> "fentry/myfunc");
>>>>> bpf_program__set_expected_attach_type(prog, BPF_TRACE_FENTRY);
>>>>> bpf_program__set_attach_target(prog, xdp_fd,
>>>>> "xdpfilt_blk_all");
>>>>> bpf_object__load(trace_obj)
>>>>>
>>>>> Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
>>>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>>>>> ---
>>>>> tools/lib/bpf/libbpf.c | 34 ++++++++++++++++++++++++++++++----
>>>>> tools/lib/bpf/libbpf.h | 4 ++++
>>>>> tools/lib/bpf/libbpf.map | 2 ++
>>>>> 3 files changed, 36 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>>>> index 514b1a524abb..0c25d78fb5d8 100644
>>>>> --- a/tools/lib/bpf/libbpf.c
>>>>> +++ b/tools/lib/bpf/libbpf.c
>>>>
>>>> [...]
>>>>
>>>>> @@ -8132,6 +8133,31 @@ void bpf_program__bpil_offs_to_addr(struct
>>>>> bpf_prog_info_linear *info_linear)
>>>>> }
>>>>> }
>>>>>
>>>>> +int bpf_program__set_attach_target(struct bpf_program *prog,
>>>>> + int attach_prog_fd,
>>>>> + const char *attach_func_name)
>>>>> +{
>>>>> + int btf_id;
>>>>> +
>>>>> + if (!prog || attach_prog_fd < 0 || !attach_func_name)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + if (attach_prog_fd)
>>>>> + btf_id = libbpf_find_prog_btf_id(attach_func_name,
>>>>> + attach_prog_fd);
>>>>> + else
>>>>> + btf_id = __find_vmlinux_btf_id(prog->obj->btf_vmlinux,
>>>>> + attach_func_name,
>>>>> +
>>>>> prog->expected_attach_type);
>>>>> +
>>>>> + if (btf_id <= 0)
>>>>> + return btf_id;
>>>>
>>>> Looks like we can get 0 as return value on both error and success
>>>> (below)? Is that intentional?
>>>
>>> Neither libbpf_find_prog_btf_id nor __find_vmlinux_btf_id are going to
>>> return 0 on failure. But I do agree that if (btf_id < 0) check would
>>> be better here.
>>
>> Is see in theory btf__find_by_name_kind() could return 0:
>>
>> if (kind == BTF_KIND_UNKN || !strcmp(type_name, "void"))
>> return 0;
>>
>> But for our case, this will not happen and is invalid, so what about
>> just to make sure its future proof?:
>>
>> if (btf_id <= 0)
>> return btf_id ? btf_id : -ENOENT;
>
> I don't see how void can be the right attach type, so I'd keep it
> simple: if (btf_id < 0) return btf_id.
> If it so happens that 0 is returned, it will fail at attach time anyways.
Ok, will send out a v5 later today…
>>> With that minor nit:
>>>
>>> Acked-by: Andrii Nakryiko <andriin@fb.com>
>>>
>>>>
>>>>> +
>>>>> + prog->attach_btf_id = btf_id;
>>>>> + prog->attach_prog_fd = attach_prog_fd;
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> int parse_cpu_mask_str(const char *s, bool **mask, int *mask_sz)
>>>>> {
>>>>> int err = 0, n, len, start, end = -1;
>>>>
>>>> [...]
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-02-20 13:21 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-17 12:42 [PATCH bpf-next v4 0/3] libbpf: Add support for dynamic program attach target Eelco Chaudron
2020-02-17 12:43 ` [PATCH bpf-next v4 1/3] libbpf: Bump libpf current version to v0.0.8 Eelco Chaudron
2020-02-17 12:43 ` [PATCH bpf-next v4 2/3] libbpf: Add support for dynamic program attach target Eelco Chaudron
2020-02-18 16:34 ` Jakub Sitnicki
2020-02-18 21:24 ` Andrii Nakryiko
2020-02-19 11:06 ` Eelco Chaudron
2020-02-19 17:41 ` Andrii Nakryiko
2020-02-20 13:21 ` Eelco Chaudron
2020-02-17 12:43 ` [PATCH bpf-next v4 3/3] selftests/bpf: update xdp_bpf2bpf test to use new set_attach_target API Eelco Chaudron
2020-02-18 21:21 ` Andrii Nakryiko
2020-02-19 10:54 ` Eelco Chaudron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.