Re: [PATCH bpf-next v2 07/10] selftests/xsx: test for huge pages only once

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 16 May 2023 at 14:58, Maciej Fijalkowski
<maciej.fijalkowski@xxxxxxxxx> wrote:
>
> On Tue, May 16, 2023 at 12:31:06PM +0200, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@xxxxxxxxx>
> >
> > Test for hugepages only once at the beginning of the execution of the
> > whole test suite, instead of before each test that needs huge
> > pages. These are the tests that use unaligned mode. As more unaligned
> > tests will be added, so the current system just does not scale.
> >
> > With this change, there are now three possible outcomes of a test run:
> > fail, pass, or skip. To simplify the handling of this, the function
> > testapp_validate_traffic() now returns this value to the main loop. As
> > this function is used by nearly all tests, it meant a small change to
> > most of them.
>
> I don't get the need for that change. Why couldn't we just store the
> retval to test_spec and then check it in run_pkt_test() just like we check
> test->fail currently? Am i missing something?

I think it is nicer to have the test return fail/pass/skip, just like
most functions return non-zero if there is an error, instead of using
a variable embedded in a struct. But maybe it is too much for a single
patch. How about breaking out the void -> int conversion of the return
value in one patch and then have the "remember unaligned mode" in a
separate one? The first one is just a means to be able to reach the
second one. What do you think?

> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@xxxxxxxxx>
> > ---
> >  tools/testing/selftests/bpf/xskxceiver.c | 186 +++++++++++------------
> >  tools/testing/selftests/bpf/xskxceiver.h |   2 +
> >  2 files changed, 94 insertions(+), 94 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > index d488d859d3a2..f0d929cb730a 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -1413,6 +1413,12 @@ static int testapp_validate_traffic(struct test_spec *test)
> >       struct ifobject *ifobj_rx = test->ifobj_rx;
> >       struct ifobject *ifobj_tx = test->ifobj_tx;
> >
> > +     if ((ifobj_rx->umem->unaligned_mode && !ifobj_rx->unaligned_supp) ||
> > +         (ifobj_tx->umem->unaligned_mode && !ifobj_tx->unaligned_supp)) {
> > +             ksft_test_result_skip("No huge pages present.\n");
> > +             return TEST_SKIP;
> > +     }
> > +
> >       xsk_attach_xdp_progs(test, ifobj_rx, ifobj_tx);
> >       return __testapp_validate_traffic(test, ifobj_rx, ifobj_tx);
> >  }
> > @@ -1422,16 +1428,18 @@ static int testapp_validate_traffic_single_thread(struct test_spec *test, struct
> >       return __testapp_validate_traffic(test, ifobj, NULL);
> >  }
> >
> > -static void testapp_teardown(struct test_spec *test)
> > +static int testapp_teardown(struct test_spec *test)
> >  {
> >       int i;
> >
> >       test_spec_set_name(test, "TEARDOWN");
> >       for (i = 0; i < MAX_TEARDOWN_ITER; i++) {
> >               if (testapp_validate_traffic(test))
> > -                     return;
> > +                     return TEST_FAILURE;
> >               test_spec_reset(test);
> >       }
> > +
> > +     return TEST_PASS;
> >  }
> >
> >  static void swap_directions(struct ifobject **ifobj1, struct ifobject **ifobj2)
> > @@ -1446,20 +1454,23 @@ static void swap_directions(struct ifobject **ifobj1, struct ifobject **ifobj2)
> >       *ifobj2 = tmp_ifobj;
> >  }
> >
> > -static void testapp_bidi(struct test_spec *test)
> > +static int testapp_bidi(struct test_spec *test)
> >  {
> > +     int res;
> > +
> >       test_spec_set_name(test, "BIDIRECTIONAL");
> >       test->ifobj_tx->rx_on = true;
> >       test->ifobj_rx->tx_on = true;
> >       test->total_steps = 2;
> >       if (testapp_validate_traffic(test))
> > -             return;
> > +             return TEST_FAILURE;
> >
> >       print_verbose("Switching Tx/Rx vectors\n");
> >       swap_directions(&test->ifobj_rx, &test->ifobj_tx);
> > -     __testapp_validate_traffic(test, test->ifobj_rx, test->ifobj_tx);
> > +     res = __testapp_validate_traffic(test, test->ifobj_rx, test->ifobj_tx);
> >
> >       swap_directions(&test->ifobj_rx, &test->ifobj_tx);
> > +     return res;
> >  }
> >
> >  static void swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj_rx)
> > @@ -1476,115 +1487,94 @@ static void swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj
> >               exit_with_error(errno);
> >  }
> >
> > -static void testapp_bpf_res(struct test_spec *test)
> > +static int testapp_bpf_res(struct test_spec *test)
> >  {
> >       test_spec_set_name(test, "BPF_RES");
> >       test->total_steps = 2;
> >       test->nb_sockets = 2;
> >       if (testapp_validate_traffic(test))
> > -             return;
> > +             return TEST_FAILURE;
> >
> >       swap_xsk_resources(test->ifobj_tx, test->ifobj_rx);
> > -     testapp_validate_traffic(test);
> > +     return testapp_validate_traffic(test);
> >  }
> >
> > -static void testapp_headroom(struct test_spec *test)
> > +static int testapp_headroom(struct test_spec *test)
> >  {
> >       test_spec_set_name(test, "UMEM_HEADROOM");
> >       test->ifobj_rx->umem->frame_headroom = UMEM_HEADROOM_TEST_SIZE;
> > -     testapp_validate_traffic(test);
> > +     return testapp_validate_traffic(test);
> >  }
> >
> > -static void testapp_stats_rx_dropped(struct test_spec *test)
> > +static int testapp_stats_rx_dropped(struct test_spec *test)
> >  {
> >       test_spec_set_name(test, "STAT_RX_DROPPED");
> > +     if (test->mode == TEST_MODE_ZC) {
> > +             ksft_test_result_skip("Can not run RX_DROPPED test for ZC mode\n");
> > +             return TEST_SKIP;
> > +     }
> > +
> >       pkt_stream_replace_half(test, MIN_PKT_SIZE * 4, 0);
> >       test->ifobj_rx->umem->frame_headroom = test->ifobj_rx->umem->frame_size -
> >               XDP_PACKET_HEADROOM - MIN_PKT_SIZE * 3;
> >       pkt_stream_receive_half(test);
> >       test->ifobj_rx->validation_func = validate_rx_dropped;
> > -     testapp_validate_traffic(test);
> > +     return testapp_validate_traffic(test);
> >  }
> >
> > -static void testapp_stats_tx_invalid_descs(struct test_spec *test)
> > +static int testapp_stats_tx_invalid_descs(struct test_spec *test)
> >  {
> >       test_spec_set_name(test, "STAT_TX_INVALID");
> >       pkt_stream_replace_half(test, XSK_UMEM__INVALID_FRAME_SIZE, 0);
> >       test->ifobj_tx->validation_func = validate_tx_invalid_descs;
> > -     testapp_validate_traffic(test);
> > +     return testapp_validate_traffic(test);
> >  }
> >
> > -static void testapp_stats_rx_full(struct test_spec *test)
> > +static int testapp_stats_rx_full(struct test_spec *test)
> >  {
> >       test_spec_set_name(test, "STAT_RX_FULL");
> >       pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS + DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
> >       test->ifobj_rx->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem,
> >                                                        DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> > -     if (!test->ifobj_rx->pkt_stream)
> > -             exit_with_error(ENOMEM);
> >
> >       test->ifobj_rx->xsk->rxqsize = DEFAULT_UMEM_BUFFERS;
> >       test->ifobj_rx->release_rx = false;
> >       test->ifobj_rx->validation_func = validate_rx_full;
> > -     testapp_validate_traffic(test);
> > +     return testapp_validate_traffic(test);
> >  }
> >
> > -static void testapp_stats_fill_empty(struct test_spec *test)
> > +static int testapp_stats_fill_empty(struct test_spec *test)
> >  {
> >       test_spec_set_name(test, "STAT_RX_FILL_EMPTY");
> >       pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS + DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
> >       test->ifobj_rx->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem,
> >                                                        DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> > -     if (!test->ifobj_rx->pkt_stream)
> > -             exit_with_error(ENOMEM);
> >
> >       test->ifobj_rx->use_fill_ring = false;
> >       test->ifobj_rx->validation_func = validate_fill_empty;
> > -     testapp_validate_traffic(test);
> > +     return testapp_validate_traffic(test);
> >  }
> >
> > -/* Simple test */
> > -static bool hugepages_present(struct ifobject *ifobject)
> > +static int testapp_unaligned(struct test_spec *test)
> >  {
> > -     size_t mmap_sz = 2 * ifobject->umem->num_frames * ifobject->umem->frame_size;
> > -     void *bufs;
> > -
> > -     bufs = mmap(NULL, mmap_sz, PROT_READ | PROT_WRITE,
> > -                 MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB | MAP_HUGE_2MB, -1, 0);
> > -     if (bufs == MAP_FAILED)
> > -             return false;
> > -
> > -     mmap_sz = ceil_u64(mmap_sz, HUGEPAGE_SIZE) * HUGEPAGE_SIZE;
> > -     munmap(bufs, mmap_sz);
> > -     return true;
> > -}
> > -
> > -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 4K buffer boundary */
> >       pkt_stream_replace_half(test, MIN_PKT_SIZE, -MIN_PKT_SIZE / 2);
> > -     testapp_validate_traffic(test);
> >
> > -     return true;
> > +     return testapp_validate_traffic(test);
> >  }
> >
> > -static void testapp_single_pkt(struct test_spec *test)
> > +static int testapp_single_pkt(struct test_spec *test)
> >  {
> >       struct pkt pkts[] = {{0, MIN_PKT_SIZE, 0, true}};
> >
> >       pkt_stream_generate_custom(test, pkts, ARRAY_SIZE(pkts));
> > -     testapp_validate_traffic(test);
> > +     return testapp_validate_traffic(test);
> >  }
> >
> > -static void testapp_invalid_desc(struct test_spec *test)
> > +static int testapp_invalid_desc(struct test_spec *test)
> >  {
> >       struct xsk_umem_info *umem = test->ifobj_tx->umem;
> >       u64 umem_size = umem->num_frames * umem->frame_size;
> > @@ -1626,10 +1616,10 @@ static void testapp_invalid_desc(struct test_spec *test)
> >       }
> >
> >       pkt_stream_generate_custom(test, pkts, ARRAY_SIZE(pkts));
> > -     testapp_validate_traffic(test);
> > +     return testapp_validate_traffic(test);
> >  }
> >
> > -static void testapp_xdp_drop(struct test_spec *test)
> > +static int testapp_xdp_drop(struct test_spec *test)
> >  {
> >       struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
> >       struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
> > @@ -1639,10 +1629,10 @@ static void testapp_xdp_drop(struct test_spec *test)
> >                              skel_rx->maps.xsk, skel_tx->maps.xsk);
> >
> >       pkt_stream_receive_half(test);
> > -     testapp_validate_traffic(test);
> > +     return testapp_validate_traffic(test);
> >  }
> >
> > -static void testapp_xdp_metadata_count(struct test_spec *test)
> > +static int testapp_xdp_metadata_count(struct test_spec *test)
> >  {
> >       struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
> >       struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
> > @@ -1663,10 +1653,10 @@ static void testapp_xdp_metadata_count(struct test_spec *test)
> >       if (bpf_map_update_elem(bpf_map__fd(data_map), &key, &count, BPF_ANY))
> >               exit_with_error(errno);
> >
> > -     testapp_validate_traffic(test);
> > +     return testapp_validate_traffic(test);
> >  }
> >
> > -static void testapp_poll_txq_tmout(struct test_spec *test)
> > +static int testapp_poll_txq_tmout(struct test_spec *test)
> >  {
> >       test_spec_set_name(test, "POLL_TXQ_FULL");
> >
> > @@ -1674,14 +1664,14 @@ static void testapp_poll_txq_tmout(struct test_spec *test)
> >       /* create invalid frame by set umem frame_size and pkt length equal to 2048 */
> >       test->ifobj_tx->umem->frame_size = 2048;
> >       pkt_stream_replace(test, 2 * DEFAULT_PKT_CNT, 2048);
> > -     testapp_validate_traffic_single_thread(test, test->ifobj_tx);
> > +     return testapp_validate_traffic_single_thread(test, test->ifobj_tx);
> >  }
> >
> > -static void testapp_poll_rxq_tmout(struct test_spec *test)
> > +static int testapp_poll_rxq_tmout(struct test_spec *test)
> >  {
> >       test_spec_set_name(test, "POLL_RXQ_EMPTY");
> >       test->ifobj_rx->use_poll = true;
> > -     testapp_validate_traffic_single_thread(test, test->ifobj_rx);
> > +     return testapp_validate_traffic_single_thread(test, test->ifobj_rx);
> >  }
> >
> >  static int xsk_load_xdp_programs(struct ifobject *ifobj)
> > @@ -1698,6 +1688,22 @@ static void xsk_unload_xdp_programs(struct ifobject *ifobj)
> >       xsk_xdp_progs__destroy(ifobj->xdp_progs);
> >  }
> >
> > +/* Simple test */
> > +static bool hugepages_present(void)
> > +{
> > +     size_t mmap_sz = 2 * DEFAULT_UMEM_BUFFERS * XSK_UMEM__DEFAULT_FRAME_SIZE;
> > +     void *bufs;
> > +
> > +     bufs = mmap(NULL, mmap_sz, PROT_READ | PROT_WRITE,
> > +                 MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, MAP_HUGE_2MB);
> > +     if (bufs == MAP_FAILED)
> > +             return false;
> > +
> > +     mmap_sz = ceil_u64(mmap_sz, HUGEPAGE_SIZE) * HUGEPAGE_SIZE;
> > +     munmap(bufs, mmap_sz);
> > +     return true;
> > +}
> > +
> >  static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
> >                      thread_func_t func_ptr)
> >  {
> > @@ -1713,94 +1719,87 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
> >               printf("Error loading XDP program\n");
> >               exit_with_error(err);
> >       }
> > +
> > +     if (hugepages_present())
> > +             ifobj->unaligned_supp = true;
> >  }
> >
> >  static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_type type)
> >  {
> > +     int ret = TEST_SKIP;
> > +
> >       switch (type) {
> >       case TEST_TYPE_STATS_RX_DROPPED:
> > -             if (mode == TEST_MODE_ZC) {
> > -                     ksft_test_result_skip("Can not run RX_DROPPED test for ZC mode\n");
> > -                     return;
> > -             }
> > -             testapp_stats_rx_dropped(test);
> > +             ret = testapp_stats_rx_dropped(test);
> >               break;
> >       case TEST_TYPE_STATS_TX_INVALID_DESCS:
> > -             testapp_stats_tx_invalid_descs(test);
> > +             ret = testapp_stats_tx_invalid_descs(test);
> >               break;
> >       case TEST_TYPE_STATS_RX_FULL:
> > -             testapp_stats_rx_full(test);
> > +             ret = testapp_stats_rx_full(test);
> >               break;
> >       case TEST_TYPE_STATS_FILL_EMPTY:
> > -             testapp_stats_fill_empty(test);
> > +             ret = testapp_stats_fill_empty(test);
> >               break;
> >       case TEST_TYPE_TEARDOWN:
> > -             testapp_teardown(test);
> > +             ret = testapp_teardown(test);
> >               break;
> >       case TEST_TYPE_BIDI:
> > -             testapp_bidi(test);
> > +             ret = testapp_bidi(test);
> >               break;
> >       case TEST_TYPE_BPF_RES:
> > -             testapp_bpf_res(test);
> > +             ret = testapp_bpf_res(test);
> >               break;
> >       case TEST_TYPE_RUN_TO_COMPLETION:
> >               test_spec_set_name(test, "RUN_TO_COMPLETION");
> > -             testapp_validate_traffic(test);
> > +             ret = testapp_validate_traffic(test);
> >               break;
> >       case TEST_TYPE_RUN_TO_COMPLETION_SINGLE_PKT:
> >               test_spec_set_name(test, "RUN_TO_COMPLETION_SINGLE_PKT");
> > -             testapp_single_pkt(test);
> > +             ret = testapp_single_pkt(test);
> >               break;
> >       case TEST_TYPE_RUN_TO_COMPLETION_2K_FRAME:
> >               test_spec_set_name(test, "RUN_TO_COMPLETION_2K_FRAME_SIZE");
> >               test->ifobj_tx->umem->frame_size = 2048;
> >               test->ifobj_rx->umem->frame_size = 2048;
> >               pkt_stream_replace(test, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
> > -             testapp_validate_traffic(test);
> > +             ret = testapp_validate_traffic(test);
> >               break;
> >       case TEST_TYPE_RX_POLL:
> >               test->ifobj_rx->use_poll = true;
> >               test_spec_set_name(test, "POLL_RX");
> > -             testapp_validate_traffic(test);
> > +             ret = testapp_validate_traffic(test);
> >               break;
> >       case TEST_TYPE_TX_POLL:
> >               test->ifobj_tx->use_poll = true;
> >               test_spec_set_name(test, "POLL_TX");
> > -             testapp_validate_traffic(test);
> > +             ret = testapp_validate_traffic(test);
> >               break;
> >       case TEST_TYPE_POLL_TXQ_TMOUT:
> > -             testapp_poll_txq_tmout(test);
> > +             ret = testapp_poll_txq_tmout(test);
> >               break;
> >       case TEST_TYPE_POLL_RXQ_TMOUT:
> > -             testapp_poll_rxq_tmout(test);
> > +             ret = testapp_poll_rxq_tmout(test);
> >               break;
> >       case TEST_TYPE_ALIGNED_INV_DESC:
> >               test_spec_set_name(test, "ALIGNED_INV_DESC");
> > -             testapp_invalid_desc(test);
> > +             ret = testapp_invalid_desc(test);
> >               break;
> >       case TEST_TYPE_ALIGNED_INV_DESC_2K_FRAME:
> >               test_spec_set_name(test, "ALIGNED_INV_DESC_2K_FRAME_SIZE");
> >               test->ifobj_tx->umem->frame_size = 2048;
> >               test->ifobj_rx->umem->frame_size = 2048;
> > -             testapp_invalid_desc(test);
> > +             ret = testapp_invalid_desc(test);
> >               break;
> >       case TEST_TYPE_UNALIGNED_INV_DESC:
> > -             if (!hugepages_present(test->ifobj_tx)) {
> > -                     ksft_test_result_skip("No 2M huge pages present.\n");
> > -                     return;
> > -             }
> >               test_spec_set_name(test, "UNALIGNED_INV_DESC");
> >               test->ifobj_tx->umem->unaligned_mode = true;
> >               test->ifobj_rx->umem->unaligned_mode = true;
> > -             testapp_invalid_desc(test);
> > +             ret = testapp_invalid_desc(test);
> >               break;
> >       case TEST_TYPE_UNALIGNED_INV_DESC_4K1_FRAME: {
> >               u64 page_size, umem_size;
> >
> > -             if (!hugepages_present(test->ifobj_tx)) {
> > -                     ksft_test_result_skip("No 2M huge pages present.\n");
> > -                     return;
> > -             }
> >               test_spec_set_name(test, "UNALIGNED_INV_DESC_4K1_FRAME_SIZE");
> >               /* Odd frame size so the UMEM doesn't end near a page boundary. */
> >               test->ifobj_tx->umem->frame_size = 4001;
> > @@ -1814,27 +1813,26 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
> >               umem_size = test->ifobj_tx->umem->num_frames * test->ifobj_tx->umem->frame_size;
> >               assert(umem_size % page_size > MIN_PKT_SIZE);
> >               assert(umem_size % page_size < page_size - MIN_PKT_SIZE);
> > -             testapp_invalid_desc(test);
> > +             ret = testapp_invalid_desc(test);
> >               break;
> >       }
> >       case TEST_TYPE_UNALIGNED:
> > -             if (!testapp_unaligned(test))
> > -                     return;
> > +             ret = testapp_unaligned(test);
> >               break;
> >       case TEST_TYPE_HEADROOM:
> > -             testapp_headroom(test);
> > +             ret = testapp_headroom(test);
> >               break;
> >       case TEST_TYPE_XDP_DROP_HALF:
> > -             testapp_xdp_drop(test);
> > +             ret = testapp_xdp_drop(test);
> >               break;
> >       case TEST_TYPE_XDP_METADATA_COUNT:
> > -             testapp_xdp_metadata_count(test);
> > +             ret = testapp_xdp_metadata_count(test);
> >               break;
> >       default:
> >               break;
> >       }
> >
> > -     if (!test->fail)
> > +     if (ret == TEST_PASS)
> >               ksft_test_result_pass("PASS: %s %s%s\n", mode_string(test), busy_poll_string(test),
> >                                     test->name);
> >       pkt_stream_restore_default(test);
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
> > index be4664a38d74..00862732e751 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.h
> > +++ b/tools/testing/selftests/bpf/xskxceiver.h
> > @@ -30,6 +30,7 @@
> >  #define TEST_PASS 0
> >  #define TEST_FAILURE -1
> >  #define TEST_CONTINUE 1
> > +#define TEST_SKIP 2
> >  #define MAX_INTERFACES 2
> >  #define MAX_INTERFACE_NAME_CHARS 16
> >  #define MAX_SOCKETS 2
> > @@ -148,6 +149,7 @@ struct ifobject {
> >       bool release_rx;
> >       bool shared_umem;
> >       bool use_metadata;
> > +     bool unaligned_supp;
> >       u8 dst_mac[ETH_ALEN];
> >       u8 src_mac[ETH_ALEN];
> >  };
> > --
> > 2.34.1
> >




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux