linux-kernel - Re: [PATCH bpf-next v3 13/14] bpf: Add tests for new BPF atomic operations

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: <X8oFJW/mMFHVxngY@google.com>
Date:   Fri, 4 Dec 2020 09:45:09 +0000
From:   Brendan Jackman <jackmanb@...gle.com>
To:     Yonghong Song <yhs@...com>
Cc:     bpf@...r.kernel.org, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        KP Singh <kpsingh@...omium.org>,
        Florent Revest <revest@...omium.org>,
        linux-kernel@...r.kernel.org, Jann Horn <jannh@...gle.com>
Subject: Re: [PATCH bpf-next v3 13/14] bpf: Add tests for new BPF atomic
 operations

On Thu, Dec 03, 2020 at 11:06:31PM -0800, Yonghong Song wrote:
> On 12/3/20 8:02 AM, Brendan Jackman wrote:
[...]
> > diff --git a/tools/testing/selftests/bpf/prog_tests/atomics_test.c b/tools/testing/selftests/bpf/prog_tests/atomics_test.c
> > new file mode 100644
> > index 000000000000..66f0ccf4f4ec
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/atomics_test.c
> > @@ -0,0 +1,262 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <test_progs.h>
> > +
> > +
> > +#include "atomics_test.skel.h"
> > +
> > +static struct atomics_test *setup(void)
> > +{
> > +	struct atomics_test *atomics_skel;
> > +	__u32 duration = 0, err;
> > +
> > +	atomics_skel = atomics_test__open_and_load();
> > +	if (CHECK(!atomics_skel, "atomics_skel_load", "atomics skeleton failed\n"))
> > +		return NULL;
> > +
> > +	if (atomics_skel->data->skip_tests) {
> > +		printf("%s:SKIP:no ENABLE_ATOMICS_TEST (missing Clang BPF atomics support)",
> > +		       __func__);
> > +		test__skip();
> > +		goto err;
> > +	}
> > +
> > +	err = atomics_test__attach(atomics_skel);
> > +	if (CHECK(err, "atomics_attach", "atomics attach failed: %d\n", err))
> > +		goto err;
> > +
> > +	return atomics_skel;
> > +
> > +err:
> > +	atomics_test__destroy(atomics_skel);
> > +	return NULL;
> > +}
> > +
> > +static void test_add(void)
> > +{
> > +	struct atomics_test *atomics_skel;
> > +	int err, prog_fd;
> > +	__u32 duration = 0, retval;
> > +
> > +	atomics_skel = setup();
> 
> When running the test, I observed a noticeable delay between skel load and
> skel attach. The reason is the bpf program object file contains
> multiple programs and the above setup() tries to do attachment
> for ALL programs but actually below only "add" program is tested.
> This will unnecessarily increase test_progs running time.
> 
> The best is for setup() here only load and attach program "add".
> The libbpf API bpf_program__set_autoload() can set a particular
> program not autoload. You can call attach function explicitly
> for one specific program. This should be able to reduce test
> running time.

Interesting, thanks a lot - I'll try this out next week. Maybe we can
actually load all the progs once at the beginning (i.e. in
test_atomics_test) then attach/detch each prog individually as needed...
Sorry, I haven't got much of a grip on libbpf yet.