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