Re: [PATCH mptcp-next v3 27/29] selftests: mptcp: add mptcp_lib_make_file

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 25/09/2023 10:42, Geliang Tang wrote:
> make_file() helper in mptcp_sockopt.sh and userspace_pm.sh are the same.
> Export it into mptcp_lib.sh and rename it as mptcp_lib_kill_wait(). Use
> it in both mptcp_connect.sh and mptcp_join.sh.

(...)

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> index 3b971d1617d8..dc4a1dd3566d 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> @@ -592,9 +592,8 @@ make_file()
>  	ksize=$((SIZE / 1024))
>  	rem=$((SIZE - (ksize * 1024)))
>  
> -	dd if=/dev/urandom of="$name" bs=1024 count=$ksize 2> /dev/null
> +	mptcp_lib_make_file $name 1024 $ksize
>  	dd if=/dev/urandom conv=notrunc of="$name" bs=1 count=$rem 2> /dev/null

Mmh, why were we doing that?

I guess we are missing something like "oflag=append" because this will
write "${rem}" bytes at the beginning of the file where there is already
some random bytes. It should write that at the end. Do you mind adding a
commit adding "oflag=append" please?

Then it means this should be done in the version of the "lib" as well
(an optional behaviour). That may increase a bit the complexity.

(Or we remove this second dd line (+ ${rem}) in a dedicated commit
because we were not using it correctly from the beginning and we are
fine like that (if I'm not mistaken)?)

> -	echo -e "\nMPTCP_TEST_FILE_END_MARKER" >> "$name"
>  
>  	echo "Created $name (size $(du -b "$name")) containing data sent by $who"
>  }
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 26ea0919810f..f88168d66fdc 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -1155,8 +1155,7 @@ make_file()
>  	local who=$2
>  	local size=$3
>  
> -	dd if=/dev/urandom of="$name" bs=1024 count=$size 2> /dev/null
> -	echo -e "\nMPTCP_TEST_FILE_END_MARKER" >> "$name"
> +	mptcp_lib_make_file $name 1024 $size
>  
>  	print_info "Test file (size $size KB) for $who"
>  }
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 0f5e34c76bb4..7b0d03c40f89 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -344,3 +344,12 @@ mptcp_lib_get_counter() {
>  
>  	echo "${count}"
>  }
> +
> +mptcp_lib_make_file() {
> +	local name=$1
> +	local bs=$2
> +	local size=$3
> +
> +	dd if=/dev/urandom of="$name" bs=$bs count=$size 2> /dev/null
> +	echo -e "\nMPTCP_TEST_FILE_END_MARKER" >> "$name"
> +}
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> index bfa744e350ef..39128fca99dd 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> @@ -245,18 +245,6 @@ do_transfer()
>  	return 1
>  }
>  
> -make_file()
> -{
> -	local name=$1
> -	local who=$2
> -	local size=$3
> -
> -	dd if=/dev/urandom of="$name" bs=1024 count=$size 2> /dev/null
> -	echo -e "\nMPTCP_TEST_FILE_END_MARKER" >> "$name"
> -
> -	echo "Created $name (size $size KB) containing data sent by $who"
> -}
> -
>  do_mptcp_sockopt_tests()
>  {
>  	local lret=0
> @@ -357,8 +345,10 @@ sout=$(mktemp)
>  cin=$(mktemp)
>  cout=$(mktemp)
>  init
> -make_file "$cin" "client" 1
> -make_file "$sin" "server" 1
> +mptcp_lib_make_file "$cin" 1024 1
> +echo "Created $cin (size 1 KB) containing data sent by client"
> +mptcp_lib_make_file "$sin" 1024 1
> +echo "Created $sin (size 1 KB) containing data sent by server"

Maybe easier to keep 'make_file()' helper to call mptcp_lib_make_file()
and print the message?

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net