From: Gabriel Goller <g.goller@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH ifupdown2] fix #8117: backport upstream fix for unavailable bond slaves
Date: Thu, 8 Oct 2026 12:23:10 +0200 [thread overview]
Message-ID: <20261008102322.95273-1-g.goller@proxmox.com> (raw)
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
reply other threads:[~2026-10-08 10:23 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261008102322.95273-1-g.goller@proxmox.com \
--to=g.goller@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox