lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ