diff mbox series

[bpf-next,16/20] selftests: xsk: introduce replacing the default packet stream

Message ID 20210901104732.10956-17-magnus.karlsson@gmail.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series selftests: xsk: facilitate adding tests | expand

Checks

Context Check Description
bpf/vmtest-bpf-next fail VM_Test
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count fail Series longer than 15 patches
netdev/tree_selection success Clearly marked for bpf-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 9 maintainers not CCed: linux-kselftest@vger.kernel.org kpsingh@kernel.org hawk@kernel.org davem@davemloft.net kuba@kernel.org songliubraving@fb.com kafai@fb.com john.fastabend@gmail.com shuah@kernel.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns WARNING: line length of 92 exceeds 80 columns WARNING: line length of 96 exceeds 80 columns
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
bpf/vmtest-bpf-next-PR fail PR summary

Commit Message

Magnus Karlsson Sept. 1, 2021, 10:47 a.m. UTC 
From: Magnus Karlsson <magnus.karlsson@intel.com>

Introduce the concept of a default packet stream that is the set of
packets sent by most tests. Then add the ability to replace it for a
test that would like to send or receive something else through the use
of the function pkt_stream_replace() and then restored with
pkt_stream_restore_default(). These are then used to convert the
STAT_TX_INVALID_TEST to use these new APIs.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 67 +++++++++++++++++-------
 tools/testing/selftests/bpf/xdpxceiver.h |  1 +
 2 files changed, 50 insertions(+), 18 deletions(-)

Comments

Fijalkowski, Maciej Sept. 3, 2021, 3:04 p.m. UTC | #1 
On Wed, Sep 01, 2021 at 12:47:28PM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Introduce the concept of a default packet stream that is the set of
> packets sent by most tests. Then add the ability to replace it for a
> test that would like to send or receive something else through the use
> of the function pkt_stream_replace() and then restored with
> pkt_stream_restore_default(). These are then used to convert the
> STAT_TX_INVALID_TEST to use these new APIs.

s/STAT_TX_INVALID_TEST/STAT_TEST_TX_INVALID

> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>  tools/testing/selftests/bpf/xdpxceiver.c | 67 +++++++++++++++++-------
>  tools/testing/selftests/bpf/xdpxceiver.h |  1 +
>  2 files changed, 50 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> index 09d2854c10e6..d4aad4833754 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.c
> +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> @@ -390,6 +390,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
>  		ifobj->umem = &ifobj->umem_arr[0];
>  		ifobj->xsk = &ifobj->xsk_arr[0];
>  		ifobj->use_poll = false;
> +		ifobj->pkt_stream = test->pkt_stream_default;
>  
>  		if (i == 0) {
>  			ifobj->rx_on = false;
> @@ -418,9 +419,12 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
>  static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
>  			   struct ifobject *ifobj_rx, enum test_mode mode)
>  {
> +	struct pkt_stream *pkt_stream;
>  	u32 i;
>  
> +	pkt_stream = test->pkt_stream_default;
>  	memset(test, 0, sizeof(*test));
> +	test->pkt_stream_default = pkt_stream;
>  
>  	for (i = 0; i < MAX_INTERFACES; i++) {
>  		struct ifobject *ifobj = i ? ifobj_rx : ifobj_tx;
> @@ -455,6 +459,19 @@ static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)
>  	return &pkt_stream->pkts[pkt_nb];
>  }
>  
> +static void pkt_stream_delete(struct pkt_stream *pkt_stream)
> +{
> +	free(pkt_stream->pkts);
> +	free(pkt_stream);
> +}
> +
> +static void pkt_stream_restore_default(struct test_spec *test)
> +{
> +	pkt_stream_delete(test->ifobj_tx->pkt_stream);

I suppose that streams are the same for both tx and rx ifobjs hence it's
enough to call the delete op a single time.

> +	test->ifobj_tx->pkt_stream = test->pkt_stream_default;
> +	test->ifobj_rx->pkt_stream = test->pkt_stream_default;
> +}
> +
>  static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
>  {
>  	struct pkt_stream *pkt_stream;
> @@ -483,6 +500,17 @@ static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb
>  	return pkt_stream;
>  }
>  
> +static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
> +{
> +	struct pkt_stream *pkt_stream;
> +
> +	pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, nb_pkts, pkt_len);
> +	test->ifobj_tx->pkt_stream = pkt_stream;
> +	test->ifobj_rx->pkt_stream = pkt_stream;
> +
> +	pkt_stream_delete(pkt_stream);

Shouldn't this be deleting the stream that got replaced? You're assigning
pkt_stream to ifobjs and then immediately free it.

I'd say that we should drop this call to pkt_stream_delete() in here
unless I'm missing something, pkt_stream_restore_default() will free the
currently assigned pkt stream and bring back the default one to the
ifobjs.

> +}
> +
>  static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
>  {
>  	struct pkt *pkt = pkt_stream_get_pkt(ifobject->pkt_stream, pkt_nb);
> @@ -557,7 +585,7 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, const struct xdp_desc *d
>  	if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
>  		u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE)));
>  
> -		if (opt_pkt_dump && test_type != TEST_TYPE_STATS)
> +		if (opt_pkt_dump)
>  			pkt_dump(data, PKT_SIZE);
>  
>  		if (pkt->len != desc->len) {
> @@ -598,9 +626,6 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
>  	unsigned int rcvd;
>  	u32 idx;
>  
> -	if (!xsk->outstanding_tx)
> -		return;
> -
>  	if (xsk_ring_prod__needs_wakeup(&xsk->tx))
>  		kick_tx(xsk);
>  
> @@ -831,6 +856,7 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
>  
>  static void testapp_cleanup_xsk_res(struct ifobject *ifobj)
>  {
> +	print_verbose("Destroying socket\n");
>  	xsk_socket__delete(ifobj->xsk->xsk);
>  	xsk_umem__delete(ifobj->umem->umem);
>  }
> @@ -878,9 +904,6 @@ static void *worker_testapp_validate_rx(void *arg)
>  	else
>  		receive_pkts(ifobject->pkt_stream, ifobject->xsk, &fds);
>  
> -	if (test_type == TEST_TYPE_TEARDOWN)
> -		print_verbose("Destroying socket\n");
> -
>  	if (test->total_steps == test->current_step)
>  		testapp_cleanup_xsk_res(ifobject);
>  	pthread_exit(NULL);
> @@ -890,19 +913,11 @@ static void testapp_validate_traffic(struct test_spec *test)
>  {
>  	struct ifobject *ifobj_tx = test->ifobj_tx;
>  	struct ifobject *ifobj_rx = test->ifobj_rx;
> -	struct pkt_stream *pkt_stream;
>  	pthread_t t0, t1;
>  
>  	if (pthread_barrier_init(&barr, NULL, 2))
>  		exit_with_error(errno);
>  
> -	if (stat_test_type == STAT_TEST_TX_INVALID)
> -		pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT,
> -						 XSK_UMEM__INVALID_FRAME_SIZE);
> -	else
> -		pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
> -	ifobj_tx->pkt_stream = pkt_stream;
> -	ifobj_rx->pkt_stream = pkt_stream;
>  	test->current_step++;
>  
>  	/*Spawn RX thread */
> @@ -982,7 +997,9 @@ static void testapp_bpf_res(struct test_spec *test)
>  
>  static void testapp_stats(struct test_spec *test)
>  {
> -	for (int i = 0; i < STAT_TEST_TYPE_MAX; i++) {
> +	int i;
> +
> +	for (i = 0; i < STAT_TEST_TYPE_MAX; i++) {
>  		test_spec_reset(test);
>  		stat_test_type = i;
>  
> @@ -991,21 +1008,27 @@ static void testapp_stats(struct test_spec *test)
>  			test_spec_set_name(test, "STAT_RX_DROPPED");
>  			test->ifobj_rx->umem->frame_headroom = test->ifobj_rx->umem->frame_size -
>  				XDP_PACKET_HEADROOM - 1;
> +			testapp_validate_traffic(test);
>  			break;
>  		case STAT_TEST_RX_FULL:
>  			test_spec_set_name(test, "STAT_RX_FULL");
>  			test->ifobj_rx->xsk->rxqsize = RX_FULL_RXQSIZE;
> +			testapp_validate_traffic(test);
>  			break;
>  		case STAT_TEST_TX_INVALID:
>  			test_spec_set_name(test, "STAT_TX_INVALID");
> -			continue;
> +			pkt_stream_replace(test, DEFAULT_PKT_CNT, XSK_UMEM__INVALID_FRAME_SIZE);
> +			testapp_validate_traffic(test);
> +
> +			pkt_stream_restore_default(test);
> +			break;
>  		case STAT_TEST_RX_FILL_EMPTY:
>  			test_spec_set_name(test, "STAT_RX_FILL_EMPTY");
> +			testapp_validate_traffic(test);
>  			break;
>  		default:
>  			break;
>  		}
> -		testapp_validate_traffic(test);
>  	}
>  
>  	/* To only see the whole stat set being completed unless an individual test fails. */
> @@ -1106,6 +1129,7 @@ int main(int argc, char **argv)
>  {
>  	struct rlimit _rlim = { RLIM_INFINITY, RLIM_INFINITY };
>  	struct ifobject *ifobj_tx, *ifobj_rx;
> +	struct pkt_stream *pkt_stream_default;

rct broken by a little?

>  	struct test_spec test;
>  	u32 i, j;
>  
> @@ -1133,6 +1157,12 @@ int main(int argc, char **argv)
>  	init_iface(ifobj_rx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1,
>  		   worker_testapp_validate_rx);
>  
> +	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);

I missed this probably while reviewing previous set, but to be consistent
with 083be682d976 ("selftests: xsk: Return correct error codes") this
probably should have -ENOMEM as an arg?

> +	test.pkt_stream_default = pkt_stream_default;
> +
>  	ksft_set_plan(TEST_MODE_MAX * TEST_TYPE_MAX);
>  
>  	for (i = 0; i < TEST_MODE_MAX; i++)
> @@ -1142,6 +1172,7 @@ int main(int argc, char **argv)
>  			usleep(USLEEP_MAX);
>  		}
>  
> +	pkt_stream_delete(pkt_stream_default);
>  	ifobject_delete(ifobj_tx);
>  	ifobject_delete(ifobj_rx);
>  
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
> index c5baa7c5f560..e27fe348ae50 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.h
> +++ b/tools/testing/selftests/bpf/xdpxceiver.h
> @@ -132,6 +132,7 @@ struct ifobject {
>  struct test_spec {
>  	struct ifobject *ifobj_tx;
>  	struct ifobject *ifobj_rx;
> +	struct pkt_stream *pkt_stream_default;
>  	u16 total_steps;
>  	u16 current_step;
>  	u16 nb_sockets;
> -- 
> 2.29.0
>
Magnus Karlsson Sept. 6, 2021, 8 a.m. UTC | #2 
On Fri, Sep 3, 2021 at 3:04 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Wed, Sep 01, 2021 at 12:47:28PM +0200, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Introduce the concept of a default packet stream that is the set of
> > packets sent by most tests. Then add the ability to replace it for a
> > test that would like to send or receive something else through the use
> > of the function pkt_stream_replace() and then restored with
> > pkt_stream_restore_default(). These are then used to convert the
> > STAT_TX_INVALID_TEST to use these new APIs.
>
> s/STAT_TX_INVALID_TEST/STAT_TEST_TX_INVALID

Will fix.

> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> >  tools/testing/selftests/bpf/xdpxceiver.c | 67 +++++++++++++++++-------
> >  tools/testing/selftests/bpf/xdpxceiver.h |  1 +
> >  2 files changed, 50 insertions(+), 18 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> > index 09d2854c10e6..d4aad4833754 100644
> > --- a/tools/testing/selftests/bpf/xdpxceiver.c
> > +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> > @@ -390,6 +390,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> >               ifobj->umem = &ifobj->umem_arr[0];
> >               ifobj->xsk = &ifobj->xsk_arr[0];
> >               ifobj->use_poll = false;
> > +             ifobj->pkt_stream = test->pkt_stream_default;
> >
> >               if (i == 0) {
> >                       ifobj->rx_on = false;
> > @@ -418,9 +419,12 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> >  static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> >                          struct ifobject *ifobj_rx, enum test_mode mode)
> >  {
> > +     struct pkt_stream *pkt_stream;
> >       u32 i;
> >
> > +     pkt_stream = test->pkt_stream_default;
> >       memset(test, 0, sizeof(*test));
> > +     test->pkt_stream_default = pkt_stream;
> >
> >       for (i = 0; i < MAX_INTERFACES; i++) {
> >               struct ifobject *ifobj = i ? ifobj_rx : ifobj_tx;
> > @@ -455,6 +459,19 @@ static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)
> >       return &pkt_stream->pkts[pkt_nb];
> >  }
> >
> > +static void pkt_stream_delete(struct pkt_stream *pkt_stream)
> > +{
> > +     free(pkt_stream->pkts);
> > +     free(pkt_stream);
> > +}
> > +
> > +static void pkt_stream_restore_default(struct test_spec *test)
> > +{
> > +     pkt_stream_delete(test->ifobj_tx->pkt_stream);
>
> I suppose that streams are the same for both tx and rx ifobjs hence it's
> enough to call the delete op a single time.

At this point in time, yes. But this has to change as we develop more tests.

> > +     test->ifobj_tx->pkt_stream = test->pkt_stream_default;
> > +     test->ifobj_rx->pkt_stream = test->pkt_stream_default;
> > +}
> > +
> >  static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
> >  {
> >       struct pkt_stream *pkt_stream;
> > @@ -483,6 +500,17 @@ static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb
> >       return pkt_stream;
> >  }
> >
> > +static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
> > +{
> > +     struct pkt_stream *pkt_stream;
> > +
> > +     pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, nb_pkts, pkt_len);
> > +     test->ifobj_tx->pkt_stream = pkt_stream;
> > +     test->ifobj_rx->pkt_stream = pkt_stream;
> > +
> > +     pkt_stream_delete(pkt_stream);
>
> Shouldn't this be deleting the stream that got replaced? You're assigning
> pkt_stream to ifobjs and then immediately free it.
>
> I'd say that we should drop this call to pkt_stream_delete() in here
> unless I'm missing something, pkt_stream_restore_default() will free the
> currently assigned pkt stream and bring back the default one to the
> ifobjs.

That is indeed a bug and should be removed.

> > +}
> > +
> >  static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
> >  {
> >       struct pkt *pkt = pkt_stream_get_pkt(ifobject->pkt_stream, pkt_nb);
> > @@ -557,7 +585,7 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, const struct xdp_desc *d
> >       if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
> >               u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE)));
> >
> > -             if (opt_pkt_dump && test_type != TEST_TYPE_STATS)
> > +             if (opt_pkt_dump)
> >                       pkt_dump(data, PKT_SIZE);
> >
> >               if (pkt->len != desc->len) {
> > @@ -598,9 +626,6 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
> >       unsigned int rcvd;
> >       u32 idx;
> >
> > -     if (!xsk->outstanding_tx)
> > -             return;
> > -
> >       if (xsk_ring_prod__needs_wakeup(&xsk->tx))
> >               kick_tx(xsk);
> >
> > @@ -831,6 +856,7 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
> >
> >  static void testapp_cleanup_xsk_res(struct ifobject *ifobj)
> >  {
> > +     print_verbose("Destroying socket\n");
> >       xsk_socket__delete(ifobj->xsk->xsk);
> >       xsk_umem__delete(ifobj->umem->umem);
> >  }
> > @@ -878,9 +904,6 @@ static void *worker_testapp_validate_rx(void *arg)
> >       else
> >               receive_pkts(ifobject->pkt_stream, ifobject->xsk, &fds);
> >
> > -     if (test_type == TEST_TYPE_TEARDOWN)
> > -             print_verbose("Destroying socket\n");
> > -
> >       if (test->total_steps == test->current_step)
> >               testapp_cleanup_xsk_res(ifobject);
> >       pthread_exit(NULL);
> > @@ -890,19 +913,11 @@ static void testapp_validate_traffic(struct test_spec *test)
> >  {
> >       struct ifobject *ifobj_tx = test->ifobj_tx;
> >       struct ifobject *ifobj_rx = test->ifobj_rx;
> > -     struct pkt_stream *pkt_stream;
> >       pthread_t t0, t1;
> >
> >       if (pthread_barrier_init(&barr, NULL, 2))
> >               exit_with_error(errno);
> >
> > -     if (stat_test_type == STAT_TEST_TX_INVALID)
> > -             pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT,
> > -                                              XSK_UMEM__INVALID_FRAME_SIZE);
> > -     else
> > -             pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
> > -     ifobj_tx->pkt_stream = pkt_stream;
> > -     ifobj_rx->pkt_stream = pkt_stream;
> >       test->current_step++;
> >
> >       /*Spawn RX thread */
> > @@ -982,7 +997,9 @@ static void testapp_bpf_res(struct test_spec *test)
> >
> >  static void testapp_stats(struct test_spec *test)
> >  {
> > -     for (int i = 0; i < STAT_TEST_TYPE_MAX; i++) {
> > +     int i;
> > +
> > +     for (i = 0; i < STAT_TEST_TYPE_MAX; i++) {
> >               test_spec_reset(test);
> >               stat_test_type = i;
> >
> > @@ -991,21 +1008,27 @@ static void testapp_stats(struct test_spec *test)
> >                       test_spec_set_name(test, "STAT_RX_DROPPED");
> >                       test->ifobj_rx->umem->frame_headroom = test->ifobj_rx->umem->frame_size -
> >                               XDP_PACKET_HEADROOM - 1;
> > +                     testapp_validate_traffic(test);
> >                       break;
> >               case STAT_TEST_RX_FULL:
> >                       test_spec_set_name(test, "STAT_RX_FULL");
> >                       test->ifobj_rx->xsk->rxqsize = RX_FULL_RXQSIZE;
> > +                     testapp_validate_traffic(test);
> >                       break;
> >               case STAT_TEST_TX_INVALID:
> >                       test_spec_set_name(test, "STAT_TX_INVALID");
> > -                     continue;
> > +                     pkt_stream_replace(test, DEFAULT_PKT_CNT, XSK_UMEM__INVALID_FRAME_SIZE);
> > +                     testapp_validate_traffic(test);
> > +
> > +                     pkt_stream_restore_default(test);
> > +                     break;
> >               case STAT_TEST_RX_FILL_EMPTY:
> >                       test_spec_set_name(test, "STAT_RX_FILL_EMPTY");
> > +                     testapp_validate_traffic(test);
> >                       break;
> >               default:
> >                       break;
> >               }
> > -             testapp_validate_traffic(test);
> >       }
> >
> >       /* To only see the whole stat set being completed unless an individual test fails. */
> > @@ -1106,6 +1129,7 @@ int main(int argc, char **argv)
> >  {
> >       struct rlimit _rlim = { RLIM_INFINITY, RLIM_INFINITY };
> >       struct ifobject *ifobj_tx, *ifobj_rx;
> > +     struct pkt_stream *pkt_stream_default;
>
> rct broken by a little?
>
> >       struct test_spec test;
> >       u32 i, j;
> >
> > @@ -1133,6 +1157,12 @@ int main(int argc, char **argv)
> >       init_iface(ifobj_rx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1,
> >                  worker_testapp_validate_rx);
> >
> > +     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);
>
> I missed this probably while reviewing previous set, but to be consistent
> with 083be682d976 ("selftests: xsk: Return correct error codes") this
> probably should have -ENOMEM as an arg?

Have eliminated exit_with_error from most of the functions I have been
touching, but not from the main function. Want the error to be printed
out before exiting the program, so kept it at this level.

> > +     test.pkt_stream_default = pkt_stream_default;
> > +
> >       ksft_set_plan(TEST_MODE_MAX * TEST_TYPE_MAX);
> >
> >       for (i = 0; i < TEST_MODE_MAX; i++)
> > @@ -1142,6 +1172,7 @@ int main(int argc, char **argv)
> >                       usleep(USLEEP_MAX);
> >               }
> >
> > +     pkt_stream_delete(pkt_stream_default);
> >       ifobject_delete(ifobj_tx);
> >       ifobject_delete(ifobj_rx);
> >
> > diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
> > index c5baa7c5f560..e27fe348ae50 100644
> > --- a/tools/testing/selftests/bpf/xdpxceiver.h
> > +++ b/tools/testing/selftests/bpf/xdpxceiver.h
> > @@ -132,6 +132,7 @@ struct ifobject {
> >  struct test_spec {
> >       struct ifobject *ifobj_tx;
> >       struct ifobject *ifobj_rx;
> > +     struct pkt_stream *pkt_stream_default;
> >       u16 total_steps;
> >       u16 current_step;
> >       u16 nb_sockets;
> > --
> > 2.29.0
> >
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 09d2854c10e6..d4aad4833754 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -390,6 +390,7 @@  static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 		ifobj->umem = &ifobj->umem_arr[0];
 		ifobj->xsk = &ifobj->xsk_arr[0];
 		ifobj->use_poll = false;
+		ifobj->pkt_stream = test->pkt_stream_default;
 
 		if (i == 0) {
 			ifobj->rx_on = false;
@@ -418,9 +419,12 @@  static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 			   struct ifobject *ifobj_rx, enum test_mode mode)
 {
+	struct pkt_stream *pkt_stream;
 	u32 i;
 
+	pkt_stream = test->pkt_stream_default;
 	memset(test, 0, sizeof(*test));
+	test->pkt_stream_default = pkt_stream;
 
 	for (i = 0; i < MAX_INTERFACES; i++) {
 		struct ifobject *ifobj = i ? ifobj_rx : ifobj_tx;
@@ -455,6 +459,19 @@  static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)
 	return &pkt_stream->pkts[pkt_nb];
 }
 
+static void pkt_stream_delete(struct pkt_stream *pkt_stream)
+{
+	free(pkt_stream->pkts);
+	free(pkt_stream);
+}
+
+static void pkt_stream_restore_default(struct test_spec *test)
+{
+	pkt_stream_delete(test->ifobj_tx->pkt_stream);
+	test->ifobj_tx->pkt_stream = test->pkt_stream_default;
+	test->ifobj_rx->pkt_stream = test->pkt_stream_default;
+}
+
 static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
 {
 	struct pkt_stream *pkt_stream;
@@ -483,6 +500,17 @@  static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb
 	return pkt_stream;
 }
 
+static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
+{
+	struct pkt_stream *pkt_stream;
+
+	pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, nb_pkts, pkt_len);
+	test->ifobj_tx->pkt_stream = pkt_stream;
+	test->ifobj_rx->pkt_stream = pkt_stream;
+
+	pkt_stream_delete(pkt_stream);
+}
+
 static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
 {
 	struct pkt *pkt = pkt_stream_get_pkt(ifobject->pkt_stream, pkt_nb);
@@ -557,7 +585,7 @@  static bool is_pkt_valid(struct pkt *pkt, void *buffer, const struct xdp_desc *d
 	if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
 		u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE)));
 
-		if (opt_pkt_dump && test_type != TEST_TYPE_STATS)
+		if (opt_pkt_dump)
 			pkt_dump(data, PKT_SIZE);
 
 		if (pkt->len != desc->len) {
@@ -598,9 +626,6 @@  static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
 	unsigned int rcvd;
 	u32 idx;
 
-	if (!xsk->outstanding_tx)
-		return;
-
 	if (xsk_ring_prod__needs_wakeup(&xsk->tx))
 		kick_tx(xsk);
 
@@ -831,6 +856,7 @@  static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
 
 static void testapp_cleanup_xsk_res(struct ifobject *ifobj)
 {
+	print_verbose("Destroying socket\n");
 	xsk_socket__delete(ifobj->xsk->xsk);
 	xsk_umem__delete(ifobj->umem->umem);
 }
@@ -878,9 +904,6 @@  static void *worker_testapp_validate_rx(void *arg)
 	else
 		receive_pkts(ifobject->pkt_stream, ifobject->xsk, &fds);
 
-	if (test_type == TEST_TYPE_TEARDOWN)
-		print_verbose("Destroying socket\n");
-
 	if (test->total_steps == test->current_step)
 		testapp_cleanup_xsk_res(ifobject);
 	pthread_exit(NULL);
@@ -890,19 +913,11 @@  static void testapp_validate_traffic(struct test_spec *test)
 {
 	struct ifobject *ifobj_tx = test->ifobj_tx;
 	struct ifobject *ifobj_rx = test->ifobj_rx;
-	struct pkt_stream *pkt_stream;
 	pthread_t t0, t1;
 
 	if (pthread_barrier_init(&barr, NULL, 2))
 		exit_with_error(errno);
 
-	if (stat_test_type == STAT_TEST_TX_INVALID)
-		pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT,
-						 XSK_UMEM__INVALID_FRAME_SIZE);
-	else
-		pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
-	ifobj_tx->pkt_stream = pkt_stream;
-	ifobj_rx->pkt_stream = pkt_stream;
 	test->current_step++;
 
 	/*Spawn RX thread */
@@ -982,7 +997,9 @@  static void testapp_bpf_res(struct test_spec *test)
 
 static void testapp_stats(struct test_spec *test)
 {
-	for (int i = 0; i < STAT_TEST_TYPE_MAX; i++) {
+	int i;
+
+	for (i = 0; i < STAT_TEST_TYPE_MAX; i++) {
 		test_spec_reset(test);
 		stat_test_type = i;
 
@@ -991,21 +1008,27 @@  static void testapp_stats(struct test_spec *test)
 			test_spec_set_name(test, "STAT_RX_DROPPED");
 			test->ifobj_rx->umem->frame_headroom = test->ifobj_rx->umem->frame_size -
 				XDP_PACKET_HEADROOM - 1;
+			testapp_validate_traffic(test);
 			break;
 		case STAT_TEST_RX_FULL:
 			test_spec_set_name(test, "STAT_RX_FULL");
 			test->ifobj_rx->xsk->rxqsize = RX_FULL_RXQSIZE;
+			testapp_validate_traffic(test);
 			break;
 		case STAT_TEST_TX_INVALID:
 			test_spec_set_name(test, "STAT_TX_INVALID");
-			continue;
+			pkt_stream_replace(test, DEFAULT_PKT_CNT, XSK_UMEM__INVALID_FRAME_SIZE);
+			testapp_validate_traffic(test);
+
+			pkt_stream_restore_default(test);
+			break;
 		case STAT_TEST_RX_FILL_EMPTY:
 			test_spec_set_name(test, "STAT_RX_FILL_EMPTY");
+			testapp_validate_traffic(test);
 			break;
 		default:
 			break;
 		}
-		testapp_validate_traffic(test);
 	}
 
 	/* To only see the whole stat set being completed unless an individual test fails. */
@@ -1106,6 +1129,7 @@  int main(int argc, char **argv)
 {
 	struct rlimit _rlim = { RLIM_INFINITY, RLIM_INFINITY };
 	struct ifobject *ifobj_tx, *ifobj_rx;
+	struct pkt_stream *pkt_stream_default;
 	struct test_spec test;
 	u32 i, j;
 
@@ -1133,6 +1157,12 @@  int main(int argc, char **argv)
 	init_iface(ifobj_rx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1,
 		   worker_testapp_validate_rx);
 
+	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);
 
 	for (i = 0; i < TEST_MODE_MAX; i++)
@@ -1142,6 +1172,7 @@  int main(int argc, char **argv)
 			usleep(USLEEP_MAX);
 		}
 
+	pkt_stream_delete(pkt_stream_default);
 	ifobject_delete(ifobj_tx);
 	ifobject_delete(ifobj_rx);
 
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index c5baa7c5f560..e27fe348ae50 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -132,6 +132,7 @@  struct ifobject {
 struct test_spec {
 	struct ifobject *ifobj_tx;
 	struct ifobject *ifobj_rx;
+	struct pkt_stream *pkt_stream_default;
 	u16 total_steps;
 	u16 current_step;
 	u16 nb_sockets;