* [PATCH net-next 1/4] selftests/net/forwarding: add slowwait functions
2024-01-24 9:58 [PATCH net-next 0/4] selftests: bonding: use busy/slowwait when waiting Hangbin Liu
@ 2024-01-24 9:58 ` Hangbin Liu
2024-01-24 13:25 ` Przemek Kitszel
2024-01-24 9:58 ` [PATCH net-next 2/4] selftests: bonding: use tc filter to check if LACP was sent Hangbin Liu
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Hangbin Liu @ 2024-01-24 9:58 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Liang Li, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH net-next 1/4] selftests/net/forwarding: add slowwait functions
2024-01-24 9:58 ` [PATCH net-next 1/4] selftests/net/forwarding: add slowwait functions Hangbin Liu
@ 2024-01-24 13:25 ` Przemek Kitszel
2024-01-26 9:22 ` Hangbin Liu
0 siblings, 1 reply; 11+ messages in thread
From: Przemek Kitszel @ 2024-01-24 13:25 UTC (permalink / raw)
To: Hangbin Liu, netdev
Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Liang Li
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 ;)
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net-next 1/4] selftests/net/forwarding: add slowwait functions
2024-01-24 13:25 ` Przemek Kitszel
@ 2024-01-26 9:22 ` Hangbin Liu
2024-01-26 9:53 ` Paolo Abeni
0 siblings, 1 reply; 11+ messages in thread
From: Hangbin Liu @ 2024-01-26 9:22 UTC (permalink / raw)
To: Przemek Kitszel
Cc: netdev, Jay Vosburgh, David S . Miller, Jakub Kicinski,
Paolo Abeni, Eric Dumazet, Liang Li
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net-next 1/4] selftests/net/forwarding: add slowwait functions
2024-01-26 9:22 ` Hangbin Liu
@ 2024-01-26 9:53 ` Paolo Abeni
0 siblings, 0 replies; 11+ messages in thread
From: Paolo Abeni @ 2024-01-26 9:53 UTC (permalink / raw)
To: Hangbin Liu, Przemek Kitszel
Cc: netdev, Jay Vosburgh, David S . Miller, Jakub Kicinski,
Eric Dumazet, Liang Li
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next 2/4] selftests: bonding: use tc filter to check if LACP was sent
2024-01-24 9:58 [PATCH net-next 0/4] selftests: bonding: use busy/slowwait when waiting Hangbin Liu
2024-01-24 9:58 ` [PATCH net-next 1/4] selftests/net/forwarding: add slowwait functions Hangbin Liu
@ 2024-01-24 9:58 ` Hangbin Liu
2024-01-24 9:58 ` [PATCH net-next 3/4] selftests: bonding: reduce garp_test/arp_validate test time Hangbin Liu
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Hangbin Liu @ 2024-01-24 9:58 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Liang Li, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH net-next 3/4] selftests: bonding: reduce garp_test/arp_validate test time
2024-01-24 9:58 [PATCH net-next 0/4] selftests: bonding: use busy/slowwait when waiting Hangbin Liu
2024-01-24 9:58 ` [PATCH net-next 1/4] selftests/net/forwarding: add slowwait functions Hangbin Liu
2024-01-24 9:58 ` [PATCH net-next 2/4] selftests: bonding: use tc filter to check if LACP was sent Hangbin Liu
@ 2024-01-24 9:58 ` Hangbin Liu
2024-01-26 9:57 ` Paolo Abeni
2024-01-24 9:58 ` [PATCH net-next 4/4] selftests: bonding: use busy/slowwait instead of hard code sleep Hangbin Liu
2024-01-24 13:26 ` [PATCH net-next 0/4] selftests: bonding: use busy/slowwait when waiting Przemek Kitszel
4 siblings, 1 reply; 11+ messages in thread
From: Hangbin Liu @ 2024-01-24 9:58 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Liang Li, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH net-next 3/4] selftests: bonding: reduce garp_test/arp_validate test time
2024-01-24 9:58 ` [PATCH net-next 3/4] selftests: bonding: reduce garp_test/arp_validate test time Hangbin Liu
@ 2024-01-26 9:57 ` Paolo Abeni
2024-01-26 12:52 ` Hangbin Liu
0 siblings, 1 reply; 11+ messages in thread
From: Paolo Abeni @ 2024-01-26 9:57 UTC (permalink / raw)
To: Hangbin Liu, netdev
Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Eric Dumazet,
Liang Li
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net-next 3/4] selftests: bonding: reduce garp_test/arp_validate test time
2024-01-26 9:57 ` Paolo Abeni
@ 2024-01-26 12:52 ` Hangbin Liu
0 siblings, 0 replies; 11+ messages in thread
From: Hangbin Liu @ 2024-01-26 12:52 UTC (permalink / raw)
To: Paolo Abeni, Jay Vosburgh
Cc: netdev, David S . Miller, Jakub Kicinski, Eric Dumazet, Liang Li
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next 4/4] selftests: bonding: use busy/slowwait instead of hard code sleep
2024-01-24 9:58 [PATCH net-next 0/4] selftests: bonding: use busy/slowwait when waiting Hangbin Liu
` (2 preceding siblings ...)
2024-01-24 9:58 ` [PATCH net-next 3/4] selftests: bonding: reduce garp_test/arp_validate test time Hangbin Liu
@ 2024-01-24 9:58 ` Hangbin Liu
2024-01-24 13:26 ` [PATCH net-next 0/4] selftests: bonding: use busy/slowwait when waiting Przemek Kitszel
4 siblings, 0 replies; 11+ messages in thread
From: Hangbin Liu @ 2024-01-24 9:58 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Liang Li, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH net-next 0/4] selftests: bonding: use busy/slowwait when waiting
2024-01-24 9:58 [PATCH net-next 0/4] selftests: bonding: use busy/slowwait when waiting Hangbin Liu
` (3 preceding siblings ...)
2024-01-24 9:58 ` [PATCH net-next 4/4] selftests: bonding: use busy/slowwait instead of hard code sleep Hangbin Liu
@ 2024-01-24 13:26 ` Przemek Kitszel
4 siblings, 0 replies; 11+ messages in thread
From: Przemek Kitszel @ 2024-01-24 13:26 UTC (permalink / raw)
To: Hangbin Liu, netdev
Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Liang Li
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>
^ permalink raw reply [flat|nested] 11+ messages in thread