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 + 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} + # Just wait for 2 seconds + timeout 2 ip netns exec "${NAMESPACE}" socat UDP-LISTEN:"${PORT}",fork "${OUTPUT}" +} + +function validate_result() { + 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." + 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}
On Tue, 13 Aug 2024 11:38:16 -0700 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.
We're getting a:
SKIP: directory /sys/kernel/config/netconsole does not exist. Check if NETCONSOLE_DYNAMIC is enabled
https://netdev-3.bots.linux.dev/vmksft-net-drv-dbg/results/726381/4-netcons-...
Gotta extend tools/testing/selftests/drivers/net/config
Hello Jakub,
On Tue, Aug 13, 2024 at 03:37:16PM -0700, Jakub Kicinski wrote:
On Tue, 13 Aug 2024 11:38:16 -0700 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.
We're getting a:
SKIP: directory /sys/kernel/config/netconsole does not exist. Check if NETCONSOLE_DYNAMIC is enabled
https://netdev-3.bots.linux.dev/vmksft-net-drv-dbg/results/726381/4-netcons-...
Gotta extend tools/testing/selftests/drivers/net/config
Thanks, I've added the following changes in tools/testing/selftests/drivers/net/config:
CONFIG_NETCONSOLE=m CONFIG_NETCONSOLE_DYNAMIC=y
Then I was able to get the test executing running the following commands:
# vng --build --config tools/testing/selftests/net/config # vng -v --run . --user root --cpus 4 -- \ make -C tools/testing/selftests TARGETS=drivers/net \ TEST_PROGS="netcons_basic.sh" TEST_GEN_PROGS="" run_tests ..... # timeout set to 45 # selftests: drivers/net: netcons_basic.sh [ 11.172987] netdevsim netdevsim407 eni407np1: renamed from eth1 [ 12.441965] printk: legacy console [netcon_ext0] enabled [ 12.443049] netpoll: netconsole: local port 6665 [ 12.444104] netpoll: netconsole: local IPv4 address 192.168.1.1 [ 12.445281] netpoll: netconsole: interface 'eni407np1' [ 12.446299] netpoll: netconsole: remote port 6666 [ 12.447246] netpoll: netconsole: remote IPv4 address 192.168.1.2 [ 12.448419] netpoll: netconsole: remote ethernet address aa:c8:83:a5:05:b3 [ 12.450646] netconsole: network logging started [ 13.547405] netconsole selftest: netcons_GjLI0 ok 1 selftests: drivers/net: netcons_basic.sh
I will wait a bit more, and then integrate this change into v2.
Thanks for the review --breno
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:
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"cleanupexit ${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." >&2return ${ksft_fail}
The indentation seems wrong here.
- fi
- if ! grep -q "${MSG}" "${TMPFILENAME}"; then
echo "FAIL: ${MSG} not found in ${TMPFILENAME}" >&2cat "${TMPFILENAME}" >&2return ${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" >&2exit "${ksft_skip}"- fi
- if ! which socat > /dev/null ; then
echo "SKIP: socat(1) is not available" >&2exit "${ksft_skip}"- fi
- if ! which ip > /dev/null ; then
echo "SKIP: ip(1) is not available" >&2exit "${ksft_skip}"- fi
- if ! which udevadm > /dev/null ; then
echo "SKIP: udevadm(1) is not available" >&2exit "${ksft_skip}"- fi
- if [ ! -d "${NETCONS_CONFIGFS}" ]; then
echo "SKIP: directory ${NETCONS_CONFIGFS} does not exist. Check if NETCONSOLE_DYNAMIC is enabled" >&2exit "${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}
Hi Petr, Breno,
On 14/08/2024 12:24, Petr Machata wrote:
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.
(...)
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 @@
(...)
+NAMESPACE="netconsns_dst"
(...)
+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)"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.
Thank you for having CCed me.
I don't know if lib.sh needs to be modified: setup_ns() is supposed to be called with the name of an existing variable. Can you not define this variable before?
I mean: the modification from Petr looks good to me to support 'set -u', but it sounds safer to define the variable before in the script, just in case it is defined by in the environment, before starting the test, and not taking the expected path.
Note that in all the other selftests, setup_ns() is called with the name of the variable, not a variable like you did, e.g.
NAMESPACE= setup_ns NAMESPACE
instead of:
NAMESPACE="netconsns_dst" setup_ns "${NAMESPACE}" NAMESPACE=${NS_LIST[0]}
Maybe better to do like the others?
Cheers, Matt
On Wed, Aug 14, 2024 at 12:24:46PM +0200, Petr Machata wrote:
Breno Leitao leitao@debian.org writes:
- fi
- if ! grep -q "${MSG}" "${TMPFILENAME}"; then
echo "FAIL: ${MSG} not found in ${TMPFILENAME}" >&2cat "${TMPFILENAME}" >&2return ${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().
Thanks for all the other feedbacks, all of them make sense.
Regarding this one, I kept like this, because I only remove the file if the test succeed, otherwise I keep the file here for debugging purposes, as described in the comment above.
If that is not a good practice, I am more than happy to move this to cleanup.
Thanks for the detailed review, --breno
On Tue, Aug 13, 2024 at 11:38:16AM -0700, 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:
v2:
- Change the location of the path (Jakub)
- Move from veth to netdevsim
- Other small changes in dependency checks and cleanup
v1:
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
If you want to source net/lib.sh, you need to add it to Makefile. e.g.
TEST_INCLUDES := ../../../net/lib.sh
See example in tools/testing/selftests/drivers/net/bonding/Makefile
Thanks Hangbin
On Thu, Aug 15, 2024 at 03:56:36PM +0800, Hangbin Liu wrote:
On Tue, Aug 13, 2024 at 11:38:16AM -0700, 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:
v2:
- Change the location of the path (Jakub)
- Move from veth to netdevsim
- Other small changes in dependency checks and cleanup
v1:
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
If you want to source net/lib.sh, you need to add it to Makefile. e.g.
TEST_INCLUDES := ../../../net/lib.sh
See example in tools/testing/selftests/drivers/net/bonding/Makefile
Thanks. I will update. --breno
linux-kselftest-mirror@lists.linaro.org
-
Breno Leitao -
Hangbin Liu -
Jakub Kicinski -
Matthieu Baerts -
Petr Machata