libnm-core: add normalize of MTU for NMSettingInfiniband

Previously, NMSettingInfiniband:verify() silently modifies the
setting for invalid MTU. verify() should not do that.

For libnm-core we can change behavior and implement normalization
of MTU. This changes behavior for NMSettingInfiniband:verify() so
that MTU gets no longer fixed by verify() alone. Instead verify()
fails with a verification error.

Due the possibility to normalize the MTU, NM still can receive
invalid settings and fix it.

For libnm-core we don't change behavior, merely add a code comment.

Signed-off-by: Thomas Haller <thaller@redhat.com>

4 files changed, 86 insertions, 2 deletions

diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c
index 24b1cd47f6..8ff956e298 100644
--- a/libnm-core/nm-connection.c
+++ b/libnm-core/nm-connection.c
@@ -710,6 +710,30 @@ _normalize_ip_config (NMConnection *self, GHashTable *parameters)
 	}
 }
 
+static gboolean
+_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, max_mtu, NULL);
+				return TRUE;
+			}
+		}
+	}
+	return FALSE;
+}
+
 /**
  * nm_connection_verify:
  * @connection: the #NMConnection to verify
@@ -924,6 +948,7 @@ nm_connection_normalize (NMConnection *connection,
 	was_modified |= _normalize_virtual_iface_name (connection);
 	was_modified |= _normalize_connection_slave_type (connection);
 	was_modified |= _normalize_ip_config (connection, parameters);
+	was_modified |= _normalize_infiniband_mtu (connection, parameters);
 
 	/* Verify anew. */
 	success = _nm_connection_verify (connection, error);
diff --git a/libnm-core/nm-setting-infiniband.c b/libnm-core/nm-setting-infiniband.c
index 57708460ba..d60164ad85 100644
--- a/libnm-core/nm-setting-infiniband.c
+++ b/libnm-core/nm-setting-infiniband.c
@@ -194,6 +194,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 {
 	NMSettingConnection *s_con;
 	NMSettingInfinibandPrivate *priv = NM_SETTING_INFINIBAND_GET_PRIVATE (setting);
+	guint32 normerr_max_mtu = 0;
 
 	if (priv->mac_address && priv->mac_address->len != INFINIBAND_ALEN) {
 		g_set_error_literal (error,
@@ -206,10 +207,10 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 
 	if (!g_strcmp0 (priv->transport_mode, "datagram")) {
 		if (priv->mtu > 2044)
-			priv->mtu = 2044;
+			normerr_max_mtu = 2044;
 	} else if (!g_strcmp0 (priv->transport_mode, "connected")) {
 		if (priv->mtu > 65520)
-			priv->mtu = 65520;
+			normerr_max_mtu = 65520;
 	} else {
 		g_set_error_literal (error,
 		                     NM_SETTING_INFINIBAND_ERROR,
@@ -287,6 +288,18 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 		}
 	}
 
+	/* *** errors above here should be always fatal, below NORMALIZABLE_ERROR *** */
+
+	if (normerr_max_mtu > 0) {
+		g_set_error (error,
+		             NM_SETTING_INFINIBAND_ERROR,
+		             NM_SETTING_INFINIBAND_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);
+		g_prefix_error (error, "%s.%s: ", NM_SETTING_INFINIBAND_SETTING_NAME, NM_SETTING_INFINIBAND_MTU);
+		return NM_SETTING_VERIFY_NORMALIZABLE_ERROR;
+	}
+
 	return TRUE;
 }
 
diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c
index e620e1de75..8e01677294 100644
--- a/libnm-core/tests/test-general.c
+++ b/libnm-core/tests/test-general.c
@@ -3059,6 +3059,50 @@ test_connection_normalize_slave_type_2 (void)
 	g_assert_cmpstr (nm_setting_connection_get_slave_type (s_con), ==, NM_SETTING_BRIDGE_SETTING_NAME);
 }
 
+static void
+test_connection_normalize_infiniband_mtu (void)
+{
+	gs_unref_object NMConnection *con = NULL;
+	NMSettingInfiniband *s_infini;
+
+	con = nmtst_create_minimal_connection ("test_connection_normalize_infiniband_mtu", NULL,
+	                                       NM_SETTING_INFINIBAND_SETTING_NAME, NULL);
+
+	s_infini = nm_connection_get_setting_infiniband (con);
+	g_object_set (s_infini,
+	              NM_SETTING_INFINIBAND_TRANSPORT_MODE, "connected",
+	              NULL);
+	nmtst_assert_connection_verifies_and_normalizable (con);
+
+	g_object_set (s_infini,
+	              NM_SETTING_INFINIBAND_TRANSPORT_MODE, "datagram",
+	              NM_SETTING_INFINIBAND_MTU, (guint) 2044,
+	              NULL);
+	nmtst_assert_connection_verifies_without_normalization (con);
+	g_assert_cmpint (2044, ==, nm_setting_infiniband_get_mtu (s_infini));
+
+	g_object_set (s_infini,
+	              NM_SETTING_INFINIBAND_TRANSPORT_MODE, "datagram",
+	              NM_SETTING_INFINIBAND_MTU, (guint) 2045,
+	              NULL);
+	nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_INFINIBAND_ERROR, NM_SETTING_INFINIBAND_ERROR_INVALID_PROPERTY);
+	g_assert_cmpint (2044, ==, nm_setting_infiniband_get_mtu (s_infini));
+
+	g_object_set (s_infini,
+	              NM_SETTING_INFINIBAND_TRANSPORT_MODE, "connected",
+	              NM_SETTING_INFINIBAND_MTU, (guint) 65520,
+	              NULL);
+	nmtst_assert_connection_verifies_without_normalization (con);
+	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) 65521,
+	              NULL);
+	nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_INFINIBAND_ERROR, NM_SETTING_INFINIBAND_ERROR_INVALID_PROPERTY);
+	g_assert_cmpint (65520, ==, nm_setting_infiniband_get_mtu (s_infini));
+}
+
 NMTST_DEFINE ();
 
 int main (int argc, char **argv)
@@ -3103,6 +3147,7 @@ int main (int argc, char **argv)
 	g_test_add_func ("/core/general/test_connection_normalize_type", test_connection_normalize_type);
 	g_test_add_func ("/core/general/test_connection_normalize_slave_type_1", test_connection_normalize_slave_type_1);
 	g_test_add_func ("/core/general/test_connection_normalize_slave_type_2", test_connection_normalize_slave_type_2);
+	g_test_add_func ("/core/general/test_connection_normalize_infiniband_mtu", test_connection_normalize_infiniband_mtu);
 
 	g_test_add_func ("/core/general/test_setting_connection_permissions_helpers", test_setting_connection_permissions_helpers);
 	g_test_add_func ("/core/general/test_setting_connection_permissions_property", test_setting_connection_permissions_property);
diff --git a/libnm-util/nm-setting-infiniband.c b/libnm-util/nm-setting-infiniband.c
index 4e470e561b..a9088d20cb 100644
--- a/libnm-util/nm-setting-infiniband.c
+++ b/libnm-util/nm-setting-infiniband.c
@@ -206,6 +206,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 		return FALSE;
 	}
 
+	/* FIXME: verify() should not modify the setting, but return NORMALIZABLE success. */
 	if (!g_strcmp0 (priv->transport_mode, "datagram")) {
 		if (priv->mtu > 2044)
 			priv->mtu = 2044;