[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ8uoz0sef8MwwPeRS9_y94AFAXahrRaJ7S-RewF2pKG6sE-iA@mail.gmail.com>
Date: Thu, 14 Sep 2023 09:48:29 +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, przemyslaw.kitszel@...el.com
Subject: Re: [PATCH bpf-next v3 03/10] selftests/xsk: add option to only run
tests in a single mode
On Wed, 13 Sept 2023 at 19:12, Maciej Fijalkowski
<maciej.fijalkowski@...el.com> wrote:
>
> On Wed, Sep 13, 2023 at 01:02:25PM +0200, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@...el.com>
> >
> > Add an option -m on the command line that allows the user to run the
> > tests in a single mode instead of all of them. Valid modes are skb,
> > drv, and zc (zero-copy). An example:
> >
> > To run test suite in drv mode only:
> >
> > ./test_xsk.sh -m drv
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@...el.com>
> > ---
> > tools/testing/selftests/bpf/test_xsk.sh | 10 ++++++-
> > tools/testing/selftests/bpf/xskxceiver.c | 34 +++++++++++++++++++++---
> > tools/testing/selftests/bpf/xskxceiver.h | 4 +--
> > 3 files changed, 40 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> > index 2aa5a3445056..85e7a7e843f7 100755
> > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > @@ -73,17 +73,21 @@
> > #
> > # Run test suite for physical device in loopback mode
> > # sudo ./test_xsk.sh -i IFACE
> > +#
> > +# Run test suite in a specific mode only [skb,drv,zc]
> > +# sudo ./test_xsk.sh -m MODE
> >
> > . xsk_prereqs.sh
> >
> > ETH=""
> >
> > -while getopts "vi:d" flag
> > +while getopts "vi:dm:" flag
> > do
> > case "${flag}" in
> > v) verbose=1;;
> > d) debug=1;;
> > i) ETH=${OPTARG};;
> > + m) MODE=${OPTARG};;
> > esac
> > done
> >
> > @@ -153,6 +157,10 @@ if [[ $verbose -eq 1 ]]; then
> > ARGS+="-v "
> > fi
> >
> > +if [ -n "$MODE" ]; then
> > + ARGS+="-m ${MODE} "
> > +fi
> > +
> > retval=$?
> > test_status $retval "${TEST_NAME}"
> >
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > index 514fe994e02b..9f79c2b6aa97 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -107,6 +107,9 @@
> > 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 enum test_mode opt_mode = TEST_MODE_ALL;
> > +
> > static void __exit_with_error(int error, const char *file, const char *func, int line)
> > {
> > ksft_test_result_fail("[%s:%s:%i]: ERROR: %d/\"%s\"\n", file, func, line, error,
> > @@ -310,17 +313,19 @@ static struct option long_options[] = {
> > {"interface", required_argument, 0, 'i'},
> > {"busy-poll", no_argument, 0, 'b'},
> > {"verbose", no_argument, 0, 'v'},
> > + {"mode", required_argument, 0, 'm'},
> > {0, 0, 0, 0}
> > };
> >
> > static void usage(const char *prog)
> > {
> > const char *str =
> > - " Usage: %s [OPTIONS]\n"
> > + " Usage: xskxceiver [OPTIONS]\n"
> > " Options:\n"
> > " -i, --interface Use interface\n"
> > " -v, --verbose Verbose output\n"
> > - " -b, --busy-poll Enable busy poll\n";
> > + " -b, --busy-poll Enable busy poll\n"
> > + " -m, --mode Run only mode skb, drv, or zc\n";
> >
> > ksft_print_msg(str, prog);
> > }
> > @@ -342,7 +347,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> > opterr = 0;
> >
> > for (;;) {
> > - c = getopt_long(argc, argv, "i:vb", long_options, &option_index);
> > + c = getopt_long(argc, argv, "i:vbm:", long_options, &option_index);
> > if (c == -1)
> > break;
> >
> > @@ -371,6 +376,21 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> > ifobj_tx->busy_poll = true;
> > ifobj_rx->busy_poll = true;
> > break;
> > + case 'm':
> > + if (!strncmp("skb", optarg, min_t(size_t, strlen(optarg),
> > + strlen("skb")))) {
> > + opt_mode = TEST_MODE_SKB;
> > + } else if (!strncmp("drv", optarg, min_t(size_t, strlen(optarg),
> > + strlen("drv")))) {
> > + opt_mode = TEST_MODE_DRV;
> > + } else if (!strncmp("zc", optarg, min_t(size_t, strlen(optarg),
> > + strlen("zc")))) {
>
> with that logic i can type -m zcafxdprocks and this will still fly.
Anyone who writes that deserves at least a bonus level in my mind ;-).
Will fix it.
> Sorry for being such a PITA over this but couldn't we simplify this to
>
> if (!strncmp("skb", optarg, strlen(optarg)))
> opt_mode = TEST_MODE_SKB;
> else if (!strncmp("drv", optarg, strlen(optarg)))
> opt_mode = TEST_MODE_DRV;
> else if (!strncmp("zc", optarg, strlen(optarg)))
> opt_mode = TEST_MODE_ZC;
>
> as one of the next patches moves ksft_exit_xfail() to print_usage which
> makes the braces in branch statements redundant. Using len of optarg
> solves the -m zcafxdprocks problem.
>
> > + opt_mode = TEST_MODE_ZC;
> > + } else {
> > + usage(basename(argv[0]));
> > + ksft_exit_xfail();
> > + }
> > + break;
> > default:
> > usage(basename(argv[0]));
> > ksft_exit_xfail();
> > @@ -2365,9 +2385,15 @@ int main(int argc, char **argv)
> > test.tx_pkt_stream_default = tx_pkt_stream_default;
> > test.rx_pkt_stream_default = rx_pkt_stream_default;
> >
> > - ksft_set_plan(modes * TEST_TYPE_MAX);
> > + if (opt_mode == TEST_MODE_ALL)
> > + ksft_set_plan(modes * TEST_TYPE_MAX);
> > + else
> > + ksft_set_plan(TEST_TYPE_MAX);
> >
> > for (i = 0; i < modes; i++) {
> > + if (opt_mode != TEST_MODE_ALL && i != opt_mode)
> > + continue;
> > +
> > for (j = 0; j < TEST_TYPE_MAX; j++) {
> > test_spec_init(&test, ifobj_tx, ifobj_rx, i);
> > run_pkt_test(&test, i, j);
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
> > index 233b66cef64a..1412492e9618 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.h
> > +++ b/tools/testing/selftests/bpf/xskxceiver.h
> > @@ -63,7 +63,7 @@ enum test_mode {
> > TEST_MODE_SKB,
> > TEST_MODE_DRV,
> > TEST_MODE_ZC,
> > - TEST_MODE_MAX
> > + TEST_MODE_ALL
> > };
> >
> > enum test_type {
> > @@ -98,8 +98,6 @@ enum test_type {
> > TEST_TYPE_MAX
> > };
> >
> > -static bool opt_verbose;
> > -
> > struct xsk_umem_info {
> > struct xsk_ring_prod fq;
> > struct xsk_ring_cons cq;
> > --
> > 2.42.0
> >
Powered by blists - more mailing lists