Merge pull request #1406 from c-po/equuleus-interface-fixes

equuleus: Bond and Bridge interface fixes + new smoketests

5 files changed, 117 insertions, 47 deletions

diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py
index be10cbdfc..70e5cd889 100644
--- a/python/vyos/configdict.py
+++ b/python/vyos/configdict.py
@@ -203,11 +203,12 @@ def is_member(conf, interface, intftype=None):
     intftype is optional, if not passed it will search all known types
     (currently bridge and bonding)
 
-    Returns:
-    None -> Interface is not a member
-    interface name -> Interface is a member of this interface
-    False -> interface type cannot have members
+    Returns: dict
+    empty -> Interface is not a member
+    key -> Interface is a member of this interface
     """
+    from vyos.ifconfig import Section
+
     ret_val = {}
     intftypes = ['bonding', 'bridge']
 
@@ -227,9 +228,24 @@ def is_member(conf, interface, intftype=None):
         for intf in conf.list_nodes(base):
             member = base + [intf, 'member', 'interface', interface]
             if conf.exists(member):
-                tmp = conf.get_config_dict(member, key_mangling=('-', '_'),
-                                           get_first_key=True, no_tag_node_value_mangle=True)
-                ret_val.update({intf : tmp})
+                member_type = Section.section(interface)
+                # Check if it's a VLAN (QinQ) interface
+                interface = interface.split('.')
+                if len(interface) == 3:
+                    if conf.exists(['interfaces', member_type, interface[0], 'vif-s', interface[1], 'vif-c', interface[2]]):
+                        tmp = conf.get_config_dict(['interfaces', member_type, interface[0]],
+                                                   key_mangling=('-', '_'), get_first_key=True)
+                        ret_val.update({intf : tmp})
+                elif len(interface) == 2:
+                    if conf.exists(['interfaces', member_type, interface[0], 'vif', interface[1]]):
+                        tmp = conf.get_config_dict(['interfaces', member_type, interface[0]],
+                                                   key_mangling=('-', '_'), get_first_key=True)
+                        ret_val.update({intf : tmp})
+                else:
+                    if conf.exists(['interfaces', member_type, interface[0]]):
+                        tmp = conf.get_config_dict(['interfaces', member_type, interface[0]],
+                                                   key_mangling=('-', '_'), get_first_key=True)
+                        ret_val.update({intf : tmp})
 
     old_level = conf.set_level(old_level)
     return ret_val
diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py
index 6566e8863..44342f289 100644
--- a/python/vyos/configverify.py
+++ b/python/vyos/configverify.py
@@ -84,6 +84,18 @@ def verify_mtu_ipv6(config):
             tmp = dict_search('ipv6.address.eui64', config)
             if tmp != None: raise ConfigError(error_msg)
 
+def verify_bond_bridge_member(config):
+    """
+    Checks if interface has a VRF configured and is also part of a bond or
+    bridge, which is not allowed!
+    """
+    if 'vrf' in config:
+        ifname = config['ifname']
+        if 'is_bond_member' in config:
+            raise ConfigError(f'Can not add interface "{ifname}" to bond, it has a VRF assigned!')
+        if 'is_bridge_member' in config:
+            raise ConfigError(f'Can not add interface "{ifname}" to bridge, it has a VRF assigned!')
+
 def verify_tunnel(config):
     """
     This helper is used to verify the common part of the tunnel
diff --git a/python/vyos/ifconfig/bond.py b/python/vyos/ifconfig/bond.py
index 27d0182e9..47e0598ab 100644
--- a/python/vyos/ifconfig/bond.py
+++ b/python/vyos/ifconfig/bond.py
@@ -176,6 +176,21 @@ class BondIf(Interface):
         """
         self.set_interface('bond_lacp_rate', slow_fast)
 
+    def set_miimon_interval(self, interval):
+        """
+        Specifies the MII link monitoring frequency in milliseconds. This
+        determines how often the link state of each slave is inspected for link
+        failures. A value of zero disables MII link monitoring. A value of 100
+        is a good starting point.
+
+        The default value is 0.
+
+        Example:
+        >>> from vyos.ifconfig import BondIf
+        >>> BondIf('bond0').set_miimon_interval('100')
+        """
+        return self.set_interface('bond_miimon', interval)
+
     def set_arp_interval(self, interval):
         """
         Specifies the ARP link monitoring frequency in milliseconds.
@@ -199,16 +214,7 @@ class BondIf(Interface):
         >>> from vyos.ifconfig import BondIf
         >>> BondIf('bond0').set_arp_interval('100')
         """
-        if int(interval) == 0:
-            """
-            Specifies the MII link monitoring frequency in milliseconds.
-            This determines how often the link state of each slave is
-            inspected for link failures. A value of zero disables MII
-            link monitoring. A value of 100 is a good starting point.
-            """
-            return self.set_interface('bond_miimon', interval)
-        else:
-            return self.set_interface('bond_arp_interval', interval)
+        return self.set_interface('bond_arp_interval', interval)
 
     def get_arp_ip_target(self):
         """
@@ -365,26 +371,9 @@ class BondIf(Interface):
         if 'shutdown_required' in config:
             self.set_admin_state('down')
 
-        # 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 addresses prior to adding new ones, this will remove
-        # addresses manually added 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.
-        arp_tgt_addr = list(map(str, self.get_arp_ip_target().split()))
-        for addr in arp_tgt_addr:
-            self.set_arp_ip_target('-' + addr)
-
-        # Add configured ARP target addresses
-        value = dict_search('arp_monitor.target', config)
-        if isinstance(value, str):
-            value = [value]
-        if value:
-            for addr in value:
-                self.set_arp_ip_target('+' + addr)
+        # Specifies the MII link monitoring frequency in milliseconds
+        value = config.get('mii_mon_interval')
+        self.set_miimon_interval(value)
 
         # Bonding transmit hash policy
         value = config.get('hash_policy')
@@ -414,6 +403,32 @@ class BondIf(Interface):
             if mode == '802.3ad':
                 self.set_lacp_rate(config.get('lacp_rate'))
 
+            if mode not in ['802.3ad', 'balance-tlb', 'balance-alb']:
+                tmp = dict_search('arp_monitor.interval', config)
+                value = tmp if (tmp != None) else '0'
+                self.set_arp_interval(value)
+
+                # 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 addresses prior to adding new ones, this will remove
+                # addresses manually added 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.
+                arp_tgt_addr = list(map(str, self.get_arp_ip_target().split()))
+                for addr in arp_tgt_addr:
+                    self.set_arp_ip_target('-' + addr)
+
+                # Add configured ARP target addresses
+                value = dict_search('arp_monitor.target', config)
+                if isinstance(value, str):
+                    value = [value]
+                if value:
+                    for addr in value:
+                        self.set_arp_ip_target('+' + addr)
+
             # Add (enslave) interfaces to bond
             value = dict_search('member.interface', config)
             for interface in (value or []):
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 9eed3acb9..9b08d83de 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -1304,14 +1304,22 @@ class Interface(Control):
         if dhcpv6pd:
             self.set_dhcpv6(True)
 
-        # There are some items in the configuration which can only be applied
-        # if this instance is not bound to a bridge. This should be checked
-        # by the caller but better save then sorry!
-        if not any(k in ['is_bond_member', 'is_bridge_member'] for k in config):
-            # Bind interface to given VRF or unbind it if vrf node is not set.
-            # unbinding will call 'ip link set dev eth0 nomaster' which will
-            # also drop the interface out of a bridge or bond - thus this is
-            # checked before
+        # XXX: Bind interface to given VRF or unbind it if vrf is not set. Unbinding
+        # will call 'ip link set dev eth0 nomaster' which will also drop the
+        # interface out of any bridge or bond - thus this is checked before.
+        if 'is_bond_member' in config:
+            bond_if = next(iter(config['is_bond_member']))
+            tmp = get_interface_config(config['ifname'])
+            if 'master' in tmp and tmp['master'] != bond_if:
+                self.set_vrf('')
+
+        elif 'is_bridge_member' in config:
+            bridge_if = next(iter(config['is_bridge_member']))
+            tmp = get_interface_config(config['ifname'])
+            if 'master' in tmp and tmp['master'] != bridge_if:
+                self.set_vrf('')
+
+        else:
             self.set_vrf(config.get('vrf', ''))
 
         # Add this section after vrf T4331
@@ -1412,8 +1420,8 @@ class Interface(Control):
 
         # re-add ourselves to any bridge we might have fallen out of
         if 'is_bridge_member' in config:
-            bridge_dict = config.get('is_bridge_member')
-            self.add_to_bridge(bridge_dict)
+            tmp = config.get('is_bridge_member')
+            self.add_to_bridge(tmp)
 
         # configure port mirror
         self.set_mirror()
diff --git a/python/vyos/validate.py b/python/vyos/validate.py
index 23e88b5ac..9aa23d3dc 100644
--- a/python/vyos/validate.py
+++ b/python/vyos/validate.py
@@ -260,3 +260,22 @@ def has_address_configured(conf, intf):
 
     conf.set_level(old_level)
     return ret
+
+def has_vrf_configured(conf, intf):
+    """
+    Checks if interface has a VRF configured.
+
+    Returns True if interface has VRF configured, False if it doesn't.
+    """
+    from vyos.ifconfig import Section
+    ret = False
+
+    old_level = conf.get_level()
+    conf.set_level([])
+
+    tmp = ['interfaces', Section.get_config_path(intf), 'vrf']
+    if conf.exists(tmp):
+        ret = True
+
+    conf.set_level(old_level)
+    return ret