[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJ8uoz22vVMt_XP3uV2QEpAQRZcfnhVN8tsYg6E4PZeSCvhsNQ@mail.gmail.com>
Date: Tue, 22 Aug 2023 15:52:22 +0200
From: Magnus Karlsson <magnus.karlsson@...il.com>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc: magnus.karlsson@...el.com, bjorn@...nel.org, ast@...nel.org,
daniel@...earbox.net, netdev@...r.kernel.org, bpf@...r.kernel.org, yhs@...com,
andrii@...nel.org, martin.lau@...ux.dev, song@...nel.org,
john.fastabend@...il.com, kpsingh@...nel.org, sdf@...gle.com,
haoluo@...gle.com, jolsa@...nel.org
Subject: Re: [PATCH bpf-next 06/10] selftests/xsk: add option that lists all tests
On Tue, 22 Aug 2023 at 14:37, Maciej Fijalkowski
<maciej.fijalkowski@...el.com> wrote:
>
> On Wed, Aug 09, 2023 at 02:43:39PM +0200, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@...el.com>
> >
> > Add a command line option (-l) that lists all the tests. The number
> > before the test will be used in the next commit for specifying a
> > single test to run. Here is an example of the output:
>
> I was thinking whether we should have a way of combining -l with -m, but I
> believe there is only a single test currently that can not be run in ZC
> mode (rx dropped) ?
>
> >
> > Tests:
> > 0: SEND_RECEIVE
> > 1: SEND_RECEIVE_2K_FRAME
> > 2: SEND_RECEIVE_SINGLE_PKT
> > 3: POLL_RX
> > 4: POLL_TX
> > 5: POLL_RXQ_FULL
> > 6: POLL_TXQ_FULL
> > 7: SEND_RECEIVE_UNALIGNED
> > :
> > :
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@...el.com>
> > ---
> > tools/testing/selftests/bpf/test_xsk.sh | 11 +++++++++-
> > tools/testing/selftests/bpf/xsk_prereqs.sh | 10 +++++----
> > tools/testing/selftests/bpf/xskxceiver.c | 24 ++++++++++++++++++++--
> > 3 files changed, 38 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> > index 4ec621f4d3db..00a504f0929a 100755
> > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > @@ -81,13 +81,14 @@
> >
> > ETH=""
> >
> > -while getopts "vi:dm:" flag
> > +while getopts "vi:dm:l" flag
> > do
> > case "${flag}" in
> > v) verbose=1;;
> > d) debug=1;;
> > i) ETH=${OPTARG};;
> > m) MODE=${OPTARG};;
> > + l) list=1;;
> > esac
> > done
> >
> > @@ -157,6 +158,10 @@ if [[ $verbose -eq 1 ]]; then
> > ARGS+="-v "
> > fi
> >
> > +if [[ $list -eq 1 ]]; then
> > + ARGS+="-l "
> > +fi
> > +
> > if [ ! -z $MODE ]; then
> > ARGS+="-m ${MODE} "
> > fi
> > @@ -183,6 +188,10 @@ else
> > cleanup_iface ${ETH} ${MTU}
> > fi
> >
> > +if [[ $list -eq 1 ]]; then
> > + exit
> > +fi
> > +
> > TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
> > busy_poll=1
> >
> > diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
> > index 29175682c44d..47c7b8064f38 100755
> > --- a/tools/testing/selftests/bpf/xsk_prereqs.sh
> > +++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
> > @@ -83,9 +83,11 @@ exec_xskxceiver()
> > fi
> >
> > ./${XSKOBJ} -i ${VETH0} -i ${VETH1} ${ARGS}
> > -
> > retval=$?
> > - test_status $retval "${TEST_NAME}"
> > - statusList+=($retval)
> > - nameList+=(${TEST_NAME})
> > +
> > + if [[ $list -ne 1 ]]; then
> > + test_status $retval "${TEST_NAME}"
> > + statusList+=($retval)
> > + nameList+=(${TEST_NAME})
> > + fi
> > }
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > index b1d0c69f21b8..a063b9af7fff 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -108,6 +108,7 @@ static const char *MAC1 = "\x00\x0A\x56\x9E\xEE\x62";
> > static const char *MAC2 = "\x00\x0A\x56\x9E\xEE\x61";
> >
> > static bool opt_verbose;
> > +static bool opt_print_tests;
> > static enum test_mode opt_mode = TEST_MODE_ALL;
> >
> > static void __exit_with_error(int error, const char *file, const char *func, int line)
> > @@ -314,6 +315,7 @@ static struct option long_options[] = {
> > {"busy-poll", no_argument, 0, 'b'},
> > {"verbose", no_argument, 0, 'v'},
> > {"mode", required_argument, 0, 'm'},
> > + {"list", no_argument, 0, 'l'},
> > {0, 0, 0, 0}
> > };
> >
> > @@ -325,7 +327,8 @@ static void usage(const char *prog)
> > " -i, --interface Use interface\n"
> > " -v, --verbose Verbose output\n"
> > " -b, --busy-poll Enable busy poll\n"
> > - " -m, --mode Run only mode skb, drv, or zc\n";
> > + " -m, --mode Run only mode skb, drv, or zc\n"
> > + " -l, --list List all available tests\n";
> >
> > ksft_print_msg(str, prog);
> > }
> > @@ -347,7 +350,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> > opterr = 0;
> >
> > for (;;) {
> > - c = getopt_long(argc, argv, "i:vbm:", long_options, &option_index);
> > + c = getopt_long(argc, argv, "i:vbm:l", long_options, &option_index);
> > if (c == -1)
> > break;
> >
> > @@ -391,6 +394,9 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> > ksft_exit_xfail();
> > }
> > break;
> > + case 'l':
> > + opt_print_tests = true;
> > + break;
> > default:
> > usage(basename(argv[0]));
> > ksft_exit_xfail();
> > @@ -2310,6 +2316,15 @@ static const struct test_spec tests[] = {
> > {.name = "TOO_MANY_FRAGS", .test_func = testapp_too_many_frags},
> > };
> >
> > +static void print_tests(void)
> > +{
> > + u32 i;
> > +
> > + printf("Tests:\n");
> > + for (i = 0; i < ARRAY_SIZE(tests); i++)
>
> Nit: I believe you can do
> for (u32 i = 0; i < ARRAY_SIZE(tests); i++)
Thank you for reviewing this Maciej. Will fix all your comments in all
the patches except this one. There are none of these embedded
declarations previously in the file, so let us just stick to declaring
variables right after "{", for consistency.
> > + printf("%u: %s\n", i, tests[i].name);
> > +}
> > +
> > int main(int argc, char **argv)
> > {
> > struct pkt_stream *rx_pkt_stream_default;
> > @@ -2334,6 +2349,11 @@ int main(int argc, char **argv)
> >
> > parse_command_line(ifobj_tx, ifobj_rx, argc, argv);
> >
> > + if (opt_print_tests) {
> > + print_tests();
> > + ksft_exit_xpass();
> > + }
> > +
> > shared_netdev = (ifobj_tx->ifindex == ifobj_rx->ifindex);
> > ifobj_tx->shared_umem = shared_netdev;
> > ifobj_rx->shared_umem = shared_netdev;
> > --
> > 2.34.1
> >
Powered by blists - more mailing lists