netdev - Re: [PATCH bpf-next v2 5/5] selftests/bpf: xsk selftests - Bi-directional Sockets

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]

Date:   Tue, 24 Nov 2020 15:11:16 +0000
From:   Weqaar Janjua <weqaar.janjua@...il.com>
To:     Yonghong Song <yhs@...com>
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org,
        Daniel Borkmann <daniel@...earbox.net>, ast@...nel.org,
        Magnus Karlsson <magnus.karlsson@...il.com>,
        Björn Töpel <bjorn.topel@...el.com>,
        Weqaar Janjua <weqaar.a.janjua@...el.com>, shuah@...nel.org,
        skhan@...uxfoundation.org, linux-kselftest@...r.kernel.org,
        Anders Roxell <anders.roxell@...aro.org>,
        jonathan.lemon@...il.com
Subject: Re: [PATCH bpf-next v2 5/5] selftests/bpf: xsk selftests -
 Bi-directional Sockets - SKB, DRV

On Sat, 21 Nov 2020 at 20:14, Weqaar Janjua <weqaar.janjua@...il.com> wrote:
>
> On Fri, 20 Nov 2020 at 20:45, Yonghong Song <yhs@...com> wrote:
> >
> >
> >
> > On 11/20/20 5:00 AM, Weqaar Janjua wrote:
> > > Adds following tests:
> > >
> > > 1. AF_XDP SKB mode
> > >     d. Bi-directional Sockets
> > >        Configure sockets as bi-directional tx/rx sockets, sets up fill
> > >        and completion rings on each socket, tx/rx in both directions.
> > >        Only nopoll mode is used
> > >
> > > 2. AF_XDP DRV/Native mode
> > >     d. Bi-directional Sockets
> > >     * Only copy mode is supported because veth does not currently support
> > >       zero-copy mode
> > >
> > > Signed-off-by: Weqaar Janjua <weqaar.a.janjua@...el.com>
> > > ---
> > >   tools/testing/selftests/bpf/Makefile          |   4 +-
> > >   .../bpf/test_xsk_drv_bidirectional.sh         |  23 ++++
> > >   .../selftests/bpf/test_xsk_drv_teardown.sh    |   3 -
> > >   .../bpf/test_xsk_skb_bidirectional.sh         |  20 ++++
> > >   tools/testing/selftests/bpf/xdpxceiver.c      | 100 +++++++++++++-----
> > >   tools/testing/selftests/bpf/xdpxceiver.h      |   4 +
> > >   6 files changed, 126 insertions(+), 28 deletions(-)
> > >   create mode 100755 tools/testing/selftests/bpf/test_xsk_drv_bidirectional.sh
> > >   create mode 100755 tools/testing/selftests/bpf/test_xsk_skb_bidirectional.sh
> > >
> > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > index 515b29d321d7..258bd72812e0 100644
> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -78,7 +78,9 @@ TEST_PROGS := test_kmod.sh \
> > >       test_xsk_drv_nopoll.sh \
> > >       test_xsk_drv_poll.sh \
> > >       test_xsk_skb_teardown.sh \
> > > -     test_xsk_drv_teardown.sh
> > > +     test_xsk_drv_teardown.sh \
> > > +     test_xsk_skb_bidirectional.sh \
> > > +     test_xsk_drv_bidirectional.sh
> > >
> > >   TEST_PROGS_EXTENDED := with_addr.sh \
> > >       with_tunnels.sh \
> > > diff --git a/tools/testing/selftests/bpf/test_xsk_drv_bidirectional.sh b/tools/testing/selftests/bpf/test_xsk_drv_bidirectional.sh
> > > new file mode 100755
> > > index 000000000000..d3a7e2934d83
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/bpf/test_xsk_drv_bidirectional.sh
> > > @@ -0,0 +1,23 @@
> > > +#!/bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright(c) 2020 Intel Corporation.
> > > +
> > > +# See test_xsk_prerequisites.sh for detailed information on tests
> > > +
> > > +. xsk_prereqs.sh
> > > +. xsk_env.sh
> > > +
> > > +TEST_NAME="DRV BIDIRECTIONAL SOCKETS"
> > > +
> > > +vethXDPnative ${VETH0} ${VETH1} ${NS1}
> > > +
> > > +params=("-N" "-B")
> > > +execxdpxceiver params
> > > +
> > > +retval=$?
> > > +test_status $retval "${TEST_NAME}"
> > > +
> > > +# Must be called in the last test to execute
> > > +cleanup_exit ${VETH0} ${VETH1} ${NS1}
> >
> > This also makes hard to run tests as users will not know this unless
> > they are familiar with the details of the tests.
> >
> > How about you have another scripts test_xsk.sh which includes all these
> > individual tests and pull the above cleanup_exit into test_xsk.sh?
> > User just need to run test_xsk.sh will be able to run all tests you
> > implemented here.
> >
> This works, test_xsk_* >> test_xsk.sh, will ship out as v3.
>
An issue with merging all tests in a single test_xsk.sh is reporting
number of test failures, with this approach a single test status is
printed by kselftest:

# PREREQUISITES: [ PASS ]
# SKB NOPOLL: [ FAIL ]
# SKB POLL: [ PASS ]
ok 1 selftests: xsk-patch2: test_xsk.sh

This is due to the fact Makefile has one TEST_PROGS = test_xsk.sh
(thus kselftest considers it one test?), where in the original
approach all tests have separate TEST_PROGS .sh which makes reporting
match each test and status. This can be a problem for automation.

An alternative would be to exit each test with failure status but then
the tests will stop execution at the failed test without executing the
rest of xsk tests, which we probably wouldn't want.

Suggestions please?

> > > +
> > > +test_exit $retval 0
> > > diff --git a/tools/testing/selftests/bpf/test_xsk_drv_teardown.sh b/tools/testing/selftests/bpf/test_xsk_drv_teardown.sh
> > [...]