From 06ae82ea0b9b4cd06758820cf379783e93d0bc1f Mon Sep 17 00:00:00 2001 From: Christoph Heiss Date: Wed, 27 Aug 2025 11:30:33 +0200 Subject: [PATCH 1/2] addons, nlcache: set interface mtu through netlink instead of sysfs The sysfs-based methods do not handle altnames at all. Instead, get/set interface MTUs through netlink directly, which also has transparent altname support, and is a lot more robust way in general. At the same time, this simplifies some things as it means that the MTU will always be present in the netlink cache. Along the way also clean up some weird mtu_{int,str}, where the MTU is passed around in the `address` as both string and integer. Signed-off-by: Christoph Heiss --- ifupdown2/addons/address.py | 71 +++++++++++++++++++----------- ifupdown2/addons/addressvirtual.py | 5 +-- ifupdown2/addons/bridge.py | 2 +- ifupdown2/lib/iproute2.py | 5 +-- ifupdown2/lib/nlcache.py | 34 ++++++++++++++ ifupdown2/lib/sysfs.py | 16 ------- 6 files changed, 83 insertions(+), 50 deletions(-) diff --git a/ifupdown2/addons/address.py b/ifupdown2/addons/address.py index 3d961032..bbc90d8d 100644 --- a/ifupdown2/addons/address.py +++ b/ifupdown2/addons/address.py @@ -412,7 +412,7 @@ def syntax_check_mtu(self, ifaceobj, ifaceobj_getfunc): except ValueError as e: self.logger.warning("%s: invalid mtu %s: %s" % (ifaceobj.name, mtu_str, str(e))) return False - return self._check_mtu_config(ifaceobj, mtu_str, mtu_int, ifaceobj_getfunc, syntaxcheck=True) + return self._check_mtu_config(ifaceobj, mtu_int, ifaceobj_getfunc, syntaxcheck=True) return True def syntax_check_addr_allowed_on(self, ifaceobj, syntax_check=False): @@ -790,7 +790,14 @@ def _get_prev_gateway(self, ifaceobj, gateways): return ipv return prev_gateways - def _check_mtu_config(self, ifaceobj, mtu_str, mtu_int, ifaceobj_getfunc, syntaxcheck=False): + def _check_mtu_config(self, ifaceobj, mtu, ifaceobj_getfunc, syntaxcheck=False): + """ + :param ifaceobj: + :param mtu: integer + :param ifaceobj_getfunc: + :param syntaxcheck: boolean + """ + retval = True if (ifaceobj.link_kind & ifaceLinkKind.BRIDGE): if syntaxcheck: @@ -804,10 +811,10 @@ def _check_mtu_config(self, ifaceobj, mtu_str, mtu_int, ifaceobj_getfunc, syntax masterobj = ifaceobj_getfunc(ifaceobj.upperifaces[0]) if masterobj: master_mtu = masterobj[0].get_attr_value_first('mtu') - if master_mtu and master_mtu != mtu_str: + if master_mtu and master_mtu != str(mtu): log_msg = ("%s: bond slave mtu %s is different from bond master %s mtu %s. " "There is no need to configure mtu on a bond slave." % - (ifaceobj.name, mtu_str, masterobj[0].name, master_mtu)) + (ifaceobj.name, mtu, masterobj[0].name, master_mtu)) if syntaxcheck: self.logger.warning(log_msg) retval = False @@ -821,24 +828,30 @@ def _check_mtu_config(self, ifaceobj, mtu_str, mtu_int, ifaceobj_getfunc, syntax lowerdev_mtu = int(lowerobj[0].get_attr_value_first('mtu') or 0) else: lowerdev_mtu = self.cache.get_link_mtu(lowerobj[0].name) # return type: int - if lowerdev_mtu and mtu_int > lowerdev_mtu: + if lowerdev_mtu and mtu > lowerdev_mtu: self.logger.warning('%s: vlan dev mtu %s is greater than lower realdev %s mtu %s' - %(ifaceobj.name, mtu_str, lowerobj[0].name, lowerdev_mtu)) + %(ifaceobj.name, mtu, lowerobj[0].name, lowerdev_mtu)) retval = False elif (not lowerobj[0].link_kind and not (lowerobj[0].link_privflags & ifaceLinkPrivFlags.LOOPBACK) and - not lowerdev_mtu and self.default_mtu and (mtu_int > self.default_mtu_int)): + not lowerdev_mtu and self.default_mtu and (mtu > self.default_mtu_int)): # only check default mtu on lower device which is a physical interface self.logger.warning('%s: vlan dev mtu %s is greater than lower realdev %s mtu %s' - %(ifaceobj.name, mtu_str, lowerobj[0].name, self.default_mtu)) + %(ifaceobj.name, mtu, lowerobj[0].name, self.default_mtu)) retval = False - if self.max_mtu and mtu_int > self.max_mtu: + if self.max_mtu and mtu > self.max_mtu: self.logger.warning('%s: specified mtu %s is greater than max mtu %s' - %(ifaceobj.name, mtu_str, self.max_mtu)) + %(ifaceobj.name, mtu, self.max_mtu)) retval = False return retval - def _propagate_mtu_to_upper_devs(self, ifaceobj, mtu_str, mtu_int, ifaceobj_getfunc): + def _propagate_mtu_to_upper_devs(self, ifaceobj, mtu, ifaceobj_getfunc): + """ + :param ifaceobj: + :param mtu: integer + :param ifaceobj_getfunc: + """ + if not ( (not ifupdownflags.flags.ALL or ifupdownconfig.diff_mode) and not ifaceobj.link_kind and @@ -863,16 +876,22 @@ def _propagate_mtu_to_upper_devs(self, ifaceobj, mtu_str, mtu_int, ifaceobj_getf umtu = upperobjs[0].get_attr_value_first('mtu') if not umtu: running_mtu = self.cache.get_link_mtu(upperobjs[0].name) - if not running_mtu or running_mtu != mtu_int: - self.sysfs.link_set_mtu(u, mtu_str=mtu_str, mtu_int=mtu_int) + if not running_mtu or running_mtu != mtu: + self.netlink.link_set_mtu(u, mtu) + + def _process_mtu_config_mtu_valid(self, ifaceobj, ifaceobj_getfunc, mtu): + """ + :param ifaceobj: + :param ifaceobj_getfunc: + :param mtu: integer + """ - def _process_mtu_config_mtu_valid(self, ifaceobj, ifaceobj_getfunc, mtu_str, mtu_int): - if not self._check_mtu_config(ifaceobj, mtu_str, mtu_int, ifaceobj_getfunc): + if not self._check_mtu_config(ifaceobj, mtu, ifaceobj_getfunc): return - if mtu_int != self.cache.get_link_mtu(ifaceobj.name): - self.sysfs.link_set_mtu(ifaceobj.name, mtu_str=mtu_str, mtu_int=mtu_int) - self._propagate_mtu_to_upper_devs(ifaceobj, mtu_str, mtu_int, ifaceobj_getfunc) + if mtu != self.cache.get_link_mtu(ifaceobj.name): + self.netlink.link_set_mtu(ifaceobj.name, mtu) + self._propagate_mtu_to_upper_devs(ifaceobj, mtu, ifaceobj_getfunc) def _process_mtu_config_mtu_none(self, ifaceobj, ifaceobj_getfunc): if (ifaceobj.link_privflags & ifaceLinkPrivFlags.MGMT_INTF): @@ -888,7 +907,7 @@ def _process_mtu_config_mtu_none(self, ifaceobj, ifaceobj_getfunc): or ifaceobj.link_kind & ifaceLinkKind.BRIDGE \ or ifaceobj.link_kind & ifaceLinkKind.OTHER: if cached_link_mtu != self.default_mtu_int: - self.sysfs.link_set_mtu(ifaceobj.name, mtu_str=self.default_mtu, mtu_int=self.default_mtu_int) + self.netlink.link_set_mtu(ifaceobj.name, self.default_mtu_int) return # set vlan interface mtu to lower device mtu @@ -898,10 +917,10 @@ def _process_mtu_config_mtu_none(self, ifaceobj, ifaceobj_getfunc): and ifaceobj.link_kind & ifaceLinkKind.VLAN ): lower_iface = ifaceobj.lowerifaces[0] - lower_iface_mtu_int = self.cache.get_link_mtu(lower_iface) + lower_iface_mtu = self.cache.get_link_mtu(lower_iface) - if lower_iface_mtu_int != cached_link_mtu: - self.sysfs.link_set_mtu(ifaceobj.name, mtu_str=str(lower_iface_mtu_int), mtu_int=lower_iface_mtu_int) + if lower_iface_mtu != cached_link_mtu: + self.netlink.link_set_mtu(ifaceobj.name, lower_iface_mtu) elif ( ifaceobj.name != 'lo' @@ -918,9 +937,9 @@ def _process_mtu_config_mtu_none(self, ifaceobj, ifaceobj_getfunc): # 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 - self.sysfs.link_set_mtu(ifaceobj.name, mtu_str=self.default_mtu, mtu_int=self.default_mtu_int) + self.netlink.link_set_mtu(ifaceobj.name, self.default_mtu_int) if ifupdownconfig.diff_mode: - self._propagate_mtu_to_upper_devs(ifaceobj, self.default_mtu, self.default_mtu_int, ifaceobj_getfunc) + self._propagate_mtu_to_upper_devs(ifaceobj, self.default_mtu_int, ifaceobj_getfunc) def _set_bridge_forwarding(self, ifaceobj): """ set ip forwarding to 0 if bridge interface does not have a @@ -1063,7 +1082,7 @@ def process_mtu(self, ifaceobj, ifaceobj_getfunc): self.logger.warning("%s: invalid MTU value: %s" % (ifaceobj.name, str(e))) return - self._process_mtu_config_mtu_valid(ifaceobj, ifaceobj_getfunc, mtu_str, mtu_int) + self._process_mtu_config_mtu_valid(ifaceobj, ifaceobj_getfunc, mtu_int) else: self._process_mtu_config_mtu_none(ifaceobj, ifaceobj_getfunc) @@ -1358,7 +1377,7 @@ def _down(self, ifaceobj, ifaceobj_getfunc=None): # ifupdown2. If this MTU is different from our default mtu, # if so we need to reset it back to default. if not ifaceobj.link_kind and self.default_mtu and ifaceobj.get_attr_value_first('mtu') and self.cache.get_link_mtu(ifaceobj.name) != self.default_mtu_int: - self.sysfs.link_set_mtu(ifaceobj.name, mtu_str=self.default_mtu, mtu_int=self.default_mtu_int) + self.netlink.link_set_mtu(ifaceobj.name, self.default_mtu_int) # # alias diff --git a/ifupdown2/addons/addressvirtual.py b/ifupdown2/addons/addressvirtual.py index edcb02f7..043ece0e 100644 --- a/ifupdown2/addons/addressvirtual.py +++ b/ifupdown2/addons/addressvirtual.py @@ -432,13 +432,12 @@ def create_macvlan_and_apply_config(self, ifaceobj, intf_config_list, vrrp=False purge_existing = False if ifupdownflags.flags.PERFMODE else True ifname = ifaceobj.name - update_mtu = lower_iface_mtu = lower_iface_mtu_str = None + update_mtu = lower_iface_mtu = None if ifupdownconfig.config.get("adjust_logical_dev_mtu", "1") != "0" and ifaceobj.lowerifaces and intf_config_list: update_mtu = True if update_mtu: lower_iface_mtu = self.cache.get_link_mtu(ifaceobj.name) - lower_iface_mtu_str = str(lower_iface_mtu) self.iproute2.batch_start() # TODO: make sure we only do 1 ip link set down and set up (only one flap in the batch) @@ -556,7 +555,7 @@ def create_macvlan_and_apply_config(self, ifaceobj, intf_config_list, vrrp=False update_mtu = False try: - self.sysfs.link_set_mtu(macvlan_ifname, mtu_str=lower_iface_mtu_str, mtu_int=lower_iface_mtu) + self.netlink.link_set_mtu(macvlan_ifname, lower_iface_mtu) except Exception as e: self.logger.info('%s: failed to set mtu %s: %s' % (macvlan_ifname, lower_iface_mtu, e)) diff --git a/ifupdown2/addons/bridge.py b/ifupdown2/addons/bridge.py index e70710ee..e124480b 100644 --- a/ifupdown2/addons/bridge.py +++ b/ifupdown2/addons/bridge.py @@ -2861,7 +2861,7 @@ def up_bridge(self, ifaceobj, ifaceobj_getfunc): bridge_mtu = self.get_bridge_mtu(ifaceobj) if bridge_mtu: - self.sysfs.link_set_mtu(ifname, bridge_mtu, int(bridge_mtu)) + self.netlink.link_set_mtu(ifname, int(bridge_mtu)) else: link_just_created = False self.logger.info('%s: bridge already exists' % ifname) diff --git a/ifupdown2/lib/iproute2.py b/ifupdown2/lib/iproute2.py index 5f1d6006..522a447b 100644 --- a/ifupdown2/lib/iproute2.py +++ b/ifupdown2/lib/iproute2.py @@ -521,10 +521,7 @@ def link_set_ipv6_addrgen(self, ifname, addrgen, link_created): self.logger.info("%s: cannot set addrgen: ipv6 is disabled on this device" % ifname) return False - if link_created: - link_mtu = self.sysfs.link_get_mtu(ifname) - else: - link_mtu = self.cache.get_link_mtu(ifname) + link_mtu = self.cache.get_link_mtu(ifname) if link_mtu < 1280: self.logger.info("%s: ipv6 addrgen is disabled on device with MTU " diff --git a/ifupdown2/lib/nlcache.py b/ifupdown2/lib/nlcache.py index e2fd5681..30a6a20c 100644 --- a/ifupdown2/lib/nlcache.py +++ b/ifupdown2/lib/nlcache.py @@ -60,6 +60,7 @@ NLM_F_REQUEST, \ NLM_F_CREATE, \ NLM_F_ACK, \ + NLM_F_REPLACE, \ RT_SCOPES, \ INFINITY_LIFE_TIME @@ -91,6 +92,7 @@ NLM_F_REQUEST, \ NLM_F_CREATE, \ NLM_F_ACK, \ + NLM_F_REPLACE, \ RT_SCOPES, \ INFINITY_LIFE_TIME @@ -3137,6 +3139,38 @@ def link_set_brport_with_info_slave_data_dry_run(self, ifname, kind, ifla_info_d self.log_info_ifname_dry_run(ifname, "netlink: ip link set dev %s: bridge port attributes" % ifname) self.logger.debug("attributes: %s" % ifla_info_slave_data) + ### + + def link_set_mtu(self, ifname, mtu): + """ + Sets the MTU of the given link, updating the cache on success. + + :param ifname: str - Name of the interface to update + :param mtu: int - New MTU to set for the interface. + :return: True if the operation was successful + """ + + if self.cache.get_link_mtu(ifname) == mtu: + # no need to update + return + + self.logger.info(f'{ifname}: netlink: ip link set dev {ifname} mtu {mtu}') + + debug = RTM_SETLINK in self.debug + try: + link = Link(RTM_SETLINK, debug, use_color=self.use_color) + link.flags = NLM_F_REPLACE | NLM_F_REQUEST | NLM_F_ACK + link.body = struct.pack('Bxxxiii', socket.AF_UNSPEC, 0, 0, 0) + link.add_attribute(Link.IFLA_IFNAME, ifname) + link.add_attribute(Link.IFLA_MTU, mtu) + link.build_message(next(self.sequence), self.pid) + result = self.tx_nlpacket_get_response_with_error(link) + self.cache.override_link_mtu(ifname, mtu) + + return result + except Exception as e: + raise Exception(f'{ifname}: netlink: failed to set mtu to {mtu}: {str(e)}') + ############################################################################ # ADDRESS ############################################################################ diff --git a/ifupdown2/lib/sysfs.py b/ifupdown2/lib/sysfs.py index 42577807..8e8d9949 100644 --- a/ifupdown2/lib/sysfs.py +++ b/ifupdown2/lib/sysfs.py @@ -106,22 +106,6 @@ def get_link_address(self, ifname): """ return self.read_file_oneline("/sys/class/net/%s/address" % ifname) - # - # MTU - # - - def link_get_mtu(self, ifname): - return int(self.read_file_oneline("/sys/class/net/%s/mtu" % ifname) or 0) - - def link_set_mtu(self, ifname, mtu_str, mtu_int): - if self.cache.get_link_mtu(ifname) != mtu_int: - if self.write_to_file('/sys/class/net/%s/mtu' % ifname, mtu_str): - self.cache.override_link_mtu(ifname, mtu_int) - - def link_set_mtu_dry_run(self, ifname, mtu_str, mtu_int): - # we can remove the cache check in DRYRUN mode - self.write_to_file('/sys/class/net/%s/mtu' % ifname, mtu_str) - # # ALIAS # From 3243dbdbf2a60e70938faac18cb01954c388d510 Mon Sep 17 00:00:00 2001 From: Christoph Heiss Date: Wed, 27 Aug 2025 11:32:50 +0200 Subject: [PATCH 2/2] addons, nlcache: set interface alias through netlink instead of sysfs The sysfs-based methods do not handle altnames at all. Instead, get/set interface aliases through netlink directly, which also has transparent altname support, and is a more robust way in general. Signed-off-by: Christoph Heiss --- ifupdown2/addons/address.py | 4 ++-- ifupdown2/lib/nlcache.py | 44 +++++++++++++++++++++++++++++++++++++ ifupdown2/lib/sysfs.py | 22 ------------------- 3 files changed, 46 insertions(+), 24 deletions(-) diff --git a/ifupdown2/addons/address.py b/ifupdown2/addons/address.py index bbc90d8d..a2619d10 100644 --- a/ifupdown2/addons/address.py +++ b/ifupdown2/addons/address.py @@ -1152,7 +1152,7 @@ def _pre_up(self, ifaceobj, ifaceobj_getfunc=None): # # alias # - self.sysfs.link_set_alias(ifaceobj.name, ifaceobj.get_attr_value_first("alias")) + self.netlink.link_set_alias(ifaceobj.name, ifaceobj.get_attr_value_first("alias")) self._sysctl_config(ifaceobj) @@ -1385,7 +1385,7 @@ def _down(self, ifaceobj, ifaceobj_getfunc=None): if not ifaceobj.link_kind: alias = ifaceobj.get_attr_value_first("alias") if alias: - self.sysfs.link_set_alias(ifaceobj.name, None) # None to reset alias. + self.netlink.link_set_alias(ifaceobj.name, None) # None to reset alias. hwaddress = self.process_hwaddress_reset_to_default(ifaceobj) if hwaddress != None and not ifaceobj.link_kind: diff --git a/ifupdown2/lib/nlcache.py b/ifupdown2/lib/nlcache.py index 30a6a20c..29ad3fe4 100644 --- a/ifupdown2/lib/nlcache.py +++ b/ifupdown2/lib/nlcache.py @@ -409,6 +409,19 @@ def override_link_mtu(self, ifname, mtu): except Exception: pass + def override_link_alias(self, ifname, alias): + """ + Manually override link alias in the cache and ignore any failures + :param ifname: Name of the interface to update + :param alias: New interface alias + """ + try: + with self._cache_lock: + self._link_cache[ifname].attributes[Link.IFLA_IFALIAS].value = alias + except Exception: + pass + + def override_cache_unslave_link(self, slave, master): """ Manually update the cache unslaving SLAVE from MASTER @@ -3171,6 +3184,37 @@ def link_set_mtu(self, ifname, mtu): except Exception as e: raise Exception(f'{ifname}: netlink: failed to set mtu to {mtu}: {str(e)}') + def link_set_alias(self, ifname, alias): + """ + Sets the alias of the given link, updating the cache on success. + + :param ifname: Name of the interface to update + :param alias: New alias to set for the interface. + :return: True if the operation was successful + """ + + if self.cache.get_link_alias(ifname) == alias: + # no need to update + return + + self.logger.info(f'{ifname}: netlink: ip link set dev {ifname} alias {alias}') + + debug = RTM_SETLINK in self.debug + try: + link = Link(RTM_SETLINK, debug, use_color=self.use_color) + link.flags = NLM_F_REPLACE | NLM_F_REQUEST | NLM_F_ACK + link.body = struct.pack('Bxxxiii', socket.AF_UNSPEC, 0, 0, 0) + link.add_attribute(Link.IFLA_IFNAME, ifname) + link.add_attribute(Link.IFLA_IFALIAS, alias or '') # empty string removes alias + link.build_message(next(self.sequence), self.pid) + result = self.tx_nlpacket_get_response_with_error(link) + self.cache.override_link_alias(ifname, alias) + + return result + except Exception as e: + raise Exception(f'{ifname}: netlink: failed to set alias to {alias}: {str(e)}') + + ############################################################################ # ADDRESS ############################################################################ diff --git a/ifupdown2/lib/sysfs.py b/ifupdown2/lib/sysfs.py index 8e8d9949..e18b605a 100644 --- a/ifupdown2/lib/sysfs.py +++ b/ifupdown2/lib/sysfs.py @@ -106,28 +106,6 @@ def get_link_address(self, ifname): """ return self.read_file_oneline("/sys/class/net/%s/address" % ifname) - # - # ALIAS - # - - def link_set_alias(self, ifname, alias): - cached_alias = self.cache.get_link_alias(ifname) - - if cached_alias == alias: - return - - if not alias: - alias = "\n" - - if self.write_to_file("/sys/class/net/%s/ifalias" % ifname, alias): - pass # self.cache.override_link_mtu(ifname, mtu_int) - - def link_set_alias_dry_run(self, ifname, alias): - # we can remove the cache check in DRYRUN mode - if not alias: - alias = "" - self.write_to_file("/sys/class/net/%s/ifalias" % ifname, alias) - ############################################################################ # BRIDGE ############################################################################