linux-kernel - Re: [PATCH v1 01/11] selftests: forwarding: custom_multipath_hash.sh: add cleanup for SIGTERM sent by timeout

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: <bdc5e82b-596d-d531-7685-0d1e52f2d125@alu.unizg.hr>
Date:   Mon, 31 Jul 2023 17:23:37 +0200
From:   Mirsad Todorovac <mirsad.todorovac@....unizg.hr>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     petrm@...dia.com, razor@...ckwall.org,
        Ido Schimmel <idosch@...dia.com>, netdev@...r.kernel.org,
        linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, Shuah Khan <shuah@...nel.org>
Subject: Re: [PATCH v1 01/11] selftests: forwarding: custom_multipath_hash.sh:
 add cleanup for SIGTERM sent by timeout

On 7/31/23 14:02, Ido Schimmel wrote:

>> NOTE: The error happened because two patches collided. This patch
>>
>> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
>> index 975fc5168c6334..40a8c1541b7f81 100755
>> --- a/tools/testing/selftests/net/forwarding/lib.sh
>> +++ b/tools/testing/selftests/net/forwarding/lib.sh
>> @@ -30,6 +30,7 @@ REQUIRE_MZ=${REQUIRE_MZ:=yes}
>>   REQUIRE_MTOOLS=${REQUIRE_MTOOLS:=no}
>>   STABLE_MAC_ADDRS=${STABLE_MAC_ADDRS:=no}
>>   TCPDUMP_EXTRA_FLAGS=${TCPDUMP_EXTRA_FLAGS:=}
>> +TROUTE6=${TROUTE6:=traceroute6}
>>   relative_path="${BASH_SOURCE%/*}"
>>   if [[ "$relative_path" == "${BASH_SOURCE}" ]]; then
>>
>> and this patch
>>
>> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
>> index 71f7c0c49677..5b0183013017 100755
>> --- a/tools/testing/selftests/net/forwarding/lib.sh
>> +++ b/tools/testing/selftests/net/forwarding/lib.sh
>> @@ -16,8 +16,6 @@ TEAMD=${TEAMD:=teamd}
>>   WAIT_TIME=${WAIT_TIME:=5}
>>   PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
>>   PAUSE_ON_CLEANUP=${PAUSE_ON_CLEANUP:=no}
>> -NETIF_TYPE=${NETIF_TYPE:=veth}
>> -NETIF_CREATE=${NETIF_CREATE:=yes}
>>   MCD=${MCD:=smcrouted}
>>   MC_CLI=${MC_CLI:=smcroutectl}
>>   PING_COUNT=${PING_COUNT:=10}
>> @@ -30,6 +28,20 @@ REQUIRE_MZ=${REQUIRE_MZ:=yes}
>>   REQUIRE_MTOOLS=${REQUIRE_MTOOLS:=no}
>>   STABLE_MAC_ADDRS=${STABLE_MAC_ADDRS:=no}
>>   TCPDUMP_EXTRA_FLAGS=${TCPDUMP_EXTRA_FLAGS:=}
>> +NETIF_TYPE=${NETIF_TYPE:=veth}
>> +NETIF_CREATE=${NETIF_CREATE:=yes}
>> +declare -A NETIFS=(
>> +       [p1]=veth0
>> +       [p2]=veth1
>> +       [p3]=veth2
>> +       [p4]=veth3
>> +       [p5]=veth4
>> +       [p6]=veth5
>> +       [p7]=veth6
>> +       [p8]=veth7
>> +       [p9]=veth8
>> +       [p10]=veth9
>> +)
>>
>>   relative_path="${BASH_SOURCE%/*}"
>>   if [[ "$relative_path" == "${BASH_SOURCE}" ]]; then
>>
>> are not compatible.
>>
>> I have applied the 'require_command $TROUTE6' patch manually.
>>
>> I suppose this is what you intended to have:
>>
>> # Can be overridden by the configuration file.
>> PING=${PING:=ping}
>> PING6=${PING6:=ping6}
>> MZ=${MZ:=mausezahn}
>> ARPING=${ARPING:=arping}
>> TEAMD=${TEAMD:=teamd}
>> WAIT_TIME=${WAIT_TIME:=5}
>> PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
>> PAUSE_ON_CLEANUP=${PAUSE_ON_CLEANUP:=no}
>> MCD=${MCD:=smcrouted}
>> MC_CLI=${MC_CLI:=smcroutectl}
>> PING_COUNT=${PING_COUNT:=10}
>> PING_TIMEOUT=${PING_TIMEOUT:=5}
>> WAIT_TIMEOUT=${WAIT_TIMEOUT:=20}
>> INTERFACE_TIMEOUT=${INTERFACE_TIMEOUT:=600}
>> LOW_AGEING_TIME=${LOW_AGEING_TIME:=1000}
>> REQUIRE_JQ=${REQUIRE_JQ:=yes}
>> REQUIRE_MZ=${REQUIRE_MZ:=yes}
>> REQUIRE_MTOOLS=${REQUIRE_MTOOLS:=no}
>> STABLE_MAC_ADDRS=${STABLE_MAC_ADDRS:=no}
>> TCPDUMP_EXTRA_FLAGS=${TCPDUMP_EXTRA_FLAGS:=}
>> TROUTE6=${TROUTE6:=traceroute6}
>> NETIF_TYPE=${NETIF_TYPE:=veth}
>> NETIF_CREATE=${NETIF_CREATE:=yes}
>> declare -A NETIFS=(
>>         [p1]=veth0
>>         [p2]=veth1
>>         [p3]=veth2
>>         [p4]=veth3
>>         [p5]=veth4
>>         [p6]=veth5
>>         [p7]=veth6
>>         [p8]=veth7
>>         [p9]=veth8
>>         [p10]=veth9
>> )
>>
>> relative_path="${BASH_SOURCE%/*}"
>> if [[ "$relative_path" == "${BASH_SOURCE}" ]]; then
>>          relative_path="."
>> fi
>> ------------------------------------------------
>>
>> Probably for the production patch you would like to have this fixed.
> 
> No, I don't intend to submit the patch that automatically creates the
> veth pairs. It is superseded by "selftests: forwarding: Skip test when
> no interfaces are specified".

It is your call, but consider that the majority of testers will use the default setup
and maybe grep "not ok" messages in the log, because the amount of logs is overwhelming.

Knowing that there is "forwarding.config.sample" probably requires in-depth knowledge
of the selftest net/forwarding bundle and maybe direct hint from the developers?

Kind regards,
Mirsad