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

Przemek Kitszel
2024-01-24T13:26:07Z

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 ;)