On Fri, Jun 10, 2022 at 5:15 PM Maciej Fijalkowski
<maciej.fijalkowski@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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
>
