netdev - Re: [PATCH bpf-next v4 3/3] selftests/bpf: update xdp_bpf2bpf test to use new set_attach

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzYx2ZccrAu8JC=UxeHamk4dHKVa2jH4P=Hr7VzMwUphJQ@mail.gmail.com>
Date:   Tue, 18 Feb 2020 13:21:14 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Eelco Chaudron <echaudro@...hat.com>
Cc:     bpf <bpf@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin Lau <kafai@...com>, Song Liu <songliubraving@...com>,
        Yonghong Song <yhs@...com>, Andrii Nakryiko <andriin@...com>,
        Toke Høiland-Jørgensen <toke@...hat.com>
Subject: Re: [PATCH bpf-next v4 3/3] selftests/bpf: update xdp_bpf2bpf test to
 use new set_attach_target API

On Mon, Feb 17, 2020 at 5:03 AM Eelco Chaudron <echaudro@...hat.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@...hat.com>
> Signed-off-by: Eelco Chaudron <echaudro@...hat.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;
>