From 0f1d29ac0480dc202595b96357789e6d15d49f2c Mon Sep 17 00:00:00 2001 From: initramfs Date: Thu, 1 Sep 2022 20:51:21 +0800 Subject: bonding: T4668: fix live bonding member add or remove Fixes several bugs around bonding member interface states not matching the committed configuration, including: - Disabled removed interfaces coming back up - Newly added disabled interfaces not staying down - Newly added interfaces not showing up in the bond --- python/vyos/ifconfig/bond.py | 12 +++++++----- src/conf_mode/interfaces-bonding.py | 23 ++++++++++++++++++++++- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/python/vyos/ifconfig/bond.py b/python/vyos/ifconfig/bond.py index 47e0598ab..f831551d8 100644 --- a/python/vyos/ifconfig/bond.py +++ b/python/vyos/ifconfig/bond.py @@ -1,4 +1,4 @@ -# Copyright 2019-2020 VyOS maintainers and contributors +# Copyright 2019-2022 VyOS maintainers and contributors # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -389,10 +389,12 @@ class BondIf(Interface): # Remove ALL bond member interfaces for interface in self.get_slaves(): self.del_port(interface) - # Removing an interface from a bond will always place the underlaying - # physical interface in admin-down state! If physical interface is - # not disabled, re-enable it. - if not dict_search(f'member.interface_remove.{interface}.disable', config): + + # Restore correct interface status based on config + if dict_search(f'member.interface.{interface}.disable', config) is not None or \ + dict_search(f'member.interface_remove.{interface}.disable', config) is not None: + Interface(interface).set_admin_state('down') + else: Interface(interface).set_admin_state('up') # Bonding policy/mode - default value, always present diff --git a/src/conf_mode/interfaces-bonding.py b/src/conf_mode/interfaces-bonding.py index 76d8d009b..94deb1abb 100755 --- a/src/conf_mode/interfaces-bonding.py +++ b/src/conf_mode/interfaces-bonding.py @@ -88,6 +88,11 @@ def get_config(config=None): # determine which members have been removed interfaces_removed = leaf_node_changed(conf, ['member', 'interface']) + + # Reset config level to interfaces + old_level = conf.get_level() + conf.set_level(['interfaces']) + if interfaces_removed: bond['shutdown_required'] = {} if 'member' not in bond: @@ -96,7 +101,7 @@ def get_config(config=None): tmp = {} for interface in interfaces_removed: section = Section.section(interface) # this will be 'ethernet' for 'eth0' - if conf.exists(['insterfaces', section, interface, 'disable']): + if conf.exists([section, interface, 'disable']): tmp[interface] = {'disable': ''} else: tmp[interface] = {} @@ -104,8 +109,24 @@ def get_config(config=None): # also present the interfaces to be removed from the bond as dictionary bond['member']['interface_remove'] = tmp + # Restore existing config level + conf.set_level(old_level) + if dict_search('member.interface', bond): for interface, interface_config in bond['member']['interface'].items(): + # Check if member interface is a new member + if not conf.exists_effective(['member', 'interface', interface]): + bond['shutdown_required'] = {} + + # Check if member interface is disabled + conf.set_level(['interfaces']) + + section = Section.section(interface) # this will be 'ethernet' for 'eth0' + if conf.exists([section, interface, 'disable']): + interface_config['disable'] = '' + + conf.set_level(old_level) + # Check if member interface is already member of another bridge tmp = is_member(conf, interface, 'bridge') if tmp: interface_config['is_bridge_member'] = tmp -- cgit v1.2.3