* [PATCH net-next v2 1/4] netconsole: Warn if MAX_USERDATA_ITEMS limit is exceeded
2025-01-03 12:57 [PATCH net-next v2 0/4] netconsole: selftest for userdata overflow Breno Leitao
@ 2025-01-03 12:57 ` Breno Leitao
2025-01-03 12:57 ` [PATCH net-next v2 2/4] netconsole: selftest: Split the helpers from the selftest Breno Leitao
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Breno Leitao @ 2025-01-03 12:57 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Shuah Khan
Cc: netdev, linux-kernel, linux-kselftest, Breno Leitao, kernel-team,
Simon Horman
netconsole configfs helpers doesn't allow the creation of more than
MAX_USERDATA_ITEMS items.
Add a warning when netconsole userdata update function attempts sees
more than MAX_USERDATA_ITEMS entries.
Replace silent ignore mechanism with WARN_ON_ONCE() to highlight
potential misuse during development and debugging.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
---
drivers/net/netconsole.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index f422a2f666ef2276cf5b10e1dbc6badbc8ef0038..86ab4a42769a49eebe5dd6f01dafafc6c86ec54f 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -730,7 +730,7 @@ static void update_userdata(struct netconsole_target *nt)
struct userdatum *udm_item;
struct config_item *item;
- if (child_count >= MAX_USERDATA_ITEMS)
+ if (WARN_ON_ONCE(child_count >= MAX_USERDATA_ITEMS))
break;
child_count++;
--
2.43.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net-next v2 2/4] netconsole: selftest: Split the helpers from the selftest
2025-01-03 12:57 [PATCH net-next v2 0/4] netconsole: selftest for userdata overflow Breno Leitao
2025-01-03 12:57 ` [PATCH net-next v2 1/4] netconsole: Warn if MAX_USERDATA_ITEMS limit is exceeded Breno Leitao
@ 2025-01-03 12:57 ` Breno Leitao
2025-01-07 0:06 ` Jakub Kicinski
2025-01-03 12:57 ` [PATCH net-next v2 3/4] netconsole: selftest: Delete all userdata keys Breno Leitao
2025-01-03 12:57 ` [PATCH net-next v2 4/4] netconsole: selftest: verify userdata entry limit Breno Leitao
3 siblings, 1 reply; 7+ messages in thread
From: Breno Leitao @ 2025-01-03 12:57 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Shuah Khan
Cc: netdev, linux-kernel, linux-kselftest, Breno Leitao, kernel-team,
Simon Horman
Split helper functions from the netconsole basic test into a separate
library file to enable reuse across different netconsole tests. This
change only moves the existing helper functions to lib/sh/lib_netcons.sh
while preserving the same test functionality.
The helpers provide common functions for:
- Setting up network namespaces and interfaces
- Managing netconsole dynamic targets
- Setting user data
- Handling test dependencies
- Cleanup operations
Do not make any change in the code, other than the mechanical
separation.
Signed-off-by: Breno Leitao <leitao@debian.org>
Tested-by: Simon Horman <horms@kernel.org>
Reviewed-by: Simon Horman <horms@kernel.org>
---
MAINTAINERS | 1 +
.../selftests/drivers/net/lib/sh/lib_netcons.sh | 225 +++++++++++++++++++++
.../testing/selftests/drivers/net/netcons_basic.sh | 218 +-------------------
3 files changed, 227 insertions(+), 217 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 1579124ef426b8f2e066462e90e6c8e4d068f01e..d7af1c38dbab0c1ac253ced11a07ee7422c75c0a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16174,6 +16174,7 @@ M: Breno Leitao <leitao@debian.org>
S: Maintained
F: Documentation/networking/netconsole.rst
F: drivers/net/netconsole.c
+F: tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh
F: tools/testing/selftests/drivers/net/netcons_basic.sh
NETDEVSIM
diff --git a/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh
new file mode 100644
index 0000000000000000000000000000000000000000..fdd45a3468f17449eeb66d9a808b7a3b2107e47c
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh
@@ -0,0 +1,225 @@
+#!/usr/bin/env bash
+# SPDX-License-Identifier: GPL-2.0
+
+# This file contains functions and helpers to support the netconsole
+# selftests
+#
+# Author: Breno Leitao <leitao@debian.org>
+
+set -euo pipefail
+
+LIBDIR=$(dirname "$(readlink -e "${BASH_SOURCE[0]}")")
+
+SRCIF="" # to be populated later
+SRCIP=192.0.2.1
+DSTIF="" # to be populated later
+DSTIP=192.0.2.2
+
+PORT="6666"
+MSG="netconsole selftest"
+USERDATA_KEY="key"
+USERDATA_VALUE="value"
+TARGET=$(mktemp -u netcons_XXXXX)
+DEFAULT_PRINTK_VALUES=$(cat /proc/sys/kernel/printk)
+NETCONS_CONFIGFS="/sys/kernel/config/netconsole"
+NETCONS_PATH="${NETCONS_CONFIGFS}"/"${TARGET}"
+KEY_PATH="${NETCONS_PATH}/userdata/${USERDATA_KEY}"
+# NAMESPACE will be populated by setup_ns with a random value
+NAMESPACE=""
+
+# IDs for netdevsim
+NSIM_DEV_1_ID=$((256 + RANDOM % 256))
+NSIM_DEV_2_ID=$((512 + RANDOM % 256))
+NSIM_DEV_SYS_NEW="/sys/bus/netdevsim/new_device"
+
+# Used to create and delete namespaces
+source "${LIBDIR}"/../../../../net/lib.sh
+source "${LIBDIR}"/../../../../net/net_helper.sh
+
+# Create netdevsim interfaces
+create_ifaces() {
+
+ echo "$NSIM_DEV_2_ID" > "$NSIM_DEV_SYS_NEW"
+ echo "$NSIM_DEV_1_ID" > "$NSIM_DEV_SYS_NEW"
+ udevadm settle 2> /dev/null || true
+
+ local NSIM1=/sys/bus/netdevsim/devices/netdevsim"$NSIM_DEV_1_ID"
+ local NSIM2=/sys/bus/netdevsim/devices/netdevsim"$NSIM_DEV_2_ID"
+
+ # These are global variables
+ SRCIF=$(find "$NSIM1"/net -maxdepth 1 -type d ! \
+ -path "$NSIM1"/net -exec basename {} \;)
+ DSTIF=$(find "$NSIM2"/net -maxdepth 1 -type d ! \
+ -path "$NSIM2"/net -exec basename {} \;)
+}
+
+link_ifaces() {
+ local NSIM_DEV_SYS_LINK="/sys/bus/netdevsim/link_device"
+ local SRCIF_IFIDX=$(cat /sys/class/net/"$SRCIF"/ifindex)
+ local DSTIF_IFIDX=$(cat /sys/class/net/"$DSTIF"/ifindex)
+
+ exec {NAMESPACE_FD}</var/run/netns/"${NAMESPACE}"
+ exec {INITNS_FD}</proc/self/ns/net
+
+ # Bind the dst interface to namespace
+ ip link set "${DSTIF}" netns "${NAMESPACE}"
+
+ # Linking one device to the other one (on the other namespace}
+ if ! echo "${INITNS_FD}:$SRCIF_IFIDX $NAMESPACE_FD:$DSTIF_IFIDX" > $NSIM_DEV_SYS_LINK
+ then
+ echo "linking netdevsim1 with netdevsim2 should succeed"
+ cleanup
+ exit "${ksft_skip}"
+ fi
+}
+
+function configure_ip() {
+ # Configure the IPs for both interfaces
+ ip netns exec "${NAMESPACE}" ip addr add "${DSTIP}"/24 dev "${DSTIF}"
+ ip netns exec "${NAMESPACE}" ip link set "${DSTIF}" up
+
+ ip addr add "${SRCIP}"/24 dev "${SRCIF}"
+ ip link set "${SRCIF}" up
+}
+
+function set_network() {
+ # setup_ns function is coming from lib.sh
+ setup_ns NAMESPACE
+
+ # Create both interfaces, and assign the destination to a different
+ # namespace
+ create_ifaces
+
+ # Link both interfaces back to back
+ link_ifaces
+
+ configure_ip
+}
+
+function create_dynamic_target() {
+ DSTMAC=$(ip netns exec "${NAMESPACE}" \
+ ip link show "${DSTIF}" | awk '/ether/ {print $2}')
+
+ # Create a dynamic target
+ mkdir "${NETCONS_PATH}"
+
+ echo "${DSTIP}" > "${NETCONS_PATH}"/remote_ip
+ echo "${SRCIP}" > "${NETCONS_PATH}"/local_ip
+ echo "${DSTMAC}" > "${NETCONS_PATH}"/remote_mac
+ echo "${SRCIF}" > "${NETCONS_PATH}"/dev_name
+
+ echo 1 > "${NETCONS_PATH}"/enabled
+}
+
+function cleanup() {
+ local NSIM_DEV_SYS_DEL="/sys/bus/netdevsim/del_device"
+
+ # delete netconsole dynamic reconfiguration
+ echo 0 > "${NETCONS_PATH}"/enabled
+ # Remove key
+ rmdir "${KEY_PATH}"
+ # Remove the configfs entry
+ rmdir "${NETCONS_PATH}"
+
+ # Delete netdevsim devices
+ echo "$NSIM_DEV_2_ID" > "$NSIM_DEV_SYS_DEL"
+ echo "$NSIM_DEV_1_ID" > "$NSIM_DEV_SYS_DEL"
+
+ # this is coming from lib.sh
+ cleanup_all_ns
+
+ # Restoring printk configurations
+ echo "${DEFAULT_PRINTK_VALUES}" > /proc/sys/kernel/printk
+}
+
+function set_user_data() {
+ if [[ ! -d "${NETCONS_PATH}""/userdata" ]]
+ then
+ echo "Userdata path not available in ${NETCONS_PATH}/userdata"
+ exit "${ksft_skip}"
+ fi
+
+ mkdir -p "${KEY_PATH}"
+ VALUE_PATH="${KEY_PATH}""/value"
+ echo "${USERDATA_VALUE}" > "${VALUE_PATH}"
+}
+
+function listen_port_and_save_to() {
+ local OUTPUT=${1}
+ # Just wait for 2 seconds
+ timeout 2 ip netns exec "${NAMESPACE}" \
+ socat UDP-LISTEN:"${PORT}",fork "${OUTPUT}"
+}
+
+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
+
+ # Check if the file exists
+ if [ ! -f "$TMPFILENAME" ]; then
+ echo "FAIL: File was not generated." >&2
+ exit "${ksft_fail}"
+ fi
+
+ if ! grep -q "${MSG}" "${TMPFILENAME}"; then
+ echo "FAIL: ${MSG} not found in ${TMPFILENAME}" >&2
+ cat "${TMPFILENAME}" >&2
+ exit "${ksft_fail}"
+ fi
+
+ if ! grep -q "${USERDATA_KEY}=${USERDATA_VALUE}" "${TMPFILENAME}"; then
+ echo "FAIL: ${USERDATA_KEY}=${USERDATA_VALUE} not found in ${TMPFILENAME}" >&2
+ cat "${TMPFILENAME}" >&2
+ exit "${ksft_fail}"
+ fi
+
+ # Delete the file once it is validated, otherwise keep it
+ # for debugging purposes
+ rm "${TMPFILENAME}"
+ exit "${ksft_pass}"
+}
+
+function check_for_dependencies() {
+ if [ "$(id -u)" -ne 0 ]; then
+ echo "This test must be run as root" >&2
+ exit "${ksft_skip}"
+ fi
+
+ if ! which socat > /dev/null ; then
+ echo "SKIP: socat(1) is not available" >&2
+ exit "${ksft_skip}"
+ fi
+
+ if ! which ip > /dev/null ; then
+ echo "SKIP: ip(1) is not available" >&2
+ exit "${ksft_skip}"
+ fi
+
+ if ! which udevadm > /dev/null ; then
+ echo "SKIP: udevadm(1) is not available" >&2
+ exit "${ksft_skip}"
+ fi
+
+ if [ ! -f "${NSIM_DEV_SYS_NEW}" ]; then
+ echo "SKIP: file ${NSIM_DEV_SYS_NEW} does not exist. Check if CONFIG_NETDEVSIM is enabled" >&2
+ exit "${ksft_skip}"
+ fi
+
+ if [ ! -d "${NETCONS_CONFIGFS}" ]; then
+ echo "SKIP: directory ${NETCONS_CONFIGFS} does not exist. Check if NETCONSOLE_DYNAMIC is enabled" >&2
+ exit "${ksft_skip}"
+ fi
+
+ if ip link show "${DSTIF}" 2> /dev/null; then
+ echo "SKIP: interface ${DSTIF} exists in the system. Not overwriting it." >&2
+ exit "${ksft_skip}"
+ fi
+
+ if ip addr list | grep -E "inet.*(${SRCIP}|${DSTIP})" 2> /dev/null; then
+ echo "SKIP: IPs already in use. Skipping it" >&2
+ exit "${ksft_skip}"
+ fi
+}
diff --git a/tools/testing/selftests/drivers/net/netcons_basic.sh b/tools/testing/selftests/drivers/net/netcons_basic.sh
index b175f4d966e5056ddb62e335f212c03e55f50fb0..fe765da498e845d7be1fd09551363224d40ded65 100755
--- a/tools/testing/selftests/drivers/net/netcons_basic.sh
+++ b/tools/testing/selftests/drivers/net/netcons_basic.sh
@@ -18,224 +18,8 @@ set -euo pipefail
SCRIPTDIR=$(dirname "$(readlink -e "${BASH_SOURCE[0]}")")
-# Simple script to test dynamic targets in netconsole
-SRCIF="" # to be populated later
-SRCIP=192.0.2.1
-DSTIF="" # to be populated later
-DSTIP=192.0.2.2
+source "${SCRIPTDIR}"/lib/sh/lib_netcons.sh
-PORT="6666"
-MSG="netconsole selftest"
-USERDATA_KEY="key"
-USERDATA_VALUE="value"
-TARGET=$(mktemp -u netcons_XXXXX)
-DEFAULT_PRINTK_VALUES=$(cat /proc/sys/kernel/printk)
-NETCONS_CONFIGFS="/sys/kernel/config/netconsole"
-NETCONS_PATH="${NETCONS_CONFIGFS}"/"${TARGET}"
-KEY_PATH="${NETCONS_PATH}/userdata/${USERDATA_KEY}"
-# NAMESPACE will be populated by setup_ns with a random value
-NAMESPACE=""
-
-# IDs for netdevsim
-NSIM_DEV_1_ID=$((256 + RANDOM % 256))
-NSIM_DEV_2_ID=$((512 + RANDOM % 256))
-NSIM_DEV_SYS_NEW="/sys/bus/netdevsim/new_device"
-
-# Used to create and delete namespaces
-source "${SCRIPTDIR}"/../../net/lib.sh
-source "${SCRIPTDIR}"/../../net/net_helper.sh
-
-# Create netdevsim interfaces
-create_ifaces() {
-
- echo "$NSIM_DEV_2_ID" > "$NSIM_DEV_SYS_NEW"
- echo "$NSIM_DEV_1_ID" > "$NSIM_DEV_SYS_NEW"
- udevadm settle 2> /dev/null || true
-
- local NSIM1=/sys/bus/netdevsim/devices/netdevsim"$NSIM_DEV_1_ID"
- local NSIM2=/sys/bus/netdevsim/devices/netdevsim"$NSIM_DEV_2_ID"
-
- # These are global variables
- SRCIF=$(find "$NSIM1"/net -maxdepth 1 -type d ! \
- -path "$NSIM1"/net -exec basename {} \;)
- DSTIF=$(find "$NSIM2"/net -maxdepth 1 -type d ! \
- -path "$NSIM2"/net -exec basename {} \;)
-}
-
-link_ifaces() {
- local NSIM_DEV_SYS_LINK="/sys/bus/netdevsim/link_device"
- local SRCIF_IFIDX=$(cat /sys/class/net/"$SRCIF"/ifindex)
- local DSTIF_IFIDX=$(cat /sys/class/net/"$DSTIF"/ifindex)
-
- exec {NAMESPACE_FD}</var/run/netns/"${NAMESPACE}"
- exec {INITNS_FD}</proc/self/ns/net
-
- # Bind the dst interface to namespace
- ip link set "${DSTIF}" netns "${NAMESPACE}"
-
- # Linking one device to the other one (on the other namespace}
- if ! echo "${INITNS_FD}:$SRCIF_IFIDX $NAMESPACE_FD:$DSTIF_IFIDX" > $NSIM_DEV_SYS_LINK
- then
- echo "linking netdevsim1 with netdevsim2 should succeed"
- cleanup
- exit "${ksft_skip}"
- fi
-}
-
-function configure_ip() {
- # Configure the IPs for both interfaces
- ip netns exec "${NAMESPACE}" ip addr add "${DSTIP}"/24 dev "${DSTIF}"
- ip netns exec "${NAMESPACE}" ip link set "${DSTIF}" up
-
- ip addr add "${SRCIP}"/24 dev "${SRCIF}"
- ip link set "${SRCIF}" up
-}
-
-function set_network() {
- # setup_ns function is coming from lib.sh
- setup_ns NAMESPACE
-
- # Create both interfaces, and assign the destination to a different
- # namespace
- create_ifaces
-
- # Link both interfaces back to back
- link_ifaces
-
- configure_ip
-}
-
-function create_dynamic_target() {
- DSTMAC=$(ip netns exec "${NAMESPACE}" \
- ip link show "${DSTIF}" | awk '/ether/ {print $2}')
-
- # Create a dynamic target
- mkdir "${NETCONS_PATH}"
-
- echo "${DSTIP}" > "${NETCONS_PATH}"/remote_ip
- echo "${SRCIP}" > "${NETCONS_PATH}"/local_ip
- echo "${DSTMAC}" > "${NETCONS_PATH}"/remote_mac
- echo "${SRCIF}" > "${NETCONS_PATH}"/dev_name
-
- echo 1 > "${NETCONS_PATH}"/enabled
-}
-
-function cleanup() {
- local NSIM_DEV_SYS_DEL="/sys/bus/netdevsim/del_device"
-
- # delete netconsole dynamic reconfiguration
- echo 0 > "${NETCONS_PATH}"/enabled
- # Remove key
- rmdir "${KEY_PATH}"
- # Remove the configfs entry
- rmdir "${NETCONS_PATH}"
-
- # Delete netdevsim devices
- echo "$NSIM_DEV_2_ID" > "$NSIM_DEV_SYS_DEL"
- echo "$NSIM_DEV_1_ID" > "$NSIM_DEV_SYS_DEL"
-
- # this is coming from lib.sh
- cleanup_all_ns
-
- # Restoring printk configurations
- echo "${DEFAULT_PRINTK_VALUES}" > /proc/sys/kernel/printk
-}
-
-function set_user_data() {
- if [[ ! -d "${NETCONS_PATH}""/userdata" ]]
- then
- echo "Userdata path not available in ${NETCONS_PATH}/userdata"
- exit "${ksft_skip}"
- fi
-
- mkdir -p "${KEY_PATH}"
- VALUE_PATH="${KEY_PATH}""/value"
- echo "${USERDATA_VALUE}" > "${VALUE_PATH}"
-}
-
-function listen_port_and_save_to() {
- local OUTPUT=${1}
- # Just wait for 2 seconds
- timeout 2 ip netns exec "${NAMESPACE}" \
- socat UDP-LISTEN:"${PORT}",fork "${OUTPUT}"
-}
-
-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
-
- # Check if the file exists
- if [ ! -f "$TMPFILENAME" ]; then
- echo "FAIL: File was not generated." >&2
- exit "${ksft_fail}"
- fi
-
- if ! grep -q "${MSG}" "${TMPFILENAME}"; then
- echo "FAIL: ${MSG} not found in ${TMPFILENAME}" >&2
- cat "${TMPFILENAME}" >&2
- exit "${ksft_fail}"
- fi
-
- if ! grep -q "${USERDATA_KEY}=${USERDATA_VALUE}" "${TMPFILENAME}"; then
- echo "FAIL: ${USERDATA_KEY}=${USERDATA_VALUE} not found in ${TMPFILENAME}" >&2
- cat "${TMPFILENAME}" >&2
- exit "${ksft_fail}"
- fi
-
- # Delete the file once it is validated, otherwise keep it
- # for debugging purposes
- rm "${TMPFILENAME}"
- exit "${ksft_pass}"
-}
-
-function check_for_dependencies() {
- if [ "$(id -u)" -ne 0 ]; then
- echo "This test must be run as root" >&2
- exit "${ksft_skip}"
- fi
-
- if ! which socat > /dev/null ; then
- echo "SKIP: socat(1) is not available" >&2
- exit "${ksft_skip}"
- fi
-
- if ! which ip > /dev/null ; then
- echo "SKIP: ip(1) is not available" >&2
- exit "${ksft_skip}"
- fi
-
- if ! which udevadm > /dev/null ; then
- echo "SKIP: udevadm(1) is not available" >&2
- exit "${ksft_skip}"
- fi
-
- if [ ! -f "${NSIM_DEV_SYS_NEW}" ]; then
- echo "SKIP: file ${NSIM_DEV_SYS_NEW} does not exist. Check if CONFIG_NETDEVSIM is enabled" >&2
- exit "${ksft_skip}"
- fi
-
- if [ ! -d "${NETCONS_CONFIGFS}" ]; then
- echo "SKIP: directory ${NETCONS_CONFIGFS} does not exist. Check if NETCONSOLE_DYNAMIC is enabled" >&2
- exit "${ksft_skip}"
- fi
-
- if ip link show "${DSTIF}" 2> /dev/null; then
- echo "SKIP: interface ${DSTIF} exists in the system. Not overwriting it." >&2
- exit "${ksft_skip}"
- fi
-
- if ip addr list | grep -E "inet.*(${SRCIP}|${DSTIP})" 2> /dev/null; then
- echo "SKIP: IPs already in use. Skipping it" >&2
- exit "${ksft_skip}"
- fi
-}
-
-# ========== #
-# Start here #
-# ========== #
modprobe netdevsim 2> /dev/null || true
modprobe netconsole 2> /dev/null || true
--
2.43.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net-next v2 2/4] netconsole: selftest: Split the helpers from the selftest
2025-01-03 12:57 ` [PATCH net-next v2 2/4] netconsole: selftest: Split the helpers from the selftest Breno Leitao
@ 2025-01-07 0:06 ` Jakub Kicinski
2025-01-08 11:19 ` Breno Leitao
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2025-01-07 0:06 UTC (permalink / raw)
To: Breno Leitao
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Shuah Khan, netdev, linux-kernel, linux-kselftest, kernel-team,
Simon Horman
On Fri, 03 Jan 2025 04:57:50 -0800 Breno Leitao wrote:
> Split helper functions from the netconsole basic test into a separate
> library file to enable reuse across different netconsole tests. This
> change only moves the existing helper functions to lib/sh/lib_netcons.sh
> while preserving the same test functionality.
>
> The helpers provide common functions for:
> - Setting up network namespaces and interfaces
> - Managing netconsole dynamic targets
> - Setting user data
> - Handling test dependencies
> - Cleanup operations
>
> Do not make any change in the code, other than the mechanical
> separation.
You have to update the Makefile, we don't wildcard all files under
drivers/net/lib, the lib support in ksft is not great :(
$ make -C tools/testing/selftests/ \
TARGETS="drivers/net drivers/net/hw" \
install INSTALL_PATH=/tmp/ksft-net-drv
[... noise ...]
$ find /tmp/ksft-net-drv/ -iname '*netcons*'
/tmp/ksft-net-drv/drivers/net/netcons_basic.sh
/tmp/ksft-net-drv/drivers/net/netcons_overflow.sh
.. your new lib file won't get installed.
--
pw-bot: cr
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 2/4] netconsole: selftest: Split the helpers from the selftest
2025-01-07 0:06 ` Jakub Kicinski
@ 2025-01-08 11:19 ` Breno Leitao
0 siblings, 0 replies; 7+ messages in thread
From: Breno Leitao @ 2025-01-08 11:19 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Shuah Khan, netdev, linux-kernel, linux-kselftest, kernel-team,
Simon Horman
On Mon, Jan 06, 2025 at 04:06:53PM -0800, Jakub Kicinski wrote:
> On Fri, 03 Jan 2025 04:57:50 -0800 Breno Leitao wrote:
> > Split helper functions from the netconsole basic test into a separate
> > library file to enable reuse across different netconsole tests. This
> > change only moves the existing helper functions to lib/sh/lib_netcons.sh
> > while preserving the same test functionality.
> >
> > The helpers provide common functions for:
> > - Setting up network namespaces and interfaces
> > - Managing netconsole dynamic targets
> > - Setting user data
> > - Handling test dependencies
> > - Cleanup operations
> >
> > Do not make any change in the code, other than the mechanical
> > separation.
>
> You have to update the Makefile, we don't wildcard all files under
> drivers/net/lib, the lib support in ksft is not great :(
>
> $ make -C tools/testing/selftests/ \
> TARGETS="drivers/net drivers/net/hw" \
> install INSTALL_PATH=/tmp/ksft-net-drv
> [... noise ...]
> $ find /tmp/ksft-net-drv/ -iname '*netcons*'
> /tmp/ksft-net-drv/drivers/net/netcons_basic.sh
> /tmp/ksft-net-drv/drivers/net/netcons_overflow.sh
>
> .. your new lib file won't get installed.
Thanks for the detailed explanation. I found that the new lib was not
installed.
I will be adding the following to the upcoming revision.
/tools/testing/selftests/drivers/net/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
TEST_INCLUDES := $(wildcard lib/py/*.py) \
+ $(wildcard lib/sh/*.sh) \
../../net/net_helper.sh \
../../net/lib.sh \
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next v2 3/4] netconsole: selftest: Delete all userdata keys
2025-01-03 12:57 [PATCH net-next v2 0/4] netconsole: selftest for userdata overflow Breno Leitao
2025-01-03 12:57 ` [PATCH net-next v2 1/4] netconsole: Warn if MAX_USERDATA_ITEMS limit is exceeded Breno Leitao
2025-01-03 12:57 ` [PATCH net-next v2 2/4] netconsole: selftest: Split the helpers from the selftest Breno Leitao
@ 2025-01-03 12:57 ` Breno Leitao
2025-01-03 12:57 ` [PATCH net-next v2 4/4] netconsole: selftest: verify userdata entry limit Breno Leitao
3 siblings, 0 replies; 7+ messages in thread
From: Breno Leitao @ 2025-01-03 12:57 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Shuah Khan
Cc: netdev, linux-kernel, linux-kselftest, Breno Leitao, kernel-team,
Simon Horman
Modify the cleanup function to remove all userdata keys created during the
test, instead of just deleting a single predefined key. This ensures a
more thorough cleanup of temporary resources.
Move the KEY_PATH variable definition inside the set_user_data function
to reduce global variables and improve encapsulation. The KEY_PATH
variable is now dynamically created when setting user data.
This change has no effect on the current test, while improving an
upcoming test that would create several userdata entries.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
---
tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
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 fdd45a3468f17449eeb66d9a808b7a3b2107e47c..3acaba41ac7b21aa2fd8457ed640a5ac8a41bc12 100644
--- a/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh
+++ b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh
@@ -23,7 +23,6 @@ TARGET=$(mktemp -u netcons_XXXXX)
DEFAULT_PRINTK_VALUES=$(cat /proc/sys/kernel/printk)
NETCONS_CONFIGFS="/sys/kernel/config/netconsole"
NETCONS_PATH="${NETCONS_CONFIGFS}"/"${TARGET}"
-KEY_PATH="${NETCONS_PATH}/userdata/${USERDATA_KEY}"
# NAMESPACE will be populated by setup_ns with a random value
NAMESPACE=""
@@ -116,8 +115,8 @@ function cleanup() {
# delete netconsole dynamic reconfiguration
echo 0 > "${NETCONS_PATH}"/enabled
- # Remove key
- rmdir "${KEY_PATH}"
+ # Remove all the keys that got created during the selftest
+ find "${NETCONS_PATH}/userdata/" -mindepth 1 -type d -delete
# Remove the configfs entry
rmdir "${NETCONS_PATH}"
@@ -139,6 +138,7 @@ function set_user_data() {
exit "${ksft_skip}"
fi
+ KEY_PATH="${NETCONS_PATH}/userdata/${USERDATA_KEY}"
mkdir -p "${KEY_PATH}"
VALUE_PATH="${KEY_PATH}""/value"
echo "${USERDATA_VALUE}" > "${VALUE_PATH}"
--
2.43.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net-next v2 4/4] netconsole: selftest: verify userdata entry limit
2025-01-03 12:57 [PATCH net-next v2 0/4] netconsole: selftest for userdata overflow Breno Leitao
` (2 preceding siblings ...)
2025-01-03 12:57 ` [PATCH net-next v2 3/4] netconsole: selftest: Delete all userdata keys Breno Leitao
@ 2025-01-03 12:57 ` Breno Leitao
3 siblings, 0 replies; 7+ messages in thread
From: Breno Leitao @ 2025-01-03 12:57 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Shuah Khan
Cc: netdev, linux-kernel, linux-kselftest, Breno Leitao, kernel-team,
Simon Horman
Add a new selftest for netconsole that tests the userdata entry limit
functionality. The test performs two key verifications:
1. Create MAX_USERDATA_ITEMS (16) userdata entries successfully
2. Confirm that attempting to create an additional userdata entry fails
The selftest script uses the netcons library and checks the behavior
by attempting to create entries beyond the maximum allowed limit.
Signed-off-by: Breno Leitao <leitao@debian.org>
Tested-by: Simon Horman <horms@kernel.org>
Reviewed-by: Simon Horman <horms@kernel.org>
---
MAINTAINERS | 2 +-
tools/testing/selftests/drivers/net/Makefile | 1 +
.../selftests/drivers/net/netcons_overflow.sh | 67 ++++++++++++++++++++++
3 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index d7af1c38dbab0c1ac253ced11a07ee7422c75c0a..d14bfb5ab0122f0b6651e72add3b178dcf742f18 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16175,7 +16175,7 @@ S: Maintained
F: Documentation/networking/netconsole.rst
F: drivers/net/netconsole.c
F: tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh
-F: tools/testing/selftests/drivers/net/netcons_basic.sh
+F: tools/testing/selftests/drivers/net/netcons\*
NETDEVSIM
M: Jakub Kicinski <kuba@kernel.org>
diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile
index 0fec8f9801ad4d94d99ab6d0acaea6e2e7b2894d..bcd630eb072eb9cf22bb11a647de751795de2408 100644
--- a/tools/testing/selftests/drivers/net/Makefile
+++ b/tools/testing/selftests/drivers/net/Makefile
@@ -6,6 +6,7 @@ TEST_INCLUDES := $(wildcard lib/py/*.py) \
TEST_PROGS := \
netcons_basic.sh \
+ netcons_overflow.sh \
ping.py \
queues.py \
stats.py \
diff --git a/tools/testing/selftests/drivers/net/netcons_overflow.sh b/tools/testing/selftests/drivers/net/netcons_overflow.sh
new file mode 100755
index 0000000000000000000000000000000000000000..29bad56448a24a2d98c21bd53b74f3bc2ca7e64a
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/netcons_overflow.sh
@@ -0,0 +1,67 @@
+#!/usr/bin/env bash
+# SPDX-License-Identifier: GPL-2.0
+
+# This test verifies that users can successfully create up to
+# MAX_USERDATA_ITEMS userdata entries without encountering any failures.
+#
+# Additionally, it tests for expected failure when attempting to exceed this
+# maximum limit.
+#
+# Author: Breno Leitao <leitao@debian.org>
+
+set -euo pipefail
+
+SCRIPTDIR=$(dirname "$(readlink -e "${BASH_SOURCE[0]}")")
+
+source "${SCRIPTDIR}"/lib/sh/lib_netcons.sh
+# This is coming from netconsole code. Check for it in drivers/net/netconsole.c
+MAX_USERDATA_ITEMS=16
+
+# Function to create userdata entries
+function create_userdata_max_entries() {
+ # All these keys should be created without any error
+ for i in $(seq $MAX_USERDATA_ITEMS)
+ do
+ # USERDATA_KEY is used by set_user_data
+ USERDATA_KEY="key"${i}
+ set_user_data
+ done
+}
+
+# Function to verify the entry limit
+function verify_entry_limit() {
+ # Allowing the test to fail without exiting, since the next command
+ # will fail
+ set +e
+ mkdir "${NETCONS_PATH}/userdata/key_that_will_fail" 2> /dev/null
+ ret="$?"
+ set -e
+ if [ "$ret" -eq 0 ];
+ then
+ echo "Adding more than ${MAX_USERDATA_ITEMS} entries in userdata should fail, but it didn't" >&2
+ ls "${NETCONS_PATH}/userdata/" >&2
+ exit "${ksft_fail}"
+ fi
+}
+
+# ========== #
+# Start here #
+# ========== #
+
+modprobe netdevsim 2> /dev/null || true
+modprobe netconsole 2> /dev/null || true
+
+# Check for basic system dependency and exit if not found
+check_for_dependencies
+
+# Remove the namespace, interfaces and netconsole target on exit
+trap cleanup EXIT
+# Create one namespace and two interfaces
+set_network
+# Create a dynamic target for netconsole
+create_dynamic_target
+# populate the maximum number of supported keys in userdata
+create_userdata_max_entries
+# Verify an additional entry is not allowed
+verify_entry_limit
+exit "${ksft_pass}"
--
2.43.5
^ permalink raw reply related [flat|nested] 7+ messages in thread