mbox series

[bpf-next,v2,00/20] selftests: xsk: facilitate adding tests

Message ID 20210907071928.9750-1-magnus.karlsson@gmail.com
Headers show
Series selftests: xsk: facilitate adding tests | expand

Message

Magnus Karlsson Sept. 7, 2021, 7:19 a.m. UTC
This patch set facilitates adding new tests as well as describing
existing ones in the xsk selftests suite and adds 3 new test suites at
the end. The idea is to isolate the run-time that executes the test
from the actual implementation of the test. Today, implementing a test
amounts to adding test specific if-statements all around the run-time,
which is not scalable or amenable for reuse. This patch set instead
introduces a test specification that is the only thing that a test
fills in. The run-time then gets this specification and acts upon it
completely unaware of what test it is executing. This way, we can get
rid of all test specific if-statements from the run-time and the
implementation of the test can be contained in a single function. This
hopefully makes it easier to add tests and for users to understand
what the test accomplishes.

As a recap of what the run-time does: each test is based on the
run-time launching two threads and connecting a veth link between the
two threads. Each thread opens an AF_XDP socket on that veth interface
and one of them sends traffic that the other one receives and
validates. Each thread has its own umem. Note that this behavior is
not changed by this patch set.

A test specification consists of several items. Most importantly:

* Two packet streams. One for Tx thread that specifies what traffic to
  send and one for the Rx thread that specifies what that thread
  should receive. If it receives exactly what is specified, the test
  passes, otherwise it fails. A packet stream can also specify what
  buffers in the umem that should be used by the Rx and Tx threads.

* What kind of AF_XDP sockets it should create and bind to what
  interfaces

* How many times it should repeat the socket creation and destruction

* The name of the test

The interface for the test spec is the following:

void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
                    struct ifobject *ifobj_rx, enum test_mode mode);

/* Reset everything but the interface specifications and the mode */
void test_spec_reset(struct test_spec *test);

void test_spec_set_name(struct test_spec *test, const char *name);


Packet streams have the following interfaces:

struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)

struct pkt *pkt_stream_get_next_rx_pkt(struct pkt_stream *pkt_stream)

struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem,
                                       u32 nb_pkts, u32 pkt_len);

void pkt_stream_delete(struct pkt_stream *pkt_stream);

struct pkt_stream *pkt_stream_clone(struct xsk_umem_info *umem,
                                    struct pkt_stream *pkt_stream);

/* Replaces all packets in the stream*/
void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len);

/* Replaces every other packet in the stream */
void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, u32 offset);

/* For creating custom made packet streams */
void pkt_stream_generate_custom(struct test_spec *test, struct pkt *pkts,
                                u32 nb_pkts);

/* Restores the default packet stream */
void pkt_stream_restore_default(struct test_spec *test);


A test can then then in the most basic case described like this
(provided the test specification has been created before calling the
function):

static bool testapp_aligned(struct test_spec *test)
{
        test_spec_set_name(test, "RUN_TO_COMPLETION");
        testapp_validate_traffic(test);
}

Running the same test in unaligned mode would then look like this:

static bool testapp_unaligned(struct test_spec *test)
{
        if (!hugepages_present(test->ifobj_tx)) {
                ksft_test_result_skip("No 2M huge pages present.\n");
                return false;
        }

        test_spec_set_name(test, "UNALIGNED_MODE");
        test->ifobj_tx->umem->unaligned_mode = true;
        test->ifobj_rx->umem->unaligned_mode = true;
        /* Let half of the packets straddle a buffer boundrary */
        pkt_stream_replace_half(test, PKT_SIZE,
                                XSK_UMEM__DEFAULT_FRAME_SIZE - 32);
	/* Populate fill ring with addresses in the packet stream */
        test->ifobj_rx->pkt_stream->use_addr_for_fill = true;
        testapp_validate_traffic(test);

        pkt_stream_restore_default(test);
	return true;
}

3 of the last 4 patches in the set add 3 new test suites, one for
unaligned mode, one for testing the rejection of tricky invalid
descriptors plus the acceptance of some valid ones in the Tx ring, and
one for testing 2K frame sizes (the default is 4K).

What is left to do for follow-up patches:

* Convert the statistics tests to the new framework.

* Implement a way of registering new tests without having the enum
  test_type. Once this has been done (together with the previous
  bullet), all the test types can be dropped from the header
  file. This means that we should be able to add tests by just writing
  a single function with a new test specification, which is one of the
  goals.

* Introduce functions for manipulating parts of the test or interface
  spec instead of direct manipulations such as
  test->ifobj_rx->pkt_stream->use_addr_for_fill = true; which is kind
  of awkward.

* Move the run-time and its interface to its own .c and .h files. Then
  we can have all the tests in a separate file.

* Better error reporting if a test fails. Today it does not state what
  test fails and might not continue execute the rest of the tests due
  to this failure. Failures are not propagated upwards through the
  functions so a failed test will also be a passed test, which messes
  up the stats counting. This needs to be changed.

* Add option to run specific test instead of all of them

* Introduce pacing of sent packets so that they are never dropped
  by the receiver even if it is stalled for some reason. If you run
  the current tests on a heavily loaded system, they might fail in SKB
  mode due to packets being dropped by the driver on Tx. Though I have
  never seen it, it might happen.

v1 -> v2:

* Fixed a number of spelling errors [Maciej]
* Fixed use after free bug in pkt_stream_replace() [Maciej]
* pkt_stream_set -> pkt_stream_generate_custom [Maciej]
* Fixed formatting problem in testapp_invalid_desc() [Maciej]

Thanks: Magnus

Magnus Karlsson (20):
  selftests: xsk: simplify xsk and umem arrays
  selftests: xsk: introduce type for thread function
  selftests: xsk: introduce test specifications
  selftests: xsk: move num_frames and frame_headroom to xsk_umem_info
  selftests: xsk: move rxqsize into xsk_socket_info
  selftests: xsk: make frame_size configurable
  selftests: xsx: introduce test name in test spec
  selftests: xsk: add use_poll to ifobject
  selftests: xsk: introduce rx_on and tx_on in ifobject
  selftests: xsk: replace second_step global variable
  selftests: xsk: specify number of sockets to create
  selftests: xsk: make xdp_flags and bind_flags local
  selftests: xsx: make pthreads local scope
  selftests: xsk: eliminate MAX_SOCKS define
  selftests: xsk: allow for invalid packets
  selftests: xsk: introduce replacing the default packet stream
  selftests: xsk: add test for unaligned mode
  selftests: xsk: eliminate test specific if-statement in test runner
  selftests: xsk: add tests for invalid xsk descriptors
  selftests: xsk: add tests for 2K frame size

 tools/testing/selftests/bpf/xdpxceiver.c | 872 +++++++++++++++--------
 tools/testing/selftests/bpf/xdpxceiver.h |  66 +-
 2 files changed, 608 insertions(+), 330 deletions(-)


base-commit: 27151f177827d478508e756c7657273261aaf8a9
--
2.29.0

Comments

Maciej Fijalkowski Sept. 10, 2021, 12:54 p.m. UTC | #1
On Tue, Sep 07, 2021 at 09:19:08AM +0200, Magnus Karlsson wrote:
> This patch set facilitates adding new tests as well as describing

> existing ones in the xsk selftests suite and adds 3 new test suites at

> the end. The idea is to isolate the run-time that executes the test

> from the actual implementation of the test. Today, implementing a test

> amounts to adding test specific if-statements all around the run-time,

> which is not scalable or amenable for reuse. This patch set instead

> introduces a test specification that is the only thing that a test

> fills in. The run-time then gets this specification and acts upon it

> completely unaware of what test it is executing. This way, we can get

> rid of all test specific if-statements from the run-time and the

> implementation of the test can be contained in a single function. This

> hopefully makes it easier to add tests and for users to understand

> what the test accomplishes.

> 

> As a recap of what the run-time does: each test is based on the

> run-time launching two threads and connecting a veth link between the

> two threads. Each thread opens an AF_XDP socket on that veth interface

> and one of them sends traffic that the other one receives and

> validates. Each thread has its own umem. Note that this behavior is

> not changed by this patch set.

> 

> A test specification consists of several items. Most importantly:

> 

> * Two packet streams. One for Tx thread that specifies what traffic to

>   send and one for the Rx thread that specifies what that thread

>   should receive. If it receives exactly what is specified, the test

>   passes, otherwise it fails. A packet stream can also specify what

>   buffers in the umem that should be used by the Rx and Tx threads.

> 

> * What kind of AF_XDP sockets it should create and bind to what

>   interfaces

> 

> * How many times it should repeat the socket creation and destruction

> 

> * The name of the test

> 

> The interface for the test spec is the following:

> 

> void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,

>                     struct ifobject *ifobj_rx, enum test_mode mode);

> 

> /* Reset everything but the interface specifications and the mode */

> void test_spec_reset(struct test_spec *test);

> 

> void test_spec_set_name(struct test_spec *test, const char *name);

> 

> 

> Packet streams have the following interfaces:

> 

> struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)

> 

> struct pkt *pkt_stream_get_next_rx_pkt(struct pkt_stream *pkt_stream)

> 

> struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem,

>                                        u32 nb_pkts, u32 pkt_len);

> 

> void pkt_stream_delete(struct pkt_stream *pkt_stream);

> 

> struct pkt_stream *pkt_stream_clone(struct xsk_umem_info *umem,

>                                     struct pkt_stream *pkt_stream);

> 

> /* Replaces all packets in the stream*/

> void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len);

> 

> /* Replaces every other packet in the stream */

> void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, u32 offset);

> 

> /* For creating custom made packet streams */

> void pkt_stream_generate_custom(struct test_spec *test, struct pkt *pkts,

>                                 u32 nb_pkts);

> 

> /* Restores the default packet stream */

> void pkt_stream_restore_default(struct test_spec *test);

> 

> 

> A test can then then in the most basic case described like this

> (provided the test specification has been created before calling the

> function):

> 

> static bool testapp_aligned(struct test_spec *test)

> {

>         test_spec_set_name(test, "RUN_TO_COMPLETION");

>         testapp_validate_traffic(test);

> }

> 

> Running the same test in unaligned mode would then look like this:

> 

> static bool testapp_unaligned(struct test_spec *test)

> {

>         if (!hugepages_present(test->ifobj_tx)) {

>                 ksft_test_result_skip("No 2M huge pages present.\n");

>                 return false;

>         }

> 

>         test_spec_set_name(test, "UNALIGNED_MODE");

>         test->ifobj_tx->umem->unaligned_mode = true;

>         test->ifobj_rx->umem->unaligned_mode = true;

>         /* Let half of the packets straddle a buffer boundrary */

>         pkt_stream_replace_half(test, PKT_SIZE,

>                                 XSK_UMEM__DEFAULT_FRAME_SIZE - 32);

> 	/* Populate fill ring with addresses in the packet stream */

>         test->ifobj_rx->pkt_stream->use_addr_for_fill = true;

>         testapp_validate_traffic(test);

> 

>         pkt_stream_restore_default(test);

> 	return true;

> }

> 

> 3 of the last 4 patches in the set add 3 new test suites, one for

> unaligned mode, one for testing the rejection of tricky invalid

> descriptors plus the acceptance of some valid ones in the Tx ring, and

> one for testing 2K frame sizes (the default is 4K).

> 

> What is left to do for follow-up patches:

> 

> * Convert the statistics tests to the new framework.

> 

> * Implement a way of registering new tests without having the enum

>   test_type. Once this has been done (together with the previous

>   bullet), all the test types can be dropped from the header

>   file. This means that we should be able to add tests by just writing

>   a single function with a new test specification, which is one of the

>   goals.

> 

> * Introduce functions for manipulating parts of the test or interface

>   spec instead of direct manipulations such as

>   test->ifobj_rx->pkt_stream->use_addr_for_fill = true; which is kind

>   of awkward.

> 

> * Move the run-time and its interface to its own .c and .h files. Then

>   we can have all the tests in a separate file.

> 

> * Better error reporting if a test fails. Today it does not state what

>   test fails and might not continue execute the rest of the tests due

>   to this failure. Failures are not propagated upwards through the

>   functions so a failed test will also be a passed test, which messes

>   up the stats counting. This needs to be changed.

> 

> * Add option to run specific test instead of all of them

> 

> * Introduce pacing of sent packets so that they are never dropped

>   by the receiver even if it is stalled for some reason. If you run

>   the current tests on a heavily loaded system, they might fail in SKB

>   mode due to packets being dropped by the driver on Tx. Though I have

>   never seen it, it might happen.

> 

> v1 -> v2:

> 

> * Fixed a number of spelling errors [Maciej]

> * Fixed use after free bug in pkt_stream_replace() [Maciej]

> * pkt_stream_set -> pkt_stream_generate_custom [Maciej]

> * Fixed formatting problem in testapp_invalid_desc() [Maciej]

> 

> Thanks: Magnus


I like the flexibility that this set is bringing to xdpxceiver.
Went through the v2 and this LGTM.

Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>


> 

> Magnus Karlsson (20):

>   selftests: xsk: simplify xsk and umem arrays

>   selftests: xsk: introduce type for thread function

>   selftests: xsk: introduce test specifications

>   selftests: xsk: move num_frames and frame_headroom to xsk_umem_info

>   selftests: xsk: move rxqsize into xsk_socket_info

>   selftests: xsk: make frame_size configurable

>   selftests: xsx: introduce test name in test spec

>   selftests: xsk: add use_poll to ifobject

>   selftests: xsk: introduce rx_on and tx_on in ifobject

>   selftests: xsk: replace second_step global variable

>   selftests: xsk: specify number of sockets to create

>   selftests: xsk: make xdp_flags and bind_flags local

>   selftests: xsx: make pthreads local scope

>   selftests: xsk: eliminate MAX_SOCKS define

>   selftests: xsk: allow for invalid packets

>   selftests: xsk: introduce replacing the default packet stream

>   selftests: xsk: add test for unaligned mode

>   selftests: xsk: eliminate test specific if-statement in test runner

>   selftests: xsk: add tests for invalid xsk descriptors

>   selftests: xsk: add tests for 2K frame size

> 

>  tools/testing/selftests/bpf/xdpxceiver.c | 872 +++++++++++++++--------

>  tools/testing/selftests/bpf/xdpxceiver.h |  66 +-

>  2 files changed, 608 insertions(+), 330 deletions(-)

> 

> 

> base-commit: 27151f177827d478508e756c7657273261aaf8a9

> --

> 2.29.0
patchwork-bot+netdevbpf@kernel.org Sept. 10, 2021, 7:20 p.m. UTC | #2
Hello:

This series was applied to bpf/bpf-next.git (refs/heads/master):

On Tue,  7 Sep 2021 09:19:08 +0200 you wrote:
> This patch set facilitates adding new tests as well as describing

> existing ones in the xsk selftests suite and adds 3 new test suites at

> the end. The idea is to isolate the run-time that executes the test

> from the actual implementation of the test. Today, implementing a test

> amounts to adding test specific if-statements all around the run-time,

> which is not scalable or amenable for reuse. This patch set instead

> introduces a test specification that is the only thing that a test

> fills in. The run-time then gets this specification and acts upon it

> completely unaware of what test it is executing. This way, we can get

> rid of all test specific if-statements from the run-time and the

> implementation of the test can be contained in a single function. This

> hopefully makes it easier to add tests and for users to understand

> what the test accomplishes.

> 

> [...]


Here is the summary with links:
  - [bpf-next,v2,01/20] selftests: xsk: simplify xsk and umem arrays
    https://git.kernel.org/bpf/bpf-next/c/ed7b74dc7777
  - [bpf-next,v2,02/20] selftests: xsk: introduce type for thread function
    https://git.kernel.org/bpf/bpf-next/c/744eb5c882e8
  - [bpf-next,v2,03/20] selftests: xsk: introduce test specifications
    https://git.kernel.org/bpf/bpf-next/c/ce74acaf015c
  - [bpf-next,v2,04/20] selftests: xsk: move num_frames and frame_headroom to xsk_umem_info
    https://git.kernel.org/bpf/bpf-next/c/83f4ae2f26bd
  - [bpf-next,v2,05/20] selftests: xsk: move rxqsize into xsk_socket_info
    https://git.kernel.org/bpf/bpf-next/c/4bf8ee65ba4e
  - [bpf-next,v2,06/20] selftests: xsk: make frame_size configurable
    https://git.kernel.org/bpf/bpf-next/c/c160d7afba8f
  - [bpf-next,v2,07/20] selftests: xsx: introduce test name in test spec
    https://git.kernel.org/bpf/bpf-next/c/53cb3cec2f1e
  - [bpf-next,v2,08/20] selftests: xsk: add use_poll to ifobject
    https://git.kernel.org/bpf/bpf-next/c/119d4b02feb5
  - [bpf-next,v2,09/20] selftests: xsk: introduce rx_on and tx_on in ifobject
    https://git.kernel.org/bpf/bpf-next/c/1856c24db0a8
  - [bpf-next,v2,10/20] selftests: xsk: replace second_step global variable
    https://git.kernel.org/bpf/bpf-next/c/55be575dc13c
  - [bpf-next,v2,11/20] selftests: xsk: specify number of sockets to create
    https://git.kernel.org/bpf/bpf-next/c/85c6c9573970
  - [bpf-next,v2,12/20] selftests: xsk: make xdp_flags and bind_flags local
    https://git.kernel.org/bpf/bpf-next/c/af6731d1e1c6
  - [bpf-next,v2,13/20] selftests: xsx: make pthreads local scope
    https://git.kernel.org/bpf/bpf-next/c/e2d850d5346c
  - [bpf-next,v2,14/20] selftests: xsk: eliminate MAX_SOCKS define
    https://git.kernel.org/bpf/bpf-next/c/8ce7192b508d
  - [bpf-next,v2,15/20] selftests: xsk: allow for invalid packets
    https://git.kernel.org/bpf/bpf-next/c/8abf6f725a9e
  - [bpf-next,v2,16/20] selftests: xsk: introduce replacing the default packet stream
    https://git.kernel.org/bpf/bpf-next/c/605091c5100d
  - [bpf-next,v2,17/20] selftests: xsk: add test for unaligned mode
    https://git.kernel.org/bpf/bpf-next/c/a4ba98dd0c69
  - [bpf-next,v2,18/20] selftests: xsk: eliminate test specific if-statement in test runner
    https://git.kernel.org/bpf/bpf-next/c/6ce67b5165e6
  - [bpf-next,v2,19/20] selftests: xsk: add tests for invalid xsk descriptors
    https://git.kernel.org/bpf/bpf-next/c/0d1b7f3a00cf
  - [bpf-next,v2,20/20] selftests: xsk: add tests for 2K frame size
    https://git.kernel.org/bpf/bpf-next/c/909f0e28207c

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html