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

1 files changed, 17 insertions, 7 deletions

diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c
index 90b1920cc0..99d00ec4f2 100644
--- a/src/core/devices/nm-device.c
+++ b/src/core/devices/nm-device.c
@@ -3927,12 +3927,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:
@@ -9556,8 +9559,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);
 }
 
@@ -11901,6 +11902,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