libnm/infiniband: lift restriction of MTU to 2044 for IPoIB in "datagram" mode

Traditionally, the MTU in "datagram" transport mode was restricted to
2044. That is no longer the case, relax that.

In fact, choose a very large maximum and don't differenciate between
"connected" mode (they now both use now 65520). This is only the
limitation of the connection profile. Whether setting such large MTUs
actually works must be determined when activating the profile.

Initscripts "ifup-ib" from rdma-core package originally had a limit of 2044.
This was raised to 4092 in rh#1186498. It is suggested to raise it further
in bug rh#1647541.

In general, kernel often does not allow setting large MTUs. And even if it
allows it, it may not work because it also requires the entire network to
be configured accordingly. But that means, it is generally not helpful to
limit the MTU in the connection profile too strictly. Just allow large
MTUs, we need to see at activation time whether the configuration works.

Note also that all other setting types don't validate the range for MTU at
all.

Related: https://bugzilla.redhat.com/show_bug.cgi?id=1186498
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1593334
         (rdma-core: raise limit from 2044 to 4092 in ifup-ib)

Related: https://bugzilla.redhat.com/show_bug.cgi?id=1647541
         (rdma-core: raise limit beyond 4092 in ifup-ib)

Related: https://bugzilla.redhat.com/show_bug.cgi?id=1532638#c4
         (rdma-core: MTU related discussion)

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534869
       (NetworkManager bug about this topic, but with lots of unrelated
        discussion. See in particular #c16)

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1653494

4 files changed, 45 insertions, 34 deletions

diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c
index e91052a35b..3182e3460f 100644
--- a/libnm-core/nm-connection.c
+++ b/libnm-core/nm-connection.c
@@ -1059,23 +1059,14 @@ _normalize_infiniband_mtu (NMConnection *self, GHashTable *parameters)
 {
 	NMSettingInfiniband *s_infini = nm_connection_get_setting_infiniband (self);
 
-	if (s_infini) {
-		const char *transport_mode = nm_setting_infiniband_get_transport_mode (s_infini);
-		guint32 max_mtu = 0;
-
-		if (transport_mode) {
-			if (!strcmp (transport_mode, "datagram"))
-				max_mtu = 2044;
-			else if (!strcmp (transport_mode, "connected"))
-				max_mtu = 65520;
-
-			if (max_mtu && nm_setting_infiniband_get_mtu (s_infini) > max_mtu) {
-				g_object_set (s_infini, NM_SETTING_INFINIBAND_MTU, (guint) max_mtu, NULL);
-				return TRUE;
-			}
-		}
-	}
-	return FALSE;
+	if (   !s_infini
+	    || nm_setting_infiniband_get_mtu (s_infini) <= NM_INFINIBAND_MAX_MTU
+	    || !NM_IN_STRSET (nm_setting_infiniband_get_transport_mode (s_infini), "datagram",
+	                                                                           "connected"))
+		return FALSE;
+
+	g_object_set (s_infini, NM_SETTING_INFINIBAND_MTU, (guint) NM_INFINIBAND_MAX_MTU, NULL);
+	return TRUE;
 }
 
 static gboolean
diff --git a/libnm-core/nm-setting-infiniband.c b/libnm-core/nm-setting-infiniband.c
index 9348fa2be0..bde7eda986 100644
--- a/libnm-core/nm-setting-infiniband.c
+++ b/libnm-core/nm-setting-infiniband.c
@@ -170,7 +170,6 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 {
 	NMSettingConnection *s_con = NULL;
 	NMSettingInfinibandPrivate *priv = NM_SETTING_INFINIBAND_GET_PRIVATE (setting);
-	guint32 normerr_max_mtu = 0;
 
 	if (priv->mac_address && !nm_utils_hwaddr_valid (priv->mac_address, INFINIBAND_ALEN)) {
 		g_set_error_literal (error,
@@ -181,13 +180,8 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		return FALSE;
 	}
 
-	if (!g_strcmp0 (priv->transport_mode, "datagram")) {
-		if (priv->mtu > 2044)
-			normerr_max_mtu = 2044;
-	} else if (!g_strcmp0 (priv->transport_mode, "connected")) {
-		if (priv->mtu > 65520)
-			normerr_max_mtu = 65520;
-	} else {
+	if (!NM_IN_STRSET (priv->transport_mode, "datagram",
+	                                         "connected")) {
 		g_set_error_literal (error,
 		                     NM_CONNECTION_ERROR,
 		                     NM_CONNECTION_ERROR_INVALID_PROPERTY,
@@ -271,12 +265,21 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 
 	/* *** errors above here should be always fatal, below NORMALIZABLE_ERROR *** */
 
-	if (normerr_max_mtu > 0) {
+	if (priv->mtu > NM_INFINIBAND_MAX_MTU) {
+		/* Traditionally, MTU for "datagram" mode was limited to 2044
+		 * and for "connected" mode it was 65520.
+		 *
+		 * This is no longer the case, and both transport modes use the same
+		 * maximum of 65520 (NM_INFINIBAND_MAX_MTU).
+		 *
+		 * Note that this is the MTU in the connection profile. Whether
+		 * we will be able to configure large MTUs later (during activation)
+		 * is unknown at this point. */
 		g_set_error (error,
 		             NM_CONNECTION_ERROR,
 		             NM_CONNECTION_ERROR_INVALID_PROPERTY,
-		             _("mtu for transport mode '%s' can be at most %d but it is %d"),
-		             priv->transport_mode, normerr_max_mtu, priv->mtu);
+		             _("mtu can be at most %u but it is %u"),
+		             NM_INFINIBAND_MAX_MTU, priv->mtu);
 		g_prefix_error (error, "%s.%s: ", NM_SETTING_INFINIBAND_SETTING_NAME, NM_SETTING_INFINIBAND_MTU);
 		return NM_SETTING_VERIFY_NORMALIZABLE_ERROR;
 	}
diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c
index 98e5837176..765957135a 100644
--- a/libnm-core/tests/test-general.c
+++ b/libnm-core/tests/test-general.c
@@ -4737,6 +4737,7 @@ test_connection_normalize_infiniband_mtu (void)
 {
 	gs_unref_object NMConnection *con = NULL;
 	NMSettingInfiniband *s_infini;
+	guint mtu_regular = nmtst_rand_select (2044, 2045, 65520);
 
 	con = nmtst_create_minimal_connection ("test_connection_normalize_infiniband_mtu", NULL,
 	                                       NM_SETTING_INFINIBAND_SETTING_NAME, NULL);
@@ -4749,26 +4750,26 @@ test_connection_normalize_infiniband_mtu (void)
 
 	g_object_set (s_infini,
 	              NM_SETTING_INFINIBAND_TRANSPORT_MODE, "datagram",
-	              NM_SETTING_INFINIBAND_MTU, (guint) 2044,
+	              NM_SETTING_INFINIBAND_MTU, (guint) mtu_regular,
 	              NULL);
 	nmtst_assert_connection_verifies_and_normalizable (con);
 	nmtst_connection_normalize (con);
-	g_assert_cmpint (2044, ==, nm_setting_infiniband_get_mtu (s_infini));
+	g_assert_cmpint (mtu_regular, ==, nm_setting_infiniband_get_mtu (s_infini));
 
 	g_object_set (s_infini,
 	              NM_SETTING_INFINIBAND_TRANSPORT_MODE, "datagram",
-	              NM_SETTING_INFINIBAND_MTU, (guint) 2045,
+	              NM_SETTING_INFINIBAND_MTU, (guint) 65521,
 	              NULL);
 	nmtst_assert_connection_verifies_after_normalization (con, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY);
 	nmtst_connection_normalize (con);
-	g_assert_cmpint (2044, ==, nm_setting_infiniband_get_mtu (s_infini));
+	g_assert_cmpint (65520, ==, nm_setting_infiniband_get_mtu (s_infini));
 
 	g_object_set (s_infini,
 	              NM_SETTING_INFINIBAND_TRANSPORT_MODE, "connected",
-	              NM_SETTING_INFINIBAND_MTU, (guint) 65520,
+	              NM_SETTING_INFINIBAND_MTU, (guint) mtu_regular,
 	              NULL);
 	nmtst_assert_connection_verifies_without_normalization (con);
-	g_assert_cmpint (65520, ==, nm_setting_infiniband_get_mtu (s_infini));
+	g_assert_cmpint (mtu_regular, ==, nm_setting_infiniband_get_mtu (s_infini));
 
 	g_object_set (s_infini,
 	              NM_SETTING_INFINIBAND_TRANSPORT_MODE, "connected",
diff --git a/shared/nm-libnm-core-utils.h b/shared/nm-libnm-core-utils.h
index f55d6b3cc0..35d6c5ad5b 100644
--- a/shared/nm-libnm-core-utils.h
+++ b/shared/nm-libnm-core-utils.h
@@ -94,4 +94,20 @@ nm_setting_ip_config_get_addr_family (NMSettingIPConfig *s_ip)
 	g_return_val_if_reached (AF_UNSPEC);
 }
 
+/*****************************************************************************/
+
+/* The maximum MTU for infiniband.
+ *
+ * This is both in transport-mode "datagram" and "connected"
+ * and they both have the same maximum define.
+ *
+ * Note that in the past, MTU in "datagram" mode was restricted
+ * to 2044 bytes. That is no longer the case and we accept large
+ * MTUs.
+ *
+ * This define is the maxiumum for the MTU in a connection profile (the
+ * setting). Whether large MTUs can be configured later (at activation time)
+ * depends on other factors. */
+#define NM_INFINIBAND_MAX_MTU ((guint) 65520)
+
 #endif /* __NM_LIBNM_SHARED_UTILS_H__ */