From 707aeb737882cab19169b59ec85a7b2270ef09e4 Mon Sep 17 00:00:00 2001 From: Julien Fortin Date: Wed, 24 Aug 2016 14:20:45 -0700 Subject: [PATCH] netlink ip link set up/down may silently fail, adding try/except statements Ticket: CM-12609 Reviewed By: Roopa, Nikhil G Testing Done: ifupdown2 smoke tests Signed-off-by: Julien Fortin --- addons/address.py | 45 ++++++++++++++++++++-------------------- addons/bridge.py | 7 +++++-- addons/vlan.py | 5 ++++- ifupdown/ifupdownmain.py | 12 +++++++++-- 4 files changed, 41 insertions(+), 28 deletions(-) diff --git a/addons/address.py b/addons/address.py index 74d4301..0d342ef 100644 --- a/addons/address.py +++ b/addons/address.py @@ -338,34 +338,33 @@ class address(moduleBase): self.logger.error('%s: %s' % (ifaceobj.name, str(e))) ifaceobj.set_status(ifaceStatus.ERROR) - hwaddress = self._get_hwaddress(ifaceobj) - if hwaddress: - running_hwaddress = None - if not ifupdownflags.flags.PERFMODE: # system is clean - running_hwaddress = self.ipcmd.link_get_hwaddress(ifaceobj.name) - if hwaddress != running_hwaddress: - slave_down = False - netlink.link_set_updown(ifaceobj.name, "down") - if ifaceobj.link_kind & ifaceLinkKind.BOND: - # if bond, down all the slaves - if ifaceobj.lowerifaces: - for l in ifaceobj.lowerifaces: - netlink.link_set_updown(l, "down") - slave_down = True - try: - self.ipcmd.link_set(ifaceobj.name, 'address', hwaddress) - finally: - netlink.link_set_updown(ifaceobj.name, "up") - if slave_down: - for l in ifaceobj.lowerifaces: - netlink.link_set_updown(l, "up") - try: + hwaddress = self._get_hwaddress(ifaceobj) + if hwaddress: + running_hwaddress = None + if not ifupdownflags.flags.PERFMODE: # system is clean + running_hwaddress = self.ipcmd.link_get_hwaddress(ifaceobj.name) + if hwaddress != running_hwaddress: + slave_down = False + netlink.link_set_updown(ifaceobj.name, "down") + if ifaceobj.link_kind & ifaceLinkKind.BOND: + # if bond, down all the slaves + if ifaceobj.lowerifaces: + for l in ifaceobj.lowerifaces: + netlink.link_set_updown(l, "down") + slave_down = True + try: + self.ipcmd.link_set(ifaceobj.name, 'address', hwaddress) + finally: + netlink.link_set_updown(ifaceobj.name, "up") + if slave_down: + for l in ifaceobj.lowerifaces: + netlink.link_set_updown(l, "up") + # Handle special things on a bridge self._process_bridge(ifaceobj, True) except Exception, e: self.log_error('%s: %s' %(ifaceobj.name, str(e)), ifaceobj) - pass if addr_method != "dhcp": gateways = ifaceobj.get_attr_value('gateway') diff --git a/addons/bridge.py b/addons/bridge.py index c2b69a2..dc79fe3 100644 --- a/addons/bridge.py +++ b/addons/bridge.py @@ -1214,7 +1214,10 @@ class bridge(moduleBase): pass if ifaceobj.addr_method == 'manual': - netlink.link_set_updown(ifaceobj.name, "up") + try: + netlink.link_set_updown(ifaceobj.name, "up") + except Exception as e: + self.log_error('%s: %s' % (ifaceobj.name, str(e)), ifaceobj) if err: raise Exception(errstr) @@ -1229,7 +1232,7 @@ class bridge(moduleBase): map(lambda p: netlink.link_set_updown(p, "down"), ports) except Exception, e: - self.log_error(str(e)) + self.log_error('%s: %s' % (ifaceobj.name, str(e)), ifaceobj) def _query_running_vidinfo_compat(self, ifaceobjrunning, ports): running_attrs = {} diff --git a/addons/vlan.py b/addons/vlan.py index fff1d07..827047e 100644 --- a/addons/vlan.py +++ b/addons/vlan.py @@ -146,7 +146,10 @@ class vlan(moduleBase): netlink.link_add_vlan(vlanrawdevice, ifaceobj.name, vlanid) self._bridge_vid_add_del(ifaceobj, vlanrawdevice, vlanid) if ifaceobj.addr_method == 'manual': - netlink.link_set_updown(ifaceobj.name, "up") + try: + netlink.link_set_updown(ifaceobj.name, "up") + except Exception as e: + self.log_error('%s: %s' % (ifaceobj.name, str(e)), ifaceobj) def _down(self, ifaceobj): vlanid = self._get_vlan_id(ifaceobj) diff --git a/ifupdown/ifupdownmain.py b/ifupdown/ifupdownmain.py index 4d31cd9..f8d6cb8 100644 --- a/ifupdown/ifupdownmain.py +++ b/ifupdown/ifupdownmain.py @@ -129,7 +129,11 @@ class ifupdownMain(ifupdownBase): return if not self.link_exists(ifaceobj.name): return - self.link_up(ifaceobj.name) + try: + self.link_up(ifaceobj.name) + except Exception as e: + self.logger.error('%s: %s' % (ifaceobj.name, str(e))) + ifaceobj.set_status(ifaceStatus.ERROR) def run_down(self, ifaceobj): if ((ifaceobj.link_kind & ifaceLinkKind.VRF) or @@ -153,7 +157,11 @@ class ifupdownMain(ifupdownBase): return if not self.link_exists(ifaceobj.name): return - self.link_down(ifaceobj.name) + try: + self.link_down(ifaceobj.name) + except Exception as e: + self.logger.error('%s: %s' % (ifaceobj.name, str(e))) + ifaceobj.set_status(ifaceStatus.ERROR) # ifupdown object interface operation handlers ops_handlers = OrderedDict([('up', run_up),