From c523bc944d1ecffed2dcacffd6b5e48ece08722c Mon Sep 17 00:00:00 2001 From: Nikhil Date: Fri, 1 Apr 2016 21:49:00 -0700 Subject: [PATCH 01/11] Revert "addons: vrf: ifquery fixes for vrf" This reverts commit 934c4c49c0e77289e7d56349c44d14ca2c307621. Ticket: CM-10175 Reviewed By: Roopa Prabhu Testing Done: yes, by installing ifupdown .deb file onto dell-s3000-02 default addr fix for vrf check is deleting lo addrs accidentally info: rtnetlink: setting link lo up info: executing ip addr del ::1/128 dev lo info: executing ip addr del 127.0.0.1/8 dev lo info: eth0: running ops ... Signed-off-by: Nikhil --- addons/addressvirtual.py | 2 +- addons/vrf.py | 2 +- ifupdownaddons/iproute2.py | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/addons/addressvirtual.py b/addons/addressvirtual.py index bfe640c..45a582c 100644 --- a/addons/addressvirtual.py +++ b/addons/addressvirtual.py @@ -244,7 +244,7 @@ class addressvirtual(moduleBase): return True mac = mac.lower() try: - if int(mac.split(":")[0], 16) & 1 : + if int(mac.split(":")[0][1], 16) & 1 : self.logger.error("%s: Multicast bit is set in the virtual mac address '%s'" %(ifaceobj.name, mac)) return False return True diff --git a/addons/vrf.py b/addons/vrf.py index ff4a348..8d1b3cf 100644 --- a/addons/vrf.py +++ b/addons/vrf.py @@ -693,7 +693,7 @@ class vrf(moduleBase): try: master = self.ipcmd.link_get_master(ifaceobj.name) if not master or master != vrf: - ifaceobjcurr.update_config_with_status('vrf', str(master), 1) + ifaceobjcurr.update_config_with_status('vrf', master, 1) else: ifaceobjcurr.update_config_with_status('vrf', master, 0) except Exception, e: diff --git a/ifupdownaddons/iproute2.py b/ifupdownaddons/iproute2.py index add2782..15888a4 100644 --- a/ifupdownaddons/iproute2.py +++ b/ifupdownaddons/iproute2.py @@ -109,9 +109,9 @@ class iproute2(utilsBase): [linkCache.update_attrdict([ifname], linkattrs) for ifname, linkattrs in linkout.items()] - def _addr_filter(self, ifname, addr, scope=None): + def _addr_filter(self, addr, scope=None): default_addrs = ['127.0.0.1/8', '::1/128' , '0.0.0.0'] - if ifname == 'lo' and addr in default_addrs: + if addr in default_addrs: return True if scope and scope == 'link': return True @@ -151,14 +151,14 @@ class iproute2(utilsBase): except KeyError: linkout[ifname] = linkattrs if citems[2] == 'inet': - if self._addr_filter(citems[3], ifname, scope=citems[5]): + if self._addr_filter(citems[3], scope=citems[5]): continue addrattrs = {} addrattrs['scope'] = citems[5] addrattrs['type'] = 'inet' linkout[ifname]['addrs'][citems[3]] = addrattrs elif citems[2] == 'inet6': - if self._addr_filter(citems[3], ifname, scope=citems[5]): + if self._addr_filter(citems[3], scope=citems[5]): continue if citems[5] == 'link': continue #skip 'link' addresses addrattrs = {} From e8b2d486775072edcfb8f56259f35c94628314f8 Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Thu, 31 Mar 2016 16:09:37 -0700 Subject: [PATCH 02/11] addons: vrf: redo iproute2 vrf interface map handling Ticket: CM-10188, CM-10061 Reviewed By: dsa, nikhil, julien Testing Done: Tested static routes with vrf names for tables This patch does the following: - if a single vrf device is present in the config, builds the vrf map by reading vrf interfaces from the kernel (with existing link cache. Builds a shadow vrf only attribute cache) - reads existing table map and adjusts it if required - main change is the iproute2 map file on disk is updated immediately on vrf creation, so that static routes used along with the vrf slaves can use the vrf name for the table. This also helps dhclient dns hook script which may use mgmt table name directly. - cleans up default routes on down Signed-off-by: Roopa Prabhu --- addons/vrf.py | 185 ++++++++++++++++++++++++++----------- ifupdown/ifupdownmain.py | 2 + ifupdownaddons/cache.py | 2 + ifupdownaddons/iproute2.py | 19 +++- 4 files changed, 147 insertions(+), 61 deletions(-) diff --git a/addons/vrf.py b/addons/vrf.py index 8d1b3cf..338a06d 100644 --- a/addons/vrf.py +++ b/addons/vrf.py @@ -81,38 +81,10 @@ class vrf(moduleBase): #self.logger.info("vrf: ip -6 rule cache") #self.logger.info(self.ip6_rule_cache) - # XXX: check for vrf reserved overlap in /etc/iproute2/rt_tables + self._iproute2_vrf_map_initialized = False self.iproute2_vrf_map = {} - # read or create /etc/iproute2/rt_tables.d/ifupdown2.vrf_map - if os.path.exists(self.iproute2_vrf_filename): - self.vrf_map_fd = open(self.iproute2_vrf_filename, 'a+') - lines = self.vrf_map_fd.readlines() - for l in lines: - l = l.strip() - if l[0] == '#': - continue - try: - (table, vrf_name) = l.strip().split() - self.iproute2_vrf_map[table] = vrf_name - except Exception, e: - self.logger.info('vrf: iproute2_vrf_map: unable to parse %s' - %l) - pass - #self.logger.info("vrf: dumping iproute2_vrf_map") - #self.logger.info(self.iproute2_vrf_map) - - # purge vrf table entries that are not around - iproute2_vrf_map_pruned = {} - for t, v in self.iproute2_vrf_map.iteritems(): - if os.path.exists('/sys/class/net/%s' %v): - iproute2_vrf_map_pruned[int(t)] = v - else: - try: - # cleanup rules - self._del_vrf_rules(v, t) - except Exception: - pass - self.iproute2_vrf_map = iproute2_vrf_map_pruned + self.iproute2_vrf_map_fd = None + self.iproute2_vrf_map_sync_to_disk = False self.vrf_table_id_start = policymanager.policymanager_api.get_module_globals(module_name=self.__class__.__name__, attr='vrf-table-id-start') if not self.vrf_table_id_start: @@ -122,16 +94,6 @@ class vrf(moduleBase): self.vrf_table_id_end = self.VRF_TABLE_END self.vrf_max_count = policymanager.policymanager_api.get_module_globals(module_name=self.__class__.__name__, attr='vrf-max-count') - last_used_vrf_table = None - for t in range(self.vrf_table_id_start, - self.vrf_table_id_end): - if not self.iproute2_vrf_map.get(t): - break - last_used_vrf_table = t - self.last_used_vrf_table = last_used_vrf_table - - self.iproute2_write_vrf_map = False - atexit.register(self.iproute2_vrf_map_write) self.vrf_fix_local_table = True self.vrf_count = 0 self.vrf_cgroup_create = policymanager.policymanager_api.get_module_globals(module_name=self.__class__.__name__, attr='vrf-cgroup-create') @@ -141,19 +103,103 @@ class vrf(moduleBase): self.vrf_cgroup_create = True else: self.vrf_cgroup_create = False - self.vrf_mgmt_devname = policymanager.policymanager_api.get_module_globals(module_name=self.__class__.__name__, attr='vrf-mgmt-devname') - def iproute2_vrf_map_write(self): - if not self.iproute2_write_vrf_map: + def _iproute2_vrf_map_initialize(self): + if self._iproute2_vrf_map_initialized: return - self.logger.info('vrf: writing table map to %s' + + # XXX: check for vrf reserved overlap in /etc/iproute2/rt_tables + self.iproute2_vrf_map = {} + iproute2_vrf_map_force_rewrite = False + # read or create /etc/iproute2/rt_tables.d/ifupdown2.vrf_map + if os.path.exists(self.iproute2_vrf_filename): + vrf_map_fd = open(self.iproute2_vrf_filename, 'r+') + lines = vrf_map_fd.readlines() + for l in lines: + l = l.strip() + if l[0] == '#': + continue + try: + (table, vrf_name) = l.strip().split() + if self.iproute2_vrf_map.get(int(table)): + # looks like the existing file has + # duplicate entries, force rewrite of the + # file + iproute2_vrf_map_force_rewrite = True + continue + self.iproute2_vrf_map[int(table)] = vrf_name + except Exception, e: + self.logger.info('vrf: iproute2_vrf_map: unable to parse %s' + %l) + pass + + vrfs = self.ipcmd.link_get_vrfs() + running_vrf_map = {} + if vrfs: + for v, lattrs in vrfs.iteritems(): + table = lattrs.get('table', None) + if table: + running_vrf_map[int(table)] = v + + if running_vrf_map and (running_vrf_map != self.iproute2_vrf_map): + self.iproute2_vrf_map = running_vrf_map + iproute2_vrf_map_force_rewrite = True + + self.iproute2_vrf_map_fd = None + if iproute2_vrf_map_force_rewrite: + # reopen the file and rewrite the map + self._iproute2_vrf_map_open(True, False) + else: + self._iproute2_vrf_map_open(False, True) + + self.iproute2_vrf_map_sync_to_disk = False + atexit.register(self._iproute2_vrf_map_sync_to_disk) + + self.logger.info("vrf: dumping iproute2_vrf_map") + self.logger.info(self.iproute2_vrf_map) + + last_used_vrf_table = None + for t in range(self.vrf_table_id_start, + self.vrf_table_id_end): + if not self.iproute2_vrf_map.get(t): + break + last_used_vrf_table = t + self.last_used_vrf_table = last_used_vrf_table + self._iproute2_vrf_map_initialized = True + + def _iproute2_vrf_map_sync_to_disk(self): + if not self.iproute2_vrf_map_sync_to_disk: + return + self.logger.info('vrf: syncing table map to %s' %self.iproute2_vrf_filename) with open(self.iproute2_vrf_filename, 'w') as f: f.write(self.iproute2_vrf_filehdr %(self.vrf_table_id_start, self.vrf_table_id_end)) for t, v in self.iproute2_vrf_map.iteritems(): f.write('%s %s\n' %(t, v)) + f.flush() + + def _iproute2_vrf_map_open(self, sync_vrfs=False, append=False): + self.logger.info('vrf: syncing table map to %s' + %self.iproute2_vrf_filename) + fmode = 'a+' if append else 'w' + try: + self.iproute2_vrf_map_fd = open(self.iproute2_vrf_filename, + '%s' %fmode) + except Exception, e: + self.log_warn('vrf: error opening %s (%s)' + %(self.iproute2_vrf_filename, str(e))) + return + + if not append: + # write file header + self.iproute2_vrf_map_fd.write(self.iproute2_vrf_filehdr + %(self.vrf_table_id_start, + self.vrf_table_id_end)) + for t, v in self.iproute2_vrf_map.iteritems(): + self.iproute2_vrf_map_fd.write('%s %s\n' %(t, v)) + self.iproute2_vrf_map_fd.flush() def _is_vrf(self, ifaceobj): if ifaceobj.get_attr_value_first('vrf-table'): @@ -197,13 +243,19 @@ class vrf(moduleBase): return None def _iproute2_vrf_table_entry_add(self, vrf_dev_name, table_id): - self.iproute2_vrf_map[int(table_id)] = vrf_dev_name - self.iproute2_write_vrf_map = True + old_vrf_name = self.iproute2_vrf_map.get(int(table_id)) + if not old_vrf_name or (old_vrf_name != vrf_dev_name): + self.iproute2_vrf_map[int(table_id)] = vrf_dev_name + if self.iproute2_vrf_map_fd: + self.iproute2_vrf_map_fd.write('%s %s\n' + %(table_id, vrf_dev_name)) + self.iproute2_vrf_map_fd.flush() def _iproute2_vrf_table_entry_del(self, table_id): try: + # with any del of vrf map, we need to force sync to disk + self.iproute2_vrf_map_sync_to_disk = True del self.iproute2_vrf_map[int(table_id)] - self.iproute2_write_vrf_map = True except Exception, e: self.logger.info('vrf: iproute2 vrf map del failed for %d (%s)' %(table_id, str(e))) @@ -481,7 +533,7 @@ class vrf(moduleBase): return vrf_table - def _add_vrf_default_route(self, ifaceobj, vrf_table): + def _add_del_vrf_default_route(self, ifaceobj, vrf_table, add=True): vrf_default_route = ifaceobj.get_attr_value_first('vrf-default-route') if not vrf_default_route: vrf_default_route = policymanager.policymanager_api.get_attr_default( @@ -491,19 +543,33 @@ class vrf(moduleBase): return if str(vrf_default_route).lower() == "yes": try: - self.exec_command('ip route add table %s unreachable default' - ' metric %d' %(vrf_table, 240)) + if add: + self.exec_command('ip route add table %s unreachable ' + 'default metric %d' %(vrf_table, 240)) + else: + self.exec_command('ip route del table %s unreachable ' + 'default metric %d' %(vrf_table, 240)) except OSError, e: - if e.errno != 17: + if add and e.errno != 17: raise + else: + self.logger.info('%s: error deleting default route (%s)' + %(ifaceobj.name, str(e))) pass try: - self.exec_command('ip -6 route add table %s unreachable ' - 'default metric %d' %(vrf_table, 240)) + if add: + self.exec_command('ip -6 route add table %s unreachable ' + 'default metric %d' %(vrf_table, 240)) + else: + self.exec_command('ip -6 route del table %s unreachable ' + 'default metric %d' %(vrf_table, 240)) except OSError, e: - if e.errno != 17: + if add and e.errno != 17: raise + else: + self.logger.info('%s: error deleting default route (%s)' + %(ifaceobj.name, str(e))) pass def _up_vrf_dev(self, ifaceobj, vrf_table, add_slaves=True, @@ -521,7 +587,7 @@ class vrf(moduleBase): self._create_cgroup(ifaceobj) if add_slaves: self._add_vrf_slaves(ifaceobj, ifaceobj_getfunc) - self._add_vrf_default_route(ifaceobj, vrf_table) + self._add_del_vrf_default_route(ifaceobj, vrf_table) self._set_vrf_dev_processed_flag(ifaceobj) rtnetlink_api.rtnl_api.link_set(ifaceobj.name, "up") except Exception, e: @@ -596,6 +662,7 @@ class vrf(moduleBase): try: vrf_table = ifaceobj.get_attr_value_first('vrf-table') if vrf_table: + self._iproute2_vrf_map_initialize() # This is a vrf device if self.vrf_count == self.vrf_max_count: self.log_error('%s: max vrf count %d hit...not ' @@ -605,6 +672,7 @@ class vrf(moduleBase): else: vrf = ifaceobj.get_attr_value_first('vrf') if vrf: + self._iproute2_vrf_map_initialize() # This is a vrf slave self._up_vrf_slave(ifaceobj.name, vrf, ifaceobj, ifaceobj_getfunc) @@ -663,6 +731,7 @@ class vrf(moduleBase): try: self._iproute2_vrf_table_entry_del(vrf_table) + self._add_del_vrf_default_route(ifaceobj, vrf_table, False) self._delete_cgroup(ifaceobj) except Exception, e: self.logger.info('%s: %s' %(ifaceobj.name, str(e))) @@ -681,10 +750,12 @@ class vrf(moduleBase): try: vrf_table = ifaceobj.get_attr_value_first('vrf-table') if vrf_table: + self._iproute2_vrf_map_initialize() self._down_vrf_dev(ifaceobj, vrf_table, ifaceobj_getfunc) else: vrf = ifaceobj.get_attr_value_first('vrf') if vrf: + self._iproute2_vrf_map_initialize() self._down_vrf_slave(ifaceobj.name, ifaceobj, None) except Exception, e: self.log_warn(str(e)) @@ -729,10 +800,12 @@ class vrf(moduleBase): try: vrf_table = ifaceobj.get_attr_value_first('vrf-table') if vrf_table: + self._iproute2_vrf_map_initialize() self._query_check_vrf_dev(ifaceobj, ifaceobjcurr, vrf_table) else: vrf = ifaceobj.get_attr_value_first('vrf') if vrf: + self._iproute2_vrf_map_initialize() self._query_check_vrf_slave(ifaceobj, ifaceobjcurr, vrf) except Exception, e: self.log_warn(str(e)) diff --git a/ifupdown/ifupdownmain.py b/ifupdown/ifupdownmain.py index cb5ac7a..0f6a019 100644 --- a/ifupdown/ifupdownmain.py +++ b/ifupdown/ifupdownmain.py @@ -1440,6 +1440,8 @@ class ifupdownMain(ifupdownBase): else: # oldconfig not available, continue with 'up' with new config op = 'up' + new_ifaceobjdict = self.ifaceobjdict + new_dependency_graph = self.dependency_graph if op == 'reload' and ifacenames: ifacenames = self.ifaceobjdict.keys() diff --git a/ifupdownaddons/cache.py b/ifupdownaddons/cache.py index 61773b5..311082e 100644 --- a/ifupdownaddons/cache.py +++ b/ifupdownaddons/cache.py @@ -23,6 +23,8 @@ class linkCache(): : { } """ links = {} + vrfs = {} + @classmethod def get_attr(cls, mapList): return reduce(lambda d, k: d[k], mapList, linkCache.links) diff --git a/ifupdownaddons/iproute2.py b/ifupdownaddons/iproute2.py index 15888a4..fee21b4 100644 --- a/ifupdownaddons/iproute2.py +++ b/ifupdownaddons/iproute2.py @@ -24,10 +24,16 @@ class iproute2(utilsBase): def __init__(self, *args, **kargs): utilsBase.__init__(self, *args, **kargs) - if self.CACHE and not iproute2._cache_fill_done: + if self.CACHE: + self._fill_cache() + + def _fill_cache(self): + if not iproute2._cache_fill_done: self._link_fill() self._addr_fill() iproute2._cache_fill_done = True + return True + return False def _link_fill(self, ifacename=None, refresh=False): """ fills cache with link information @@ -99,6 +105,7 @@ class iproute2(utilsBase): vattrs = {'table' : citems[i+2]} linkattrs['linkinfo'] = vattrs linkattrs['kind'] = 'vrf' + linkCache.vrfs[ifname] = vattrs break elif citems[i] == 'vrf_slave': linkattrs['kind'] = 'vrf_slave' @@ -173,10 +180,8 @@ class iproute2(utilsBase): if self.DRYRUN: return False if self.CACHE: - if not iproute2._cache_fill_done: - self._link_fill() - self._addr_fill() - iproute2._cache_fill_done = True + if self._fill_cache(): + # if we filled the cache, return new data return linkCache.get_attr(attrlist) if not refresh: return linkCache.get_attr(attrlist) @@ -847,3 +852,7 @@ class iproute2(utilsBase): return os.path.basename(upper[0])[6:] except: return None + + def link_get_vrfs(self): + self._fill_cache() + return linkCache.vrfs From 37888438db54bfd62c2f831e0f5851ecbc79c90c Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Thu, 31 Mar 2016 21:39:55 -0700 Subject: [PATCH 03/11] addons: bridge: make sure bridge port enslavement is done by the bridge when it is non-vlan filtering bridge Ticket: CM-10083 Reviewed By: nikhil, julien Testing Done: Tested ifreload testcase with CM-10083 Without this patch a bridge port could enslave itself to the bridge when it finds that the bridge is around. This is a required feature for vlan filtering bridge because vlan filtering bridge port attributes are specified under the bridge port and the bridge port needs the power to enslave to the bridge and apply bridge port attrs. For the non-vlan filtering bridge, a few bridge port combinations are not allowed by default (eg, mixing different vlans under the same bridge). The bridge has the understanding of which ports are allowed. so only it should have the power to enslave bridge ports. This patch enforces that power. With this patch the sequence of deleting and enslaving bridge ports is done at the bridge with deletes followed by adds. example verbose snippent from ifreload output: ip -force -batch - [link set dev swp49s0 nomaster link set dev swp49s1 nomaster link set dev swp49s0.300 master Oldbr addr flush dev swp49s0.300 link set dev sidelink.300 master Oldbr addr flush dev sidelink.300 link set dev swp49s1.300 master Oldbr addr flush dev swp49s1.300 link set dev swp4 master Oldbr addr flush dev swp4] Signed-off-by: Roopa Prabhu --- addons/bridge.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/addons/bridge.py b/addons/bridge.py index 5ab2e31..3f152cd 100644 --- a/addons/bridge.py +++ b/addons/bridge.py @@ -988,15 +988,15 @@ class bridge(moduleBase): add_port = False bridgename = self.ipcmd.bridge_port_get_bridge_name(ifaceobj.name) if (not bridgename and - (ifaceobj.link_privflags & ifaceLinkPrivFlags.BRIDGE_PORT)): + (ifaceobj.link_privflags & ifaceLinkPrivFlags.BRIDGE_PORT)): # get bridgename and add port to bridge bridgename = self._get_bridgename(ifaceobj) add_port = True if bridgename: - if add_port: + if self.ipcmd.bridge_is_vlan_aware(bridgename): + if add_port: # add ifaceobj to bridge self.ipcmd.link_set(ifaceobj.name, 'master', bridgename) - if self.ipcmd.bridge_is_vlan_aware(bridgename): bridge_vids = self._get_bridge_vids(bridgename, ifaceobj_getfunc) bridge_pvid = self._get_bridge_pvid(bridgename, From 4489ae14eb0c8cb93c46f8b7e4ee1035ff7183fe Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Sun, 3 Apr 2016 10:29:33 -0700 Subject: [PATCH 04/11] debian: Add new SKIP_DOWN_AT_SYSRESET /etc/init.d/networking config option to skip deconfiguring interfaces Ticket: CM-5900 Reviewed By: CCR-2921 Testing Done: Tested 'service networking stop' during system reboot and shutdown this is cherry-pick of cumulus-3.2.y commit ac231e966a04eb78153d9b53f0d236a149c7bba5 --- debian/networking | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/debian/networking b/debian/networking index fe7c86d..5856199 100644 --- a/debian/networking +++ b/debian/networking @@ -15,3 +15,9 @@ SYSLOG="no" # Exclude interfaces EXCLUDE_INTERFACES= + +# Set to 'yes' if you want to skip ifdown during system reboot +# and shutdown. This is of interest in large scale interface +# deployments where you dont want to wait for interface +# deconfiguration to speed up shutdown/reboot +SKIP_DOWN_AT_SYSRESET="yes" From 1241e3e7325da63d424879398737a94a14294298 Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Sun, 3 Apr 2016 15:43:07 -0700 Subject: [PATCH 05/11] ifupdownaddons: iproute2: add optional refresh flag to addr get api Ticket: Reviewed By: trivial Testing Done: tested address gets with refresh flag + ran ssim test testifupdown2.py - also fixes refresh flag handling in cache _addr_fill function - this api can be used to get addresses from the cache by first refreshing the cache. So the caller gets fresh running addresses. (its not used right now but came up during the need to re-apply addresses for ipv6 vrf slaves since their addresses may disappear) --- ifupdownaddons/iproute2.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ifupdownaddons/iproute2.py b/ifupdownaddons/iproute2.py index fee21b4..48a6752 100644 --- a/ifupdownaddons/iproute2.py +++ b/ifupdownaddons/iproute2.py @@ -132,10 +132,10 @@ class iproute2(utilsBase): """ linkout = {} - if iproute2._cache_fill_done: return + if iproute2._cache_fill_done and not refresh: return try: # Check if ifacename is already full, in which case, return - if ifacename: + if ifacename and not refresh: linkCache.get_attr([ifacename, 'addrs']) return except: @@ -331,8 +331,9 @@ class iproute2(utilsBase): # ignore errors pass - def addr_get(self, ifacename, details=True): - addrs = self._cache_get('addr', [ifacename, 'addrs']) + def addr_get(self, ifacename, details=True, refresh=False): + addrs = self._cache_get('addr', [ifacename, 'addrs'], + refresh=refresh) if not addrs: return None if details: From 014e8c949744bad0549056e2b1f4186c5f42c64f Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Sun, 3 Apr 2016 15:31:34 -0700 Subject: [PATCH 06/11] ifupdownmain: bridgevlan: dont squash vlan iface_types with ifaces of the same name Ticket: CM-10051 Reviewed By: julien, nikhil Testing Done: tested that bridge vlan attributes get applied correctly + ran ssim test testifupdown2.py This was introduced by a patch that squashed multiple iface objects into a single object. That led to the below interfaces getting squashed into one. Which is not the right thing to do: {noformat} auto Newbr.325 iface Newbr.325 address 24.0.0.22/30 address 3101:abc:bcad:1::3/64 auto Newbr.325 vlan Newbr.325 bridge-igmp-querier-src 194.31.10.45 {noformat} The 'vlan Newbr.325' ifaceobject needs to be kept separately and it is of type BRIDGE_VLAN. so, this patch just makes sure these interfaces are kept separately in the squash function. Signed-off-by: Roopa Prabhu --- ifupdown/ifupdownmain.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ifupdown/ifupdownmain.py b/ifupdown/ifupdownmain.py index 0f6a019..d069f8e 100644 --- a/ifupdown/ifupdownmain.py +++ b/ifupdown/ifupdownmain.py @@ -696,7 +696,10 @@ class ifupdownMain(ifupdownBase): if ifaceobj.compare(currentifaceobjlist[0]): self.logger.warn('duplicate interface %s found' %ifaceobj.name) return - currentifaceobjlist[0].squash(ifaceobj) + if ifaceobj.type == ifaceType.BRIDGE_VLAN: + self.ifaceobjdict[ifaceobj.name].append(ifaceobj) + else: + currentifaceobjlist[0].squash(ifaceobj) def _save_iface(self, ifaceobj): if self._check_config_no_repeats(ifaceobj): From c18b8f0c1765a6262234971a734c2aee07e48acc Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Sun, 3 Apr 2016 19:35:51 -0700 Subject: [PATCH 07/11] ifupdown2: rtnetlink_api: fix api log message to contain commands in iproute2 format Ticket: trivial Reviewed By: wkok Testing Done: Tested ifupdown2 sanity and checked log messages from rtnetlink api Signed-off-by: Roopa Prabhu This is cherry-pick of commit 269ff43d09ca4 from cumulus2.5 --- ifupdown/rtnetlink_api.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ifupdown/rtnetlink_api.py b/ifupdown/rtnetlink_api.py index af89a29..390a642 100644 --- a/ifupdown/rtnetlink_api.py +++ b/ifupdown/rtnetlink_api.py @@ -41,7 +41,7 @@ class rtnetlinkApi(RtNetlink): return ifindex def create_vlan(self, link, ifname, vlanid): - self.logger.info('rtnetlink: creating vlan interface %s' %ifname) + self.logger.info('rtnetlink: ip link add link %s name %s type vlan id %s' %(link, ifname, vlanid)) if ifupdownmain.ifupdownFlags.DRYRUN: return try: @@ -65,7 +65,7 @@ class rtnetlinkApi(RtNetlink): self.process_wait([token]) def create_macvlan(self, ifname, link, mode='private'): - self.logger.info('rtnetlink: creating macvlan interface %s' %ifname) + self.logger.info('rtnetlink: ip link add link %s name %s type macvlan mode private' %(link, ifname)) if ifupdownmain.ifupdownFlags.DRYRUN: return try: @@ -90,7 +90,7 @@ class rtnetlinkApi(RtNetlink): def link_set(self, ifname, state): flags = 0 - self.logger.info('rtnetlink: setting link %s %s' %(ifname, state)) + self.logger.info('rtnetlink: ip link set dev %s %s' %(ifname, state)) if ifupdownmain.ifupdownFlags.DRYRUN: return @@ -122,7 +122,7 @@ class rtnetlinkApi(RtNetlink): def link_set_hwaddress(self, ifname, hwaddress): flags = 0 - self.logger.info('rtnetlink: setting link hwaddress %s %s' %(ifname, hwaddress)) + self.logger.info('rtnetlink: ip link set dev %s address %s' %(ifname, hwaddress)) if ifupdownmain.ifupdownFlags.DRYRUN: return From 675ae21a0e0509744ea5a4e4988cd9dda10aa8a5 Mon Sep 17 00:00:00 2001 From: Nikhil Date: Wed, 30 Mar 2016 16:56:37 -0700 Subject: [PATCH 08/11] addons: vrf: ifquery fixes for vrf Ticket: CM-10175 Reviewed By: Roopa Prabhu Testing Done: yes, by installing ifupdown .deb onto dell-s3000-02 This patch fixes inappropriate ifquery fails. This patch also include a review comment update for addressvirtual.py [CCR-4310], ticket: [CM-8658] Signed-off-by: Nikhil --- addons/addressvirtual.py | 2 +- addons/vrf.py | 2 +- ifupdownaddons/iproute2.py | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/addons/addressvirtual.py b/addons/addressvirtual.py index 45a582c..bfe640c 100644 --- a/addons/addressvirtual.py +++ b/addons/addressvirtual.py @@ -244,7 +244,7 @@ class addressvirtual(moduleBase): return True mac = mac.lower() try: - if int(mac.split(":")[0][1], 16) & 1 : + if int(mac.split(":")[0], 16) & 1 : self.logger.error("%s: Multicast bit is set in the virtual mac address '%s'" %(ifaceobj.name, mac)) return False return True diff --git a/addons/vrf.py b/addons/vrf.py index 338a06d..bb25427 100644 --- a/addons/vrf.py +++ b/addons/vrf.py @@ -764,7 +764,7 @@ class vrf(moduleBase): try: master = self.ipcmd.link_get_master(ifaceobj.name) if not master or master != vrf: - ifaceobjcurr.update_config_with_status('vrf', master, 1) + ifaceobjcurr.update_config_with_status('vrf', str(master), 1) else: ifaceobjcurr.update_config_with_status('vrf', master, 0) except Exception, e: diff --git a/ifupdownaddons/iproute2.py b/ifupdownaddons/iproute2.py index 48a6752..28014f3 100644 --- a/ifupdownaddons/iproute2.py +++ b/ifupdownaddons/iproute2.py @@ -116,9 +116,9 @@ class iproute2(utilsBase): [linkCache.update_attrdict([ifname], linkattrs) for ifname, linkattrs in linkout.items()] - def _addr_filter(self, addr, scope=None): + def _addr_filter(self, ifname, addr, scope=None): default_addrs = ['127.0.0.1/8', '::1/128' , '0.0.0.0'] - if addr in default_addrs: + if ifname == 'lo' and addr in default_addrs: return True if scope and scope == 'link': return True @@ -158,14 +158,14 @@ class iproute2(utilsBase): except KeyError: linkout[ifname] = linkattrs if citems[2] == 'inet': - if self._addr_filter(citems[3], scope=citems[5]): + if self._addr_filter(citems[3], ifname, scope=citems[5]): continue addrattrs = {} addrattrs['scope'] = citems[5] addrattrs['type'] = 'inet' linkout[ifname]['addrs'][citems[3]] = addrattrs elif citems[2] == 'inet6': - if self._addr_filter(citems[3], scope=citems[5]): + if self._addr_filter(citems[3], ifname, scope=citems[5]): continue if citems[5] == 'link': continue #skip 'link' addresses addrattrs = {} From 7ed24e0f2aaa6c7293bb21f27eeec3d36b4a26a8 Mon Sep 17 00:00:00 2001 From: Nikhil Date: Mon, 4 Apr 2016 02:15:56 -0700 Subject: [PATCH 09/11] addons: vrf: ifquery fixes for vrf Ticket: CM-10175 Reviewed By: Roopa Prabhu Testing Done: yes, by installing ifupdown .deb onto dell-s3000-02 This patch is a git revert of commit 934c4c49c0e77289e7d56349c44d14ca2c307621. In addition to that, order of _addr_filter() function call arguments are changed to match the function definition. Signed-off-by: Nikhil --- ifupdownaddons/iproute2.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ifupdownaddons/iproute2.py b/ifupdownaddons/iproute2.py index 28014f3..c0711a7 100644 --- a/ifupdownaddons/iproute2.py +++ b/ifupdownaddons/iproute2.py @@ -158,14 +158,14 @@ class iproute2(utilsBase): except KeyError: linkout[ifname] = linkattrs if citems[2] == 'inet': - if self._addr_filter(citems[3], ifname, scope=citems[5]): + if self._addr_filter(ifname, citems[3], scope=citems[5]): continue addrattrs = {} addrattrs['scope'] = citems[5] addrattrs['type'] = 'inet' linkout[ifname]['addrs'][citems[3]] = addrattrs elif citems[2] == 'inet6': - if self._addr_filter(citems[3], ifname, scope=citems[5]): + if self._addr_filter(ifname, citems[3], scope=citems[5]): continue if citems[5] == 'link': continue #skip 'link' addresses addrattrs = {} From 9057588f8f084bddaa6e1e6d400c9186c32e27e7 Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Mon, 4 Apr 2016 13:24:59 -0700 Subject: [PATCH 10/11] ifupdownmain: check upperfaceobj role before printing the warning Ticket: found during other testing Reviewed By: trivial Testing Done: Tested ifreload with dependency errors and dependencies with upperiface being a master and also vlan device This patch avoids false positives when set role is called twice for an interface which is a vrf slave but also has a vlan device on top of it. --- addons/vrf.py | 1 + ifupdown/ifupdownmain.py | 11 ++++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/addons/vrf.py b/addons/vrf.py index bb25427..3639051 100644 --- a/addons/vrf.py +++ b/addons/vrf.py @@ -213,6 +213,7 @@ class vrf(moduleBase): if vrf_table: ifaceobj.link_type = ifaceLinkType.LINK_MASTER ifaceobj.link_kind |= ifaceLinkKind.VRF + ifaceobj.role |= ifaceRole.MASTER vrf_iface_name = ifaceobj.get_attr_value_first('vrf') if not vrf_iface_name: return None diff --git a/ifupdown/ifupdownmain.py b/ifupdown/ifupdownmain.py index d069f8e..c6717b7 100644 --- a/ifupdown/ifupdownmain.py +++ b/ifupdown/ifupdownmain.py @@ -439,9 +439,10 @@ class ifupdownMain(ifupdownBase): %(ifaceobj.name, ifacename) + 'seem to share dependents/ports %s' %str(list(common))) - def _set_iface_role(self, ifaceobj, role): + def _set_iface_role(self, ifaceobj, role, upperifaceobj): if (self.flags.CHECK_SHARED_DEPENDENTS and - (ifaceobj.role & ifaceRole.SLAVE) and role == ifaceRole.SLAVE): + (ifaceobj.role & ifaceRole.SLAVE) and + (role == ifaceRole.SLAVE) and (upperifaceobj.role == ifaceRole.MASTER)): self.logger.error("misconfig..? %s %s is enslaved to multiple interfaces %s" %(ifaceobj.name, ifaceLinkPrivFlags.get_all_str(ifaceobj.link_privflags), str(ifaceobj.upperifaces))) @@ -452,18 +453,18 @@ class ifupdownMain(ifupdownBase): def _set_iface_role_n_kind(self, ifaceobj, upperifaceobj): if (upperifaceobj.link_kind & ifaceLinkKind.BOND): - self._set_iface_role(ifaceobj, ifaceRole.SLAVE) + self._set_iface_role(ifaceobj, ifaceRole.SLAVE, upperifaceobj) ifaceobj.link_privflags |= ifaceLinkPrivFlags.BOND_SLAVE if (upperifaceobj.link_kind & ifaceLinkKind.BRIDGE): - self._set_iface_role(ifaceobj, ifaceRole.SLAVE) + self._set_iface_role(ifaceobj, ifaceRole.SLAVE, upperifaceobj) ifaceobj.link_privflags |= ifaceLinkPrivFlags.BRIDGE_PORT # vrf masters get processed after slaves, which means # check both link_kind vrf and vrf slave if ((upperifaceobj.link_kind & ifaceLinkKind.VRF) or (ifaceobj.link_privflags & ifaceLinkPrivFlags.VRF_SLAVE)): - self._set_iface_role(ifaceobj, ifaceRole.SLAVE) + self._set_iface_role(ifaceobj, ifaceRole.SLAVE, upperifaceobj) ifaceobj.link_privflags |= ifaceLinkPrivFlags.VRF_SLAVE if self._link_master_slave: if upperifaceobj.link_type == ifaceLinkType.LINK_MASTER: From 0309264919237d263be903594cfefbb0f327e0aa Mon Sep 17 00:00:00 2001 From: Julien Fortin Date: Tue, 13 Sep 2016 17:03:24 -0700 Subject: [PATCH 11/11] addons: address: ignoring iproute2 'rtnetlink file exists' errors Ticket: CM-12798 Reviewed By: Roopa Testing Done: A customer discovered a corner case: the kernel is shrinking/reducing the ip address fields containing 0s. For example if we configure and address such as 2a01:75e0:0000:09b0::1/64 Then if we query the kernel (using iproute2 or netlink) it will come back as: 2a01:75e0:0:09b0::1/64 Because of this issue we were seeing root@r4u28:~# ifreload -a error: bridge.200: cmd 'ip -force -batch - [addr add 10.50.103.193/26 dev bridge.200 addr add 2a01:75e0:0000:09b1::1/64 dev bridge.200 ]' failed: returned 1 (RTNETLINK answers: File exists Command failed -:1 ) Before adding an address to an iface we query it to know if the change is necessary, since we only do a string comparaison between: 2a01:75e0:0000:09b1::1/64 and 2a01:75e0:0:09b1::1/64 it fails, ifupdown2 thinks that a change is needed. So we try to add the new address via iproute2. iproute2 will fail because this address in a "shrinked" format already exists. This patch hot-fixes this problem by ignoring the error if it's an "exists" error, we don't want to confuse the user. A later fix will the real issue by normalizing all ip addr to the IPNetwork format. Signed-off-by: Julien Fortin --- addons/address.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/addons/address.py b/addons/address.py index 74d4301..90117ba 100644 --- a/addons/address.py +++ b/addons/address.py @@ -335,8 +335,7 @@ class address(moduleBase): try: self.ipcmd.batch_commit() except Exception as e: - self.logger.error('%s: %s' % (ifaceobj.name, str(e))) - ifaceobj.set_status(ifaceStatus.ERROR) + self.log_error('%s: %s' % (ifaceobj.name, str(e)), ifaceobj, raise_error=False) hwaddress = self._get_hwaddress(ifaceobj) if hwaddress: