| Message ID | 20240124095814.1882509-4-liuhangbin@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | selftests: bonding: use busy/slowwait when waiting | expand |
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
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
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 }
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(-)