public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH ifupdown2] d/patches: backport upstream fix for missing bond-primary device
@ 2026-11-09 14:58 Gabriel Goller
  0 siblings, 0 replies; only message in thread
From: Gabriel Goller @ 2026-11-09 14:58 UTC (permalink / raw)
  To: pve-devel; +Cc: Shay Sevet

If the device named in bond-primary does not exist, e.g. an unplugged
USB NIC, bond setup aborts and every interface on top of the bond fails
with it. Backport upstream PR #358 to set up the bond without a primary
and warn instead. This completes my earlier
PR and backport which fixed this for bond-slaves:
https://github.com/CumulusNetworks/ifupdown2/pull/355
and
https://lore.proxmox.com/pve-devel/20261008102322.95273-1-g.goller@proxmox.com/

Upstream-Link: https://github.com/CumulusNetworks/ifupdown2/pull/358
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
This is based on top of the other ifupdown2 patches. This is just for
convenience, no real dependancy.
---
 debian/patches/series                              |   1 +
 ...-fail-the-bond-when-bond-primary-does-not.patch | 148 +++++++++++++++++++++
 2 files changed, 149 insertions(+)

diff --git a/debian/patches/series b/debian/patches/series
index 48bb4adae7f5..52e66559b188 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -23,3 +23,4 @@ pve/0016-addons-bond-skip-mac-check-for-bonds-without-slaves.patch
 pve/0017-addons-vxlan-don-t-fail-syntax-check-on-missing-vxla.patch
 pve/0018-addons-vxlan-add-vxlan-gbp-attribute.patch
 pve/0019-vxlan-match-IPv6-peers-when-reading-the-fdb.patch
+upstream/0006-bond-do-not-fail-the-bond-when-bond-primary-does-not.patch
diff --git a/debian/patches/upstream/0006-bond-do-not-fail-the-bond-when-bond-primary-does-not.patch b/debian/patches/upstream/0006-bond-do-not-fail-the-bond-when-bond-primary-does-not.patch
new file mode 100644
index 000000000000..75d559bd847e
--- /dev/null
+++ b/debian/patches/upstream/0006-bond-do-not-fail-the-bond-when-bond-primary-does-not.patch
@@ -0,0 +1,148 @@
+From cf1f500bc2f8d18d7831a830cbe81675b51c0825 Mon Sep 17 00:00:00 2001
+From: Shay Sevet <4628214+sevet@users.noreply.github.com>
+Date: Sat, 7 Nov 2026 17:42:30 +0000
+Subject: [PATCH] bond: do not fail the bond when bond-primary does not exist
+
+bond-primary is translated to an ifindex through the netlink cache. If
+the named device does not exist (an unplugged or failed USB NIC, a NIC
+whose driver loads late, a replacement NIC that came up under a new
+name), get_ifindex() raises NetlinkCacheIfnameNotFoundError. Only
+KeyError is handled there, so the exception aborts the whole bond setup:
+
+  error: ifname nic9 not present in cache
+  error: br0: bridge port bond0 does not exist
+
+The bond is never created and every interface stacked on it (bridges,
+VLANs, addresses) fails with it, at boot and on ifreload. For an
+active-backup bond this defeats the point of the redundancy: losing the
+preferred device takes down the bond instead of failing over.
+
+Treat a missing bond-primary like an invalid value: log a warning and
+set up the bond without a primary. The kernel then picks the active
+slave as for any bond without a primary. The configuration is left
+untouched; a later ifreload sets the primary once the device exists.
+
+Missing bond-slaves are handled separately by #355.
+
+Reported-by: Shay Sevet <4628214+sevet@users.noreply.github.com>
+[patch developed with AI assistance (Claude), reviewed and tested by the author]
+Upstream-Link: https://github.com/CumulusNetworks/ifupdown2/pull/358
+Signed-off-by: Shay Sevet <4628214+sevet@users.noreply.github.com>
+Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
+
+Backport notes: adjust context for the fallback import block.
+---
+ ifupdown2/addons/bond.py        |  9 +++++
+ tests/unit/__init__.py          |  0
+ tests/unit/test_bond_primary.py | 64 +++++++++++++++++++++++++++++++++
+ 3 files changed, 73 insertions(+)
+ create mode 100644 tests/unit/__init__.py
+ create mode 100644 tests/unit/test_bond_primary.py
+
+diff --git a/ifupdown2/addons/bond.py b/ifupdown2/addons/bond.py
+index 4db62ee8ac1a..7d2bc9ac401b 100644
+--- a/ifupdown2/addons/bond.py
++++ b/ifupdown2/addons/bond.py
+@@ -15,6 +15,7 @@ from contextlib import suppress
+ try:
+     from ifupdown2.nlmanager.ipnetwork import IPv4Address
+     from ifupdown2.lib.addon import Addon
++    from ifupdown2.lib.nlcache import NetlinkCacheIfnameNotFoundError
+     from ifupdown2.nlmanager.nlmanager import Link
+ 
+     from ifupdown2.ifupdown.iface import ifaceRole, ifaceLinkKind, ifaceLinkPrivFlags, ifaceLinkType, ifaceDependencyType, ifaceStatus
+@@ -28,6 +29,7 @@ try:
+ except (ImportError, ModuleNotFoundError):
+     from nlmanager.ipnetwork import IPv4Address
+     from lib.addon import Addon
++    from lib.nlcache import NetlinkCacheIfnameNotFoundError
+     from nlmanager.nlmanager import Link
+ 
+     from ifupdown.iface import ifaceRole, ifaceLinkKind, ifaceLinkPrivFlags, ifaceLinkType, ifaceDependencyType, ifaceStatus
+@@ -714,6 +716,13 @@ class bond(Addon, moduleBase):
+                 except KeyError:
+                     self.logger.warning('%s: invalid %s value %s' % (ifname, attr_name, user_config))
+ 
++                except NetlinkCacheIfnameNotFoundError:
++                    # bond-primary names a device that does not exist (e.g. an
++                    # unplugged or failed USB NIC). Set up the bond without a
++                    # primary instead of failing it and every interface above it.
++                    self.log_warn('%s: ignoring %s %s: device does not exist'
++                                  % (ifname, attr_name, user_config), ifaceobj)
++
+         self._check_bond_mode_user_config(ifname, link_exists, ifla_info_data)
+         return ifla_info_data
+ 
+diff --git a/tests/unit/__init__.py b/tests/unit/__init__.py
+new file mode 100644
+index 000000000000..e69de29bb2d1
+diff --git a/tests/unit/test_bond_primary.py b/tests/unit/test_bond_primary.py
+new file mode 100644
+index 000000000000..5d82f8f0c402
+--- /dev/null
++++ b/tests/unit/test_bond_primary.py
+@@ -0,0 +1,64 @@
++"""Bond primary regression tests; run with python3 -m unittest discover -s tests/unit."""
++
++import unittest
++from unittest.mock import Mock, patch
++
++from ifupdown2.addons.bond import bond
++from ifupdown2.ifupdown import ifupdownflags
++from ifupdown2.ifupdown.iface import iface, ifaceStatus
++from ifupdown2.lib.nlcache import NetlinkCacheIfnameNotFoundError
++from ifupdown2.nlmanager.nlmanager import Link
++
++
++class BondPrimaryTests(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.get_saved_ifaceobj = Mock(return_value=None)
++        self.addon._check_bond_mode_user_config = Mock()
++        self.addon._bond_attr_set_list = bond._bond_attr_set_list + (
++            ("bond-primary", Link.IFLA_BOND_PRIMARY, self.get_ifindex),
++        )
++        self.existing = {"nic0": 10, "nic1": 11}
++        self.ifaceobj = iface({"name": "bond0"})
++        self.ifaceobj.config = {
++            "bond-slaves": ["nic0 nic1"],
++            "bond-mode": ["active-backup"],
++        }
++        self.addCleanup(patch.stopall)
++        patch.object(ifupdownflags.flags, "IGNORE_ERRORS", False).start()
++        patch(
++            "ifupdown2.addons.bond.policymanager.policymanager_api", Mock(
++                get_iface_default=Mock(return_value=None))
++        ).start()
++
++    def get_ifindex(self, ifname):
++        try:
++            return self.existing[ifname]
++        except KeyError:
++            raise NetlinkCacheIfnameNotFoundError("ifname %s not present in cache" % ifname)
++
++    def attrs(self, primary):
++        self.ifaceobj.config["bond-primary"] = [primary]
++        return self.addon.get_ifla_bond_attr_from_user_config(self.ifaceobj, False)
++
++    def test_existing_primary_is_set(self):
++        info = self.attrs("nic1")
++        self.assertEqual(info[Link.IFLA_BOND_PRIMARY], 11)
++        self.assertNotEqual(self.ifaceobj.status, ifaceStatus.WARNING)
++
++    def test_missing_primary_is_skipped_with_warning(self):
++        info = self.attrs("usb0")
++        self.assertNotIn(Link.IFLA_BOND_PRIMARY, info)
++        # the remaining attributes are still applied
++        self.assertIn(Link.IFLA_BOND_MODE, info)
++        self.assertEqual(self.ifaceobj.status, ifaceStatus.WARNING)
++        self.addon.logger.warning.assert_called_with(
++            "bond0: ignoring bond-primary usb0: device does not exist"
++        )
++
++
++if __name__ == "__main__":
++    unittest.main()

---
base-commit: 705d9a203ae3a8a896b4fb03217701c65b10ef21
change-id: 20261109-backport_upstream_fix-0e4e32310061
prerequisite-change-id: 20261106-fix_8299_only-86419dd42acd:v2
prerequisite-patch-id: 54aca45ba1edc2781ea6b4843b662da577b08cdf
prerequisite-patch-id: bf8fa8dc4eb0f688930f892c5bf60a1cac22dc4b

Best regards,
--  
Gabriel Goller <g.goller@proxmox.com>





^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2026-11-09 14:58 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-09 14:58 [PATCH ifupdown2] d/patches: backport upstream fix for missing bond-primary device 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