Merge pull request #384 from jjakob/bridge-fix-T2241

T2241: fix interfaces falling out of bridge

6 files changed, 238 insertions, 37 deletions

diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py
index e1b704a31..5ab831258 100644
--- a/python/vyos/configdict.py
+++ b/python/vyos/configdict.py
@@ -23,7 +23,8 @@ from copy import deepcopy
 
 from vyos import ConfigError
 from vyos.ifconfig import Interface
-
+from vyos.validate import is_member
+from vyos.util import ifname_from_config
 
 def retrieve_config(path_hash, base_path, config):
     """
@@ -128,6 +129,7 @@ vlan_default = {
     'ipv6_dup_addr_detect': 1,
     'ingress_qos': '',
     'ingress_qos_changed': False,
+    'is_bridge_member': False,
     'mac': '',
     'mtu': 1500,
     'vif_c': [],
@@ -197,6 +199,7 @@ def intf_to_dict(conf, default):
     """
 
     intf = deepcopy(default)
+    intf['intf'] = ifname_from_config(conf)
 
     # retrieve configured interface addresses
     if conf.exists('address'):
@@ -263,15 +266,18 @@ def intf_to_dict(conf, default):
     if conf.exists('ipv6 disable-forwarding'):
         intf['ipv6_forwarding'] = 0
 
-    # Media Access Control (MAC) address
-    if conf.exists('mac'):
-        intf['mac'] = conf.return_value('mac')
+    # check if interface is member of a bridge
+    intf['is_bridge_member'] = is_member(conf, intf['intf'], 'bridge')
 
     # IPv6 Duplicate Address Detection (DAD) tries
     if conf.exists('ipv6 dup-addr-detect-transmits'):
         intf['ipv6_dup_addr_detect'] = int(
             conf.return_value('ipv6 dup-addr-detect-transmits'))
 
+    # Media Access Control (MAC) address
+    if conf.exists('mac'):
+        intf['mac'] = conf.return_value('mac')
+
     # Maximum Transmission Unit (MTU)
     if conf.exists('mtu'):
         intf['mtu'] = int(conf.return_value('mtu'))
@@ -338,14 +344,15 @@ def intf_to_dict(conf, default):
         intf['ipv6_eui64_prefix'] = act_eui
         intf['ipv6_eui64_prefix_remove'] = list_diff(eff_eui, act_eui)
 
-    # Remove the default link-local address if set.
-    if conf.exists('ipv6 address no-default-link-local'):
+    # Remove the default link-local address if set or if member of a bridge
+    if ( conf.exists('ipv6 address no-default-link-local')
+            or intf.get('is_bridge_member') ):
         intf['ipv6_eui64_prefix_remove'].append('fe80::/64')
     else:
         # add the link-local by default to make IPv6 work
         intf['ipv6_eui64_prefix'].append('fe80::/64')
 
-    # Find out if MAC has changed
+    # If MAC has changed, remove and re-add all IPv6 EUI64 addresses
     try:
         interface = Interface(intf['intf'], create=False)
         if intf['mac'] and intf['mac'] != interface.get_mac():
@@ -416,13 +423,13 @@ def add_to_dict(conf, disabled, ifdict, section, key):
 
 def vlan_to_dict(conf, default=vlan_default):
     vlan, disabled = intf_to_dict(conf, default)
-    # get the '100' in 'interfaces bonding bond0 vif-s 100
-    vlan['id'] = conf.get_level()[-1]
 
-    current_level = conf.get_level()
+    level = conf.get_level()
+    # get the '100' in 'interfaces bonding bond0 vif-s 100'
+    vlan['id'] = level[-1]
 
     # if this is a not within vif-s node, we are done
-    if current_level[-2] != 'vif-s':
+    if level[-2] != 'vif-s':
         return vlan
 
     # ethertype is mandatory on vif-s nodes and only exists here!
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 5b3da228e..7b42e3399 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -42,6 +42,7 @@ from vyos.ifconfig.control import Control
 from vyos.ifconfig.dhcp import DHCP
 from vyos.ifconfig.vrrp import VRRP
 from vyos.ifconfig.operational import Operational
+from vyos.ifconfig import Section
 
 
 class Interface(Control):
@@ -718,3 +719,22 @@ class Interface(Control):
 
         # flush all addresses
         self._cmd(f'ip addr flush dev "{self.ifname}"')
+
+    def add_to_bridge(self, br):
+        """
+        Adds the interface to the bridge with the passed port config.
+
+        Returns False if bridge doesn't exist.
+        """
+
+        # check if the bridge exists (on boot it doesn't)
+        if br not in Section.interfaces('bridge'):
+            return False
+
+        self.flush_addrs()
+        # add interface to bridge - use Section.klass to get BridgeIf class
+        Section.klass(br)(br, create=False).add_port(self.ifname)
+
+        # TODO: port config (STP)
+
+        return True
diff --git a/python/vyos/ifconfig/section.py b/python/vyos/ifconfig/section.py
index 092236fef..926c22e8a 100644
--- a/python/vyos/ifconfig/section.py
+++ b/python/vyos/ifconfig/section.py
@@ -54,7 +54,7 @@ class Section:
         name = name.rstrip('0123456789')
         name = name.rstrip('.')
         if vlan:
-            name = name.rstrip('0123456789')
+            name = name.rstrip('0123456789.')
         return name
 
     @classmethod
@@ -137,3 +137,22 @@ class Section:
         eth, lo, vxlan, dum, ...
         """
         return list(cls._prefixes.keys())
+
+    @classmethod
+    def get_config_path(cls, name):
+        """
+        get config path to interface with .vif or .vif-s.vif-c
+        example: eth0.1.2 -> 'ethernet eth0 vif-s 1 vif-c 2'
+        Returns False if interface name is invalid (not found in sections)
+        """
+        sect = cls.section(name)
+        if sect:
+            splinterface = name.split('.')
+            intfpath = f'{sect} {splinterface[0]}'
+            if len(splinterface) == 2:
+                intfpath += f' vif {splinterface[1]}'
+            elif len(splinterface) == 3:
+                intfpath += f' vif-s {splinterface[1]} vif-c {splinterface[2]}'
+            return intfpath
+        else:
+            return False
diff --git a/python/vyos/ifconfig_vlan.py b/python/vyos/ifconfig_vlan.py
index ee009f7f9..eb7a369ec 100644
--- a/python/vyos/ifconfig_vlan.py
+++ b/python/vyos/ifconfig_vlan.py
@@ -63,8 +63,10 @@ def apply_vlan_config(vlan, config):
     # Maximum Transmission Unit (MTU)
     vlan.set_mtu(config['mtu'])
 
-    # assign/remove VRF
-    vlan.set_vrf(config['vrf'])
+    # assign/remove VRF (ONLY when not a member of a bridge,
+    # otherwise 'nomaster' removes it from it)
+    if not config['is_bridge_member']:
+        vlan.set_vrf(config['vrf'])
 
     # Delete old IPv6 EUI64 addresses before changing MAC
     for addr in config['ipv6_eui64_prefix_remove']:
@@ -92,6 +94,10 @@ def apply_vlan_config(vlan, config):
     for addr in config['address']:
         vlan.add_addr(addr)
 
+    # re-add ourselves to any bridge we might have fallen out of
+    if config['is_bridge_member']:
+        vlan.add_to_bridge(config['is_bridge_member'])
+
 def verify_vlan_config(config):
     """
     Generic function to verify VLAN config consistency. Instead of re-
@@ -103,9 +109,22 @@ def verify_vlan_config(config):
         if vif['dhcpv6_prm_only'] and vif['dhcpv6_temporary']:
             raise ConfigError('DHCPv6 temporary and parameters-only options are mutually exclusive!')
 
-        vrf_name = vif['vrf']
-        if vrf_name and vrf_name not in interfaces():
-            raise ConfigError(f'VRF "{vrf_name}" does not exist')
+        if ( vif['is_bridge_member']
+                and ( vif['address']
+                    or vif['ipv6_eui64_prefix']
+                    or vif['ipv6_autoconf'] ) ):
+            raise ConfigError((
+                    f'Cannot assign address to vif interface {vif["intf"]} '
+                    f'which is a member of bridge {vif["is_bridge_member"]}'))
+
+        if vif['vrf']:
+            if vif['vrf'] not in interfaces():
+                raise ConfigError(f'VRF "{vif["vrf"]}" does not exist')
+
+            if vif['is_bridge_member']:
+                raise ConfigError((
+                    f'vif {vif["intf"]} cannot be member of VRF {vif["vrf"]} '
+                    f'and bridge {vif["is_bridge_member"]} at the same time!'))
 
     # e.g. wireless interface has no vif_s support
     # thus we bail out eraly.
@@ -121,17 +140,42 @@ def verify_vlan_config(config):
         if vif_s['dhcpv6_prm_only'] and vif_s['dhcpv6_temporary']:
             raise ConfigError('DHCPv6 temporary and parameters-only options are mutually exclusive!')
 
-            vrf_name = vif_s['vrf']
-            if vrf_name and vrf_name not in interfaces():
-                raise ConfigError(f'VRF "{vrf_name}" does not exist')
+        if ( vif_s['is_bridge_member']
+                and ( vif_s['address']
+                    or vif_s['ipv6_eui64_prefix']
+                    or vif_s['ipv6_autoconf'] ) ):
+            raise ConfigError((
+                    f'Cannot assign address to vif-s interface {vif_s["intf"]} '
+                    f'which is a member of bridge {vif_s["is_bridge_member"]}'))
+
+        if vif_s['vrf']:
+            if vif_s['vrf'] not in interfaces():
+                raise ConfigError(f'VRF "{vif_s["vrf"]}" does not exist')
+
+            if vif_s['is_bridge_member']:
+                raise ConfigError((
+                    f'vif-s {vif_s["intf"]} cannot be member of VRF {vif_s["vrf"]} '
+                    f'and bridge {vif_s["is_bridge_member"]} at the same time!'))
 
         for vif_c in vif_s['vif_c']:
             # DHCPv6 parameters-only and temporary address are mutually exclusive
             if vif_c['dhcpv6_prm_only'] and vif_c['dhcpv6_temporary']:
                 raise ConfigError('DHCPv6 temporary and parameters-only options are mutually exclusive!')
 
-            vrf_name = vif_c['vrf']
-            if vrf_name and vrf_name not in interfaces():
-                raise ConfigError(f'VRF "{vrf_name}" does not exist')
-
+            if ( vif_c['is_bridge_member']
+                    and ( vif_c['address']
+                        or vif_c['ipv6_eui64_prefix']
+                        or vif_c['ipv6_autoconf'] ) ):
+                raise ConfigError((
+                    f'Cannot assign address to vif-c interface {vif_c["intf"]} '
+                    f'which is a member of bridge {vif_c["is_bridge_member"]}'))
+
+            if vif_c['vrf']:
+                if vif_c['vrf'] not in interfaces():
+                    raise ConfigError(f'VRF "{vif_c["vrf"]}" does not exist')
+
+                if vif_c['is_bridge_member']:
+                    raise ConfigError((
+                    f'vif-c {vif_c["intf"]} cannot be member of VRF {vif_c["vrf"]} '
+                    f'and bridge {vif_c["is_bridge_member"]} at the same time!'))
 
diff --git a/python/vyos/util.py b/python/vyos/util.py
index 92b6f7992..e598e0ff3 100644
--- a/python/vyos/util.py
+++ b/python/vyos/util.py
@@ -498,3 +498,74 @@ def get_half_cpus():
     if cpu > 1:
         cpu /= 2
     return int(cpu)
+
+def ifname_from_config(conf):
+    """
+    Gets interface name with VLANs from current config level.
+    Level must be at the interface whose name we want.
+
+    Example:
+    >>> from vyos.util import ifname_from_config
+    >>> from vyos.config import Config
+    >>> conf = Config()
+    >>> conf.set_level('interfaces ethernet eth0 vif-s 1 vif-c 2')
+    >>> ifname_from_config(conf)
+    'eth0.1.2'
+    """
+    level = conf.get_level()
+
+    # vlans
+    if level[-2] == 'vif' or level[-2] == 'vif-s':
+        return level[-3] + '.' + level[-1]
+    if level[-2] == 'vif-c':
+        return level[-5] + '.' + level[-3] + '.' + level[-1]
+
+    # no vlans
+    return level[-1]
+
+def get_bridge_member_config(conf, br, intf):
+    """
+    Gets bridge port (member) configuration
+
+    Arguments:
+    conf: Config
+    br: bridge name
+    intf: interface name
+
+    Returns:
+    dict with the configuration
+    False if bridge or bridge port doesn't exist
+    """
+    old_level = conf.get_level()
+    conf.set_level([])
+
+    bridge = f'interfaces bridge {br}'
+    member = f'{bridge} member interface {intf}'
+    if not ( conf.exists(bridge) and conf.exists(member) ):
+        return False
+
+    # default bridge port configuration
+    # cost and priority initialized with linux defaults
+    # by reading /sys/devices/virtual/net/br0/brif/eth2/{path_cost,priority}
+    # after adding interface to bridge after reboot
+    memberconf = {
+        'cost': 100,
+        'priority': 32,
+        'arp_cache_tmo': 30,
+        'disable_link_detect': 1,
+    }
+
+    if conf.exists(f'{member} cost'):
+        memberconf['cost'] = int(conf.return_value(f'{member} cost'))
+
+    if conf.exists(f'{member} priority'):
+        memberconf['priority'] = int(conf.return_value(f'{member} priority'))
+
+    if conf.exists(f'{bridge} ip arp-cache-timeout'):
+        memberconf['arp_cache_tmo'] = int(conf.return_value(f'{bridge} ip arp-cache-timeout'))
+
+    if conf.exists(f'{bridge} disable-link-detect'):
+        memberconf['disable_link_detect'] = 2
+
+    conf.set_level(old_level)
+    return memberconf
diff --git a/python/vyos/validate.py b/python/vyos/validate.py
index 446f6e4ca..e083604c5 100644
--- a/python/vyos/validate.py
+++ b/python/vyos/validate.py
@@ -241,26 +241,66 @@ def assert_mac(m):
     if octets[:5] == (0, 0, 94, 0, 1):
         raise ValueError(f'{m} is a VRRP MAC address')
 
-def is_bridge_member(conf, interface):
+def is_member(conf, interface, intftype=None):
     """
-    Checks if passed interfaces is part of a bridge device or not.
-
-    Returns a tuple:
-    None -> Interface not a bridge member
-    Bridge -> Interface is a member of this bridge
+    Checks if passed interface is member of other interface of specified type.
+    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
     """
     ret_val = None
-    old_level = conf.get_level()
+
+    if intftype not in ['bonding', 'bridge', None]:
+        raise ValueError((
+            f'unknown interface type "{intftype}" or it cannot '
+            f'have member interfaces'))
+
+    intftype = ['bonding', 'bridge'] if intftype == None else [intftype]
 
     # set config level to root
+    old_level = conf.get_level()
     conf.set_level([])
-    base = ['interfaces', 'bridge']
-    for bridge in conf.list_nodes(base):
-        members = conf.list_nodes(base + [bridge, 'member', 'interface'])
-        if interface in members:
-            ret_val = bridge
-            break
+
+    for it in intftype:
+        base = 'interfaces ' + it
+        for intf in conf.list_nodes(base):
+            memberintf = f'{base} {intf} member interface'
+            if conf.is_tag(memberintf):
+                if interface in conf.list_nodes(memberintf):
+                    ret_val = intf
+                    break
+            elif conf.is_leaf(memberintf):
+                if ( conf.exists(memberintf) and
+                        interface in conf.return_values(memberintf) ):
+                    ret_val = intf
+                    break
 
     old_level = conf.set_level(old_level)
     return ret_val
 
+def has_address_configured(conf, intf):
+    """
+    Checks if interface has an address configured.
+    Checks the following config nodes:
+    'address', 'ipv6 address eui64', 'ipv6 address autoconf'
+
+    Returns True if interface has address configured, False if it doesn't.
+    """
+    from vyos.ifconfig import Section
+    ret = False
+
+    old_level = conf.get_level()
+    conf.set_level([])
+
+    intfpath = 'interfaces ' + Section.get_config_path(intf)
+    if ( conf.exists(f'{intfpath} address') or
+            conf.exists(f'{intfpath} ipv6 address autoconf') or
+            conf.exists(f'{intfpath} ipv6 address eui64') ):
+        ret = True
+
+    conf.set_level(old_level)
+    return ret