all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Gabriel Goller <g.goller@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH ifupdown2] fix #8130: bond MAC inheritance to avoid outages on first reload
Date: Thu,  8 Oct 2026 11:56:32 +0200	[thread overview]
Message-ID: <20261008095634.77401-1-g.goller@proxmox.com> (raw)

The netlink cache can list bond slaves in a different order than they
were initially enslaved (index vs order in config). A valid inherited
MAC is then considered invalid, which causes the first reload after boot
to flap the bond and its bridges.

Backport both upstream commits.

Upstream-Link: https://github.com/CumulusNetworks/ifupdown2/pull/318
Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=8130
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 debian/patches/series                         |   2 +
 ...reserve-mac-inherited-from-any-slave.patch | 116 ++++++++++++++++++
 ...004-bond-fix-list-index-out-of-range.patch |  40 ++++++
 3 files changed, 158 insertions(+)
 create mode 100644 debian/patches/upstream/0003-bond-preserve-mac-inherited-from-any-slave.patch
 create mode 100644 debian/patches/upstream/0004-bond-fix-list-index-out-of-range.patch

diff --git a/debian/patches/series b/debian/patches/series
index 2865533271c9..45924c8d21dc 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -16,3 +16,5 @@ upstream/0001-use-raw-strings-for-regex-to-fix-backslash-interpret.patch
 upstream/0002-vxlan-add-support-for-IPv6-vxlan-local-tunnelip.patch
 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
diff --git a/debian/patches/upstream/0003-bond-preserve-mac-inherited-from-any-slave.patch b/debian/patches/upstream/0003-bond-preserve-mac-inherited-from-any-slave.patch
new file mode 100644
index 000000000000..daa8876dfdfd
--- /dev/null
+++ b/debian/patches/upstream/0003-bond-preserve-mac-inherited-from-any-slave.patch
@@ -0,0 +1,116 @@
+From 29807929cce23e0c11875c9efddea45b187e3c86 Mon Sep 17 00:00:00 2001
+From: Julien Fortin <jfortin@nvidia.com>
+Date: Tue, 28 Nov 2023 23:17:18 +0100
+Subject: [PATCH] addons: bond: change bond mac inheritance code (any slave mac
+ is fine)
+
+Upstream-Link: https://github.com/CumulusNetworks/ifupdown2/pull/318
+Signed-off-by: Julien Fortin <jfortin@nvidia.com>
+---
+ ifupdown2/addons/bond.py | 90 +++++++++++++++++++++-------------------
+ 1 file changed, 48 insertions(+), 42 deletions(-)
+
+diff --git a/ifupdown2/addons/bond.py b/ifupdown2/addons/bond.py
+index deb88def..b691ef44 100644
+--- a/ifupdown2/addons/bond.py
++++ b/ifupdown2/addons/bond.py
+@@ -901,51 +901,57 @@ def _up(self, ifaceobj, ifaceobj_getfunc=None):
+                 bond_slaves,
+                 ifaceobj_getfunc,
+             )
+-
+-            if not self.bond_mac_mgmt or not link_exists or ifaceobj.get_attr_value_first("hwaddress"):
+-                return
+-
+-            # check if the bond mac address is correctly inherited from it's
+-            # first slave. There's a case where that might not be happening:
+-            # $ ip link show swp1 | grep ether
+-            #    link/ether 08:00:27:04:d8:01 brd ff:ff:ff:ff:ff:ff
+-            # $ ip link show swp2 | grep ether
+-            #    link/ether 08:00:27:04:d8:02 brd ff:ff:ff:ff:ff:ff
+-            # $ ip link add dev bond0 type bond
+-            # $ ip link set dev swp1 master bond0
+-            # $ ip link set dev swp2 master bond0
+-            # $ ip link show bond0 | grep ether
+-            #    link/ether 08:00:27:04:d8:01 brd ff:ff:ff:ff:ff:ff
+-            # $ ip link add dev bond1 type bond
+-            # $ ip link set dev swp1 master bond1
+-            # $ ip link show swp1 | grep ether
+-            #    link/ether 08:00:27:04:d8:01 brd ff:ff:ff:ff:ff:ff
+-            # $ ip link show swp2 | grep ether
+-            #    link/ether 08:00:27:04:d8:01 brd ff:ff:ff:ff:ff:ff
+-            # $ ip link show bond0 | grep ether
+-            #    link/ether 08:00:27:04:d8:01 brd ff:ff:ff:ff:ff:ff
+-            # $ ip link show bond1 | grep ether
+-            #    link/ether 08:00:27:04:d8:01 brd ff:ff:ff:ff:ff:ff
+-            # $
+-            # ifupdown2 will automatically correct and fix this unexpected behavior
+-            bond_mac = self.cache.get_link_address(ifaceobj.name)
+-
+-            if bond_slaves:
+-                first_slave_ifname = bond_slaves[0]
+-                first_slave_mac = self.cache.get_link_info_slave_data_attribute(
+-                    first_slave_ifname,
+-                    Link.IFLA_BOND_SLAVE_PERM_HWADDR
+-                )
+-
+-                if first_slave_mac and bond_mac != first_slave_mac:
+-                    self.logger.info(
+-                        "%s: invalid bond mac detected - resetting to %s's mac (%s)"
+-                        % (ifaceobj.name, first_slave_ifname, first_slave_mac)
+-                    )
+-                    self.netlink.link_set_address(ifaceobj.name, first_slave_mac, utils.mac_str_to_int(first_slave_mac))
++            self.set_bond_mac(link_exists, ifaceobj, bond_slaves)
+         except Exception as e:
+             self.log_error(str(e), ifaceobj)
+ 
++    def set_bond_mac(self, link_exists, ifaceobj, bond_slaves):
++        if not self.bond_mac_mgmt or not link_exists or ifaceobj.get_attr_value_first("hwaddress"):
++            return
++
++        # check if the bond mac address is correctly inherited from it's
++        # first slave. There's a case where that might not be happening:
++        # $ ip link show swp1 | grep ether
++        #    link/ether 08:00:27:04:d8:01 brd ff:ff:ff:ff:ff:ff
++        # $ ip link show swp2 | grep ether
++        #    link/ether 08:00:27:04:d8:02 brd ff:ff:ff:ff:ff:ff
++        # $ ip link add dev bond0 type bond
++        # $ ip link set dev swp1 master bond0
++        # $ ip link set dev swp2 master bond0
++        # $ ip link show bond0 | grep ether
++        #    link/ether 08:00:27:04:d8:01 brd ff:ff:ff:ff:ff:ff
++        # $ ip link add dev bond1 type bond
++        # $ ip link set dev swp1 master bond1
++        # $ ip link show swp1 | grep ether
++        #    link/ether 08:00:27:04:d8:01 brd ff:ff:ff:ff:ff:ff
++        # $ ip link show swp2 | grep ether
++        #    link/ether 08:00:27:04:d8:01 brd ff:ff:ff:ff:ff:ff
++        # $ ip link show bond0 | grep ether
++        #    link/ether 08:00:27:04:d8:01 brd ff:ff:ff:ff:ff:ff
++        # $ ip link show bond1 | grep ether
++        #    link/ether 08:00:27:04:d8:01 brd ff:ff:ff:ff:ff:ff
++        # $
++        # ifupdown2 will automatically correct and fix this unexpected behavior
++        # Although if the bond's mac belongs to any of its slave we won't update it
++        bond_mac = self.cache.get_link_address(ifaceobj.name)
++
++        # Get the list of slave macs
++        bond_slave_macs = map(
++            lambda slave_ifname: self.cache.get_link_info_slave_data_attribute(slave_ifname, Link.IFLA_BOND_SLAVE_PERM_HWADDR),
++            bond_slaves
++        )
++
++        if bond_slaves and bond_mac not in bond_slave_macs:
++            first_slave_ifname = bond_slaves[0]
++            first_slave_mac = list(bond_slave_macs)[0]
++
++            if first_slave_mac and bond_mac != first_slave_mac:
++                self.logger.info(
++                    "%s: invalid bond mac detected - resetting to %s's mac (%s)"
++                    % (ifaceobj.name, first_slave_ifname, first_slave_mac)
++                )
++                self.netlink.link_set_address(ifaceobj.name, first_slave_mac, utils.mac_str_to_int(first_slave_mac))
++
+     def _down(self, ifaceobj, ifaceobj_getfunc=None):
+         bond_slaves = self.cache.get_slaves(ifaceobj.name)
+ 
diff --git a/debian/patches/upstream/0004-bond-fix-list-index-out-of-range.patch b/debian/patches/upstream/0004-bond-fix-list-index-out-of-range.patch
new file mode 100644
index 000000000000..e79653eb8fd8
--- /dev/null
+++ b/debian/patches/upstream/0004-bond-fix-list-index-out-of-range.patch
@@ -0,0 +1,40 @@
+From 97fb31ce1af89079834d441b59a3d7be865158c4 Mon Sep 17 00:00:00 2001
+From: Julien Fortin <jfortin@nvidia.com>
+Date: Fri, 5 Jan 2024 15:57:26 +0100
+Subject: [PATCH] addons: bond: fix 'list index out of range' error when
+ removing first bond slave
+
+Upstream-Link: https://github.com/CumulusNetworks/ifupdown2/pull/318
+Signed-off-by: Julien Fortin <jfortin@nvidia.com>
+---
+ ifupdown2/addons/bond.py | 14 +++++++++-----
+ 1 file changed, 9 insertions(+), 5 deletions(-)
+
+diff --git a/ifupdown2/addons/bond.py b/ifupdown2/addons/bond.py
+index b691ef44..2e6edf2d 100644
+--- a/ifupdown2/addons/bond.py
++++ b/ifupdown2/addons/bond.py
+@@ -936,14 +936,18 @@ def set_bond_mac(self, link_exists, ifaceobj, bond_slaves):
+         bond_mac = self.cache.get_link_address(ifaceobj.name)
+ 
+         # Get the list of slave macs
+-        bond_slave_macs = map(
+-            lambda slave_ifname: self.cache.get_link_info_slave_data_attribute(slave_ifname, Link.IFLA_BOND_SLAVE_PERM_HWADDR),
++        bond_slave_macs = list(map(
++            lambda slave_ifname: self.cache.get_link_info_slave_data_attribute(
++                slave_ifname,
++                Link.IFLA_BOND_SLAVE_PERM_HWADDR,
++                default=list()
++            ),
+             bond_slaves
+-        )
++        ))
+ 
+-        if bond_slaves and bond_mac not in bond_slave_macs:
++        if bond_slaves and bond_slave_macs and bond_mac not in bond_slave_macs:
+             first_slave_ifname = bond_slaves[0]
+-            first_slave_mac = list(bond_slave_macs)[0]
++            first_slave_mac = bond_slave_macs[0]
+ 
+             if first_slave_mac and bond_mac != first_slave_mac:
+                 self.logger.info(
-- 
2.47.3





                 reply	other threads:[~2026-10-08  9:56 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=20261008095634.77401-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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal