[PATCH net-next 0/4] selftests: bonding: use busy/slowwait when waiting

[PATCH net-next 0/4] selftests: bonding: use busy/slowwait when waiting

[Hangbin Liu]

There are a lot waitings in bonding tests use sleep. Let's replace them with
busywait or slowwait(added in the first patch). This could save much test
time. e.g.

bond-break-lacpdu-tx.sh
  before: 0m16.346s
  after: 0m2.424s

bond_options.sh
  before: 9m25.299s
  after: 5m27.439s

bond-lladdr-target.sh
  before: 0m7.090s
  after: 0m6.148s

bond_macvlan.sh
  before: 0m44.999s
  after: 0m23.468s

In total, we could save about 270 seconds.

Hangbin Liu (4):
  selftests/net/forwarding: add slowwait functions
  selftests: bonding: use tc filter to check if LACP was sent
  selftests: bonding: reduce garp_test/arp_validate test time
  selftests: bonding: use busy/slowwait instead of hard code sleep

 .../net/bonding/bond-break-lacpdu-tx.sh       | 18 +++++-----
 .../drivers/net/bonding/bond-lladdr-target.sh | 21 +++++++++--
 .../drivers/net/bonding/bond_macvlan.sh       |  5 ++-
 .../drivers/net/bonding/bond_options.sh       | 22 +++++++++---
 .../drivers/net/bonding/bond_topo_2d1c.sh     |  6 ++--
 tools/testing/selftests/net/forwarding/lib.sh | 36 +++++++++++++++++++
 6 files changed, 85 insertions(+), 23 deletions(-)

-- 
2.43.0

[PATCH net-next 1/4] selftests/net/forwarding: add slowwait functions

[Hangbin Liu]

Add slowwait functions to wait for some operations that may need a long time
to finish. The busywait executes the cmd too fast, which is kind of wasting
cpu in this scenario. At the same time, if shell debugging is enabled with
`set -x`. the busywait will output too much logs.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 tools/testing/selftests/net/forwarding/lib.sh | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 8a61464ab6eb..07faedc2071b 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -41,6 +41,7 @@ fi
 # Kselftest framework requirement - SKIP code is 4.
 ksft_skip=4
 
+# timeout in milliseconds
 busywait()
 {
 	local timeout=$1; shift
@@ -64,6 +65,32 @@ busywait()
 	done
 }
 
+# timeout in seconds
+slowwait()
+{
+	local timeout=$1; shift
+
+	local start_time="$(date -u +%s)"
+	while true
+	do
+		local out
+		out=$("$@")
+		local ret=$?
+		if ((!ret)); then
+			echo -n "$out"
+			return 0
+		fi
+
+		local current_time="$(date -u +%s)"
+		if ((current_time - start_time > timeout)); then
+			echo -n "$out"
+			return 1
+		fi
+
+		sleep 1
+	done
+}
+
 ##############################################################################
 # Sanity checks
 
@@ -505,6 +532,15 @@ busywait_for_counter()
 	busywait "$timeout" until_counter_is ">= $((base + delta))" "$@"
 }
 
+slowwait_for_counter()
+{
+	local timeout=$1; shift
+	local delta=$1; shift
+
+	local base=$("$@")
+	slowwait "$timeout" until_counter_is ">= $((base + delta))" "$@"
+}
+
 setup_wait_dev()
 {
 	local dev=$1; shift
-- 
2.43.0

[PATCH net-next 2/4] selftests: bonding: use tc filter to check if LACP was sent

[Hangbin Liu]

Use tc filter to check if LACP was sent, which is accurate and save
more time.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 .../net/bonding/bond-break-lacpdu-tx.sh        | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/bonding/bond-break-lacpdu-tx.sh b/tools/testing/selftests/drivers/net/bonding/bond-break-lacpdu-tx.sh
index 6358df5752f9..5087291d15ce 100755
--- a/tools/testing/selftests/drivers/net/bonding/bond-break-lacpdu-tx.sh
+++ b/tools/testing/selftests/drivers/net/bonding/bond-break-lacpdu-tx.sh
@@ -20,21 +20,22 @@
 #    +------+ +------+
 #
 # We use veths instead of physical interfaces
+REQUIRE_MZ=no
+NUM_NETIFS=0
+lib_dir=$(dirname "$0")
+source "$lib_dir"/net_forwarding_lib.sh
 
 set -e
-tmp=$(mktemp -q dump.XXXXXX)
 cleanup() {
 	ip link del fab-br0 >/dev/null 2>&1 || :
 	ip link del fbond  >/dev/null 2>&1 || :
 	ip link del veth1-bond  >/dev/null 2>&1 || :
 	ip link del veth2-bond  >/dev/null 2>&1 || :
 	modprobe -r bonding  >/dev/null 2>&1 || :
-	rm -f -- ${tmp}
 }
 
 trap cleanup 0 1 2
 cleanup
-sleep 1
 
 # create the bridge
 ip link add fab-br0 address 52:54:00:3B:7C:A6 mtu 1500 type bridge \
@@ -67,13 +68,12 @@ ip link set fab-br0 up
 ip link set fbond up
 ip addr add dev fab-br0 10.0.0.3
 
-tcpdump -n -i veth1-end -e ether proto 0x8809 >${tmp} 2>&1 &
-sleep 15
-pkill tcpdump >/dev/null 2>&1
 rc=0
-num=$(grep "packets captured" ${tmp} | awk '{print $1}')
-if test "$num" -gt 0; then
-	echo "PASS, captured ${num}"
+tc qdisc add dev veth1-end clsact
+tc filter add dev veth1-end ingress protocol 0x8809 pref 1 handle 101 flower skip_hw action pass
+if slowwait_for_counter 15 2 \
+	tc_rule_handle_stats_get "dev veth1-end ingress" 101 ".packets" "" &> /dev/null; then
+	echo "PASS, captured 2"
 else
 	echo "FAIL"
 	rc=1
-- 
2.43.0

[PATCH net-next 3/4] selftests: bonding: reduce garp_test/arp_validate test time

[Hangbin Liu]

The purpose of grat_arp is testing commit 9949e2efb54e ("bonding: fix
send_peer_notif overflow"). As the send_peer_notif was defined to u8,
to overflow it, we need to

send_peer_notif = num_peer_notif * peer_notif_delay = num_grat_arp * peer_notify_delay / miimon > 255
  (kernel)           (kernel parameter)                   (user parameter)

e.g. 30 (num_grat_arp) * 1000 (peer_notify_delay) / 100 (miimon) > 255.

Which need 30s to complete sending garp messages. To save the testing time,
the only way is reduce the miimon number. Something like
30 (num_grat_arp) * 500 (peer_notify_delay) / 50 (miimon) > 255.

To save more time, the 50 num_grat_arp testing could be removed.

The arp_validate_test also need to check the mii_status, which sleep
too long. Use slowwait to save some time.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 .../drivers/net/bonding/bond_options.sh       | 22 ++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/bonding/bond_options.sh b/tools/testing/selftests/drivers/net/bonding/bond_options.sh
index c54d1697f439..648006763b1b 100755
--- a/tools/testing/selftests/drivers/net/bonding/bond_options.sh
+++ b/tools/testing/selftests/drivers/net/bonding/bond_options.sh
@@ -199,6 +199,15 @@ prio()
 	done
 }
 
+wait_mii_up()
+{
+	for i in $(seq 0 2); do
+		mii_status=$(cmd_jq "ip -n ${s_ns} -j -d link show eth$i" ".[].linkinfo.info_slave_data.mii_status")
+		[ ${mii_status} != "UP" ] && return 1
+	done
+	return 0
+}
+
 arp_validate_test()
 {
 	local param="$1"
@@ -211,7 +220,7 @@ arp_validate_test()
 	[ $RET -ne 0 ] && log_test "arp_validate" "$retmsg"
 
 	# wait for a while to make sure the mii status stable
-	sleep 5
+	slowwait 5 wait_mii_up
 	for i in $(seq 0 2); do
 		mii_status=$(cmd_jq "ip -n ${s_ns} -j -d link show eth$i" ".[].linkinfo.info_slave_data.mii_status")
 		if [ ${mii_status} != "UP" ]; then
@@ -276,10 +285,13 @@ garp_test()
 	active_slave=$(cmd_jq "ip -n ${s_ns} -d -j link show bond0" ".[].linkinfo.info_data.active_slave")
 	ip -n ${s_ns} link set ${active_slave} down
 
-	exp_num=$(echo "${param}" | cut -f6 -d ' ')
-	sleep $((exp_num + 2))
+	# wait for active link change
+	sleep 1
 
+	exp_num=$(echo "${param}" | cut -f6 -d ' ')
 	active_slave=$(cmd_jq "ip -n ${s_ns} -d -j link show bond0" ".[].linkinfo.info_data.active_slave")
+	slowwait_for_counter $((exp_num + 5)) $exp_num \
+		tc_rule_handle_stats_get "dev s${active_slave#eth} ingress" 101 ".packets" "-n ${g_ns}"
 
 	# check result
 	real_num=$(tc_rule_handle_stats_get "dev s${active_slave#eth} ingress" 101 ".packets" "-n ${g_ns}")
@@ -296,8 +308,8 @@ garp_test()
 num_grat_arp()
 {
 	local val
-	for val in 10 20 30 50; do
-		garp_test "mode active-backup miimon 100 num_grat_arp $val peer_notify_delay 1000"
+	for val in 10 20 30; do
+		garp_test "mode active-backup miimon 50 num_grat_arp $val peer_notify_delay 500"
 		log_test "num_grat_arp" "active-backup miimon num_grat_arp $val"
 	done
 }
-- 
2.43.0

[PATCH net-next 4/4] selftests: bonding: use busy/slowwait instead of hard code sleep

[Hangbin Liu]

Use busywait or slowwait instead of hard code sleep. When using busywait
to check the link connection, I will set ping timeout to 0.1, which
could make busywait not too busy.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 .../drivers/net/bonding/bond-lladdr-target.sh | 21 ++++++++++++++++---
 .../drivers/net/bonding/bond_macvlan.sh       |  5 ++---
 .../drivers/net/bonding/bond_topo_2d1c.sh     |  6 +++---
 3 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/bonding/bond-lladdr-target.sh b/tools/testing/selftests/drivers/net/bonding/bond-lladdr-target.sh
index 89af402fabbe..b7b60e767daa 100755
--- a/tools/testing/selftests/drivers/net/bonding/bond-lladdr-target.sh
+++ b/tools/testing/selftests/drivers/net/bonding/bond-lladdr-target.sh
@@ -17,6 +17,11 @@
 #  +----------------+
 #
 # We use veths instead of physical interfaces
+REQUIRE_MZ=no
+NUM_NETIFS=0
+lib_dir=$(dirname "$0")
+source "$lib_dir"/net_forwarding_lib.sh
+
 sw="sw-$(mktemp -u XXXXXX)"
 host="ns-$(mktemp -u XXXXXX)"
 
@@ -26,6 +31,16 @@ cleanup()
 	ip netns del $host
 }
 
+wait_lladdr_dad()
+{
+	$@ | grep fe80 | grep -qv tentative
+}
+
+wait_bond_up()
+{
+	$@ | grep -q 'state UP'
+}
+
 trap cleanup 0 1 2
 
 ip netns add $sw
@@ -37,8 +52,8 @@ ip -n $host link add veth1 type veth peer name veth1 netns $sw
 ip -n $sw link add br0 type bridge
 ip -n $sw link set br0 up
 sw_lladdr=$(ip -n $sw addr show br0 | awk '/fe80/{print $2}' | cut -d'/' -f1)
-# sleep some time to make sure bridge lladdr pass DAD
-sleep 2
+# wait some time to make sure bridge lladdr pass DAD
+slowwait 2 wait_lladdr_dad ip -n $sw addr show br0
 
 ip -n $host link add bond0 type bond mode 1 ns_ip6_target ${sw_lladdr} \
 	arp_validate 3 arp_interval 1000
@@ -53,7 +68,7 @@ ip -n $sw link set veth1 master br0
 ip -n $sw link set veth0 up
 ip -n $sw link set veth1 up
 
-sleep 5
+slowwait 5 wait_bond_up ip -n $host link show bond0
 
 rc=0
 if ip -n $host link show bond0 | grep -q LOWER_UP; then
diff --git a/tools/testing/selftests/drivers/net/bonding/bond_macvlan.sh b/tools/testing/selftests/drivers/net/bonding/bond_macvlan.sh
index b609fb6231f4..4fddb28a0715 100755
--- a/tools/testing/selftests/drivers/net/bonding/bond_macvlan.sh
+++ b/tools/testing/selftests/drivers/net/bonding/bond_macvlan.sh
@@ -58,7 +58,7 @@ macvlan_over_bond()
 	ip -n ${m2_ns} addr add ${m2_ip4}/24 dev macv0
 	ip -n ${m2_ns} addr add ${m2_ip6}/24 dev macv0
 
-	sleep 2
+	busywait 2000 ip netns exec ${c_ns} ping ${s_ip4} -c 1 -W 0.1 &> /dev/null
 
 	check_connection "${c_ns}" "${s_ip4}" "IPv4: client->server"
 	check_connection "${c_ns}" "${s_ip6}" "IPv6: client->server"
@@ -69,8 +69,7 @@ macvlan_over_bond()
 	check_connection "${m1_ns}" "${m2_ip4}" "IPv4: macvlan_1->macvlan_2"
 	check_connection "${m1_ns}" "${m2_ip6}" "IPv6: macvlan_1->macvlan_2"
 
-
-	sleep 5
+	busywait 5000 ip netns exec ${s_ns} ping ${c_ip4} -c 1 -W 0.1 &> /dev/null
 
 	check_connection "${s_ns}" "${c_ip4}" "IPv4: server->client"
 	check_connection "${s_ns}" "${c_ip6}" "IPv6: server->client"
diff --git a/tools/testing/selftests/drivers/net/bonding/bond_topo_2d1c.sh b/tools/testing/selftests/drivers/net/bonding/bond_topo_2d1c.sh
index a509ef949dcf..7c3f15bc6a9b 100644
--- a/tools/testing/selftests/drivers/net/bonding/bond_topo_2d1c.sh
+++ b/tools/testing/selftests/drivers/net/bonding/bond_topo_2d1c.sh
@@ -73,7 +73,6 @@ server_create()
 	ip -n ${s_ns} link set bond0 up
 	ip -n ${s_ns} addr add ${s_ip4}/24 dev bond0
 	ip -n ${s_ns} addr add ${s_ip6}/24 dev bond0
-	sleep 2
 }
 
 # Reset bond with new mode and options
@@ -96,7 +95,8 @@ bond_reset()
 	ip -n ${s_ns} link set bond0 up
 	ip -n ${s_ns} addr add ${s_ip4}/24 dev bond0
 	ip -n ${s_ns} addr add ${s_ip6}/24 dev bond0
-	sleep 2
+	# Wait for IPv6 address ready as it needs DAD
+	busywait 5000 ip netns exec ${s_ns} ping ${c_ip6} -c 1 -W 0.1 &> /dev/null
 }
 
 server_destroy()
@@ -150,7 +150,7 @@ bond_check_connection()
 {
 	local msg=${1:-"check connection"}
 
-	sleep 2
+	busywait 2000 ip netns exec ${s_ns} ping ${c_ip4} -c 1 -W 0.1 &> /dev/null
 	ip netns exec ${s_ns} ping ${c_ip4} -c5 -i 0.1 &>/dev/null
 	check_err $? "${msg}: ping failed"
 	ip netns exec ${s_ns} ping6 ${c_ip6} -c5 -i 0.1 &>/dev/null
-- 
2.43.0

Re: [PATCH net-next 1/4] selftests/net/forwarding: add slowwait functions

[Przemek Kitszel]

On 1/24/24 10:58, Hangbin Liu wrote:
> Add slowwait functions to wait for some operations that may need a long time
> to finish. The busywait executes the cmd too fast, which is kind of wasting
> cpu in this scenario. At the same time, if shell debugging is enabled with
> `set -x`. the busywait will output too much logs.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>   tools/testing/selftests/net/forwarding/lib.sh | 36 +++++++++++++++++++
>   1 file changed, 36 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index 8a61464ab6eb..07faedc2071b 100644
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -41,6 +41,7 @@ fi
>   # Kselftest framework requirement - SKIP code is 4.
>   ksft_skip=4
>   
> +# timeout in milliseconds
>   busywait()
>   {
>   	local timeout=$1; shift
> @@ -64,6 +65,32 @@ busywait()
>   	done
>   }
>   
> +# timeout in seconds
> +slowwait()
> +{
> +	local timeout=$1; shift
> +
> +	local start_time="$(date -u +%s)"
> +	while true
> +	do
> +		local out
> +		out=$("$@")
> +		local ret=$?
> +		if ((!ret)); then

it would be nice to have some exit code used (or just reserved) for
"operation failed, no need to wait, fail the test please"
similar to the xargs, eg:
               126    if the command cannot be run

> +			echo -n "$out"
> +			return 0
> +		fi
> +
> +		local current_time="$(date -u +%s)"
> +		if ((current_time - start_time > timeout)); then
> +			echo -n "$out"
> +			return 1
> +		fi
> +
> +		sleep 1

I see that `sleep 1` is simplest correct impl, but it's tempting to
suggest exponential back-off, perhaps with saturation at 15

(but then you will have to cap last sleep to don't exceed timeout by
more than 1).

> +	done
> +}
> +
>   ##############################################################################
>   # Sanity checks
>   
> @@ -505,6 +532,15 @@ busywait_for_counter()
>   	busywait "$timeout" until_counter_is ">= $((base + delta))" "$@"
>   }
>   
> +slowwait_for_counter()
> +{
> +	local timeout=$1; shift
> +	local delta=$1; shift
> +
> +	local base=$("$@")
> +	slowwait "$timeout" until_counter_is ">= $((base + delta))" "$@"
> +}
> +
>   setup_wait_dev()
>   {
>   	local dev=$1; shift

just nitpicks so I will provide my RB in case you want to ignore ;)

Re: [PATCH net-next 0/4] selftests: bonding: use busy/slowwait when waiting

[Przemek Kitszel]

On 1/24/24 10:58, Hangbin Liu wrote:
> There are a lot waitings in bonding tests use sleep. Let's replace them with
> busywait or slowwait(added in the first patch). This could save much test
> time. e.g.
> 
> bond-break-lacpdu-tx.sh
>    before: 0m16.346s
>    after: 0m2.424s
> 
> bond_options.sh
>    before: 9m25.299s
>    after: 5m27.439s
> 
> bond-lladdr-target.sh
>    before: 0m7.090s
>    after: 0m6.148s
> 
> bond_macvlan.sh
>    before: 0m44.999s
>    after: 0m23.468s
> 
> In total, we could save about 270 seconds.
> 
> Hangbin Liu (4):
>    selftests/net/forwarding: add slowwait functions
>    selftests: bonding: use tc filter to check if LACP was sent
>    selftests: bonding: reduce garp_test/arp_validate test time
>    selftests: bonding: use busy/slowwait instead of hard code sleep
> 
>   .../net/bonding/bond-break-lacpdu-tx.sh       | 18 +++++-----
>   .../drivers/net/bonding/bond-lladdr-target.sh | 21 +++++++++--
>   .../drivers/net/bonding/bond_macvlan.sh       |  5 ++-
>   .../drivers/net/bonding/bond_options.sh       | 22 +++++++++---
>   .../drivers/net/bonding/bond_topo_2d1c.sh     |  6 ++--
>   tools/testing/selftests/net/forwarding/lib.sh | 36 +++++++++++++++++++
>   6 files changed, 85 insertions(+), 23 deletions(-)
> 

for the series:
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

Re: [PATCH net-next 1/4] selftests/net/forwarding: add slowwait functions

[Hangbin Liu]

Hi Przemek,

Thanks for your review.

On Wed, Jan 24, 2024 at 02:25:57PM +0100, Przemek Kitszel wrote:
> > +# timeout in seconds
> > +slowwait()
> > +{
> > +	local timeout=$1; shift
> > +
> > +	local start_time="$(date -u +%s)"
> > +	while true
> > +	do
> > +		local out
> > +		out=$("$@")
> > +		local ret=$?
> > +		if ((!ret)); then
> 
> it would be nice to have some exit code used (or just reserved) for
> "operation failed, no need to wait, fail the test please"
> similar to the xargs, eg:
>               126    if the command cannot be run

Return directly instead of wait may confuse the caller. Maybe we can
add a parameter and let user decide whether to wait if return some value.
e.g.

slowwait nowait 126 $timeout $commands

> 
> > +			echo -n "$out"
> > +			return 0
> > +		fi
> > +
> > +		local current_time="$(date -u +%s)"
> > +		if ((current_time - start_time > timeout)); then
> > +			echo -n "$out"
> > +			return 1
> > +		fi
> > +
> > +		sleep 1
> 
> I see that `sleep 1` is simplest correct impl, but it's tempting to
> suggest exponential back-off, perhaps with saturation at 15
> 
> (but then you will have to cap last sleep to don't exceed timeout by
> more than 1).

Do you mean sleep longer when cmd exec failed? I'm not sure if it's a good
idea as the caller still wants to return ASAP when cmd exec succeeds.

Thanks
Hangbin

Re: [PATCH net-next 1/4] selftests/net/forwarding: add slowwait functions

[Paolo Abeni]

On Fri, 2024-01-26 at 17:22 +0800, Hangbin Liu wrote:
> On Wed, Jan 24, 2024 at 02:25:57PM +0100, Przemek Kitszel wrote:
> > 
> > > +			echo -n "$out"
> > > +			return 0
> > > +		fi
> > > +
> > > +		local current_time="$(date -u +%s)"
> > > +		if ((current_time - start_time > timeout)); then
> > > +			echo -n "$out"
> > > +			return 1
> > > +		fi
> > > +
> > > +		sleep 1
> > 
> > I see that `sleep 1` is simplest correct impl, but it's tempting to
> > suggest exponential back-off, perhaps with saturation at 15
> > 
> > (but then you will have to cap last sleep to don't exceed timeout by
> > more than 1).
> 
> Do you mean sleep longer when cmd exec failed? I'm not sure if it's a good
> idea as the caller still wants to return ASAP when cmd exec succeeds.

I think exponential backoff is not needed here: we don't care about 
minimizing the CPU usage overhead, and there should not be 'collision'
issues.

Instead I *think* you could use a smaller sleep, e.g.

	sleep 0.1

and hopefully reduce the latency even further.

Cheers,

Paolo

Re: [PATCH net-next 3/4] selftests: bonding: reduce garp_test/arp_validate test time

[Paolo Abeni]

On Wed, 2024-01-24 at 17:58 +0800, Hangbin Liu wrote:
> @@ -276,10 +285,13 @@ garp_test()
>  	active_slave=$(cmd_jq "ip -n ${s_ns} -d -j link show bond0" ".[].linkinfo.info_data.active_slave")
>  	ip -n ${s_ns} link set ${active_slave} down
>  
> -	exp_num=$(echo "${param}" | cut -f6 -d ' ')
> -	sleep $((exp_num + 2))
> +	# wait for active link change
> +	sleep 1

If 'slowwait' would loop around a sub-second sleep, I guess you could
use 'slowwait' here, too.

>  
> +	exp_num=$(echo "${param}" | cut -f6 -d ' ')
>  	active_slave=$(cmd_jq "ip -n ${s_ns} -d -j link show bond0" ".[].linkinfo.info_data.active_slave")
> +	slowwait_for_counter $((exp_num + 5)) $exp_num \
> +		tc_rule_handle_stats_get "dev s${active_slave#eth} ingress" 101 ".packets" "-n ${g_ns}"
>  
>  	# check result
>  	real_num=$(tc_rule_handle_stats_get "dev s${active_slave#eth} ingress" 101 ".packets" "-n ${g_ns}")
> @@ -296,8 +308,8 @@ garp_test()
>  num_grat_arp()
>  {
>  	local val
> -	for val in 10 20 30 50; do
> -		garp_test "mode active-backup miimon 100 num_grat_arp $val peer_notify_delay 1000"
> +	for val in 10 20 30; do
> +		garp_test "mode active-backup miimon 50 num_grat_arp $val peer_notify_delay 500"

Can we reduce 'peer_notify_delay' even further, say to '250' and
preserve the test effectiveness?

Thanks,

Paolo

Re: [PATCH net-next 3/4] selftests: bonding: reduce garp_test/arp_validate test time

[Hangbin Liu]

On Fri, Jan 26, 2024 at 10:57:31AM +0100, Paolo Abeni wrote:
> On Wed, 2024-01-24 at 17:58 +0800, Hangbin Liu wrote:
> > @@ -276,10 +285,13 @@ garp_test()
> >  	active_slave=$(cmd_jq "ip -n ${s_ns} -d -j link show bond0" ".[].linkinfo.info_data.active_slave")
> >  	ip -n ${s_ns} link set ${active_slave} down
> >  
> > -	exp_num=$(echo "${param}" | cut -f6 -d ' ')
> > -	sleep $((exp_num + 2))
> > +	# wait for active link change
> > +	sleep 1
> 
> If 'slowwait' would loop around a sub-second sleep, I guess you could
> use 'slowwait' here, too.

OK, let me change the slowwait to sleep 0.1s.

> 
> >  
> > +	exp_num=$(echo "${param}" | cut -f6 -d ' ')
> >  	active_slave=$(cmd_jq "ip -n ${s_ns} -d -j link show bond0" ".[].linkinfo.info_data.active_slave")
> > +	slowwait_for_counter $((exp_num + 5)) $exp_num \
> > +		tc_rule_handle_stats_get "dev s${active_slave#eth} ingress" 101 ".packets" "-n ${g_ns}"
> >  
> >  	# check result
> >  	real_num=$(tc_rule_handle_stats_get "dev s${active_slave#eth} ingress" 101 ".packets" "-n ${g_ns}")
> > @@ -296,8 +308,8 @@ garp_test()
> >  num_grat_arp()
> >  {
> >  	local val
> > -	for val in 10 20 30 50; do
> > -		garp_test "mode active-backup miimon 100 num_grat_arp $val peer_notify_delay 1000"
> > +	for val in 10 20 30; do
> > +		garp_test "mode active-backup miimon 50 num_grat_arp $val peer_notify_delay 500"
> 
> Can we reduce 'peer_notify_delay' even further, say to '250' and
> preserve the test effectiveness?

Hmm, maybe we can set miimon to 10. Then we can reduce peer_notify_delay to 100.

Jay, do you think if there is any side efect to set miimon to 10?

Thanks
Hangbin