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: <6f008322-0b8f-223a-9148-ce9fee0810dc@fb.com>
Date:   Fri, 4 Dec 2020 07:28:12 -0800
From:   Yonghong Song <yhs@...com>
To:     Brendan Jackman <jackmanb@...gle.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 12/4/20 1:45 AM, Brendan Jackman wrote:
> 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

If you have subtest, people expects subtest can be individual runable.
This will complicate your logic.

> test_atomics_test) then attach/detch each prog individually as needed...
> Sorry, I haven't got much of a grip on libbpf yet.

One alternative is not to do subtests. There is nothing run to have
just one bpf program instead of many. This way, you load all and attach
once, then do all the test verification.