* [PATCH ifupdown2] fix #8117: backport upstream fix for unavailable bond slaves
@ 2026-10-08 10:23 Gabriel Goller
2026-10-15 14:43 ` Christoph Heiss
0 siblings, 1 reply; 3+ messages in thread
From: Gabriel Goller @ 2026-10-08 10:23 UTC (permalink / raw)
To: pve-devel
A missing slave at boot can abort bond setup before available slaves are
enslaved, leaving the bond broken. Backport upstream PR #355 to continue
setup after single slave failures.
Upstream-Link: https://github.com/CumulusNetworks/ifupdown2/pull/355
Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=8117
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
Based on top of
https://lore.proxmox.com/pve-devel/20261008095634.77401-1-g.goller@proxmox.com/,
but not related -- just for convenience when applying.
debian/patches/series | 1 +
...rate-unavailable-slaves-during-setup.patch | 214 ++++++++++++++++++
2 files changed, 215 insertions(+)
create mode 100644 debian/patches/upstream/0005-bond-tolerate-unavailable-slaves-during-setup.patch
diff --git a/debian/patches/series b/debian/patches/series
index 45924c8d21dc..e9060c933868 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -18,3 +18,4 @@ pve/0014-nlmanager-read-ipv6-devconf-disable_ipv6-attribute-t.patch
pve/0015-revert-addons-bond-warn-if-sub-interface-is-detected-on-bond-slave.patch
upstream/0003-bond-preserve-mac-inherited-from-any-slave.patch
upstream/0004-bond-fix-list-index-out-of-range.patch
+upstream/0005-bond-tolerate-unavailable-slaves-during-setup.patch
diff --git a/debian/patches/upstream/0005-bond-tolerate-unavailable-slaves-during-setup.patch b/debian/patches/upstream/0005-bond-tolerate-unavailable-slaves-during-setup.patch
new file mode 100644
index 000000000000..38b58be10aec
--- /dev/null
+++ b/debian/patches/upstream/0005-bond-tolerate-unavailable-slaves-during-setup.patch
@@ -0,0 +1,214 @@
+From 93da1fe8338bad42f736047d478979d88944f0b4 Mon Sep 17 00:00:00 2001
+From: Gabriel Goller <g.goller@proxmox.com>
+Date: Thu, 8 Oct 2026 12:04:25 +0200
+Subject: [PATCH] bond: tolerate unavailable slaves during setup
+
+A missing slave at boot can leave an active-backup bond without any
+members, even when later configured slaves are available. PERFMODE
+bypasses the missing-device check, and the resulting exception aborts
+setup before those slaves are attempted:
+
+lib.nlcache.NetlinkError: netlink: nic0: cannot enslave link nic0 to bond0: operation failed with 'No such device' (19)
+
+Allow the available slaves to join regardless of an individual slave
+failure, including a device disappearing during setup, while still
+reporting the incomplete configuration.
+
+Fixes: #338
+Upstream-Link: https://github.com/CumulusNetworks/ifupdown2/pull/355
+Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
+
+Backport notes: adjust context for the retained slave-speed validation and
+PVE's reverted subinterface check; mock PVE's interface-alias translation
+in the tests. The functional changes are unchanged from upstream.
+---
+ ifupdown2/addons/bond.py | 56 ++++++++++++---------
+ tests/unit/test_bond.py | 103 +++++++++++++++++++++++++++++++++++++++
+ 2 files changed, 135 insertions(+), 24 deletions(-)
+ create mode 100644 tests/unit/test_bond.py
+
+diff --git a/ifupdown2/addons/bond.py b/ifupdown2/addons/bond.py
+index dcccf5ab..42709a15 100644
+--- a/ifupdown2/addons/bond.py
++++ b/ifupdown2/addons/bond.py
+@@ -432,39 +432,47 @@ def _add_slaves(self, ifaceobj, runningslaves, ifaceobj_getfunc=None):
+ devices_to_enslave.append(s)
+
+ for slave in devices_to_enslave:
+- if (not ifupdownflags.flags.PERFMODE and
+- not self.cache.link_exists(slave)):
+- self.log_error('%s: skipping slave %s, does not exist'
+- %(ifaceobj.name, slave), ifaceobj,
+- raise_error=False)
+- continue
++ # Missing slaves must not prevent the remaining slaves from
++ # joining the bond, including during boot in performance mode.
++ if not self.cache.link_exists(slave):
++ self.log_error('%s: skipping slave %s, does not exist'
++ % (ifaceobj.name, slave), ifaceobj,
++ raise_error=False)
++ continue
+
+ try:
+ # making sure the slave-to-be has the right speed
+ if not self.valid_slave_speed(ifaceobj, runningslaves, slave):
+ continue
+ except Exception as e:
+ self.logger.debug("%s: bond-slave (%s) speed validation failed: %s" % (ifaceobj.name, slave, str(e)))
+
+- link_up = False
+- if self.cache.link_is_up(slave):
+- self.netlink.link_down_force(slave)
+- link_up = True
+-
+- # if clag or ES bond: place the slave in a protodown state;
+- # (clagd will proto-up it when it is ready)
+- if clag_bond or ifaceobj.link_privflags & ifaceLinkPrivFlags.ES_BOND:
+- try:
+- self.netlink.link_set_protodown_on(slave)
+- if clag_bond:
+- self.iproute2.link_set_protodown_reason_clag_on(slave)
+- else:
+- self.iproute2.link_set_protodown_reason_frr_on(slave)
+- except Exception as e:
+- self.logger.error('%s: %s' % (ifaceobj.name, str(e)))
++ try:
++ link_up = False
++ if self.cache.link_is_up(slave):
++ self.netlink.link_down_force(slave)
++ link_up = True
++
++ # if clag or ES bond: place the slave in a protodown state;
++ # (clagd will proto-up it when it is ready)
++ if clag_bond or ifaceobj.link_privflags & ifaceLinkPrivFlags.ES_BOND:
++ try:
++ self.netlink.link_set_protodown_on(slave)
++ if clag_bond:
++ self.iproute2.link_set_protodown_reason_clag_on(slave)
++ else:
++ self.iproute2.link_set_protodown_reason_frr_on(slave)
++ except Exception as e:
++ self.logger.error('%s: %s' % (ifaceobj.name, str(e)))
+
+- self.enable_ipv6_if_prev_brport(slave)
+- self.netlink.link_set_master(slave, ifaceobj.name)
++ self.enable_ipv6_if_prev_brport(slave)
++ self.netlink.link_set_master(slave, ifaceobj.name)
++ except Exception as e:
++ # A slave can disappear after the cache check above.
++ self.log_error('%s: failed to enslave %s: %s'
++ % (ifaceobj.name, slave, str(e)), ifaceobj,
++ raise_error=False)
++ continue
+ runningslaves.append(slave)
+ # TODO: if this fail we should switch to iproute2
+ # start a batch: down - set master - up
+diff --git a/tests/unit/test_bond.py b/tests/unit/test_bond.py
+new file mode 100644
+index 00000000..01fab468
+--- /dev/null
++++ b/tests/unit/test_bond.py
+@@ -0,0 +1,103 @@
++"""Bond regression tests; run with python3 -m unittest discover -s tests/unit."""
++
++import unittest
++from unittest.mock import Mock, call, patch
++
++from ifupdown2.addons.bond import bond
++from ifupdown2.ifupdown import ifupdownflags
++from ifupdown2.ifupdown.iface import iface, ifaceStatus
++
++
++class BondSlaveTests(unittest.TestCase):
++ def setUp(self):
++ # Avoid initializing netlink sockets or touching the host network.
++ self.addon = bond.__new__(bond)
++ self.addon.logger = Mock()
++ self.addon.cache = Mock()
++ self.addon.cache.link_translate_altnames.side_effect = lambda names: names
++ self.addon.cache.link_exists.return_value = True
++ self.addon.cache.link_is_up.return_value = False
++ self.addon.cache.get_link_protodown.return_value = False
++ self.addon.netlink = Mock()
++ self.addon.iproute2 = Mock()
++ self.addon.sysfs = Mock()
++ self.addon.enable_ipv6_if_prev_brport = Mock()
++ self.ifaceobj = iface({"name": "bond0"})
++ self.slave_iface = iface({"name": "nic1"})
++ self.ifaceobj_getfunc = Mock(return_value=[self.slave_iface])
++ self.running = []
++ self.addCleanup(patch.stopall)
++ patch.object(ifupdownflags.flags, "IGNORE_ERRORS", False).start()
++
++ def add_slaves(self, slaves, perfmode):
++ self.ifaceobj.priv_data = slaves
++ with patch.object(ifupdownflags.flags, "PERFMODE", perfmode):
++ return self.addon._add_slaves(
++ self.ifaceobj, self.running, self.ifaceobj_getfunc
++ )
++
++ def test_missing_slave_does_not_block_available_slaves(self):
++ for perfmode in (False, True):
++ for slaves in (["missing", "nic1"], ["nic1", "missing"]):
++ with self.subTest(perfmode=perfmode, slaves=slaves):
++ self.running = []
++ self.addon.netlink.reset_mock()
++ self.addon.cache.link_exists.side_effect = lambda name: name != "missing"
++ self.assertEqual(self.add_slaves(slaves, perfmode), ["nic1"])
++ self.addon.netlink.link_set_master.assert_called_once_with("nic1", "bond0")
++ self.assertEqual(self.ifaceobj.status, ifaceStatus.ERROR)
++ self.addon.logger.error.assert_called_with(
++ "bond0: skipping slave missing, does not exist"
++ )
++
++ def test_all_slaves_missing(self):
++ for perfmode in (False, True):
++ with self.subTest(perfmode=perfmode):
++ self.addon.cache.link_exists.return_value = False
++ self.assertEqual(self.add_slaves(["missing0", "missing1"], perfmode), [])
++ self.addon.netlink.link_set_master.assert_not_called()
++ self.assertEqual(self.ifaceobj.status, ifaceStatus.ERROR)
++
++ def test_failed_enslavement_does_not_block_next_slave(self):
++ for perfmode in (False, True):
++ with self.subTest(perfmode=perfmode):
++ self.running = []
++ self.addon.netlink.reset_mock()
++ self.addon.netlink.link_set_master.side_effect = [
++ RuntimeError("No such device"), None
++ ]
++ self.assertEqual(self.add_slaves(["nic0", "nic1"], perfmode), ["nic1"])
++ self.assertEqual(self.addon.netlink.link_set_master.call_args_list, [
++ call("nic0", "bond0"), call("nic1", "bond0")
++ ])
++ self.assertEqual(self.ifaceobj.status, ifaceStatus.ERROR)
++ self.addon.logger.error.assert_called_with(
++ "bond0: failed to enslave nic0: No such device"
++ )
++
++ def test_slave_disappearing_before_link_down_does_not_block_next_slave(self):
++ self.addon.cache.link_is_up.side_effect = lambda name: name == "nic0"
++ self.addon.netlink.link_down_force.side_effect = RuntimeError("No such device")
++ self.assertEqual(self.add_slaves(["nic0", "nic1"], True), ["nic1"])
++ self.addon.netlink.link_set_master.assert_called_once_with("nic1", "bond0")
++ self.assertEqual(self.ifaceobj.status, ifaceStatus.ERROR)
++
++ def test_duplicates_and_existing_slaves_are_not_enslaved_again(self):
++ self.running = ["nic0"]
++ self.assertEqual(self.add_slaves(["nic0", "nic1", "nic1"], True), ["nic0", "nic1"])
++ self.addon.netlink.link_set_master.assert_called_once_with("nic1", "bond0")
++ self.assertNotEqual(self.ifaceobj.status, ifaceStatus.ERROR)
++
++ def test_up_continues_with_successfully_enslaved_slaves(self):
++ self.ifaceobj.priv_data = ["nic0", "nic1"]
++ self.addon.create_or_set_bond_config = Mock(return_value=(True, self.running))
++ self.addon.set_bond_mac = Mock()
++ self.addon.netlink.link_set_master.side_effect = [RuntimeError("No such device"), None]
++ with patch.object(ifupdownflags.flags, "PERFMODE", True):
++ self.addon._up(self.ifaceobj, self.ifaceobj_getfunc)
++ self.addon.set_bond_mac.assert_called_once_with(True, self.ifaceobj, ["nic1"])
++ self.assertEqual(self.ifaceobj.status, ifaceStatus.ERROR)
++
++
++if __name__ == "__main__":
++ unittest.main()
--
2.47.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH ifupdown2] fix #8117: backport upstream fix for unavailable bond slaves
2026-10-08 10:23 [PATCH ifupdown2] fix #8117: backport upstream fix for unavailable bond slaves Gabriel Goller
@ 2026-10-15 14:43 ` Christoph Heiss
2026-10-15 14:49 ` Gabriel Goller
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Heiss @ 2026-10-15 14:43 UTC (permalink / raw)
To: Gabriel Goller; +Cc: pve-devel
Thanks for tackling this!
On 10/8/26 12:23, Gabriel Goller wrote:
> A missing slave at boot can abort bond setup before available slaves are
> enslaved, leaving the bond broken. Backport upstream PR #355 to continue
> setup after single slave failures.
>
> Upstream-Link: https://github.com/CumulusNetworks/ifupdown2/pull/355
> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=8117
>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
Tested this with a similar setup to [0], just with `bond-slaves` set to:
bond-slaves doesnotexist enp6s20.1 enp6s21.1
Please consider this:
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
[0]
https://lore.proxmox.com/pve-devel/DM5AYOQZLL1E.2J6DHNGY4O0N4@proxmox.com/
[..]
> +diff --git a/tests/unit/test_bond.py b/tests/unit/test_bond.py
> +new file mode 100644
> +index 00000000..01fab468
> +--- /dev/null
> ++++ b/tests/unit/test_bond.py
> +@@ -0,0 +1,103 @@
> ++"""Bond regression tests; run with python3 -m unittest discover -s tests/unit."""
These tests don't run when doing a `make deb` with our (downstream)
packaging:
dh_auto_test -O--buildsystem=pybuild
I: pybuild base:311: cd
[..]ifupdown2/ifupdown2-3.3.0-1+pmx12/.pybuild/cpython3_3.13_ifupdown2/build;
python3.13 -m unittest discover -v
----------------------------------------------------------------------
Ran 0 tests in 0.000s
NO TESTS RAN
IMO would make sense to have them run on a `make deb` too.
> ++
> ++import unittest
> ++from unittest.mock import Mock, call, patch
> ++
> ++from ifupdown2.addons.bond import bond
> ++from ifupdown2.ifupdown import ifupdownflags
> ++from ifupdown2.ifupdown.iface import iface, ifaceStatus
> ++
> ++
> ++class BondSlaveTests(unittest.TestCase):
[..]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH ifupdown2] fix #8117: backport upstream fix for unavailable bond slaves
2026-10-15 14:43 ` Christoph Heiss
@ 2026-10-15 14:49 ` Gabriel Goller
0 siblings, 0 replies; 3+ messages in thread
From: Gabriel Goller @ 2026-10-15 14:49 UTC (permalink / raw)
To: Christoph Heiss; +Cc: pve-devel
On 15.10.2026 16:43, Christoph Heiss wrote:
> Thanks for tackling this!
>
> On 10/8/26 12:23, Gabriel Goller wrote:
> > A missing slave at boot can abort bond setup before available slaves are
> > enslaved, leaving the bond broken. Backport upstream PR #355 to continue
> > setup after single slave failures.
> >
> > Upstream-Link: https://github.com/CumulusNetworks/ifupdown2/pull/355
> > Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=8117
> >
> > Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
>
> Tested this with a similar setup to [0], just with `bond-slaves` set to:
>
> bond-slaves doesnotexist enp6s20.1 enp6s21.1
>
> Please consider this:
>
> Tested-by: Christoph Heiss <c.heiss@proxmox.com>
>
> [0]
> https://lore.proxmox.com/pve-devel/DM5AYOQZLL1E.2J6DHNGY4O0N4@proxmox.com/
>
> [..]
>
> > +diff --git a/tests/unit/test_bond.py b/tests/unit/test_bond.py
> > +new file mode 100644
> > +index 00000000..01fab468
> > +--- /dev/null
> > ++++ b/tests/unit/test_bond.py
> > +@@ -0,0 +1,103 @@
> > ++"""Bond regression tests; run with python3 -m unittest discover -s tests/unit."""
>
> These tests don't run when doing a `make deb` with our (downstream)
> packaging:
>
> dh_auto_test -O--buildsystem=pybuild
> I: pybuild base:311: cd
> [..]ifupdown2/ifupdown2-3.3.0-1+pmx12/.pybuild/cpython3_3.13_ifupdown2/build;
> python3.13 -m unittest discover -v
>
> ----------------------------------------------------------------------
> Ran 0 tests in 0.000s
>
> NO TESTS RAN
>
> IMO would make sense to have them run on a `make deb` too.
Hmm good catch, didn't run the tests :/
This should be fixed with something like:
diff --git a/debian/rules b/debian/rules
index cc9547dce6..0443852924 100755
--- a/debian/rules
+++ b/debian/rules
@@ -3,6 +3,7 @@
#export DH_VERBOSE=1
export PYBUILD_NAME=ifupdown2
export PYBUILD_INSTALL_ARGS=--install-lib=/usr/share/ --install-scripts=/usr/share/
+export PYBUILD_TEST_ARGS=--start-directory {dir}/tests/unit
%:
dh $@ --with=python3 --buildsystem=pybuild
I'll send a patch.
Thanks for the review!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-15 14:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-08 10:23 [PATCH ifupdown2] fix #8117: backport upstream fix for unavailable bond slaves Gabriel Goller
2026-10-15 14:43 ` Christoph Heiss
2026-10-15 14:49 ` Gabriel Goller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.