netdev - Re: [bpf-next PATCH] bpf: sockmap, test tool support to skip cgroup arg

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAEf4Bza3CMqp5pqtkEFh+3GAHcH+qcKYJhbL02jbauWjD_K0Xg@mail.gmail.com>
Date:   Thu, 25 Apr 2019 10:34:34 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     John Fastabend <john.fastabend@...il.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Networking <netdev@...r.kernel.org>, bpf@...r.kernel.org
Subject: Re: [bpf-next PATCH] bpf: sockmap, test tool support to skip cgroup arg

On Thu, Apr 25, 2019 at 12:19 AM John Fastabend
<john.fastabend@...il.com> wrote:
>
> Its handy to not have to setup a cgroup environment when running
> test_sockmap with user specified options. This patch setups a cgroup
> environment, similar to how it is done with the auto test suite, when
> the user does not provide a cgroup arg.
>
> Signed-off-by: John Fastabend <john.fastabend@...il.com>
> ---
>  tools/testing/selftests/bpf/test_sockmap.c |   24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
> index 3845144e2c91..11dfb1f10e15 100644
> --- a/tools/testing/selftests/bpf/test_sockmap.c
> +++ b/tools/testing/selftests/bpf/test_sockmap.c
> @@ -1734,6 +1734,7 @@ int main(int argc, char **argv)
>         int opt, longindex, err, cg_fd = 0;
>         char *bpf_file = BPF_SOCKMAP_FILENAME;
>         int test = PING_PONG;
> +       bool cleanup = true;
>
>         if (argc < 2)
>                 return test_suite(-1);
> @@ -1773,6 +1774,7 @@ int main(int argc, char **argv)
>                                         cg_fd, optarg);
>                                 return cg_fd;
>                         }
> +                       cleanup = false;
>                         break;
>                 case 'r':
>                         rate = atoi(optarg);
> @@ -1818,9 +1820,23 @@ int main(int argc, char **argv)

Code one line above this, assumes that tool in non-test mode always
has argc > 3. With --cgroup being optional, that's not true anymore,
right? Or am I missing how it's supposed to be used?

>                 return test_suite(cg_fd);
>
>         if (!cg_fd) {
> -               fprintf(stderr, "%s requires cgroup option: --cgroup <path>\n",
> -                       argv[0]);

Now that --cgroup is optional, would it make sense to update usage
function at line 121 to remove "--cgroup <cgroup_path>"?

> -               return -1;
> +               if (setup_cgroup_environment()) {
> +                       fprintf(stderr, "ERROR: cgroup env failed\n");
> +                       return -EINVAL;
> +               }
> +
> +               cg_fd = create_and_get_cgroup(CG_PATH);
> +               if (cg_fd < 0) {
> +                       fprintf(stderr,
> +                               "ERROR: (%i) open cg path failed: %s\n",
> +                               cg_fd, optarg);
> +                       return cg_fd;
> +               }
> +
> +               if (join_cgroup(CG_PATH)) {
> +                       fprintf(stderr, "ERROR: failed to join cgroup\n");
> +                       return -EINVAL;
> +               }
>         }
>
>         err = populate_progs(bpf_file);
> @@ -1839,6 +1855,8 @@ int main(int argc, char **argv)
>         options.rate = rate;
>
>         err = run_options(&options, cg_fd, test);
> +       if (cleanup)
> +               cleanup_cgroup_environment();
>         close(cg_fd);
>         return err;
>  }
>