Message-ID: <e6764450-b0f8-4f50-b761-6321dfe2ad71@redhat.com>
Date: Tue, 7 Oct 2025 11:47:22 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Breno Leitao <leitao@...ian.org>, Andrew Lunn <andrew+netdev@...n.ch>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Shuah Khan <shuah@...nel.org>,
 Simon Horman <horms@...nel.org>, david decotigny <decot@...glers.com>
Cc: linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
 linux-kselftest@...r.kernel.org, asantostc@...il.com, efault@....de,
 calvin@...nvd.org, kernel-team@...a.com, jv@...sburgh.net
Subject: Re: [PATCH net v7 4/4] selftest: netcons: add test for netconsole
 over bonded interfaces

On 10/3/25 1:57 PM, Breno Leitao wrote:
> +# Test #1 : Create an bonding interface and attach netpoll into
> +# the bonding interface. Netconsole/netpoll should work on
> +# the bonding interface.
> +send_netcons_msg_through_bond_iface
> +echo "test #1: netpoll on bonding interface worked. Test passed" >&2
> +
> +# Test #2: Attach netpoll to an enslaved interface
> +# Try to attach netpoll to an enslaved sub-interface (while still being part of
> +# a bonding interface), which shouldn't be allowed
> +enable_netpoll_on_enslaved_iface
> +echo "test #2: netpoll correctly rejected enslaved interface (expected behavior). Test passed." >&2
> +
> +# Test #3: Unplug the sub-interface from bond and enable netconsole
> +# Detach the interface from a bonding interface and attach netpoll again
> +delete_bond_and_reenable_target
> +echo "test #3: Able to attach to an unbound interface. Test passed." >&2
> +
> +# Test #4: Enslave a sub-interface that had netconsole enabled
> +# Try to enslave an interface that has netconsole/netpoll enabled.
> +# Previous test has netconsole enabled in BOND_TX1_SLAVE_IF, try to enslave it
> +enslave_netcons_enabled_iface
> +echo "test #4: Enslaving an interface with netpoll attached. Test passed." >&2
> +
> +# Test #5: Enslave a sub-interface to a bonding interface
> +# Enslave an interface to a bond interface that has netpoll attached
> +# At this stage, BOND_TX_MAIN_IF is created and BOND_TX1_SLAVE_IF is part of
> +# it. Netconsole is currently disabled
> +enslave_iface_to_bond
> +echo "test #5: Enslaving an interface to bond+netpoll. Test passed." >&2

I think this is missing the negative/fail to add test case asked by
Jakub. AFAICS you should be able to trigger such case trying to add a
veth device to the netpoll enabled bond (since the latter carries the
IFF_DISABLE_NETPOLL priv_flag).

> +
> +cleanup_bond
> +trap - EXIT
> +exit "${EXIT_STATUS}"
> 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 9b5ef8074440c..30e4f357b47e9 100644
> --- a/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh
> +++ b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh
> @@ -28,17 +28,24 @@ NETCONS_PATH="${NETCONS_CONFIGFS}"/"${TARGET}"
>  # NAMESPACE will be populated by setup_ns with a random value
>  NAMESPACE=""
>  
> -# IDs for netdevsim
> +# IDs for netdevsim. We either use NSIM_DEV_{1,2}_ID for standard test
> +# or NSIM_BOND_{T,R}X_{1,2} for the bonding tests. Not both at the
> +# same time.
>  NSIM_DEV_1_ID=$((256 + RANDOM % 256))
>  NSIM_DEV_2_ID=$((512 + RANDOM % 256))
> +NSIM_BOND_TX_1=$((768 + RANDOM % 256))
> +NSIM_BOND_TX_2=$((1024 + RANDOM % 256))
> +NSIM_BOND_RX_1=$((1280 + RANDOM % 256))
> +NSIM_BOND_RX_2=$((1536 + RANDOM % 256))
>  NSIM_DEV_SYS_NEW="/sys/bus/netdevsim/new_device"
> +NSIM_DEV_SYS_LINK="/sys/bus/netdevsim/link_device"
> +NSIM_DEV_SYS_DEL="/sys/bus/netdevsim/del_device"
>  
>  # Used to create and delete namespaces
>  source "${LIBDIR}"/../../../../net/lib.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
> @@ -54,7 +61,6 @@ create_ifaces() {
>  }
>  
>  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)
>  
> @@ -96,6 +102,33 @@ function select_ipv4_or_ipv6()
>  	fi
>  }
>  
> +# Create 4 netdevsim interfaces. Two of them will be bound to TX bonding iface
> +# and the other two will be bond to the RX interface (on the other namespace)
> +function create_ifaces_bond() {
> +	echo "$NSIM_BOND_TX_1" > "$NSIM_DEV_SYS_NEW"
> +	echo "$NSIM_BOND_TX_2" > "$NSIM_DEV_SYS_NEW"
> +	echo "$NSIM_BOND_RX_1" > "$NSIM_DEV_SYS_NEW"
> +	echo "$NSIM_BOND_RX_2" > "$NSIM_DEV_SYS_NEW"
> +	udevadm settle 2> /dev/null || true
> +
> +	local BOND_TX1=/sys/bus/netdevsim/devices/netdevsim"$NSIM_BOND_TX_1"
> +	local BOND_TX2=/sys/bus/netdevsim/devices/netdevsim"$NSIM_BOND_TX_2"
> +	local BOND_RX1=/sys/bus/netdevsim/devices/netdevsim"$NSIM_BOND_RX_1"
> +	local BOND_RX2=/sys/bus/netdevsim/devices/netdevsim"$NSIM_BOND_RX_2"

Note that with the create_netdevsim() helper from
tools/testing/selftests/net/lib.sh you could create the netdevsim device
directly in the target namespace and avoid some duplicate code.

It would be probably safer to create both rx and tx devices in child
namespaces.

> +
> +	# TX
> +	BOND_TX1_SLAVE_IF=$(find "$BOND_TX1"/net -maxdepth 1 -type d ! \
> +		-path "$BOND_TX1"/net -exec basename {} \; | grep -v net)
> +	BOND_TX2_SLAVE_IF=$(find "$BOND_TX2"/net -maxdepth 1 -type d ! \
> +		-path "$BOND_TX2"/net -exec basename {} \; | grep -v net)
> +
> +	# RX
> +	BOND_RX1_SLAVE_IF=$(find "$BOND_RX1"/net -maxdepth 1 -type d ! \
> +		-path "$BOND_RX1"/net -exec basename {} \; | grep -v net)
> +	BOND_RX2_SLAVE_IF=$(find "$BOND_RX2"/net -maxdepth 1 -type d ! \
> +		-path "$BOND_RX2"/net -exec basename {} \; | grep -v net)
> +}
> +
>  function set_network() {
>  	local IP_VERSION=${1:-"ipv4"}
>  
> @@ -180,8 +213,6 @@ function disable_release_append() {
>  }
>  
>  function do_cleanup() {
> -	local NSIM_DEV_SYS_DEL="/sys/bus/netdevsim/del_device"
> -
>  	# Delete netdevsim devices
>  	echo "$NSIM_DEV_2_ID" > "$NSIM_DEV_SYS_DEL"
>  	echo "$NSIM_DEV_1_ID" > "$NSIM_DEV_SYS_DEL"
> @@ -193,14 +224,26 @@ function do_cleanup() {
>  	echo "${DEFAULT_PRINTK_VALUES}" > /proc/sys/kernel/printk
>  }
>  
> -function cleanup() {
> +function cleanup_netcons() {
>  	# delete netconsole dynamic reconfiguration
> -	echo 0 > "${NETCONS_PATH}"/enabled
> +	# do not fail if the target is already disabled
> +	if [[ ! -d "${NETCONS_PATH}" ]]
> +	then
> +		# in some cases this is called before netcons path is created
> +		return
> +	fi
> +	if [[ $(cat "${NETCONS_PATH}"/enabled) != 0 ]]
> +	then
> +		echo 0 > "${NETCONS_PATH}"/enabled || true
> +	fi
>  	# 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}"
> +}
>  
> +function cleanup() {
> +	cleanup_netcons
>  	do_cleanup
>  }
>  
> @@ -377,3 +420,104 @@ function wait_for_port() {
>  	# more frequently on IPv6
>  	sleep 1
>  }
> +
> +# netdevsim link BOND_TX to BOND_RX interfaces
> +function link_ifaces_bond() {
> +	local BOND_TX1_SLAVE_IFIDX
> +	local BOND_TX2_SLAVE_IFIDX
> +	local BOND_RX1_SLAVE_IFIDX
> +	local BOND_RX2_SLAVE_IFIDX
> +
> +	BOND_TX1_SLAVE_IFIDX=$(cat /sys/class/net/"$BOND_TX1_SLAVE_IF"/ifindex)
> +	BOND_TX2_SLAVE_IFIDX=$(cat /sys/class/net/"$BOND_TX2_SLAVE_IF"/ifindex)
> +	BOND_RX1_SLAVE_IFIDX=$(cat /sys/class/net/"$BOND_RX1_SLAVE_IF"/ifindex)
> +	BOND_RX2_SLAVE_IFIDX=$(cat /sys/class/net/"$BOND_RX2_SLAVE_IF"/ifindex)
> +
> +	exec {NAMESPACE_FD}</var/run/netns/"${NAMESPACE}"
> +	exec {INITNS_FD}</proc/self/ns/net
> +
> +	# Bind the dst interfaces to namespace
> +	ip link set "${BOND_RX1_SLAVE_IF}" netns "${NAMESPACE}"
> +	ip link set "${BOND_RX2_SLAVE_IF}" netns "${NAMESPACE}"
> +
> +	# Linking TX ifaces to the RX ones (on the other namespace)
> +	echo "${INITNS_FD}:$BOND_TX1_SLAVE_IFIDX $NAMESPACE_FD:$BOND_RX1_SLAVE_IFIDX"  \
> +		> "$NSIM_DEV_SYS_LINK"
> +	echo "${INITNS_FD}:$BOND_TX2_SLAVE_IFIDX $NAMESPACE_FD:$BOND_RX2_SLAVE_IFIDX"  \
> +		> "$NSIM_DEV_SYS_LINK"
> +}
> +
> +# Create "bond_tx_XX" and "bond_rx_XX" interfaces, and set DSTIF and SRCIF with
> +# the bonding interfaces
> +function setup_bonding_ifaces() {
> +	local RAND=$(( RANDOM % 100 ))
> +	BOND_TX_MAIN_IF="bond_tx_$RAND"
> +	BOND_RX_MAIN_IF="bond_rx_$RAND"
> +
> +	if ! ip link add "${BOND_TX_MAIN_IF}" type bond mode balance-rr
> +	then
> +		echo "Failed to create bond TX interface. Is CONFIG_BONDING set?" >&2
> +		# only clean nsim ifaces and namespace. Nothing else has been
> +		# initialized
> +		cleanup_bond_nsim
> +		trap - EXIT
> +		exit "${ksft_skip}"
> +	fi
> +	ip link set "${BOND_TX1_SLAVE_IF}" down
> +	ip link set "${BOND_TX2_SLAVE_IF}" down
> +
> +	ip link set "${BOND_TX1_SLAVE_IF}" master "${BOND_TX_MAIN_IF}"
> +	ip link set "${BOND_TX2_SLAVE_IF}" master "${BOND_TX_MAIN_IF}"
> +	ip link set "${BOND_TX_MAIN_IF}" up
> +
> +	# now create the RX bonding iface
> +	ip netns exec "${NAMESPACE}" \
> +		ip link add "${BOND_RX_MAIN_IF}" type bond mode balance-rr

Minor nit:

	ip -n "${NAMESPACE}" link ...

will yield the same result with a little less wording.


> +	ip netns exec "${NAMESPACE}" \
> +		ip link set "${BOND_RX1_SLAVE_IF}" down
> +	ip netns exec "${NAMESPACE}" \
> +		ip link set "${BOND_RX2_SLAVE_IF}" down
> +
> +	ip netns exec "${NAMESPACE}" \
> +		ip link set "${BOND_RX1_SLAVE_IF}" master "${BOND_RX_MAIN_IF}"
> +	ip netns exec "${NAMESPACE}" \
> +		ip link set "${BOND_RX2_SLAVE_IF}" master "${BOND_RX_MAIN_IF}"
> +	ip netns exec "${NAMESPACE}" \
> +		ip link set "${BOND_RX_MAIN_IF}" up
> +	ip netns exec "${NAMESPACE}" \
> +		ip link set "${BOND_RX1_SLAVE_IF}" up
> +	ip netns exec "${NAMESPACE}" \
> +		ip link set "${BOND_RX2_SLAVE_IF}" up
> +}
> +
> +# Clean up netdevsim ifaces created for bonding test
> +function cleanup_bond_nsim() {
> +	echo "$NSIM_BOND_TX_1" > "$NSIM_DEV_SYS_DEL"
> +	echo "$NSIM_BOND_TX_2" > "$NSIM_DEV_SYS_DEL"
> +	echo "$NSIM_BOND_RX_1" > "$NSIM_DEV_SYS_DEL"
> +	echo "$NSIM_BOND_RX_2" > "$NSIM_DEV_SYS_DEL"
> +	cleanup_all_ns

If all devices are created in child netns, you will not need explicit
per device cleanup.

/P

Powered by blists - more mailing lists