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

From: Mirsad Todorovac
Date: Mon Jul 31 2023 - 11:14:34 EST

Next message: Greg KH: "Re: [PATCH 0/7] tty: synclink_gt: mark as BROKEN"
Previous message: Greg Kroah-Hartman: "Re: [PATCH v5 3/3] serial: core: Fix serial core controller port name to show controller id"
In reply to: Ido Schimmel: "Re: [PATCH v1 01/11] selftests: forwarding: custom_multipath_hash.sh: add cleanup for SIGTERM sent by timeout"
Next in thread: Ido Schimmel: "Re: [PATCH v1 01/11] selftests: forwarding: custom_multipath_hash.sh: add cleanup for SIGTERM sent by timeout"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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

On Mon, Jul 31, 2023 at 11:24:27AM +0200, Mirsad Todorovac wrote:

I guess that means only three are left.

# ./bridge_mdb.sh
dev br0 port veth1 grp 239.1.1.1 src 192.0.2.1 temp filter_mode include proto static vid 10 259.99
TEST: IPv4 (S, G) port group entries configuration tests [FAIL]
Entry has an unpending group timer after replace
dev br0 port veth1 grp ff0e::1 src 2001:db8:1::1 temp filter_mode include proto static vid 10 259.99
TEST: IPv6 (S, G) port group entries configuration tests [FAIL]
Entry has an unpending group timer after replace

I suspect that what happens here is that you have a faster system
than me or a different HZ value (check CONFIG_HZ, mine is 1000). The
group membership time is probably 260.00 which is why grepping for
"0.00" works when it shouldn't. Can you try the patch below? No need to
run all the other tests.

diff --git a/tools/testing/selftests/net/forwarding/bridge_mdb.sh b/tools/testing/selftests/net/forwarding/bridge_mdb.sh
index 8493c3dfc01e..41c33a2de0a6 100755
--- a/tools/testing/selftests/net/forwarding/bridge_mdb.sh
+++ b/tools/testing/selftests/net/forwarding/bridge_mdb.sh
@@ -617,7 +617,7 @@ __cfg_test_port_ip_sg()
grep -q "permanent"
check_err $? "Entry not added as \"permanent\" when should"
bridge -d -s mdb show dev br0 vid 10 | grep "$grp_key" | \
- grep -q "0.00"
+ grep -q " 0.00"
check_err $? "\"permanent\" entry has a pending group timer"
bridge mdb del dev br0 port $swp1 $grp_key vid 10
@@ -626,7 +626,7 @@ __cfg_test_port_ip_sg()
grep -q "temp"
check_err $? "Entry not added as \"temp\" when should"
bridge -d -s mdb show dev br0 vid 10 | grep "$grp_key" | \
- grep -q "0.00"
+ grep -q " 0.00"
check_fail $? "\"temp\" entry has an unpending group timer"
bridge mdb del dev br0 port $swp1 $grp_key vid 10
@@ -659,7 +659,7 @@ __cfg_test_port_ip_sg()
grep -q "permanent"
check_err $? "Entry not marked as \"permanent\" after replace"
bridge -d -s mdb show dev br0 vid 10 | grep "$grp_key" | \
- grep -q "0.00"
+ grep -q " 0.00"
check_err $? "Entry has a pending group timer after replace"
bridge mdb replace dev br0 port $swp1 $grp_key vid 10 temp
@@ -667,7 +667,7 @@ __cfg_test_port_ip_sg()
grep -q "temp"
check_err $? "Entry not marked as \"temp\" after replace"
bridge -d -s mdb show dev br0 vid 10 | grep "$grp_key" | \
- grep -q "0.00"
+ grep -q " 0.00"
check_fail $? "Entry has an unpending group timer after replace"
bridge mdb del dev br0 port $swp1 $grp_key vid 10

Congrats! This worked:

root@defiant:tools/testing/selftests/net/forwarding# ./bridge_mdb.sh

INFO: # Host entries configuration tests
TEST: Common host entries configuration tests (IPv4) [ OK ]
TEST: Common host entries configuration tests (IPv6) [ OK ]
TEST: Common host entries configuration tests (L2) [ OK ]

INFO: # Port group entries configuration tests - (*, G)
TEST: Common port group entries configuration tests (IPv4 (*, G)) [ OK ]
TEST: Common port group entries configuration tests (IPv6 (*, G)) [ OK ]
TEST: IPv4 (*, G) port group entries configuration tests [ OK ]
TEST: IPv6 (*, G) port group entries configuration tests [ OK ]

INFO: # Port group entries configuration tests - (S, G)
TEST: Common port group entries configuration tests (IPv4 (S, G)) [ OK ]
TEST: Common port group entries configuration tests (IPv6 (S, G)) [ OK ]
dev br0 port veth1 grp 239.1.1.1 src 192.0.2.1 temp filter_mode include proto static vid 10 259.99
TEST: IPv4 (S, G) port group entries configuration tests [ OK ]
dev br0 port veth1 grp ff0e::1 src 2001:db8:1::1 temp filter_mode include proto static vid 10 259.99
TEST: IPv6 (S, G) port group entries configuration tests [ OK ]

INFO: # Port group entries configuration tests - L2
TEST: Common port group entries configuration tests (L2 (*, G)) [ OK ]
TEST: L2 (*, G) port group entries configuration tests [ OK ]

INFO: # Large scale dump tests
TEST: IPv4 large scale dump tests [ OK ]
TEST: IPv6 large scale dump tests [ OK ]
TEST: L2 large scale dump tests [ OK ]

INFO: # Forwarding tests
TEST: IPv4 host entries forwarding tests [ OK ]
TEST: IPv6 host entries forwarding tests [ OK ]
TEST: L2 host entries forwarding tests [ OK ]
TEST: IPv4 port group "exclude" entries forwarding tests [ OK ]
TEST: IPv6 port group "exclude" entries forwarding tests [ OK ]
TEST: IPv4 port group "include" entries forwarding tests [ OK ]
TEST: IPv6 port group "include" entries forwarding tests [ OK ]
TEST: L2 port entries forwarding tests [ OK ]

INFO: # Control packets tests
TEST: IGMPv3 MODE_IS_INCLUDE tests [ OK ]
TEST: MLDv2 MODE_IS_INCLUDE tests [ OK ]
root@defiant:tools/testing/selftests/net/forwarding#

# ./bridge_vlan_mcast.sh
TEST: Vlan mcast_startup_query_interval global option default value [FAIL]
Wrong default mcast_startup_query_interval global vlan option value
# ./mirror_gre_changes.sh
TEST: mirror to gretap: TTL change (skip_hw) [FAIL]
Expected to capture 10 packets, got 15.
TEST: mirror to ip6gretap: TTL change (skip_hw) [FAIL]
Expected to capture 10 packets, got 13.
WARN: Could not test offloaded functionality

I hope Nik and Petr will find the time to look into those. If not, I
will check when I can.

Great. I will wait for the follow-up then ...

You can add:

Tested-by: Mirsad Todorovac <mirsad.todorovac@xxxxxxxxxxxx>

at your convenience. I think that it appropriate with the Code of Conduct.

Kind regards,
Mirsad

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".

Next message: Greg KH: "Re: [PATCH 0/7] tty: synclink_gt: mark as BROKEN"
Previous message: Greg Kroah-Hartman: "Re: [PATCH v5 3/3] serial: core: Fix serial core controller port name to show controller id"
In reply to: Ido Schimmel: "Re: [PATCH v1 01/11] selftests: forwarding: custom_multipath_hash.sh: add cleanup for SIGTERM sent by timeout"
Next in thread: Ido Schimmel: "Re: [PATCH v1 01/11] selftests: forwarding: custom_multipath_hash.sh: add cleanup for SIGTERM sent by timeout"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]