* [PATCH net-next 1/4] netconsole: Warn if MAX_USERDATA_ITEMS limit is exceeded
2024-12-04 16:40 [PATCH net-next 0/4] netconsole: selftest for userdata overflow Breno Leitao
@ 2024-12-04 16:40 ` Breno Leitao
2024-12-06 15:11 ` Simon Horman
2024-12-04 16:40 ` [PATCH net-next 2/4] netconsole: selftest: Split the helpers from the selftest Breno Leitao
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Breno Leitao @ 2024-12-04 16:40 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
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>
---
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 4ea44a2f48f7b1f9059d275f0f0edc40cc1997f0..8b9dd4842f3e516c7eaa08205a45092e64417440 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -705,7 +705,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] 10+ messages in thread* Re: [PATCH net-next 1/4] netconsole: Warn if MAX_USERDATA_ITEMS limit is exceeded
2024-12-04 16:40 ` [PATCH net-next 1/4] netconsole: Warn if MAX_USERDATA_ITEMS limit is exceeded Breno Leitao
@ 2024-12-06 15:11 ` Simon Horman
0 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2024-12-06 15:11 UTC (permalink / raw)
To: Breno Leitao
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Shuah Khan, netdev, linux-kernel, linux-kselftest
On Wed, Dec 04, 2024 at 08:40:42AM -0800, Breno Leitao wrote:
> 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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net-next 2/4] netconsole: selftest: Split the helpers from the selftest
2024-12-04 16:40 [PATCH net-next 0/4] netconsole: selftest for userdata overflow Breno Leitao
2024-12-04 16:40 ` [PATCH net-next 1/4] netconsole: Warn if MAX_USERDATA_ITEMS limit is exceeded Breno Leitao
@ 2024-12-04 16:40 ` Breno Leitao
2024-12-06 15:11 ` Simon Horman
2024-12-04 16:40 ` [PATCH net-next 3/4] netconsole: selftest: Delete all userdata keys Breno Leitao
2024-12-04 16:40 ` [PATCH net-next 4/4] netconsole: selftest: verify userdata entry limit Breno Leitao
3 siblings, 1 reply; 10+ messages in thread
From: Breno Leitao @ 2024-12-04 16:40 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
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>
---
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 0456a33ef65792bacb5d305a6384d245844fb743..8af5c9a28e68c4b6a785e2e6b82db20b3cf59822 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16091,6 +16091,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] 10+ messages in thread* Re: [PATCH net-next 2/4] netconsole: selftest: Split the helpers from the selftest
2024-12-04 16:40 ` [PATCH net-next 2/4] netconsole: selftest: Split the helpers from the selftest Breno Leitao
@ 2024-12-06 15:11 ` Simon Horman
0 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2024-12-06 15:11 UTC (permalink / raw)
To: Breno Leitao
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Shuah Khan, netdev, linux-kernel, linux-kselftest
On Wed, Dec 04, 2024 at 08:40:43AM -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.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
Tested-by: Simon Horman <horms@kernel.org>
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net-next 3/4] netconsole: selftest: Delete all userdata keys
2024-12-04 16:40 [PATCH net-next 0/4] netconsole: selftest for userdata overflow Breno Leitao
2024-12-04 16:40 ` [PATCH net-next 1/4] netconsole: Warn if MAX_USERDATA_ITEMS limit is exceeded Breno Leitao
2024-12-04 16:40 ` [PATCH net-next 2/4] netconsole: selftest: Split the helpers from the selftest Breno Leitao
@ 2024-12-04 16:40 ` Breno Leitao
2024-12-06 15:11 ` Simon Horman
2024-12-04 16:40 ` [PATCH net-next 4/4] netconsole: selftest: verify userdata entry limit Breno Leitao
3 siblings, 1 reply; 10+ messages in thread
From: Breno Leitao @ 2024-12-04 16:40 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
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>
---
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] 10+ messages in thread* Re: [PATCH net-next 3/4] netconsole: selftest: Delete all userdata keys
2024-12-04 16:40 ` [PATCH net-next 3/4] netconsole: selftest: Delete all userdata keys Breno Leitao
@ 2024-12-06 15:11 ` Simon Horman
0 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2024-12-06 15:11 UTC (permalink / raw)
To: Breno Leitao
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Shuah Khan, netdev, linux-kernel, linux-kselftest
On Wed, Dec 04, 2024 at 08:40:44AM -0800, Breno Leitao wrote:
> 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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net-next 4/4] netconsole: selftest: verify userdata entry limit
2024-12-04 16:40 [PATCH net-next 0/4] netconsole: selftest for userdata overflow Breno Leitao
` (2 preceding siblings ...)
2024-12-04 16:40 ` [PATCH net-next 3/4] netconsole: selftest: Delete all userdata keys Breno Leitao
@ 2024-12-04 16:40 ` Breno Leitao
2024-12-06 15:09 ` Simon Horman
3 siblings, 1 reply; 10+ messages in thread
From: Breno Leitao @ 2024-12-04 16:40 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
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>
---
MAINTAINERS | 2 +-
.../selftests/drivers/net/netcons_overflow.sh | 67 ++++++++++++++++++++++
2 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 8af5c9a28e68c4b6a785e2e6b82db20b3cf59822..62192db4641a4056d1eab911f5c141fb37eaed36 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16092,7 +16092,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/netcons_overflow.sh b/tools/testing/selftests/drivers/net/netcons_overflow.sh
new file mode 100755
index 0000000000000000000000000000000000000000..a19f613553578dc185b7332a827463d9b0c6685f
--- /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] 10+ messages in thread* Re: [PATCH net-next 4/4] netconsole: selftest: verify userdata entry limit
2024-12-04 16:40 ` [PATCH net-next 4/4] netconsole: selftest: verify userdata entry limit Breno Leitao
@ 2024-12-06 15:09 ` Simon Horman
2025-01-03 11:48 ` Breno Leitao
0 siblings, 1 reply; 10+ messages in thread
From: Simon Horman @ 2024-12-06 15:09 UTC (permalink / raw)
To: Breno Leitao
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Shuah Khan, netdev, linux-kernel, linux-kselftest
On Wed, Dec 04, 2024 at 08:40:45AM -0800, Breno Leitao wrote:
> 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>
> ---
> MAINTAINERS | 2 +-
> .../selftests/drivers/net/netcons_overflow.sh | 67 ++++++++++++++++++++++
> 2 files changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8af5c9a28e68c4b6a785e2e6b82db20b3cf59822..62192db4641a4056d1eab911f5c141fb37eaed36 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16092,7 +16092,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/netcons_overflow.sh b/tools/testing/selftests/drivers/net/netcons_overflow.sh
Nit: I think you need to add netcons_overflow.sh to
tools/testing/selftests/drivers/net/Makefile
Other than that, this looks good to me.
Tested-by: Simon Horman <horms@kernel.org>
Reviewed-by: Simon Horman <horms@kernel.org>
...
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net-next 4/4] netconsole: selftest: verify userdata entry limit
2024-12-06 15:09 ` Simon Horman
@ 2025-01-03 11:48 ` Breno Leitao
0 siblings, 0 replies; 10+ messages in thread
From: Breno Leitao @ 2025-01-03 11:48 UTC (permalink / raw)
To: Simon Horman
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Shuah Khan, netdev, linux-kernel, linux-kselftest
Hello Simon,
On Fri, Dec 06, 2024 at 03:09:18PM +0000, Simon Horman wrote:
> On Wed, Dec 04, 2024 at 08:40:45AM -0800, Breno Leitao wrote:
> > 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>
> > ---
> > MAINTAINERS | 2 +-
> > .../selftests/drivers/net/netcons_overflow.sh | 67 ++++++++++++++++++++++
> > 2 files changed, 68 insertions(+), 1 deletion(-)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 8af5c9a28e68c4b6a785e2e6b82db20b3cf59822..62192db4641a4056d1eab911f5c141fb37eaed36 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -16092,7 +16092,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/netcons_overflow.sh b/tools/testing/selftests/drivers/net/netcons_overflow.sh
>
> Nit: I think you need to add netcons_overflow.sh to
> tools/testing/selftests/drivers/net/Makefile
Good catch. I will fix it and send a v2.
Thanks for the review,
--breno
^ permalink raw reply [flat|nested] 10+ messages in thread