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 or per address family.

https://bugzilla.redhat.com/show_bug.cgi?id=1640494
This commit is contained in:
Thomas Haller 2018-10-24 08:43:45 +02:00
parent 35cecd32fd
commit b9eb264efe
12 changed files with 68 additions and 7 deletions

View file

@ -1365,6 +1365,11 @@ enable=nm-version-min:1.3,nm-version-min:1.2.6,nm-version-min:1.0.16
Optionally, a driver version may be specified separated by '/'. Globbing is supported for the version.
</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

View file

@ -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:

View file

@ -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);

View file

@ -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) {

View file

@ -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;
}

View file

@ -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__ */

View file

@ -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

View file

@ -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 */

View file

@ -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,
},

View file

@ -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);

View file

@ -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 (FALSE);
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;
}

View file

@ -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