diff mbox series

[bpf-next,v4,3/3] selftests/bpf: update xdp_bpf2bpf test to use new set_attach_target API

Message ID 158194342478.104074.6851588870108514192.stgit@xdp-tutorial
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series libbpf: Add support for dynamic program attach target | expand

Commit Message

Eelco Chaudron Feb. 17, 2020, 12:43 p.m. UTC
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(-)

Comments

Andrii Nakryiko Feb. 18, 2020, 9:21 p.m. UTC | #1
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;
>
Eelco Chaudron Feb. 19, 2020, 10:54 a.m. UTC | #2
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…
diff mbox series

Patch

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;