selftests: mlxsw: Add few fixes for sharedbuffer test

PATCH Applied Petr Machata 2 participants 5 messages Dec 5, 2024 – Dec 7, 2024

Subsystems (4)

Networking Network drivers Kselftest Mellanox Ethernet switch drivers

lore

Share & tools

lore

Petr Machata

author maintainer

2024-12-05 16:38 UTC #0 lore raw

cover selftests: mlxsw: Add few fixes for sharedbuffer test

Danielle Ratson writes:

Currently, the sharedbuffer test fails sometimes because it is reading a maximum occupancy that is larger than expected on some different cases.

This is happening because the test assumes that the packet it is sending is the only packet being passed to the device.

In addition, some duplications on one hand, and redundant test cases on the other hand, were found in the test.

Add egress filters on h1 and h2 that will guarantee that the packets in the buffer are sent in the test, and remove the redundant test cases.

Danielle Ratson (3):
selftests: mlxsw: sharedbuffer: Remove h1 ingress test case
selftests: mlxsw: sharedbuffer: Remove duplicate test cases
selftests: mlxsw: sharedbuffer: Ensure no extra packets are counted

 .../drivers/net/mlxsw/sharedbuffer.sh         | 55 ++++++++++++++-----
 1 file changed, 40 insertions(+), 15 deletions(-)

Signature · 1 line

2.47.0

Petr Machata

author maintainer

2024-12-05 16:38 UTC #1 lore raw

PATCH 1/3 selftests: mlxsw: sharedbuffer: Remove h1 ingress test case

From: Danielle Ratson <[email protected]>

The test is sending only one packet generated with mausezahn from $h1 to $h2. However, for some reason, it is testing for non-zero maximum occupancy in both the ingress pool of $h1 and $h2. The former only passes when $h2 happens to send a packet.

Avoid intermittent failures by removing unintentional test case regarding the ingress pool of $h1.

Fixes a865ad999603 ("selftests: mlxsw: Add shared buffer traffic test")

Signed-off-by Danielle Ratson <[email protected]>

Reviewed-by Ido Schimmel <[email protected]>

Signed-off-by Ido Schimmel <[email protected]>

Signed-off-by Petr Machata <[email protected]>

---
 tools/testing/selftests/drivers/net/mlxsw/sharedbuffer.sh | 5 -----
 1 file changed, 5 deletions(-)

tools/testing/selftests/drivers/net/mlxsw/sharedbuffer.sh +0 -5

index 0c47faff9274..a7b3d6cf3185 100755
@@ -108,11 +108,6 @@ port_pool_test()
 
 	devlink sb occupancy snapshot $DEVLINK_DEV
 
-	RET=0
-	max_occ=$(sb_occ_pool_check $dl_port1 $SB_POOL_ING $exp_max_occ)
-	check_err $? "Expected iPool($SB_POOL_ING) max occupancy to be $exp_max_occ, but got $max_occ"
-	log_test "physical port's($h1) ingress pool"
-
 	RET=0
 	max_occ=$(sb_occ_pool_check $dl_port2 $SB_POOL_ING $exp_max_occ)
 	check_err $? "Expected iPool($SB_POOL_ING) max occupancy to be $exp_max_occ, but got $max_occ"

Signature · 1 line

2.47.0

Petr Machata

author maintainer

2024-12-05 16:39 UTC #2 lore raw

PATCH 2/3 selftests: mlxsw: sharedbuffer: Remove duplicate test cases

In reply to Petr Machata

From: Danielle Ratson <[email protected]>

On both port_tc_ip_test() and port_tc_arp_test(), the max occupancy is checked on $h2 twice, when only the error message is different and does not match the check itself.

Remove the two duplicated test cases from the test.

Fixes a865ad999603 ("selftests: mlxsw: Add shared buffer traffic test")

Signed-off-by Danielle Ratson <[email protected]>

Reviewed-by Ido Schimmel <[email protected]>

Signed-off-by Ido Schimmel <[email protected]>

Signed-off-by Petr Machata <[email protected]>

---
 .../selftests/drivers/net/mlxsw/sharedbuffer.sh        | 10 ----------
 1 file changed, 10 deletions(-)

tools/testing/selftests/drivers/net/mlxsw/sharedbuffer.sh +0 ~~-10~~

index a7b3d6cf3185..21bebc5726f6 100755
@@ -131,11 +131,6 @@ port_tc_ip_test()
 
 	devlink sb occupancy snapshot $DEVLINK_DEV
 
-	RET=0
-	max_occ=$(sb_occ_itc_check $dl_port2 $SB_ITC $exp_max_occ)
-	check_err $? "Expected ingress TC($SB_ITC) max occupancy to be $exp_max_occ, but got $max_occ"
-	log_test "physical port's($h1) ingress TC - IP packet"
-
 	RET=0
 	max_occ=$(sb_occ_itc_check $dl_port2 $SB_ITC $exp_max_occ)
 	check_err $? "Expected ingress TC($SB_ITC) max occupancy to be $exp_max_occ, but got $max_occ"

@@ -158,11 +153,6 @@ port_tc_arp_test()
 
 	devlink sb occupancy snapshot $DEVLINK_DEV
 
-	RET=0
-	max_occ=$(sb_occ_itc_check $dl_port2 $SB_ITC $exp_max_occ)
-	check_err $? "Expected ingress TC($SB_ITC) max occupancy to be $exp_max_occ, but got $max_occ"
-	log_test "physical port's($h1) ingress TC - ARP packet"
-
 	RET=0
 	max_occ=$(sb_occ_itc_check $dl_port2 $SB_ITC $exp_max_occ)
 	check_err $? "Expected ingress TC($SB_ITC) max occupancy to be $exp_max_occ, but got $max_occ"

Signature · 1 line

2.47.0

Petr Machata

author maintainer

2024-12-05 16:39 UTC #3 lore raw

PATCH 3/3 selftests: mlxsw: sharedbuffer: Ensure no extra packets are counted

In reply to Petr Machata

From: Danielle Ratson <[email protected]>

The test assumes that the packet it is sending is the only packet being passed to the device.

However, it is not the case and so other packets are filling the buffers as well. Therefore, the test sometimes fails because it is reading a maximum occupancy that is larger than expected.

Add egress filters on $h1 and $h2 that will guarantee the above.

Fixes a865ad999603 ("selftests: mlxsw: Add shared buffer traffic test")

Signed-off-by Danielle Ratson <[email protected]>

Reviewed-by Ido Schimmel <[email protected]>

Signed-off-by Ido Schimmel <[email protected]>

Signed-off-by Petr Machata <[email protected]>

---
 .../drivers/net/mlxsw/sharedbuffer.sh         | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)

tools/testing/selftests/drivers/net/mlxsw/sharedbuffer.sh +40 -0

index 21bebc5726f6..c068e6c2a580 100755
@@ -22,20 +22,34 @@ SB_ITC=0
 h1_create()
 {
 	simple_if_init $h1 192.0.1.1/24
+	tc qdisc add dev $h1 clsact
+
+	# Add egress filter on $h1 that will guarantee that the packet sent,
+	# will be the only packet being passed to the device.
+	tc filter add dev $h1 egress pref 2 handle 102 matchall action drop
 }
 
 h1_destroy()
 {
+	tc filter del dev $h1 egress pref 2 handle 102 matchall action drop
+	tc qdisc del dev $h1 clsact
 	simple_if_fini $h1 192.0.1.1/24
 }
 
 h2_create()
 {
 	simple_if_init $h2 192.0.1.2/24
+	tc qdisc add dev $h2 clsact
+
+	# Add egress filter on $h2 that will guarantee that the packet sent,
+	# will be the only packet being passed to the device.
+	tc filter add dev $h2 egress pref 1 handle 101 matchall action drop
 }
 
 h2_destroy()
 {
+	tc filter del dev $h2 egress pref 1 handle 101 matchall action drop
+	tc qdisc del dev $h2 clsact
 	simple_if_fini $h2 192.0.1.2/24
 }

@@ -101,6 +115,11 @@ port_pool_test()
 	local exp_max_occ=$(devlink_cell_size_get)
 	local max_occ
 
+	tc filter add dev $h1 egress protocol ip pref 1 handle 101 flower \
+		src_mac $h1mac dst_mac $h2mac \
+		src_ip 192.0.1.1 dst_ip 192.0.1.2 \
+		action pass
+
 	devlink sb occupancy clearmax $DEVLINK_DEV
 
 	$MZ $h1 -c 1 -p 10 -a $h1mac -b $h2mac -A 192.0.1.1 -B 192.0.1.2 \

@@ -117,6 +136,11 @@ port_pool_test()
 	max_occ=$(sb_occ_pool_check $cpu_dl_port $SB_POOL_EGR_CPU $exp_max_occ)
 	check_err $? "Expected ePool($SB_POOL_EGR_CPU) max occupancy to be $exp_max_occ, but got $max_occ"
 	log_test "CPU port's egress pool"
+
+	tc filter del dev $h1 egress protocol ip pref 1 handle 101 flower \
+		src_mac $h1mac dst_mac $h2mac \
+		src_ip 192.0.1.1 dst_ip 192.0.1.2 \
+		action pass
 }
 
 port_tc_ip_test()

@@ -124,6 +148,11 @@ port_tc_ip_test()
 	local exp_max_occ=$(devlink_cell_size_get)
 	local max_occ
 
+	tc filter add dev $h1 egress protocol ip pref 1 handle 101 flower \
+		src_mac $h1mac dst_mac $h2mac \
+		src_ip 192.0.1.1 dst_ip 192.0.1.2 \
+		action pass
+
 	devlink sb occupancy clearmax $DEVLINK_DEV
 
 	$MZ $h1 -c 1 -p 10 -a $h1mac -b $h2mac -A 192.0.1.1 -B 192.0.1.2 \

@@ -140,6 +169,11 @@ port_tc_ip_test()
 	max_occ=$(sb_occ_etc_check $cpu_dl_port $SB_ITC_CPU_IP $exp_max_occ)
 	check_err $? "Expected egress TC($SB_ITC_CPU_IP) max occupancy to be $exp_max_occ, but got $max_occ"
 	log_test "CPU port's egress TC - IP packet"
+
+	tc filter del dev $h1 egress protocol ip pref 1 handle 101 flower \
+		src_mac $h1mac dst_mac $h2mac \
+		src_ip 192.0.1.1 dst_ip 192.0.1.2 \
+		action pass
 }
 
 port_tc_arp_test()

@@ -147,6 +181,9 @@ port_tc_arp_test()
 	local exp_max_occ=$(devlink_cell_size_get)
 	local max_occ
 
+	tc filter add dev $h1 egress protocol arp pref 1 handle 101 flower \
+		src_mac $h1mac action pass
+
 	devlink sb occupancy clearmax $DEVLINK_DEV
 
 	$MZ $h1 -c 1 -p 10 -a $h1mac -A 192.0.1.1 -t arp -q

@@ -162,6 +199,9 @@ port_tc_arp_test()
 	max_occ=$(sb_occ_etc_check $cpu_dl_port $SB_ITC_CPU_ARP $exp_max_occ)
 	check_err $? "Expected egress TC($SB_ITC_IP2ME) max occupancy to be $exp_max_occ, but got $max_occ"
 	log_test "CPU port's egress TC - ARP packet"
+
+	tc filter del dev $h1 egress protocol arp pref 1 handle 101 flower \
+		src_mac $h1mac action pass
 }
 
 setup_prepare()

Signature · 1 line

2.47.0

[email protected]

CI Applied

2024-12-07 01:40 UTC #4 lore raw

In reply to Petr Machata

This series was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

Quoted 9 lines · On Thu, 5 Dec 2024 17:35:58 +0100 you wrote:

Danielle Ratson writes:
Currently, the sharedbuffer test fails sometimes because it is reading a maximum occupancy that is larger than expected on some different cases.
This is happening because the test assumes that the packet it is sending is the only packet being passed to the device.
[...]

Here is the summary with links:
- [net,1/3] selftests: mlxsw: sharedbuffer: Remove h1 ingress test case

    https://git.kernel.org/netdev/net/c/cf3515c55690
  - [net,2/3] selftests: mlxsw: sharedbuffer: Remove duplicate test cases
    https://git.kernel.org/netdev/net/c/6c46ad4d1bb2
  - [net,3/3] selftests: mlxsw: sharedbuffer: Ensure no extra packets are counted
    https://git.kernel.org/netdev/net/c/5f2c7ab15fd8

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html