netdev - Re: [PATCH net-next] selftests: netconsole: Validate interface selection by MAC address

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] [day] [month] [year] [list]

Message-ID: <6gefv2nqfkxpjicu3ji6ljqwtr4sbjlrwee4ntpvr427qwv3qv@icuhokcpkl2j>
Date: Tue, 12 Aug 2025 07:01:37 -0700
From: Breno Leitao <leitao@...ian.org>
To: Andre Carvalho <asantostc@...il.com>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Shuah Khan <shuah@...nel.org>, 
	netdev@...r.kernel.org, linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] selftests: netconsole: Validate interface
 selection by MAC address

Hello Andre,

On Mon, Aug 11, 2025 at 07:32:09PM +0100, Andre Carvalho wrote:
> Extend the existing netconsole cmdline selftest to also validate that
> interface selection can be performed via MAC address.
> 
> The test now validates that netconsole works with both interface name
> and MAC address, improving test coverage.

Thanks for adding this test!

> --- a/tools/testing/selftests/drivers/net/netcons_cmdline.sh
> +++ b/tools/testing/selftests/drivers/net/netcons_cmdline.sh
> @@ -17,10 +17,6 @@ source "${SCRIPTDIR}"/lib/sh/lib_netcons.sh

> +# Run the test twice, with different cmdline parameters
> +for BINDMODE in "ifname" "mac"
> +do
> +	echo "Running with bind mode: ${BINDMODE}"

Please echo this message to stderr.

> +	# Create the command line for netconsole, with the configuration from the

This is over 80 chars, move the 'the' word in to live below.

> +	# function above
> +	CMDLINE="$(create_cmdline_str "${BINDMODE}")"

I don't think these nested double quotes would work as expected. Maybe
something as:

	CMDLINE=$(create_cmdline_str "${BINDMODE}")

> +	# The content of kmsg will be save to the following file
> +	OUTPUT_FILE="/tmp/${TARGET}-${BINDMODE}"
> +
> +	# Unload the module, if present
> +	rmmod netconsole 2> /dev/null || true

In this case, you do not want netconsole to fail to unload, otherwise
your test will fail, right?

You might want to remove '|| true' completely.

Other than that, the rest looks fine, and with the changes above, please
add:

Reviewed-by: Breno Leitao <leitao@...ian.org>