From 9f30b2cca7b62e6d85edd53c10785f779679bbaf Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Mon, 7 Nov 2016 10:48:24 -0800 Subject: [PATCH] addons: address: various fixes for mtu handling Ticket: CM-6908, CM-6110, CM-13221 Reviewed By: julien, nikhil Testing Done: added a new test in ifupdown2-tests which covers all cases - move all mtu handling to a single function in addons/address.py - Have an ifupdown2 default of 1500 mtu - add a policy manager max_mtu check (we want to default cumulus max mtu to 9216) - special handling for bond and bridges - print an info log abt setting mtu on bridge - this can be enhanced in the future to look at individual port mtu and rejecting the bridge mtu. this operation can be expensive right now. Hence just an info log. - bond and vxlan dev mtu follow the rules of physical device mtu Signed-off-by: Roopa Prabhu --- addons/address.py | 79 ++++++++++++++++++++++++++++++-------- addons/vlan.py | 4 -- ifupdownaddons/iproute2.py | 4 ++ 3 files changed, 66 insertions(+), 21 deletions(-) diff --git a/addons/address.py b/addons/address.py index 900aa41..da6e83f 100644 --- a/addons/address.py +++ b/addons/address.py @@ -88,6 +88,15 @@ class address(moduleBase): self.ipcmd = None self._bridge_fdb_query_cache = {} self.default_mtu = policymanager.policymanager_api.get_attr_default(module_name=self.__class__.__name__, attr='mtu') + self.max_mtu = policymanager.policymanager_api.get_module_globals(module_name=self.__class__.__name__, attr='max_mtu') + + if not self.default_mtu: + self.default_mtu = '1500' + + self.logger.info('address: using default mtu %s' %self.default_mtu) + + if self.max_mtu: + self.logger.info('address: using max mtu %s' %self.max_mtu) def _address_valid(self, addrs): if not addrs: @@ -286,6 +295,58 @@ class address(moduleBase): return ipv return prev_gateways + def _process_mtu_config(self, ifaceobj, ifaceobj_getfunc): + mtu = ifaceobj.get_attr_value_first('mtu') + if mtu: + if (ifaceobj.link_kind & ifaceLinkKind.BRIDGE): + self.logger.info('%s: bridge inherits mtu from its ports. There is no need to assign mtu on a bridge' %ifaceobj.name) + elif (ifaceobj_getfunc and + (ifaceobj.link_privflags & ifaceLinkPrivFlags.BOND_SLAVE) and + ifaceobj.upperifaces): + masterobj = ifaceobj_getfunc(ifaceobj.upperifaces[0]) + if masterobj: + master_mtu = masterobj[0].get_attr_value_first('mtu') + if master_mtu and master_mtu != mtu: + self.logger.info('%s: bond slave mtu %s is different from bond master mtu %s. There is no need to configure mtu on a bond slave.' %(ifaceobj.name, mtu, master_mtu)) + return + if self.max_mtu and mtu > self.max_mtu: + self.logger.warn('%s: specified mtu %s is greater than max mtu %s' + %(ifaceobj.name, mtu, self.max_mtu)) + self.ipcmd.link_set(ifaceobj.name, 'mtu', mtu) + return + + if ifaceobj.link_kind: + # bonds and vxlan devices need an explicit set of mtu. + # bridges don't need mtu set + if (ifaceobj.link_kind & ifaceLinkKind.BOND or + ifaceobj.link_kind & ifaceLinkKind.VXLAN): + running_mtu = self.ipcmd.link_get_mtu(ifaceobj.name) + if (self.default_mtu and running_mtu != self.default_mtu): + self.ipcmd.link_set(ifaceobj.name, 'mtu', self.default_mtu) + return + if (ifupdownConfig.config.get('adjust_logical_dev_mtu', '1') != '0' + and ifaceobj.lowerifaces): + # set vlan interface mtu to lower device mtu + if (ifaceobj.link_kind & ifaceLinkKind.VLAN): + lower_iface_mtu = self.ipcmd.link_get_mtu(ifaceobj.lowerifaces[0], refresh=True) + if not lower_iface_mtu == self.ipcmd.link_get_mtu(ifaceobj.name): + self.ipcmd.link_set_mtu(ifaceobj.name, lower_iface_mtu) + + elif (not (ifaceobj.name == 'lo') and not ifaceobj.link_kind and + not (ifaceobj.link_privflags & ifaceLinkPrivFlags.BOND_SLAVE) and + self.default_mtu): + # logical devices like bridges and vlan devices rely on mtu + # from their lower devices. ie mtu travels from + # lower devices to upper devices. For bonds mtu travels from + # upper to lower devices. running mtu depends on upper and + # lower device mtu. With all this implicit mtu + # config by the kernel in play, we try to be cautious here + # on which devices we want to reset mtu to default. + # essentially only physical interfaces which are not bond slaves + running_mtu = self.ipcmd.link_get_mtu(ifaceobj.name) + if running_mtu != self.default_mtu: + self.ipcmd.link_set(ifaceobj.name, 'mtu', self.default_mtu) + def _up(self, ifaceobj, ifaceobj_getfunc=None): if not self.ipcmd.link_exists(ifaceobj.name): return @@ -313,23 +374,7 @@ class address(moduleBase): if addr_method != "dhcp": self._inet_address_config(ifaceobj, ifaceobj_getfunc, force_reapply) - mtu = ifaceobj.get_attr_value_first('mtu') - if mtu: - self.ipcmd.link_set(ifaceobj.name, 'mtu', mtu) - elif (not (ifaceobj.name == 'lo') and not ifaceobj.link_kind and - not (ifaceobj.link_privflags & ifaceLinkPrivFlags.BOND_SLAVE) and - self.default_mtu): - # logical devices like bridges and vlan devices rely on mtu - # from their lower devices. ie mtu travels from - # lower devices to upper devices. For bonds mtu travels from - # upper to lower devices. running mtu depends on upper and - # lower device mtu. With all this implicit mtu - # config by the kernel in play, we try to be cautious here - # on which devices we want to reset mtu to default. - # essentially only physical interfaces which are not bond slaves - running_mtu = self.ipcmd.link_get_mtu(ifaceobj.name) - if running_mtu != self.default_mtu: - self.ipcmd.link_set(ifaceobj.name, 'mtu', self.default_mtu) + self._process_mtu_config(ifaceobj, ifaceobj_getfunc) alias = ifaceobj.get_attr_value_first('alias') if alias: diff --git a/addons/vlan.py b/addons/vlan.py index 827047e..ad6dfaf 100644 --- a/addons/vlan.py +++ b/addons/vlan.py @@ -138,10 +138,6 @@ class vlan(moduleBase): raise Exception('rawdevice %s not present' %vlanrawdevice) if self.ipcmd.link_exists(ifaceobj.name): self._bridge_vid_add_del(ifaceobj, vlanrawdevice, vlanid) - if ifupdownConfig.config.get('adjust_logical_dev_mtu', '1') != '0' and len(ifaceobj.lowerifaces): - lower_iface_mtu = self.ipcmd.link_get_mtu(ifaceobj.lowerifaces[0], refresh=True) - if not lower_iface_mtu == self.ipcmd.link_get_mtu(ifaceobj.name): - self.ipcmd.link_set_mtu(ifaceobj.name, lower_iface_mtu) return netlink.link_add_vlan(vlanrawdevice, ifaceobj.name, vlanid) self._bridge_vid_add_del(ifaceobj, vlanrawdevice, vlanid) diff --git a/ifupdownaddons/iproute2.py b/ifupdownaddons/iproute2.py index dddbb41..974ea25 100644 --- a/ifupdownaddons/iproute2.py +++ b/ifupdownaddons/iproute2.py @@ -663,6 +663,10 @@ class iproute2(utilsBase): def link_get_mtu(self, ifacename, refresh=False): return self._cache_get('link', [ifacename, 'mtu'], refresh=refresh) + def link_get_mtu_sysfs(self, ifacename): + return self.read_file_oneline('/sys/class/net/%s/mtu' + %ifacename) + def link_get_kind(self, ifacename): return self._cache_get('link', [ifacename, 'kind'])