ifupdownmain: redo shared dependent checks

Ticket: CM-10027
Reviewed By: julien, nikhil
Testing Done: Tested with an interfaces file with shared dependents

In the process of fixing this saw a few more issues with link kind
handing. Its better to separate kind from interface private flags
like bond slave and bridge port. this patch cleans up all that handling.

Example errors:
error: misconfig..? swp5.2 vrfslave  is enslaved to multiple interfaces
['vrf1012', 'br2']
error: misconfig..? swp5.2 bridgeport  is enslaved to multiple
interfaces ['vrf1012', 'br2']

diff --git a/addons/address.py b/addons/address.py
index f98d70ec45aefceede0dcf0570ca09b477cc37cb..293d13d3532fc3c4c6fbdc50a08c2b14225b1eaa 100644 (file)
--- a/addons/address.py
+++ b/addons/address.py
@@ -128,8 +128,10 @@ class address(moduleBase):
             if not addrs:
                 continue
 
-            if ((ifaceobj.role & ifaceRole.SLAVE) or
-                (ifaceobj.link_kind & ifaceLinkKind.BRIDGE_VLAN_AWARE)):
+            if (((ifaceobj.role & ifaceRole.SLAVE) and
+                not (ifaceobj.link_privflags & ifaceLinkPrivFlags.VRF_SLAVE)) or
+                ((ifaceobj.link_kind & ifaceLinkKind.BRIDGE) and
+                 (ifaceobj.link_privflags & ifaceLinkPrivFlags.BRIDGE_VLAN_AWARE))):
                 # we must not configure an IP address if the interface is
                 # enslaved or is a VLAN AWARE BRIDGE
                 self.logger.info('%s: ignoring ip address. Interface is '

diff --git a/addons/bridge.py b/addons/bridge.py
index 7209e5b9c57b7040d4b4536c1e659c5d07097720..5ab2e3150b38f1ac967800042565010fe476d018 100644 (file)
--- a/addons/bridge.py
+++ b/addons/bridge.py
@@ -244,7 +244,8 @@ class bridge(moduleBase):
         ifaceobj.link_kind |= ifaceLinkKind.BRIDGE
         # for special vlan aware bridges, we need to add another bit
         if ifaceobj.get_attr_value_first('bridge-vlan-aware') == 'yes':
-            ifaceobj.link_kind |= ifaceLinkKind.BRIDGE_VLAN_AWARE
+            ifaceobj.link_kind |= ifaceLinkKind.BRIDGE
+            ifaceobj.link_privflags |= ifaceLinkPrivFlags.BRIDGE_VLAN_AWARE
         ifaceobj.role |= ifaceRole.MASTER
         ifaceobj.dependency_type = ifaceDependencyType.MASTER_SLAVE
         return self.parse_port_list(ifaceobj.name,
@@ -987,7 +988,7 @@ class bridge(moduleBase):
         add_port = False
         bridgename = self.ipcmd.bridge_port_get_bridge_name(ifaceobj.name)
         if (not bridgename and
-                (ifaceobj.link_kind & ifaceLinkKind.BRIDGE_PORT)):
+                (ifaceobj.link_privflags & ifaceLinkPrivFlags.BRIDGE_PORT)):
             # get bridgename and add port to bridge
             bridgename = self._get_bridgename(ifaceobj)
             add_port = True

diff --git a/addons/vrf.py b/addons/vrf.py
index 8870b64705553544b5a428dc4a8488922bdb6ad8..1a7d6faa2e8d6ff9a5145d4a81c7b2c52060132f 100644 (file)
--- a/addons/vrf.py
+++ b/addons/vrf.py
@@ -169,7 +169,7 @@ class vrf(moduleBase):
         if not vrf_iface_name:
             return None
         ifaceobj.link_type = ifaceLinkType.LINK_SLAVE
-        ifaceobj.link_kind |= ifaceLinkKind.VRF_SLAVE
+        ifaceobj.link_privflags |= ifaceLinkPrivFlags.VRF_SLAVE
 
         return [vrf_iface_name]
 

diff --git a/ifupdown/iface.py b/ifupdown/iface.py
index 1a9ffc711aa3be979ddab4b0ad6efae7638565b2..cccd39fb2f09113348ef2d7d370641f054fb4adb 100644 (file)
--- a/ifupdown/iface.py
+++ b/ifupdown/iface.py
@@ -26,10 +26,9 @@ class ifaceStatusUserStrs():
     UNKNOWN = "unknown"
 
 class ifaceType():
-    UNKNOWN = 0x0
-    IFACE = 0x1
-    BRIDGE_VLAN = 0x2
-
+    UNKNOWN =     0x00
+    IFACE =       0x01
+    BRIDGE_VLAN = 0x10
 
 class ifaceRole():
     """ ifaceRole is used to classify the ifaceobj.role of
@@ -37,9 +36,9 @@ class ifaceRole():
         with bond-slaves or bridge-ports.  A bond in a bridge
         is both a master and slave (0x3)
     """
-    UNKNOWN = 0x0
-    SLAVE = 0x1
-    MASTER = 0x2
+    UNKNOWN = 0x00
+    SLAVE =   0x01
+    MASTER =  0x10
 
 class ifaceLinkKind():
     """ ifaceLlinkKind is used to identify interfaces
@@ -47,16 +46,47 @@ class ifaceLinkKind():
         bond have an ifaceobj.role attribute of SLAVE and the bridge or
         bond itself has ifaceobj.role of MASTER.
     """
-    UNKNOWN = 0x0
-    BRIDGE = 0x1
-    BOND = 0x2
-    VLAN = 0x4
-    VXLAN = 0x8
-    BRIDGE_VLAN_AWARE = 0x10
-    BRIDGE_PORT = 0x20
-    BOND_SLAVE = 0x40
-    VRF = 0x80
-    VRF_SLAVE = 0x100
+    UNKNOWN = 0x000000
+    BRIDGE =  0x000001
+    BOND =    0x000010
+    VLAN =    0x000100
+    VXLAN =   0x001000
+    VRF =     0x010000
+
+class ifaceLinkPrivFlags():
+    """ This corresponds to kernel netdev->priv_flags
+        and can be BRIDGE_PORT, BOND_SLAVE etc """
+    UNKNOWN =           0x0000
+    BRIDGE_PORT =       0x0001
+    BOND_SLAVE =        0x0010
+    VRF_SLAVE =         0x0100
+    BRIDGE_VLAN_AWARE = 0x1000
+
+    @classmethod
+    def get_str(cls, flag):
+        if flag == cls.UNKNOWN:
+            return 'unknown'
+        elif flag == cls.BRIDGE_PORT:
+            return 'bridge port'
+        elif flag == cls.BOND_SLAVE:
+            return 'bond slave'
+        elif flag == cls.VRF_SLAVE:
+            return 'vrf slave'
+        elif flag == cls.BRIDGE_VLAN_AWARE:
+            return 'vlan aware bridge'
+
+    @classmethod
+    def get_all_str(cls, flags):
+        str = ''
+        if (flags & cls.BRIDGE_PORT):
+            str += 'bridgeport '
+        if (flags == cls.BOND_SLAVE):
+            str += 'bondslave '
+        elif flags == cls.VRF_SLAVE:
+            str += 'vrfslave '
+        elif flags == cls.BRIDGE_VLAN_AWARE:
+            str += 'vlanawarebridge '
+        return str
 
 class ifaceLinkType():
     LINK_UNKNOWN = 0x0
@@ -344,6 +374,7 @@ class iface():
         self.realname = None
         self.link_type = ifaceLinkType.LINK_UNKNOWN
         self.link_kind = ifaceLinkKind.UNKNOWN
+        self.link_privflags = ifaceLinkPrivFlags.UNKNOWN
 
         # The below attribute is used to disambiguate between various
         # types of dependencies
@@ -561,6 +592,7 @@ class iface():
         del odict['env']
         del odict['link_type']
         del odict['link_kind']
+        del odict['link_privflags']
         del odict['role']
         del odict['dependency_type']
         del odict['blacklisted']
@@ -584,6 +616,7 @@ class iface():
         self.flags |= self._PICKLED
         self.link_type = ifaceLinkType.LINK_NA
         self.link_kind = ifaceLinkKind.UNKNOWN
+        self.link_privflags = ifaceLinkPrivFlags.UNKNOWN
         self.dependency_type = ifaceDependencyType.UNKNOWN
         self.blacklisted = False
 

diff --git a/ifupdown/ifupdownmain.py b/ifupdown/ifupdownmain.py
index 75ca345d0f8f32a85cc95ee9b0a86cee299d3ede..680e995e1734f8f6ee44943f3bca938883751e31 100644 (file)
--- a/ifupdown/ifupdownmain.py
+++ b/ifupdown/ifupdownmain.py
@@ -140,7 +140,7 @@ class ifupdownMain(ifupdownBase):
     def run_down(self, ifaceobj):
         # Skip link sets on ifaceobjs of type 'vlan' (used for l2 attrs)
         # there is no real interface behind it
-        if ifaceobj.kind & ifaceLinkKind.VRF:
+        if ifaceobj.link_kind & ifaceLinkKind.VRF:
             return
         if ifaceobj.type == ifaceType.BRIDGE_VLAN:
             return
@@ -417,7 +417,7 @@ class ifupdownMain(ifupdownBase):
         return self.is_ifaceobj_noconfig(ifaceobj)
 
     def check_shared_dependents(self, ifaceobj, dlist):
-        """ Check if dlist intersects with any other
+        """ ABSOLETE: Check if dlist intersects with any other
             interface with slave dependents.
             example: bond and bridges.
             This function logs such errors """
@@ -439,22 +439,38 @@ class ifupdownMain(ifupdownBase):
                             %(ifaceobj.name, ifacename) +
                             'seem to share dependents/ports %s' %str(list(common)))
 
+    def _set_iface_role(self, ifaceobj, role):
+        if (self.flags.CHECK_SHARED_DEPENDENTS and
+            (ifaceobj.role & ifaceRole.SLAVE) and role == ifaceRole.SLAVE):
+               self.logger.error("misconfig..? %s %s is enslaved to multiple interfaces %s"
+                                  %(ifaceobj.name,
+                                    ifaceLinkPrivFlags.get_all_str(ifaceobj.link_privflags), str(ifaceobj.upperifaces)))
+                ifaceobj.set_status(ifaceStatus.ERROR)
+                return
+        ifaceobj.role = role
+
     def _set_iface_role_n_kind(self, ifaceobj, upperifaceobj):
+
         if (upperifaceobj.link_kind & ifaceLinkKind.BOND):
-            ifaceobj.role |= ifaceRole.SLAVE
-            ifaceobj.link_kind |= ifaceLinkKind.BOND_SLAVE
+            self._set_iface_role(ifaceobj, ifaceRole.SLAVE)
+            ifaceobj.link_privflags |= ifaceLinkPrivFlags.BOND_SLAVE
+
         if (upperifaceobj.link_kind & ifaceLinkKind.BRIDGE):
-            ifaceobj.role |= ifaceRole.SLAVE
-            ifaceobj.link_kind |= ifaceLinkKind.BRIDGE_PORT
+            self._set_iface_role(ifaceobj, ifaceRole.SLAVE)
+            ifaceobj.link_privflags |= ifaceLinkPrivFlags.BRIDGE_PORT
+
+        # vrf masters get processed after slaves, which means
+        # check both link_kind vrf and vrf slave
+        if ((upperifaceobj.link_kind & ifaceLinkKind.VRF) or
+            (ifaceobj.link_privflags & ifaceLinkPrivFlags.VRF_SLAVE)):
+            self._set_iface_role(ifaceobj, ifaceRole.SLAVE)
+            ifaceobj.link_privflags |= ifaceLinkPrivFlags.VRF_SLAVE
         if self._link_master_slave:
             if upperifaceobj.link_type == ifaceLinkType.LINK_MASTER:
                 ifaceobj.link_type = ifaceLinkType.LINK_SLAVE
         else:
             upperifaceobj.link_type = ifaceLinkType.LINK_NA
             ifaceobj.link_type = ifaceLinkType.LINK_NA
-       if (ifaceobj.link_kind == ifaceLinkKind.BOND_SLAVE and
-                       len(ifaceobj.upperifaces) > 1):
-               self.logger.warn("misconfig..? bond slave \'%s\' is enslaved to multiple interfaces %s" %(ifaceobj.name, str(ifaceobj.upperifaces)))
 
     def dump_iface_dependency_info(self):
         """ debug funtion to print raw dependency 
@@ -485,10 +501,6 @@ class ifupdownMain(ifupdownBase):
         """
         del_list = []
 
-        if (upperifaceobj.dependency_type == ifaceDependencyType.MASTER_SLAVE
-                and self.flags.CHECK_SHARED_DEPENDENTS):
-            self.check_shared_dependents(upperifaceobj, dlist)
-
         for d in dlist:
             dilist = self.get_ifaceobjs(d)
             if not dilist:
@@ -630,6 +642,7 @@ class ifupdownMain(ifupdownBase):
             ifaceobj = self.get_ifaceobj_first(i)
             if not ifaceobj:
                 continue
+
             if ifaceobj.blacklisted and not ifaceobj.upperifaces:
                 # if blacklisted and was not picked up as a
                 # dependent of a upper interface, delete the
@@ -649,6 +662,7 @@ class ifupdownMain(ifupdownBase):
                             pass
                 self.logger.debug("populate_dependency_info: deleting blacklisted interface %s" %i)
                 del self.dependency_graph[i]
+                continue
 
     def _check_config_no_repeats(self, ifaceobj):
         """ check if object has an attribute that is