Re: [PATCH net-next v2] net: netconsole: selftests: Create a new netconsole selftest

Breno Leitao leitao@debian.org writes:

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@debian.org

Changelog:

v2:

• Change the location of the path (Jakub)
• Move from veth to netdevsim
• Other small changes in dependency checks and cleanup

v1:

• https://lore.kernel.org/all/ZqyUHN770pjSofTC@gmail.com/

MAINTAINERS | 1 + tools/testing/selftests/drivers/net/Makefile | 1 + .../selftests/drivers/net/netcons_basic.sh | 223 ++++++++++++++++++ 3 files changed, 225 insertions(+) create mode 100755 tools/testing/selftests/drivers/net/netcons_basic.sh

diff --git a/MAINTAINERS b/MAINTAINERS index a9dace908305..ded45f1dff7e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15770,6 +15770,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/netcons_basic.sh NETDEVSIM M: Jakub Kicinski kuba@kernel.org diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile index e54f382bcb02..928530b26abc 100644 --- a/tools/testing/selftests/drivers/net/Makefile +++ b/tools/testing/selftests/drivers/net/Makefile @@ -3,6 +3,7 @@ TEST_INCLUDES := $(wildcard lib/py/*.py) TEST_PROGS := \

• netcons_basic.sh \ ping.py \ queues.py \ stats.py \

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..e0e58fc7e89f --- /dev/null +++ b/tools/testing/selftests/drivers/net/netcons_basic.sh @@ -0,0 +1,223 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: GPL-2.0

+# This test creates two netdevsim virtual interfaces, assigns one of them (the +# "destination interface") to a new namespace, and assigns IP addresses to both +# interfaces. +# +# It listens on the destination interface using socat and configures a dynamic +# target on netconsole, pointing to the destination IP address. +# +# Finally, it checks whether the message was received properly on the +# destination interface. Note that this test may pollute the kernel log buffer +# (dmesg) and relies on dynamic configuration and namespaces being configured. +# +# Author: Breno Leitao leitao@debian.org

+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.168.1.1 +DSTIF="" # to be populated later +DSTIP=192.168.1.2

+PORT="6666" +MSG="netconsole selftest" +TARGET=$(mktemp -u netcons_XXXXX) +NETCONS_CONFIGFS="/sys/kernel/config/netconsole" +NETCONS_PATH="${NETCONS_CONFIGFS}"/"${TARGET}" +# This will have some tmp values appended to it in set_network() +NAMESPACE="netconsns_dst"

+# IDs for netdevsim +NSIM_DEV_1_ID=$((256 + RANDOM % 256)) +NSIM_DEV_2_ID=$((512 + RANDOM % 256))

+# Used to create and delete namespaces +source "${SCRIPTDIR}"/../../net/lib.sh

+# Create netdevsim interfaces +create_ifaces() {

• local NSIM_DEV_SYS_NEW=/sys/bus/netdevsim/new_device
• echo "$NSIM_DEV_2_ID" > "$NSIM_DEV_SYS_NEW"
• echo "$NSIM_DEV_1_ID" > "$NSIM_DEV_SYS_NEW"
• udevadm settle || true
• local NSIM_DEV_1_SYS=/sys/bus/netdevsim/devices/netdevsim"$NSIM_DEV_1_ID"
• local NSIM_DEV_2_SYS=/sys/bus/netdevsim/devices/netdevsim"$NSIM_DEV_2_ID"
• # These are global variables
• SRCIF=$(find "$NSIM_DEV_1_SYS"/net -maxdepth 1 -type d ! \

• -path "$NSIM_DEV_1_SYS"/net -exec basename {} \;)
  

• DSTIF=$(find "$NSIM_DEV_2_SYS"/net -maxdepth 1 -type d ! \

• -path "$NSIM_DEV_2_SYS"/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}
• echo "${INITNS_FD}:$SRCIF_IFIDX $NAMESPACE_FD:$DSTIF_IFIDX" > $NSIM_DEV_SYS_LINK
• if [ $? -ne 0 ]; 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() {

• # This is coming from lib.sh. And it does unbound variable access
• set +u
• setup_ns "${NAMESPACE}"
• set -u

It would make sense to fix lib.sh. I think this is what is needed?

modified tools/testing/selftests/net/lib.sh @@ -178,7 +178,7 @@ setup_ns() fi

# Some test may setup/remove same netns multi times - if [ -z "${!ns_name}" ]; then + if ! declare -p "$ns_name" &> /dev/null; then eval "${ns_name}=${ns_name,,}-$(mktemp -u XXXXXX)" else cleanup_ns "${!ns_name}"

CC'd Geliang Tang geliang@kernel.org, Hangbin Liu liuhangbin@gmail.com, Matthieu Baerts (NGI0) matttbe@kernel.org who were in the vicinity in the past.

• NAMESPACE=${NS_LIST[0]}
• # 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 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

+}

+function listen_port_and_save_to() {

• OUTPUT=${1}

local

• # Just wait for 2 seconds
• timeout 2 ip netns exec "${NAMESPACE}" socat UDP-LISTEN:"${PORT}",fork "${OUTPUT}"

+}

+function validate_result() {

• TMPFILENAME="$1"

local

• # Check if the file exists
• if [ ! -f "$TMPFILENAME" ]; then

•    echo "FAIL: File was not generated." >&2
  

•    return ${ksft_fail}
  

The indentation seems wrong here.

• fi
• if ! grep -q "${MSG}" "${TMPFILENAME}"; then

•    echo "FAIL: ${MSG} not found in ${TMPFILENAME}" >&2
  

•    cat "${TMPFILENAME}" >&2
  

•    return ${ksft_fail}
  

• fi
• # Delete the file once it is validated, otherwise keep it
• # for debugging purposes
• rm "${TMPFILENAME}"

Seeing the removal within the validation function is odd, I would expect it to be part of cleanup().

• return ${ksft_pass}

+}

+function check_for_dependencies() {

• if [ "$(id -u)" -ne 0 ]; then

• echo "This script 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 [ ! -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."
  

• exit "${ksft_skip}"
  

• fi

+}

+# ========== # +# Start here # +# ========== # +modprobe netdevsim || 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 +# 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 +# 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. +sleep 1 +# Send the message +echo "${MSG}: ${TARGET}" > /dev/kmsg +# Wait until socat saves the file to disk +sleep 1

+# Make sure the message was received in the dst part +validate_result "${OUTPUT_FILE}" +ret=$?

+exit ${ret}