[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cd9473bb-36d5-4b62-8523-f9112dc176f2@kernel.org>
Date: Fri, 16 Aug 2024 16:02:51 +0200
From: Matthieu Baerts <matttbe@...nel.org>
To: Breno Leitao <leitao@...ian.org>, davem@...emloft.net,
edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
liuhangbin@...il.com, petrm@...dia.com, Shuah Khan <shuah@...nel.org>
Cc: netdev@...r.kernel.org, Willem de Bruijn <willemb@...gle.com>,
David Wei <dw@...idwei.uk>, open list <linux-kernel@...r.kernel.org>,
"open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH net-next v4] net: netconsole: selftests: Create a new
netconsole selftest
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@...ian.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@...nel.org>
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
Powered by blists - more mailing lists