linux-kernel - Re: [PATCH net v7 4/4] selftest: netcons: add test for netconsole over bonded interfaces

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <m2dwihyj3vddvipam555ewxej663brejyv5gdnsw4ks5mis2y7@2mu2gus2o7ys>
Date: Wed, 8 Oct 2025 06:02:56 -0700
From: Breno Leitao <leitao@...ian.org>
To: Paolo Abeni <pabeni@...hat.com>
Cc: 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>, 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

Hello Paolo,

On Tue, Oct 07, 2025 at 11:47:22AM +0200, Paolo Abeni wrote:
> On 10/3/25 1:57 PM, Breno Leitao wrote:

> > +# 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).

Thanks for the review. I misunderstood what Jakub said, sorry about it.

I've tried to enslave a veth interface manually into a bonding
interface, and I can see:

	# ip link set veth0 master bond_tx_78
	 aborting
	 RTNETLINK answers: Device or resource busy
	
and dmesg shows:

	netpoll: (null): veth0 doesn't support polling,

If that is the test case that is missing, I will add it as an additional
test in the selftest, and send a new version.

> > +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.

Awesome. I am more than happy to create_netdevsim() in this selftest,
and move the others to use it as well.

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

Sure, that is doable, but, I need to change a few common helpers, to
start netconsole from inside the "tx namespace" instead of the default
namespace.

Given all the other netconsole selftest uses TX from the default net
namespace, I would like to move them at all the same time.

Do you think it is Ok to have this test using TX interfaces from the
main net namespace (as is now), and then I submit a follow patch to
migrate all the netcons tests (including this one) to use a TX
namespace? Then I can change the helpers at the same time, simplifying
the code review.

> > +	# 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.

Ack. I will update. Thanks

> > +# 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.

Ack!

Thanks for the review,
--breno