From 352e8bb86547f849ddd05cffce52a2e7760ce853 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 26 Apr 2022 10:35:49 +0200 Subject: [PATCH] device: set MTU after attaching bond port When attaching a bond port, kernel will reset the MTU of the port ([1], [2]). Configuring a different MTU on the port seems not a sensible thing for the user to do. Still, before commit e67ddd826fae ('device: commit MTU during stage2') we would first attach the bond port before setting the MTU. That changed, and now the MTU set by kernel wins. Btw, this change in behavior happens because we attach the port in stage3 (ip-config), which seems an ugly thing to do. Anyway, fix this by setting the MTU after attaching the ports, but still in stage3. It is probably not sensible for the user to configure a different MTU. Still, if the user requested it by configuration, we should apply it. Note that NetworkManager has some logic to constrain the MTU based on the parent/child and controller/port. In many regards however, NetworkManager does not fully understand or enforce the correct MTU and relies on the user to configure it correctly. After all, if the user misconfigures the MTU, the setup will have problems anyway (and in many cases neither kernel nor NetworkManager could know that the configuration is wrong). [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/bonding/bond_main.c?h=v5.17#n3603 [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/bonding/bond_main.c?h=v5.17#n4372 https://bugzilla.redhat.com/show_bug.cgi?id=2071985 Fixes: e67ddd826fae ('device: commit MTU during stage2') https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1199 (cherry picked from commit 6804c2ba0479a44c314a61bbdcc29e0cd6987166) --- src/core/devices/nm-device.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 84c1a08e8e..639fe82f96 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -3898,12 +3898,15 @@ _dev_l3_cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, N case NM_L3_CONFIG_NOTIFY_TYPE_PRE_COMMIT: { const NML3ConfigData *l3cd; + NMDeviceState state = nm_device_get_state(self); - /* FIXME(l3cfg): MTU handling should be moved to l3cfg. */ - l3cd = nm_l3cfg_get_combined_l3cd(l3cfg, TRUE); - if (l3cd) - priv->ip6_mtu = nm_l3_config_data_get_ip6_mtu(l3cd); - _commit_mtu(self); + if (state >= NM_DEVICE_STATE_IP_CONFIG && state < NM_DEVICE_STATE_DEACTIVATING) { + /* FIXME(l3cfg): MTU handling should be moved to l3cfg. */ + l3cd = nm_l3cfg_get_combined_l3cd(l3cfg, TRUE); + if (l3cd) + priv->ip6_mtu = nm_l3_config_data_get_ip6_mtu(l3cd); + _commit_mtu(self); + } return; } case NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT: @@ -9527,8 +9530,6 @@ activate_stage2_device_config(NMDevice *self) lldp_setup(self, NM_TERNARY_DEFAULT); - _commit_mtu(self); - nm_device_activate_schedule_stage3_ip_config(self, TRUE); } @@ -11872,6 +11873,15 @@ activate_stage3_ip_config(NMDevice *self) nm_device_get_ip_iface(self)); } + /* We currently will attach ports in the state change NM_DEVICE_STATE_IP_CONFIG above. + * Note that kernel changes the MTU of bond ports, so we want to commit the MTU + * afterwards! + * + * This might reset the MTU to something different from the bond controller and + * it might not be a working configuration. But it's what the user asked for, so + * let's do it! */ + _commit_mtu(self); + ipv4_method = nm_device_get_effective_ip_config_method(self, AF_INET); if (nm_streq(ipv4_method, NM_SETTING_IP4_CONFIG_METHOD_AUTO)) { /* "auto" usually means DHCPv4 or autoconf6, but it doesn't have to be. Subclasses