device: add "dhcp-plugin" match spec for device

The need for this is the following:

"ipv4.dhcp-client-id" can be specified via global connection defaults.
In absence of any configuration in NetworkManager, the default depends
on the DHCP client plugin. In case of "dhclient", the default further
depends on /etc/dhcp.

For "internal" plugin, we may very well want to change the default
client-id to "mac" by universally installing a configuration
snippet

    [connection-use-mac-client-id]
    ipv4.dhcp-client-id=mac

However, if we the user happens to enable "dhclient" plugin, this also
forces the client-id and overrules configuration from /etc/dhcp. The real
problem is, that dhclient can be configured via means outside of NetworkManager,
so our defaults shall not overwrite defaults from /etc/dhcp.

With the new device spec, we can avoid this issue:

    [connection-dhcp-client-id]
    match-device=except:dhcp-plugin:dhclient
    ipv4.dhcp-client-id=mac

This will be part of the solution for rh#1640494. Note that merely
dropping a configuration snippet is not yet enough. More fixes for
DHCP will follow. Also, bug rh#1640494 may have alternative solutions
as well. The nice part of this new feature is that it is generally
useful for configuring connection defaults and not specifically for
the client-id issue.

Note that this match spec is per-device, although the plugin is selected
globally. That makes some sense, because in the future we may or may not
configure the DHCP plugin per-device.

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

12 files changed, 68 insertions, 7 deletions

diff --git a/man/NetworkManager.conf.xml b/man/NetworkManager.conf.xml
index 8773e6e916..b277db0d4e 100644
--- a/man/NetworkManager.conf.xml
+++ b/man/NetworkManager.conf.xml
@@ -1366,6 +1366,11 @@ enable=nm-version-min:1.3,nm-version-min:1.2.6,nm-version-min:1.0.16
           </para></listitem>
         </varlistentry>
         <varlistentry>
+          <term>dhcp-plugin:DHCP</term>
+          <listitem><para>Match the configured DHCP plugin "<literal>main.dhcp</literal>".
+          </para></listitem>
+        </varlistentry>
+        <varlistentry>
           <term>except:SPEC</term>
           <listitem><para>Negative match of a device. <literal>SPEC</literal> must be explicitly qualified with
              a prefix such as <literal>interface-name:</literal>. A negative match has higher priority then the positive
diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index ae17ee5179..3ac6077642 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -882,6 +882,7 @@ nm_utils_match_connection (NMConnection *const*connections,
 int
 nm_match_spec_device_by_pllink (const NMPlatformLink *pllink,
                                 const char *match_device_type,
+                                const char *match_dhcp_plugin,
                                 const GSList *specs,
                                 int no_match_value)
 {
@@ -898,7 +899,8 @@ nm_match_spec_device_by_pllink (const NMPlatformLink *pllink,
 	                          pllink ? pllink->driver : NULL,
 	                          NULL,
 	                          NULL,
-	                          NULL);
+	                          NULL,
+	                          match_dhcp_plugin);
 
 	switch (m) {
 	case NM_MATCH_SPEC_MATCH:
diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h
index 93edb912dd..e39df8676d 100644
--- a/src/NetworkManagerUtils.h
+++ b/src/NetworkManagerUtils.h
@@ -50,6 +50,7 @@ NMConnection *nm_utils_match_connection (NMConnection *const*connections,
 
 int nm_match_spec_device_by_pllink (const NMPlatformLink *pllink,
                                     const char *match_device_type,
+                                    const char *match_dhcp_plugin,
                                     const GSList *specs,
                                     int no_match_value);
 
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 14015d9da3..c4c3365901 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -15776,6 +15776,7 @@ nm_device_spec_match_list_full (NMDevice *self, const GSList *specs, int no_matc
 	                          nm_device_get_driver (self),
 	                          nm_device_get_driver_version (self),
 	                          nm_device_get_permanent_hw_address (self),
+	                          nm_dhcp_manager_get_config (nm_dhcp_manager_get ()),
 	                          klass->get_s390_subchannels ? klass->get_s390_subchannels (self) : NULL);
 
 	switch (m) {
diff --git a/src/dhcp/nm-dhcp-manager.c b/src/dhcp/nm-dhcp-manager.c
index 5ae16d721a..372f241d47 100644
--- a/src/dhcp/nm-dhcp-manager.c
+++ b/src/dhcp/nm-dhcp-manager.c
@@ -384,6 +384,12 @@ nm_dhcp_manager_get_config (NMDhcpManager *self)
 
 NM_DEFINE_SINGLETON_GETTER (NMDhcpManager, nm_dhcp_manager_get, NM_TYPE_DHCP_MANAGER);
 
+void
+nmtst_dhcp_manager_unget (gpointer self)
+{
+	_nmtst_nm_dhcp_manager_get_reset (self);
+}
+
 static void
 nm_dhcp_manager_init (NMDhcpManager *self)
 {
@@ -446,6 +452,10 @@ nm_dhcp_manager_init (NMDhcpManager *self)
 
 	nm_log_info (LOGD_DHCP, "dhcp-init: Using DHCP client '%s'", client_factory->name);
 
+	/* NOTE: currently the DHCP plugin is chosen once at start. It's not
+	 * possible to reload that configuration. If that ever becomes possible,
+	 * beware that the "dhcp-plugin" device spec made decisions based on
+	 * the previous plugin and may need reevaluation. */
 	priv->client_factory = client_factory;
 }
 
diff --git a/src/dhcp/nm-dhcp-manager.h b/src/dhcp/nm-dhcp-manager.h
index 1d9e5c21dc..f8f39e53e5 100644
--- a/src/dhcp/nm-dhcp-manager.h
+++ b/src/dhcp/nm-dhcp-manager.h
@@ -87,4 +87,6 @@ extern const char* nm_dhcp_helper_path;
 
 extern const NMDhcpClientFactory *const _nm_dhcp_manager_factories[4];
 
+void nmtst_dhcp_manager_unget (gpointer singleton_instance);
+
 #endif /* __NETWORKMANAGER_DHCP_MANAGER_H__ */
diff --git a/src/nm-config-data.c b/src/nm-config-data.c
index c37e36b421..bff4ed3c8a 100644
--- a/src/nm-config-data.c
+++ b/src/nm-config-data.c
@@ -1250,9 +1250,13 @@ _match_section_infos_lookup (const MatchSectionInfo *match_section_infos,
                              const char *match_device_type,
                              char **out_value)
 {
+	const char *match_dhcp_plugin;
+
 	if (!match_section_infos)
 		return NULL;
 
+	match_dhcp_plugin = nm_dhcp_manager_get_config (nm_dhcp_manager_get ());
+
 	for (; match_section_infos->group_name; match_section_infos++) {
 		char *value = NULL;
 		gboolean match;
@@ -1272,7 +1276,7 @@ _match_section_infos_lookup (const MatchSectionInfo *match_section_infos,
 			if (device)
 				match = nm_device_spec_match_list (device, match_section_infos->match_device.spec);
 			else if (pllink)
-				match = nm_match_spec_device_by_pllink (pllink, match_device_type, match_section_infos->match_device.spec, FALSE);
+				match = nm_match_spec_device_by_pllink (pllink, match_device_type, match_dhcp_plugin, match_section_infos->match_device.spec, FALSE);
 			else
 				match = FALSE;
 		} else
diff --git a/src/nm-config-data.h b/src/nm-config-data.h
index 9d1e20a6a2..64a35a48e8 100644
--- a/src/nm-config-data.h
+++ b/src/nm-config-data.h
@@ -233,5 +233,14 @@ GKeyFile *_nm_config_data_get_keyfile (const NMConfigData *self);
 GKeyFile *_nm_config_data_get_keyfile_user (const NMConfigData *self);
 GKeyFile *_nm_config_data_get_keyfile_intern (const NMConfigData *self);
 
+/*****************************************************************************/
+
+/* nm-config-data.c requires getting the DHCP manager's configuration. That is a bit
+ * ugly, and optimally, NMConfig* is independent of NMDhcpManager. Instead of
+ * including the header, forward declare the two functions that we need. */
+struct _NMDhcpManager;
+struct _NMDhcpManager *nm_dhcp_manager_get (void);
+const char *nm_dhcp_manager_get_config (struct _NMDhcpManager *self);
+
 #endif /* NM_CONFIG_DATA_H */
 
diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index 75dd2e2c22..a844449e29 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -1141,6 +1141,7 @@ nm_utils_read_link_absolute (const char *link_file, GError **error)
 #define DEVICE_TYPE_TAG "type:"
 #define DRIVER_TAG "driver:"
 #define SUBCHAN_TAG "s390-subchannels:"
+#define DHCP_PLUGIN_TAG "dhcp-plugin:"
 #define EXCEPT_TAG "except:"
 #define MATCH_TAG_CONFIG_NM_VERSION             "nm-version:"
 #define MATCH_TAG_CONFIG_NM_VERSION_MIN         "nm-version-min:"
@@ -1152,6 +1153,7 @@ typedef struct {
 	const char *device_type;
 	const char *driver;
 	const char *driver_version;
+	const char *dhcp_plugin;
 	struct {
 		const char *value;
 		gboolean is_parsed;
@@ -1369,6 +1371,9 @@ match_device_eval (const char *spec_str,
 	if (_MATCH_CHECK (spec_str, SUBCHAN_TAG))
 		return match_data_s390_subchannels_eval (spec_str, match_data);
 
+	if (_MATCH_CHECK (spec_str, DHCP_PLUGIN_TAG))
+		return nm_streq0 (spec_str, match_data->dhcp_plugin);
+
 	if (allow_fuzzy) {
 		if (match_device_hwaddr_eval (spec_str, match_data))
 			return TRUE;
@@ -1387,7 +1392,8 @@ nm_match_spec_device (const GSList *specs,
                       const char *driver,
                       const char *driver_version,
                       const char *hwaddr,
-                      const char *s390_subchannels)
+                      const char *s390_subchannels,
+                      const char *dhcp_plugin)
 {
 	const GSList *iter;
 	NMMatchSpecMatchType match;
@@ -1398,6 +1404,7 @@ nm_match_spec_device (const GSList *specs,
 	    .device_type = nm_str_not_empty (device_type),
 	    .driver = nm_str_not_empty (driver),
 	    .driver_version = nm_str_not_empty (driver_version),
+	    .dhcp_plugin = nm_str_not_empty (dhcp_plugin),
 	    .hwaddr = {
 	        .value = hwaddr,
 	    },
diff --git a/src/nm-core-utils.h b/src/nm-core-utils.h
index 79bef0a870..af651a2afb 100644
--- a/src/nm-core-utils.h
+++ b/src/nm-core-utils.h
@@ -225,7 +225,8 @@ NMMatchSpecMatchType nm_match_spec_device (const GSList *specs,
                                            const char *driver,
                                            const char *driver_version,
                                            const char *hwaddr,
-                                           const char *s390_subchannels);
+                                           const char *s390_subchannels,
+                                           const char *dhcp_plugin);
 NMMatchSpecMatchType nm_match_spec_config (const GSList *specs,
                                            guint nm_version,
                                            const char *env);
diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c
index 75fef4fad1..3050e6fa52 100644
--- a/src/tests/config/test-config.c
+++ b/src/tests/config/test-config.c
@@ -25,6 +25,7 @@
 #include "nm-config.h"
 #include "nm-test-device.h"
 #include "platform/nm-fake-platform.h"
+#include "dhcp/nm-dhcp-manager.h"
 #include "nm-dbus-manager.h"
 #include "nm-connectivity.h"
 
@@ -123,6 +124,24 @@ setup_config (GError **error, const char *config_file, const char *intern_config
 		g_assert_no_error (local_error);
 	}
 	nm_config_cmd_line_options_free (cli);
+
+	if (config) {
+		NMDhcpManager *dhcp_manager;
+		gpointer logging_old_state;
+
+		logging_old_state = nmtst_logging_disable ();
+
+		dhcp_manager = nm_dhcp_manager_get ();
+		g_test_assert_expected_messages ();
+
+		nmtst_logging_reenable (logging_old_state);
+
+		g_object_set_data_full (G_OBJECT (config),
+		                        "nmtst-config-keep-dhcp-manager-alive",
+		                        dhcp_manager,
+		                        nmtst_dhcp_manager_unget);
+	}
+
 	return config;
 }
 
diff --git a/src/tests/test-general.c b/src/tests/test-general.c
index e436b36d3a..51f9fedbd3 100644
--- a/src/tests/test-general.c
+++ b/src/tests/test-general.c
@@ -1093,7 +1093,7 @@ static NMMatchSpecMatchType
 _test_match_spec_device (const GSList *specs, const char *match_str)
 {
 	if (match_str && g_str_has_prefix (match_str, MATCH_S390))
-		return nm_match_spec_device (specs, NULL, NULL, NULL, NULL, NULL, &match_str[NM_STRLEN (MATCH_S390)]);
+		return nm_match_spec_device (specs, NULL, NULL, NULL, NULL, NULL, &match_str[NM_STRLEN (MATCH_S390)], NULL);
 	if (match_str && g_str_has_prefix (match_str, MATCH_DRIVER)) {
 		gs_free char *s = g_strdup (&match_str[NM_STRLEN (MATCH_DRIVER)]);
 		char *t;
@@ -1103,9 +1103,9 @@ _test_match_spec_device (const GSList *specs, const char *match_str)
 			t[0] = '\0';
 			t++;
 		}
-		return nm_match_spec_device (specs, NULL, NULL, s, t, NULL, NULL);
+		return nm_match_spec_device (specs, NULL, NULL, s, t, NULL, NULL, NULL);
 	}
-	return nm_match_spec_device (specs, match_str, NULL, NULL, NULL, NULL, NULL);
+	return nm_match_spec_device (specs, match_str, NULL, NULL, NULL, NULL, NULL, NULL);
 }
 
 static void