Message-ID: <CAPLEeBa3PL03mW4STe_avwK8wr-KKu_epJ1uAM07OgMzUFJw1w@mail.gmail.com>
Date:   Tue, 3 Nov 2020 12:26:40 +0000
From:   Weqaar Janjua <weqaar.janjua@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org, 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, andrii.nakryiko@...il.com
Subject: Re: [PATCH bpf-next 0/5] selftests/xsk: xsk selftests

On Mon, 2 Nov 2020 at 23:08, Daniel Borkmann <daniel@...earbox.net> wrote:
>
> On 10/30/20 1:13 PM, Weqaar Janjua wrote:
> > This patch set adds AF_XDP selftests based on veth to selftests/xsk/.
> >
> > # Topology:
> > # ---------
> > #                 -----------
> > #               _ | Process | _
> > #              /  -----------  \
> > #             /        |        \
> > #            /         |         \
> > #      -----------     |     -----------
> > #      | Thread1 |     |     | Thread2 |
> > #      -----------     |     -----------
> > #           |          |          |
> > #      -----------     |     -----------
> > #      |  xskX   |     |     |  xskY   |
> > #      -----------     |     -----------
> > #           |          |          |
> > #      -----------     |     ----------
> > #      |  vethX  | --------- |  vethY |
> > #      -----------   peer    ----------
> > #           |          |          |
> > #      namespaceX      |     namespaceY
> >
> > These selftests test AF_XDP SKB and Native/DRV modes using veth Virtual
> > Ethernet interfaces.
> >
> > The test program contains two threads, each thread is single socket with
> > a unique UMEM. It validates in-order packet delivery and packet content
> > by sending packets to each other.
> >
> > Prerequisites setup by script TEST_PREREQUISITES.sh:
> >
> >     Set up veth interfaces as per the topology shown ^^:
> >     * setup two veth interfaces and one namespace
> >     ** veth<xxxx> in root namespace
> >     ** veth<yyyy> in af_xdp<xxxx> namespace
> >     ** namespace af_xdp<xxxx>
> >     * create a spec file veth.spec that includes this run-time configuration
> >       that is read by test scripts - filenames prefixed with TEST_XSK
> >     *** xxxx and yyyy are randomly generated 4 digit numbers used to avoid
> >         conflict with any existing interface.
> >
> > The following tests are provided:
> >
> > 1. AF_XDP SKB mode
> >     Generic mode XDP is driver independent, used when the driver does
> >     not have support for XDP. Works on any netdevice using sockets and
> >     generic XDP path. XDP hook from netif_receive_skb().
> >     a. nopoll - soft-irq processing
> >     b. poll - using poll() syscall
> >     c. Socket Teardown
> >        Create a Tx and a Rx socket, Tx from one socket, Rx on another.
> >        Destroy both sockets, then repeat multiple times. Only nopoll mode
> >         is used
> >     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
> >     Works on any netdevice with XDP_REDIRECT support, driver dependent.
> >     Processes packets before SKB allocation. Provides better performance
> >     than SKB. Driver hook available just after DMA of buffer descriptor.
> >     a. nopoll
> >     b. poll
> >     c. Socket Teardown
> >     d. Bi-directional Sockets
> >     * Only copy mode is supported because veth does not currently support
> >       zero-copy mode
> >
> > Total tests: 8.
> >
> > Flow:
> > * Single process spawns two threads: Tx and Rx
> > * Each of these two threads attach to a veth interface within their
> >    assigned namespaces
> > * Each thread creates one AF_XDP socket connected to a unique umem
> >    for each veth interface
> > * Tx thread transmits 10k packets from veth<xxxx> to veth<yyyy>
> > * Rx thread verifies if all 10k packets were received and delivered
> >    in-order, and have the right content
> >
> > Structure of the patch set:
> >
> > Patch 1: This patch adds XSK Selftests framework under
> >           tools/testing/selftests/xsk, and README
> > Patch 2: Adds tests: SKB poll and nopoll mode, mac-ip-udp debug,
> >           and README updates
> > Patch 3: Adds tests: DRV poll and nopoll mode, and README updates
> > Patch 4: Adds tests: SKB and DRV Socket Teardown, and README updates
> > Patch 5: Adds tests: SKB and DRV Bi-directional Sockets, and README
> >           updates
> >
> > Thanks: Weqaar
> >
> > Weqaar Janjua (5):
> >    selftests/xsk: xsk selftests framework
> >    selftests/xsk: xsk selftests - SKB POLL, NOPOLL
> >    selftests/xsk: xsk selftests - DRV POLL, NOPOLL
> >    selftests/xsk: xsk selftests - Socket Teardown - SKB, DRV
> >    selftests/xsk: xsk selftests - Bi-directional Sockets - SKB, DRV
>
> Thanks a lot for adding the selftests, Weqaar! Given this needs to copy quite
> a bit of BPF selftest base infra e.g. from Makefiles I'd prefer if you could
> place these under selftests/bpf/ instead to avoid duplicating changes into two
> locations. I understand that these tests don't integrate well into test_progs,
> but for example see test_tc_redirect.sh or test_tc_edt.sh for stand-alone tests
> which could be done similarly with the xsk ones. Would be great if you could
> integrate them and spin a v2 with that.
>
> Thanks,
> Daniel

Hi Daniel,

Appreciate the pointers and suggestions which I will re-evaluate
against merging of selftests/xsk into selftests/bpf, let me explain a
bit to get your opinion re-evaluated on this - perhaps selftests/xsk
could still work (as per my clarifications below) or we somehow have
selftests/bpf/Makefile trigger selftests/bpf/test_xsk/Makefile<or
whatever>.

I had a look at selftests/bpf earlier, the problem was the same as you
indicated - xsk tests do not integrate well into selftests/bpf as the
semantics in the top level Makefile for both do no merge well - the
way xsk tests are designed is systematically different.

If you look closely into the patch set patches -> 2/5 - 5/5, that is
where the major difference shows up, xsk/xdpprogs/xdpxceiver.* is a
self-contained binary utilized for testing and it is a major piece of
code:
tools/testing/selftests/xsk/xdpprogs/Makefile |   64 ++
 .../selftests/xsk/xdpprogs/Makefile.target    |   68 ++
 .../selftests/xsk/xdpprogs/xdpxceiver.c       | 1000 +++++++++++++++++
 .../selftests/xsk/xdpprogs/xdpxceiver.h       |  159 +++
...
 21 files changed, 1833 insertions(+)

Bits that -> copy quite a bit of BPF selftest base infra isn't the
top-level selftests/xsk/Makefile|*, it is these (patches -> 2/5 -
5/5):
tools/testing/selftests/xsk/xdpprogs/Makefile |   64 ++
 .../selftests/xsk/xdpprogs/Makefile.target    |   68 ++

This is 132++ of 1833++ or ~7% of similarity.

I had a look at these today, and some other tests earlier:
- test_tc_redirect.sh -> test_xdp_redirect.sh (perhaps you meant this?)
- test_tc_edt.sh

Patch 1/5 might look similar to these^^ as that sets up the base xsk
test framework, but the xsk tests really start with patches -> 2/5 -
5/5 which will give you a better picture, please let me know if this
clarifies my intent for selftests/xsk/, or otherwise if you insist
merging into selftests/bpf, I will then go ahead and work it out
accordingly.

Thanks,

/Weqaar

Powered by blists - more mailing lists