From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id EB8411FF0AF for ; Thu, 08 Oct 2026 12:23:36 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id D7065213C5; Thu, 08 Oct 2026 12:23:33 +0200 (CEST) From: Gabriel Goller 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 Message-ID: <20261008102322.95273-1-g.goller@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1791455006969 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.264 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: BQWVAHF67IRE7PDKTGDBJG62PTPY3JCH X-Message-ID-Hash: BQWVAHF67IRE7PDKTGDBJG62PTPY3JCH X-MailFrom: g.goller@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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 +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 + +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