From 05e8b018ec10d626bf1c629f4357debd8cb3c445 Mon Sep 17 00:00:00 2001 From: Antonio Cardace Date: Fri, 23 Oct 2020 09:41:01 +0200 Subject: [PATCH 1/3] platform: allow setting the MTU at link creation time Add a parameter to the 'link_add()' virtual function so that the MTU will be configured (via netlink) by the kernel when creating the link. https://bugzilla.redhat.com/show_bug.cgi?id=1778590 Signed-off-by: Antonio Cardace (cherry picked from commit ba2ee462541ea3c1507e635accc380edea225f58) --- src/platform/nm-fake-platform.c | 19 +++++++------ src/platform/nm-linux-platform.c | 4 +++ src/platform/nm-platform.c | 8 +++++- src/platform/nm-platform.h | 47 ++++++++++++++++++++++++-------- 4 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index 94c3f0e833..5bb0b426fe 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -225,7 +225,8 @@ link_add_pre(NMPlatform *platform, const char *name, NMLinkType type, const void *address, - size_t address_len) + size_t address_len, + guint32 mtu) { NMFakePlatformPrivate *priv = NM_FAKE_PLATFORM_GET_PRIVATE(platform); NMFakePlatformLink * device; @@ -251,6 +252,7 @@ link_add_pre(NMPlatform *platform, link->ifindex = name ? ifindex : 0; link->type = type; link->kind = g_intern_string(nm_link_type_to_string(type)); + link->mtu = mtu; link->initialized = TRUE; if (name) strcpy(link->name, name); @@ -285,6 +287,7 @@ link_add(NMPlatform * platform, int parent, const void * address, size_t address_len, + guint32 mtu, gconstpointer extra_data, const NMPlatformLink **out_link) { @@ -300,7 +303,7 @@ link_add(NMPlatform * platform, NMPObject * dev_obj; NMPObject * dev_lnk = NULL; - device = link_add_pre(platform, name, type, address, address_len); + device = link_add_pre(platform, name, type, address, address_len, mtu); g_assert(device); @@ -326,7 +329,7 @@ link_add(NMPlatform * platform, case NM_LINK_TYPE_VETH: veth_peer = extra_data; g_assert(veth_peer); - device_veth = link_add_pre(platform, veth_peer, type, NULL, 0); + device_veth = link_add_pre(platform, veth_peer, type, NULL, 0, 0); break; case NM_LINK_TYPE_VLAN: { @@ -401,7 +404,7 @@ link_add_one(NMPlatform *platform, NMPCacheOpsType cache_op; int ifindex; - device = link_add_pre(platform, name, NM_LINK_TYPE_VLAN, NULL, 0); + device = link_add_pre(platform, name, NM_LINK_TYPE_VLAN, NULL, 0, 0); ifindex = NMP_OBJECT_CAST_LINK(device->obj)->ifindex; @@ -1313,10 +1316,10 @@ nm_fake_platform_setup(void) nm_platform_setup(platform); - link_add(platform, NM_LINK_TYPE_LOOPBACK, "lo", 0, NULL, 0, NULL, NULL); - link_add(platform, NM_LINK_TYPE_ETHERNET, "eth0", 0, NULL, 0, NULL, NULL); - link_add(platform, NM_LINK_TYPE_ETHERNET, "eth1", 0, NULL, 0, NULL, NULL); - link_add(platform, NM_LINK_TYPE_ETHERNET, "eth2", 0, NULL, 0, NULL, NULL); + link_add(platform, NM_LINK_TYPE_LOOPBACK, "lo", 0, NULL, 0, 0, NULL, NULL); + link_add(platform, NM_LINK_TYPE_ETHERNET, "eth0", 0, NULL, 0, 0, NULL, NULL); + link_add(platform, NM_LINK_TYPE_ETHERNET, "eth1", 0, NULL, 0, 0, NULL, NULL); + link_add(platform, NM_LINK_TYPE_ETHERNET, "eth2", 0, NULL, 0, 0, NULL, NULL); } static void diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 3fa70bbc30..b377c85e7a 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -7341,6 +7341,7 @@ link_add(NMPlatform * platform, int parent, const void * address, size_t address_len, + guint32 mtu, gconstpointer extra_data, const NMPlatformLink **out_link) { @@ -7368,6 +7369,9 @@ link_add(NMPlatform * platform, if (address && address_len) NLA_PUT(nlmsg, IFLA_ADDRESS, address_len, address); + if (mtu) + NLA_PUT_U32(nlmsg, IFLA_MTU, mtu); + if (!_nl_msg_new_link_set_linkinfo(nlmsg, type, extra_data)) return -NME_UNSPEC; diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index d1ff6fbc42..7969206cba 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -1230,11 +1230,13 @@ nm_platform_link_add(NMPlatform * self, int parent, const void * address, size_t address_len, + guint32 mtu, gconstpointer extra_data, const NMPlatformLink **out_link) { int r; char addr_buf[NM_UTILS_HWADDR_LEN_MAX * 3]; + char mtu_buf[16]; char parent_buf[64]; char buf[512]; @@ -1254,6 +1256,7 @@ nm_platform_link_add(NMPlatform * self, "\"%s\"" /* name */ "%s%s" /* parent */ "%s%s" /* address */ + "%s%s" /* mtu */ "%s" /* extra_data */ "", nm_link_type_to_string(type), @@ -1263,6 +1266,8 @@ nm_platform_link_add(NMPlatform * self, address ? ", address: " : "", address ? _nm_utils_hwaddr_ntoa(address, address_len, FALSE, addr_buf, sizeof(addr_buf)) : "", + mtu ? ", mtu: " : "", + mtu ? nm_sprintf_buf(mtu_buf, "%u", mtu) : "", ({ char *buf_p = buf; gsize buf_len = sizeof(buf); @@ -1345,7 +1350,8 @@ nm_platform_link_add(NMPlatform * self, buf; })); - return klass->link_add(self, type, name, parent, address, address_len, extra_data, out_link); + return klass + ->link_add(self, type, name, parent, address, address_len, mtu, extra_data, out_link); } /** diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index c7b8017a40..a7396a7107 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -1085,6 +1085,7 @@ typedef struct { int parent, const void * address, size_t address_len, + guint32 mtu, gconstpointer extra_data, const NMPlatformLink **out_link); gboolean (*link_delete)(NMPlatform *self, int ifindex); @@ -1532,6 +1533,7 @@ int nm_platform_link_add(NMPlatform * self, int parent, const void * address, size_t address_len, + guint32 mtu, gconstpointer extra_data, const NMPlatformLink **out_link); @@ -1541,13 +1543,13 @@ nm_platform_link_veth_add(NMPlatform * self, const char * peer, const NMPlatformLink **out_link) { - return nm_platform_link_add(self, NM_LINK_TYPE_VETH, name, 0, NULL, 0, peer, out_link); + return nm_platform_link_add(self, NM_LINK_TYPE_VETH, name, 0, NULL, 0, 0, peer, out_link); } static inline int nm_platform_link_dummy_add(NMPlatform *self, const char *name, const NMPlatformLink **out_link) { - return nm_platform_link_add(self, NM_LINK_TYPE_DUMMY, name, 0, NULL, 0, NULL, out_link); + return nm_platform_link_add(self, NM_LINK_TYPE_DUMMY, name, 0, NULL, 0, 0, NULL, out_link); } static inline int @@ -1564,6 +1566,7 @@ nm_platform_link_bridge_add(NMPlatform * self, 0, address, address_len, + 0, props, out_link); } @@ -1571,19 +1574,19 @@ nm_platform_link_bridge_add(NMPlatform * self, static inline int nm_platform_link_bond_add(NMPlatform *self, const char *name, const NMPlatformLink **out_link) { - return nm_platform_link_add(self, NM_LINK_TYPE_BOND, name, 0, NULL, 0, NULL, out_link); + return nm_platform_link_add(self, NM_LINK_TYPE_BOND, name, 0, NULL, 0, 0, NULL, out_link); } static inline int nm_platform_link_team_add(NMPlatform *self, const char *name, const NMPlatformLink **out_link) { - return nm_platform_link_add(self, NM_LINK_TYPE_TEAM, name, 0, NULL, 0, NULL, out_link); + return nm_platform_link_add(self, NM_LINK_TYPE_TEAM, name, 0, NULL, 0, 0, NULL, out_link); } static inline int nm_platform_link_wireguard_add(NMPlatform *self, const char *name, const NMPlatformLink **out_link) { - return nm_platform_link_add(self, NM_LINK_TYPE_WIREGUARD, name, 0, NULL, 0, NULL, out_link); + return nm_platform_link_add(self, NM_LINK_TYPE_WIREGUARD, name, 0, NULL, 0, 0, NULL, out_link); } static inline int @@ -1602,6 +1605,7 @@ nm_platform_link_gre_add(NMPlatform * self, 0, address, address_len, + 0, props, out_link); } @@ -1612,7 +1616,7 @@ nm_platform_link_sit_add(NMPlatform * self, const NMPlatformLnkSit *props, const NMPlatformLink ** out_link) { - return nm_platform_link_add(self, NM_LINK_TYPE_SIT, name, 0, NULL, 0, props, out_link); + return nm_platform_link_add(self, NM_LINK_TYPE_SIT, name, 0, NULL, 0, 0, props, out_link); } static inline int @@ -1632,6 +1636,7 @@ nm_platform_link_vlan_add(NMPlatform * self, parent, NULL, 0, + 0, &((NMPlatformLnkVlan){ .id = vlanid, .flags = vlanflags, @@ -1645,7 +1650,7 @@ nm_platform_link_vrf_add(NMPlatform * self, const NMPlatformLnkVrf *props, const NMPlatformLink ** out_link) { - return nm_platform_link_add(self, NM_LINK_TYPE_VRF, name, 0, NULL, 0, props, out_link); + return nm_platform_link_add(self, NM_LINK_TYPE_VRF, name, 0, NULL, 0, 0, props, out_link); } static inline int @@ -1654,7 +1659,7 @@ nm_platform_link_vxlan_add(NMPlatform * self, const NMPlatformLnkVxlan *props, const NMPlatformLink ** out_link) { - return nm_platform_link_add(self, NM_LINK_TYPE_VXLAN, name, 0, NULL, 0, props, out_link); + return nm_platform_link_add(self, NM_LINK_TYPE_VXLAN, name, 0, NULL, 0, 0, props, out_link); } static inline int @@ -1663,7 +1668,15 @@ nm_platform_link_6lowpan_add(NMPlatform * self, int parent, const NMPlatformLink **out_link) { - return nm_platform_link_add(self, NM_LINK_TYPE_6LOWPAN, name, parent, NULL, 0, NULL, out_link); + return nm_platform_link_add(self, + NM_LINK_TYPE_6LOWPAN, + name, + parent, + NULL, + 0, + 0, + NULL, + out_link); } static inline int @@ -1675,7 +1688,7 @@ nm_platform_link_ip6tnl_add(NMPlatform * self, g_return_val_if_fail(props, -NME_BUG); g_return_val_if_fail(!props->is_gre, -NME_BUG); - return nm_platform_link_add(self, NM_LINK_TYPE_IP6TNL, name, 0, NULL, 0, props, out_link); + return nm_platform_link_add(self, NM_LINK_TYPE_IP6TNL, name, 0, NULL, 0, 0, props, out_link); } static inline int @@ -1695,6 +1708,7 @@ nm_platform_link_ip6gre_add(NMPlatform * self, 0, address, address_len, + 0, props, out_link); } @@ -1707,7 +1721,7 @@ nm_platform_link_ipip_add(NMPlatform * self, { g_return_val_if_fail(props, -NME_BUG); - return nm_platform_link_add(self, NM_LINK_TYPE_IPIP, name, 0, NULL, 0, props, out_link); + return nm_platform_link_add(self, NM_LINK_TYPE_IPIP, name, 0, NULL, 0, 0, props, out_link); } static inline int @@ -1720,7 +1734,15 @@ nm_platform_link_macsec_add(NMPlatform * self, g_return_val_if_fail(props, -NME_BUG); g_return_val_if_fail(parent > 0, -NME_BUG); - return nm_platform_link_add(self, NM_LINK_TYPE_MACSEC, name, parent, NULL, 0, props, out_link); + return nm_platform_link_add(self, + NM_LINK_TYPE_MACSEC, + name, + parent, + NULL, + 0, + 0, + props, + out_link); } static inline int @@ -1739,6 +1761,7 @@ nm_platform_link_macvlan_add(NMPlatform * self, parent, NULL, 0, + 0, props, out_link); } From 8d8017a62073ae03471f982dbb01fc5caaefa698 Mon Sep 17 00:00:00 2001 From: Antonio Cardace Date: Fri, 30 Oct 2020 16:35:25 +0100 Subject: [PATCH 2/3] bridge: set MTU at link creation time https://bugzilla.redhat.com/show_bug.cgi?id=1778590 Signed-off-by: Antonio Cardace (cherry picked from commit 516c6236183b361eccf3e0693616b48c680d0f7b) --- src/devices/nm-device-bridge.c | 7 +++++++ src/platform/nm-platform.h | 3 ++- src/platform/tests/test-common.c | 2 +- src/platform/tests/test-link.c | 3 +++ 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index 3c27b349d5..d9dbd5e356 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -1027,6 +1027,7 @@ create_and_realize(NMDevice * device, const NMPlatformLink **out_plink, GError ** error) { + NMSettingWired * s_wired; NMSettingBridge * s_bridge; const char * iface = nm_device_get_iface(device); const char * hwaddr; @@ -1034,12 +1035,17 @@ create_and_realize(NMDevice * device, guint8 mac_address[NM_UTILS_HWADDR_LEN_MAX]; NMPlatformLnkBridge props; int r; + guint32 mtu = 0; nm_assert(iface); s_bridge = nm_connection_get_setting_bridge(connection); nm_assert(s_bridge); + s_wired = nm_connection_get_setting_wired(connection); + if (s_wired) + mtu = nm_setting_wired_get_mtu(s_wired); + hwaddr = nm_setting_bridge_get_mac_address(s_bridge); if (!hwaddr && nm_device_hw_addr_get_cloned(device, connection, FALSE, &hwaddr_cloned, NULL, NULL)) { @@ -1101,6 +1107,7 @@ create_and_realize(NMDevice * device, iface, hwaddr ? mac_address : NULL, hwaddr ? ETH_ALEN : 0, + mtu, &props, out_plink); if (r < 0) { diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index a7396a7107..a1314000c3 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -1557,6 +1557,7 @@ nm_platform_link_bridge_add(NMPlatform * self, const char * name, const void * address, size_t address_len, + guint32 mtu, const NMPlatformLnkBridge *props, const NMPlatformLink ** out_link) { @@ -1566,7 +1567,7 @@ nm_platform_link_bridge_add(NMPlatform * self, 0, address, address_len, - 0, + mtu, props, out_link); } diff --git a/src/platform/tests/test-common.c b/src/platform/tests/test-common.c index e305357a7a..fb43004630 100644 --- a/src/platform/tests/test-common.c +++ b/src/platform/tests/test-common.c @@ -1534,7 +1534,7 @@ nmtstp_link_bridge_add(NMPlatform * platform, } if (!pllink) { - r = nm_platform_link_bridge_add(platform, name, NULL, 0, lnk, &pllink); + r = nm_platform_link_bridge_add(platform, name, NULL, 0, 0, lnk, &pllink); } _assert_pllink(platform, r == 0, pllink, name, NM_LINK_TYPE_BRIDGE); diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c index 829abcc9ac..5fd67ec41f 100644 --- a/src/platform/tests/test-link.c +++ b/src/platform/tests/test-link.c @@ -101,6 +101,7 @@ software_add(NMLinkType link_type, const char *name) name, NULL, 0, + 0, &nm_platform_lnk_bridge_default, NULL)); case NM_LINK_TYPE_BOND: @@ -131,6 +132,7 @@ software_add(NMLinkType link_type, const char *name) PARENT_NAME, NULL, 0, + 0, &nm_platform_lnk_bridge_default, NULL))) accept_signal(parent_added); @@ -581,6 +583,7 @@ test_bridge_addr(void) DEVICE_NAME, addr, sizeof(addr), + 0, &nm_platform_lnk_bridge_default, &plink))); g_assert(plink); From 864872e9a8c694ee7472154bf45f869040310b71 Mon Sep 17 00:00:00 2001 From: Antonio Cardace Date: Wed, 21 Oct 2020 18:57:18 +0200 Subject: [PATCH 3/3] bridge: force (hack)-set of the MTU when explicitly set in the profile Kernel does a auto-mtu adjusting process whenever a port is added/removed from the bridge, this can cause issues when NM wants to explicitly set an MTU which is equal to the bridge default one (1500) because if later a port is added with a different MTU the kernel will assign the bridge that port's MTU resulting in the bridge runtime configuration differing from the bridge's NM connection profile. What we can do is to always apply the MTU manually for the bridge (if explicitly set by the profile), after doing so the kernel won't modify the MTU anymore, which is what we want, problem is that kernel won't actually apply the MTU to the netdev if it's not actually changing so we first apply it to MTU-1 and then to the desired value. https://bugzilla.redhat.com/show_bug.cgi?id=1778590 Signed-off-by: Antonio Cardace (cherry picked from commit e23798a5e5e8d7eb615f0e8b305fc20b8b645c66) --- src/devices/nm-device-bridge.c | 7 +++++++ src/devices/nm-device.c | 16 ++++++++++++++++ src/devices/nm-device.h | 11 +++++++++++ 3 files changed, 34 insertions(+) diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index d9dbd5e356..48dcec1b1d 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -1103,6 +1103,12 @@ create_and_realize(NMDevice * device, to_sysfs_group_address_sys(nm_setting_bridge_get_group_address(s_bridge), &props.group_addr); + /* If mtu != 0, we set the MTU of the new bridge at creation time. However, kernel will still + * automatically adjust the MTU of the bridge based on the minimum of the slave's MTU. + * We don't want this automatism as the user asked for a fixed MTU. + * + * To workaround this behavior of kernel, we will later toggle the MTU twice. See + * NMDeviceClass.mtu_force_set. */ r = nm_platform_link_bridge_add(nm_device_get_platform(device), iface, hwaddr ? mac_address : NULL, @@ -1159,6 +1165,7 @@ nm_device_bridge_class_init(NMDeviceBridgeClass *klass) device_class->link_types = NM_DEVICE_DEFINE_LINK_TYPES(NM_LINK_TYPE_BRIDGE); device_class->is_master = TRUE; + device_class->mtu_force_set = TRUE; device_class->get_generic_capabilities = get_generic_capabilities; device_class->check_connection_compatible = check_connection_compatible; device_class->check_connection_available = check_connection_available; diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 2dab5075a6..0be05f23d7 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -659,6 +659,8 @@ typedef struct _NMDevicePrivate { guint64 tx_bytes; guint64 rx_bytes; } stats; + + bool mtu_force_set_done : 1; } NMDevicePrivate; G_DEFINE_ABSTRACT_TYPE(NMDevice, nm_device, NM_TYPE_DBUS_OBJECT) @@ -10433,6 +10435,18 @@ _commit_mtu(NMDevice *self, const NMIP4Config *config) } } + if (mtu_desired && NM_DEVICE_GET_CLASS(self)->mtu_force_set && !priv->mtu_force_set_done) { + priv->mtu_force_set_done = TRUE; + + if (mtu_desired == mtu_plat) { + mtu_plat--; + if (NM_DEVICE_GET_CLASS(self)->set_platform_mtu(self, mtu_desired - 1)) { + _LOGD(LOGD_DEVICE, "mtu: force-set MTU to %u", mtu_desired - 1); + } else + _LOGW(LOGD_DEVICE, "mtu: failure to force-set MTU to %u", mtu_desired - 1); + } + } + _LOGT(LOGD_DEVICE, "mtu: device-mtu: %u%s, ipv6-mtu: %u%s, ifindex: %d", (guint) mtu_desired, @@ -15773,6 +15787,8 @@ _cleanup_generic_post(NMDevice *self, CleanupType cleanup_type) priv->linklocal6_dad_counter = 0; + priv->mtu_force_set_done = FALSE; + /* Clean up IP configs; this does not actually deconfigure the * interface; the caller must flush routes and addresses explicitly. */ diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 13782417e3..3eae931834 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -232,6 +232,17 @@ typedef struct _NMDeviceClass { * type (NMDeviceClass), not the actual device instance. */ bool is_master : 1; + /* Force setting the MTU actually means first setting the MTU + * to (desired_MTU-1) and then setting the desired_MTU + * so that kernel actually applies the MTU, otherwise + * kernel will ignore the request if the link's MTU is the + * same as the desired one. + * + * This is just a workaround made for bridges (ATM) that employ + * a auto-MTU adjust mechanism if no MTU is manually set. + */ + bool mtu_force_set : 1; + void (*state_changed)(NMDevice * device, NMDeviceState new_state, NMDeviceState old_state,