From ee9d01f724abec7b62133598a10b78ae0fffc0f0 Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Sat, 14 Jun 2014 22:53:23 -0700 Subject: [PATCH 01/11] Fix scheduler for --allow option Ticket: Reviewed By: nobody Testing Done: Tested --allow --- pkg/scheduler.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/scheduler.py b/pkg/scheduler.py index 6fc19c5..b0eaa0c 100644 --- a/pkg/scheduler.py +++ b/pkg/scheduler.py @@ -375,6 +375,11 @@ class ifaceScheduler(): ifupdownobj.logger.debug('graph roots (interfaces that dont have ' 'dependents):' + ' %s' %str(run_queue)) - cls.run_iface_list(ifupdownobj, run_queue, ops, + if run_queue: + cls.run_iface_list(ifupdownobj, run_queue, ops, + parent=None,order=order, + followdependents=followdependents) + else: + cls.run_iface_list(ifupdownobj, ifacenames, ops, parent=None,order=order, followdependents=followdependents) From e2021a9531ad8438a6912ae9ee776ab86550eab8 Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Wed, 25 Jun 2014 15:23:04 -0700 Subject: [PATCH 02/11] Fix handling of interfaces with `--allow CLASS` Ticket: CM-3105 Reviewed By: Testing Done: precommit and some interface class tests - the patch mostly involves code reorg --- pkg/ifupdownmain.py | 9 ++- pkg/scheduler.py | 146 ++++++++++++++++++++++++++++---------------- sbin/ifupdown | 6 +- 3 files changed, 105 insertions(+), 56 deletions(-) diff --git a/pkg/ifupdownmain.py b/pkg/ifupdownmain.py index 56107e2..9eff533 100644 --- a/pkg/ifupdownmain.py +++ b/pkg/ifupdownmain.py @@ -35,6 +35,7 @@ class ifupdownMain(ifupdownBase): # Flags WITH_DEPENDS = False ALL = False + IFACE_CLASS = False COMPAT_EXEC_SCRIPTS = False STATEMANAGER_ENABLE = True STATEMANAGER_UPDATE = True @@ -625,6 +626,8 @@ class ifupdownMain(ifupdownBase): excludepats=None, printdependency=None, syntaxcheck=False): """ up an interface """ + if allow_classes: + self.IFACE_CLASS = True if not self.ADDONS_ENABLE: self.STATEMANAGER_UPDATE = False if auto: self.ALL = True @@ -670,6 +673,8 @@ class ifupdownMain(ifupdownBase): excludepats=None, printdependency=None, usecurrentconfig=False): """ down an interface """ + if allow_classes: + self.IFACE_CLASS = True if not self.ADDONS_ENABLE: self.STATEMANAGER_UPDATE = False if auto: self.ALL = True @@ -726,9 +731,10 @@ class ifupdownMain(ifupdownBase): format='native'): """ query an interface """ + if allow_classes: + self.IFACE_CLASS = True if self.STATEMANAGER_ENABLE and ops[0] == 'query-savedstate': return self.statemanager.dump_pretty(ifacenames) - self.STATEMANAGER_UPDATE = False if auto: self.logger.debug('setting flag ALL') @@ -790,7 +796,6 @@ class ifupdownMain(ifupdownBase): def reload(self, upops, downops, auto=False, allow=None, ifacenames=None, excludepats=None, usecurrentconfig=False): """ reload interface config """ - allow_classes = [] new_ifaceobjdict = {} diff --git a/pkg/scheduler.py b/pkg/scheduler.py index b0eaa0c..4ab7636 100644 --- a/pkg/scheduler.py +++ b/pkg/scheduler.py @@ -307,6 +307,29 @@ class ifaceScheduler(): else: raise Exception('%s : (%s)' %(ifacename, str(e))) + @classmethod + def get_sorted_iface_list(cls, ifupdownobj, ifacenames, ops, + dependency_graph, indegrees=None): + if len(ifacenames) == 1: + return ifacenames + # 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) + ifacenames_all_sorted = graph.topological_sort_graphs_all( + dependency_graph, indegrees) + # if ALL was set, return all interfaces + if ifupdownobj.ALL: + return ifacenames_all_sorted + + # else return ifacenames passed as argument in sorted order + ifacenames_sorted = [] + [ifacenames_sorted.append(ifacename) + for ifacename in ifacenames_all_sorted + if ifacename in ifacenames] + return ifacenames_sorted + @classmethod def sched_ifaces(cls, ifupdownobj, ifacenames, ops, dependency_graph=None, indegrees=None, @@ -322,64 +345,81 @@ class ifaceScheduler(): format (contains more than one dependency graph) ops : list of operations to perform eg ['pre-up', 'up', 'post-up'] - indegrees : indegree array if present is used to determine roots - of the graphs in the dependency_graph + indegrees : indegree array if present is used to topologically sort + the graphs in the dependency_graph """ + # + # Algo: + # if ALL/auto interfaces are specified, + # - walk the dependency tree in postorder or inorder depending + # on the operation. + # (This is to run interfaces correctly in order) + # else: + # - sort iface list if the ifaces belong to a "class" + # - else just run iface list in the order they were specified + # + # Run any upperifaces if available + # + followupperifaces = [] + run_queue = [] + skip_ifacesort = int(ifupdownobj.config.get('skip_ifacesort', '0')) + if not skip_ifacesort and not indegrees: + indegrees = OrderedDict() + for ifacename in dependency_graph.keys(): + indegrees[ifacename] = ifupdownobj.get_iface_refcnt(ifacename) - if not ifupdownobj.ALL or not followdependents or len(ifacenames) == 1: + if not ifupdownobj.ALL: # If there is any interface that does exist, maybe it is a - # logical interface and we have to followupperifaces + # logical interface and we have to followupperifaces when it + # comes up, so get that list. followupperifaces = (True if [i for i in ifacenames if not ifupdownobj.link_exists(i)] else False) - cls.run_iface_list(ifupdownobj, ifacenames, ops, - parent=None,order=order, - followdependents=followdependents) - if (not ifupdownobj.ALL and - (followdependents or followupperifaces) 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 - - if ifupdownobj.config.get('skip_ifacesort', '0') == '1': - # This is a backdoor to skip sorting of interfaces, if required - cls.run_iface_list(ifupdownobj, ifacenames, ops, - parent=None,order=order, - followdependents=followdependents) - 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, - indegrees) - ifupdownobj.logger.debug('sorted ifacenames %s : ' - %str(sorted_ifacenames)) - - # From the sorted list, pick interfaces that user asked - # and those that dont have any dependents first - [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)) - if run_queue: - cls.run_iface_list(ifupdownobj, run_queue, ops, - parent=None,order=order, - followdependents=followdependents) + if not skip_ifacesort and ifupdownobj.IFACE_CLASS: + # sort interfaces only if allow class was specified and + # not skip_ifacesort + run_queue = cls.get_sorted_iface_list(ifupdownobj, ifacenames, + ops, dependency_graph, indegrees) + if run_queue and 'up' in ops[0]: + run_queue.reverse() else: - cls.run_iface_list(ifupdownobj, ifacenames, ops, - parent=None,order=order, - followdependents=followdependents) + # if -a is set, we dont really have to sort. We pick the interfaces + # that have no parents and + if not skip_ifacesort: + sorted_ifacenames = cls.get_sorted_iface_list(ifupdownobj, + ifacenames, ops, dependency_graph, + indegrees) + if sorted_ifacenames: + # pick interfaces that user asked + # and those that dont have any dependents first + [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)) + else: + ifupdownobj.logger.warn('interface sort returned None') + + # If queue not present, just run interfaces that were asked by the user + if not run_queue: + run_queue = list(ifacenames) + if 'down' in ops[0]: + run_queue.reverse() + + # run interface list + ifupdownobj.logger.info('running interfaces: %s' %str(run_queue)) + cls.run_iface_list(ifupdownobj, run_queue, ops, + parent=None, order=order, + followdependents=followdependents) + if (((not ifupdownobj.ALL and followdependents) or + followupperifaces) 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 diff --git a/sbin/ifupdown b/sbin/ifupdown index 43d168a..09038ec 100755 --- a/sbin/ifupdown +++ b/sbin/ifupdown @@ -338,12 +338,16 @@ def validate_args(op, args): # return True if op == 'query' and args.syntaxhelp: return True - if not args.iflist and not args.all: + if not args.iflist and not args.all and not args.CLASS: print '\'-a\' option or interface list are required' return False if args.iflist and args.all: print '\'-a\' option and interface list are mutually exclusive' return False + if args.CLASS and (args.all or args.iflist): + print ('\'--allow\' option is mutually exclusive ' + + 'with interface list and \'-a\'') + return False return True def read_config(): From 1d787a1775a05f91d69ce0c2649a1d38929ec1de Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Fri, 27 Jun 2014 23:44:36 -0700 Subject: [PATCH 03/11] Fix splits everywhere to include space and tabs. Use regex split Ticket: CM-3121 Reviewed By: Testing Done: Ran precommit --- docs/examples/interfaces_with_template | 4 +--- pkg/networkinterfaces.py | 23 ++++++++++++----------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/docs/examples/interfaces_with_template b/docs/examples/interfaces_with_template index 8bb43a2..af9d571 100644 --- a/docs/examples/interfaces_with_template +++ b/docs/examples/interfaces_with_template @@ -17,10 +17,8 @@ auto vlan-${v} iface vlan-${v} inet static bridge-ports glob swp1-6.${v} bridge-stp on - bridge-treeprio 32768 bridge-ageing 200 - bridge-maxhops 10 bridge-maxage 10 - bridge-fdelay 10 + bridge-fd 10 %endfor diff --git a/pkg/networkinterfaces.py b/pkg/networkinterfaces.py index 092e51b..5d1d227 100644 --- a/pkg/networkinterfaces.py +++ b/pkg/networkinterfaces.py @@ -42,6 +42,7 @@ class networkInterfaces(): self._template_engine = templateEngine(template_engine, template_lookuppath) self._currentfile_has_template = False + self._ws_split_regex = re.compile(r'[\s\t]\s*') @property def _currentfile(self): @@ -90,7 +91,7 @@ class networkInterfaces(): def process_allow(self, lines, cur_idx, lineno): allow_line = lines[cur_idx] - words = allow_line.split() + words = re.split(self._ws_split_regex, allow_line) if len(words) <= 1: raise Exception('invalid allow line \'%s\' at line %d' %(allow_line, lineno)) @@ -108,7 +109,7 @@ class networkInterfaces(): def process_source(self, lines, cur_idx, lineno): # Support regex self.logger.debug('processing sourced line ..\'%s\'' %lines[cur_idx]) - sourced_file = lines[cur_idx].split(' ', 2)[1] + sourced_file = re.split(self._ws_split_regex, lines[cur_idx], 2)[1] if sourced_file: for f in glob.glob(sourced_file): self.read_file(f) @@ -118,7 +119,7 @@ class networkInterfaces(): return 0 def process_auto(self, lines, cur_idx, lineno): - auto_ifaces = lines[cur_idx].split()[1:] + auto_ifaces = re.split(self._ws_split_regex, lines[cur_idx])[1:] if not auto_ifaces: self._parse_error(self._currentfile, lineno, 'invalid auto line \'%s\''%lines[cur_idx]) @@ -166,7 +167,7 @@ class networkInterfaces(): ifaceobj = iface() iface_line = lines[cur_idx].strip(whitespaces) - iface_attrs = iface_line.split() + iface_attrs = re.split(self._ws_split_regex, iface_line) ifacename = iface_attrs[1] ifaceobj.raw_config.append(iface_line) @@ -176,19 +177,19 @@ class networkInterfaces(): l = lines[line_idx].strip(whitespaces) if self.ignore_line(l) == 1: continue - if self._is_keyword(l.split()[0]): + attrs = re.split(self._ws_split_regex, l, 1) + if self._is_keyword(attrs[0]): line_idx -= 1 break - ifaceobj.raw_config.append(l) - # preprocess vars (XXX: only preprocesses $IFACE for now) - l = re.sub(r'\$IFACE', ifacename, l) - attrs = l.split(' ', 1) + # if not a keyword, every line must have at least a key and value if len(attrs) < 2: self._parse_error(self._currentfile, line_idx, 'iface %s: invalid syntax \'%s\'' %(ifacename, l)) continue + ifaceobj.raw_config.append(l) attrname = attrs[0] - attrval = attrs[1].strip(' ') + # preprocess vars (XXX: only preprocesses $IFACE for now) + attrval = re.sub(r'\$IFACE', ifacename, attrs[1]) self._add_to_iface_config(ifacename, iface_config, attrname, attrval, line_idx+1) lines_consumed = line_idx - cur_idx @@ -254,7 +255,7 @@ class networkInterfaces(): if self.ignore_line(lines[line_idx]): line_idx += 1 continue - words = lines[line_idx].split() + words = re.split(self._ws_split_regex, lines[line_idx]) if not words: line_idx += 1 continue From 1104a8609efe3a30c3303c921669725d66fd6b3f Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Wed, 2 Jul 2014 10:03:02 -0700 Subject: [PATCH 04/11] exit with non-zero return code if any of the ifaces have errors Ticket: CM-2960 Reviewed By: Testing Done: ifupdown2 sanity + error cases + precommit uses a class variable to store return value --- pkg/scheduler.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/scheduler.py b/pkg/scheduler.py index 4ab7636..4f8e366 100644 --- a/pkg/scheduler.py +++ b/pkg/scheduler.py @@ -48,14 +48,15 @@ class ifaceScheduler(): _STATE_CHECK = True + _SCHED_RETVAL = True + @classmethod def run_iface_op(cls, ifupdownobj, ifaceobj, op, cenv=None): """ Runs sub operation on an interface """ ifacename = ifaceobj.name if (cls._STATE_CHECK and - (ifaceobj.state >= ifaceState.from_str(op)) and - (ifaceobj.status == ifaceStatus.SUCCESS)): + (ifaceobj.state >= ifaceState.from_str(op))): ifupdownobj.logger.debug('%s: already in state %s' %(ifacename, op)) return if not ifupdownobj.ADDONS_ENABLE: return @@ -84,6 +85,8 @@ class ifaceScheduler(): if err: ifaceobj.set_state_n_status(ifaceState.from_str(op), ifaceStatus.ERROR) + if 'up' in op or 'down' in op: + cls._SCHED_RETVAL = False else: ifaceobj.set_state_n_status(ifaceState.from_str(op), ifaceStatus.SUCCESS) @@ -423,3 +426,6 @@ class ifaceScheduler(): cls.run_iface_list_upper(ifupdownobj, ifacenames, ops, skip_root=True) cls._STATE_CHECK = True + + if not cls._SCHED_RETVAL: + raise Exception() From 0bb5110d654829d609d80c18f12a89e2280814fd Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Thu, 3 Jul 2014 14:43:26 -0700 Subject: [PATCH 05/11] Fix error flag for cases where errors are ignored (part of commit 3afb698d60a7ec0bb3029efccc82c0bd1aa49197) Ticket: CM-2960 Reviewed By: Testing Done: ifupdown2 sanity + precommit --- pkg/scheduler.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/scheduler.py b/pkg/scheduler.py index 4f8e366..e0c1079 100644 --- a/pkg/scheduler.py +++ b/pkg/scheduler.py @@ -81,6 +81,8 @@ class ifaceScheduler(): except Exception, e: err = 1 ifupdownobj.log_error(str(e)) + err = 0 # error can be ignored by log_error, in which case + # reset err flag finally: if err: ifaceobj.set_state_n_status(ifaceState.from_str(op), From 89aa6573d6e904d56a3e2c3dba22b6313f168ebf Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Mon, 7 Jul 2014 11:30:52 -0700 Subject: [PATCH 06/11] Fix use of args.CLASS with ifreload (reload does not support CLASS yet). Ticket: CM-3176 Reviewed By: trivial Testing Done: Tested ifreload with the testcase in the bug This broke when i recently fixed --allow-classes support for ifup/ifdown --- sbin/ifupdown | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/sbin/ifupdown b/sbin/ifupdown index 09038ec..91e2525 100755 --- a/sbin/ifupdown +++ b/sbin/ifupdown @@ -338,13 +338,18 @@ def validate_args(op, args): # return True if op == 'query' and args.syntaxhelp: return True - if not args.iflist and not args.all and not args.CLASS: + if op == 'reload': + if not args.all: + print '\'-a\' option is required' + return False + elif (not args.iflist and + not args.all and not args.CLASS): print '\'-a\' option or interface list are required' return False if args.iflist and args.all: print '\'-a\' option and interface list are mutually exclusive' return False - if args.CLASS and (args.all or args.iflist): + if op != 'reload' and args.CLASS and (args.all or args.iflist): print ('\'--allow\' option is mutually exclusive ' + 'with interface list and \'-a\'') return False From df4cba54e2284935494d9eb65c0a3af22c11b166 Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Tue, 8 Jul 2014 08:52:05 -0700 Subject: [PATCH 07/11] Bump kernel ethtool get/set wait to 20 + ifupdown2 convert ethtool errors to warns Ticket: CM-3159 Reviewed By: briefly ran this by jtoppins and andy (sfeldma is on vacation this week). Testing Done: tested ifupdown2 with ethtool config during boot (sam will also be adding the testcase mentioned in the bug to ifupdown2 smoke) The kernel timeout increase helps right now. we should revisit this again in 2.3 to close all corner cases. ifupdown2 will now warn on ethtool errors and will also return non-zero exit status --- pkg/scheduler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/scheduler.py b/pkg/scheduler.py index e0c1079..a98ee7e 100644 --- a/pkg/scheduler.py +++ b/pkg/scheduler.py @@ -84,7 +84,7 @@ class ifaceScheduler(): err = 0 # error can be ignored by log_error, in which case # reset err flag finally: - if err: + if err or ifaceobj.status == ifaceStatus.ERROR: ifaceobj.set_state_n_status(ifaceState.from_str(op), ifaceStatus.ERROR) if 'up' in op or 'down' in op: From a9ee5e18308094693b0f619772295d941216971f Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Wed, 9 Jul 2014 14:14:14 -0700 Subject: [PATCH 08/11] Add a new ifupdown2 example to cover bridge igmp and mstp attributes Ticket: CM-1438 Reviewed By: TBD Testing Done: Tested the example file with ifupdown2 --- docs/examples/interfaces_bridge_igmp_mstp | 45 +++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 docs/examples/interfaces_bridge_igmp_mstp diff --git a/docs/examples/interfaces_bridge_igmp_mstp b/docs/examples/interfaces_bridge_igmp_mstp new file mode 100644 index 0000000..5efb99c --- /dev/null +++ b/docs/examples/interfaces_bridge_igmp_mstp @@ -0,0 +1,45 @@ + +# +# This example lists all attributes for bridge configuration. +# The attributes with mstpctl- prefix indicate mstp settings on a bridge. +# The attributes with bridge- prefix indicate bridge stp and igmp attributes. +# Except bridge-ports, none of the other attributes are required. Default +# values are documented in the ifupdown-addons-interfaces(5) man page. +# +auto br-300 +iface br-300 inet static + address 12.0.0.3/24 + bridge-ports swp13.300 swp14.300 + bridge-stp on + mstpctl-maxage 20 + mstpctl-fdelay 15 + mstpctl-maxhops 20 + mstpctl-txholdcount 6 + mstpctl-forcevers rstp + mstpctl-portpathcost swp13.300=0 swp14.300=0 + mstpctl-portadminedge swp13.300=no swp14.300=no + mstpctl-portautoedge swp13.300=yes swp14.300=yes + mstpctl-portp2p swp13.300=no swp13.300=no + mstpctl-portrestrrole swp13.300=no swp14.300=no + mstpctl-bpduguard swp13.300=no swp14.300=no + mstpctl-portrestrtcn swp13.300=no swp14.300=no + mstpctl-treeprio 32768 + mstpctl-treeportprio swp13.300=128 + mstpctl-hello 2 + mstpctl-portnetwork swp13.300=no + bridge-mclmc 3 + bridge-mcrouter 0 + bridge-mcsnoop 1 + bridge-mcsqc 3 + bridge-mcqifaddr 1 + bridge-mcquerier 1 + bridge-hashel 3 + bridge-hashmax 4 + bridge-mclmi 3 + bridge-mcmi 200 + bridge-mcqpi 200 + bridge-mcqi 100 + bridge-mcqri 20 + bridge-mcsqi 50 + bridge-portmcrouter swp13.300=0 + bridge-portmcfl swp13.300=1 From 0c3c69e020145465c856c63017c585e8582aebef Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Wed, 9 Jul 2014 20:23:21 -0700 Subject: [PATCH 09/11] log /etc/init.d/networking errors into syslog using /usr/bin/logger Ticket: CM-3193 Reviewed By: Testing Done: Tested logging of errors in syslog from ifup at bootup Example error msg: "Jul 10 03:13:17 cumulus /etc/init.d/networking[1183]: error: /etc/network/interfaces: line16: invalid auto line 'auto'" This patch logs into syslog only during bootup --- init.d/networking | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/init.d/networking b/init.d/networking index b539ebd..df5e43f 100644 --- a/init.d/networking +++ b/init.d/networking @@ -13,6 +13,9 @@ PATH="/sbin:/bin" RUN_DIR="/run/network" IFSTATE="$RUN_DIR/ifstate" +NAME=networking +SCRIPTNAME=/etc/init.d/$NAME + [ -x /sbin/ifup ] || exit 0 [ -x /sbin/ifdown ] || exit 0 @@ -20,6 +23,7 @@ IFSTATE="$RUN_DIR/ifstate" CONFIGURE_INTERFACES=yes EXCLUDE_INTERFACES= +REMOTE_SYSLOG_SERVER= VERBOSE=no verbose= @@ -47,11 +51,20 @@ gen_examples() { return } +is_bootup() { + # Return 0 if its bootup or return 1 + [ -f /var/tmp/network/ifstatenew ] && return 1 + + return 0 +} + perf_options() { # At bootup lets set perfmode - [ -f /var/tmp/network/ifstatenew ] && echo -n "" && return - - echo -n "--perfmode" + if is_bootup ; then + echo -n "--perfmode" + else + echo -n "" + fi } process_exclusions() { @@ -139,12 +152,13 @@ start) exclusions=$(process_exclusions) perfoptions=$(perf_options) log_action_begin_msg "Configuring network interfaces" - if ifup -a $verbose $perfoptions - then - log_action_end_msg $? + if is_bootup ; then + ifup -a $verbose $perfoptions 2>&1 | /usr/bin/logger \ + $REMOTE_SYSLOG_SERVER -s -i -t $SCRIPTNAME else - log_action_end_msg $? + ifup -a $verbose $perfoptions fi + log_action_end_msg $? ;; stop) From 7ea57ea91fab498f827121c6364c0fc80a5e62ac Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Thu, 10 Jul 2014 21:08:21 -0700 Subject: [PATCH 10/11] Fix return value when upperifaces are brought up Ticket: CM-3208 Reviewed By: Testing Done: Tested with testcase listed in the bug This patch does the following: - moves the interface error exit check to before upperifaces are brought up - changes errors to warns on upperiface error (this is because upperiface 'up' is done as best effort to reconfigure the interface in question as slave device to the upper device. But if the upper device is not in a right state config steps can fail. And we should just warn). - Implicitly bringing up the upperifaces helps in most of the cases. especially when a bond is brought down and up. The upperiface handling code adds the bond back into bridges it was part of. or creates the vlan devices on the bond that got deleted. But there can be cases where upperifaces are not in the right state and this results in warnings. To disable the implicit upperiface handling, this patch also supports 'skip_upperifaces=1' in /etc/network/ifupdown2/ifupdown2.conf in future, i am thinking of an option --skip-upperifaces to ifup --- pkg/scheduler.py | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/pkg/scheduler.py b/pkg/scheduler.py index a98ee7e..99cc9a8 100644 --- a/pkg/scheduler.py +++ b/pkg/scheduler.py @@ -301,16 +301,10 @@ class ifaceScheduler(): 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('%s : (%s)' %(ifacename, str(e))) + if ifupdownobj.logger.isEnabledFor(logging.DEBUG): + traceback.print_tb(sys.exc_info()[2]) + ifupdownobj.logger.warn('%s : %s' %(ifacename, str(e))) + pass @classmethod def get_sorted_iface_list(cls, ifupdownobj, ifacenames, ops, @@ -418,16 +412,18 @@ class ifaceScheduler(): cls.run_iface_list(ifupdownobj, run_queue, ops, parent=None, order=order, followdependents=followdependents) - if (((not ifupdownobj.ALL and followdependents) or + if not cls._SCHED_RETVAL: + raise Exception() + + if (ifupdownobj.config.get('skip_upperifaces', '0') == '0' and + ((not ifupdownobj.ALL and followdependents) or followupperifaces) 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 ..') + ifupdownobj.logger.info('running upperifaces (parent interfaces) ' + + 'if available ..') cls._STATE_CHECK = False cls.run_iface_list_upper(ifupdownobj, ifacenames, ops, skip_root=True) cls._STATE_CHECK = True - - if not cls._SCHED_RETVAL: - raise Exception() From 2e87a5b01a55846587c96174fdd5ebb59ade7f95 Mon Sep 17 00:00:00 2001 From: Wilson Kok Date: Fri, 25 Jul 2014 10:33:44 -0700 Subject: [PATCH 11/11] Revert "log /etc/init.d/networking errors into syslog using /usr/bin/logger" This reverts commit 99d97bfcd931d40b84387f073a6c1b16866fc1e2. --- init.d/networking | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/init.d/networking b/init.d/networking index df5e43f..b539ebd 100644 --- a/init.d/networking +++ b/init.d/networking @@ -13,9 +13,6 @@ PATH="/sbin:/bin" RUN_DIR="/run/network" IFSTATE="$RUN_DIR/ifstate" -NAME=networking -SCRIPTNAME=/etc/init.d/$NAME - [ -x /sbin/ifup ] || exit 0 [ -x /sbin/ifdown ] || exit 0 @@ -23,7 +20,6 @@ SCRIPTNAME=/etc/init.d/$NAME CONFIGURE_INTERFACES=yes EXCLUDE_INTERFACES= -REMOTE_SYSLOG_SERVER= VERBOSE=no verbose= @@ -51,20 +47,11 @@ gen_examples() { return } -is_bootup() { - # Return 0 if its bootup or return 1 - [ -f /var/tmp/network/ifstatenew ] && return 1 - - return 0 -} - perf_options() { # At bootup lets set perfmode - if is_bootup ; then - echo -n "--perfmode" - else - echo -n "" - fi + [ -f /var/tmp/network/ifstatenew ] && echo -n "" && return + + echo -n "--perfmode" } process_exclusions() { @@ -152,13 +139,12 @@ start) exclusions=$(process_exclusions) perfoptions=$(perf_options) log_action_begin_msg "Configuring network interfaces" - if is_bootup ; then - ifup -a $verbose $perfoptions 2>&1 | /usr/bin/logger \ - $REMOTE_SYSLOG_SERVER -s -i -t $SCRIPTNAME + if ifup -a $verbose $perfoptions + then + log_action_end_msg $? else - ifup -a $verbose $perfoptions + log_action_end_msg $? fi - log_action_end_msg $? ;; stop)