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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <31c8afe0-86fe-4b39-ba7d-a26d157972c9@intel.com>
Date: Wed, 24 Jan 2024 14:25:57 +0100
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Hangbin Liu <liuhangbin@...il.com>, <netdev@...r.kernel.org>
CC: Jay Vosburgh <j.vosburgh@...il.com>, "David S . Miller"
	<davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>, Liang Li
	<liali@...hat.com>
Subject: Re: [PATCH net-next 1/4] selftests/net/forwarding: add slowwait
 functions

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@...il.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 ;)