[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87r0arl5qw.fsf@nvidia.com>
Date: Wed, 14 Aug 2024 12:24:46 +0200
From: Petr Machata <petrm@...dia.com>
To: Breno Leitao <leitao@...ian.org>
CC: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
<pabeni@...hat.com>, Shuah Khan <shuah@...nel.org>, <netdev@...r.kernel.org>,
David Wei <dw@...idwei.uk>, Willem de Bruijn <willemb@...gle.com>, "Petr
Machata" <petrm@...dia.com>, open list <linux-kernel@...r.kernel.org>, "open
list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>, "Geliang
Tang" <geliang@...nel.org>, Hangbin Liu <liuhangbin@...il.com>, "Matthieu
Baerts" <matttbe@...nel.org>
Subject: Re: [PATCH net-next v2] net: netconsole: selftests: Create a new
netconsole selftest
Breno Leitao <leitao@...ian.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@...ian.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@...ian.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@...nel.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@...ian.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@...nel.org>, Hangbin Liu <liuhangbin@...il.com>,
Matthieu Baerts (NGI0) <matttbe@...nel.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}
Powered by blists - more mailing lists