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:
v3: * Defined CONFIGs in config file (Jakub) * Identention fixes (Petr Machata) * Use setup_ns in a better way (Matthieu Baerts) * Add dependencies in TEST_INCLUDES (Hangbin Liu)
v2: * Change the location of the path (Jakub) * Move from veth to netdevsim * Other small changes in dependency checks and cleanup * https://lore.kernel.org/all/20240813183825.837091-1-leitao@debian.org/
v1: * https://lore.kernel.org/all/ZqyUHN770pjSofTC@gmail.com/
MAINTAINERS | 1 + tools/testing/selftests/drivers/net/Makefile | 4 +- tools/testing/selftests/drivers/net/config | 2 + .../selftests/drivers/net/netcons_basic.sh | 226 ++++++++++++++++++ 4 files changed, 232 insertions(+), 1 deletion(-) create mode 100755 tools/testing/selftests/drivers/net/netcons_basic.sh
diff --git a/MAINTAINERS b/MAINTAINERS index 7b291c3a9aa4..9f0be36939f9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15772,6 +15772,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..8bee2e94049b 100644 --- a/tools/testing/selftests/drivers/net/Makefile +++ b/tools/testing/selftests/drivers/net/Makefile @@ -1,8 +1,10 @@ # SPDX-License-Identifier: GPL-2.0
-TEST_INCLUDES := $(wildcard lib/py/*.py) +TEST_INCLUDES := $(wildcard lib/py/*.py) \ + ../../net/lib.sh \
TEST_PROGS := \ + netcons_basic.sh \ ping.py \ queues.py \ stats.py \ diff --git a/tools/testing/selftests/drivers/net/config b/tools/testing/selftests/drivers/net/config index f6a58ce8a230..8f46112263f0 100644 --- a/tools/testing/selftests/drivers/net/config +++ b/tools/testing/selftests/drivers/net/config @@ -1,2 +1,4 @@ CONFIG_IPV6=y CONFIG_NETDEVSIM=m +CONFIG_NETCONSOLE=m +CONFIG_NETCONSOLE_DYNAMIC=y 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..929f27a0fd9c --- /dev/null +++ b/tools/testing/selftests/drivers/net/netcons_basic.sh @@ -0,0 +1,226 @@ +#!/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 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} + 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 + 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 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() { + 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" + + # Check if the file exists + if [ ! -f "$TMPFILENAME" ]; then + echo "FAIL: File was not generated." >&2 + return ${ksft_fail} + 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}" + 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." >&2 + exit "${ksft_skip}" + fi +} + +# ========== # +# Start here # +# ========== # +modprobe netdevsim || true +modprobe netconsole || 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}
Hi Breno,
On 15/08/2024 11:51, Breno Leitao wrote:
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:
v3:
- Defined CONFIGs in config file (Jakub)
- Identention fixes (Petr Machata)
- Use setup_ns in a better way (Matthieu Baerts)
- Add dependencies in TEST_INCLUDES (Hangbin Liu)
Thank you for the v3!
I only looked here at how 'setup_ns' was used, (and a few other Bash-related stuff), but not at the test itself.
I have a few comments, but I don't consider them as blocking if you prefer to continue with the current version.
(...)
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..929f27a0fd9c --- /dev/null +++ b/tools/testing/selftests/drivers/net/netcons_basic.sh @@ -0,0 +1,226 @@
(...)
+# This will have some tmp values appended to it in set_network() +NAMESPACE="netconsns_dst"
nit: the comment is no longer correct: if this variable is set before being used with setup_ns, the netns will not have a unique name, but it will use the one defined here. Maybe not what you want?
See this code from lib.sh where "ns_name" is "NAMESPACE":
# Some test may setup/remove same netns multi times if [ -z "${!ns_name}" ]; then eval "${ns_name}=${ns_name,,}-$(mktemp -u XXXXXX)" else cleanup_ns "${!ns_name}" fi
So it will not set a new value, but it will try to clean any netns with this "netconsns_dst" name. I guess that's fine, but maybe you prefer to do like the others and simply define "NAMESPACE" to an empty string?
(...)
+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
Because of the 'set -e' defined above, I guess the script will stop just before in case of error, no? Maybe better with:
if ! echo "(...)" > $NSIM_DEV_SYS_LINK; then
(note that shellcheck should help to spot such issues I think)
echo "linking netdevsim1 with netdevsim2 should succeed"cleanupexit ${ksft_skip}- fi
+}
(...)
+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"
- # Check if the file exists
- if [ ! -f "$TMPFILENAME" ]; then
echo "FAIL: File was not generated." >&2return ${ksft_fail}- fi
- if ! grep -q "${MSG}" "${TMPFILENAME}"; then
echo "FAIL: ${MSG} not found in ${TMPFILENAME}" >&2cat "${TMPFILENAME}" >&2- return ${ksft_fail}
nit: a tab is missing here.
- fi
- # Delete the file once it is validated, otherwise keep it
- # for debugging purposes
- rm "${TMPFILENAME}"
- return ${ksft_pass}
+}
(...)
+# ========== # +# Start here # +# ========== # +modprobe netdevsim || true +modprobe netconsole || true
If errors can be expected, maybe clearer to mute stderr, not to confuse the people reading the logs?
Same above with 'udevadm settle || 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
I guess that's fine as it is, but it is often better to avoid a sleep with a "random" value: CI can be very slow, e.g. when running without KVM and/or with a debug kernel config. Here, wait_local_port_listen() from net_helper.sh could probably be used. The script will then probably wait less than 1 second.
+# Send the message +echo "${MSG}: ${TARGET}" > /dev/kmsg +# Wait until socat saves the file to disk +sleep 1
For here, I'm not sure, but 'busywait()' could be used, waiting for the OUTPUT_FILE to have a non 0 size?
If you do that, you can maybe increase the timeout you used above, to support very slow environments.
But if you prefer, I guess you can also leave things like they are and see if CIs are complaining (but these errors might not be easy to debug).
+# Make sure the message was received in the dst part +validate_result "${OUTPUT_FILE}" +ret=$?
Here as well, because of 'set -e', this line is probably useless.
validate_result "${OUTPUT_FILE}" || ret=$?
(or exit directly from validate_result() )
+exit ${ret}
Cheers, Matt
Hello Matthieu,
On Thu, Aug 15, 2024 at 12:47:22PM +0200, Matthieu Baerts wrote:
On 15/08/2024 11:51, Breno Leitao wrote:
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:
v3:
- Defined CONFIGs in config file (Jakub)
- Identention fixes (Petr Machata)
- Use setup_ns in a better way (Matthieu Baerts)
- Add dependencies in TEST_INCLUDES (Hangbin Liu)
Thank you for the v3!
I only looked here at how 'setup_ns' was used, (and a few other Bash-related stuff), but not at the test itself.
I have a few comments, but I don't consider them as blocking if you prefer to continue with the current version.
Thanks. I've adjusted all the suggestions you gave me, let me send a v4 and we can continue from there.
Thanks for the in-depth review. --breno
On Thu, 15 Aug 2024 02:51:44 -0700 Breno Leitao wrote:
+CONFIG_NETCONSOLE=m +CONFIG_NETCONSOLE_DYNAMIC=y
It's missing dependencies, configfs but maybe more. Please test the build as described here: https://github.com/linux-netdev/nipa/wiki/How-to-run-netdev-selftests-CI-sty...
linux-kselftest-mirror@lists.linaro.org
-
Breno Leitao -
Jakub Kicinski -
Matthieu Baerts