[PATCH net-next v2 7/7] selftest: netconsole: add test for cmdline configuration

Breno Leitao posted 7 patches 4 months ago
There is a newer version of this series
[PATCH net-next v2 7/7] selftest: netconsole: add test for cmdline configuration
Posted by Breno Leitao 4 months ago
Add a new selftest to verify netconsole module loading with command
line arguments. This test exercises the init_netconsole() path and
validates proper parsing of the netconsole= parameter format.

The test:
- Loads netconsole module with cmdline configuration instead of
  dynamic reconfiguration
- Validates message transmission through the configured target
- Adds helper functions for cmdline string generation and module
  validation

This complements existing netconsole selftests by covering the
module initialization code path that processes boot-time parameters.
This test is useful to test issues like the one described in [1].

Link: https://lore.kernel.org/netdev/Z36TlACdNMwFD7wv@dev-ushankar.dev.purestorage.com/ [1]
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 tools/testing/selftests/drivers/net/Makefile       |  1 +
 .../selftests/drivers/net/lib/sh/lib_netcons.sh    | 39 +++++++++++++---
 .../selftests/drivers/net/netcons_cmdline.sh       | 52 ++++++++++++++++++++++
 3 files changed, 86 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile
index be780bcb73a3b..bd309b2d39095 100644
--- a/tools/testing/selftests/drivers/net/Makefile
+++ b/tools/testing/selftests/drivers/net/Makefile
@@ -12,6 +12,7 @@ TEST_GEN_FILES := \
 TEST_PROGS := \
 	napi_id.py \
 	netcons_basic.sh \
+	netcons_cmdline.sh \
 	netcons_fragmented_msg.sh \
 	netcons_overflow.sh \
 	netcons_sysdata.sh \
diff --git a/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh
index 71a5a8b1712c0..42fd458be56c9 100644
--- a/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh
+++ b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh
@@ -121,6 +121,17 @@ function create_dynamic_target() {
 	echo 1 > "${NETCONS_PATH}"/enabled
 }
 
+# Generate the command line argument for netconsole following:
+#  netconsole=[+][src-port]@[src-ip]/[<dev>],[tgt-port]@<tgt-ip>/[tgt-macaddr]
+function create_cmdline_str() {
+	DSTMAC=$(ip netns exec "${NAMESPACE}" \
+		 ip link show "${DSTIF}" | awk '/ether/ {print $2}')
+	SRCPORT="1514"
+	TGTPORT="6666"
+
+	echo "netconsole=\"+${SRCPORT}@${SRCIP}/${SRCIF},${TGTPORT}@${DSTIP}/${DSTMAC}\""
+}
+
 # Do not append the release to the header of the message
 function disable_release_append() {
 	echo 0 > "${NETCONS_PATH}"/enabled
@@ -169,13 +180,9 @@ function listen_port_and_save_to() {
 		socat UDP-LISTEN:"${PORT}",fork "${OUTPUT}"
 }
 
-function validate_result() {
+# Only validate that the message arrived properly
+function validate_msg() {
 	local TMPFILENAME="$1"
-	local FORMAT=${2:-"extended"}
-
-	# TMPFILENAME will contain something like:
-	# 6.11.1-0_fbk0_rc13_509_g30d75cea12f7,13,1822,115075213798,-;netconsole selftest: netcons_gtJHM
-	#  key=value
 
 	# Check if the file exists
 	if [ ! -f "$TMPFILENAME" ]; then
@@ -188,6 +195,17 @@ function validate_result() {
 		cat "${TMPFILENAME}" >&2
 		exit "${ksft_fail}"
 	fi
+}
+
+# Validate the message and userdata
+function validate_result() {
+	local TMPFILENAME="$1"
+
+	# TMPFILENAME will contain something like:
+	# 6.11.1-0_fbk0_rc13_509_g30d75cea12f7,13,1822,115075213798,-;netconsole selftest: netcons_gtJHM
+	#  key=value
+
+	validate_msg "${TMPFILENAME}"
 
 	# userdata is not supported on basic format target,
 	# thus, do not validate it.
@@ -263,3 +281,12 @@ function pkill_socat() {
 	pkill -f "${PROCESS_NAME}"
 	set -e
 }
+
+# Check if netconsole was compiled as a module, otherwise exit
+function check_netconsole_module() {
+	if modinfo netconsole | grep filename: | grep -q builtin
+	then
+		echo "SKIP: netconsole should be compiled as a module" >&2
+		exit "${ksft_skip}"
+	fi
+}
diff --git a/tools/testing/selftests/drivers/net/netcons_cmdline.sh b/tools/testing/selftests/drivers/net/netcons_cmdline.sh
new file mode 100755
index 0000000000000..6d743b6e4922b
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/netcons_cmdline.sh
@@ -0,0 +1,52 @@
+#!/usr/bin/env bash
+# SPDX-License-Identifier: GPL-2.0
+
+# This is a selftest to test cmdline arguments on netconsole.
+# It exercises loading of netconsole from cmdline instead of the dynamic
+# reconfiguration. This includes parsing the long netconsole= line and all the
+# flow through init_netconsole().
+#
+# Author: Breno Leitao <leitao@debian.org>
+
+set -euo pipefail
+
+SCRIPTDIR=$(dirname "$(readlink -e "${BASH_SOURCE[0]}")")
+
+source "${SCRIPTDIR}"/lib/sh/lib_netcons.sh
+
+check_netconsole_module
+
+modprobe netdevsim 2> /dev/null || true
+rmmod netconsole 2> /dev/null || true
+
+# The content of kmsg will be save to the following file
+OUTPUT_FILE="/tmp/${TARGET}"
+
+# Check for basic system dependency and exit if not found
+# check_for_dependencies
+# Set current loglevel to KERN_INFO(6), and default to KERN_NOTICE(5)
+echo "6 5" > /proc/sys/kernel/printk
+# Remove the namespace and network interfaces
+trap cleanup_all_ns EXIT
+# Create one namespace and two interfaces
+set_network
+# Create the command line for netconsole, with the configuration from the
+# function above
+CMDLINE="$(create_cmdline_str)"
+
+# Load the module, with the cmdline set
+modprobe netconsole "${CMDLINE}"
+
+# Listed for netconsole port inside the namespace and destination interface
+listen_port_and_save_to "${OUTPUT_FILE}" &
+# Wait for socat to start and listen to the port.
+wait_local_port_listen "${NAMESPACE}" "${PORT}" udp
+# Send the message
+echo "${MSG}: ${TARGET}" > /dev/kmsg
+# Wait until socat saves the file to disk
+busywait "${BUSYWAIT_TIMEOUT}" test -s "${OUTPUT_FILE}"
+# Make sure the message was received in the dst part
+# and exit
+validate_msg "${OUTPUT_FILE}"
+
+exit "${ksft_pass}"

-- 
2.47.1
Re: [PATCH net-next v2 7/7] selftest: netconsole: add test for cmdline configuration
Posted by Jakub Kicinski 4 months ago
On Wed, 11 Jun 2025 07:18:56 -0700 Breno Leitao wrote:
> Add a new selftest to verify netconsole module loading with command
> line arguments. This test exercises the init_netconsole() path and
> validates proper parsing of the netconsole= parameter format.
> 
> The test:
> - Loads netconsole module with cmdline configuration instead of
>   dynamic reconfiguration
> - Validates message transmission through the configured target
> - Adds helper functions for cmdline string generation and module
>   validation
> 
> This complements existing netconsole selftests by covering the
> module initialization code path that processes boot-time parameters.
> This test is useful to test issues like the one described in [1].

I think this leaks the IP address, because if netcons_overflow.sh
runs afterwards it skips with:

#     inet 192.0.2.1/24 scope global eni370np1
# SKIP: IPs already in use. Skipping it


if netcons_overflow.sh runs first everything is fine.
-- 
pw-bot: cr
Re: [PATCH net-next v2 7/7] selftest: netconsole: add test for cmdline configuration
Posted by Breno Leitao 3 months, 4 weeks ago
On Thu, Jun 12, 2025 at 03:05:42PM -0700, Jakub Kicinski wrote:
> On Wed, 11 Jun 2025 07:18:56 -0700 Breno Leitao wrote:
> > Add a new selftest to verify netconsole module loading with command
> > line arguments. This test exercises the init_netconsole() path and
> > validates proper parsing of the netconsole= parameter format.
> > 
> > The test:
> > - Loads netconsole module with cmdline configuration instead of
> >   dynamic reconfiguration
> > - Validates message transmission through the configured target
> > - Adds helper functions for cmdline string generation and module
> >   validation
> > 
> > This complements existing netconsole selftests by covering the
> > module initialization code path that processes boot-time parameters.
> > This test is useful to test issues like the one described in [1].
> 
> I think this leaks the IP address, because if netcons_overflow.sh
> runs afterwards it skips with:
> 
> #     inet 192.0.2.1/24 scope global eni370np1
> # SKIP: IPs already in use. Skipping it
> 
> if netcons_overflow.sh runs first everything is fine.

Thanks for the report. I acknowledge there is a bug in the cleanup part
of the code. Basically `trap cleanup_all_ns EXIT` is not enought because
it was not removing the local netdevsim interface, which must be removed
as well. I will update and resend. Good catch, again.

--breno