From f32151277177de2eb15c35133e3f908ac0dcc47b Mon Sep 17 00:00:00 2001 From: roopa Date: Wed, 12 Feb 2014 22:29:41 -0800 Subject: [PATCH] Fix l3 lag test failure Ticket: CM-1438 Reviewed By: Testing Done: l3 lag test with help from purna - THe down sequence in the new ifupdown was causing switchd some grief (wilson is looking at it). readded the topological sort which i had removed in favor of only walking the tree. With the fix,i dont see the switchd problem anymore. - And another down bug was causing the bond to go away prematurely (only with the all depends option). Added a upperdevice list to track upperdev references --- pkg/iface.py | 46 ++++++++++--------- pkg/ifupdownbase.py | 4 ++ pkg/ifupdownmain.py | 45 +++++++++---------- pkg/scheduler.py | 105 ++++++++++++++++++++++++++++---------------- 4 files changed, 117 insertions(+), 83 deletions(-) diff --git a/pkg/iface.py b/pkg/iface.py index b3ea7b7..045096c 100644 --- a/pkg/iface.py +++ b/pkg/iface.py @@ -134,12 +134,8 @@ class iface(): self.flags = 0x0 self.priv_flags = 0x0 self.refcnt = 0 - # dependents that are listed as in the - # config file - self.dependents = None - # All dependents (includes dependents that - # are not listed in the config file) - self.realdev_dependents = None + self.lowerifaces = None + self.upperifaces = None self.auto = False self.classes = [] self.env = None @@ -254,17 +250,24 @@ class iface(): def clear_flag(self, flag): self.flags &= ~flag - def set_dependents(self, dlist): - self.dependents = dlist + def set_lowerifaces(self, dlist): + self.lowerifaces = dlist - def get_dependents(self): - return self.dependents + def get_lowerifaces(self): + return self.lowerifaces - def set_realdev_dependents(self, dlist): - self.realdev_dependents = dlist + def set_upperifaces(self, dlist): + self.upperifaces = dlist - def get_realdev_dependents(self): - return self.realdev_dependents + def add_to_upperifaces(self, upperifacename): + if self.upperifaces: + if upperifacename not in self.upperifaces: + self.upperifaces.append(upperifacename) + else: + self.upperifaces = [upperifacename] + + def get_upperifaces(self): + return self.upperifaces def set_linkstate(self, l): self.linkstate = l @@ -358,8 +361,7 @@ class iface(): odict = self.__dict__.copy() del odict['state'] del odict['status'] - del odict['dependents'] - del odict['realdev_dependents'] + del odict['lowerifaces'] del odict['refcnt'] return odict @@ -369,8 +371,7 @@ class iface(): self.state = ifaceState.NEW self.status = ifaceStatus.UNKNOWN self.refcnt = 0 - self.dependents = None - self.realdev_dependents = None + self.lowerifaces = None self.linkstate = None def dump_raw(self, logger): @@ -389,14 +390,11 @@ class iface(): logger.info(indent + 'status: %s' %ifaceStatus.to_str(self.get_status())) logger.info(indent + 'refcnt: %d' %self.get_refcnt()) - d = self.get_dependents() + d = self.get_lowerdevs() if d is not None: - logger.info(indent + 'dependents: %s' %str(d)) + logger.info(indent + 'lowerdevs: %s' %str(d)) else: - logger.info(indent + 'dependents: None') - - logger.info(indent + 'realdev dependents: %s' - %str(self.get_realdev_dependents())) + logger.info(indent + 'lowerdevs: None') logger.info(indent + 'config: ') config = self.get_config() diff --git a/pkg/ifupdownbase.py b/pkg/ifupdownbase.py index 9fc2895..f427753 100644 --- a/pkg/ifupdownbase.py +++ b/pkg/ifupdownbase.py @@ -10,6 +10,7 @@ import logging import subprocess import re +import os from ifupdown.iface import * class ifupdownBase(object): @@ -60,3 +61,6 @@ class ifupdownBase(object): #raise Exception(str) else: pass + + def link_exists(self, ifacename): + return os.path.exists('/sys/class/net/%s' %ifacename) diff --git a/pkg/ifupdownmain.py b/pkg/ifupdownmain.py index b957cad..dc76806 100644 --- a/pkg/ifupdownmain.py +++ b/pkg/ifupdownmain.py @@ -199,7 +199,7 @@ class ifupdownMain(): ifaceobjcurr = iface() ifaceobjcurr.set_name(ifacename) - ifaceobjcurr.set_dependents(ifaceobj.get_dependents()) + ifaceobjcurr.set_lowerifaces(ifaceobj.get_lowerifaces()) self.ifaceobjcurrdict[ifacename] = ifaceobjcurr return ifaceobjcurr @@ -240,6 +240,8 @@ class ifupdownMain(): ifaceobj.inc_refcnt() self.ifaceobjdict[ifacename] = [ifaceobj] + return ifaceobj + def is_iface_builtin_byname(self, ifacename): """ Returns true if iface name is a builtin interface. @@ -274,7 +276,7 @@ class ifupdownMain(): return self.is_ifaceobj_noconfig(ifaceobj) - def preprocess_dependency_list(self, dlist, ops): + def preprocess_dependency_list(self, upperifacename, dlist, ops): """ We go through the dependency list and delete or add interfaces from the interfaces dict by applying the following rules: @@ -295,17 +297,17 @@ class ifupdownMain(): dilist = self.get_iface_objs(d) if not dilist: if self.is_iface_builtin_byname(d): - self.create_n_save_ifaceobj(d, - self.BUILTIN | self.NOCONFIG, True), - elif self._DELETE_DEPENDENT_IFACES_WITH_NOCONFIG == False: - # create fake devices to all dependents that dont - # have config - self.create_n_save_ifaceobj(d, self.NOCONFIG, True), + self.create_n_save_ifaceobj(d, self.BUILTIN | self.NOCONFIG, + True).add_to_upperifaces(upperifacename) + elif not self._DELETE_DEPENDENT_IFACES_WITH_NOCONFIG: + self.create_n_save_ifaceobj(d, self.NOCONFIG, + True).add_to_upperifaces(upperifacename) else: del_list.append(d) else: for di in dilist: di.inc_refcnt() + di.add_to_upperifaces(upperifacename) for d in del_list: dlist.remove(d) @@ -329,9 +331,8 @@ class ifupdownMain(): dlist = module.get_dependent_ifacenames(ifaceobj, self.ifaceobjdict.keys()) if dlist and len(dlist): - ifaceobj.set_realdev_dependents(dlist[:]) self.logger.debug('%s: ' %ifaceobj.get_name() + - 'got dependency list: %s' %str(dlist)) + 'got lowerifaces/dependents: %s' %str(dlist)) break return dlist @@ -353,17 +354,18 @@ class ifupdownMain(): if ifaceobj is None: continue - dlist = ifaceobj.get_dependents() + dlist = ifaceobj.get_lowerifaces() if dlist is None: dlist = self.query_dependents(ifaceobj, ops) else: continue if dlist is not None: - self.preprocess_dependency_list(dlist, ops) - self.logger.debug('%s: dependency list after processing: %s' + self.preprocess_dependency_list(ifaceobj.get_name(), + dlist, ops) + self.logger.debug('%s: lowerifaces/dependents after processing: %s' %(i, str(dlist))) - ifaceobj.set_dependents(dlist) + ifaceobj.set_lowerifaces(dlist) [iqueue.append(d) for d in dlist] if self.dependency_graph.get(i) is None: @@ -529,7 +531,7 @@ class ifupdownMain(): %(str(ops), str(ifacenames))) ifaceSched = ifaceScheduler(force=self.FORCE) - ifaceSched.run_iface_list(self, ifacenames, ops, + ifaceSched.run_iface_list(self, ifacenames, ops, parent=None, order=ifaceSchedulerFlags.INORDER if 'down' in ops[0] else ifaceSchedulerFlags.POSTORDER, @@ -544,9 +546,8 @@ class ifupdownMain(): if ifacenames is None: ifacenames = self.ifaceobjdict.keys() - if self.logger.isEnabledFor(logging.DEBUG): - self.logger.debug('dependency graph:') - self.logger.debug(self.pp.pformat(self.dependency_graph)) + self.logger.info('dependency graph:') + self.logger.info(self.pp.pformat(self.dependency_graph)) if self.njobs > 1: ret = ifaceSched.run_iface_dependency_graph_parallel(self, @@ -981,7 +982,7 @@ class ifupdownMain(): ifaceobj.dump_raw(self.logger) print '\n' if self.WITH_DEPENDS: - dlist = ifaceobj.get_dependents() + dlist = ifaceobj.get_lowerifaces() if not dlist or not len(dlist): continue self.print_ifaceobjs_pretty(dlist, format) @@ -997,7 +998,7 @@ class ifupdownMain(): ifaceobj.dump_pretty() if self.WITH_DEPENDS: - dlist = ifaceobj.get_dependents() + dlist = ifaceobj.get_lowerifaces() if not dlist or not len(dlist): continue self.print_ifaceobjs_pretty(dlist, format) @@ -1035,7 +1036,7 @@ class ifupdownMain(): ifaceobj.dump_pretty() if self.WITH_DEPENDS: - dlist = ifaceobj.get_dependents() + dlist = ifaceobj.get_lowerifaces() if not dlist or not len(dlist): continue self.print_ifaceobjscurr_pretty(dlist, format) @@ -1057,7 +1058,7 @@ class ifupdownMain(): ifaceobj.dump_pretty() if self.WITH_DEPENDS: - dlist = ifaceobj.get_dependents() + dlist = ifaceobj.get_lowerifaces() if dlist is None or len(dlist) == 0: continue self.print_ifaceobjsrunning_pretty(dlist, format) return diff --git a/pkg/scheduler.py b/pkg/scheduler.py index 3796b56..ca7bacf 100644 --- a/pkg/scheduler.py +++ b/pkg/scheduler.py @@ -100,15 +100,15 @@ class ifaceScheduler(ifupdownBase): [self.run_iface_op(ifupdownobj, ifaceobj, op, cenv) for op in ops] - def run_iface_graph(self, ifupdownobj, ifacename, ops, + def run_iface_graph(self, ifupdownobj, ifacename, ops, parent=None, order=ifaceSchedulerFlags.POSTORDER, followdependents=True): """ runs interface by traversing all nodes rooted at itself """ # minor optimization. If operation is 'down', proceed only # if interface exists in the system - if ('down' in ops[0] and - not os.path.exists('/sys/class/net/%s' %ifacename)): + if 'down' in ops[0] and not self.link_exists(ifacename): + self.logger.info('%s: does not exist' %ifacename) return # Each ifacename can have a list of iface objects @@ -117,32 +117,62 @@ class ifaceScheduler(ifupdownBase): raise Exception('%s: not found' %ifacename) for ifaceobj in ifaceobjs: + # Deal with upperdevs first + ulist = ifaceobj.get_upperifaces() + if ulist: + self.logger.debug('%s: parent = %s, ulist = %s' + %(ifacename, parent, ulist)) + tmpulist = ([u for u in ulist if u != parent] if parent + else ulist) + if tmpulist: + self.logger.debug('%s: parent = %s, tmpulist = %s' + %(ifacename, parent, tmpulist)) + if 'down' in ops[0]: + # XXX: This is expensive. Find a cheaper way to do this + # if any of the upperdevs are present, + # dont down this interface + for u in tmpulist: + if self.link_exists(u): + if not ifupdownobj.ALL: + self.logger.warn('%s: skip interface ' + 'down upperiface %s still around' + %(ifacename, u)) + return + elif 'up' in ops[0] and not ifupdownobj.ALL: + # For 'up', just warn that there is an upperdev which is + # probably not up + for u in tmpulist: + if not self.link_exists(u): + self.logger.warn('%s: upper iface %s does not' + ' exist' %(ifacename, u)) + if order == ifaceSchedulerFlags.INORDER: - # Run all sub operations sequentially - try: - self.run_iface_ops(ifupdownobj, ifaceobj, ops) - except Exception, e: - raise Exception(str(e)) - # Run dependents - dlist = ifaceobj.get_dependents() - if dlist and len(dlist): + # If inorder, run the iface first and then its dependents + self.run_iface_ops(ifupdownobj, ifaceobj, ops) + + # Run lowerifaces or dependents + dlist = ifaceobj.get_lowerifaces() + if dlist: self.logger.info('%s:' %ifacename + ' found dependents: %s' %str(dlist)) try: if not followdependents: # XXX: this is yet another extra step, # but is needed for interfaces that are - # implicit dependents - # up without dependents, but + # implicit dependents. even though we are asked to + # not follow dependents, we must follow the ones + # that dont have user given config. Because we own them new_dlist = [d for d in dlist if ifupdownobj.is_iface_noconfig(d)] if new_dlist: self.run_iface_list(ifupdownobj, new_dlist, ops, - order, followdependents, + ifacename, order, + followdependents, continueonfailure=False) else: self.run_iface_list(ifupdownobj, dlist, ops, - order, followdependents, + ifacename, order, + followdependents, continueonfailure=False) except Exception, e: if (self.ignore_error(str(e))): @@ -152,21 +182,18 @@ class ifaceScheduler(ifupdownBase): ifaceobj.set_state(ifaceState.NEW) ifaceobj.set_status(ifaceStatus.ERROR) raise - if order == ifaceSchedulerFlags.POSTORDER: - try: - self.run_iface_ops(ifupdownobj, ifaceobj, ops) - except Exception, e: - raise Exception(str(e)) + self.run_iface_ops(ifupdownobj, ifaceobj, ops) + def run_iface_list(self, ifupdownobj, ifacenames, - ops, order=ifaceSchedulerFlags.POSTORDER, + ops, parent=None, order=ifaceSchedulerFlags.POSTORDER, followdependents=True, continueonfailure=True): """ Runs interface list """ for ifacename in ifacenames: try: - self.run_iface_graph(ifupdownobj, ifacename, ops, + self.run_iface_graph(ifupdownobj, ifacename, ops, parent, order, followdependents) except Exception, e: if continueonfailure: @@ -196,24 +223,28 @@ class ifaceScheduler(ifupdownBase): indegrees : indegree array if present is used to determine roots of the graphs in the dependency_graph """ - run_queue = [] - # Build a list of ifaces that dont have any dependencies - if indegrees: - # use indegrees array if specified - for ifacename, degree in indegrees.items(): - if not indegrees.get(ifacename): - run_queue.append(ifacename) - else: - for ifacename in dependency_graph.keys(): - if not ifupdownobj.get_iface_refcnt(ifacename): - run_queue.append(ifacename) - self.logger.debug('graph roots (interfaces that dont have ' + if indegrees is None: + 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)) + self.logger.debug('sorted ifacenames %s : ' %str(sorted_ifacenames)) + + # Build a list of ifaces that dont have any dependencies + for ifacename in sorted_ifacenames: + if not indegrees.get(ifacename): + run_queue.append(ifacename) + + self.logger.info('graph roots (interfaces that dont have ' 'dependents):' + ' %s' %str(run_queue)) - return self.run_iface_list(ifupdownobj, run_queue, ops, order, - followdependents) + return self.run_iface_list(ifupdownobj, run_queue, ops, + parent=None,order=order, + followdependents=followdependents) def run_iface(self, ifupdownobj, ifacename, ops): @@ -328,7 +359,7 @@ class ifaceScheduler(ifupdownBase): for ifaceobj in ifaceobjs: # Run dependents - dlist = ifaceobj.get_dependents() + dlist = ifaceobj.get_lowerifaces() if dlist is not None and len(dlist) > 0: self.logger.debug('%s:' %ifacename + ' found dependents: %s' %str(dlist))