settings/ifupdown: in plugin drop listening to udev for devices

Don't listen to udev to find out about devices. First of all, using udev
for this is already very wrong, because we have the platform cache.

Anyway, all that the device information is used, is pointless as well.
Drop it.

It's pointless because:

The entires in eni_ifaces are already indexed by the interface name.
Likewise, all NMIfupdownConnection set "connection.interface-name" to
restict the profile by name.
/e/n/i matches devices is by name, that's it. We don't need udev to
look up the MAC address (by name!!) to later ignore/match devices
by MAC address. Especially, because NetworkMaanger can already
restrict profiles to devices based on the interface name.
Likewise, NetworkMaanger can use the interface name for the
unmanaged-specs.
It's wrong to extend the on-disk configuration from /e/n/i with runtime
information from udev, especially, because other NetworkMaanger layers
are perfectly content using the interface name for this purpose.

Also, bind_device_to_connection() was fundamentally wrong. It's wrong
to modify the settings connection at random moments (on udev event).
If at all, that should only happen during connection load/reload.

This may have been necessary a long time ago, when unmanaged devices were
not expressed by device-match-specs, but by the HAL UDI. That was since
improved, for example by commit c9067d8fedf6f6f2d530fd68bbfca7ce68638d38.

1 files changed, 4 insertions, 192 deletions

diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c
index 92ab732e40..07bf7ae985 100644
--- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c
+++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c
@@ -29,7 +29,6 @@
 #include <string.h>
 #include <arpa/inet.h>
 #include <gmodule.h>
-#include <libudev.h>
 
 #include "nm-setting-connection.h"
 #include "nm-dbus-interface.h"
@@ -42,7 +41,6 @@
 #include "nm-core-internal.h"
 #include "NetworkManagerUtils.h"
 #include "nm-config.h"
-#include "nm-utils/nm-udev-utils.h"
 
 #include "nms-ifupdown-interface-parser.h"
 #include "nms-ifupdown-connection.h"
@@ -55,16 +53,11 @@
 /*****************************************************************************/
 
 typedef struct {
-	NMUdevClient *udev_client;
-
 	/* Stores an entry for blocks/interfaces read from /e/n/i and (if exists)
 	 * the NMIfupdownConnection associated with the block.
 	 */
 	GHashTable *eni_ifaces;
 
-	/* Stores any network interfaces the kernel knows about */
-	GHashTable *kernel_ifaces;
-
 	bool ifupdown_managed;
 } SettingsPluginIfupdownPrivate;
 
@@ -98,148 +91,6 @@ NM_DEFINE_SINGLETON_GETTER (SettingsPluginIfupdown, settings_plugin_ifupdown_get
 
 /*****************************************************************************/
 
-static void
-bind_device_to_connection (SettingsPluginIfupdown *self,
-                           struct udev_device *device,
-                           NMIfupdownConnection *conn)
-{
-	NMSettingWired *s_wired;
-	NMSettingWireless *s_wifi;
-	const char *iface, *address;
-
-	iface = udev_device_get_sysname (device);
-	if (!iface) {
-		_LOGD ("bind-to-connection: failed to get ifname for device.");
-		return;
-	}
-
-	address = udev_device_get_sysattr_value (device, "address");
-	if (!address || !address[0]) {
-		_LOGD ("bind-to-connection: failed to get MAC address for %s", iface);
-		return;
-	}
-
-	if (!nm_utils_hwaddr_valid (address, ETH_ALEN)) {
-		_LOGD ("bind-to-connection: failed to parse MAC address '%s' for %s",
-		       address, iface);
-		return;
-	}
-
-	s_wired = nm_connection_get_setting_wired (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (conn)));
-	s_wifi = nm_connection_get_setting_wireless (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (conn)));
-	if (s_wired) {
-		_LOGD ("bind-to-connection: locking wired connection setting");
-		g_object_set (s_wired, NM_SETTING_WIRED_MAC_ADDRESS, address, NULL);
-	} else if (s_wifi) {
-		_LOGD ("bind-to-connection: locking wireless connection setting");
-		g_object_set (s_wifi, NM_SETTING_WIRELESS_MAC_ADDRESS, address, NULL);
-	}
-
-	nm_settings_connection_update (NM_SETTINGS_CONNECTION (conn),
-	                               NULL,
-	                               NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK,
-	                               NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE,
-	                               "ifupdown-new",
-	                               NULL);
-}
-
-static void
-udev_device_added (SettingsPluginIfupdown *self, struct udev_device *device)
-{
-	SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self);
-	const char *iface, *path;
-	NMIfupdownConnection *conn;
-
-	iface = udev_device_get_sysname (device);
-	path = udev_device_get_syspath (device);
-	if (!iface || !path)
-		return;
-
-	_LOGD ("udev: devices added (path: %s, iface: %s)", path, iface);
-
-	/* if we have a configured connection for this particular iface
-	 * we want to either unmanage the device or lock it
-	 */
-	if (!g_hash_table_lookup_extended (priv->eni_ifaces, iface, NULL, (gpointer *) &conn)) {
-		_LOGD ("udev: device added (path: %s, iface: %s): no ifupdown configuration found.",
-		       path, iface);
-		return;
-	}
-
-	g_hash_table_insert (priv->kernel_ifaces, g_strdup (iface), udev_device_ref (device));
-
-	if (conn)
-		bind_device_to_connection (self, device, conn);
-
-	if (!priv->ifupdown_managed)
-		_nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self));
-}
-
-static void
-udev_device_removed (SettingsPluginIfupdown *self, struct udev_device *device)
-{
-	SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self);
-	const char *iface, *path;
-
-	iface = udev_device_get_sysname (device);
-	path = udev_device_get_syspath (device);
-	if (!iface || !path)
-		return;
-
-	_LOGD ("udev: devices removed (path: %s, iface: %s)", path, iface);
-
-	if (!g_hash_table_remove (priv->kernel_ifaces, iface))
-		return;
-
-	if (!priv->ifupdown_managed)
-		_nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self));
-}
-
-static void
-udev_device_changed (SettingsPluginIfupdown *self, struct udev_device *device)
-{
-	SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self);
-	const char *iface, *path;
-
-	iface = udev_device_get_sysname (device);
-	path = udev_device_get_syspath (device);
-	if (!iface || !path)
-		return;
-
-	_LOGD ("udev: device changed (path: %s, iface: %s)", path, iface);
-
-	if (!g_hash_table_lookup (priv->kernel_ifaces, iface))
-		return;
-
-	if (!priv->ifupdown_managed)
-		_nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self));
-}
-
-static void
-handle_uevent (NMUdevClient *client,
-               struct udev_device *device,
-               gpointer user_data)
-{
-	SettingsPluginIfupdown *self = SETTINGS_PLUGIN_IFUPDOWN (user_data);
-	const char *subsys;
-	const char *action;
-
-	action = udev_device_get_action (device);
-
-	g_return_if_fail (action != NULL);
-
-	/* A bit paranoid */
-	subsys = udev_device_get_subsystem (device);
-	g_return_if_fail (nm_streq0 (subsys, "net"));
-
-	if (nm_streq (action, "add"))
-		udev_device_added (self, device);
-	else if (nm_streq (action, "remove"))
-		udev_device_removed (self, device);
-	else if (nm_streq (action, "change"))
-		udev_device_changed (self, device);
-}
-
 /* Returns the plugins currently known list of connections.  The returned
  * list is freed by the system settings service.
  */
@@ -279,52 +130,33 @@ get_unmanaged_specs (NMSettingsPlugin *plugin)
 	SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self);
 	GSList *specs = NULL;
 	GHashTableIter iter;
-	struct udev_device *device;
 	const char *iface;
 
 	if (priv->ifupdown_managed)
 		return NULL;
 
 	_LOGD ("unmanaged-specs: unmanaged devices count %u",
-	       g_hash_table_size (priv->kernel_ifaces));
+	       g_hash_table_size (priv->eni_ifaces));
 
-	g_hash_table_iter_init (&iter, priv->kernel_ifaces);
-	while (g_hash_table_iter_next (&iter, (gpointer) &iface, (gpointer) &device)) {
-		const char *address;
-
-		address = udev_device_get_sysattr_value (device, "address");
-		if (address)
-			specs = g_slist_append (specs, g_strdup_printf ("mac:%s", address));
-		else
-			specs = g_slist_append (specs, g_strdup_printf ("interface-name:%s", iface));
-	}
+	g_hash_table_iter_init (&iter, priv->eni_ifaces);
+	while (g_hash_table_iter_next (&iter, (gpointer) &iface, NULL))
+		specs = g_slist_append (specs, g_strdup_printf ("interface-name:=%s", iface));
 	return specs;
 }
 
 /*****************************************************************************/
 
 static void
-_udev_device_unref (gpointer ptr)
-{
-	udev_device_unref (ptr);
-}
-
-static void
 initialize (NMSettingsPlugin *plugin)
 {
 	SettingsPluginIfupdown *self = SETTINGS_PLUGIN_IFUPDOWN (plugin);
 	SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self);
 	gs_unref_hashtable GHashTable *auto_ifaces = NULL;
 	if_block *block = NULL;
-	struct udev_enumerate *enumerate;
-	struct udev_list_entry *keys;
 	GHashTableIter con_iter;
 	const char *block_name;
 	NMIfupdownConnection *conn;
 
-	priv->udev_client = nm_udev_client_new ((const char *[]) { "net", NULL },
-	                                        handle_uevent, self);
-
 	/* Read in all the interfaces */
 	ifparser_init (ENI_INTERFACES_FILE, 0);
 	for (block = ifparser_getfirst (); block; block = block->next) {
@@ -443,22 +275,6 @@ initialize (NMSettingsPlugin *plugin)
 	                                                           !IFUPDOWN_UNMANAGE_WELL_KNOWN_DEFAULT);
 	_LOGI ("management mode: %s", priv->ifupdown_managed ? "managed" : "unmanaged");
 
-	/* Add well-known interfaces */
-	enumerate = nm_udev_client_enumerate_new (priv->udev_client);
-	udev_enumerate_scan_devices (enumerate);
-	keys = udev_enumerate_get_list_entry (enumerate);
-	for (; keys; keys = udev_list_entry_get_next (keys)) {
-		struct udev_device *udevice;
-
-		udevice = udev_device_new_from_syspath (udev_enumerate_get_udev (enumerate),
-		                                        udev_list_entry_get_name (keys));
-		if (udevice) {
-			udev_device_added (self, udevice);
-			udev_device_unref (udevice);
-		}
-	}
-	udev_enumerate_unref (enumerate);
-
 	/* Now if we're running in managed mode, let NM know there are new connections */
 	if (priv->ifupdown_managed) {
 		GHashTableIter iter;
@@ -479,7 +295,6 @@ settings_plugin_ifupdown_init (SettingsPluginIfupdown *self)
 	SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self);
 
 	priv->eni_ifaces = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, g_object_unref);
-	priv->kernel_ifaces = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, _udev_device_unref);
 }
 
 static void
@@ -488,11 +303,8 @@ dispose (GObject *object)
 	SettingsPluginIfupdown *plugin = SETTINGS_PLUGIN_IFUPDOWN (object);
 	SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (plugin);
 
-	g_clear_pointer (&priv->kernel_ifaces, g_hash_table_destroy);
 	g_clear_pointer (&priv->eni_ifaces, g_hash_table_destroy);
 
-	priv->udev_client = nm_udev_client_unref (priv->udev_client);
-
 	G_OBJECT_CLASS (settings_plugin_ifupdown_parent_class)->dispose (object);
 }