execute 'up' on upper devices if ifup is called with --with-depends

Ticket: CM-1438
Reviewed By: review pending
Testing Done: Tested ifup/ifdown

Before this patch, `ifup --with-depends <iface>` only brought up
lowerdevices. Because those were enough for iface to function.

And if ifaces above it (upperdevices) needed fixing, user could just
execute `ifup --with-depends <ifaceupper>`.

But in a recent, bond under a bridge bug in 2.0, got me thinking that
its probably better to up the upperdevices which might be impacted as
well. and this patch does just that.

The patch includes changes to make ifupdown generate dependency
information for all interfaces even if the user requested to operate
on a single interface. This is to get a full view of the interfaces file.
This might add some overhead. Should not change anything during boot.
Still looking at ways to optimize.

pkg/graph.py

@@ -53,51 +53,6 @@ class graph():
 
         return S
 
-    @classmethod
-    def topological_sort_graph(cls, dependency_graph, indegrees, rootifname):
-        S = []
-        Q = deque()
-
-        Q.append(rootifname)
-
-        while len(Q):
-            # initialize queue
-            x = Q.popleft()
-
-            # Get dependents of x
-            dlist = dependency_graph.get(x)
-            if not dlist:
-                S.append(x)
-                continue
-
-            for y in dlist:
-                indegrees[y] = indegrees.get(y) - 1
-                if indegrees.get(y) == 0:
-                    Q.append(y)
-
-            S.append(x)
-
-        return S
-
-    @classmethod
-    def topological_sort_graphs(cls, dependency_graphs, indegrees):
-        """ Sorts graph one at a time merges all the sorted graph
-        lists and returns a combined list
-       
-        """
-        sorted_graphs_list = []
-        for ifname,indegree in indegrees.items():
-            if indegree == 0:
-                sorted_graphs_list += cls.topological_sort_graph(
-                                        dependency_graphs, indegrees, ifname)
-        # If some indegrees are non zero, we have a cycle
-        for ifname,indegree in indegrees.items():
-            if indegree != 0:
-                raise Exception('cycle found involving iface %s' %ifname +
-                                ' (indegree %d)' %indegree)
-
-        return sorted_graphs_list
-
     @classmethod
     def generate_dots(cls, dependency_graph, indegrees):
         gvgraph = GvGen()

pkg/iface.py

@@ -382,6 +382,7 @@ class iface():
         del odict['state']
         del odict['status']
         del odict['lowerifaces']
+        del odict['upperifaces']
         del odict['refcnt']
         del odict['config_status']
         del odict['flags']
@@ -399,6 +400,7 @@ class iface():
         self.refcnt = 0
         self.flags = 0
         self.lowerifaces = None
+        self.upperifaces = None
         self.linkstate = None
         self.env = None
         self.priv_flags = 0

pkg/ifupdownmain.py

@@ -27,7 +27,6 @@ class ifupdownMain(ifupdownBase):
     # Flags
     WITH_DEPENDS = False
     ALL = False
-    STATE_CHECK = False
     COMPAT_EXEC_SCRIPTS = False
     STATEMANAGER_ENABLE = True
     STATEMANAGER_UPDATE = True
@@ -219,6 +218,9 @@ class ifupdownMain(ifupdownBase):
             return ifaceobjs[0]
         return None
 
+    def get_ifacenames(self):
+        return self.ifaceobjdict.keys()
+
     def get_iface_obj_last(self, ifacename):
         return self.ifaceobjdict.get(ifacename)[-1]
 
@@ -368,11 +370,12 @@ class ifupdownMain(ifupdownBase):
                 break
         return dlist
 
-    def populate_dependency_info(self, ifacenames, ops):
+    def populate_dependency_info(self, ops, ifacenames=None):
         """ recursive function to generate iface dependency info """
 
         if not ifacenames:
             ifacenames = self.ifaceobjdict.keys()
+
         self.logger.debug('populating dependency info for %s' %str(ifacenames))
         iqueue = deque(ifacenames)
         while iqueue:
@@ -549,49 +552,26 @@ class ifupdownMain(ifupdownBase):
             iface_objs.append(iface_obj)
         return iface_objs
 
-
-    def run_without_dependents(self, ops, ifacenames):
-        """ Run interface list without their dependents """
-        if not ifacenames:
-            raise ifupdownInvalidValue('no interfaces found')
-
-        self.logger.debug('run_without_dependents for ops %s for %s'
-                %(str(ops), str(ifacenames)))
-
-        ifaceScheduler.run_iface_list(self, ifacenames, ops, parent=None,
-                                  order=ifaceSchedulerFlags.INORDER
-                                    if 'down' in ops[0]
-                                        else ifaceSchedulerFlags.POSTORDER,
-                                                followdependents=False)
-
     def _pretty_print_ordered_dict(self, argdict):
         for k, vlist in argdict.items():
             self.logger.info('%s : %s' %(k, str(vlist)))
 
-    def run_with_dependents(self, ops, ifacenames):
+    def sched_ifaces(self, ifacenames, ops):
-        ret = 0
+        self.logger.debug('scheduling \'%s\' for %s'
-        self.logger.debug('running \'%s\' with dependents for %s'
                           %(str(ops), str(ifacenames)))
 
-        if not ifacenames:
-            ifacenames = self.ifaceobjdict.keys()
-
         self.logger.info('dependency graph:')
         self._pretty_print_ordered_dict(self.dependency_graph)
 
-        if self.njobs > 1:
+        return ifaceScheduler.sched_ifaces(self, ifacenames, ops,
-            ret = ifaceScheduler.run_iface_dependency_graph_parallel(self,
+                        dependency_graph=self.dependency_graph,
-                        self.dependency_graph, ops)
-        else:
-            ret = ifaceScheduler.run_iface_dependency_graphs(self,
-                        self.dependency_graph, ops,
                         order=ifaceSchedulerFlags.INORDER
                             if 'down' in ops[0]
-                                else ifaceSchedulerFlags.POSTORDER)
+                                else ifaceSchedulerFlags.POSTORDER,
-        return ret
+                        followdependents=True if self.WITH_DEPENDS else False)
 
     def print_dependency(self, ifacenames, format):
-        if ifacenames is None:
+        if not ifacenames:
             ifacenames = self.ifaceobjdict.keys()
 
         if format == 'list':
@@ -706,16 +686,14 @@ class ifupdownMain(ifupdownBase):
         if not filtered_ifacenames:
             raise Exception('no ifaces found matching given allow lists')
 
-        self.populate_dependency_info(filtered_ifacenames, ops)
-
         if printdependency:
+            self.populate_dependency_info(ops, filtered_ifacenames)
             self.print_dependency(filtered_ifacenames, printdependency)
             return
-
-        if self.WITH_DEPENDS:
-            self.run_with_dependents(ops, filtered_ifacenames)
         else:
-            self.run_without_dependents(ops, filtered_ifacenames)
+            self.populate_dependency_info(ops)
+
+        self.sched_ifaces(filtered_ifacenames, ops)
 
         if self.DRYRUN and self.ADDONS_ENABLE:
             return
@@ -758,16 +736,14 @@ class ifupdownMain(ifupdownBase):
         if not filtered_ifacenames:
             raise Exception('no ifaces found matching given allow lists')
 
-        self.populate_dependency_info(filtered_ifacenames, ops)
         if printdependency:
+            self.populate_dependency_info(ops, filtered_ifacenames)
             self.print_dependency(filtered_ifacenames, printdependency)
             return
-
-        if self.WITH_DEPENDS:
-            self.run_with_dependents(ops, filtered_ifacenames)
         else:
-            self.run_without_dependents(ops, filtered_ifacenames)
+            self.populate_dependency_info(ops)
 
+        self.sched_ifaces(filtered_ifacenames, ops)
         if self.DRYRUN and self.ADDONS_ENABLE:
             return
 
@@ -817,20 +793,19 @@ class ifupdownMain(ifupdownBase):
                 raise Exception('no ifaces found matching ' +
                         'given allow lists')
 
-        self.populate_dependency_info(filtered_ifacenames, ops)
         if ops[0] == 'query-dependency' and printdependency:
+            self.populate_dependency_info(ops, filtered_ifacenames)
             self.print_dependency(filtered_ifacenames, printdependency)
             return
+        else:
+            self.populate_dependency_info(ops)
 
         if ops[0] == 'query':
             return self.print_ifaceobjs_pretty(filtered_ifacenames, format)
         elif ops[0] == 'query-raw':
             return self.print_ifaceobjs_raw(filtered_ifacenames)
 
-        if self.WITH_DEPENDS:
+        self.sched_ifaces(filtered_ifacenames, ops)
-            self.run_with_dependents(ops, filtered_ifacenames)
-        else:
-            self.run_without_dependents(ops, filtered_ifacenames)
 
         if ops[0] == 'query-checkcurr':
             ret = self.print_ifaceobjscurr_pretty(filtered_ifacenames, format)
@@ -858,7 +833,7 @@ class ifupdownMain(ifupdownBase):
             raise
 
         # generate dependency graph of interfaces
-        self.populate_dependency_info(ifacenames, upops)
+        self.populate_dependency_info(upops)
 
         # Save a copy of new iface objects and dependency_graph
         new_ifaceobjdict = dict(self.get_ifaceobjdict())
@@ -918,8 +893,8 @@ class ifupdownMain(ifupdownBase):
                 # reinitialize dependency graph 
                 self.dependency_graph = OrderedDict({})
                 # Generate dependency info for old config
-                self.populate_dependency_info(ifacedownlist, downops)
+                self.populate_dependency_info(downops)
-                self.run_with_dependents(downops, ifacedownlist)
+                self.sched_ifaces(ifacedownlist, downops)
             else:
                 self.logger.debug('no interfaces to down ..')
 
@@ -934,12 +909,11 @@ class ifupdownMain(ifupdownBase):
                                excludepats, i)]
         self.logger.info('Executing up on interfaces: %s'
                                   %str(filtered_ifacenames))
-        if self.WITH_DEPENDS:
+
-            self.run_with_dependents(upops, filtered_ifacenames)
+        self.sched_ifaces(filtered_ifacenames, upops)
-        else:
-            self.run_without_dependents(upops, filtered_ifacenames)
         if self.DRYRUN:
             return
+
         self.save_state()
 
     def dump(self):
@@ -947,8 +921,8 @@ class ifupdownMain(ifupdownBase):
 
         print 'ifupdown object dump'
         print self.pp.pprint(self.modules)
-        print self.pp.pprint(self.ifaces)
+        print self.pp.pprint(self.ifaceobjdict)
-        self.state_manager.dump()
+        #self.state_manager.dump()
 
     def print_state(self, ifacenames=None):
         self.statemanager.dump(ifacenames)

pkg/scheduler.py

@@ -21,8 +21,8 @@ from threading import *
 from ifupdownbase import *
 
 class ifaceSchedulerFlags():
-    INORDER = 1
+    INORDER = 0x1
-    POSTORDER = 2
+    POSTORDER = 0x2
 
 class ifaceScheduler():
     """ scheduler functions to schedule configuration of interfaces.
@@ -31,6 +31,8 @@ class ifaceScheduler():
     or dependency graph format.
     """
 
+    _STATE_CHECK = True
+
     token_pool = None
 
     @classmethod
@@ -38,8 +40,9 @@ class ifaceScheduler():
         """ Runs sub operation on an interface """
         ifacename = ifaceobj.get_name()
 
-        if (ifaceobj.get_state() >= ifaceState.from_str(op) and
+        if (cls._STATE_CHECK and
-           ifaceobj.get_status() == ifaceStatus.SUCCESS):
+            (ifaceobj.get_state() >= ifaceState.from_str(op)) and
+            (ifaceobj.get_status() == ifaceStatus.SUCCESS)):
             ifupdownobj.logger.debug('%s: already in state %s' %(ifacename, op))
             return
 
@@ -111,11 +114,14 @@ class ifaceScheduler():
 
 
     @classmethod
-    def _check_upperifaces(cls, ifupdownobj, ifaceobj, ops, parent):
+    def _check_upperifaces(cls, ifupdownobj, ifaceobj, ops, parent, followdependents=False):
         """ Check if conflicting upper ifaces are around and warn if required
 
         Returns False if this interface needs to be skipped, else return True """
 
+        if 'up' in ops[0] and followdependents:
+            return True
+
         ifacename = ifaceobj.get_name()
         # Deal with upperdevs first
         ulist = ifaceobj.get_upperifaces()
@@ -156,7 +162,8 @@ class ifaceScheduler():
             raise Exception('%s: not found' %ifacename)
 
         for ifaceobj in ifaceobjs:
-            if not cls._check_upperifaces(ifupdownobj, ifaceobj, ops, parent):
+            if not cls._check_upperifaces(ifupdownobj, ifaceobj, ops, parent,
+                                          followdependents):
                 return
             if order == ifaceSchedulerFlags.INORDER:
                 # If inorder, run the iface first and then its dependents
@@ -221,8 +228,62 @@ class ifaceScheduler():
                                 %(ifacename, str(e)))
 
     @classmethod
-    def run_iface_dependency_graphs(cls, ifupdownobj,
+    def run_iface_graph_upper(cls, ifupdownobj, ifacename, ops, parent=None,
-                dependency_graph, ops, indegrees=None,
+                        followdependents=True, skip_root=False):
+        """ runs interface by traversing all nodes rooted at itself """
+
+        # Each ifacename can have a list of iface objects
+        ifaceobjs = ifupdownobj.get_ifaceobjs(ifacename)
+        if not ifaceobjs:
+            raise Exception('%s: not found' %ifacename)
+
+        for ifaceobj in ifaceobjs:
+            if not skip_root:
+                # run the iface first and then its upperifaces
+                cls.run_iface_ops(ifupdownobj, ifaceobj, ops)
+
+            # Run upperifaces
+            ulist = ifaceobj.get_upperifaces()
+            if ulist:
+                ifupdownobj.logger.debug('%s:' %ifacename +
+                    ' found upperifaces: %s' %str(ulist))
+                try:
+                    cls.run_iface_list_upper(ifupdownobj, ulist, ops,
+                                            ifacename,
+                                            followdependents,
+                                            continueonfailure=True)
+                except Exception, e:
+                    if (ifupdownobj.ignore_error(str(e))):
+                        pass
+                    else:
+                        raise
+
+    @classmethod
+    def run_iface_list_upper(cls, ifupdownobj, ifacenames,
+                       ops, parent=None, followdependents=True,
+                       continueonfailure=True, skip_root=False):
+        """ Runs interface list """
+
+        for ifacename in ifacenames:
+            try:
+              cls.run_iface_graph_upper(ifupdownobj, ifacename, ops, parent,
+                      followdependents, skip_root)
+            except Exception, e:
+                if continueonfailure:
+                    if ifupdownobj.logger.isEnabledFor(logging.DEBUG):
+                        traceback.print_tb(sys.exc_info()[2])
+                    ifupdownobj.logger.error('%s : %s' %(ifacename, str(e)))
+                    pass
+                else:
+                    if (ifupdownobj.ignore_error(str(e))):
+                        pass
+                    else:
+                        raise Exception('error running iface %s (%s)'
+                                %(ifacename, str(e)))
+
+    @classmethod
+    def sched_ifaces(cls, ifupdownobj, ifacenames, ops,
+                dependency_graph=None, indegrees=None,
                 order=ifaceSchedulerFlags.POSTORDER,
                 followdependents=True):
         """ Runs iface dependeny graph by visiting all the nodes
@@ -238,27 +299,42 @@ class ifaceScheduler():
         indegrees : indegree array if present is used to determine roots
                     of the graphs in the dependency_graph
         """
+
+        if not ifupdownobj.ALL or not followdependents or len(ifacenames) == 1:
+            cls.run_iface_list(ifupdownobj, ifacenames, ops,
+                                  parent=None,order=order,
+                                  followdependents=followdependents)
+            if not ifupdownobj.ALL and followdependents and 'up' in ops[0]:
+                # If user had given a set of interfaces to bring up
+                # try and execute 'up' on the upperifaces
+                ifupdownobj.logger.info('running upperifaces if available')
+                cls._STATE_CHECK = False
+                cls.run_iface_list_upper(ifupdownobj, ifacenames, ops,
+                                         skip_root=True)
+                cls._STATE_CHECK = True
+            return
         run_queue = []
 
+        # Get a sorted list of all interfaces
         if not indegrees:
             indegrees = OrderedDict()
             for ifacename in dependency_graph.keys():
                 indegrees[ifacename] = ifupdownobj.get_iface_refcnt(ifacename)
-
         sorted_ifacenames = graph.topological_sort_graphs_all(dependency_graph,
                                                           dict(indegrees))
         ifupdownobj.logger.debug('sorted ifacenames %s : '
                                  %str(sorted_ifacenames))
 
-        # Build a list of ifaces that dont have any dependencies
+        # From the sorted list, pick interfaces that user asked
-        for ifacename in sorted_ifacenames:
+        # and those that dont have any dependents first
-            if not indegrees.get(ifacename):
+        [run_queue.append(ifacename)
-                run_queue.append(ifacename)
+                    for ifacename in sorted_ifacenames
+                        if ifacename in ifacenames and
+                        not indegrees.get(ifacename)]
 
         ifupdownobj.logger.debug('graph roots (interfaces that dont have '
                                  'dependents):' + ' %s' %str(run_queue))
-
+        cls.run_iface_list(ifupdownobj, run_queue, ops,
-        return cls.run_iface_list(ifupdownobj, run_queue, ops,
                                   parent=None,order=order,
                                   followdependents=followdependents)
 

sbin/ifupdown

@@ -78,9 +78,6 @@ def run_query(args):
         if args.checkcurr:
             qop='query-checkcurr'
         elif args.running:
-            if not iflist:
-                iflist = [i for i in os.listdir('/sys/class/net/')
-                                    if os.path.isdir('/sys/class/net/%s' %i)]
             qop='query-running'
         elif args.raw:
             qop='query-raw'
@@ -96,6 +93,9 @@ def run_query(args):
                             args.perfmode or args.syntaxhelp or
                             (qop != 'query-checkcurr' and
                             qop != 'query-running')) else True)
+        if not iflist and qop == 'query-running':
+            iflist = [i for i in os.listdir('/sys/class/net/')
+                        if os.path.isdir('/sys/class/net/%s' %i)]
         logger.debug('creating ifupdown object ..')
         ifupdown_handle = ifupdownMain(withdepends=args.withdepends,
                                        perfmode=args.perfmode,