netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 00/17] selftests: forwarding: Various fixes
@ 2023-08-02  7:51 Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 01/17] selftests: forwarding: Skip test when no interfaces are specified Ido Schimmel
                   ` (16 more replies)
  0 siblings, 17 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel

Fix various problems with forwarding selftests. See individual patches
for problem description and solution.

Ido Schimmel (17):
  selftests: forwarding: Skip test when no interfaces are specified
  selftests: forwarding: Switch off timeout
  selftests: forwarding: bridge_mdb: Check iproute2 version
  selftests: forwarding: bridge_mdb_max: Check iproute2 version
  selftests: forwarding: Set default IPv6 traceroute utility
  selftests: forwarding: Add a helper to skip test when using veth pairs
  selftests: forwarding: ethtool: Skip when using veth pairs
  selftests: forwarding: ethtool_extended_state: Skip when using veth
    pairs
  selftests: forwarding: hw_stats_l3_gre: Skip when using veth pairs
  selftests: forwarding: ethtool_mm: Skip when using veth pairs
  selftests: forwarding: tc_actions: Use ncat instead of nc
  selftests: forwarding: tc_flower: Relax success criterion
  selftests: forwarding: tc_tunnel_key: Make filters more specific
  selftests: forwarding: tc_flower_l2_miss: Fix failing test with old
    libnet
  selftests: forwarding: bridge_mdb: Fix failing test with old libnet
  selftests: forwarding: bridge_mdb_max: Fix failing test with old
    libnet
  selftests: forwarding: bridge_mdb: Make test more robust

 .../selftests/net/forwarding/bridge_mdb.sh    | 59 +++++++++++--------
 .../net/forwarding/bridge_mdb_max.sh          | 19 ++++--
 .../selftests/net/forwarding/ethtool.sh       |  2 +
 .../net/forwarding/ethtool_extended_state.sh  |  2 +
 .../selftests/net/forwarding/ethtool_mm.sh    |  2 +
 .../net/forwarding/hw_stats_l3_gre.sh         |  2 +
 .../net/forwarding/ip6_forward_instats_vrf.sh |  2 +
 tools/testing/selftests/net/forwarding/lib.sh | 17 ++++++
 .../testing/selftests/net/forwarding/settings |  1 +
 .../selftests/net/forwarding/tc_actions.sh    |  6 +-
 .../selftests/net/forwarding/tc_flower.sh     |  8 +--
 .../net/forwarding/tc_flower_l2_miss.sh       | 13 ++--
 .../selftests/net/forwarding/tc_tunnel_key.sh |  9 ++-
 13 files changed, 98 insertions(+), 44 deletions(-)
 create mode 100644 tools/testing/selftests/net/forwarding/settings

-- 
2.40.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH net 01/17] selftests: forwarding: Skip test when no interfaces are specified
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 02/17] selftests: forwarding: Switch off timeout Ido Schimmel
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel

As explained in [1], the forwarding selftests are meant to be run with
either physical loopbacks or veth pairs. The interfaces are expected to
be specified in a user-provided forwarding.config file or as command
line arguments. By default, this file is not present and the tests fail:

 # make -C tools/testing/selftests TARGETS=net/forwarding run_tests
 [...]
 TAP version 13
 1..102
 # timeout set to 45
 # selftests: net/forwarding: bridge_igmp.sh
 # Command line is not complete. Try option "help"
 # Failed to create netif
 not ok 1 selftests: net/forwarding: bridge_igmp.sh # exit=1
 [...]

Fix by skipping a test if interfaces are not provided either via the
configuration file or command line arguments.

 # make -C tools/testing/selftests TARGETS=net/forwarding run_tests
 [...]
 TAP version 13
 1..102
 # timeout set to 45
 # selftests: net/forwarding: bridge_igmp.sh
 # SKIP: Cannot create interface. Name not specified
 ok 1 selftests: net/forwarding: bridge_igmp.sh # SKIP

[1] tools/testing/selftests/net/forwarding/README

Fixes: 81573b18f26d ("selftests/net/forwarding: add Makefile to install tests")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/856d454e-f83c-20cf-e166-6dc06cbc1543@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
 tools/testing/selftests/net/forwarding/lib.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 9ddb68dd6a08..975fc5168c63 100755
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -225,6 +225,11 @@ create_netif_veth()
 	for ((i = 1; i <= NUM_NETIFS; ++i)); do
 		local j=$((i+1))
 
+		if [ -z ${NETIFS[p$i]} ]; then
+			echo "SKIP: Cannot create interface. Name not specified"
+			exit $ksft_skip
+		fi
+
 		ip link show dev ${NETIFS[p$i]} &> /dev/null
 		if [[ $? -ne 0 ]]; then
 			ip link add ${NETIFS[p$i]} type veth \
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 02/17] selftests: forwarding: Switch off timeout
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 01/17] selftests: forwarding: Skip test when no interfaces are specified Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 03/17] selftests: forwarding: bridge_mdb: Check iproute2 version Ido Schimmel
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel

The default timeout for selftests is 45 seconds, but it is not enough
for forwarding selftests which can takes minutes to finish depending on
the number of tests cases:

 # make -C tools/testing/selftests TARGETS=net/forwarding run_tests
 TAP version 13
 1..102
 # timeout set to 45
 # selftests: net/forwarding: bridge_igmp.sh
 # TEST: IGMPv2 report 239.10.10.10                                    [ OK ]
 # TEST: IGMPv2 leave 239.10.10.10                                     [ OK ]
 # TEST: IGMPv3 report 239.10.10.10 is_include                         [ OK ]
 # TEST: IGMPv3 report 239.10.10.10 include -> allow                   [ OK ]
 #
 not ok 1 selftests: net/forwarding: bridge_igmp.sh # TIMEOUT 45 seconds

Fix by switching off the timeout and setting it to 0. A similar change
was done for BPF selftests in commit 6fc5916cc256 ("selftests: bpf:
Switch off timeout").

Fixes: 81573b18f26d ("selftests/net/forwarding: add Makefile to install tests")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/8d149f8c-818e-d141-a0ce-a6bae606bc22@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
 tools/testing/selftests/net/forwarding/settings | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 tools/testing/selftests/net/forwarding/settings

diff --git a/tools/testing/selftests/net/forwarding/settings b/tools/testing/selftests/net/forwarding/settings
new file mode 100644
index 000000000000..e7b9417537fb
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/settings
@@ -0,0 +1 @@
+timeout=0
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 03/17] selftests: forwarding: bridge_mdb: Check iproute2 version
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 01/17] selftests: forwarding: Skip test when no interfaces are specified Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 02/17] selftests: forwarding: Switch off timeout Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 04/17] selftests: forwarding: bridge_mdb_max: " Ido Schimmel
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel

The selftest relies on iproute2 changes present in version 6.3, but the
test does not check for it, resulting in error:

 # ./bridge_mdb.sh

 INFO: # Host entries configuration tests
 TEST: Common host entries configuration tests (IPv4)                [FAIL]
         Managed to add IPv4 host entry with a filter mode
 TEST: Common host entries configuration tests (IPv6)                [FAIL]
         Managed to add IPv6 host entry with a filter mode
 TEST: Common host entries configuration tests (L2)                  [FAIL]
         Managed to add L2 host entry with a filter mode

 INFO: # Port group entries configuration tests - (*, G)
 Command "replace" is unknown, try "bridge mdb help".
 [...]

Fix by skipping the test if iproute2 is too old.

Fixes: b6d00da08610 ("selftests: forwarding: Add bridge MDB test")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/6b04b2ba-2372-6f6b-3ac8-b7cba1cfae83@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
 tools/testing/selftests/net/forwarding/bridge_mdb.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/testing/selftests/net/forwarding/bridge_mdb.sh b/tools/testing/selftests/net/forwarding/bridge_mdb.sh
index ae3f9462a2b6..6f830b5f03c9 100755
--- a/tools/testing/selftests/net/forwarding/bridge_mdb.sh
+++ b/tools/testing/selftests/net/forwarding/bridge_mdb.sh
@@ -1206,6 +1206,11 @@ ctrl_test()
 	ctrl_mldv2_is_in_test
 }
 
+if ! bridge mdb help 2>&1 | grep -q "replace"; then
+	echo "SKIP: iproute2 too old, missing bridge mdb replace support"
+	exit $ksft_skip
+fi
+
 trap cleanup EXIT
 
 setup_prepare
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 04/17] selftests: forwarding: bridge_mdb_max: Check iproute2 version
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
                   ` (2 preceding siblings ...)
  2023-08-02  7:51 ` [PATCH net 03/17] selftests: forwarding: bridge_mdb: Check iproute2 version Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 05/17] selftests: forwarding: Set default IPv6 traceroute utility Ido Schimmel
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel

The selftest relies on iproute2 changes present in version 6.3, but the
test does not check for it, resulting in errors:

 # ./bridge_mdb_max.sh
  INFO: 802.1d tests
  TEST: cfg4: port: ngroups reporting                                 [FAIL]
          Number of groups was null, now is null, but 5 expected
  TEST: ctl4: port: ngroups reporting                                 [FAIL]
          Number of groups was null, now is null, but 5 expected
  TEST: cfg6: port: ngroups reporting                                 [FAIL]
          Number of groups was null, now is null, but 5 expected
  [...]

Fix by skipping the test if iproute2 is too old.

Fixes: 3446dcd7df05 ("selftests: forwarding: bridge_mdb_max: Add a new selftest")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/6b04b2ba-2372-6f6b-3ac8-b7cba1cfae83@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
 tools/testing/selftests/net/forwarding/bridge_mdb_max.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/testing/selftests/net/forwarding/bridge_mdb_max.sh b/tools/testing/selftests/net/forwarding/bridge_mdb_max.sh
index ae255b662ba3..fa762b716288 100755
--- a/tools/testing/selftests/net/forwarding/bridge_mdb_max.sh
+++ b/tools/testing/selftests/net/forwarding/bridge_mdb_max.sh
@@ -1328,6 +1328,11 @@ test_8021qvs()
 	switch_destroy
 }
 
+if ! bridge link help 2>&1 | grep -q "mcast_max_groups"; then
+	echo "SKIP: iproute2 too old, missing bridge \"mcast_max_groups\" support"
+	exit $ksft_skip
+fi
+
 trap cleanup EXIT
 
 setup_prepare
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 05/17] selftests: forwarding: Set default IPv6 traceroute utility
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
                   ` (3 preceding siblings ...)
  2023-08-02  7:51 ` [PATCH net 04/17] selftests: forwarding: bridge_mdb_max: " Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 06/17] selftests: forwarding: Add a helper to skip test when using veth pairs Ido Schimmel
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel, ssuryaextr

The test uses the 'TROUTE6' environment variable to encode the name of
the IPv6 traceroute utility. By default (without a configuration file),
this variable is not set, resulting in failures:

 # ./ip6_forward_instats_vrf.sh
 TEST: ping6                                                         [ OK ]
 TEST: Ip6InTooBigErrors                                             [ OK ]
 TEST: Ip6InHdrErrors                                                [FAIL]
 TEST: Ip6InAddrErrors                                               [ OK ]
 TEST: Ip6InDiscards                                                 [ OK ]

Fix by setting a default utility name and skip the test if the utility
is not present.

Fixes: 0857d6f8c759 ("ipv6: When forwarding count rx stats on the orig netdev")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
Cc: ssuryaextr@gmail.com
---
 .../testing/selftests/net/forwarding/ip6_forward_instats_vrf.sh | 2 ++
 tools/testing/selftests/net/forwarding/lib.sh                   | 1 +
 2 files changed, 3 insertions(+)

diff --git a/tools/testing/selftests/net/forwarding/ip6_forward_instats_vrf.sh b/tools/testing/selftests/net/forwarding/ip6_forward_instats_vrf.sh
index 9f5b3e2e5e95..49fa94b53a1c 100755
--- a/tools/testing/selftests/net/forwarding/ip6_forward_instats_vrf.sh
+++ b/tools/testing/selftests/net/forwarding/ip6_forward_instats_vrf.sh
@@ -14,6 +14,8 @@ ALL_TESTS="
 NUM_NETIFS=4
 source lib.sh
 
+require_command $TROUTE6
+
 h1_create()
 {
 	simple_if_init $h1 2001:1:1::2/64
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 975fc5168c63..40a8c1541b7f 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
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 06/17] selftests: forwarding: Add a helper to skip test when using veth pairs
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
                   ` (4 preceding siblings ...)
  2023-08-02  7:51 ` [PATCH net 05/17] selftests: forwarding: Set default IPv6 traceroute utility Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 07/17] selftests: forwarding: ethtool: Skip " Ido Schimmel
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel

A handful of tests require physical loopbacks to be used instead of veth
pairs. Add a helper that these tests will invoke in order to be skipped
when executed with veth pairs.

Fixes: 64916b57c0b1 ("selftests: forwarding: Add speed and auto-negotiation test")
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
 tools/testing/selftests/net/forwarding/lib.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 40a8c1541b7f..f69015bf2dea 100755
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -164,6 +164,17 @@ check_port_mab_support()
 	fi
 }
 
+skip_on_veth()
+{
+	local kind=$(ip -j -d link show dev ${NETIFS[p1]} |
+		jq -r '.[].linkinfo.info_kind')
+
+	if [[ $kind == veth ]]; then
+		echo "SKIP: Test cannot be run with veth pairs"
+		exit $ksft_skip
+	fi
+}
+
 if [[ "$(id -u)" -ne 0 ]]; then
 	echo "SKIP: need root privileges"
 	exit $ksft_skip
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 07/17] selftests: forwarding: ethtool: Skip when using veth pairs
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
                   ` (5 preceding siblings ...)
  2023-08-02  7:51 ` [PATCH net 06/17] selftests: forwarding: Add a helper to skip test when using veth pairs Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 08/17] selftests: forwarding: ethtool_extended_state: " Ido Schimmel
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel

Auto-negotiation cannot be tested with veth pairs, resulting in
failures:

 # ./ethtool.sh
 TEST: force of same speed autoneg off                               [FAIL]
         error in configuration. swp1 speed Not autoneg off
 [...]

Fix by skipping the test when used with veth pairs.

Fixes: 64916b57c0b1 ("selftests: forwarding: Add speed and auto-negotiation test")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
 tools/testing/selftests/net/forwarding/ethtool.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/net/forwarding/ethtool.sh b/tools/testing/selftests/net/forwarding/ethtool.sh
index dbb9fcf759e0..aa2eafb7b243 100755
--- a/tools/testing/selftests/net/forwarding/ethtool.sh
+++ b/tools/testing/selftests/net/forwarding/ethtool.sh
@@ -286,6 +286,8 @@ different_speeds_autoneg_on()
 	ethtool -s $h1 autoneg on
 }
 
+skip_on_veth
+
 trap cleanup EXIT
 
 setup_prepare
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 08/17] selftests: forwarding: ethtool_extended_state: Skip when using veth pairs
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
                   ` (6 preceding siblings ...)
  2023-08-02  7:51 ` [PATCH net 07/17] selftests: forwarding: ethtool: Skip " Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 09/17] selftests: forwarding: hw_stats_l3_gre: " Ido Schimmel
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel

Ethtool extended state cannot be tested with veth pairs, resulting in
failures:

 # ./ethtool_extended_state.sh
 TEST: Autoneg, No partner detected                                  [FAIL]
         Expected "Autoneg", got "Link detected: no"
 [...]

Fix by skipping the test when used with veth pairs.

Fixes: 7d10bcce98cd ("selftests: forwarding: Add tests for ethtool extended state")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
 .../testing/selftests/net/forwarding/ethtool_extended_state.sh  | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/net/forwarding/ethtool_extended_state.sh b/tools/testing/selftests/net/forwarding/ethtool_extended_state.sh
index 072faa77f53b..17f89c3b7c02 100755
--- a/tools/testing/selftests/net/forwarding/ethtool_extended_state.sh
+++ b/tools/testing/selftests/net/forwarding/ethtool_extended_state.sh
@@ -108,6 +108,8 @@ no_cable()
 	ip link set dev $swp3 down
 }
 
+skip_on_veth
+
 setup_prepare
 
 tests_run
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 09/17] selftests: forwarding: hw_stats_l3_gre: Skip when using veth pairs
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
                   ` (7 preceding siblings ...)
  2023-08-02  7:51 ` [PATCH net 08/17] selftests: forwarding: ethtool_extended_state: " Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 10/17] selftests: forwarding: ethtool_mm: " Ido Schimmel
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel

Layer 3 hardware stats cannot be used when the underlying interfaces are
veth pairs, resulting in failures:

 # ./hw_stats_l3_gre.sh
 TEST: ping gre flat                                                 [ OK ]
 TEST: Test rx packets:                                              [FAIL]
         Traffic not reflected in the counter: 0 -> 0
 TEST: Test tx packets:                                              [FAIL]
         Traffic not reflected in the counter: 0 -> 0

Fix by skipping the test when used with veth pairs.

Fixes: 813f97a26860 ("selftests: forwarding: Add a tunnel-based test for L3 HW stats")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
 tools/testing/selftests/net/forwarding/hw_stats_l3_gre.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/net/forwarding/hw_stats_l3_gre.sh b/tools/testing/selftests/net/forwarding/hw_stats_l3_gre.sh
index eb9ec4a68f84..7594bbb49029 100755
--- a/tools/testing/selftests/net/forwarding/hw_stats_l3_gre.sh
+++ b/tools/testing/selftests/net/forwarding/hw_stats_l3_gre.sh
@@ -99,6 +99,8 @@ test_stats_rx()
 	test_stats g2a rx
 }
 
+skip_on_veth
+
 trap cleanup EXIT
 
 setup_prepare
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 10/17] selftests: forwarding: ethtool_mm: Skip when using veth pairs
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
                   ` (8 preceding siblings ...)
  2023-08-02  7:51 ` [PATCH net 09/17] selftests: forwarding: hw_stats_l3_gre: " Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02 10:52   ` Vladimir Oltean
  2023-08-02  7:51 ` [PATCH net 11/17] selftests: forwarding: tc_actions: Use ncat instead of nc Ido Schimmel
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel, vladimir.oltean

MAC Merge cannot be tested with veth pairs, resulting in failures:

 # ./ethtool_mm.sh
 [...]
 TEST: Manual configuration with verification: swp1 to swp2          [FAIL]
         Verification did not succeed

Fix by skipping the test when used with veth pairs.

Fixes: e6991384ace5 ("selftests: forwarding: add a test for MAC Merge layer")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
Cc: vladimir.oltean@nxp.com
---
 tools/testing/selftests/net/forwarding/ethtool_mm.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
index c580ad623848..4331e2161e8d 100755
--- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh
+++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
@@ -278,6 +278,8 @@ cleanup()
 	h1_destroy
 }
 
+skip_on_veth
+
 trap cleanup EXIT
 
 setup_prepare
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 11/17] selftests: forwarding: tc_actions: Use ncat instead of nc
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
                   ` (9 preceding siblings ...)
  2023-08-02  7:51 ` [PATCH net 10/17] selftests: forwarding: ethtool_mm: " Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 12/17] selftests: forwarding: tc_flower: Relax success criterion Ido Schimmel
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel, dcaratti

The test relies on 'nc' being the netcat version from the nmap project.
While this seems to be the case on Fedora, it is not the case on Ubuntu,
resulting in failures such as [1].

Fix by explicitly using the 'ncat' utility from the nmap project and the
skip the test in case it is not installed.

[1]
 # timeout set to 0
 # selftests: net/forwarding: tc_actions.sh
 # TEST: gact drop and ok (skip_hw)                                    [ OK ]
 # TEST: mirred egress flower redirect (skip_hw)                       [ OK ]
 # TEST: mirred egress flower mirror (skip_hw)                         [ OK ]
 # TEST: mirred egress matchall mirror (skip_hw)                       [ OK ]
 # TEST: mirred_egress_to_ingress (skip_hw)                            [ OK ]
 # nc: invalid option -- '-'
 # usage: nc [-46CDdFhklNnrStUuvZz] [-I length] [-i interval] [-M ttl]
 #         [-m minttl] [-O length] [-P proxy_username] [-p source_port]
 #         [-q seconds] [-s sourceaddr] [-T keyword] [-V rtable] [-W recvlimit]
 #         [-w timeout] [-X proxy_protocol] [-x proxy_address[:port]]
 #         [destination] [port]
 # nc: invalid option -- '-'
 # usage: nc [-46CDdFhklNnrStUuvZz] [-I length] [-i interval] [-M ttl]
 #         [-m minttl] [-O length] [-P proxy_username] [-p source_port]
 #         [-q seconds] [-s sourceaddr] [-T keyword] [-V rtable] [-W recvlimit]
 #         [-w timeout] [-X proxy_protocol] [-x proxy_address[:port]]
 #         [destination] [port]
 # TEST: mirred_egress_to_ingress_tcp (skip_hw)                        [FAIL]
 #       server output check failed
 # INFO: Could not test offloaded functionality
 not ok 80 selftests: net/forwarding: tc_actions.sh # exit=1

Fixes: ca22da2fbd69 ("act_mirred: use the backlog for nested calls to mirred ingress")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
Cc: dcaratti@redhat.com
---
 tools/testing/selftests/net/forwarding/tc_actions.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/tc_actions.sh b/tools/testing/selftests/net/forwarding/tc_actions.sh
index a96cff8e7219..b0f5e55d2d0b 100755
--- a/tools/testing/selftests/net/forwarding/tc_actions.sh
+++ b/tools/testing/selftests/net/forwarding/tc_actions.sh
@@ -9,6 +9,8 @@ NUM_NETIFS=4
 source tc_common.sh
 source lib.sh
 
+require_command ncat
+
 tcflags="skip_hw"
 
 h1_create()
@@ -220,9 +222,9 @@ mirred_egress_to_ingress_tcp_test()
 		ip_proto icmp \
 			action drop
 
-	ip vrf exec v$h1 nc --recv-only -w10 -l -p 12345 -o $mirred_e2i_tf2  &
+	ip vrf exec v$h1 ncat --recv-only -w10 -l -p 12345 -o $mirred_e2i_tf2 &
 	local rpid=$!
-	ip vrf exec v$h1 nc -w1 --send-only 192.0.2.2 12345 <$mirred_e2i_tf1
+	ip vrf exec v$h1 ncat -w1 --send-only 192.0.2.2 12345 <$mirred_e2i_tf1
 	wait -n $rpid
 	cmp -s $mirred_e2i_tf1 $mirred_e2i_tf2
 	check_err $? "server output check failed"
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 12/17] selftests: forwarding: tc_flower: Relax success criterion
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
                   ` (10 preceding siblings ...)
  2023-08-02  7:51 ` [PATCH net 11/17] selftests: forwarding: tc_actions: Use ncat instead of nc Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 13/17] selftests: forwarding: tc_tunnel_key: Make filters more specific Ido Schimmel
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel

The test checks that filters that match on source or destination MAC
were only hit once. A host can send more than one packet with a given
source or destination MAC, resulting in failures.

Fix by relaxing the success criterion and instead check that the filters
were not hit zero times. Using tc_check_at_least_x_packets() is also an
option, but it is not available in older kernels.

Fixes: 07e5c75184a1 ("selftests: forwarding: Introduce tc flower matching tests")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
 tools/testing/selftests/net/forwarding/tc_flower.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/tc_flower.sh b/tools/testing/selftests/net/forwarding/tc_flower.sh
index 683711f41aa9..b1daad19b01e 100755
--- a/tools/testing/selftests/net/forwarding/tc_flower.sh
+++ b/tools/testing/selftests/net/forwarding/tc_flower.sh
@@ -52,8 +52,8 @@ match_dst_mac_test()
 	tc_check_packets "dev $h2 ingress" 101 1
 	check_fail $? "Matched on a wrong filter"
 
-	tc_check_packets "dev $h2 ingress" 102 1
-	check_err $? "Did not match on correct filter"
+	tc_check_packets "dev $h2 ingress" 102 0
+	check_fail $? "Did not match on correct filter"
 
 	tc filter del dev $h2 ingress protocol ip pref 1 handle 101 flower
 	tc filter del dev $h2 ingress protocol ip pref 2 handle 102 flower
@@ -78,8 +78,8 @@ match_src_mac_test()
 	tc_check_packets "dev $h2 ingress" 101 1
 	check_fail $? "Matched on a wrong filter"
 
-	tc_check_packets "dev $h2 ingress" 102 1
-	check_err $? "Did not match on correct filter"
+	tc_check_packets "dev $h2 ingress" 102 0
+	check_fail $? "Did not match on correct filter"
 
 	tc filter del dev $h2 ingress protocol ip pref 1 handle 101 flower
 	tc filter del dev $h2 ingress protocol ip pref 2 handle 102 flower
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 13/17] selftests: forwarding: tc_tunnel_key: Make filters more specific
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
                   ` (11 preceding siblings ...)
  2023-08-02  7:51 ` [PATCH net 12/17] selftests: forwarding: tc_flower: Relax success criterion Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  8:30   ` Davide Caratti
  2023-08-02  7:51 ` [PATCH net 14/17] selftests: forwarding: tc_flower_l2_miss: Fix failing test with old libnet Ido Schimmel
                   ` (3 subsequent siblings)
  16 siblings, 1 reply; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel, dcaratti

The test installs filters that match on various IP fragments (e.g., no
fragment, first fragment) and expects a certain amount of packets to hit
each filter. This is problematic as the filters are not specific enough
and can match IP packets (e.g., IGMP) generated by the stack, resulting
in failures [1].

Fix by making the filters more specific and match on more fields in the
IP header: Source IP, destination IP and protocol.

[1]
 # timeout set to 0
 # selftests: net/forwarding: tc_tunnel_key.sh
 # TEST: tunnel_key nofrag (skip_hw)                                   [FAIL]
 #       packet smaller than MTU was not tunneled
 # INFO: Could not test offloaded functionality
 not ok 89 selftests: net/forwarding: tc_tunnel_key.sh # exit=1

Fixes: 533a89b1940f ("selftests: forwarding: add tunnel_key "nofrag" test case")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
Cc: dcaratti@redhat.com
---
 tools/testing/selftests/net/forwarding/tc_tunnel_key.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh b/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh
index 5ac184d51809..5a5dd9034819 100755
--- a/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh
+++ b/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh
@@ -104,11 +104,14 @@ tunnel_key_nofrag_test()
 	local i
 
 	tc filter add dev $swp1 ingress protocol ip pref 100 handle 100 \
-		flower ip_flags nofrag action drop
+		flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \
+		ip_flags nofrag action drop
 	tc filter add dev $swp1 ingress protocol ip pref 101 handle 101 \
-		flower ip_flags firstfrag action drop
+		flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \
+		ip_flags firstfrag action drop
 	tc filter add dev $swp1 ingress protocol ip pref 102 handle 102 \
-		flower ip_flags nofirstfrag action drop
+		flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \
+		ip_flags nofirstfrag action drop
 
 	# test 'nofrag' set
 	tc filter add dev h1-et egress protocol all pref 1 handle 1 matchall $tcflags \
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 14/17] selftests: forwarding: tc_flower_l2_miss: Fix failing test with old libnet
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
                   ` (12 preceding siblings ...)
  2023-08-02  7:51 ` [PATCH net 13/17] selftests: forwarding: tc_tunnel_key: Make filters more specific Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 15/17] selftests: forwarding: bridge_mdb: " Ido Schimmel
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel

As explained in commit 8bcfb4ae4d97 ("selftests: forwarding: Fix failing
tests with old libnet"), old versions of libnet (used by mausezahn) do
not use the "SO_BINDTODEVICE" socket option. For IP unicast packets,
this can be solved by prefixing mausezahn invocations with "ip vrf
exec". However, IP multicast packets do not perform routing and simply
egress the bound device, which does not exist in this case.

Fix by specifying the source and destination MAC of the packet which
will cause mausezahn to use a packet socket instead of an IP socket.

Fixes: 8c33266ae26a ("selftests: forwarding: Add layer 2 miss test cases")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
 .../selftests/net/forwarding/tc_flower_l2_miss.sh   | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/tc_flower_l2_miss.sh b/tools/testing/selftests/net/forwarding/tc_flower_l2_miss.sh
index e22c2d28b6eb..20a7cb7222b8 100755
--- a/tools/testing/selftests/net/forwarding/tc_flower_l2_miss.sh
+++ b/tools/testing/selftests/net/forwarding/tc_flower_l2_miss.sh
@@ -127,6 +127,7 @@ test_l2_miss_multicast_common()
 	local proto=$1; shift
 	local sip=$1; shift
 	local dip=$1; shift
+	local dmac=$1; shift
 	local mode=$1; shift
 	local name=$1; shift
 
@@ -142,7 +143,7 @@ test_l2_miss_multicast_common()
 	   action pass
 
 	# Before adding MDB entry.
-	$MZ $mode $h1 -t ip -A $sip -B $dip -c 1 -p 100 -q
+	$MZ $mode $h1 -a own -b $dmac -t ip -A $sip -B $dip -c 1 -p 100 -q
 
 	tc_check_packets "dev $swp2 egress" 101 1
 	check_err $? "Unregistered multicast filter was not hit before adding MDB entry"
@@ -153,7 +154,7 @@ test_l2_miss_multicast_common()
 	# Adding MDB entry.
 	bridge mdb replace dev br1 port $swp2 grp $dip permanent
 
-	$MZ $mode $h1 -t ip -A $sip -B $dip -c 1 -p 100 -q
+	$MZ $mode $h1 -a own -b $dmac -t ip -A $sip -B $dip -c 1 -p 100 -q
 
 	tc_check_packets "dev $swp2 egress" 101 1
 	check_err $? "Unregistered multicast filter was hit after adding MDB entry"
@@ -164,7 +165,7 @@ test_l2_miss_multicast_common()
 	# Deleting MDB entry.
 	bridge mdb del dev br1 port $swp2 grp $dip
 
-	$MZ $mode $h1 -t ip -A $sip -B $dip -c 1 -p 100 -q
+	$MZ $mode $h1 -a own -b $dmac -t ip -A $sip -B $dip -c 1 -p 100 -q
 
 	tc_check_packets "dev $swp2 egress" 101 2
 	check_err $? "Unregistered multicast filter was not hit after deleting MDB entry"
@@ -183,10 +184,11 @@ test_l2_miss_multicast_ipv4()
 	local proto="ipv4"
 	local sip=192.0.2.1
 	local dip=239.1.1.1
+	local dmac=01:00:5e:01:01:01
 	local mode="-4"
 	local name="IPv4"
 
-	test_l2_miss_multicast_common $proto $sip $dip $mode $name
+	test_l2_miss_multicast_common $proto $sip $dip $dmac $mode $name
 }
 
 test_l2_miss_multicast_ipv6()
@@ -194,10 +196,11 @@ test_l2_miss_multicast_ipv6()
 	local proto="ipv6"
 	local sip=2001:db8:1::1
 	local dip=ff0e::1
+	local dmac=33:33:00:00:00:01
 	local mode="-6"
 	local name="IPv6"
 
-	test_l2_miss_multicast_common $proto $sip $dip $mode $name
+	test_l2_miss_multicast_common $proto $sip $dip $dmac $mode $name
 }
 
 test_l2_miss_multicast()
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 15/17] selftests: forwarding: bridge_mdb: Fix failing test with old libnet
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
                   ` (13 preceding siblings ...)
  2023-08-02  7:51 ` [PATCH net 14/17] selftests: forwarding: tc_flower_l2_miss: Fix failing test with old libnet Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 16/17] selftests: forwarding: bridge_mdb_max: " Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 17/17] selftests: forwarding: bridge_mdb: Make test more robust Ido Schimmel
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel

As explained in commit 8bcfb4ae4d97 ("selftests: forwarding: Fix failing
tests with old libnet"), old versions of libnet (used by mausezahn) do
not use the "SO_BINDTODEVICE" socket option. For IP unicast packets,
this can be solved by prefixing mausezahn invocations with "ip vrf
exec". However, IP multicast packets do not perform routing and simply
egress the bound device, which does not exist in this case.

Fix by specifying the source and destination MAC of the packet which
will cause mausezahn to use a packet socket instead of an IP socket.

Fixes: b6d00da08610 ("selftests: forwarding: Add bridge MDB test")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
 .../selftests/net/forwarding/bridge_mdb.sh    | 46 ++++++++++---------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/bridge_mdb.sh b/tools/testing/selftests/net/forwarding/bridge_mdb.sh
index 6f830b5f03c9..4853b8e4f8d3 100755
--- a/tools/testing/selftests/net/forwarding/bridge_mdb.sh
+++ b/tools/testing/selftests/net/forwarding/bridge_mdb.sh
@@ -850,6 +850,7 @@ cfg_test()
 __fwd_test_host_ip()
 {
 	local grp=$1; shift
+	local dmac=$1; shift
 	local src=$1; shift
 	local mode=$1; shift
 	local name
@@ -872,27 +873,27 @@ __fwd_test_host_ip()
 	# Packet should only be flooded to multicast router ports when there is
 	# no matching MDB entry. The bridge is not configured as a multicast
 	# router port.
-	$MZ $mode $h1.10 -c 1 -p 128 -A $src -B $grp -t udp -q
+	$MZ $mode $h1.10 -a own -b $dmac -c 1 -p 128 -A $src -B $grp -t udp -q
 	tc_check_packets "dev br0 ingress" 1 0
 	check_err $? "Packet locally received after flood"
 
 	# Install a regular port group entry and expect the packet to not be
 	# locally received.
 	bridge mdb add dev br0 port $swp2 grp $grp temp vid 10
-	$MZ $mode $h1.10 -c 1 -p 128 -A $src -B $grp -t udp -q
+	$MZ $mode $h1.10 -a own -b $dmac -c 1 -p 128 -A $src -B $grp -t udp -q
 	tc_check_packets "dev br0 ingress" 1 0
 	check_err $? "Packet locally received after installing a regular entry"
 
 	# Add a host entry and expect the packet to be locally received.
 	bridge mdb add dev br0 port br0 grp $grp temp vid 10
-	$MZ $mode $h1.10 -c 1 -p 128 -A $src -B $grp -t udp -q
+	$MZ $mode $h1.10 -a own -b $dmac -c 1 -p 128 -A $src -B $grp -t udp -q
 	tc_check_packets "dev br0 ingress" 1 1
 	check_err $? "Packet not locally received after adding a host entry"
 
 	# Remove the host entry and expect the packet to not be locally
 	# received.
 	bridge mdb del dev br0 port br0 grp $grp vid 10
-	$MZ $mode $h1.10 -c 1 -p 128 -A $src -B $grp -t udp -q
+	$MZ $mode $h1.10 -a own -b $dmac -c 1 -p 128 -A $src -B $grp -t udp -q
 	tc_check_packets "dev br0 ingress" 1 1
 	check_err $? "Packet locally received after removing a host entry"
 
@@ -905,8 +906,8 @@ __fwd_test_host_ip()
 
 fwd_test_host_ip()
 {
-	__fwd_test_host_ip "239.1.1.1" "192.0.2.1" "-4"
-	__fwd_test_host_ip "ff0e::1" "2001:db8:1::1" "-6"
+	__fwd_test_host_ip "239.1.1.1" "01:00:5e:01:01:01" "192.0.2.1" "-4"
+	__fwd_test_host_ip "ff0e::1" "33:33:00:00:00:01" "2001:db8:1::1" "-6"
 }
 
 fwd_test_host_l2()
@@ -966,6 +967,7 @@ fwd_test_host()
 __fwd_test_port_ip()
 {
 	local grp=$1; shift
+	local dmac=$1; shift
 	local valid_src=$1; shift
 	local invalid_src=$1; shift
 	local mode=$1; shift
@@ -999,43 +1001,43 @@ __fwd_test_port_ip()
 		vlan_ethtype $eth_type vlan_id 10 dst_ip $grp \
 		src_ip $invalid_src action drop
 
-	$MZ $mode $h1.10 -c 1 -p 128 -A $valid_src -B $grp -t udp -q
+	$MZ $mode $h1.10 -a own -b $dmac -c 1 -p 128 -A $valid_src -B $grp -t udp -q
 	tc_check_packets "dev $h2 ingress" 1 0
 	check_err $? "Packet from valid source received on H2 before adding entry"
 
-	$MZ $mode $h1.10 -c 1 -p 128 -A $invalid_src -B $grp -t udp -q
+	$MZ $mode $h1.10 -a own -b $dmac -c 1 -p 128 -A $invalid_src -B $grp -t udp -q
 	tc_check_packets "dev $h2 ingress" 2 0
 	check_err $? "Packet from invalid source received on H2 before adding entry"
 
 	bridge mdb add dev br0 port $swp2 grp $grp vid 10 \
 		filter_mode $filter_mode source_list $src_list
 
-	$MZ $mode $h1.10 -c 1 -p 128 -A $valid_src -B $grp -t udp -q
+	$MZ $mode $h1.10 -a own -b $dmac -c 1 -p 128 -A $valid_src -B $grp -t udp -q
 	tc_check_packets "dev $h2 ingress" 1 1
 	check_err $? "Packet from valid source not received on H2 after adding entry"
 
-	$MZ $mode $h1.10 -c 1 -p 128 -A $invalid_src -B $grp -t udp -q
+	$MZ $mode $h1.10 -a own -b $dmac -c 1 -p 128 -A $invalid_src -B $grp -t udp -q
 	tc_check_packets "dev $h2 ingress" 2 0
 	check_err $? "Packet from invalid source received on H2 after adding entry"
 
 	bridge mdb replace dev br0 port $swp2 grp $grp vid 10 \
 		filter_mode exclude
 
-	$MZ $mode $h1.10 -c 1 -p 128 -A $valid_src -B $grp -t udp -q
+	$MZ $mode $h1.10 -a own -b $dmac -c 1 -p 128 -A $valid_src -B $grp -t udp -q
 	tc_check_packets "dev $h2 ingress" 1 2
 	check_err $? "Packet from valid source not received on H2 after allowing all sources"
 
-	$MZ $mode $h1.10 -c 1 -p 128 -A $invalid_src -B $grp -t udp -q
+	$MZ $mode $h1.10 -a own -b $dmac -c 1 -p 128 -A $invalid_src -B $grp -t udp -q
 	tc_check_packets "dev $h2 ingress" 2 1
 	check_err $? "Packet from invalid source not received on H2 after allowing all sources"
 
 	bridge mdb del dev br0 port $swp2 grp $grp vid 10
 
-	$MZ $mode $h1.10 -c 1 -p 128 -A $valid_src -B $grp -t udp -q
+	$MZ $mode $h1.10 -a own -b $dmac -c 1 -p 128 -A $valid_src -B $grp -t udp -q
 	tc_check_packets "dev $h2 ingress" 1 2
 	check_err $? "Packet from valid source received on H2 after deleting entry"
 
-	$MZ $mode $h1.10 -c 1 -p 128 -A $invalid_src -B $grp -t udp -q
+	$MZ $mode $h1.10 -a own -b $dmac -c 1 -p 128 -A $invalid_src -B $grp -t udp -q
 	tc_check_packets "dev $h2 ingress" 2 1
 	check_err $? "Packet from invalid source received on H2 after deleting entry"
 
@@ -1047,11 +1049,11 @@ __fwd_test_port_ip()
 
 fwd_test_port_ip()
 {
-	__fwd_test_port_ip "239.1.1.1" "192.0.2.1" "192.0.2.2" "-4" "exclude"
-	__fwd_test_port_ip "ff0e::1" "2001:db8:1::1" "2001:db8:1::2" "-6" \
+	__fwd_test_port_ip "239.1.1.1" "01:00:5e:01:01:01" "192.0.2.1" "192.0.2.2" "-4" "exclude"
+	__fwd_test_port_ip "ff0e::1" "33:33:00:00:00:01" "2001:db8:1::1" "2001:db8:1::2" "-6" \
 		"exclude"
-	__fwd_test_port_ip "239.1.1.1" "192.0.2.1" "192.0.2.2" "-4" "include"
-	__fwd_test_port_ip "ff0e::1" "2001:db8:1::1" "2001:db8:1::2" "-6" \
+	__fwd_test_port_ip "239.1.1.1" "01:00:5e:01:01:01" "192.0.2.1" "192.0.2.2" "-4" "include"
+	__fwd_test_port_ip "ff0e::1" "33:33:00:00:00:01" "2001:db8:1::1" "2001:db8:1::2" "-6" \
 		"include"
 }
 
@@ -1127,7 +1129,7 @@ ctrl_igmpv3_is_in_test()
 		filter_mode include source_list 192.0.2.1
 
 	# IS_IN ( 192.0.2.2 )
-	$MZ $h1.10 -c 1 -A 192.0.2.1 -B 239.1.1.1 \
+	$MZ $h1.10 -c 1 -a own -b 01:00:5e:01:01:01 -A 192.0.2.1 -B 239.1.1.1 \
 		-t ip proto=2,p=$(igmpv3_is_in_get 239.1.1.1 192.0.2.2) -q
 
 	bridge -d mdb show dev br0 vid 10 | grep 239.1.1.1 | grep -q 192.0.2.2
@@ -1140,7 +1142,7 @@ ctrl_igmpv3_is_in_test()
 		filter_mode include source_list 192.0.2.1
 
 	# IS_IN ( 192.0.2.2 )
-	$MZ $h1.10 -c 1 -A 192.0.2.1 -B 239.1.1.1 \
+	$MZ $h1.10 -a own -b 01:00:5e:01:01:01 -c 1 -A 192.0.2.1 -B 239.1.1.1 \
 		-t ip proto=2,p=$(igmpv3_is_in_get 239.1.1.1 192.0.2.2) -q
 
 	bridge -d mdb show dev br0 vid 10 | grep 239.1.1.1 | grep -v "src" | \
@@ -1167,7 +1169,7 @@ ctrl_mldv2_is_in_test()
 
 	# IS_IN ( 2001:db8:1::2 )
 	local p=$(mldv2_is_in_get fe80::1 ff0e::1 2001:db8:1::2)
-	$MZ -6 $h1.10 -c 1 -A fe80::1 -B ff0e::1 \
+	$MZ -6 $h1.10 -a own -b 33:33:00:00:00:01 -c 1 -A fe80::1 -B ff0e::1 \
 		-t ip hop=1,next=0,p="$p" -q
 
 	bridge -d mdb show dev br0 vid 10 | grep ff0e::1 | \
@@ -1181,7 +1183,7 @@ ctrl_mldv2_is_in_test()
 		filter_mode include source_list 2001:db8:1::1
 
 	# IS_IN ( 2001:db8:1::2 )
-	$MZ -6 $h1.10 -c 1 -A fe80::1 -B ff0e::1 \
+	$MZ -6 $h1.10 -a own -b 33:33:00:00:00:01 -c 1 -A fe80::1 -B ff0e::1 \
 		-t ip hop=1,next=0,p="$p" -q
 
 	bridge -d mdb show dev br0 vid 10 | grep ff0e::1 | grep -v "src" | \
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 16/17] selftests: forwarding: bridge_mdb_max: Fix failing test with old libnet
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
                   ` (14 preceding siblings ...)
  2023-08-02  7:51 ` [PATCH net 15/17] selftests: forwarding: bridge_mdb: " Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  2023-08-02  7:51 ` [PATCH net 17/17] selftests: forwarding: bridge_mdb: Make test more robust Ido Schimmel
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel

As explained in commit 8bcfb4ae4d97 ("selftests: forwarding: Fix failing
tests with old libnet"), old versions of libnet (used by mausezahn) do
not use the "SO_BINDTODEVICE" socket option. For IP unicast packets,
this can be solved by prefixing mausezahn invocations with "ip vrf
exec". However, IP multicast packets do not perform routing and simply
egress the bound device, which does not exist in this case.

Fix by specifying the source and destination MAC of the packet which
will cause mausezahn to use a packet socket instead of an IP socket.

Fixes: 3446dcd7df05 ("selftests: forwarding: bridge_mdb_max: Add a new selftest")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
 .../selftests/net/forwarding/bridge_mdb_max.sh     | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/bridge_mdb_max.sh b/tools/testing/selftests/net/forwarding/bridge_mdb_max.sh
index fa762b716288..3da9d93ab36f 100755
--- a/tools/testing/selftests/net/forwarding/bridge_mdb_max.sh
+++ b/tools/testing/selftests/net/forwarding/bridge_mdb_max.sh
@@ -252,7 +252,8 @@ ctl4_entries_add()
 	local IPs=$(seq -f 192.0.2.%g 1 $((n - 1)))
 	local peer=$(locus_dev_peer $locus)
 	local GRP=239.1.1.${grp}
-	$MZ $peer -c 1 -A 192.0.2.1 -B $GRP \
+	local dmac=01:00:5e:01:01:$(printf "%02x" $grp)
+	$MZ $peer -a own -b $dmac -c 1 -A 192.0.2.1 -B $GRP \
 		-t ip proto=2,p=$(igmpv3_is_in_get $GRP $IPs) -q
 	sleep 1
 
@@ -272,7 +273,8 @@ ctl4_entries_del()
 
 	local peer=$(locus_dev_peer $locus)
 	local GRP=239.1.1.${grp}
-	$MZ $peer -c 1 -A 192.0.2.1 -B 224.0.0.2 \
+	local dmac=01:00:5e:00:00:02
+	$MZ $peer -a own -b $dmac -c 1 -A 192.0.2.1 -B 224.0.0.2 \
 		-t ip proto=2,p=$(igmpv2_leave_get $GRP) -q
 	sleep 1
 	! bridge mdb show dev br0 | grep -q $GRP
@@ -289,8 +291,10 @@ ctl6_entries_add()
 	local peer=$(locus_dev_peer $locus)
 	local SIP=fe80::1
 	local GRP=ff0e::${grp}
+	local dmac=33:33:00:00:00:$(printf "%02x" $grp)
 	local p=$(mldv2_is_in_get $SIP $GRP $IPs)
-	$MZ -6 $peer -c 1 -A $SIP -B $GRP -t ip hop=1,next=0,p="$p" -q
+	$MZ -6 $peer -a own -b $dmac -c 1 -A $SIP -B $GRP \
+		-t ip hop=1,next=0,p="$p" -q
 	sleep 1
 
 	local nn=$(bridge mdb show dev br0 | grep $GRP | wc -l)
@@ -310,8 +314,10 @@ ctl6_entries_del()
 	local peer=$(locus_dev_peer $locus)
 	local SIP=fe80::1
 	local GRP=ff0e::${grp}
+	local dmac=33:33:00:00:00:$(printf "%02x" $grp)
 	local p=$(mldv1_done_get $SIP $GRP)
-	$MZ -6 $peer -c 1 -A $SIP -B $GRP -t ip hop=1,next=0,p="$p" -q
+	$MZ -6 $peer -a own -b $dmac -c 1 -A $SIP -B $GRP \
+		-t ip hop=1,next=0,p="$p" -q
 	sleep 1
 	! bridge mdb show dev br0 | grep -q $GRP
 }
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH net 17/17] selftests: forwarding: bridge_mdb: Make test more robust
  2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
                   ` (15 preceding siblings ...)
  2023-08-02  7:51 ` [PATCH net 16/17] selftests: forwarding: bridge_mdb_max: " Ido Schimmel
@ 2023-08-02  7:51 ` Ido Schimmel
  16 siblings, 0 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  7:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, petrm, razor, mirsad.todorovac,
	Ido Schimmel

Some test cases check that the group timer is (or isn't) 0. Instead of
grepping for "0.00" grep for " 0.00" as the former can also match
"260.00" which is the default group membership interval.

Fixes: b6d00da08610 ("selftests: forwarding: Add bridge MDB test")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
---
 tools/testing/selftests/net/forwarding/bridge_mdb.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/bridge_mdb.sh b/tools/testing/selftests/net/forwarding/bridge_mdb.sh
index 4853b8e4f8d3..d0c6c499d5da 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
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH net 13/17] selftests: forwarding: tc_tunnel_key: Make filters more specific
  2023-08-02  7:51 ` [PATCH net 13/17] selftests: forwarding: tc_tunnel_key: Make filters more specific Ido Schimmel
@ 2023-08-02  8:30   ` Davide Caratti
  2023-08-02  8:37     ` Ido Schimmel
  0 siblings, 1 reply; 26+ messages in thread
From: Davide Caratti @ 2023-08-02  8:30 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev, davem, kuba, pabeni, edumazet, petrm, razor,
	mirsad.todorovac

On Wed, Aug 02, 2023 at 10:51:14AM +0300, Ido Schimmel wrote:
> The test installs filters that match on various IP fragments (e.g., no
> fragment, first fragment) and expects a certain amount of packets to hit
> each filter. This is problematic as the filters are not specific enough
> and can match IP packets (e.g., IGMP) generated by the stack, resulting
> in failures [1].

[...]

> --- a/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh
> +++ b/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh
> @@ -104,11 +104,14 @@ tunnel_key_nofrag_test()
>  	local i
>  
>  	tc filter add dev $swp1 ingress protocol ip pref 100 handle 100 \
> -		flower ip_flags nofrag action drop
> +		flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \
> +		ip_flags nofrag action drop
>  	tc filter add dev $swp1 ingress protocol ip pref 101 handle 101 \
> -		flower ip_flags firstfrag action drop
> +		flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \
> +		ip_flags firstfrag action drop
>  	tc filter add dev $swp1 ingress protocol ip pref 102 handle 102 \
> -		flower ip_flags nofirstfrag action drop
> +		flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \
> +		ip_flags nofirstfrag action drop


hello Ido, my 2 cents:

is it safe to match on the UDP protocol without changing the mausezahn
command line? I see that it's generating generic IP packets at the
moment (i.e. it does '-t ip'). Maybe it's more robust to change
the test to generate ICMP and then match on the ICMP protocol?

thanks!
-- 
davide

 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH net 13/17] selftests: forwarding: tc_tunnel_key: Make filters more specific
  2023-08-02  8:30   ` Davide Caratti
@ 2023-08-02  8:37     ` Ido Schimmel
  2023-08-02  8:52       ` Davide Caratti
  0 siblings, 1 reply; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02  8:37 UTC (permalink / raw)
  To: Davide Caratti
  Cc: Ido Schimmel, netdev, davem, kuba, pabeni, edumazet, petrm, razor,
	mirsad.todorovac

On Wed, Aug 02, 2023 at 10:30:52AM +0200, Davide Caratti wrote:
> On Wed, Aug 02, 2023 at 10:51:14AM +0300, Ido Schimmel wrote:
> > The test installs filters that match on various IP fragments (e.g., no
> > fragment, first fragment) and expects a certain amount of packets to hit
> > each filter. This is problematic as the filters are not specific enough
> > and can match IP packets (e.g., IGMP) generated by the stack, resulting
> > in failures [1].
> 
> [...]
> 
> > --- a/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh
> > +++ b/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh
> > @@ -104,11 +104,14 @@ tunnel_key_nofrag_test()
> >  	local i
> >  
> >  	tc filter add dev $swp1 ingress protocol ip pref 100 handle 100 \
> > -		flower ip_flags nofrag action drop
> > +		flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \
> > +		ip_flags nofrag action drop
> >  	tc filter add dev $swp1 ingress protocol ip pref 101 handle 101 \
> > -		flower ip_flags firstfrag action drop
> > +		flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \
> > +		ip_flags firstfrag action drop
> >  	tc filter add dev $swp1 ingress protocol ip pref 102 handle 102 \
> > -		flower ip_flags nofirstfrag action drop
> > +		flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \
> > +		ip_flags nofirstfrag action drop
> 
> 
> hello Ido, my 2 cents:
> 
> is it safe to match on the UDP protocol without changing the mausezahn
> command line? I see that it's generating generic IP packets at the
> moment (i.e. it does '-t ip'). Maybe it's more robust to change
> the test to generate ICMP and then match on the ICMP protocol?

My understanding of the test is that it's transmitting IP packets on the
VXLAN device and what $swp1 sees are the encapsulated packets (UDP).

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH net 13/17] selftests: forwarding: tc_tunnel_key: Make filters more specific
  2023-08-02  8:37     ` Ido Schimmel
@ 2023-08-02  8:52       ` Davide Caratti
  0 siblings, 0 replies; 26+ messages in thread
From: Davide Caratti @ 2023-08-02  8:52 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: Ido Schimmel, netdev, davem, kuba, pabeni, edumazet, petrm, razor,
	mirsad.todorovac

On Wed, Aug 2, 2023 at 10:38 AM Ido Schimmel <idosch@idosch.org> wrote:
>
> On Wed, Aug 02, 2023 at 10:30:52AM +0200, Davide Caratti wrote:

[...]

> >
> > hello Ido, my 2 cents:
> >
> > is it safe to match on the UDP protocol without changing the mausezahn
> > command line? I see that it's generating generic IP packets at the
> > moment (i.e. it does '-t ip'). Maybe it's more robust to change
> > the test to generate ICMP and then match on the ICMP protocol?
>
> My understanding of the test is that it's transmitting IP packets on the
> VXLAN device and what $swp1 sees are the encapsulated packets (UDP).
>

Ah, right :) sorry for the noise!

Acked-by: Davide Caratti <dcaratti@redhat.com>


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH net 10/17] selftests: forwarding: ethtool_mm: Skip when using veth pairs
  2023-08-02  7:51 ` [PATCH net 10/17] selftests: forwarding: ethtool_mm: " Ido Schimmel
@ 2023-08-02 10:52   ` Vladimir Oltean
  2023-08-02 12:27     ` Petr Machata
  0 siblings, 1 reply; 26+ messages in thread
From: Vladimir Oltean @ 2023-08-02 10:52 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev, davem, kuba, pabeni, edumazet, petrm, razor,
	mirsad.todorovac

Hi Ido,

On Wed, Aug 02, 2023 at 10:51:11AM +0300, Ido Schimmel wrote:
> MAC Merge cannot be tested with veth pairs, resulting in failures:
> 
>  # ./ethtool_mm.sh
>  [...]
>  TEST: Manual configuration with verification: swp1 to swp2          [FAIL]
>          Verification did not succeed
> 
> Fix by skipping the test when used with veth pairs.
> 
> Fixes: e6991384ace5 ("selftests: forwarding: add a test for MAC Merge layer")
> Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
> Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> Reviewed-by: Petr Machata <petrm@nvidia.com>
> Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
> ---

That will skip the selftest just for veth pairs. This will skip it for
any device that doesn't support the MAC Merge layer:

diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
index c580ad623848..5432848a3c59 100755
--- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh
+++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
@@ -224,6 +224,8 @@ h1_create()
 		hw 1

 	ethtool --set-mm $h1 pmac-enabled on tx-enabled off verify-enabled off
+
+	h1_created=yes
 }

 h2_create()
@@ -236,10 +238,16 @@ h2_create()
 		queues 1@0 1@1 1@2 1@3 \
 		fp P E E E \
 		hw 1
+
+	h2_created=yes
 }

 h1_destroy()
 {
+	if ! [[ $h1_created = yes ]]; then
+		return
+	fi
+
 	ethtool --set-mm $h1 pmac-enabled off tx-enabled off verify-enabled off

 	tc qdisc del dev $h1 root
@@ -249,6 +257,10 @@ h1_destroy()

 h2_destroy()
 {
+	if ! [[ $h2_created = yes ]]; then
+		return
+	fi
+
 	tc qdisc del dev $h2 root

 	ethtool --set-mm $h2 pmac-enabled off tx-enabled off verify-enabled off
@@ -266,6 +278,14 @@ setup_prepare()
 	h1=${NETIFS[p1]}
 	h2=${NETIFS[p2]}

+	for netif in ${NETIFS[@]}; do
+		ethtool --show-mm $netif 2>&1 &> /dev/null
+		if [[ $? -ne 0 ]]; then
+			echo "SKIP: $netif does not support MAC Merge"
+			exit $ksft_skip
+		fi
+	done
+
 	h1_create
 	h2_create
 }
--
2.34.1

I assume that both situations are equally problematic, and not just veth?

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH net 10/17] selftests: forwarding: ethtool_mm: Skip when using veth pairs
  2023-08-02 10:52   ` Vladimir Oltean
@ 2023-08-02 12:27     ` Petr Machata
  2023-08-02 13:30       ` Ido Schimmel
  0 siblings, 1 reply; 26+ messages in thread
From: Petr Machata @ 2023-08-02 12:27 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Ido Schimmel, netdev, davem, kuba, pabeni, edumazet, petrm, razor,
	mirsad.todorovac


Vladimir Oltean <vladimir.oltean@nxp.com> writes:

> Hi Ido,
>
> On Wed, Aug 02, 2023 at 10:51:11AM +0300, Ido Schimmel wrote:
>> MAC Merge cannot be tested with veth pairs, resulting in failures:
>> 
>>  # ./ethtool_mm.sh
>>  [...]
>>  TEST: Manual configuration with verification: swp1 to swp2          [FAIL]
>>          Verification did not succeed
>> 
>> Fix by skipping the test when used with veth pairs.
>> 
>> Fixes: e6991384ace5 ("selftests: forwarding: add a test for MAC Merge layer")
>> Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
>> Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
>> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
>> Reviewed-by: Petr Machata <petrm@nvidia.com>
>> Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
>> ---
>
> That will skip the selftest just for veth pairs. This will skip it for
> any device that doesn't support the MAC Merge layer:
>
> diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> index c580ad623848..5432848a3c59 100755
> --- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> +++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> @@ -224,6 +224,8 @@ h1_create()
>  		hw 1
>
>  	ethtool --set-mm $h1 pmac-enabled on tx-enabled off verify-enabled off
> +
> +	h1_created=yes
>  }
>
>  h2_create()
> @@ -236,10 +238,16 @@ h2_create()
>  		queues 1@0 1@1 1@2 1@3 \
>  		fp P E E E \
>  		hw 1
> +
> +	h2_created=yes
>  }
>
>  h1_destroy()
>  {
> +	if ! [[ $h1_created = yes ]]; then
> +		return
> +	fi
> +
>  	ethtool --set-mm $h1 pmac-enabled off tx-enabled off verify-enabled off
>
>  	tc qdisc del dev $h1 root
> @@ -249,6 +257,10 @@ h1_destroy()
>
>  h2_destroy()
>  {
> +	if ! [[ $h2_created = yes ]]; then
> +		return
> +	fi
> +
>  	tc qdisc del dev $h2 root
>
>  	ethtool --set-mm $h2 pmac-enabled off tx-enabled off verify-enabled off
> @@ -266,6 +278,14 @@ setup_prepare()
>  	h1=${NETIFS[p1]}
>  	h2=${NETIFS[p2]}
>
> +	for netif in ${NETIFS[@]}; do
> +		ethtool --show-mm $netif 2>&1 &> /dev/null
> +		if [[ $? -ne 0 ]]; then
> +			echo "SKIP: $netif does not support MAC Merge"
> +			exit $ksft_skip
> +		fi
> +	done
> +

Ido, if you decide to go this route, just hoist the loop to the global
scope before registering the trap, then you don't need tho hX_created
business.

>  	h1_create
>  	h2_create
>  }


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH net 10/17] selftests: forwarding: ethtool_mm: Skip when using veth pairs
  2023-08-02 12:27     ` Petr Machata
@ 2023-08-02 13:30       ` Ido Schimmel
  2023-08-02 13:33         ` Vladimir Oltean
  2023-08-02 14:22         ` Petr Machata
  0 siblings, 2 replies; 26+ messages in thread
From: Ido Schimmel @ 2023-08-02 13:30 UTC (permalink / raw)
  To: Petr Machata, vladimir.oltean
  Cc: Ido Schimmel, netdev, davem, kuba, pabeni, edumazet, razor,
	mirsad.todorovac

On Wed, Aug 02, 2023 at 02:27:49PM +0200, Petr Machata wrote:
> 
> Vladimir Oltean <vladimir.oltean@nxp.com> writes:
> 
> > Hi Ido,
> >
> > On Wed, Aug 02, 2023 at 10:51:11AM +0300, Ido Schimmel wrote:
> >> MAC Merge cannot be tested with veth pairs, resulting in failures:
> >> 
> >>  # ./ethtool_mm.sh
> >>  [...]
> >>  TEST: Manual configuration with verification: swp1 to swp2          [FAIL]
> >>          Verification did not succeed
> >> 
> >> Fix by skipping the test when used with veth pairs.
> >> 
> >> Fixes: e6991384ace5 ("selftests: forwarding: add a test for MAC Merge layer")
> >> Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
> >> Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/
> >> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> >> Reviewed-by: Petr Machata <petrm@nvidia.com>
> >> Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
> >> ---
> >
> > That will skip the selftest just for veth pairs. This will skip it for
> > any device that doesn't support the MAC Merge layer:
> >
> > diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> > index c580ad623848..5432848a3c59 100755
> > --- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> > +++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> > @@ -224,6 +224,8 @@ h1_create()
> >  		hw 1
> >
> >  	ethtool --set-mm $h1 pmac-enabled on tx-enabled off verify-enabled off
> > +
> > +	h1_created=yes
> >  }
> >
> >  h2_create()
> > @@ -236,10 +238,16 @@ h2_create()
> >  		queues 1@0 1@1 1@2 1@3 \
> >  		fp P E E E \
> >  		hw 1
> > +
> > +	h2_created=yes
> >  }
> >
> >  h1_destroy()
> >  {
> > +	if ! [[ $h1_created = yes ]]; then
> > +		return
> > +	fi
> > +
> >  	ethtool --set-mm $h1 pmac-enabled off tx-enabled off verify-enabled off
> >
> >  	tc qdisc del dev $h1 root
> > @@ -249,6 +257,10 @@ h1_destroy()
> >
> >  h2_destroy()
> >  {
> > +	if ! [[ $h2_created = yes ]]; then
> > +		return
> > +	fi
> > +
> >  	tc qdisc del dev $h2 root
> >
> >  	ethtool --set-mm $h2 pmac-enabled off tx-enabled off verify-enabled off
> > @@ -266,6 +278,14 @@ setup_prepare()
> >  	h1=${NETIFS[p1]}
> >  	h2=${NETIFS[p2]}
> >
> > +	for netif in ${NETIFS[@]}; do
> > +		ethtool --show-mm $netif 2>&1 &> /dev/null
> > +		if [[ $? -ne 0 ]]; then
> > +			echo "SKIP: $netif does not support MAC Merge"
> > +			exit $ksft_skip
> > +		fi
> > +	done
> > +
> 
> Ido, if you decide to go this route, just hoist the loop to the global
> scope before registering the trap, then you don't need tho hX_created
> business.

I think the idea was to run this check after verifying that ethtool
supports MAC Merge in setup_prepare(). How about moving all these checks
before doing any configuration and registering a trap handler?

diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
index 4331e2161e8d..39e736f30322 100755
--- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh
+++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
@@ -258,11 +258,6 @@ h2_destroy()
 
 setup_prepare()
 {
-       check_ethtool_mm_support
-       check_tc_fp_support
-       require_command lldptool
-       bail_on_lldpad "autoconfigure the MAC Merge layer" "configure it manually"
-
        h1=${NETIFS[p1]}
        h2=${NETIFS[p2]}
 
@@ -278,7 +273,18 @@ cleanup()
        h1_destroy
 }
 
-skip_on_veth
+check_ethtool_mm_support
+check_tc_fp_support
+require_command lldptool
+bail_on_lldpad "autoconfigure the MAC Merge layer" "configure it manually"
+
+for netif in ${NETIFS[@]}; do
+       ethtool --show-mm $netif 2>&1 &> /dev/null
+       if [[ $? -ne 0 ]]; then
+               echo "SKIP: $netif does not support MAC Merge"
+               exit $ksft_skip
+       fi
+done
 
 trap cleanup EXIT

> 
> >  	h1_create
> >  	h2_create
> >  }
> 
> 

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH net 10/17] selftests: forwarding: ethtool_mm: Skip when using veth pairs
  2023-08-02 13:30       ` Ido Schimmel
@ 2023-08-02 13:33         ` Vladimir Oltean
  2023-08-02 14:22         ` Petr Machata
  1 sibling, 0 replies; 26+ messages in thread
From: Vladimir Oltean @ 2023-08-02 13:33 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: Petr Machata, Ido Schimmel, netdev, davem, kuba, pabeni, edumazet,
	razor, mirsad.todorovac

On Wed, Aug 02, 2023 at 04:30:30PM +0300, Ido Schimmel wrote:
> On Wed, Aug 02, 2023 at 02:27:49PM +0200, Petr Machata wrote:
> > Ido, if you decide to go this route, just hoist the loop to the global
> > scope before registering the trap, then you don't need tho hX_created
> > business.
> 
> I think the idea was to run this check after verifying that ethtool
> supports MAC Merge in setup_prepare(). How about moving all these checks
> before doing any configuration and registering a trap handler?

That should work.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH net 10/17] selftests: forwarding: ethtool_mm: Skip when using veth pairs
  2023-08-02 13:30       ` Ido Schimmel
  2023-08-02 13:33         ` Vladimir Oltean
@ 2023-08-02 14:22         ` Petr Machata
  1 sibling, 0 replies; 26+ messages in thread
From: Petr Machata @ 2023-08-02 14:22 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: Petr Machata, vladimir.oltean, Ido Schimmel, netdev, davem, kuba,
	pabeni, edumazet, razor, mirsad.todorovac


Ido Schimmel <idosch@idosch.org> writes:

> On Wed, Aug 02, 2023 at 02:27:49PM +0200, Petr Machata wrote:
>> 
>> Vladimir Oltean <vladimir.oltean@nxp.com> writes:
>> 
>> > @@ -266,6 +278,14 @@ setup_prepare()
>> >  	h1=${NETIFS[p1]}
>> >  	h2=${NETIFS[p2]}
>> >
>> > +	for netif in ${NETIFS[@]}; do
>> > +		ethtool --show-mm $netif 2>&1 &> /dev/null
>> > +		if [[ $? -ne 0 ]]; then
>> > +			echo "SKIP: $netif does not support MAC Merge"
>> > +			exit $ksft_skip
>> > +		fi
>> > +	done
>> > +
>> 
>> Ido, if you decide to go this route, just hoist the loop to the global
>> scope before registering the trap, then you don't need the hX_created
>> business.
>
> I think the idea was to run this check after verifying that ethtool
> supports MAC Merge in setup_prepare(). How about moving all these checks

True, I missed that.

> before doing any configuration and registering a trap handler?
>
> diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> index 4331e2161e8d..39e736f30322 100755
> --- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> +++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> @@ -258,11 +258,6 @@ h2_destroy()
>  
>  setup_prepare()
>  {
> -       check_ethtool_mm_support
> -       check_tc_fp_support
> -       require_command lldptool
> -       bail_on_lldpad "autoconfigure the MAC Merge layer" "configure it manually"
> -
>         h1=${NETIFS[p1]}
>         h2=${NETIFS[p2]}
>  
> @@ -278,7 +273,18 @@ cleanup()
>         h1_destroy
>  }
>  
> -skip_on_veth
> +check_ethtool_mm_support
> +check_tc_fp_support
> +require_command lldptool
> +bail_on_lldpad "autoconfigure the MAC Merge layer" "configure it manually"
> +
> +for netif in ${NETIFS[@]}; do
> +       ethtool --show-mm $netif 2>&1 &> /dev/null
> +       if [[ $? -ne 0 ]]; then
> +               echo "SKIP: $netif does not support MAC Merge"
> +               exit $ksft_skip
> +       fi
> +done
>  
>  trap cleanup EXIT
>

Looks good. These checks are usually placed right after sourcing the
libraries, but I don't care much one way or another.

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2023-08-02 14:27 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-02  7:51 [PATCH net 00/17] selftests: forwarding: Various fixes Ido Schimmel
2023-08-02  7:51 ` [PATCH net 01/17] selftests: forwarding: Skip test when no interfaces are specified Ido Schimmel
2023-08-02  7:51 ` [PATCH net 02/17] selftests: forwarding: Switch off timeout Ido Schimmel
2023-08-02  7:51 ` [PATCH net 03/17] selftests: forwarding: bridge_mdb: Check iproute2 version Ido Schimmel
2023-08-02  7:51 ` [PATCH net 04/17] selftests: forwarding: bridge_mdb_max: " Ido Schimmel
2023-08-02  7:51 ` [PATCH net 05/17] selftests: forwarding: Set default IPv6 traceroute utility Ido Schimmel
2023-08-02  7:51 ` [PATCH net 06/17] selftests: forwarding: Add a helper to skip test when using veth pairs Ido Schimmel
2023-08-02  7:51 ` [PATCH net 07/17] selftests: forwarding: ethtool: Skip " Ido Schimmel
2023-08-02  7:51 ` [PATCH net 08/17] selftests: forwarding: ethtool_extended_state: " Ido Schimmel
2023-08-02  7:51 ` [PATCH net 09/17] selftests: forwarding: hw_stats_l3_gre: " Ido Schimmel
2023-08-02  7:51 ` [PATCH net 10/17] selftests: forwarding: ethtool_mm: " Ido Schimmel
2023-08-02 10:52   ` Vladimir Oltean
2023-08-02 12:27     ` Petr Machata
2023-08-02 13:30       ` Ido Schimmel
2023-08-02 13:33         ` Vladimir Oltean
2023-08-02 14:22         ` Petr Machata
2023-08-02  7:51 ` [PATCH net 11/17] selftests: forwarding: tc_actions: Use ncat instead of nc Ido Schimmel
2023-08-02  7:51 ` [PATCH net 12/17] selftests: forwarding: tc_flower: Relax success criterion Ido Schimmel
2023-08-02  7:51 ` [PATCH net 13/17] selftests: forwarding: tc_tunnel_key: Make filters more specific Ido Schimmel
2023-08-02  8:30   ` Davide Caratti
2023-08-02  8:37     ` Ido Schimmel
2023-08-02  8:52       ` Davide Caratti
2023-08-02  7:51 ` [PATCH net 14/17] selftests: forwarding: tc_flower_l2_miss: Fix failing test with old libnet Ido Schimmel
2023-08-02  7:51 ` [PATCH net 15/17] selftests: forwarding: bridge_mdb: " Ido Schimmel
2023-08-02  7:51 ` [PATCH net 16/17] selftests: forwarding: bridge_mdb_max: " Ido Schimmel
2023-08-02  7:51 ` [PATCH net 17/17] selftests: forwarding: bridge_mdb: Make test more robust Ido Schimmel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).