public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH ifupdown2] fix #8117: backport upstream fix for unavailable bond slaves
@ 2026-10-08 10:23 Gabriel Goller
  0 siblings, 0 replies; only message 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] only message in thread

only message in thread, other threads:[~2026-10-08 10:23 UTC | newest]

Thread overview: (only message) (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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal