dhcp: merge branch 'th/dhcp-factory-cleanup'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/221

10 files changed, 79 insertions, 35 deletions

diff --git a/src/dhcp/nm-dhcp-client.h b/src/dhcp/nm-dhcp-client.h
index 9eb76f33a8..29f83259f8 100644
--- a/src/dhcp/nm-dhcp-client.h
+++ b/src/dhcp/nm-dhcp-client.h
@@ -207,15 +207,20 @@ void nm_dhcp_client_set_client_id_bin (NMDhcpClient *self,
  *****************************************************************************/
 
 typedef struct {
-	GType (*get_type)(void);
+	GType (*get_type) (void);
+	GType (*get_type_per_addr_family) (int addr_family);
 	const char *name;
 	const char *(*get_path) (void);
+	bool experimental:1;
 } NMDhcpClientFactory;
 
+GType nm_dhcp_nettools_get_type (void);
+
 extern const NMDhcpClientFactory _nm_dhcp_client_factory_dhcpcanon;
 extern const NMDhcpClientFactory _nm_dhcp_client_factory_dhclient;
 extern const NMDhcpClientFactory _nm_dhcp_client_factory_dhcpcd;
 extern const NMDhcpClientFactory _nm_dhcp_client_factory_internal;
+extern const NMDhcpClientFactory _nm_dhcp_client_factory_systemd;
 extern const NMDhcpClientFactory _nm_dhcp_client_factory_nettools;
 
 #endif /* __NETWORKMANAGER_DHCP_CLIENT_H__ */
diff --git a/src/dhcp/nm-dhcp-dhclient.c b/src/dhcp/nm-dhcp-dhclient.c
index 54b50479dd..72ea3d48f8 100644
--- a/src/dhcp/nm-dhcp-dhclient.c
+++ b/src/dhcp/nm-dhcp-dhclient.c
@@ -723,7 +723,7 @@ nm_dhcp_dhclient_class_init (NMDhcpDhclientClass *dhclient_class)
 }
 
 const NMDhcpClientFactory _nm_dhcp_client_factory_dhclient = {
-	.name = "dhclient",
+	.name     = "dhclient",
 	.get_type = nm_dhcp_dhclient_get_type,
 	.get_path = nm_dhcp_dhclient_get_path,
 };
diff --git a/src/dhcp/nm-dhcp-dhcpcanon.c b/src/dhcp/nm-dhcp-dhcpcanon.c
index 2d2113cc76..c193e36780 100644
--- a/src/dhcp/nm-dhcp-dhcpcanon.c
+++ b/src/dhcp/nm-dhcp-dhcpcanon.c
@@ -248,7 +248,7 @@ nm_dhcp_dhcpcanon_class_init (NMDhcpDhcpcanonClass *dhcpcanon_class)
 }
 
 const NMDhcpClientFactory _nm_dhcp_client_factory_dhcpcanon = {
-	.name = "dhcpcanon",
+	.name     = "dhcpcanon",
 	.get_type = nm_dhcp_dhcpcanon_get_type,
 	.get_path = nm_dhcp_dhcpcanon_get_path,
 };
diff --git a/src/dhcp/nm-dhcp-dhcpcd.c b/src/dhcp/nm-dhcp-dhcpcd.c
index c300bbe2f0..b07d5a1bba 100644
--- a/src/dhcp/nm-dhcp-dhcpcd.c
+++ b/src/dhcp/nm-dhcp-dhcpcd.c
@@ -242,7 +242,7 @@ nm_dhcp_dhcpcd_class_init (NMDhcpDhcpcdClass *dhcpcd_class)
 }
 
 const NMDhcpClientFactory _nm_dhcp_client_factory_dhcpcd = {
-	.name = "dhcpcd",
+	.name     = "dhcpcd",
 	.get_type = nm_dhcp_dhcpcd_get_type,
 	.get_path = nm_dhcp_dhcpcd_get_path,
 };
diff --git a/src/dhcp/nm-dhcp-listener.c b/src/dhcp/nm-dhcp-listener.c
index 88aafeb0b1..ce86d55622 100644
--- a/src/dhcp/nm-dhcp-listener.c
+++ b/src/dhcp/nm-dhcp-listener.c
@@ -38,7 +38,7 @@
 
 /*****************************************************************************/
 
-const NMDhcpClientFactory *const _nm_dhcp_manager_factories[5] = {
+const NMDhcpClientFactory *const _nm_dhcp_manager_factories[6] = {
 	/* the order here matters, as we will try the plugins in this order to find
 	 * the first available plugin. */
 
@@ -52,6 +52,7 @@ const NMDhcpClientFactory *const _nm_dhcp_manager_factories[5] = {
 	&_nm_dhcp_client_factory_dhcpcd,
 #endif
 	&_nm_dhcp_client_factory_internal,
+	&_nm_dhcp_client_factory_systemd,
 	&_nm_dhcp_client_factory_nettools,
 };
 
diff --git a/src/dhcp/nm-dhcp-manager.c b/src/dhcp/nm-dhcp-manager.c
index 304a7b9961..3c78d7c2dc 100644
--- a/src/dhcp/nm-dhcp-manager.c
+++ b/src/dhcp/nm-dhcp-manager.c
@@ -89,10 +89,11 @@ _client_factory_available (const NMDhcpClientFactory *client_factory)
 	return NULL;
 }
 
-static const NMDhcpClientFactory *
-_client_factory_get_effective (const NMDhcpClientFactory *client_factory,
-                               int addr_family)
+static GType
+_client_factory_get_gtype (const NMDhcpClientFactory *client_factory,
+                           int addr_family)
 {
+	GType gtype;
 	nm_auto_unref_gtypeclass NMDhcpClientClass *klass = NULL;
 
 	nm_assert (client_factory);
@@ -118,23 +119,38 @@ _client_factory_get_effective (const NMDhcpClientFactory *client_factory,
 	 * to those plugins. But we don't intend to do so. The internal plugin is the way forward and
 	 * not extending other plugins. */
 
+	if (client_factory->get_type_per_addr_family)
+		gtype = client_factory->get_type_per_addr_family (addr_family);
+	else
+		gtype = client_factory->get_type ();
+
 	if (client_factory == &_nm_dhcp_client_factory_internal) {
-		/* already using internal plugin. Nothing to do. */
-		return client_factory;
+		/* we are already using the internal plugin. Nothing to do. */
+		goto out;
 	}
 
-	klass = g_type_class_ref (client_factory->get_type ());
+	klass = g_type_class_ref (gtype);
 
 	nm_assert (NM_IS_DHCP_CLIENT_CLASS (klass));
 
 	if (addr_family == AF_INET6) {
-		return   klass->ip6_start
-		       ? client_factory
-		       : &_nm_dhcp_client_factory_internal;
+		if (!klass->ip6_start)
+			gtype = _client_factory_get_gtype (&_nm_dhcp_client_factory_internal, addr_family);
+	} else {
+		if (!klass->ip4_start)
+			gtype = _client_factory_get_gtype (&_nm_dhcp_client_factory_internal, addr_family);
 	}
-	return   klass->ip4_start
-	       ? client_factory
-	       : &_nm_dhcp_client_factory_internal;
+
+out:
+	nm_assert (g_type_is_a (gtype, NM_TYPE_DHCP_CLIENT));
+	nm_assert (({
+	               nm_auto_unref_gtypeclass NMDhcpClientClass *k = g_type_class_ref (gtype);
+
+	                  (addr_family == AF_INET6 && k->ip6_start)
+	               || (addr_family == AF_INET  && k->ip4_start);
+	            }));
+
+	return gtype;
 }
 
 /*****************************************************************************/
@@ -225,7 +241,7 @@ client_start (NMDhcpManager *self,
 	NMDhcpClient *client;
 	gboolean success = FALSE;
 	gsize hwaddr_len;
-	const NMDhcpClientFactory *client_factory;
+	GType gtype;
 
 	g_return_val_if_fail (NM_IS_DHCP_MANAGER (self), NULL);
 	g_return_val_if_fail (iface, NULL);
@@ -255,8 +271,6 @@ client_start (NMDhcpManager *self,
 
 	priv = NM_DHCP_MANAGER_GET_PRIVATE (self);
 
-	client_factory = _client_factory_get_effective (priv->client_factory, addr_family);
-
 	/* Kill any old client instance */
 	client = get_client_for_ifindex (self, addr_family, ifindex);
 	if (client) {
@@ -271,7 +285,14 @@ client_start (NMDhcpManager *self,
 		g_object_unref (client);
 	}
 
-	client = g_object_new (client_factory->get_type (),
+	gtype = _client_factory_get_gtype (priv->client_factory, addr_family);
+
+	nm_log_trace (LOGD_DHCP , "dhcp%c: creating IPv%c DHCP client of type %s",
+	              nm_utils_addr_family_to_char (addr_family),
+	              nm_utils_addr_family_to_char (addr_family),
+	              g_type_name (gtype));
+
+	client = g_object_new (gtype,
 	                       NM_DHCP_CLIENT_MULTI_IDX, multi_idx,
 	                       NM_DHCP_CLIENT_ADDR_FAMILY, addr_family,
 	                       NM_DHCP_CLIENT_INTERFACE, iface,
@@ -529,9 +550,10 @@ nm_dhcp_manager_init (NMDhcpManager *self)
 		if (!f)
 			continue;
 
-		nm_log_dbg (LOGD_DHCP, "dhcp-init: enabled DHCP client '%s' (%s)%s",
-		            f->name, g_type_name (f->get_type ()),
-		            _client_factory_available (f) ? "" : " (not available)");
+		nm_log_dbg (LOGD_DHCP, "dhcp-init: enabled DHCP client '%s'%s%s",
+		            f->name,
+		            _client_factory_available (f) ? "" : " (not available)",
+		            f->experimental ? " (undocumented internal plugin)" : "");
 	}
 
 	/* Client-specific setup */
diff --git a/src/dhcp/nm-dhcp-manager.h b/src/dhcp/nm-dhcp-manager.h
index ff0d6f5472..1e0a972b1f 100644
--- a/src/dhcp/nm-dhcp-manager.h
+++ b/src/dhcp/nm-dhcp-manager.h
@@ -86,7 +86,7 @@ NMDhcpClient * nm_dhcp_manager_start_ip6     (NMDhcpManager *manager,
 /* For testing only */
 extern const char* nm_dhcp_helper_path;
 
-extern const NMDhcpClientFactory *const _nm_dhcp_manager_factories[5];
+extern const NMDhcpClientFactory *const _nm_dhcp_manager_factories[6];
 
 void nmtst_dhcp_manager_unget (gpointer singleton_instance);
 
diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c
index 3aa921807b..5191f5a4e9 100644
--- a/src/dhcp/nm-dhcp-nettools.c
+++ b/src/dhcp/nm-dhcp-nettools.c
@@ -55,8 +55,6 @@
 typedef struct _NMDhcpNettools NMDhcpNettools;
 typedef struct _NMDhcpNettoolsClass NMDhcpNettoolsClass;
 
-static GType nm_dhcp_nettools_get_type (void);
-
 /*****************************************************************************/
 
 typedef struct {
@@ -1272,7 +1270,7 @@ nm_dhcp_nettools_class_init (NMDhcpNettoolsClass *class)
 }
 
 const NMDhcpClientFactory _nm_dhcp_client_factory_nettools = {
-	.name = "nettools",
-	.get_type = nm_dhcp_nettools_get_type,
-	.get_path = NULL,
+	.name         = "nettools",
+	.get_type     = nm_dhcp_nettools_get_type,
+	.experimental = TRUE,
 };
diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c
index 073a6da051..f0084f1b86 100644
--- a/src/dhcp/nm-dhcp-systemd.c
+++ b/src/dhcp/nm-dhcp-systemd.c
@@ -1173,8 +1173,25 @@ nm_dhcp_systemd_class_init (NMDhcpSystemdClass *sdhcp_class)
 	client_class->stop = stop;
 }
 
+const NMDhcpClientFactory _nm_dhcp_client_factory_systemd = {
+	.name         = "systemd",
+	.get_type     = nm_dhcp_systemd_get_type,
+	.experimental = TRUE,
+};
+
+/*****************************************************************************/
+
+static GType
+_get_type_per_addr_family (int addr_family)
+{
+	nm_assert_addr_family (addr_family);
+
+	if (FALSE && addr_family == AF_INET)
+		return nm_dhcp_nettools_get_type ();
+	return nm_dhcp_systemd_get_type ();
+}
+
 const NMDhcpClientFactory _nm_dhcp_client_factory_internal = {
-	.name = "internal",
-	.get_type = nm_dhcp_systemd_get_type,
-	.get_path = NULL,
+	.name                     = "internal",
+	.get_type_per_addr_family = _get_type_per_addr_family,
 };
diff --git a/src/nm-iface-helper.c b/src/nm-iface-helper.c
index dd5bb327a0..239b46a140 100644
--- a/src/nm-iface-helper.c
+++ b/src/nm-iface-helper.c
@@ -596,9 +596,10 @@ main (int argc, char *argv[])
 
 /*****************************************************************************/
 
-const NMDhcpClientFactory *const _nm_dhcp_manager_factories[5] = {
+const NMDhcpClientFactory *const _nm_dhcp_manager_factories[6] = {
+	/* For nm-iface-helper there is no option to choose a DHCP plugin.
+	 * It just uses the "internal" one. */
 	&_nm_dhcp_client_factory_internal,
-	&_nm_dhcp_client_factory_nettools,
 };
 
 /*****************************************************************************/