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