netdev - Re: [PATCH bpf-next v4 1/2] selftests/bpf: do not export subtest as standalone test

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 8 Feb 2022 09:36:50 +0800
From:   Hou Tao <houtao1@...wei.com>
To:     Yonghong Song <yhs@...com>, Hou Tao <hotforest@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>
CC:     Alexei Starovoitov <ast@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Andrii Nakryiko <andrii@...nel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        <netdev@...r.kernel.org>, <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next v4 1/2] selftests/bpf: do not export subtest as
 standalone test

Hi,

On 2/8/2022 2:17 AM, Yonghong Song wrote:
>
>
> On 2/5/22 8:31 PM, Hou Tao wrote:
>> Two subtests in ksyms_module.c are not qualified as static, so these
>> subtests are exported as standalone tests in tests.h and lead to
>> confusion for the output of "./test_progs -t ksyms_module".
>>
>> By using the following command:
>>
>>    grep "^void \(serial_\)\?test_[a-zA-Z0-9_]\+(\(void\)\?)" \
>>        tools/testing/selftests/bpf/prog_tests/*.c | \
>>     awk -F : '{print $1}' | sort | uniq -c | awk '$1 != 1'
>>
>> Find out that other tests also have the similar problem, so fix
>> these tests by marking subtests in these tests as static. For
>> xdp_adjust_frags.c, there is just one subtest, so just export
>> the subtest directly.
[...]
>> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
>> b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
>> index 134d0ac32f59..fc2d8fa8dac5 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
>> @@ -102,9 +102,3 @@ void test_xdp_update_frags(void)
>>   out:
>>       bpf_object__close(obj);
>>   }
>> -
>> -void test_xdp_adjust_frags(void)
>> -{
>> -    if (test__start_subtest("xdp_adjust_frags"))
>> -        test_xdp_update_frags();
>> -}
>
> I suggest keep test_xdp_adjust_frags and mark
> test_xdp_update_frags as static function, and
> this is also good for future extension.
> It is confusing that test_xdp_update_frags
> test in file xdp_adjust_frags.c. Typical
> prog_tests/ test has {test,serial_test}_<TEST> test
> with file name <TEST>.c file.
>
Will do and thanks for your suggestion.

Regards,
Tao
> [...]
> .