Re: [PATCH net-next v4] net: netconsole: selftests: Create a new netconsole selftest

[Matthieu Baerts <matttbe@kernel.org>]

Hi Breno,

On 16/08/2024 15:24, Breno Leitao wrote:
> Adds a selftest that creates two virtual interfaces, assigns one to a
> new namespace, and assigns IP addresses to both.
> 
> It listens on the destination interface using socat and configures a
> dynamic target on netconsole, pointing to the destination IP address.
> 
> The test then checks if the message was received properly on the
> destination interface.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> Changelog:
> 
> v4:
>  * Avoid sleeping in waiting for sockets and files (Matthieu Baerts)
>  * Some other improvements (Matthieu Baerts)
>  * Add configfs as a dependency (Jakub)

Thank you for the new version!

It looks good to me, but again, my review mainly focused on the
Bash-related stuff, not on the netconsole test itself.

I just have one question below, but not blocking.

(...)

> diff --git a/tools/testing/selftests/drivers/net/netcons_basic.sh b/tools/testing/selftests/drivers/net/netcons_basic.sh
> new file mode 100755
> index 000000000000..5c3686af1fe8
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/netcons_basic.sh
> @@ -0,0 +1,249 @@

(...)

> +check_file_size() {
> +	local file="$1"
> +
> +	if [[ ! -f "$file" ]]; then
> +		# File might not exist yet
> +		return 1
> +	fi
> +
> +	# Get file size
> +	local size=$(stat -c %s "$file" 2>/dev/null)
> +	# Check if stat command succeeded
> +	if [[ $? -ne 0 ]]; then
> +		return 1
> +	fi
> +
> +	# Check if size is greater than zero
> +	if [[ "$size" -gt 0 ]]; then
> +		return 0  # file size > 0
> +	else
> +		return 1  # file size == 0
> +	fi
> +}

(...)

> +# Wait until socat saves the file to disk
> +busywait "${BUSYWAIT_TIMEOUT}" check_file_size "${OUTPUT_FILE}"

It looks like your 'check_file_size' helper is a reimplementation of
'test -s <FILE>', no? Can you not simply use:

  busywait "${BUSYWAIT_TIMEOUT}" test -s "${OUTPUT_FILE}"

Apart from that, the rest looks good to me!

Acked-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.