netdev - Re: [PATCH bpf-next 04/10] selftests: xsk: query for native XDP support

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: <CAJ8uoz3vd_Qhe9=oixMfq6zyuaHBwrQZvSQpU3OYA4Oh-9wmnQ@mail.gmail.com>
Date:   Mon, 13 Jun 2022 11:47:17 +0200
From:   Magnus Karlsson <magnus.karlsson@...il.com>
To:     Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc:     bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Network Development <netdev@...r.kernel.org>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        Björn Töpel <bjorn@...nel.org>
Subject: Re: [PATCH bpf-next 04/10] selftests: xsk: query for native XDP support

On Fri, Jun 10, 2022 at 5:15 PM Maciej Fijalkowski
<maciej.fijalkowski@...el.com> wrote:
>
> Currently, xdpxceiver assumes that underlying device supports XDP in
> native mode - it is fine by now since tests can run only on a veth pair.
> Future commit is going to allow running test suite against physical
> devices, so let us query the device if it is capable of running XDP
> programs in native mode. This way xdpxceiver will not try to run
> TEST_MODE_DRV if device being tested is not supporting it.
>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
> ---
>  tools/testing/selftests/bpf/xdpxceiver.c | 38 ++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> index e5992a6b5e09..da8098f1b655 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.c
> +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> @@ -98,6 +98,8 @@
>  #include <unistd.h>
>  #include <stdatomic.h>
>  #include <bpf/xsk.h>
> +#include <bpf/bpf.h>
> +#include <linux/filter.h>
>  #include "xdpxceiver.h"
>  #include "../kselftest.h"
>
> @@ -1605,10 +1607,39 @@ static void ifobject_delete(struct ifobject *ifobj)
>         free(ifobj);
>  }
>
> +static bool is_xdp_supported(struct ifobject *ifobject)
> +{
> +       int flags = XDP_FLAGS_DRV_MODE;
> +
> +       LIBBPF_OPTS(bpf_link_create_opts, opts, .flags = flags);
> +       struct bpf_insn insns[2] = {
> +               BPF_MOV64_IMM(BPF_REG_0, XDP_PASS),
> +               BPF_EXIT_INSN()
> +       };
> +       int ifindex = if_nametoindex(ifobject->ifname);
> +       int prog_fd, insn_cnt = ARRAY_SIZE(insns);
> +       bool ret = false;
> +       int err;
> +
> +       prog_fd = bpf_prog_load(BPF_PROG_TYPE_XDP, NULL, "GPL", insns, insn_cnt, NULL);
> +       if (prog_fd < 0)
> +               return ret;
> +
> +       err = bpf_xdp_attach(ifindex, prog_fd, flags, NULL);
> +
> +       if (!err) {
> +               ret = true;
> +               bpf_xdp_detach(ifindex, flags, NULL);
> +       }
> +
> +       return ret;

Think it would be clearer if you got rid of the bool ret and just
wrote "return false" and "return true" where applicable.

> +}
> +
>  int main(int argc, char **argv)
>  {
>         struct pkt_stream *pkt_stream_default;
>         struct ifobject *ifobj_tx, *ifobj_rx;
> +       int modes = TEST_MODE_SKB + 1;

Why not keep it a u32? A nit in any way.

>         u32 i, j, failed_tests = 0;
>         struct test_spec test;
>
> @@ -1636,15 +1667,18 @@ int main(int argc, char **argv)
>         init_iface(ifobj_rx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1,
>                    worker_testapp_validate_rx);
>
> +       if (is_xdp_supported(ifobj_tx))
> +               modes++;
> +
>         test_spec_init(&test, ifobj_tx, ifobj_rx, 0);
>         pkt_stream_default = pkt_stream_generate(ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
>         if (!pkt_stream_default)
>                 exit_with_error(ENOMEM);
>         test.pkt_stream_default = pkt_stream_default;
>
> -       ksft_set_plan(TEST_MODE_MAX * TEST_TYPE_MAX);
> +       ksft_set_plan(modes * TEST_TYPE_MAX);
>
> -       for (i = 0; i < TEST_MODE_MAX; i++)
> +       for (i = 0; i < modes; i++)
>                 for (j = 0; j < TEST_TYPE_MAX; j++) {
>                         test_spec_init(&test, ifobj_tx, ifobj_rx, i);
>                         run_pkt_test(&test, i, j);
> --
> 2.27.0
>