From 6205c4d6701bda5f8a859291a5e152e009252301 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Tue, 3 Sep 2019 13:52:33 +0200
Subject: bonding: T1614: Initial version in new style XML/Python interface

The node 'interfaces ethernet eth0 bond-group' has been changed and
de-nested. Bond members are now configured in the bond interface itself.

set interfaces bonding bond0 member interface eth0
---
 src/conf_mode/interface-bonding.py | 409 +++++++++++++++++++++++++++++++++++++
 1 file changed, 409 insertions(+)
 create mode 100755 src/conf_mode/interface-bonding.py

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
new file mode 100755
index 000000000..6cd8fdc56
--- /dev/null
+++ b/src/conf_mode/interface-bonding.py
@@ -0,0 +1,409 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2019 VyOS maintainers and contributors
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 or later as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+#
+
+import os
+
+from copy import deepcopy
+from sys import exit
+from netifaces import interfaces
+from vyos.ifconfig import BondIf
+from vyos.config import Config
+from vyos import ConfigError
+
+default_config_data = {
+    'address': [],
+    'address_remove': [],
+    'arp_mon_intvl': 0,
+    'arp_mon_tgt': [],
+    'description': '',
+    'deleted': False,
+    'dhcp_client_id': '',
+    'dhcp_hostname': '',
+    'dhcpv6_prm_only': False,
+    'dhcpv6_temporary': False,
+    'disable': False,
+    'disable_link_detect': 1,
+    'hash_policy': 'layer2',
+    'ip_arp_cache_tmo': 30,
+    'ip_proxy_arp': 0,
+    'ip_proxy_arp_pvlan': 0,
+    'intf': '',
+    'mac': '',
+    'mode': '802.3ad',
+    'member': [],
+    'member_remove': [],
+    'mtu': 1500,
+    'primary': '',
+    'vif_s': [],
+    'vif': []
+}
+
+def diff(first, second):
+    second = set(second)
+    return [item for item in first if item not in second]
+
+def get_bond_mode(mode):
+    if mode == 'round-robin':
+        return 'balance-rr'
+    elif mode == 'active-backup':
+        return 'active-backup'
+    elif mode == 'xor-hash':
+        return 'balance-xor'
+    elif mode == 'broadcast':
+        return 'broadcast'
+    elif mode == '802.3ad':
+        return '802.3ad'
+    elif mode == 'transmit-load-balance':
+        return 'balance-tlb'
+    elif mode == 'adaptive-load-balance':
+        return 'balance-alb'
+    else:
+        raise ConfigError('invalid bond mode "{}"'.format(mode))
+
+def vlan_to_dict(conf):
+    """
+    Common used function which will extract VLAN related information from config
+    and represent the result as Python dictionary.
+
+    Function call's itself recursively if a vif-s/vif-c pair is detected.
+    """
+    vlan = {
+        'id': conf.get_level().split(' ')[-1], # get the '100' in 'interfaces bonding bond0 vif-s 100'
+        'address': [],
+        'address_remove': [],
+        'description': '',
+        'dhcp_client_id': '',
+        'dhcp_hostname': '',
+        'dhcpv6_prm_only': False,
+        'dhcpv6_temporary': False,
+        'disable': False,
+        'disable_link_detect': 1,
+        'mac': '',
+        'mtu': 1500
+    }
+    # retrieve configured interface addresses
+    if conf.exists('address'):
+        vlan['address'] = conf.return_values('address')
+
+    # Determine interface addresses (currently effective) - to determine which
+    # address is no longer valid and needs to be removed from the bond
+    eff_addr = conf.return_effective_values('address')
+    act_addr = conf.return_values('address')
+    vlan['address_remove'] = diff(eff_addr, act_addr)
+
+    # retrieve interface description
+    if conf.exists('description'):
+        vlan['description'] = conf.return_value('description')
+
+    # get DHCP client identifier
+    if conf.exists('dhcp-options client-id'):
+        vlan['dhcp_client_id'] = conf.return_value('dhcp-options client-id')
+
+    # DHCP client host name (overrides the system host name)
+    if conf.exists('dhcp-options host-name'):
+        vlan['dhcp_hostname'] = conf.return_value('dhcp-options host-name')
+
+    # DHCPv6 only acquire config parameters, no address
+    if conf.exists('dhcpv6-options parameters-only'):
+        vlan['dhcpv6_prm_only'] = conf.return_value('dhcpv6-options parameters-only')
+
+    # DHCPv6 temporary IPv6 address
+    if conf.exists('dhcpv6-options temporary'):
+        vlan['dhcpv6_temporary'] = conf.return_value('dhcpv6-options temporary')
+
+    # ignore link state changes
+    if conf.exists('disable-link-detect'):
+        vlan['disable_link_detect'] = 2
+
+    # disable bond interface
+    if conf.exists('disable'):
+        vlan['disable'] = True
+
+    # ethertype (only on vif-s nodes)
+    if conf.exists('ethertype'):
+        vlan['ethertype'] = conf.return_value('ethertype')
+
+    # Media Access Control (MAC) address
+    if conf.exists('mac'):
+        vlan['mac'] = conf.return_value('mac')
+
+    # Maximum Transmission Unit (MTU)
+    if conf.exists('mtu'):
+        vlan['mtu'] = int(conf.return_value('mtu'))
+
+    # check if there is a Q-in-Q vlan customer interface
+    # and call this function recursively
+    if conf.exists('vif-c'):
+        cfg_level = conf.get_level()
+        # add new key (vif-c) to dictionary
+        vlan['vif-c'] = []
+        for vif in conf.list_nodes('vif-c'):
+            # set config level to vif interface
+            conf.set_level(cfg_level + ' vif-c ' + vif)
+            vlan['vif-c'].append(vlan_to_dict(conf))
+
+    return vlan
+
+def get_config():
+    # initialize kernel module if not loaded
+    if not os.path.isfile('/sys/class/net/bonding_masters'):
+        import syslog
+        syslog.syslog(syslog.LOG_NOTICE, "loading bonding kernel module")
+        if os.system('modprobe bonding max_bonds=0 miimon=250') != 0:
+            syslog.syslog(syslog.LOG_NOTICE, "failed loading bonding kernel module")
+            raise ConfigError("failed loading bonding kernel module")
+
+    bond = deepcopy(default_config_data)
+    conf = Config()
+
+    # determine tagNode instance
+    try:
+        bond['intf'] = os.environ['VYOS_TAGNODE_VALUE']
+    except KeyError as E:
+        print("Interface not specified")
+
+    # check if bond has been removed
+    cfg_base = 'interfaces bonding ' + bond['intf']
+    if not conf.exists(cfg_base):
+        bond['deleted'] = True
+        return bond
+
+    # set new configuration level
+    conf.set_level(cfg_base)
+
+    # retrieve configured interface addresses
+    if conf.exists('address'):
+        bond['address'] = conf.return_values('address')
+
+    # ARP link monitoring frequency in milliseconds
+    if conf.exists('arp-monitor interval'):
+        bond['arp_mon_intvl'] = conf.return_value('arp-monitor interval')
+
+    # IP address to use for ARP monitoring
+    if conf.exists('arp-monitor target'):
+        bond['arp_mon_tgt'] = conf.return_values('arp-monitor target')
+
+    # retrieve interface description
+    if conf.exists('description'):
+        bond['description'] = conf.return_value('description')
+    else:
+        bond['description'] = bond['intf']
+
+    # get DHCP client identifier
+    if conf.exists('dhcp-options client-id'):
+        bond['dhcp_client_id'] = conf.return_value('dhcp-options client-id')
+
+    # DHCP client host name (overrides the system host name)
+    if conf.exists('dhcp-options host-name'):
+        bond['dhcp_hostname'] = conf.return_value('dhcp-options host-name')
+
+    # DHCPv6 only acquire config parameters, no address
+    if conf.exists('dhcpv6-options parameters-only'):
+        bond['dhcpv6_prm_only'] = conf.return_value('dhcpv6-options parameters-only')
+
+    # DHCPv6 temporary IPv6 address
+    if conf.exists('dhcpv6-options temporary'):
+        bond['dhcpv6_temporary'] = conf.return_value('dhcpv6-options temporary')
+
+    # ignore link state changes
+    if conf.exists('disable-link-detect'):
+        bond['disable_link_detect'] = 2
+
+    # disable bond interface
+    if conf.exists('disable'):
+        bond['disable'] = True
+
+    # Bonding transmit hash policy
+    if conf.exists('hash-policy'):
+        bond['hash_policy'] = conf.return_value('hash-policy')
+
+    # ARP cache entry timeout in seconds
+    if conf.exists('ip arp-cache-timeout'):
+        bond['ip_arp_cache_tmo'] = int(conf.return_value('ip arp-cache-timeout'))
+
+    # Enable proxy-arp on this interface
+    if conf.exists('ip enable-proxy-arp'):
+        bond['ip_proxy_arp'] = 1
+
+    # Enable private VLAN proxy ARP on this interface
+    if conf.exists('ip proxy-arp-pvlan'):
+        bond['ip_proxy_arp_pvlan'] = 1
+
+    # Media Access Control (MAC) address
+    if conf.exists('mac'):
+        bond['mac'] = conf.return_value('mac')
+
+    # Bonding mode
+    if conf.exists('mode'):
+        bond['mode'] = get_bond_mode(conf.return_value('mode'))
+
+    # Maximum Transmission Unit (MTU)
+    if conf.exists('mtu'):
+        bond['mtu'] = int(conf.return_value('mtu'))
+
+    # determine bond member interfaces (currently configured)
+    if conf.exists('member interface'):
+        bond['member'] = conf.return_values('member interface')
+
+    # Determine bond member interface (currently effective) - to determine which
+    # interfaces is no longer assigend to the bond and thus can be removed
+    eff_intf = conf.return_effective_values('member interface')
+    act_intf = conf.return_values('member interface')
+    bond['member_remove'] = diff(eff_intf, act_intf)
+
+    # Determine interface addresses (currently effective) - to determine which
+    # address is no longer valid and needs to be removed from the bond
+    eff_addr = conf.return_effective_values('address')
+    act_addr = conf.return_values('address')
+    bond['address_remove'] = diff(eff_addr, act_addr)
+
+    # Primary device interface
+    if conf.exists('primary'):
+        bond['primary'] = conf.return_value('primary')
+
+    # re-set configuration level and retrieve vif-s interfaces
+    conf.set_level(cfg_base)
+    if conf.exists('vif-s'):
+        for vif_s in conf.list_nodes('vif-s'):
+            # set config level to vif-s interface
+            conf.set_level(cfg_base + ' vif-s ' + vif_s)
+            bond['vif_s'].append(vlan_to_dict(conf))
+
+    # re-set configuration level and retrieve vif-s interfaces
+    conf.set_level(cfg_base)
+    if conf.exists('vif'):
+        for vif in conf.list_nodes('vif'):
+            # set config level to vif interface
+            conf.set_level(cfg_base + ' vif ' + vif)
+            bond['vif'].append(vlan_to_dict(conf))
+
+    return bond
+
+def verify(bond):
+    if len (bond['arp_mon_tgt']) > 16:
+        raise ConfigError('The maximum number of targets that can be specified is 16')
+
+    if bond['primary']:
+        if bond['mode'] not in ['active-backup', 'balance-tlb', 'balance-alb']:
+            raise ConfigError('Mode dependency failed, primary not supported in this mode.'.format())
+
+    return None
+
+def generate(bond):
+    return None
+
+def apply(bond):
+    b = BondIf(bond['intf'])
+
+    if bond['deleted']:
+        # delete bonding interface
+        b.remove()
+    else:
+        # Some parameters can not be changed when the bond is up.
+        # Always disable the bond prior changing anything
+        b.state = 'down'
+
+        # Configure interface address(es)
+        for addr in bond['address_remove']:
+            b.del_addr(addr)
+        for addr in bond['address']:
+            b.add_addr(addr)
+
+        # ARP link monitoring frequency
+        b.arp_interval = bond['arp_mon_intvl']
+        # reset miimon on arp-montior deletion
+        if bond['arp_mon_intvl'] == 0:
+            # reset miimon to default
+            b.bond_miimon = 250
+
+        # ARP monitor targets need to be synchronized between sysfs and CLI.
+        # Unfortunately an address can't be send twice to sysfs as this will
+        # result in the following exception:  OSError: [Errno 22] Invalid argument.
+        #
+        # We remove ALL adresses prior adding new ones, this will remove addresses
+        # added manually by the user too - but as we are limited to 16 adresses
+        # from the kernel side this looks valid to me. We won't run into an error
+        # when a user added manual adresses which would result in having more
+        # then 16 adresses in total.
+        cur_addr = list(map(str, b.arp_ip_target.split()))
+        for addr in cur_addr:
+            b.arp_ip_target = '-' + addr
+
+        # Add configured ARP target addresses
+        for addr in bond['arp_mon_tgt']:
+            b.arp_ip_target = '+' + addr
+
+        # update interface description used e.g. within SNMP
+        b.ifalias = bond['description']
+
+        #
+        # missing DHCP/DHCPv6 options go here
+        #
+
+        # ignore link state changes
+        b.link_detect = bond['disable_link_detect']
+        # Bonding transmit hash policy
+        b.xmit_hash_policy = bond['hash_policy']
+        # configure ARP cache timeout in milliseconds
+        b.arp_cache_tmp = bond['ip_arp_cache_tmo']
+        # Enable proxy-arp on this interface
+        b.proxy_arp = bond['ip_proxy_arp']
+        # Enable private VLAN proxy ARP on this interface
+        b.proxy_arp_pvlan = bond['ip_proxy_arp_pvlan']
+
+        # Change interface MAC address
+        if bond['mac']:
+            b.mac = bond['mac']
+
+        # The bonding mode can not be changed when there are interfaces enslaved
+        # to this bond, thus we will free all interfaces from the bond first!
+        for intf in b.get_slaves():
+            b.del_port(intf)
+
+        # Bonding policy
+        b.mode = bond['mode']
+        # Maximum Transmission Unit (MTU)
+        b.mtu = bond['mtu']
+        # Primary device interface
+        b.primary = bond['primary']
+
+        #
+        # VLAN config goes here
+        #
+
+        # Add (enslave) interfaces to bond
+        for intf in bond['member']:
+                b.add_port(intf)
+
+        # As the bond interface is always disabled first when changing
+        # parameters we will only re-enable the interface if it is not
+        # administratively disabled
+        if not bond['disable']:
+            b.state = 'up'
+
+    return None
+
+if __name__ == '__main__':
+    try:
+        c = get_config()
+        verify(c)
+        generate(c)
+        apply(c)
+    except ConfigError as e:
+        print(e)
+        exit(1)
-- 
cgit v1.2.3


From 3fad8f2f74224502018c5e53b5931251595ad3b7 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Tue, 3 Sep 2019 14:56:21 +0200
Subject: bonding: T1614: can not set primary interface when it's not part of
 the bond

---
 src/conf_mode/interface-bonding.py | 3 +++
 1 file changed, 3 insertions(+)

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index 6cd8fdc56..b150578e3 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -302,6 +302,9 @@ def verify(bond):
         if bond['mode'] not in ['active-backup', 'balance-tlb', 'balance-alb']:
             raise ConfigError('Mode dependency failed, primary not supported in this mode.'.format())
 
+        if bond['primary'] not in bond['member']:
+            raise ConfigError('Interface "{}" is not part of the bond'.format(bond['primary']))
+
     return None
 
 def generate(bond):
-- 
cgit v1.2.3


From 029085250c4e2dea8a98f3031cf6b8037ffdd534 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Tue, 3 Sep 2019 17:14:11 +0200
Subject: bonding: T1614: remove obsolete 'member_remove' dict-key

---
 src/conf_mode/interface-bonding.py | 7 -------
 1 file changed, 7 deletions(-)

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index b150578e3..a623ca524 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -46,7 +46,6 @@ default_config_data = {
     'mac': '',
     'mode': '802.3ad',
     'member': [],
-    'member_remove': [],
     'mtu': 1500,
     'primary': '',
     'vif_s': [],
@@ -260,12 +259,6 @@ def get_config():
     if conf.exists('member interface'):
         bond['member'] = conf.return_values('member interface')
 
-    # Determine bond member interface (currently effective) - to determine which
-    # interfaces is no longer assigend to the bond and thus can be removed
-    eff_intf = conf.return_effective_values('member interface')
-    act_intf = conf.return_values('member interface')
-    bond['member_remove'] = diff(eff_intf, act_intf)
-
     # Determine interface addresses (currently effective) - to determine which
     # address is no longer valid and needs to be removed from the bond
     eff_addr = conf.return_effective_values('address')
-- 
cgit v1.2.3


From 1fc4c201d7771ac4164e9480d55a654b7674d098 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Tue, 3 Sep 2019 18:58:27 +0200
Subject: bonding: T1614: T1557: add vif/vif-s VLAN interface support

Support for vif-c interfaces is still missing
---
 python/vyos/ifconfig.py            |  47 ++++++++++++++++-
 src/conf_mode/interface-bonding.py | 101 +++++++++++++++++++++++++++++++++----
 2 files changed, 136 insertions(+), 12 deletions(-)

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/python/vyos/ifconfig.py b/python/vyos/ifconfig.py
index 1d03e4e74..bb2d23d0d 100644
--- a/python/vyos/ifconfig.py
+++ b/python/vyos/ifconfig.py
@@ -87,7 +87,9 @@ class Interface:
 
     def remove(self):
         """
-        Remove system interface
+        Remove interface from operating system. Removing the interface
+        deconfigures all assigned IP addresses and clear possible DHCP(v6)
+        client processes.
 
         Example:
         >>> from vyos.ifconfig import Interface
@@ -964,10 +966,53 @@ class BridgeIf(Interface):
                                  .format(self._ifname, interface), priority)
 
 
+
 class EthernetIf(Interface):
     def __init__(self, ifname, type=None):
         super().__init__(ifname, type)
 
+    def add_vlan(self, vlan_id, ethertype=''):
+        """
+        A virtual LAN (VLAN) is any broadcast domain that is partitioned and
+        isolated in a computer network at the data link layer (OSI layer 2).
+        Use this function to create a new VLAN interface on a given physical
+        interface.
+
+        This function creates both 802.1q and 802.1ad (Q-in-Q) interfaces. Proto
+        parameter is used to indicate VLAN type.
+
+        A new object of type EthernetIf is returned once the interface has been
+        created.
+        """
+        vlan_ifname = self._ifname + '.' + str(vlan_id)
+        if not os.path.exists('/sys/class/net/{}'.format(vlan_ifname)):
+            self._vlan_id = int(vlan_id)
+
+            if ethertype:
+                self._ethertype = ethertype
+                ethertype='proto {}'.format(ethertype)
+
+            # create interface in the system
+            cmd = 'ip link add link {intf} name {intf}.{vlan} type vlan {proto} id {vlan}'.format(intf=self._ifname, vlan=self._vlan_id, proto=ethertype)
+            self._cmd(cmd)
+
+        # return new object mapping to the newly created interface
+        # we can now work on this object for e.g. IP address setting
+        # or interface description and so on
+        return EthernetIf(vlan_ifname)
+
+
+    def del_vlan(self, vlan_id):
+        """
+        Remove VLAN interface from operating system. Removing the interface
+        deconfigures all assigned IP addresses and clear possible DHCP(v6)
+        client processes.
+        """
+        vlan_ifname = self._ifname + '.' + str(vlan_id)
+        tmp = EthernetIf(vlan_ifname)
+        tmp.remove()
+
+
 class BondIf(EthernetIf):
     """
     The Linux bonding driver provides a method for aggregating multiple network
diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index a623ca524..9e7be19f5 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -21,7 +21,8 @@ import os
 from copy import deepcopy
 from sys import exit
 from netifaces import interfaces
-from vyos.ifconfig import BondIf
+
+from vyos.ifconfig import BondIf, EthernetIf
 from vyos.config import Config
 from vyos import ConfigError
 
@@ -49,7 +50,9 @@ default_config_data = {
     'mtu': 1500,
     'primary': '',
     'vif_s': [],
-    'vif': []
+    'vif_s_remove': [],
+    'vif': [],
+    'vif_remove': []
 }
 
 def diff(first, second):
@@ -74,6 +77,15 @@ def get_bond_mode(mode):
     else:
         raise ConfigError('invalid bond mode "{}"'.format(mode))
 
+def get_ethertype(ethertype_val):
+    if ethertype_val == '0x88A8':
+        return '802.1ad'
+    elif ethertype_val == '0x8100':
+        return '802.1q'
+    else:
+        raise ConfigError('invalid ethertype "{}"'.format(ethertype_val))
+
+
 def vlan_to_dict(conf):
     """
     Common used function which will extract VLAN related information from config
@@ -82,7 +94,7 @@ def vlan_to_dict(conf):
     Function call's itself recursively if a vif-s/vif-c pair is detected.
     """
     vlan = {
-        'id': conf.get_level().split(' ')[-1], # get the '100' in 'interfaces bonding bond0 vif-s 100'
+        'id': conf.get_level().split()[-1], # get the '100' in 'interfaces bonding bond0 vif-s 100'
         'address': [],
         'address_remove': [],
         'description': '',
@@ -133,9 +145,14 @@ def vlan_to_dict(conf):
     if conf.exists('disable'):
         vlan['disable'] = True
 
-    # ethertype (only on vif-s nodes)
-    if conf.exists('ethertype'):
-        vlan['ethertype'] = conf.return_value('ethertype')
+    # ethertype is mandatory on vif-s nodes and only exists here!
+    # check if this is a vif-s node at all:
+    if conf.get_level().split()[-2] == 'vif-s':
+        # ethertype uses a default of 0x88A8
+        tmp = '0x88A8'
+        if conf.exists('ethertype'):
+             tmp = conf.return_value('ethertype')
+        vlan['ethertype'] = get_ethertype(tmp)
 
     # Media Access Control (MAC) address
     if conf.exists('mac'):
@@ -158,6 +175,39 @@ def vlan_to_dict(conf):
 
     return vlan
 
+
+def apply_vlan_config(vlan, config):
+    """
+    Generic function to apply a VLAN configuration from a dictionary
+    to a VLAN interface
+    """
+
+    if type(vlan) != type(EthernetIf("lo")):
+        raise TypeError()
+
+    # Configure interface address(es)
+    for addr in config['address_remove']:
+        vlan.del_addr(addr)
+    for addr in config['address']:
+        vlan.add_addr(addr)
+
+    # update interface description used e.g. within SNMP
+    vlan.ifalias = config['description']
+    # ignore link state changes
+    vlan.link_detect = config['disable_link_detect']
+    # Maximum Transmission Unit (MTU)
+    vlan.mtu = config['mtu']
+    # Change VLAN interface MAC address
+    if config['mac']:
+        vlan.mac = config['mac']
+
+    # enable/disable VLAN interface
+    if config['disable']:
+        vlan.state = 'down'
+    else:
+        vlan.state = 'up'
+
+
 def get_config():
     # initialize kernel module if not loaded
     if not os.path.isfile('/sys/class/net/bonding_masters'):
@@ -271,6 +321,12 @@ def get_config():
 
     # re-set configuration level and retrieve vif-s interfaces
     conf.set_level(cfg_base)
+    # Determine vif-s interfaces (currently effective) - to determine which
+    # vif-s interface is no longer present and needs to be removed
+    eff_intf = conf.list_effective_nodes('vif-s')
+    act_intf = conf.list_nodes('vif-s')
+    bond['vif_s_remove'] = diff(eff_intf, act_intf)
+
     if conf.exists('vif-s'):
         for vif_s in conf.list_nodes('vif-s'):
             # set config level to vif-s interface
@@ -279,6 +335,12 @@ def get_config():
 
     # re-set configuration level and retrieve vif-s interfaces
     conf.set_level(cfg_base)
+    # Determine vif interfaces (currently effective) - to determine which
+    # vif interface is no longer present and needs to be removed
+    eff_intf = conf.list_effective_nodes('vif')
+    act_intf = conf.list_nodes('vif')
+    bond['vif_remove'] = diff(eff_intf, act_intf)
+
     if conf.exists('vif'):
         for vif in conf.list_nodes('vif'):
             # set config level to vif interface
@@ -287,6 +349,7 @@ def get_config():
 
     return bond
 
+
 def verify(bond):
     if len (bond['arp_mon_tgt']) > 16:
         raise ConfigError('The maximum number of targets that can be specified is 16')
@@ -300,9 +363,11 @@ def verify(bond):
 
     return None
 
+
 def generate(bond):
     return None
 
+
 def apply(bond):
     b = BondIf(bond['intf'])
 
@@ -378,13 +443,27 @@ def apply(bond):
         # Primary device interface
         b.primary = bond['primary']
 
-        #
-        # VLAN config goes here
-        #
-
         # Add (enslave) interfaces to bond
         for intf in bond['member']:
-                b.add_port(intf)
+            b.add_port(intf)
+
+        # remove no longer required service VLAN interfaces (vif-s)
+        for vif_s in bond['vif_s_remove']:
+            b.del_vlan(vif_s)
+
+        # create service VLAN interfaces (vif-s)
+        for vif_s in bond['vif_s']:
+            vlan = b.add_vlan(vif_s['id'], ethertype=vif_s['ethertype'])
+            apply_vlan_config(vlan, vif_s)
+
+        # remove no longer required VLAN interfaces (vif)
+        for vif in bond['vif_remove']:
+            b.del_vlan(vif)
+
+        # create VLAN interfaces (vif)
+        for vif in bond['vif']:
+            vlan = b.add_vlan(vif['id'])
+            apply_vlan_config(vlan, vif)
 
         # As the bond interface is always disabled first when changing
         # parameters we will only re-enable the interface if it is not
-- 
cgit v1.2.3


From da2bb10b8e4c15aaf993fc82eb0f9b8c564a2113 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Tue, 3 Sep 2019 21:55:49 +0200
Subject: bonding: T1614: identical ID on vif and vif-s is not allowed

---
 src/conf_mode/interface-bonding.py | 5 +++++
 1 file changed, 5 insertions(+)

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index 9e7be19f5..c2f0086aa 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -361,6 +361,11 @@ def verify(bond):
         if bond['primary'] not in bond['member']:
             raise ConfigError('Interface "{}" is not part of the bond'.format(bond['primary']))
 
+    for vif_s in bond['vif_s']:
+        for vif in bond['vif']:
+            if vif['id'] == vif_s['id']:
+                raise ConfigError('Can not use identical ID on vif and vif-s interface')
+
     return None
 
 
-- 
cgit v1.2.3


From 0d156a3947981e71774359b4566ac2af5892abe9 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Tue, 3 Sep 2019 22:23:38 +0200
Subject: bonding: T1614: add vif-c VLAN interface support

Tested using:
=============
set interfaces bonding bond0 address 192.0.2.1/24
set interfaces bonding bond0 description "VyOS bonding"
set interfaces bonding bond0 disable-link-detect
set interfaces bonding bond0 hash-policy layer2+3
set interfaces bonding bond0 ip arp-cache-timeout 86400
set interfaces bonding bond0 mac 00:91:00:00:00:01
set interfaces bonding bond0 mode active-backup
set interfaces bonding bond0 mtu 9000
set interfaces bonding bond0 member interface eth1
set interfaces bonding bond0 member interface eth2
set interfaces bonding bond0 vif-s 100 address 192.168.10.1/24
set interfaces bonding bond0 vif-s 100 description "802.1ad service VLAN 100"
set interfaces bonding bond0 vif-s 100 mtu 1500
set interfaces bonding bond0 vif-s 100 mac 00:91:00:00:00:02
set interfaces bonding bond0 vif-s 100 vif-c 110 address "192.168.110.1/24"
set interfaces bonding bond0 vif-s 100 vif-c 110 description "client VLAN 110"
set interfaces bonding bond0 vif-s 100 vif-c 120 address "192.168.120.1/24"
set interfaces bonding bond0 vif-s 100 vif-c 120 description "client VLAN 120"
set interfaces bonding bond0 vif-s 100 vif-c 130 address "192.168.130.1/24"
set interfaces bonding bond0 vif-s 100 vif-c 130 description "client VLAN 130"
set interfaces bonding bond0 vif 400 address 192.168.40.1/24
set interfaces bonding bond0 vif 400 description "802.1q VLAN 400"
set interfaces bonding bond0 vif 400 mtu 1500
set interfaces bonding bond0 vif 400 mac 00:91:00:00:00:03
---
 src/conf_mode/interface-bonding.py | 65 ++++++++++++++++++++++++--------------
 1 file changed, 42 insertions(+), 23 deletions(-)

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index c2f0086aa..b01018e5b 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -145,33 +145,41 @@ def vlan_to_dict(conf):
     if conf.exists('disable'):
         vlan['disable'] = True
 
+    # Media Access Control (MAC) address
+    if conf.exists('mac'):
+        vlan['mac'] = conf.return_value('mac')
+
+    # Maximum Transmission Unit (MTU)
+    if conf.exists('mtu'):
+        vlan['mtu'] = int(conf.return_value('mtu'))
+
     # ethertype is mandatory on vif-s nodes and only exists here!
     # check if this is a vif-s node at all:
     if conf.get_level().split()[-2] == 'vif-s':
+        vlan['vif_c'] = []
+        vlan['vif_c_remove'] = []
+
         # ethertype uses a default of 0x88A8
         tmp = '0x88A8'
         if conf.exists('ethertype'):
              tmp = conf.return_value('ethertype')
         vlan['ethertype'] = get_ethertype(tmp)
 
-    # Media Access Control (MAC) address
-    if conf.exists('mac'):
-        vlan['mac'] = conf.return_value('mac')
-
-    # Maximum Transmission Unit (MTU)
-    if conf.exists('mtu'):
-        vlan['mtu'] = int(conf.return_value('mtu'))
-
-    # check if there is a Q-in-Q vlan customer interface
-    # and call this function recursively
-    if conf.exists('vif-c'):
-        cfg_level = conf.get_level()
-        # add new key (vif-c) to dictionary
-        vlan['vif-c'] = []
-        for vif in conf.list_nodes('vif-c'):
-            # set config level to vif interface
-            conf.set_level(cfg_level + ' vif-c ' + vif)
-            vlan['vif-c'].append(vlan_to_dict(conf))
+        # get vif-c interfaces (currently effective) - to determine which vif-c
+        # interface is no longer present and needs to be removed
+        eff_intf = conf.list_effective_nodes('vif-c')
+        act_intf = conf.list_nodes('vif-c')
+        vlan['vif_c_remove'] = diff(eff_intf, act_intf)
+
+        # check if there is a Q-in-Q vlan customer interface
+        # and call this function recursively
+        if conf.exists('vif-c'):
+            cfg_level = conf.get_level()
+            # add new key (vif-c) to dictionary
+            for vif in conf.list_nodes('vif-c'):
+                # set config level to vif interface
+                conf.set_level(cfg_level + ' vif-c ' + vif)
+                vlan['vif_c'].append(vlan_to_dict(conf))
 
     return vlan
 
@@ -309,7 +317,7 @@ def get_config():
     if conf.exists('member interface'):
         bond['member'] = conf.return_values('member interface')
 
-    # Determine interface addresses (currently effective) - to determine which
+    # get interface addresses (currently effective) - to determine which
     # address is no longer valid and needs to be removed from the bond
     eff_addr = conf.return_effective_values('address')
     act_addr = conf.return_values('address')
@@ -321,8 +329,8 @@ def get_config():
 
     # re-set configuration level and retrieve vif-s interfaces
     conf.set_level(cfg_base)
-    # Determine vif-s interfaces (currently effective) - to determine which
-    # vif-s interface is no longer present and needs to be removed
+    # get vif-s interfaces (currently effective) - to determine which vif-s
+    # interface is no longer present and needs to be removed
     eff_intf = conf.list_effective_nodes('vif-s')
     act_intf = conf.list_nodes('vif-s')
     bond['vif_s_remove'] = diff(eff_intf, act_intf)
@@ -458,8 +466,19 @@ def apply(bond):
 
         # create service VLAN interfaces (vif-s)
         for vif_s in bond['vif_s']:
-            vlan = b.add_vlan(vif_s['id'], ethertype=vif_s['ethertype'])
-            apply_vlan_config(vlan, vif_s)
+            s_vlan = b.add_vlan(vif_s['id'], ethertype=vif_s['ethertype'])
+            apply_vlan_config(s_vlan, vif_s)
+
+            # remove no longer required client VLAN interfaces (vif-c)
+            # on lower service VLAN interface
+            for vif_c in vif_s['vif_c_remove']:
+                s_vlan.del_vlan(vif_c)
+
+            # create client VLAN interfaces (vif-c)
+            # on lower service VLAN interface
+            for vif_c in vif_s['vif_c']:
+                c_vlan = s_vlan.add_vlan(vif_c['id'])
+                apply_vlan_config(c_vlan, vif_c)
 
         # remove no longer required VLAN interfaces (vif)
         for vif in bond['vif_remove']:
-- 
cgit v1.2.3


From 07ebd7589a961aaa8d4f3099836dd94e4bce2379 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Wed, 4 Sep 2019 15:52:39 +0200
Subject: bonding: T1614: T532: new commit validators

As in the past during the priority race of the bash script invalid configuration
could appear in the CLI and are de-synced from the kernle state, e.g. some
bonding modes do not support arp_interval.

This is no longer allowed and added to the migration script so that the config
again represents the truth.
---
 src/conf_mode/interface-bonding.py      | 53 ++++++++++++++++++++++++++++-----
 src/migration-scripts/interfaces/1-to-2 | 19 ++++++++++++
 2 files changed, 65 insertions(+), 7 deletions(-)

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index b01018e5b..03d28954d 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -249,7 +249,7 @@ def get_config():
 
     # ARP link monitoring frequency in milliseconds
     if conf.exists('arp-monitor interval'):
-        bond['arp_mon_intvl'] = conf.return_value('arp-monitor interval')
+        bond['arp_mon_intvl'] = int(conf.return_value('arp-monitor interval'))
 
     # IP address to use for ARP monitoring
     if conf.exists('arp-monitor target'):
@@ -374,6 +374,43 @@ def verify(bond):
             if vif['id'] == vif_s['id']:
                 raise ConfigError('Can not use identical ID on vif and vif-s interface')
 
+
+    conf = Config()
+    for intf in bond['member']:
+        # we can not add disabled slave interfaces to our bond
+        if conf.exists('interfaces ethernet ' + intf + ' disable'):
+            raise ConfigError('can not add disabled interface {} to {}'.format(intf, bond['intf']))
+
+        # can not add interfaces with an assigned address to a bond
+        if conf.exists('interfaces ethernet ' + intf + ' address'):
+            raise ConfigError('can not add interface {} with an assigned address to {}'.format(intf, bond['intf']))
+
+        # bond members are not allowed to be bridge members, too
+        for bridge in conf.list_nodes('interfaces bridge'):
+            if conf.exists('interfaces bridge ' + bridge + ' member interface ' + intf):
+                raise ConfigError('can not add interface {} that is part of bridge {} to {}'.format(intf, bridge, bond['intf']))
+
+        # bond members are not allowed to be vrrp members, too
+        for vrrp in conf.list_nodes('high-availability vrrp group'):
+            if conf.exists('high-availability vrrp group ' + vrrp + ' interface ' + intf):
+                raise ConfigError('can not add interface {} which belongs to a VRRP group to {}'.format(intf, bond['intf']))
+
+        # bond members are not allowed to be underlaying psuedo-ethernet devices
+        for peth in conf.list_nodes('interfaces pseudo-ethernet'):
+            if conf.exists('interfaces pseudo-ethernet ' + peth + ' link ' + intf):
+                raise ConfigError('can not add interface {} used by pseudo-ethernet {} to {}'.format(intf, peth, bond['intf']))
+
+    if bond['primary']:
+        if bond['primary'] not in bond['member']:
+            raise ConfigError('primary interface must be a member interface of {}'.format(bond['intf']))
+
+        if bond['mode'] not in ['active-backup', 'balance-tlb', 'balance-alb']:
+            raise ConfigError('primary interface only works for mode active-backup, transmit-load-balance or adaptive-load-balance')
+
+    if bond['arp_mon_intvl'] > 0:
+        if bond['mode'] in ['802.3ad', 'balance-tlb', 'balance-alb']:
+            raise ConfigError('ARP link monitoring does not work for mode 802.3ad, transmit-load-balance or adaptive-load-balance')
+
     return None
 
 
@@ -392,6 +429,11 @@ def apply(bond):
         # Always disable the bond prior changing anything
         b.state = 'down'
 
+        # The bonding mode can not be changed when there are interfaces enslaved
+        # to this bond, thus we will free all interfaces from the bond first!
+        for intf in b.get_slaves():
+            b.del_port(intf)
+
         # Configure interface address(es)
         for addr in bond['address_remove']:
             b.del_addr(addr)
@@ -444,17 +486,14 @@ def apply(bond):
         if bond['mac']:
             b.mac = bond['mac']
 
-        # The bonding mode can not be changed when there are interfaces enslaved
-        # to this bond, thus we will free all interfaces from the bond first!
-        for intf in b.get_slaves():
-            b.del_port(intf)
-
         # Bonding policy
         b.mode = bond['mode']
         # Maximum Transmission Unit (MTU)
         b.mtu = bond['mtu']
+
         # Primary device interface
-        b.primary = bond['primary']
+        if bond['primary']:
+            b.primary = bond['primary']
 
         # Add (enslave) interfaces to bond
         for intf in bond['member']:
diff --git a/src/migration-scripts/interfaces/1-to-2 b/src/migration-scripts/interfaces/1-to-2
index 10d542d1d..050137318 100755
--- a/src/migration-scripts/interfaces/1-to-2
+++ b/src/migration-scripts/interfaces/1-to-2
@@ -36,6 +36,25 @@ else:
             # create new bond member interface
             config.set(base + [bond, 'member', 'interface'], value=intf, replace=False)
 
+    #
+    # some combinations were allowed in the past from a CLI perspective
+    # but the kernel overwrote them - remove from CLI to not confuse the users.
+    # In addition new consitency checks are in place so users can't repeat the
+    # mistake. One of those nice issues is https://phabricator.vyos.net/T532
+    for bond in config.list_nodes(base):
+        if config.exists(base + [bond, 'arp-monitor', 'interval']) and config.exists(base + [bond, 'mode']):
+            mode = config.return_value(base + [bond, 'mode'])
+            if mode in ['802.3ad', 'transmit-load-balance', 'adaptive-load-balance']:
+                intvl = int(config.return_value(base + [bond, 'arp-monitor', 'interval']))
+                if intvl > 0:
+                    # this is not allowed and the linux kernel replies with:
+                    # option arp_interval: mode dependency failed, not supported in mode 802.3ad(4)
+                    # option arp_interval: mode dependency failed, not supported in mode balance-alb(6)
+                    # option arp_interval: mode dependency failed, not supported in mode balance-tlb(5)
+                    #
+                    # so we simply disable arp_interval by setting it to 0 and miimon will take care about the link
+                    config.set(base + [bond, 'arp-monitor', 'interval'], value='0')
+
     try:
         with open(file_name, 'w') as f:
             f.write(config.to_string())
-- 
cgit v1.2.3


From 64d58eda4c1ebaa9d346d65d606d2a75694467ee Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Wed, 4 Sep 2019 21:50:04 +0200
Subject: Python/configdict: add list_diff function to compare two lists

A list containing only unique elements not part of the other list is
returned. This is usefull to check e.g. which IP addresses need to be
removed from the OS.
---
 python/vyos/configdict.py           |  8 ++++++++
 src/conf_mode/interface-bonding.py  | 11 +++++------
 src/conf_mode/interface-bridge.py   | 12 +++++-------
 src/conf_mode/interface-dummy.py    | 10 ++++------
 src/conf_mode/interface-loopback.py |  7 +++----
 5 files changed, 25 insertions(+), 23 deletions(-)

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py
index 157011839..a723c5322 100644
--- a/python/vyos/configdict.py
+++ b/python/vyos/configdict.py
@@ -78,3 +78,11 @@ def retrieve_config(path_hash, base_path, config):
                 config_hash[k][node] = retrieve_config(inner_hash, path + [node], config)
 
     return config_hash
+
+
+def list_diff(first, second):
+    """
+    Diff two dictionaries and return only unique items
+    """
+    second = set(second)
+    return [item for item in first if item not in second]
diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index 03d28954d..ba4577900 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -23,6 +23,7 @@ from sys import exit
 from netifaces import interfaces
 
 from vyos.ifconfig import BondIf, EthernetIf
+from vyos.configdict import list_diff
 from vyos.config import Config
 from vyos import ConfigError
 
@@ -55,9 +56,6 @@ default_config_data = {
     'vif_remove': []
 }
 
-def diff(first, second):
-    second = set(second)
-    return [item for item in first if item not in second]
 
 def get_bond_mode(mode):
     if mode == 'round-robin':
@@ -77,6 +75,7 @@ def get_bond_mode(mode):
     else:
         raise ConfigError('invalid bond mode "{}"'.format(mode))
 
+
 def get_ethertype(ethertype_val):
     if ethertype_val == '0x88A8':
         return '802.1ad'
@@ -321,7 +320,7 @@ def get_config():
     # address is no longer valid and needs to be removed from the bond
     eff_addr = conf.return_effective_values('address')
     act_addr = conf.return_values('address')
-    bond['address_remove'] = diff(eff_addr, act_addr)
+    bond['address_remove'] = list_diff(eff_addr, act_addr)
 
     # Primary device interface
     if conf.exists('primary'):
@@ -333,7 +332,7 @@ def get_config():
     # interface is no longer present and needs to be removed
     eff_intf = conf.list_effective_nodes('vif-s')
     act_intf = conf.list_nodes('vif-s')
-    bond['vif_s_remove'] = diff(eff_intf, act_intf)
+    bond['vif_s_remove'] = list_diff(eff_intf, act_intf)
 
     if conf.exists('vif-s'):
         for vif_s in conf.list_nodes('vif-s'):
@@ -347,7 +346,7 @@ def get_config():
     # vif interface is no longer present and needs to be removed
     eff_intf = conf.list_effective_nodes('vif')
     act_intf = conf.list_nodes('vif')
-    bond['vif_remove'] = diff(eff_intf, act_intf)
+    bond['vif_remove'] = list_diff(eff_intf, act_intf)
 
     if conf.exists('vif'):
         for vif in conf.list_nodes('vif'):
diff --git a/src/conf_mode/interface-bridge.py b/src/conf_mode/interface-bridge.py
index 1d3587114..b165428ee 100755
--- a/src/conf_mode/interface-bridge.py
+++ b/src/conf_mode/interface-bridge.py
@@ -21,8 +21,10 @@ import os
 from copy import deepcopy
 from sys import exit
 from netifaces import interfaces
-from vyos.config import Config
+
 from vyos.ifconfig import BridgeIf, Interface
+from vyos.configdict import list_diff
+from vyos.config import Config
 from vyos import ConfigError
 
 default_config_data = {
@@ -46,10 +48,6 @@ default_config_data = {
     'stp': 0
 }
 
-def diff(first, second):
-    second = set(second)
-    return [item for item in first if item not in second]
-
 def get_config():
     bridge = deepcopy(default_config_data)
     conf = Config()
@@ -137,13 +135,13 @@ def get_config():
     # interfaces is no longer assigend to the bridge and thus can be removed
     eff_intf = conf.list_effective_nodes('member interface')
     act_intf = conf.list_nodes('member interface')
-    bridge['member_remove'] = diff(eff_intf, act_intf)
+    bridge['member_remove'] = list_diff(eff_intf, act_intf)
 
     # Determine interface addresses (currently effective) - to determine which
     # address is no longer valid and needs to be removed from the bridge
     eff_addr = conf.return_effective_values('address')
     act_addr = conf.return_values('address')
-    bridge['address_remove'] = diff(eff_addr, act_addr)
+    bridge['address_remove'] = list_diff(eff_addr, act_addr)
 
     # Priority for this bridge
     if conf.exists('priority'):
diff --git a/src/conf_mode/interface-dummy.py b/src/conf_mode/interface-dummy.py
index 03afdc668..4a1179672 100755
--- a/src/conf_mode/interface-dummy.py
+++ b/src/conf_mode/interface-dummy.py
@@ -19,8 +19,10 @@
 from os import environ
 from copy import deepcopy
 from sys import exit
-from vyos.config import Config
+
 from vyos.ifconfig import DummyIf
+from vyos.configdict import list_diff
+from vyos.config import Config
 from vyos import ConfigError
 
 default_config_data = {
@@ -32,10 +34,6 @@ default_config_data = {
     'intf': ''
 }
 
-def diff(first, second):
-    second = set(second)
-    return [item for item in first if item not in second]
-
 def get_config():
     dummy = deepcopy(default_config_data)
     conf = Config()
@@ -70,7 +68,7 @@ def get_config():
     # address is no longer valid and needs to be removed from the interface
     eff_addr = conf.return_effective_values('address')
     act_addr = conf.return_values('address')
-    dummy['address_remove'] = diff(eff_addr, act_addr)
+    dummy['address_remove'] = list_diff(eff_addr, act_addr)
 
     return dummy
 
diff --git a/src/conf_mode/interface-loopback.py b/src/conf_mode/interface-loopback.py
index be47324c1..e2df37655 100755
--- a/src/conf_mode/interface-loopback.py
+++ b/src/conf_mode/interface-loopback.py
@@ -18,7 +18,9 @@
 from os import environ
 from sys import exit
 from copy import deepcopy
+
 from vyos.ifconfig import LoopbackIf
+from vyos.configdict import list_diff
 from vyos.config import Config
 from vyos import ConfigError
 
@@ -29,9 +31,6 @@ default_config_data = {
     'description': '',
 }
 
-def diff(first, second):
-    second = set(second)
-    return [item for item in first if item not in second]
 
 def get_config():
     loopback = deepcopy(default_config_data)
@@ -62,7 +61,7 @@ def get_config():
     # address is no longer valid and needs to be removed from the interface
     eff_addr = conf.return_effective_values('address')
     act_addr = conf.return_values('address')
-    loopback['address_remove'] = diff(eff_addr, act_addr)
+    loopback['address_remove'] = list_diff(eff_addr, act_addr)
 
     return loopback
 
-- 
cgit v1.2.3


From 952871200ecee584e7ed1bcb37bdaa06111e3a72 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Wed, 4 Sep 2019 21:50:46 +0200
Subject: Python/configdict: add function vlan_to_dict

A generic function which can parse the VLAN (vif, vif-s, cif-c) nodes in a
config session. A dictionary describing the VLAN is returned.

A good example will be the interface-bonding.py script used to generate bond
interfaces in the system. It is used as follows:

  if conf.exists('vif'):
      for vif in conf.list_nodes('vif'):
          # set config level to vif interface
          conf.set_level(cfg_base + ' vif ' + vif)
          bond['vif'].append(vlan_to_dict(conf))
---
 python/vyos/configdict.py          | 108 ++++++++++++++++++++++++++++++++++++
 src/conf_mode/interface-bonding.py | 109 +------------------------------------
 2 files changed, 109 insertions(+), 108 deletions(-)

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py
index a723c5322..4bc8863bb 100644
--- a/python/vyos/configdict.py
+++ b/python/vyos/configdict.py
@@ -18,6 +18,7 @@ A library for retrieving value dicts from VyOS configs in a declarative fashion.
 
 """
 
+from vyos import ConfigError
 
 def retrieve_config(path_hash, base_path, config):
     """
@@ -86,3 +87,110 @@ def list_diff(first, second):
     """
     second = set(second)
     return [item for item in first if item not in second]
+
+
+def get_ethertype(ethertype_val):
+    if ethertype_val == '0x88A8':
+        return '802.1ad'
+    elif ethertype_val == '0x8100':
+        return '802.1q'
+    else:
+        raise ConfigError('invalid ethertype "{}"'.format(ethertype_val))
+
+
+def vlan_to_dict(conf):
+    """
+    Common used function which will extract VLAN related information from config
+    and represent the result as Python dictionary.
+
+    Function call's itself recursively if a vif-s/vif-c pair is detected.
+    """
+    vlan = {
+        'id': conf.get_level().split()[-1], # get the '100' in 'interfaces bonding bond0 vif-s 100'
+        'address': [],
+        'address_remove': [],
+        'description': '',
+        'dhcp_client_id': '',
+        'dhcp_hostname': '',
+        'dhcpv6_prm_only': False,
+        'dhcpv6_temporary': False,
+        'disable': False,
+        'disable_link_detect': 1,
+        'mac': '',
+        'mtu': 1500
+    }
+    # retrieve configured interface addresses
+    if conf.exists('address'):
+        vlan['address'] = conf.return_values('address')
+
+    # Determine interface addresses (currently effective) - to determine which
+    # address is no longer valid and needs to be removed from the bond
+    eff_addr = conf.return_effective_values('address')
+    act_addr = conf.return_values('address')
+    vlan['address_remove'] = list_diff(eff_addr, act_addr)
+
+    # retrieve interface description
+    if conf.exists('description'):
+        vlan['description'] = conf.return_value('description')
+
+    # get DHCP client identifier
+    if conf.exists('dhcp-options client-id'):
+        vlan['dhcp_client_id'] = conf.return_value('dhcp-options client-id')
+
+    # DHCP client host name (overrides the system host name)
+    if conf.exists('dhcp-options host-name'):
+        vlan['dhcp_hostname'] = conf.return_value('dhcp-options host-name')
+
+    # DHCPv6 only acquire config parameters, no address
+    if conf.exists('dhcpv6-options parameters-only'):
+        vlan['dhcpv6_prm_only'] = conf.return_value('dhcpv6-options parameters-only')
+
+    # DHCPv6 temporary IPv6 address
+    if conf.exists('dhcpv6-options temporary'):
+        vlan['dhcpv6_temporary'] = conf.return_value('dhcpv6-options temporary')
+
+    # ignore link state changes
+    if conf.exists('disable-link-detect'):
+        vlan['disable_link_detect'] = 2
+
+    # disable bond interface
+    if conf.exists('disable'):
+        vlan['disable'] = True
+
+    # Media Access Control (MAC) address
+    if conf.exists('mac'):
+        vlan['mac'] = conf.return_value('mac')
+
+    # Maximum Transmission Unit (MTU)
+    if conf.exists('mtu'):
+        vlan['mtu'] = int(conf.return_value('mtu'))
+
+    # ethertype is mandatory on vif-s nodes and only exists here!
+    # check if this is a vif-s node at all:
+    if conf.get_level().split()[-2] == 'vif-s':
+        vlan['vif_c'] = []
+        vlan['vif_c_remove'] = []
+
+        # ethertype uses a default of 0x88A8
+        tmp = '0x88A8'
+        if conf.exists('ethertype'):
+             tmp = conf.return_value('ethertype')
+        vlan['ethertype'] = get_ethertype(tmp)
+
+        # get vif-c interfaces (currently effective) - to determine which vif-c
+        # interface is no longer present and needs to be removed
+        eff_intf = conf.list_effective_nodes('vif-c')
+        act_intf = conf.list_nodes('vif-c')
+        vlan['vif_c_remove'] = list_diff(eff_intf, act_intf)
+
+        # check if there is a Q-in-Q vlan customer interface
+        # and call this function recursively
+        if conf.exists('vif-c'):
+            cfg_level = conf.get_level()
+            # add new key (vif-c) to dictionary
+            for vif in conf.list_nodes('vif-c'):
+                # set config level to vif interface
+                conf.set_level(cfg_level + ' vif-c ' + vif)
+                vlan['vif_c'].append(vlan_to_dict(conf))
+
+    return vlan
diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index ba4577900..2ec764965 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -23,7 +23,7 @@ from sys import exit
 from netifaces import interfaces
 
 from vyos.ifconfig import BondIf, EthernetIf
-from vyos.configdict import list_diff
+from vyos.configdict import list_diff, vlan_to_dict
 from vyos.config import Config
 from vyos import ConfigError
 
@@ -76,113 +76,6 @@ def get_bond_mode(mode):
         raise ConfigError('invalid bond mode "{}"'.format(mode))
 
 
-def get_ethertype(ethertype_val):
-    if ethertype_val == '0x88A8':
-        return '802.1ad'
-    elif ethertype_val == '0x8100':
-        return '802.1q'
-    else:
-        raise ConfigError('invalid ethertype "{}"'.format(ethertype_val))
-
-
-def vlan_to_dict(conf):
-    """
-    Common used function which will extract VLAN related information from config
-    and represent the result as Python dictionary.
-
-    Function call's itself recursively if a vif-s/vif-c pair is detected.
-    """
-    vlan = {
-        'id': conf.get_level().split()[-1], # get the '100' in 'interfaces bonding bond0 vif-s 100'
-        'address': [],
-        'address_remove': [],
-        'description': '',
-        'dhcp_client_id': '',
-        'dhcp_hostname': '',
-        'dhcpv6_prm_only': False,
-        'dhcpv6_temporary': False,
-        'disable': False,
-        'disable_link_detect': 1,
-        'mac': '',
-        'mtu': 1500
-    }
-    # retrieve configured interface addresses
-    if conf.exists('address'):
-        vlan['address'] = conf.return_values('address')
-
-    # Determine interface addresses (currently effective) - to determine which
-    # address is no longer valid and needs to be removed from the bond
-    eff_addr = conf.return_effective_values('address')
-    act_addr = conf.return_values('address')
-    vlan['address_remove'] = diff(eff_addr, act_addr)
-
-    # retrieve interface description
-    if conf.exists('description'):
-        vlan['description'] = conf.return_value('description')
-
-    # get DHCP client identifier
-    if conf.exists('dhcp-options client-id'):
-        vlan['dhcp_client_id'] = conf.return_value('dhcp-options client-id')
-
-    # DHCP client host name (overrides the system host name)
-    if conf.exists('dhcp-options host-name'):
-        vlan['dhcp_hostname'] = conf.return_value('dhcp-options host-name')
-
-    # DHCPv6 only acquire config parameters, no address
-    if conf.exists('dhcpv6-options parameters-only'):
-        vlan['dhcpv6_prm_only'] = conf.return_value('dhcpv6-options parameters-only')
-
-    # DHCPv6 temporary IPv6 address
-    if conf.exists('dhcpv6-options temporary'):
-        vlan['dhcpv6_temporary'] = conf.return_value('dhcpv6-options temporary')
-
-    # ignore link state changes
-    if conf.exists('disable-link-detect'):
-        vlan['disable_link_detect'] = 2
-
-    # disable bond interface
-    if conf.exists('disable'):
-        vlan['disable'] = True
-
-    # Media Access Control (MAC) address
-    if conf.exists('mac'):
-        vlan['mac'] = conf.return_value('mac')
-
-    # Maximum Transmission Unit (MTU)
-    if conf.exists('mtu'):
-        vlan['mtu'] = int(conf.return_value('mtu'))
-
-    # ethertype is mandatory on vif-s nodes and only exists here!
-    # check if this is a vif-s node at all:
-    if conf.get_level().split()[-2] == 'vif-s':
-        vlan['vif_c'] = []
-        vlan['vif_c_remove'] = []
-
-        # ethertype uses a default of 0x88A8
-        tmp = '0x88A8'
-        if conf.exists('ethertype'):
-             tmp = conf.return_value('ethertype')
-        vlan['ethertype'] = get_ethertype(tmp)
-
-        # get vif-c interfaces (currently effective) - to determine which vif-c
-        # interface is no longer present and needs to be removed
-        eff_intf = conf.list_effective_nodes('vif-c')
-        act_intf = conf.list_nodes('vif-c')
-        vlan['vif_c_remove'] = diff(eff_intf, act_intf)
-
-        # check if there is a Q-in-Q vlan customer interface
-        # and call this function recursively
-        if conf.exists('vif-c'):
-            cfg_level = conf.get_level()
-            # add new key (vif-c) to dictionary
-            for vif in conf.list_nodes('vif-c'):
-                # set config level to vif interface
-                conf.set_level(cfg_level + ' vif-c ' + vif)
-                vlan['vif_c'].append(vlan_to_dict(conf))
-
-    return vlan
-
-
 def apply_vlan_config(vlan, config):
     """
     Generic function to apply a VLAN configuration from a dictionary
-- 
cgit v1.2.3


From 35c7d66165da2102ee8986c3999fe9fea16c38da Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Fri, 6 Sep 2019 10:10:11 +0200
Subject: Python/ifconfig: T1557: {add,del}_addr() now supports dhcp/dhcpv6

Instead of manually starting DHCP/DHCPv6 for every interface and have an
identical if/elif/else statement checking for dhcp/dhcpv6 rather move this
repeating stement into add_addr()/del_addr().

Single source is always preferred.
---
 python/vyos/ifconfig.py            | 111 ++++++++++++++++++++++++-------------
 src/conf_mode/interface-bonding.py |  56 ++++++++++---------
 src/conf_mode/interface-bridge.py  |  35 ++++--------
 3 files changed, 112 insertions(+), 90 deletions(-)

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/python/vyos/ifconfig.py b/python/vyos/ifconfig.py
index c5d3e447b..1886addfc 100644
--- a/python/vyos/ifconfig.py
+++ b/python/vyos/ifconfig.py
@@ -61,6 +61,7 @@ class Interface:
         >>> i = Interface('eth0')
         """
         self._ifname = str(ifname)
+        self._state = 'down'
 
         if not os.path.exists('/sys/class/net/{}'.format(ifname)) and not type:
             raise Exception('interface "{}" not found'.format(self._ifname))
@@ -111,8 +112,8 @@ class Interface:
         # All subinterfaces are now removed, continue on the physical interface
 
         # stop DHCP(v6) if running
-        self.del_dhcp()
-        self.del_dhcpv6()
+        self._del_dhcp()
+        self._del_dhcpv6()
 
         # NOTE (Improvement):
         # after interface removal no other commands should be allowed
@@ -359,6 +360,8 @@ class Interface:
         if state not in ['up', 'down']:
             raise ValueError('state must be "up" or "down"')
 
+        self._state = state
+
         # Assemble command executed on system. Unfortunately there is no way
         # to up/down an interface via sysfs
         cmd = 'ip link set dev {} {}'.format(self._ifname, state)
@@ -495,8 +498,14 @@ class Interface:
 
     def add_addr(self, addr):
         """
-        Add IP address to interface. Address is only added if it yet not added
-        to that interface.
+        Add IP(v6) address to interface. Address is only added if it is not
+        already assigned to that interface.
+
+        addr: can be an IPv4 address, IPv6 address, dhcp or dhcpv6!
+              IPv4: add IPv4 address to interface
+              IPv6: add IPv6 address to interface
+              dhcp: start dhclient (IPv4) on interface
+              dhcpv6: start dhclient (IPv6) on interface
 
         Example:
         >>> from vyos.ifconfig import Interface
@@ -506,13 +515,25 @@ class Interface:
         >>> j.get_addr()
         ['192.0.2.1/24', '2001:db8::ffff/64']
         """
-        if not is_intf_addr_assigned(self._ifname, addr):
-            cmd = 'ip addr add "{}" dev "{}"'.format(addr, self._ifname)
-            self._cmd(cmd)
+        if addr == 'dhcp':
+            self._set_dhcp()
+        elif addr == 'dhcpv6':
+            self._set_dhcpv6()
+        else:
+            if not is_intf_addr_assigned(self._ifname, addr):
+                cmd = 'ip addr add "{}" dev "{}"'.format(addr, self._ifname)
+                self._cmd(cmd)
 
     def del_addr(self, addr):
         """
-        Remove IP address from interface.
+        Delete IP(v6) address to interface. Address is only added if it is
+        assigned to that interface.
+
+        addr: can be an IPv4 address, IPv6 address, dhcp or dhcpv6!
+              IPv4: delete IPv4 address from interface
+              IPv6: delete IPv6 address from interface
+              dhcp: stop dhclient (IPv4) on interface
+              dhcpv6: stop dhclient (IPv6) on interface
 
         Example:
         >>> from vyos.ifconfig import Interface
@@ -525,12 +546,17 @@ class Interface:
         >>> j.get_addr()
         ['2001:db8::ffff/64']
         """
-        if is_intf_addr_assigned(self._ifname, addr):
-            cmd = 'ip addr del "{}" dev "{}"'.format(addr, self._ifname)
-            self._cmd(cmd)
+        if addr == 'dhcp':
+            self._del_dhcp()
+        elif addr == 'dhcpv6':
+            self._del_dhcpv6()
+        else:
+            if is_intf_addr_assigned(self._ifname, addr):
+                cmd = 'ip addr del "{}" dev "{}"'.format(addr, self._ifname)
+                self._cmd(cmd)
 
     # replace dhcpv4/v6 with systemd.networkd?
-    def set_dhcp(self):
+    def _set_dhcp(self):
         """
         Configure interface as DHCP client. The dhclient binary is automatically
         started in background!
@@ -557,15 +583,17 @@ class Interface:
         with open(self._dhcp_cfg_file, 'w') as f:
             f.write(dhcp_text)
 
-        cmd  = 'start-stop-daemon --start --quiet --pidfile ' + \
-            self._dhcp_pid_file
-        cmd += ' --exec /sbin/dhclient --'
-        # now pass arguments to dhclient binary
-        cmd += ' -4 -nw -cf {} -pf {} -lf {} {}'.format(
-            self._dhcp_cfg_file, self._dhcp_pid_file, self._dhcp_lease_file, self._ifname)
-        self._cmd(cmd)
+        if self._state == 'up':
+            cmd  = 'start-stop-daemon --start --quiet --pidfile ' + \
+                self._dhcp_pid_file
+            cmd += ' --exec /sbin/dhclient --'
+            # now pass arguments to dhclient binary
+            cmd += ' -4 -nw -cf {} -pf {} -lf {} {}'.format(
+                self._dhcp_cfg_file, self._dhcp_pid_file, self._dhcp_lease_file, self._ifname)
+            self._cmd(cmd)
 
-    def del_dhcp(self):
+
+    def _del_dhcp(self):
         """
         De-configure interface as DHCP clinet. All auto generated files like
         pid, config and lease will be removed.
@@ -601,7 +629,8 @@ class Interface:
         if os.path.isfile(self._dhcp_lease_file):
             os.remove(self._dhcp_lease_file)
 
-    def set_dhcpv6(self):
+
+    def _set_dhcpv6(self):
         """
         Configure interface as DHCPv6 client. The dhclient binary is automatically
         started in background!
@@ -622,25 +651,28 @@ class Interface:
         with open(self._dhcpv6_cfg_file, 'w') as f:
             f.write(dhcpv6_text)
 
-        # https://bugs.launchpad.net/ubuntu/+source/ifupdown/+bug/1447715
-        #
-        # wee need to wait for IPv6 DAD to finish once and interface is added
-        # this suxx :-(
-        sleep(5)
+        if self._state == 'up':
+            # https://bugs.launchpad.net/ubuntu/+source/ifupdown/+bug/1447715
+            #
+            # wee need to wait for IPv6 DAD to finish once and interface is added
+            # this suxx :-(
+            sleep(5)
 
-        # no longer accept router announcements on this interface
-        cmd = 'sysctl -q -w net.ipv6.conf.{}.accept_ra=0'.format(self._ifname)
-        self._cmd(cmd)
+            # no longer accept router announcements on this interface
+            cmd = 'sysctl -q -w net.ipv6.conf.{}.accept_ra=0'.format(self._ifname)
+            self._cmd(cmd)
 
-        cmd  = 'start-stop-daemon --start --quiet --pidfile ' + \
-            self._dhcpv6_pid_file
-        cmd += ' --exec /sbin/dhclient --'
-        # now pass arguments to dhclient binary
-        cmd += ' -6 -nw -cf {} -pf {} -lf {} {}'.format(
-            self._dhcpv6_cfg_file, self._dhcpv6_pid_file, self._dhcpv6_lease_file, self._ifname)
-        self._cmd(cmd)
+            # assemble command-line to start DHCPv6 client (dhclient)
+            cmd  = 'start-stop-daemon --start --quiet --pidfile ' + \
+                self._dhcpv6_pid_file
+            cmd += ' --exec /sbin/dhclient --'
+            # now pass arguments to dhclient binary
+            cmd += ' -6 -nw -cf {} -pf {} -lf {} {}'.format(
+                self._dhcpv6_cfg_file, self._dhcpv6_pid_file, self._dhcpv6_lease_file, self._ifname)
+            self._cmd(cmd)
 
-    def del_dhcpv6(self):
+
+    def _del_dhcpv6(self):
         """
         De-configure interface as DHCPv6 clinet. All auto generated files like
         pid, config and lease will be removed.
@@ -1281,7 +1313,6 @@ class BondIf(EthernetIf):
 
 
 class WireGuardIf(Interface):
-
     """
     Wireguard interface class, contains a comnfig dictionary since
     wireguard VPN is being comnfigured via the wg command rather than
@@ -1293,11 +1324,11 @@ class WireGuardIf(Interface):
     >>> from vyos.ifconfig import WireGuardIf as wg_if
     >>> wg_intfc = wg_if("wg01")
     >>> print (wg_intfc.wg_config)
-    {'private-key': None, 'keepalive': 0, 'endpoint': None, 'port': 0, 
+    {'private-key': None, 'keepalive': 0, 'endpoint': None, 'port': 0,
     'allowed-ips': [], 'pubkey': None, 'fwmark': 0, 'psk': '/dev/null'}
     >>> wg_intfc.wg_config['keepalive'] = 100
     >>> print (wg_intfc.wg_config)
-    {'private-key': None, 'keepalive': 100, 'endpoint': None, 'port': 0, 
+    {'private-key': None, 'keepalive': 100, 'endpoint': None, 'port': 0,
     'allowed-ips': [], 'pubkey': None, 'fwmark': 0, 'psk': '/dev/null'}
     """
 
diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index 2ec764965..81667289d 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -85,12 +85,6 @@ def apply_vlan_config(vlan, config):
     if type(vlan) != type(EthernetIf("lo")):
         raise TypeError()
 
-    # Configure interface address(es)
-    for addr in config['address_remove']:
-        vlan.del_addr(addr)
-    for addr in config['address']:
-        vlan.add_addr(addr)
-
     # update interface description used e.g. within SNMP
     vlan.ifalias = config['description']
     # ignore link state changes
@@ -107,6 +101,14 @@ def apply_vlan_config(vlan, config):
     else:
         vlan.state = 'up'
 
+    # Configure interface address(es)
+    # - not longer required addresses get removed first
+    # - newly addresses will be added second
+    for addr in config['address_remove']:
+        vlan.del_addr(addr)
+    for addr in config['address']:
+        vlan.add_addr(addr)
+
 
 def get_config():
     # initialize kernel module if not loaded
@@ -139,6 +141,11 @@ def get_config():
     if conf.exists('address'):
         bond['address'] = conf.return_values('address')
 
+    # get interface addresses (currently effective) - to determine which
+    # address is no longer valid and needs to be removed from the bond
+    eff_addr = conf.return_effective_values('address')
+    bond['address_remove'] = list_diff(eff_addr, bond['address'])
+
     # ARP link monitoring frequency in milliseconds
     if conf.exists('arp-monitor interval'):
         bond['arp_mon_intvl'] = int(conf.return_value('arp-monitor interval'))
@@ -209,12 +216,6 @@ def get_config():
     if conf.exists('member interface'):
         bond['member'] = conf.return_values('member interface')
 
-    # get interface addresses (currently effective) - to determine which
-    # address is no longer valid and needs to be removed from the bond
-    eff_addr = conf.return_effective_values('address')
-    act_addr = conf.return_values('address')
-    bond['address_remove'] = list_diff(eff_addr, act_addr)
-
     # Primary device interface
     if conf.exists('primary'):
         bond['primary'] = conf.return_value('primary')
@@ -314,6 +315,7 @@ def apply(bond):
     b = BondIf(bond['intf'])
 
     if bond['deleted']:
+        #
         # delete bonding interface
         b.remove()
     else:
@@ -326,12 +328,6 @@ def apply(bond):
         for intf in b.get_slaves():
             b.del_port(intf)
 
-        # Configure interface address(es)
-        for addr in bond['address_remove']:
-            b.del_addr(addr)
-        for addr in bond['address']:
-            b.add_addr(addr)
-
         # ARP link monitoring frequency
         b.arp_interval = bond['arp_mon_intvl']
         # reset miimon on arp-montior deletion
@@ -348,8 +344,8 @@ def apply(bond):
         # from the kernel side this looks valid to me. We won't run into an error
         # when a user added manual adresses which would result in having more
         # then 16 adresses in total.
-        cur_addr = list(map(str, b.arp_ip_target.split()))
-        for addr in cur_addr:
+        arp_tgt_addr = list(map(str, b.arp_ip_target.split()))
+        for addr in arp_tgt_addr:
             b.arp_ip_target = '-' + addr
 
         # Add configured ARP target addresses
@@ -391,6 +387,20 @@ def apply(bond):
         for intf in bond['member']:
             b.add_port(intf)
 
+        # As the bond interface is always disabled first when changing
+        # parameters we will only re-enable the interface if it is not
+        # administratively disabled
+        if not bond['disable']:
+            b.state = 'up'
+
+        # Configure interface address(es)
+        # - not longer required addresses get removed first
+        # - newly addresses will be added second
+        for addr in bond['address_remove']:
+            b.del_addr(addr)
+        for addr in bond['address']:
+            b.add_addr(addr)
+
         # remove no longer required service VLAN interfaces (vif-s)
         for vif_s in bond['vif_s_remove']:
             b.del_vlan(vif_s)
@@ -420,12 +430,6 @@ def apply(bond):
             vlan = b.add_vlan(vif['id'])
             apply_vlan_config(vlan, vif)
 
-        # As the bond interface is always disabled first when changing
-        # parameters we will only re-enable the interface if it is not
-        # administratively disabled
-        if not bond['disable']:
-            b.state = 'up'
-
     return None
 
 if __name__ == '__main__':
diff --git a/src/conf_mode/interface-bridge.py b/src/conf_mode/interface-bridge.py
index b165428ee..8996fceec 100755
--- a/src/conf_mode/interface-bridge.py
+++ b/src/conf_mode/interface-bridge.py
@@ -61,9 +61,7 @@ def get_config():
     # Check if bridge has been removed
     if not conf.exists('interfaces bridge ' + bridge['intf']):
         bridge['deleted'] = True
-        # we should not bail out early here b/c we should
-        # find possible DHCP interfaces later on.
-        # DHCP interfaces invoke dhclient which should be stopped, too
+        return bridge
 
     # set new configuration level
     conf.set_level('interfaces bridge ' + bridge['intf'])
@@ -72,6 +70,11 @@ def get_config():
     if conf.exists('address'):
         bridge['address'] = conf.return_values('address')
 
+    # Determine interface addresses (currently effective) - to determine which
+    # address is no longer valid and needs to be removed from the bridge
+    eff_addr = conf.return_effective_values('address')
+    bridge['address_remove'] = list_diff(eff_addr, bridge['address'])
+
     # retrieve aging - how long addresses are retained
     if conf.exists('aging'):
         bridge['aging'] = int(conf.return_value('aging'))
@@ -137,12 +140,6 @@ def get_config():
     act_intf = conf.list_nodes('member interface')
     bridge['member_remove'] = list_diff(eff_intf, act_intf)
 
-    # Determine interface addresses (currently effective) - to determine which
-    # address is no longer valid and needs to be removed from the bridge
-    eff_addr = conf.return_effective_values('address')
-    act_addr = conf.return_values('address')
-    bridge['address_remove'] = list_diff(eff_addr, act_addr)
-
     # Priority for this bridge
     if conf.exists('priority'):
         bridge['priority'] = int(conf.return_value('priority'))
@@ -225,23 +222,13 @@ def apply(bridge):
         if bridge['disable']:
             br.state = 'down'
 
-        # remove configured network interface addresses/DHCP(v6) configuration
+        # Configure interface address(es)
+        # - not longer required addresses get removed first
+        # - newly addresses will be added second
         for addr in bridge['address_remove']:
-            if addr == 'dhcp':
-                br.del_dhcp()
-            elif addr == 'dhcpv6':
-                br.del_dhcpv6()
-            else:
-                br.del_addr(addr)
-
-        # add configured network interface addresses/DHCP(v6) configuration
+            br.del_addr(addr)
         for addr in bridge['address']:
-            if addr == 'dhcp':
-                br.set_dhcp()
-            elif addr == 'dhcpv6':
-                br.set_dhcpv6()
-            else:
-                br.add_addr(addr)
+            br.add_addr(addr)
 
         # configure additional bridge member options
         for member in bridge['member']:
-- 
cgit v1.2.3


From 96e0f5697b181be4f2f3c5c763821ef574881a93 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Fri, 6 Sep 2019 10:41:55 +0200
Subject: bonding: T1614: enslaved interfaces can be added to only one bond at
 a time

---
 src/conf_mode/interface-bonding.py | 6 ++++++
 1 file changed, 6 insertions(+)

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index 81667289d..7b86a0528 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -270,6 +270,12 @@ def verify(bond):
 
     conf = Config()
     for intf in bond['member']:
+        # a bond member is only allowed to be assigned to any one bond
+        for tmp in conf.list_nodes('interfaces bonding'):
+            if conf.exists('interfaces bonding ' + tmp + ' member interface ' + intf):
+                raise ConfigError('can not add interface {} that is part of another bond ({}) to {}'.format(
+                    intf, tmp, bond['intf']))
+
         # we can not add disabled slave interfaces to our bond
         if conf.exists('interfaces ethernet ' + intf + ' disable'):
             raise ConfigError('can not add disabled interface {} to {}'.format(intf, bond['intf']))
-- 
cgit v1.2.3


From f9eff4fcdf30223638169e6ed5fa7ca49fcfa223 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Fri, 6 Sep 2019 10:49:02 +0200
Subject: bonding: T1614: reword verify() error messages

---
 src/conf_mode/interface-bonding.py | 46 +++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 18 deletions(-)

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index 7b86a0528..acf088de8 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -257,10 +257,12 @@ def verify(bond):
 
     if bond['primary']:
         if bond['mode'] not in ['active-backup', 'balance-tlb', 'balance-alb']:
-            raise ConfigError('Mode dependency failed, primary not supported in this mode.'.format())
+            raise ConfigError('Mode dependency failed, primary not supported ' \
+                              'in this mode.'.format())
 
         if bond['primary'] not in bond['member']:
-            raise ConfigError('Interface "{}" is not part of the bond'.format(bond['primary']))
+            raise ConfigError('Interface "{}" is not part of the bond' \
+                              .format(bond['primary']))
 
     for vif_s in bond['vif_s']:
         for vif in bond['vif']:
@@ -273,42 +275,50 @@ def verify(bond):
         # a bond member is only allowed to be assigned to any one bond
         for tmp in conf.list_nodes('interfaces bonding'):
             if conf.exists('interfaces bonding ' + tmp + ' member interface ' + intf):
-                raise ConfigError('can not add interface {} that is part of another bond ({}) to {}'.format(
-                    intf, tmp, bond['intf']))
+                raise ConfigError('can not enslave interface {} which already ' \
+                                  'belongs to bond {}'.format(intf, tmp))
 
         # we can not add disabled slave interfaces to our bond
         if conf.exists('interfaces ethernet ' + intf + ' disable'):
-            raise ConfigError('can not add disabled interface {} to {}'.format(intf, bond['intf']))
+            raise ConfigError('can not enslave disabled interface {}' \
+                              .format(intf))
 
         # can not add interfaces with an assigned address to a bond
         if conf.exists('interfaces ethernet ' + intf + ' address'):
-            raise ConfigError('can not add interface {} with an assigned address to {}'.format(intf, bond['intf']))
+            raise ConfigError('can not enslave interface {} which has an address ' \
+                              'assigned'.format(intf))
 
         # bond members are not allowed to be bridge members, too
-        for bridge in conf.list_nodes('interfaces bridge'):
-            if conf.exists('interfaces bridge ' + bridge + ' member interface ' + intf):
-                raise ConfigError('can not add interface {} that is part of bridge {} to {}'.format(intf, bridge, bond['intf']))
+        for tmp in conf.list_nodes('interfaces bridge'):
+            if conf.exists('interfaces bridge ' + tmp + ' member interface ' + intf):
+                raise ConfigError('can not enslave interface {} which belongs to ' \
+                                  'bridge {}'.format(intf, tmp))
 
         # bond members are not allowed to be vrrp members, too
-        for vrrp in conf.list_nodes('high-availability vrrp group'):
-            if conf.exists('high-availability vrrp group ' + vrrp + ' interface ' + intf):
-                raise ConfigError('can not add interface {} which belongs to a VRRP group to {}'.format(intf, bond['intf']))
+        for tmp in conf.list_nodes('high-availability vrrp group'):
+            if conf.exists('high-availability vrrp group ' + tmp + ' interface ' + intf):
+                raise ConfigError('can not enslave interface {} which belongs to ' \
+                                  'VRRP group {}'.format(intf, tmp))
 
         # bond members are not allowed to be underlaying psuedo-ethernet devices
-        for peth in conf.list_nodes('interfaces pseudo-ethernet'):
-            if conf.exists('interfaces pseudo-ethernet ' + peth + ' link ' + intf):
-                raise ConfigError('can not add interface {} used by pseudo-ethernet {} to {}'.format(intf, peth, bond['intf']))
+        for tmp in conf.list_nodes('interfaces pseudo-ethernet'):
+            if conf.exists('interfaces pseudo-ethernet ' + tmp + ' link ' + intf):
+                raise ConfigError('can not enslave interface {} which belongs to ' \
+                                  'pseudo-ethernet {}'.format(intf, tmp))
 
     if bond['primary']:
         if bond['primary'] not in bond['member']:
-            raise ConfigError('primary interface must be a member interface of {}'.format(bond['intf']))
+            raise ConfigError('primary interface must be a member interface of {}' \
+                              .format(bond['intf']))
 
         if bond['mode'] not in ['active-backup', 'balance-tlb', 'balance-alb']:
-            raise ConfigError('primary interface only works for mode active-backup, transmit-load-balance or adaptive-load-balance')
+            raise ConfigError('primary interface only works for mode active-backup, ' \
+                              'transmit-load-balance or adaptive-load-balance')
 
     if bond['arp_mon_intvl'] > 0:
         if bond['mode'] in ['802.3ad', 'balance-tlb', 'balance-alb']:
-            raise ConfigError('ARP link monitoring does not work for mode 802.3ad, transmit-load-balance or adaptive-load-balance')
+            raise ConfigError('ARP link monitoring does not work for mode 802.3ad, ' \
+                              'transmit-load-balance or adaptive-load-balance')
 
     return None
 
-- 
cgit v1.2.3


From 5f87266d9ef3b72908b6f429e37df280f1be8cdf Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Fri, 6 Sep 2019 10:54:50 +0200
Subject: bonding: T1614: members are not allowed to be underlaying vxlan
 devices

---
 src/conf_mode/interface-bonding.py | 7 +++++++
 1 file changed, 7 insertions(+)

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index acf088de8..bce8f98fb 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -306,6 +306,13 @@ def verify(bond):
                 raise ConfigError('can not enslave interface {} which belongs to ' \
                                   'pseudo-ethernet {}'.format(intf, tmp))
 
+        # bond members are not allowed to be underlaying vxlan devices
+        for tmp in conf.list_nodes('interfaces vxlan'):
+            if conf.exists('interfaces vxlan ' + tmp + ' link ' + intf):
+                raise ConfigError('can not enslave interface {} which belongs to ' \
+                                  'vxlan {}'.format(intf, tmp))
+
+
     if bond['primary']:
         if bond['primary'] not in bond['member']:
             raise ConfigError('primary interface must be a member interface of {}' \
-- 
cgit v1.2.3


From a9756cfd49b169e93afe70415ce9155ebf4e5ffa Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Sat, 7 Sep 2019 14:18:23 +0200
Subject: bridge: bonding: minor comment cleanup

---
 src/conf_mode/interface-bonding.py | 2 +-
 src/conf_mode/interface-bridge.py  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index bce8f98fb..cd4c447ac 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -142,7 +142,7 @@ def get_config():
         bond['address'] = conf.return_values('address')
 
     # get interface addresses (currently effective) - to determine which
-    # address is no longer valid and needs to be removed from the bond
+    # address is no longer valid and needs to be removed
     eff_addr = conf.return_effective_values('address')
     bond['address_remove'] = list_diff(eff_addr, bond['address'])
 
diff --git a/src/conf_mode/interface-bridge.py b/src/conf_mode/interface-bridge.py
index 8996fceec..401182a0d 100755
--- a/src/conf_mode/interface-bridge.py
+++ b/src/conf_mode/interface-bridge.py
@@ -71,7 +71,7 @@ def get_config():
         bridge['address'] = conf.return_values('address')
 
     # Determine interface addresses (currently effective) - to determine which
-    # address is no longer valid and needs to be removed from the bridge
+    # address is no longer valid and needs to be removed
     eff_addr = conf.return_effective_values('address')
     bridge['address_remove'] = list_diff(eff_addr, bridge['address'])
 
-- 
cgit v1.2.3


From 6f666f0a62fb98fcab800be813141f44dd1ab8a7 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Sat, 7 Sep 2019 15:30:53 +0200
Subject: bonding: T1614: bugfix in validate - enslave failed

Forgot to exclude our current bond interface in the search for duplicate
interface enslavement.
---
 src/conf_mode/interface-bonding.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

(limited to 'src/conf_mode/interface-bonding.py')

diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index cd4c447ac..dc0363fb7 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -272,11 +272,14 @@ def verify(bond):
 
     conf = Config()
     for intf in bond['member']:
-        # a bond member is only allowed to be assigned to any one bond
-        for tmp in conf.list_nodes('interfaces bonding'):
+        # a bonding member interface is only allowed to be assigned to one bond!
+        all_bonds = conf.list_nodes('interfaces bonding')
+        # We do not need to check our own bond
+        all_bonds.remove(bond['intf'])
+        for tmp in all_bonds:
             if conf.exists('interfaces bonding ' + tmp + ' member interface ' + intf):
                 raise ConfigError('can not enslave interface {} which already ' \
-                                  'belongs to bond {}'.format(intf, tmp))
+                                  'belongs to {}'.format(intf, tmp))
 
         # we can not add disabled slave interfaces to our bond
         if conf.exists('interfaces ethernet ' + intf + ' disable'):
-- 
cgit v1.2.3