libnm: validate "connection.interface-name" at one place only

Don't spread the validation for the interface name between multiple
places. There should be one place only, so when you search for how
this property gets verified, you can find the single place.

That requires to move the special handling for OVS interfaces to
NMSettingConnection.
Since we already have _nm_setting_ovs_interface_verify_interface_type(),
that is easy.

2 files changed, 48 insertions, 46 deletions

diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c
index 6ea3c5fb29..2e8fa37a2c 100644
--- a/libnm-core/nm-setting-connection.c
+++ b/libnm-core/nm-setting-connection.c
@@ -1032,22 +1032,50 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 
 	if (priv->interface_name) {
 		GError *tmp_error = NULL;
-		gboolean valid_ifname = FALSE;
+		NMUtilsIfaceType iface_type;
 
-		/* do not perform a interface name length check for OVS connection types
-		 * as they don't have a corresponding kernel link that enforces the 15 bytes limit.
-		 * Here we're whitelisting the OVS interface type as well, even if most OVS
-		 * iface types do have the limit, to let the OVS specific nm-setting verify whether the iface name
-		 * is good or not according to the internal type (internal, patch, ...) */
 		if (NM_IN_STRSET (type,
 		                  NM_SETTING_OVS_BRIDGE_SETTING_NAME,
-		                  NM_SETTING_OVS_PORT_SETTING_NAME,
-		                  NM_SETTING_OVS_INTERFACE_SETTING_NAME))
-			valid_ifname = nm_utils_ifname_valid (priv->interface_name, NMU_IFACE_OVS, &tmp_error);
-		else
-			valid_ifname = nm_utils_ifname_valid (priv->interface_name, NMU_IFACE_KERNEL, &tmp_error);
+		                  NM_SETTING_OVS_PORT_SETTING_NAME))
+			iface_type = NMU_IFACE_OVS;
+		else if (nm_streq (type, NM_SETTING_OVS_INTERFACE_SETTING_NAME)) {
+			NMSettingOvsInterface *s_ovs_iface = NULL;
+			const char *ovs_iface_type;
+
+			if (connection)
+				s_ovs_iface = nm_connection_get_setting_ovs_interface (connection);
+			_nm_setting_ovs_interface_verify_interface_type (s_ovs_iface,
+			                                                 s_ovs_iface ? nm_setting_ovs_interface_get_interface_type (s_ovs_iface) : NULL,
+			                                                 connection,
+			                                                 FALSE,
+			                                                 NULL,
+			                                                 &ovs_iface_type,
+			                                                 NULL);
+			if (!ovs_iface_type) {
+				/* We cannot determine to OVS interface type. Consequently, we cannot
+				 * fully validate the interface name.
+				 *
+				 * If we have a connection (and we do a full validation anyway), skip the
+				 * check. The connection will fail validation when we validate the OVS setting.
+				 *
+				 * Otherwise, do the most basic validation.
+				 */
+				if (connection)
+					goto after_interface_name;
+				iface_type = NMU_IFACE_ANY;
+			} else if (NM_IN_STRSET (ovs_iface_type, "patch")) {
+				/* this interface type is internal to OVS. */
+				iface_type = NMU_IFACE_OVS;
+			} else {
+				/* This interface type also requires a netdev. We need to validate
+				 * for both OVS and KERNEL. */
+				nm_assert (NM_IN_STRSET (ovs_iface_type, "internal", "system", "dpdk"));
+				iface_type = NMU_IFACE_OVS_AND_KERNEL;
+			}
+		} else
+			iface_type = NMU_IFACE_KERNEL;
 
-		if (!valid_ifname) {
+		if (!nm_utils_ifname_valid (priv->interface_name, iface_type, &tmp_error)) {
 			g_set_error (error,
 			             NM_CONNECTION_ERROR,
 			             NM_CONNECTION_ERROR_INVALID_PROPERTY,
@@ -1057,6 +1085,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 			return FALSE;
 		}
 	}
+after_interface_name:
 
 	is_slave = FALSE;
 	slave_setting_type = NULL;
diff --git a/libnm-core/nm-setting-ovs-interface.c b/libnm-core/nm-setting-ovs-interface.c
index 114e0205d3..b9a133d042 100644
--- a/libnm-core/nm-setting-ovs-interface.c
+++ b/libnm-core/nm-setting-ovs-interface.c
@@ -257,8 +257,6 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 {
 	NMSettingOvsInterface *self = NM_SETTING_OVS_INTERFACE (setting);
 	NMSettingConnection *s_con = NULL;
-	const char *normalized_type = NULL;
-	int result = NM_SETTING_VERIFY_ERROR;
 
 	if (connection) {
 		const char *slave_type;
@@ -298,38 +296,13 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		}
 	}
 
-	result = _nm_setting_ovs_interface_verify_interface_type (self,
-	                                                          self->type,
-	                                                          connection,
-	                                                          FALSE,
-	                                                          NULL,
-	                                                          &normalized_type,
-	                                                          error);
-
-	/* From 'man ovs-vswitchd.conf.db': OVS patch interfaces do not have
-	 * a limit on interface name length, all the other types do */
-	if (result != NM_SETTING_VERIFY_ERROR && s_con) {
-		gs_free_error GError *ifname_error = NULL;
-		const char *ifname = nm_setting_connection_get_interface_name (s_con);
-
-		if (   ifname
-		    && !nm_streq0 (normalized_type, "patch")
-		    && !nm_utils_ifname_valid (ifname,
-		                               NMU_IFACE_KERNEL,
-		                               &ifname_error)) {
-			g_clear_error (error);
-			g_set_error (error,
-			             NM_CONNECTION_ERROR,
-			             NM_CONNECTION_ERROR_INVALID_PROPERTY,
-			             "'%s': %s", ifname, ifname_error->message);
-			g_prefix_error (error, "%s.%s: ",
-			                NM_SETTING_CONNECTION_SETTING_NAME,
-			                NM_SETTING_CONNECTION_INTERFACE_NAME);
-			return NM_SETTING_VERIFY_ERROR;
-		}
-	}
-
-	return result;
+	return _nm_setting_ovs_interface_verify_interface_type (self,
+	                                                        self->type,
+	                                                        connection,
+	                                                        FALSE,
+	                                                        NULL,
+	                                                        NULL,
+	                                                        error);
 }
 
 /*****************************************************************************/