Multiple fixes and cleanup

Ticket: CM-3346
Reviewed By:
Testing Done: Tested ifupdown2 sanity

- moved 'admin up' delays that we introduced recently to be
configurable via two ifupdown2.conf attributes
    # Let link master (bridges, bonds) own the link state of slaves
    link_master_slave=1

    # Delay admin state change till the end
    delay_admin_state_change=0

- reduced some redundant traversal of dependency trees

- fixed a few bugs in query check

addons/bridge.py

@@ -218,6 +218,8 @@ class bridge(moduleBase):
     def get_dependent_ifacenames(self, ifaceobj, ifacenames_all=None):
         if not self._is_bridge(ifaceobj):
             return None
+        if ifaceobj.link_type != ifaceLinkType.LINK_NA:
+           ifaceobj.link_type = ifaceLinkType.LINK_MASTER
         return self.parse_port_list(ifaceobj.get_attr_value_first(
                                     'bridge-ports'), ifacenames_all)
 
@@ -808,8 +810,10 @@ class bridge(moduleBase):
         except Exception, e:
             self.log_error(str(e))
         finally:
-            running_ports = self.brctlcmd.get_bridge_ports(ifaceobj.name)
+            if ifaceobj.link_type != ifaceLinkType.LINK_NA:
-            [rtnetlink_api.rtnl_api.link_set(p, "up") for p in running_ports]
+                running_ports = self.brctlcmd.get_bridge_ports(ifaceobj.name)
+                [rtnetlink_api.rtnl_api.link_set(p, "up")
+                                    for p in running_ports]
             if link_exists and ifaceobj.addr_method == 'manual':
                rtnetlink_api.rtnl_api.link_set(ifaceobj.name, "up")
         if porterr:
@@ -819,12 +823,14 @@ class bridge(moduleBase):
         try:
             if ifaceobj.get_attr_value_first('bridge-ports'):
                 ports = self.brctlcmd.get_bridge_ports(ifaceobj.name)
+                self.brctlcmd.delete_bridge(ifaceobj.name)
                 if ports:
                     for p in ports:
                         proc_file = ('/proc/sys/net/ipv6/conf/%s' %p +
                                      '/disable_ipv6')
                         self.write_file(proc_file, '0')
-                self.brctlcmd.delete_bridge(ifaceobj.name)
+                        if ifaceobj.link_type == ifaceLinkType.LINK_MASTER:
+                           rtnetlink_api.rtnl_api.link_set(p, "down")
         except Exception, e:
             self.log_error(str(e))
 
@@ -1284,11 +1290,11 @@ class bridge(moduleBase):
                                  ifaceobj_getfunc):
         if not self._is_bridge_port(ifaceobj):
             # Mark all bridge attributes as failed
-            ifaceobj.check_n_update_config_with_status_many(
+            ifaceobjcurr.check_n_update_config_with_status_many(ifaceobj,
                     ['bridge-vids', 'bridge-pvid', 'bridge-access',
                      'bridge-pathcosts', 'bridge-portprios',
                      'bridge-portmcrouter',
-                     'bridge-portmcfl'], 0)
+                     'bridge-portmcfl'], 1)
             return
         bridgename = self._get_bridge_name(ifaceobj)
         if not bridgename:

addons/ifenslave.py

@@ -121,7 +121,8 @@ class ifenslave(moduleBase):
 
         # Also save a copy for future use
         ifaceobj.priv_data = list(slave_list)
-        ifaceobj.link_type = ifaceLinkType.LINK_MASTER
+        if ifaceobj.link_type != ifaceLinkType.LINK_NA:
+           ifaceobj.link_type = ifaceLinkType.LINK_MASTER
         return slave_list
 
     def get_dependent_ifacenames_running(self, ifaceobj):
@@ -217,10 +218,13 @@ class ifenslave(moduleBase):
                 self.log_warn('%s: skipping slave %s, does not exist'
                         %(ifaceobj.name, slave))
                 continue
+            link_up = False
             if self.ipcmd.is_link_up(slave):
                rtnetlink_api.rtnl_api.link_set(slave, "down")
+               link_up = True
             self.ipcmd.link_set(slave, 'master', ifaceobj.name)
-            rtnetlink_api.rtnl_api.link_set(slave, "up")
+            if link_up or ifaceobj.link_type != ifaceLinkType.LINK_NA:
+               rtnetlink_api.rtnl_api.link_set(slave, "up")
 
     def _apply_slaves_lacp_bypass_prio(self, ifaceobj):
         slaves = self.ifenslavecmd.get_slaves(ifaceobj.name)

addons/mstpctl.py

@@ -595,7 +595,7 @@ class mstpctl(moduleBase):
         # Check if this is a bridge port
         if not self._is_bridge_port(ifaceobj):
             # mark all the bridge attributes as error
-            ifaceobj.check_n_update_config_with_status_many(
+            ifaceobjcurr.check_n_update_config_with_status_many(ifaceobj,
                             self._port_attrs_map.keys(), 0)
             return
         bridgename = self.ipcmd.bridge_port_get_bridge_name(ifaceobj.name)

addons/vxlan.py

@@ -80,7 +80,7 @@ class vxlan(moduleBase):
         # Update vxlan object
         vxlanattrs = self.ipcmd.get_vxlandev_attrs(ifaceobj.name)
         if not vxlanattrs:
-            ifaceobjcurr.check_n_update_config_with_status_many(
+            ifaceobjcurr.check_n_update_config_with_status_many(ifaceobj,
                     self.get_mod_attrs(), -1)
             return
         self._query_check_n_update(ifaceobjcurr, 'vxlan-id',

config/ifupdown2.conf

@@ -32,3 +32,9 @@ ifquery_check_unknown_str=
 # in ifquery default output.
 ifquery_ifacename_expand_range=0
 
+# Let link master (bridges, bonds) own the link state of slaves
+link_master_slave=1
+
+# Delay admin state change till the end
+delay_admin_state_change=0
+

ifupdown/iface.py

@@ -23,9 +23,10 @@ class ifaceType():
     BRIDGE_VLAN = 0x2
 
 class ifaceLinkType():
-    UNKNOWN = 0x0
+    LINK_UNKNOWN = 0x0
     LINK_SLAVE = 0x1
     LINK_MASTER = 0x2
+    LINK_NA = 0x3
 
 class ifaceStatus():
     """Enumerates iface status """
@@ -221,7 +222,7 @@ class iface():
         """interface type"""
         self.priv_data = None
         self.realname = None
-        self.link_type = ifaceLinkType.UNKNOWN
+        self.link_type = ifaceLinkType.LINK_UNKNOWN
 
     def _set_attrs_from_dict(self, attrdict):
         self.auto = attrdict.get('auto', False)
@@ -358,13 +359,13 @@ class iface():
             # Not already error, mark success
             self.status = ifaceStatus.SUCCESS
 
-    def check_n_update_config_with_status_many(self, attr_names,
+    def check_n_update_config_with_status_many(self, ifaceobjorig, attr_names,
                                                attr_status=0):
         # set multiple attribute status to zero
         # also updates status only if the attribute is present
         for attr_name in attr_names:
-            if not self.get_attr_value_first(attr_name):
+            if not ifaceobjorig.get_attr_value_first(attr_name):
-                return
+               continue
             self.config.setdefault(attr_name, []).append('')
             self._config_status.setdefault(attr_name, []).append(attr_status)
 
@@ -422,7 +423,7 @@ class iface():
         self.priv_flags = 0
         self.raw_config = []
         self.flags |= self._PICKLED
-        self.link_type = ifaceLinkType.UNKNOWN
+        self.link_type = ifaceLinkType.LINK_UNKNOWN
 
     def dump_raw(self, logger):
         indent = '  '

ifupdown/ifupdownmain.py

@@ -60,8 +60,8 @@ class ifupdownMain(ifupdownBase):
     ADDONS_ENABLE = False
 
     # priv flags to mark iface objects
-    BUILTIN = 0x1
+    BUILTIN = 0x0001
-    NOCONFIG = 0x2
+    NOCONFIG = 0x0010
 
     scripts_dir='/etc/network'
     addon_modules_dir='/usr/share/ifupdownaddons'
@@ -108,6 +108,12 @@ class ifupdownMain(ifupdownBase):
 
     # Handlers for ops that ifupdown2 owns
     def run_up(self, ifaceobj):
+        if (ifaceobj.addr_method and
+            ifaceobj.addr_method == 'manual'):
+            return
+        if self._delay_admin_state:
+            self._delay_admin_state_iface_queue.append(ifaceobj.name)
+            return
         # If this object is a link slave, ie its link is controlled
         # by its link master interface, then dont set the link state.
         # But do allow user to change state of the link if the interface
@@ -115,19 +121,27 @@ class ifupdownMain(ifupdownBase):
         if ((ifaceobj.link_type == ifaceLinkType.LINK_SLAVE) and
              not os.path.exists('/sys/class/net/%s/master' %ifaceobj.name)):
             return
-        if self.link_exists(ifaceobj.name):
+        if not self.link_exists(ifaceobj.name):
-            self.link_up(ifaceobj.name)
+           return
+        self.link_up(ifaceobj.name)
 
     def run_down(self, ifaceobj):
+        if (ifaceobj.addr_method and
+            ifaceobj.addr_method == 'manual'):
+            return
+        if self._delay_admin_state:
+            self._delay_admin_state_iface_queue.append(ifaceobj.name)
+            return
         # If this object is a link slave, ie its link is controlled
         # by its link master interface, then dont set the link state.
         # But do allow user to change state of the link if the interface
         # is already with its link master (hence the master check).
         if ((ifaceobj.link_type == ifaceLinkType.LINK_SLAVE) and
-            not os.path.exists('/sys/class/net/%s/master' %ifaceobj.name)):
+             not os.path.exists('/sys/class/net/%s/master' %ifaceobj.name)):
             return
-        if self.link_exists(ifaceobj.name):
+        if not self.link_exists(ifaceobj.name):
-            self.link_down(ifaceobj.name)
+           return
+        self.link_down(ifaceobj.name)
 
     # ifupdown object interface operation handlers
     ops_handlers = OrderedDict([('up', run_up),
@@ -215,6 +229,14 @@ class ifupdownMain(ifupdownBase):
                 raise
         else:
             self.STATEMANAGER_UPDATE = False
+        self._delay_admin_state = True if self.config.get(
+                            'delay_admin_state_change', '0') == '1' else False
+        self._delay_admin_state_iface_queue = []
+        if not self._delay_admin_state:
+            self._link_master_slave = True if self.config.get(
+                      'link_master_slave', '0') == '1' else False
+        else:
+            self._link_master_slave = False
 
     def get_ifaceobjs(self, ifacename):
         return self.ifaceobjdict.get(ifacename)
@@ -348,8 +370,8 @@ class ifupdownMain(ifupdownBase):
             if not dilist:
                 ni = None
                 if self.is_iface_builtin_byname(d):
-                    ni = self.create_n_save_ifaceobj(d, self.BUILTIN | self.NOCONFIG,
+                    ni = self.create_n_save_ifaceobj(d,
-                            True)
+                            self.BUILTIN | self.NOCONFIG, True)
                 elif not self._DELETE_DEPENDENT_IFACES_WITH_NOCONFIG:
                     ni = self.create_n_save_ifaceobj(d, self.NOCONFIG,
                             True)
@@ -365,7 +387,6 @@ class ifupdownMain(ifupdownBase):
                     di.add_to_upperifaces(upperifaceobj.name)
                     if upperifaceobj.link_type == ifaceLinkType.LINK_MASTER:
                         di.link_type = ifaceLinkType.LINK_SLAVE
-
         for d in del_list:
             dlist.remove(d)
 
@@ -448,6 +469,8 @@ class ifupdownMain(ifupdownBase):
         if ifaceobj.compare(currentifaceobjlist[0]):
             self.logger.warn('duplicate interface %s found' %ifaceobj.name)
             return
+        if not self._link_master_slave:
+           ifaceobj.link_type = ifaceLinkType.LINK_NA
         if currentifaceobjlist[0].type == ifaceobj.type:
             currentifaceobjlist[0].flags |= iface.HAS_SIBLINGS
             ifaceobj.flags |= iface.HAS_SIBLINGS
@@ -747,6 +770,23 @@ class ifupdownMain(ifupdownBase):
         else:
             self.type = ifaceType.UNKNOWN
 
+    def _process_delay_admin_state_queue(self, op):
+        if not self._delay_admin_state_iface_queue:
+           return
+        if op == 'up':
+           func = self.link_up
+        elif op == 'down':
+           func = self.link_down
+        else:
+           return
+        for i in self._delay_admin_state_iface_queue:
+            try:
+                if self.link_exists(i):
+                   func(i)
+            except Exception, e:
+                self.logger.warn(str(e))
+                pass
+
     def up(self, ops, auto=False, allow_classes=None, ifacenames=None,
            excludepats=None, printdependency=None, syntaxcheck=False,
            type=None, skipupperifaces=False):
@@ -805,6 +845,7 @@ class ifupdownMain(ifupdownBase):
             self._sched_ifaces(filtered_ifacenames, ops,
                     skipupperifaces=skipupperifaces)
         finally:
+            self._process_delay_admin_state_queue('up')
             if not self.DRYRUN and self.ADDONS_ENABLE:
                 self._save_state()
 
@@ -865,6 +906,7 @@ class ifupdownMain(ifupdownBase):
         try:
             self._sched_ifaces(filtered_ifacenames, ops)
         finally:
+            self._process_delay_admin_state_queue('down')
             if not self.DRYRUN and self.ADDONS_ENABLE:
                 self._save_state()
 
@@ -1090,7 +1132,13 @@ class ifupdownMain(ifupdownBase):
                 self.dependency_graph = OrderedDict({})
                 # Generate dependency info for old config
                 self.populate_dependency_info(downops, ifacedownlist)
-                self._sched_ifaces(ifacedownlist, downops)
+                try:
+                    self._sched_ifaces(ifacedownlist, downops)
+                except Exception, e:
+                    self.logger.error(str(e))
+                    pass
+                finally:
+                    self._process_delay_admin_state_queue('down')
             else:
                 self.logger.debug('no interfaces to down ..')
 
@@ -1107,7 +1155,13 @@ class ifupdownMain(ifupdownBase):
 
         self.logger.info('reload: scheduling up on interfaces: %s'
                          %str(filtered_ifacenames))
-        self._sched_ifaces(filtered_ifacenames, upops)
+        try:
+            self._sched_ifaces(filtered_ifacenames, upops)
+        except Exception, e:
+            self.logger.error(str(e))
+            pass
+        finally:
+            self._process_delay_admin_state_queue('up')
         if self.DRYRUN:
             return
         self._save_state()

ifupdown/networkinterfaces.py

@@ -287,9 +287,9 @@ class networkInterfaces():
         if range_val:
            for v in range(range_val[1], range_val[2]):
                 ifaceobj_new = copy.deepcopy(ifaceobj)
-                ifaceobj_new.realname = ifaceobj.name
+                ifaceobj_new.realname = '%s' %ifaceobj.name
-                ifaceobj_new.name = "%s%d" %(range_val[0], v)
+                ifaceobj_new.name = '%s%d' %(range_val[0], v)
-                ifaceobj_new.flags |= iface.IFACERANGE_ENTRY
+                ifaceobj_new.flags = iface.IFACERANGE_ENTRY
                 if v == range_val[1]:
                     ifaceobj_new.flags |= iface.IFACERANGE_START
                 self.callbacks.get('iface_found')(ifaceobj_new)
@@ -306,10 +306,10 @@ class networkInterfaces():
         if range_val:
            for v in range(range_val[1], range_val[2]):
                 ifaceobj_new = copy.deepcopy(ifaceobj)
-                ifaceobj_new.realname = ifaceobj.name
+                ifaceobj_new.realname = '%s' %ifaceobj.name
-                ifaceobj_new.name = "%s%d" %(range_val[0], v)
+                ifaceobj_new.name = '%s%d' %(range_val[0], v)
                 ifaceobj_new.type = ifaceType.BRIDGE_VLAN
-                ifaceobj_new.flags |= iface.IFACERANGE_ENTRY
+                ifaceobj_new.flags = iface.IFACERANGE_ENTRY
                 if v == range_val[1]:
                     ifaceobj_new.flags |= iface.IFACERANGE_START
                 self.callbacks.get('iface_found')(ifaceobj_new)

ifupdown/rtnetlink_api.py

@@ -40,7 +40,7 @@ class rtnetlinkApi(RtNetlink):
         return ifindex
 
     def create_vlan(self, link, ifname, vlanid):
-        self.logger.info('rtnetlink: creating vlan %s' %ifname)
+        self.logger.info('rtnetlink: creating vlan interface %s' %ifname)
         if ifupdownmain.ifupdownFlags.DRYRUN:
             return
         try:
@@ -64,7 +64,7 @@ class rtnetlinkApi(RtNetlink):
         self.process_wait([token])
 
     def create_macvlan(self, ifname, link, mode='private'):
-        self.logger.info('rtnetlink: creating macvlan %s' %ifname)
+        self.logger.info('rtnetlink: creating macvlan interface %s' %ifname)
         if ifupdownmain.ifupdownFlags.DRYRUN:
             return
         try:

ifupdown/scheduler.py

@@ -46,10 +46,6 @@ class ifaceScheduler():
         if ifupdownobj.type and ifupdownobj.type != ifaceobj.type:
             return
 
-        if (cls._STATE_CHECK and
-            (ifaceobj.state >= ifaceState.from_str(op))):
-            ifupdownobj.logger.debug('%s: already in state %s' %(ifacename, op))
-            return
         if not ifupdownobj.ADDONS_ENABLE: return
         if op == 'query-checkcurr':
             query_ifaceobj=ifupdownobj.create_n_save_ifaceobjcurr(ifaceobj)
@@ -114,6 +110,7 @@ class ifaceScheduler():
         """ Runs all operations on a list of interface
             configurations for the same interface
         """
+
         # minor optimization. If operation is 'down', proceed only
         # if interface exists in the system
         ifacename = ifaceobjs[0].name
@@ -135,14 +132,12 @@ class ifaceScheduler():
             # for the first object in the list
             handler = ifupdownobj.ops_handlers.get(op)
             if handler:
-                if (not ifaceobjs[0].addr_method or
+                try:
-                    (ifaceobjs[0].addr_method and
+                    handler(ifupdownobj, ifaceobjs[0])
-                    ifaceobjs[0].addr_method != 'manual')):
+                except Exception, e:
-                    try:
+                    ifupdownobj.logger.warn('%s: %s'
-                        handler(ifupdownobj, ifaceobjs[0])
+                                            %(ifaceobjs[0].name, str(e)))
-                    except Exception, e:
+                    pass
-                        ifupdownobj.logger.warn('%s' %str(e))
-                        pass
             for ifaceobj in ifaceobjs:
                 cls.run_iface_op(ifupdownobj, ifaceobj, op,
                     cenv=ifupdownobj.generate_running_env(ifaceobj, op)
@@ -212,10 +207,16 @@ class ifaceScheduler():
         if not ifaceobjs:
             raise Exception('%s: not found' %ifacename)
 
+        # Check state of the dependent. If it is already brought up, return
+        if (cls._STATE_CHECK and
+            (ifaceobjs[0].state == ifaceState.from_str(ops[-1]))):
+            ifupdownobj.logger.debug('%s: already processed' %ifacename)
+            return
+
         for ifaceobj in ifaceobjs:
             if not cls._check_upperifaces(ifupdownobj, ifaceobj,
                                           ops, parent, followdependents):
-                return
+               return
 
         # If inorder, run the iface first and then its dependents
         if order == ifaceSchedulerFlags.INORDER:
@@ -288,6 +289,11 @@ class ifaceScheduler():
         if not ifaceobjs:
             raise Exception('%s: not found' %ifacename)
 
+        if (cls._STATE_CHECK and
+            (ifaceobjs[0].state == ifaceState.from_str(ops[-1]))):
+            ifupdownobj.logger.debug('%s: already processed' %ifacename)
+            return
+
         if not skip_root:
             # run the iface first and then its upperifaces
             cls.run_iface_list_ops(ifupdownobj, ifaceobjs, ops)