From be0f8e2854c598b73b1418698bdbc1626e63e789 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 15 Dec 2015 11:14:34 +0100
Subject: [PATCH 01/27] platform/trivial: move code

sysctl handling is independent from netlink-cache. Move the code.
---
 src/platform/nm-linux-platform.c | 364 +++++++++++++++----------------
 1 file changed, 182 insertions(+), 182 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 93a8be9f6..eb9f18348 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -2372,6 +2372,188 @@ nm_linux_platform_setup (void)
 
 /******************************************************************/
 
+static void
+_log_dbg_sysctl_set_impl (NMPlatform *platform, const char *path, const char *value)
+{
+	GError *error = NULL;
+	char *contents, *contents_escaped;
+	char *value_escaped = g_strescape (value, NULL);
+
+	if (!g_file_get_contents (path, &contents, NULL, &error)) {
+		_LOGD ("sysctl: setting '%s' to '%s' (current value cannot be read: %s)", path, value_escaped, error->message);
+		g_clear_error (&error);
+	} else {
+		g_strstrip (contents);
+		contents_escaped = g_strescape (contents, NULL);
+		if (strcmp (contents, value) == 0)
+			_LOGD ("sysctl: setting '%s' to '%s' (current value is identical)", path, value_escaped);
+		else
+			_LOGD ("sysctl: setting '%s' to '%s' (current value is '%s')", path, value_escaped, contents_escaped);
+		g_free (contents);
+		g_free (contents_escaped);
+	}
+	g_free (value_escaped);
+}
+
+#define _log_dbg_sysctl_set(platform, path, value) \
+	G_STMT_START { \
+		if (_LOGD_ENABLED ()) { \
+			_log_dbg_sysctl_set_impl (platform, path, value); \
+		} \
+	} G_STMT_END
+
+static gboolean
+sysctl_set (NMPlatform *platform, const char *path, const char *value)
+{
+	int fd, len, nwrote, tries;
+	char *actual;
+
+	g_return_val_if_fail (path != NULL, FALSE);
+	g_return_val_if_fail (value != NULL, FALSE);
+
+	/* Don't write outside known locations */
+	g_assert (g_str_has_prefix (path, "/proc/sys/")
+	          || g_str_has_prefix (path, "/sys/"));
+	/* Don't write to suspicious locations */
+	g_assert (!strstr (path, "/../"));
+
+	fd = open (path, O_WRONLY | O_TRUNC);
+	if (fd == -1) {
+		if (errno == ENOENT) {
+			_LOGD ("sysctl: failed to open '%s': (%d) %s",
+			       path, errno, strerror (errno));
+		} else {
+			_LOGE ("sysctl: failed to open '%s': (%d) %s",
+			       path, errno, strerror (errno));
+		}
+		return FALSE;
+	}
+
+	_log_dbg_sysctl_set (platform, path, value);
+
+	/* Most sysfs and sysctl options don't care about a trailing LF, while some
+	 * (like infiniband) do.  So always add the LF.  Also, neither sysfs nor
+	 * sysctl support partial writes so the LF must be added to the string we're
+	 * about to write.
+	 */
+	actual = g_strdup_printf ("%s\n", value);
+
+	/* Try to write the entire value three times if a partial write occurs */
+	len = strlen (actual);
+	for (tries = 0, nwrote = 0; tries < 3 && nwrote != len; tries++) {
+		nwrote = write (fd, actual, len);
+		if (nwrote == -1) {
+			if (errno == EINTR) {
+				_LOGD ("sysctl: interrupted, will try again");
+				continue;
+			}
+			break;
+		}
+	}
+	if (nwrote == -1 && errno != EEXIST) {
+		_LOGE ("sysctl: failed to set '%s' to '%s': (%d) %s",
+		       path, value, errno, strerror (errno));
+	} else if (nwrote < len) {
+		_LOGE ("sysctl: failed to set '%s' to '%s' after three attempts",
+		       path, value);
+	}
+
+	g_free (actual);
+	close (fd);
+	return (nwrote == len);
+}
+
+static GSList *sysctl_clear_cache_list;
+
+void
+_nm_linux_platform_sysctl_clear_cache (void)
+{
+	while (sysctl_clear_cache_list) {
+		NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (sysctl_clear_cache_list->data);
+
+		sysctl_clear_cache_list = g_slist_delete_link (sysctl_clear_cache_list, sysctl_clear_cache_list);
+
+		g_hash_table_destroy (priv->sysctl_get_prev_values);
+		priv->sysctl_get_prev_values = NULL;
+		priv->sysctl_get_warned = FALSE;
+	}
+}
+
+static void
+_log_dbg_sysctl_get_impl (NMPlatform *platform, const char *path, const char *contents)
+{
+	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
+	const char *prev_value = NULL;
+
+	if (!priv->sysctl_get_prev_values) {
+		sysctl_clear_cache_list = g_slist_prepend (sysctl_clear_cache_list, platform);
+		priv->sysctl_get_prev_values = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
+	} else
+		prev_value = g_hash_table_lookup (priv->sysctl_get_prev_values, path);
+
+	if (prev_value) {
+		if (strcmp (prev_value, contents) != 0) {
+			char *contents_escaped = g_strescape (contents, NULL);
+			char *prev_value_escaped = g_strescape (prev_value, NULL);
+
+			_LOGD ("sysctl: reading '%s': '%s' (changed from '%s' on last read)", path, contents_escaped, prev_value_escaped);
+			g_free (contents_escaped);
+			g_free (prev_value_escaped);
+			g_hash_table_insert (priv->sysctl_get_prev_values, g_strdup (path), g_strdup (contents));
+		}
+	} else {
+		char *contents_escaped = g_strescape (contents, NULL);
+
+		_LOGD ("sysctl: reading '%s': '%s'", path, contents_escaped);
+		g_free (contents_escaped);
+		g_hash_table_insert (priv->sysctl_get_prev_values, g_strdup (path), g_strdup (contents));
+	}
+
+	if (   !priv->sysctl_get_warned
+	    && g_hash_table_size (priv->sysctl_get_prev_values) > 50000) {
+		_LOGW ("sysctl: the internal cache for debug-logging of sysctl values grew pretty large. You can clear it by disabling debug-logging: `nmcli general logging level KEEP domains PLATFORM:INFO`.");
+		priv->sysctl_get_warned = TRUE;
+	}
+}
+
+#define _log_dbg_sysctl_get(platform, path, contents) \
+	G_STMT_START { \
+		if (_LOGD_ENABLED ()) \
+			_log_dbg_sysctl_get_impl (platform, path, contents); \
+	} G_STMT_END
+
+static char *
+sysctl_get (NMPlatform *platform, const char *path)
+{
+	GError *error = NULL;
+	char *contents;
+
+	/* Don't write outside known locations */
+	g_assert (g_str_has_prefix (path, "/proc/sys/")
+	          || g_str_has_prefix (path, "/sys/"));
+	/* Don't write to suspicious locations */
+	g_assert (!strstr (path, "/../"));
+
+	if (!g_file_get_contents (path, &contents, NULL, &error)) {
+		/* We assume FAILED means EOPNOTSUP */
+		if (   g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT)
+		    || g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_FAILED))
+			_LOGD ("error reading %s: %s", path, error->message);
+		else
+			_LOGE ("error reading %s: %s", path, error->message);
+		g_clear_error (&error);
+		return NULL;
+	}
+
+	g_strstrip (contents);
+
+	_log_dbg_sysctl_get (platform, path, contents);
+
+	return contents;
+}
+
+/******************************************************************/
+
 static gboolean
 check_support_kernel_extended_ifa_flags (NMPlatform *platform)
 {
@@ -3205,188 +3387,6 @@ event_notification (struct nl_msg *msg, gpointer user_data)
 
 /******************************************************************/
 
-static void
-_log_dbg_sysctl_set_impl (NMPlatform *platform, const char *path, const char *value)
-{
-	GError *error = NULL;
-	char *contents, *contents_escaped;
-	char *value_escaped = g_strescape (value, NULL);
-
-	if (!g_file_get_contents (path, &contents, NULL, &error)) {
-		_LOGD ("sysctl: setting '%s' to '%s' (current value cannot be read: %s)", path, value_escaped, error->message);
-		g_clear_error (&error);
-	} else {
-		g_strstrip (contents);
-		contents_escaped = g_strescape (contents, NULL);
-		if (strcmp (contents, value) == 0)
-			_LOGD ("sysctl: setting '%s' to '%s' (current value is identical)", path, value_escaped);
-		else
-			_LOGD ("sysctl: setting '%s' to '%s' (current value is '%s')", path, value_escaped, contents_escaped);
-		g_free (contents);
-		g_free (contents_escaped);
-	}
-	g_free (value_escaped);
-}
-
-#define _log_dbg_sysctl_set(platform, path, value) \
-	G_STMT_START { \
-		if (_LOGD_ENABLED ()) { \
-			_log_dbg_sysctl_set_impl (platform, path, value); \
-		} \
-	} G_STMT_END
-
-static gboolean
-sysctl_set (NMPlatform *platform, const char *path, const char *value)
-{
-	int fd, len, nwrote, tries;
-	char *actual;
-
-	g_return_val_if_fail (path != NULL, FALSE);
-	g_return_val_if_fail (value != NULL, FALSE);
-
-	/* Don't write outside known locations */
-	g_assert (g_str_has_prefix (path, "/proc/sys/")
-	          || g_str_has_prefix (path, "/sys/"));
-	/* Don't write to suspicious locations */
-	g_assert (!strstr (path, "/../"));
-
-	fd = open (path, O_WRONLY | O_TRUNC);
-	if (fd == -1) {
-		if (errno == ENOENT) {
-			_LOGD ("sysctl: failed to open '%s': (%d) %s",
-			       path, errno, strerror (errno));
-		} else {
-			_LOGE ("sysctl: failed to open '%s': (%d) %s",
-			       path, errno, strerror (errno));
-		}
-		return FALSE;
-	}
-
-	_log_dbg_sysctl_set (platform, path, value);
-
-	/* Most sysfs and sysctl options don't care about a trailing LF, while some
-	 * (like infiniband) do.  So always add the LF.  Also, neither sysfs nor
-	 * sysctl support partial writes so the LF must be added to the string we're
-	 * about to write.
-	 */
-	actual = g_strdup_printf ("%s\n", value);
-
-	/* Try to write the entire value three times if a partial write occurs */
-	len = strlen (actual);
-	for (tries = 0, nwrote = 0; tries < 3 && nwrote != len; tries++) {
-		nwrote = write (fd, actual, len);
-		if (nwrote == -1) {
-			if (errno == EINTR) {
-				_LOGD ("sysctl: interrupted, will try again");
-				continue;
-			}
-			break;
-		}
-	}
-	if (nwrote == -1 && errno != EEXIST) {
-		_LOGE ("sysctl: failed to set '%s' to '%s': (%d) %s",
-		       path, value, errno, strerror (errno));
-	} else if (nwrote < len) {
-		_LOGE ("sysctl: failed to set '%s' to '%s' after three attempts",
-		       path, value);
-	}
-
-	g_free (actual);
-	close (fd);
-	return (nwrote == len);
-}
-
-static GSList *sysctl_clear_cache_list;
-
-void
-_nm_linux_platform_sysctl_clear_cache (void)
-{
-	while (sysctl_clear_cache_list) {
-		NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (sysctl_clear_cache_list->data);
-
-		sysctl_clear_cache_list = g_slist_delete_link (sysctl_clear_cache_list, sysctl_clear_cache_list);
-
-		g_hash_table_destroy (priv->sysctl_get_prev_values);
-		priv->sysctl_get_prev_values = NULL;
-		priv->sysctl_get_warned = FALSE;
-	}
-}
-
-static void
-_log_dbg_sysctl_get_impl (NMPlatform *platform, const char *path, const char *contents)
-{
-	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
-	const char *prev_value = NULL;
-
-	if (!priv->sysctl_get_prev_values) {
-		sysctl_clear_cache_list = g_slist_prepend (sysctl_clear_cache_list, platform);
-		priv->sysctl_get_prev_values = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
-	} else
-		prev_value = g_hash_table_lookup (priv->sysctl_get_prev_values, path);
-
-	if (prev_value) {
-		if (strcmp (prev_value, contents) != 0) {
-			char *contents_escaped = g_strescape (contents, NULL);
-			char *prev_value_escaped = g_strescape (prev_value, NULL);
-
-			_LOGD ("sysctl: reading '%s': '%s' (changed from '%s' on last read)", path, contents_escaped, prev_value_escaped);
-			g_free (contents_escaped);
-			g_free (prev_value_escaped);
-			g_hash_table_insert (priv->sysctl_get_prev_values, g_strdup (path), g_strdup (contents));
-		}
-	} else {
-		char *contents_escaped = g_strescape (contents, NULL);
-
-		_LOGD ("sysctl: reading '%s': '%s'", path, contents_escaped);
-		g_free (contents_escaped);
-		g_hash_table_insert (priv->sysctl_get_prev_values, g_strdup (path), g_strdup (contents));
-	}
-
-	if (   !priv->sysctl_get_warned
-	    && g_hash_table_size (priv->sysctl_get_prev_values) > 50000) {
-		_LOGW ("sysctl: the internal cache for debug-logging of sysctl values grew pretty large. You can clear it by disabling debug-logging: `nmcli general logging level KEEP domains PLATFORM:INFO`.");
-		priv->sysctl_get_warned = TRUE;
-	}
-}
-
-#define _log_dbg_sysctl_get(platform, path, contents) \
-	G_STMT_START { \
-		if (_LOGD_ENABLED ()) \
-			_log_dbg_sysctl_get_impl (platform, path, contents); \
-	} G_STMT_END
-
-static char *
-sysctl_get (NMPlatform *platform, const char *path)
-{
-	GError *error = NULL;
-	char *contents;
-
-	/* Don't write outside known locations */
-	g_assert (g_str_has_prefix (path, "/proc/sys/")
-	          || g_str_has_prefix (path, "/sys/"));
-	/* Don't write to suspicious locations */
-	g_assert (!strstr (path, "/../"));
-
-	if (!g_file_get_contents (path, &contents, NULL, &error)) {
-		/* We assume FAILED means EOPNOTSUP */
-		if (   g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT)
-		    || g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_FAILED))
-			_LOGD ("error reading %s: %s", path, error->message);
-		else
-			_LOGE ("error reading %s: %s", path, error->message);
-		g_clear_error (&error);
-		return NULL;
-	}
-
-	g_strstrip (contents);
-
-	_log_dbg_sysctl_get (platform, path, contents);
-
-	return contents;
-}
-
-/******************************************************************/
-
 static const NMPObject *
 cache_lookup_link (NMPlatform *platform, int ifindex)
 {

From 2f6b5d412513a77f172d095f73ee1631ccce269c Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 11 Dec 2015 19:25:00 +0100
Subject: [PATCH 02/27] platform: reimplement nl_recvmsg() in platform

Calling nl_recvmsgs_default() leads dirctly to recvmsgs() from
"nl.c". This functions reads messages (recvmsg) in a loop and
invokes the callbacks.

Later we want to merge nlh and nlh_event, meaning that we must
anticipate parsing unrelated messages while waiting for an ACK.
While that would be possible by registering different callbacks
and letting them interact, it is actually more complicated. Just
assume full control over the message parsing.

Basically, copy recvmsgs() to event_handler_recvmsg(). For now just
copy the function and do little adjustment (to show the similarity
to the original). Cleanup follows.
---
 src/platform/nm-linux-platform.c | 168 ++++++++++++++++++++++++++++---
 1 file changed, 156 insertions(+), 12 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index eb9f18348..e1da582a2 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -3307,14 +3307,13 @@ event_seq_check (struct nl_msg *msg, gpointer user_data)
 	return NL_OK;
 }
 
-static int
+static void
 event_err (struct sockaddr_nl *nla, struct nlmsgerr *nlerr, gpointer platform)
 {
 	_LOGt ("event_err(): error from kernel: %s (%d) for request %d",
 	       strerror (nlerr ? -nlerr->error : 0),
 	       nlerr ? -nlerr->error : 0,
 	       NM_LINUX_PLATFORM_GET_PRIVATE (platform)->nlh_seq_last);
-	return NL_OK;
 }
 
 /* This function does all the magic to avoid race conditions caused
@@ -5397,6 +5396,160 @@ event_handler (GIOChannel *channel,
 	return TRUE;
 }
 
+/*****************************************************************************/
+
+#define NL_CB_CALL(cmd) \
+do { \
+	err = (cmd); \
+	switch (err) { \
+	case NL_OK: \
+		err = 0; \
+		break; \
+	case NL_SKIP: \
+		goto skip; \
+	case NL_STOP: \
+		goto stop; \
+	default: \
+		goto out; \
+	} \
+} while (0)
+
+/* copied from libnl3's recvmsgs() */
+static int
+event_handler_recvmsgs (NMPlatform *platform)
+{
+	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
+	struct nl_sock *sk = priv->nlh_event;
+	int n, err = 0, multipart = 0, interrupted = 0, nrecv = 0;
+	unsigned char *buf = NULL;
+	struct nlmsghdr *hdr;
+
+	/*
+	nla is passed on to not only to nl_recv() but may also be passed
+	to a function pointer provided by the caller which may or may not
+	initialize the variable. Thomas Graf.
+	*/
+	struct sockaddr_nl nla = {0};
+	struct nl_msg *msg = NULL;
+	struct ucred *creds = NULL;
+
+continue_reading:
+	n = nl_recv(sk, &nla, &buf, &creds);
+
+	if (n <= 0)
+		return n;
+
+	hdr = (struct nlmsghdr *) buf;
+	while (nlmsg_ok(hdr, n)) {
+		nlmsg_free(msg);
+		msg = nlmsg_convert(hdr);
+		if (!msg) {
+			err = -NLE_NOMEM;
+			goto out;
+		}
+
+		nlmsg_set_proto(msg, NETLINK_ROUTE);
+		nlmsg_set_src(msg, &nla);
+		if (creds)
+			nlmsg_set_creds(msg, creds);
+
+		nrecv++;
+
+		/* NL_CB_MSG_IN */
+		NL_CB_CALL (verify_source (msg, platform));
+
+		NL_CB_CALL (event_seq_check (msg, platform));
+
+		if (hdr->nlmsg_flags & NLM_F_MULTI)
+			multipart = 1;
+
+		if (hdr->nlmsg_flags & NLM_F_DUMP_INTR) {
+			/*
+			 * We have to continue reading to clear
+			 * all messages until a NLMSG_DONE is
+			 * received and report the inconsistency.
+			 */
+			interrupted = 1;
+		}
+
+		/* Other side wishes to see an ack for this message */
+		if (hdr->nlmsg_flags & NLM_F_ACK) {
+			/* FIXME: implement */
+		}
+
+		if (hdr->nlmsg_type == NLMSG_DONE) {
+			/* messages terminates a multipart message, this is
+			 * usually the end of a message and therefore we slip
+			 * out of the loop by default. the user may overrule
+			 * this action by skipping this packet. */
+			multipart = 0;
+		} else if (hdr->nlmsg_type == NLMSG_NOOP) {
+			/* Message to be ignored, the default action is to
+			 * skip this message if no callback is specified. The
+			 * user may overrule this action by returning
+			 * NL_PROCEED. */
+			goto skip;
+		} else if (hdr->nlmsg_type == NLMSG_OVERRUN) {
+			/* Data got lost, report back to user. The default action is to
+			 * quit parsing. The user may overrule this action by retuning
+			 * NL_SKIP or NL_PROCEED (dangerous) */
+			err = -NLE_MSG_OVERFLOW;
+			goto out;
+		} else if (hdr->nlmsg_type == NLMSG_ERROR) {
+			/* Message carries a nlmsgerr */
+			struct nlmsgerr *e = nlmsg_data(hdr);
+
+			if (hdr->nlmsg_len < nlmsg_size(sizeof(*e))) {
+				/* Truncated error message, the default action
+				 * is to stop parsing. The user may overrule
+				 * this action by returning NL_SKIP or
+				 * NL_PROCEED (dangerous) */
+				err = -NLE_MSG_TRUNC;
+				goto out;
+			} else if (e->error) {
+				/* Error message reported back from kernel. */
+				event_err (&nla, e, platform);
+			}
+		} else {
+			/* Valid message (not checking for MULTIPART bit to
+			 * get along with broken kernels. NL_SKIP has no
+			 * effect on this.  */
+			event_notification (msg, platform);
+		}
+skip:
+		err = 0;
+		hdr = nlmsg_next(hdr, &n);
+	}
+
+	nlmsg_free(msg);
+	free(buf);
+	free(creds);
+	buf = NULL;
+	msg = NULL;
+	creds = NULL;
+
+	if (multipart) {
+		/* Multipart message not yet complete, continue reading */
+		goto continue_reading;
+	}
+stop:
+	err = 0;
+out:
+	nlmsg_free(msg);
+	free(buf);
+	free(creds);
+
+	if (interrupted)
+		err = -NLE_DUMP_INTR;
+
+	if (!err)
+		err = nrecv;
+
+	return err;
+}
+
+/*****************************************************************************/
+
 static gboolean
 event_handler_read_netlink_one (NMPlatform *platform)
 {
@@ -5404,7 +5557,7 @@ event_handler_read_netlink_one (NMPlatform *platform)
 	int nle;
 
 	errno = 0;
-	nle = nl_recvmsgs_default (priv->nlh_event);
+	nle = event_handler_recvmsgs (platform);
 
 	/* Work around a libnl bug fixed in 3.2.22 (375a6294) */
 	if (nle == 0 && errno == EAGAIN) {
@@ -5659,15 +5812,6 @@ constructed (GObject *_object)
 		priv->nlh_event = nl_socket_alloc ();
 		g_assert (priv->nlh_event);
 
-		nle = nl_socket_modify_cb (priv->nlh_event, NL_CB_MSG_IN, NL_CB_CUSTOM, (nl_recvmsg_msg_cb_t) verify_source, platform);
-		g_assert (!nle);
-		nle = nl_socket_modify_cb (priv->nlh_event, NL_CB_VALID, NL_CB_CUSTOM, event_notification, platform);
-		g_assert (!nle);
-		nle = nl_socket_modify_cb (priv->nlh_event, NL_CB_SEQ_CHECK, NL_CB_CUSTOM, event_seq_check, platform);
-		g_assert (!nle);
-		nle = nl_socket_modify_err_cb (priv->nlh_event, NL_CB_CUSTOM, event_err, platform);
-		g_assert (!nle);
-
 		nle = nl_connect (priv->nlh_event, NETLINK_ROUTE);
 		g_assert (!nle);
 		nle = nl_socket_set_passcred (priv->nlh_event, 1);

From 3e2d0d69688cd1875a87cd0a0337a28e552cabac Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Sun, 13 Dec 2015 09:59:10 +0100
Subject: [PATCH 03/27] platform: cleanup event_handler_recvmsg() (code-style)

---
 src/platform/nm-linux-platform.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index e1da582a2..cb563c923 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -5434,24 +5434,24 @@ event_handler_recvmsgs (NMPlatform *platform)
 	struct ucred *creds = NULL;
 
 continue_reading:
-	n = nl_recv(sk, &nla, &buf, &creds);
+	n = nl_recv (sk, &nla, &buf, &creds);
 
 	if (n <= 0)
 		return n;
 
 	hdr = (struct nlmsghdr *) buf;
-	while (nlmsg_ok(hdr, n)) {
-		nlmsg_free(msg);
-		msg = nlmsg_convert(hdr);
+	while (nlmsg_ok (hdr, n)) {
+		nlmsg_free (msg);
+		msg = nlmsg_convert (hdr);
 		if (!msg) {
 			err = -NLE_NOMEM;
 			goto out;
 		}
 
-		nlmsg_set_proto(msg, NETLINK_ROUTE);
-		nlmsg_set_src(msg, &nla);
+		nlmsg_set_proto (msg, NETLINK_ROUTE);
+		nlmsg_set_src (msg, &nla);
 		if (creds)
-			nlmsg_set_creds(msg, creds);
+			nlmsg_set_creds (msg, creds);
 
 		nrecv++;
 
@@ -5497,9 +5497,9 @@ continue_reading:
 			goto out;
 		} else if (hdr->nlmsg_type == NLMSG_ERROR) {
 			/* Message carries a nlmsgerr */
-			struct nlmsgerr *e = nlmsg_data(hdr);
+			struct nlmsgerr *e = nlmsg_data (hdr);
 
-			if (hdr->nlmsg_len < nlmsg_size(sizeof(*e))) {
+			if (hdr->nlmsg_len < nlmsg_size (sizeof (*e))) {
 				/* Truncated error message, the default action
 				 * is to stop parsing. The user may overrule
 				 * this action by returning NL_SKIP or
@@ -5518,12 +5518,12 @@ continue_reading:
 		}
 skip:
 		err = 0;
-		hdr = nlmsg_next(hdr, &n);
+		hdr = nlmsg_next (hdr, &n);
 	}
 
-	nlmsg_free(msg);
-	free(buf);
-	free(creds);
+	nlmsg_free (msg);
+	free (buf);
+	free (creds);
 	buf = NULL;
 	msg = NULL;
 	creds = NULL;
@@ -5535,9 +5535,9 @@ skip:
 stop:
 	err = 0;
 out:
-	nlmsg_free(msg);
-	free(buf);
-	free(creds);
+	nlmsg_free (msg);
+	free (buf);
+	free (creds);
 
 	if (interrupted)
 		err = -NLE_DUMP_INTR;

From bdd2c31d39b3da9e5b5ef199a21531f1f1190083 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Sun, 13 Dec 2015 10:03:22 +0100
Subject: [PATCH 04/27] platform: cleanup event_handler_recvmsg() (inline
 verify_source())

---
 src/platform/nm-linux-platform.c | 42 +++++++++++---------------------
 1 file changed, 14 insertions(+), 28 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index cb563c923..237799ef1 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -3281,17 +3281,16 @@ next:
 		delayed_action_handle_all (platform, FALSE);
 }
 
-static int
-event_seq_check (struct nl_msg *msg, gpointer user_data)
+static void
+event_seq_check (NMPlatform *platform, struct nl_msg *msg)
 {
-	NMPlatform *platform = NM_PLATFORM (user_data);
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	struct nlmsghdr *hdr;
 
 	hdr = nlmsg_hdr (msg);
 
 	if (hdr->nlmsg_seq == 0)
-		return NL_OK;
+		return;
 
 	priv->nlh_seq_last = hdr->nlmsg_seq;
 
@@ -3303,8 +3302,6 @@ event_seq_check (struct nl_msg *msg, gpointer user_data)
 		priv->nlh_seq_expect = 0;
 	} else
 		_LOGt ("sequence-number: seq %u received (wait for %u)", hdr->nlmsg_seq, priv->nlh_seq_last);
-
-	return NL_OK;
 }
 
 static void
@@ -5398,22 +5395,6 @@ event_handler (GIOChannel *channel,
 
 /*****************************************************************************/
 
-#define NL_CB_CALL(cmd) \
-do { \
-	err = (cmd); \
-	switch (err) { \
-	case NL_OK: \
-		err = 0; \
-		break; \
-	case NL_SKIP: \
-		goto skip; \
-	case NL_STOP: \
-		goto stop; \
-	default: \
-		goto out; \
-	} \
-} while (0)
-
 /* copied from libnl3's recvmsgs() */
 static int
 event_handler_recvmsgs (NMPlatform *platform)
@@ -5450,15 +5431,20 @@ continue_reading:
 
 		nlmsg_set_proto (msg, NETLINK_ROUTE);
 		nlmsg_set_src (msg, &nla);
+		nrecv++;
+
+		if (!creds || creds->pid) {
+			if (creds)
+				_LOGW ("netlink: received non-kernel message (pid %d)", creds->pid);
+			else
+				_LOGW ("netlink: received message without credentials");
+			goto stop;
+		}
+
 		if (creds)
 			nlmsg_set_creds (msg, creds);
 
-		nrecv++;
-
-		/* NL_CB_MSG_IN */
-		NL_CB_CALL (verify_source (msg, platform));
-
-		NL_CB_CALL (event_seq_check (msg, platform));
+		event_seq_check (platform, msg);
 
 		if (hdr->nlmsg_flags & NLM_F_MULTI)
 			multipart = 1;

From c13163cd55fde6432424f1cc16b44ee1b0a83788 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Sun, 13 Dec 2015 10:09:02 +0100
Subject: [PATCH 05/27] platform: cleanup event_handler_recvmsg() (move EAGAIN
 workaround)

---
 src/platform/nm-linux-platform.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 237799ef1..e584d4551 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -5415,8 +5415,17 @@ event_handler_recvmsgs (NMPlatform *platform)
 	struct ucred *creds = NULL;
 
 continue_reading:
+	errno = 0;
 	n = nl_recv (sk, &nla, &buf, &creds);
 
+	/* Work around a libnl bug fixed in 3.2.22 (375a6294) */
+	if (n == 0 && errno == EAGAIN) {
+		/* EAGAIN is equal to EWOULDBLOCK. If it would not be, we'd have to
+		 * workaround libnl3 mapping EWOULDBLOCK to -NLE_FAILURE. */
+		G_STATIC_ASSERT (EAGAIN == EWOULDBLOCK);
+		n = -NLE_AGAIN;
+	}
+
 	if (n <= 0)
 		return n;
 
@@ -5542,17 +5551,8 @@ event_handler_read_netlink_one (NMPlatform *platform)
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	int nle;
 
-	errno = 0;
 	nle = event_handler_recvmsgs (platform);
 
-	/* Work around a libnl bug fixed in 3.2.22 (375a6294) */
-	if (nle == 0 && errno == EAGAIN) {
-		/* EAGAIN is equal to EWOULDBLOCK. If it would not be, we'd have to
-		 * workaround libnl3 mapping EWOULDBLOCK to -NLE_FAILURE. */
-		G_STATIC_ASSERT (EAGAIN == EWOULDBLOCK);
-		nle = -NLE_AGAIN;
-	}
-
 	if (nle < 0)
 		switch (nle) {
 		case -NLE_AGAIN:

From 47773c80ac2f19eeeb583d65cc71c5f11eb0f250 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Sun, 13 Dec 2015 10:15:04 +0100
Subject: [PATCH 06/27] platform: cleanup event_handler_recvmsg() (inline
 event_err())

---
 src/platform/nm-linux-platform.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index e584d4551..3836de444 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -3304,15 +3304,6 @@ event_seq_check (NMPlatform *platform, struct nl_msg *msg)
 		_LOGt ("sequence-number: seq %u received (wait for %u)", hdr->nlmsg_seq, priv->nlh_seq_last);
 }
 
-static void
-event_err (struct sockaddr_nl *nla, struct nlmsgerr *nlerr, gpointer platform)
-{
-	_LOGt ("event_err(): error from kernel: %s (%d) for request %d",
-	       strerror (nlerr ? -nlerr->error : 0),
-	       nlerr ? -nlerr->error : 0,
-	       NM_LINUX_PLATFORM_GET_PRIVATE (platform)->nlh_seq_last);
-}
-
 /* This function does all the magic to avoid race conditions caused
  * by concurrent usage of synchronous commands and an asynchronous cache. This
  * might be a nice future addition to libnl but it requires to do all operations
@@ -5503,7 +5494,10 @@ continue_reading:
 				goto out;
 			} else if (e->error) {
 				/* Error message reported back from kernel. */
-				event_err (&nla, e, platform);
+				_LOGt ("event_err(): error from kernel: %s (%d) for request %d",
+				       strerror (e->error),
+				       e->error,
+				       priv->nlh_seq_last);
 			}
 		} else {
 			/* Valid message (not checking for MULTIPART bit to

From 385e68327f9412d2b1e29992c44568aa2c9fe5ab Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Sun, 13 Dec 2015 10:25:17 +0100
Subject: [PATCH 07/27] platform: cleanup event_handler_recvmsg() (adjust log
 messages)

---
 src/platform/nm-linux-platform.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 3836de444..58783b139 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -5435,9 +5435,9 @@ continue_reading:
 
 		if (!creds || creds->pid) {
 			if (creds)
-				_LOGW ("netlink: received non-kernel message (pid %d)", creds->pid);
+				_LOGW ("netlink: recvmsg: received non-kernel message (pid %d)", creds->pid);
 			else
-				_LOGW ("netlink: received message without credentials");
+				_LOGW ("netlink: recvmsg: received message without credentials");
 			goto stop;
 		}
 
@@ -5494,7 +5494,7 @@ continue_reading:
 				goto out;
 			} else if (e->error) {
 				/* Error message reported back from kernel. */
-				_LOGt ("event_err(): error from kernel: %s (%d) for request %d",
+				_LOGD ("netlink: recvmsg: error message from kernel: %s (%d) for request %d",
 				       strerror (e->error),
 				       e->error,
 				       priv->nlh_seq_last);
@@ -5552,10 +5552,10 @@ event_handler_read_netlink_one (NMPlatform *platform)
 		case -NLE_AGAIN:
 			return FALSE;
 		case -NLE_DUMP_INTR:
-			_LOGD ("Uncritical failure to retrieve incoming events: %s (%d)", nl_geterror (nle), nle);
+			_LOGD ("netlink: read-one: uncritical failure to retrieve incoming events: %s (%d)", nl_geterror (nle), nle);
 			break;
 		case -NLE_NOMEM:
-			_LOGI ("Too many netlink events. Need to resynchronize platform cache");
+			_LOGI ("netlink: read-one: too many netlink events. Need to resynchronize platform cache");
 			/* Drain the event queue, we've lost events and are out of sync anyway and we'd
 			 * like to free up some space. We'll read in the status synchronously. */
 			_nl_sock_flush_data (priv->nlh_event);
@@ -5569,7 +5569,7 @@ event_handler_read_netlink_one (NMPlatform *platform)
 			                         NULL);
 			break;
 		default:
-			_LOGE ("Failed to retrieve incoming events: %s (%d)", nl_geterror (nle), nle);
+			_LOGE ("netlink: read-one: failed to retrieve incoming events: %s (%d)", nl_geterror (nle), nle);
 			break;
 	}
 	return TRUE;
@@ -5593,7 +5593,7 @@ event_handler_read_netlink_all (NMPlatform *platform, gboolean wait_for_acks)
 
 		if (!wait_for_acks || priv->nlh_seq_expect == 0) {
 			if (wait_for_seq)
-				_LOGt ("read-netlink-all: ACK for sequence number %u received", priv->nlh_seq_expect);
+				_LOGT ("netlink: read-all: ACK for sequence number %u received", priv->nlh_seq_expect);
 			return any;
 		}
 
@@ -5601,7 +5601,7 @@ event_handler_read_netlink_all (NMPlatform *platform, gboolean wait_for_acks)
 		if (wait_for_seq != priv->nlh_seq_expect) {
 			/* We are waiting for a new sequence number (or we will wait for the first time).
 			 * Reset/start counting the overall wait time. */
-			_LOGt ("read-netlink-all: wait for ACK for sequence number %u...", priv->nlh_seq_expect);
+			_LOGT ("netlink: read-all: wait for ACK for sequence number %u...", priv->nlh_seq_expect);
 			wait_for_seq = priv->nlh_seq_expect;
 			timestamp = now;
 			timeout = TIMEOUT;
@@ -5628,14 +5628,14 @@ event_handler_read_netlink_all (NMPlatform *platform, gboolean wait_for_acks)
 			int errsv = errno;
 
 			if (errsv != EINTR) {
-				_LOGE ("read-netlink-all: poll failed with %s", strerror (errsv));
+				_LOGE ("netlink: read-all: poll failed with %s", strerror (errsv));
 				return any;
 			}
 			/* Continue to read again, even if there might be nothing to read after EINTR. */
 		}
 	}
 
-	_LOGW ("read-netlink-all: timeout waiting for ACK to sequence number %u...", wait_for_seq);
+	_LOGW ("netlink: read-all: timeout waiting for ACK to sequence number %u...", wait_for_seq);
 	priv->nlh_seq_expect = 0;
 	return any;
 }

From 6ee868678bcc0b7e2479b126f9bdfeffe2e2cd65 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Sun, 13 Dec 2015 10:30:26 +0100
Subject: [PATCH 08/27] platform: cleanup event_handler_recvmsg() (drop
 _nl_sock_flush_data())

Also avoids/fixes a bug in _nl_sock_flush_data() where we would loop
endlessly, if nl_recvmsgs() fails for reasons other then EAGAIN.
---
 src/platform/nm-linux-platform.c | 43 ++++++--------------------------
 1 file changed, 8 insertions(+), 35 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 58783b139..067d8ebfd 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -2249,38 +2249,6 @@ nla_put_failure:
 
 /******************************************************************/
 
-static int
-_nl_sock_flush_data (struct nl_sock *sk)
-{
-	int nle;
-	struct nl_cb *cb;
-	struct nl_cb *cb0;
-
-	cb0 = nl_socket_get_cb (sk);
-	cb = nl_cb_clone (cb0);
-	nl_cb_put (cb0);
-	if (cb == NULL)
-		return -NLE_NOMEM;
-
-	nl_cb_set (cb, NL_CB_VALID, NL_CB_DEFAULT, NULL, NULL);
-	nl_cb_set (cb, NL_CB_SEQ_CHECK, NL_CB_DEFAULT, NULL, NULL);
-	nl_cb_err (cb, NL_CB_DEFAULT, NULL, NULL);
-	do {
-		errno = 0;
-
-		nle = nl_recvmsgs (sk, cb);
-
-		/* Work around a libnl bug fixed in 3.2.22 (375a6294) */
-		if (nle == 0 && (errno == EAGAIN || errno == EWOULDBLOCK))
-			nle = -NLE_AGAIN;
-	} while (nle != -NLE_AGAIN);
-
-	nl_cb_put (cb);
-	return nle;
-}
-
-/******************************************************************/
-
 static int _support_kernel_extended_ifa_flags = -1;
 
 #define _support_kernel_extended_ifa_flags_still_undecided() (G_UNLIKELY (_support_kernel_extended_ifa_flags == -1))
@@ -5388,7 +5356,7 @@ event_handler (GIOChannel *channel,
 
 /* copied from libnl3's recvmsgs() */
 static int
-event_handler_recvmsgs (NMPlatform *platform)
+event_handler_recvmsgs (NMPlatform *platform, gboolean handle_events)
 {
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	struct nl_sock *sk = priv->nlh_event;
@@ -5420,6 +5388,11 @@ continue_reading:
 	if (n <= 0)
 		return n;
 
+	if (!handle_events) {
+		/* we read until failure or there is nothing to read (EAGAIN). */
+		goto continue_reading;
+	}
+
 	hdr = (struct nlmsghdr *) buf;
 	while (nlmsg_ok (hdr, n)) {
 		nlmsg_free (msg);
@@ -5545,7 +5518,7 @@ event_handler_read_netlink_one (NMPlatform *platform)
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	int nle;
 
-	nle = event_handler_recvmsgs (platform);
+	nle = event_handler_recvmsgs (platform, TRUE);
 
 	if (nle < 0)
 		switch (nle) {
@@ -5558,7 +5531,7 @@ event_handler_read_netlink_one (NMPlatform *platform)
 			_LOGI ("netlink: read-one: too many netlink events. Need to resynchronize platform cache");
 			/* Drain the event queue, we've lost events and are out of sync anyway and we'd
 			 * like to free up some space. We'll read in the status synchronously. */
-			_nl_sock_flush_data (priv->nlh_event);
+			event_handler_recvmsgs (platform, FALSE);
 			priv->nlh_seq_expect = 0;
 			delayed_action_schedule (platform,
 			                         DELAYED_ACTION_TYPE_REFRESH_ALL_LINKS |

From ecdcfda0dd007819751d0be93c58dabe0e7ef12f Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Sun, 13 Dec 2015 10:37:40 +0100
Subject: [PATCH 09/27] platform: cleanup event_handler_recvmsg() (rename
 event_valid_msg())

---
 src/platform/nm-linux-platform.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 067d8ebfd..c2b881bf5 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -3278,11 +3278,10 @@ event_seq_check (NMPlatform *platform, struct nl_msg *msg)
  * through the cache manager. In this case, nm-linux-platform serves as the
  * cache manager instead of the one provided by libnl.
  */
-static int
-event_notification (struct nl_msg *msg, gpointer user_data)
+static void
+event_valid_msg (NMPlatform *platform, struct nl_msg *msg)
 {
-	NMPlatform *platform = NM_PLATFORM (user_data);
-	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (user_data);
+	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	nm_auto_nmpobj NMPObject *obj = NULL;
 	nm_auto_nmpobj NMPObject *obj_cache = NULL;
 	NMPCacheOpsType cache_op;
@@ -3307,7 +3306,7 @@ event_notification (struct nl_msg *msg, gpointer user_data)
 		_LOGT ("event-notification: %s, seq %u: ignore",
 		       _nl_nlmsg_type_to_str (msghdr->nlmsg_type, buf_nlmsg_type, sizeof (buf_nlmsg_type)),
 		       msghdr->nlmsg_seq);
-		return NL_OK;
+		return;
 	}
 
 	_LOGT ("event-notification: %s, seq %u: %s",
@@ -3336,8 +3335,6 @@ event_notification (struct nl_msg *msg, gpointer user_data)
 	}
 
 	cache_prune_candidates_drop (platform, obj_cache);
-
-	return NL_OK;
 }
 
 /******************************************************************/
@@ -5476,7 +5473,7 @@ continue_reading:
 			/* Valid message (not checking for MULTIPART bit to
 			 * get along with broken kernels. NL_SKIP has no
 			 * effect on this.  */
-			event_notification (msg, platform);
+			event_valid_msg (platform, msg);
 		}
 skip:
 		err = 0;

From 4ea711e03e68a4905193978529c8c9086a0f4938 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Sun, 13 Dec 2015 10:46:37 +0100
Subject: [PATCH 10/27] platform/trivial: remove obsolete code comment

---
 src/platform/nm-linux-platform.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index c2b881bf5..1e9f5fbf8 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -3272,12 +3272,6 @@ event_seq_check (NMPlatform *platform, struct nl_msg *msg)
 		_LOGt ("sequence-number: seq %u received (wait for %u)", hdr->nlmsg_seq, priv->nlh_seq_last);
 }
 
-/* This function does all the magic to avoid race conditions caused
- * by concurrent usage of synchronous commands and an asynchronous cache. This
- * might be a nice future addition to libnl but it requires to do all operations
- * through the cache manager. In this case, nm-linux-platform serves as the
- * cache manager instead of the one provided by libnl.
- */
 static void
 event_valid_msg (NMPlatform *platform, struct nl_msg *msg)
 {

From c9e04c963e0cfa024688171a9ca31291aebb9a8c Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 14 Dec 2015 11:53:46 +0100
Subject: [PATCH 11/27] platform: implement our own sequence counter

Instead of using the one from libnl's socket.
---
 src/platform/nm-linux-platform.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 1e9f5fbf8..7ec6c13ce 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -2296,6 +2296,7 @@ typedef struct _NMLinuxPlatformPrivate NMLinuxPlatformPrivate;
 struct _NMLinuxPlatformPrivate {
 	struct nl_sock *nlh;
 	struct nl_sock *nlh_event;
+	guint32 nlh_seq_next;
 	guint32 nlh_seq_expect;
 	guint32 nlh_seq_last;
 	NMPCache *cache;
@@ -3122,12 +3123,8 @@ _nl_send_auto_with_seq (NMPlatform *platform, struct nl_msg *nlmsg)
 	guint32 seq;
 	int nle;
 
-	/* complete the message, by choosing our own sequence number, because libnl
-	 * does not ensure that it isn't zero -- which would confuse our checking for
-	 * outstanding messages. */
-	seq = nl_socket_use_seq (priv->nlh_event);
-	if (seq == 0)
-		seq = nl_socket_use_seq (priv->nlh_event);
+	/* complete the message with a sequence number (ensuring it's not zero). */
+	seq = priv->nlh_seq_next++ ?: priv->nlh_seq_next++;
 
 	nlmsg_hdr (nlmsg)->nlmsg_seq = seq;
 
@@ -5718,6 +5715,7 @@ nm_linux_platform_init (NMLinuxPlatform *self)
 
 	self->priv = priv;
 
+	priv->nlh_seq_next = 1;
 	priv->cache = nmp_cache_new ();
 	priv->delayed_action.list_master_connected = g_ptr_array_new ();
 	priv->delayed_action.list_refresh_link = g_ptr_array_new ();

From eb182c2e1c683b47873826cc1303c6f33c094d35 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 14 Dec 2015 12:09:50 +0100
Subject: [PATCH 12/27] platform: minor refactoring in
 delayed_action_schedule()

---
 src/platform/nm-linux-platform.c | 45 +++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 7ec6c13ce..6218daf12 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -2664,8 +2664,34 @@ delayed_action_to_string (DelayedActionType action_type)
 	}
 }
 
-#define _LOGt_delayed_action(action_type, arg, operation) \
-    _LOGt ("delayed-action: %s %s (%d) [%p / %d]", ""operation, delayed_action_to_string (action_type), (int) action_type, arg, GPOINTER_TO_INT (arg))
+static const char *
+delayed_action_to_string_full (DelayedActionType action_type, gpointer user_data, char *buf, gsize buf_size)
+{
+	char *buf0 = buf;
+
+	nm_utils_strbuf_append_str (&buf, &buf_size, delayed_action_to_string (action_type));
+	switch (action_type) {
+	case DELAYED_ACTION_TYPE_MASTER_CONNECTED:
+		nm_utils_strbuf_append (&buf, &buf_size, " (master-ifindex %d)", GPOINTER_TO_INT (user_data));
+		break;
+	case DELAYED_ACTION_TYPE_REFRESH_LINK:
+		nm_utils_strbuf_append (&buf, &buf_size, " (ifindex %d)", GPOINTER_TO_INT (user_data));
+		break;
+	default:
+		nm_assert (!user_data);
+		break;
+	}
+	return buf0;
+}
+
+#define _LOGt_delayed_action(action_type, user_data, operation) \
+    G_STMT_START { \
+        char _buf[255]; \
+        \
+        _LOGt ("delayed-action: %s %s", \
+               ""operation, \
+               delayed_action_to_string_full (action_type, user_data, _buf, sizeof (_buf))); \
+    } G_STMT_END
 
 static void
 delayed_action_handle_MASTER_CONNECTED (NMPlatform *platform, int master_ifindex)
@@ -2801,16 +2827,21 @@ delayed_action_schedule (NMPlatform *platform, DelayedActionType action_type, gp
 
 	nm_assert (action_type != DELAYED_ACTION_TYPE_NONE);
 
-	if (NM_FLAGS_HAS (action_type, DELAYED_ACTION_TYPE_REFRESH_LINK)) {
-		nm_assert (nm_utils_is_power_of_two (action_type));
+	switch (action_type) {
+	case DELAYED_ACTION_TYPE_REFRESH_LINK:
 		if (_nm_utils_ptrarray_find_first (priv->delayed_action.list_refresh_link->pdata, priv->delayed_action.list_refresh_link->len, user_data) < 0)
 			g_ptr_array_add (priv->delayed_action.list_refresh_link, user_data);
-	} else if (NM_FLAGS_HAS (action_type, DELAYED_ACTION_TYPE_MASTER_CONNECTED)) {
-		nm_assert (nm_utils_is_power_of_two (action_type));
+		break;
+	case DELAYED_ACTION_TYPE_MASTER_CONNECTED:
 		if (_nm_utils_ptrarray_find_first (priv->delayed_action.list_master_connected->pdata, priv->delayed_action.list_master_connected->len, user_data) < 0)
 			g_ptr_array_add (priv->delayed_action.list_master_connected, user_data);
-	} else
+		break;
+	default:
 		nm_assert (!user_data);
+		nm_assert (!NM_FLAGS_HAS (action_type, DELAYED_ACTION_TYPE_REFRESH_LINK));
+		nm_assert (!NM_FLAGS_HAS (action_type, DELAYED_ACTION_TYPE_MASTER_CONNECTED));
+		break;
+	}
 
 	priv->delayed_action.flags |= action_type;
 

From 6d67e6e9c4002ab61475363cf5a932e130cbab62 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 14 Dec 2015 14:47:41 +0100
Subject: [PATCH 13/27] platform: rework waiting for netlink response

Track pending netlink requests and properly report the resulting errno
back.

Currently we send only requests in do_request_link() and do_request_all().
These callers don't actually care about the result, all they care that the
request is answered before returning back from platform code to the caller.
Thus, up to now the tracking of the sequence number was pretty simple.

Later we also want to get the errno from a request, thus rework sending
requests to also remember about outstanding sequence numbers and
properly track muliple parallel requests.

Later the synchronous actions (e.g. add-link) will also be handled
via the asynchronous socket.
---
 src/platform/nm-linux-platform.c | 419 +++++++++++++++++++++++--------
 1 file changed, 310 insertions(+), 109 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 6218daf12..5c355d2e3 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -154,6 +154,7 @@ typedef enum {
 	DELAYED_ACTION_TYPE_REFRESH_LINK                = (1LL << 5),
 	DELAYED_ACTION_TYPE_MASTER_CONNECTED            = (1LL << 6),
 	DELAYED_ACTION_TYPE_READ_NETLINK                = (1LL << 7),
+	DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE        = (1LL << 8),
 	__DELAYED_ACTION_TYPE_MAX,
 
 	DELAYED_ACTION_TYPE_REFRESH_ALL                 = DELAYED_ACTION_TYPE_REFRESH_ALL_LINKS |
@@ -165,13 +166,60 @@ typedef enum {
 	DELAYED_ACTION_TYPE_MAX                         = __DELAYED_ACTION_TYPE_MAX -1,
 } DelayedActionType;
 
+typedef enum {
+	/* Negative values are errors from kernel. Add dummy member to
+	 * make enum signed. */
+	_WAIT_FOR_NL_RESPONSE_RESULT_SYSTEM_ERROR = -1,
+
+	WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN = 0,
+	WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK,
+	WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_UNKNOWN,
+	WAIT_FOR_NL_RESPONSE_RESULT_FAILED_RESYNC,
+	WAIT_FOR_NL_RESPONSE_RESULT_FAILED_POLL,
+	WAIT_FOR_NL_RESPONSE_RESULT_FAILED_TIMEOUT,
+	WAIT_FOR_NL_RESPONSE_RESULT_FAILED_DISPOSING,
+} WaitForNlResponseResult;
+
+typedef void (*WaitForNlResponseCallback) (NMPlatform *platform,
+                                           guint32 seq_number,
+                                           WaitForNlResponseResult seq_result,
+                                           gpointer user_data);
+
 static void delayed_action_schedule (NMPlatform *platform, DelayedActionType action_type, gpointer user_data);
 static gboolean delayed_action_handle_all (NMPlatform *platform, gboolean read_netlink);
-static void do_request_link (NMPlatform *platform, int ifindex, const char *name, gboolean handle_delayed_action);
-static void do_request_all (NMPlatform *platform, DelayedActionType action_type, gboolean handle_delayed_action);
+static void do_request_link_no_delayed_actions (NMPlatform *platform, int ifindex, const char *name);
+static void do_request_all_no_delayed_actions (NMPlatform *platform, DelayedActionType action_type);
 static void cache_pre_hook (NMPCache *cache, const NMPObject *old, const NMPObject *new, NMPCacheOpsType ops_type, gpointer user_data);
+static void cache_prune_candidates_prune (NMPlatform *platform);
 static gboolean event_handler_read_netlink_all (NMPlatform *platform, gboolean wait_for_acks);
 
+/*****************************************************************************/
+
+static const char *
+wait_for_nl_response_to_string (WaitForNlResponseResult seq_result, char *buf, gsize buf_size)
+{
+	char *buf0 = buf;
+
+	switch (seq_result) {
+	case WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN:
+		nm_utils_strbuf_append_str (&buf, &buf_size, "unknown");
+		break;
+	case WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK:
+		nm_utils_strbuf_append_str (&buf, &buf_size, "success");
+		break;
+	case WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_UNKNOWN:
+		nm_utils_strbuf_append_str (&buf, &buf_size, "failure");
+		break;
+	default:
+		if (seq_result < 0)
+			nm_utils_strbuf_append (&buf, &buf_size, "failure %d (%s)", -((int) seq_result), g_strerror (-((int) seq_result)));
+		else
+			nm_utils_strbuf_append (&buf, &buf_size, "internal failure %d", (int) seq_result);
+		break;
+	}
+	return buf0;
+}
+
 /******************************************************************
  * Support IFLA_INET6_ADDR_GEN_MODE
  ******************************************************************/
@@ -2291,14 +2339,20 @@ _support_kernel_extended_ifa_flags_get (void)
  * NMPlatform types and functions
  ******************************************************************/
 
+typedef struct {
+	guint32 seq_number;
+	gint64 timeout_abs_ns;
+	WaitForNlResponseResult seq_result;
+	WaitForNlResponseResult *out_seq_result;
+} DelayedActionWaitForNlResponseData;
+
 typedef struct _NMLinuxPlatformPrivate NMLinuxPlatformPrivate;
 
 struct _NMLinuxPlatformPrivate {
 	struct nl_sock *nlh;
 	struct nl_sock *nlh_event;
 	guint32 nlh_seq_next;
-	guint32 nlh_seq_expect;
-	guint32 nlh_seq_last;
+	guint32 nlh_seq_last_handled;
 	NMPCache *cache;
 	GIOChannel *event_channel;
 	guint event_id;
@@ -2312,6 +2366,7 @@ struct _NMLinuxPlatformPrivate {
 		DelayedActionType flags;
 		GPtrArray *list_master_connected;
 		GPtrArray *list_refresh_link;
+		GArray *list_wait_for_nl_response;
 		gint is_handling;
 		guint idle_id;
 	} delayed_action;
@@ -2659,6 +2714,7 @@ delayed_action_to_string (DelayedActionType action_type)
 	case DELAYED_ACTION_TYPE_REFRESH_LINK                   : return "refresh-link";
 	case DELAYED_ACTION_TYPE_MASTER_CONNECTED               : return "master-connected";
 	case DELAYED_ACTION_TYPE_READ_NETLINK                   : return "read-netlink";
+	case DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE           : return "wait-for-nl-response";
 	default:
 		return "unknown";
 	}
@@ -2668,6 +2724,7 @@ static const char *
 delayed_action_to_string_full (DelayedActionType action_type, gpointer user_data, char *buf, gsize buf_size)
 {
 	char *buf0 = buf;
+	const DelayedActionWaitForNlResponseData *data;
 
 	nm_utils_strbuf_append_str (&buf, &buf_size, delayed_action_to_string (action_type));
 	switch (action_type) {
@@ -2677,6 +2734,23 @@ delayed_action_to_string_full (DelayedActionType action_type, gpointer user_data
 	case DELAYED_ACTION_TYPE_REFRESH_LINK:
 		nm_utils_strbuf_append (&buf, &buf_size, " (ifindex %d)", GPOINTER_TO_INT (user_data));
 		break;
+	case DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE:
+		data = user_data;
+
+		if (data) {
+			gint64 timeout = data->timeout_abs_ns - nm_utils_get_monotonic_timestamp_ns ();
+			char b[255];
+
+			nm_utils_strbuf_append (&buf, &buf_size, " (seq %u, timeout in %s%ld.%09ld%s%s)",
+			                        data->seq_number,
+			                        timeout < 0 ? "-" : "",
+			                        (timeout < 0 ? -timeout : timeout) / NM_UTILS_NS_PER_SECOND,
+			                        (timeout < 0 ? -timeout : timeout) % NM_UTILS_NS_PER_SECOND,
+			                        data->seq_result ? ", " : "",
+			                        data->seq_result ? wait_for_nl_response_to_string (data->seq_result, b, sizeof (b)) : "");
+		} else
+			nm_utils_strbuf_append_str (&buf, &buf_size, " (any)");
+		break;
 	default:
 		nm_assert (!user_data);
 		break;
@@ -2693,6 +2767,53 @@ delayed_action_to_string_full (DelayedActionType action_type, gpointer user_data
                delayed_action_to_string_full (action_type, user_data, _buf, sizeof (_buf))); \
     } G_STMT_END
 
+/*****************************************************************************/
+
+static void
+delayed_action_wait_for_nl_response_complete (NMPlatform *platform,
+                                              guint idx,
+                                              WaitForNlResponseResult seq_result)
+{
+	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
+	DelayedActionWaitForNlResponseData *data;
+	WaitForNlResponseResult *out_seq_result;
+
+	nm_assert (NM_FLAGS_HAS (priv->delayed_action.flags, DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE));
+	nm_assert (idx < priv->delayed_action.list_wait_for_nl_response->len);
+	nm_assert (seq_result);
+
+	data = &g_array_index (priv->delayed_action.list_wait_for_nl_response, DelayedActionWaitForNlResponseData, idx);
+
+	_LOGt_delayed_action (DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE, data, "complete");
+
+	out_seq_result = data->out_seq_result;
+
+	g_array_remove_index_fast (priv->delayed_action.list_wait_for_nl_response, idx);
+	/* Note: @data is invalidated at this point */
+
+	if (priv->delayed_action.list_wait_for_nl_response->len <= 0)
+		priv->delayed_action.flags &= ~DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE;
+
+	if (out_seq_result)
+		*out_seq_result = seq_result;
+}
+
+static void
+delayed_action_wait_for_nl_response_complete_all (NMPlatform *platform,
+                                                  WaitForNlResponseResult result)
+{
+	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
+
+	if (NM_FLAGS_HAS (priv->delayed_action.flags, DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE)) {
+		while (priv->delayed_action.list_wait_for_nl_response->len > 0)
+			delayed_action_wait_for_nl_response_complete (platform, priv->delayed_action.list_wait_for_nl_response->len - 1, result);
+	}
+	nm_assert (!NM_FLAGS_HAS (priv->delayed_action.flags, DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE));
+	nm_assert (priv->delayed_action.list_wait_for_nl_response->len == 0);
+}
+
+/*****************************************************************************/
+
 static void
 delayed_action_handle_MASTER_CONNECTED (NMPlatform *platform, int master_ifindex)
 {
@@ -2708,17 +2829,23 @@ delayed_action_handle_MASTER_CONNECTED (NMPlatform *platform, int master_ifindex
 static void
 delayed_action_handle_REFRESH_LINK (NMPlatform *platform, int ifindex)
 {
-	do_request_link (platform, ifindex, NULL, FALSE);
+	do_request_link_no_delayed_actions (platform, ifindex, NULL);
 }
 
 static void
 delayed_action_handle_REFRESH_ALL (NMPlatform *platform, DelayedActionType flags)
 {
-	do_request_all (platform, flags, FALSE);
+	do_request_all_no_delayed_actions (platform, flags);
 }
 
 static void
 delayed_action_handle_READ_NETLINK (NMPlatform *platform)
+{
+	event_handler_read_netlink_all (platform, FALSE);
+}
+
+static void
+delayed_action_handle_WAIT_FOR_NL_RESPONSE (NMPlatform *platform)
 {
 	event_handler_read_netlink_all (platform, TRUE);
 }
@@ -2761,7 +2888,8 @@ delayed_action_handle_one (NMPlatform *platform)
 		return TRUE;
 	}
 
-	if (NM_FLAGS_ANY (priv->delayed_action.flags, DELAYED_ACTION_TYPE_REFRESH_ALL)) {
+	if (   NM_FLAGS_ANY (priv->delayed_action.flags, DELAYED_ACTION_TYPE_REFRESH_ALL)
+	    && priv->delayed_action.is_handling < 5 /* avoid deep recursive stacks */) {
 		DelayedActionType flags, iflags;
 
 		flags = priv->delayed_action.flags & DELAYED_ACTION_TYPE_REFRESH_ALL;
@@ -2779,20 +2907,31 @@ delayed_action_handle_one (NMPlatform *platform)
 		return TRUE;
 	}
 
-	nm_assert (priv->delayed_action.flags == DELAYED_ACTION_TYPE_REFRESH_LINK);
-	nm_assert (priv->delayed_action.list_refresh_link->len > 0);
+	if (   NM_FLAGS_HAS (priv->delayed_action.flags, DELAYED_ACTION_TYPE_REFRESH_LINK)
+	    && priv->delayed_action.is_handling < 5 /* avoid deep recursive stacks */) {
+		nm_assert (priv->delayed_action.list_refresh_link->len > 0);
 
-	user_data = priv->delayed_action.list_refresh_link->pdata[0];
-	g_ptr_array_remove_index_fast (priv->delayed_action.list_refresh_link, 0);
-	if (priv->delayed_action.list_refresh_link->len == 0)
-		priv->delayed_action.flags &= ~DELAYED_ACTION_TYPE_REFRESH_LINK;
-	nm_assert (_nm_utils_ptrarray_find_first (priv->delayed_action.list_refresh_link->pdata, priv->delayed_action.list_refresh_link->len, user_data) < 0);
+		user_data = priv->delayed_action.list_refresh_link->pdata[0];
+		g_ptr_array_remove_index_fast (priv->delayed_action.list_refresh_link, 0);
+		if (priv->delayed_action.list_refresh_link->len == 0)
+			priv->delayed_action.flags &= ~DELAYED_ACTION_TYPE_REFRESH_LINK;
+		nm_assert (_nm_utils_ptrarray_find_first (priv->delayed_action.list_refresh_link->pdata, priv->delayed_action.list_refresh_link->len, user_data) < 0);
 
-	_LOGt_delayed_action (DELAYED_ACTION_TYPE_REFRESH_LINK, user_data, "handle");
+		_LOGt_delayed_action (DELAYED_ACTION_TYPE_REFRESH_LINK, user_data, "handle");
 
-	delayed_action_handle_REFRESH_LINK (platform, GPOINTER_TO_INT (user_data));
+		delayed_action_handle_REFRESH_LINK (platform, GPOINTER_TO_INT (user_data));
 
-	return TRUE;
+		return TRUE;
+	}
+
+	if (NM_FLAGS_HAS (priv->delayed_action.flags, DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE)) {
+		nm_assert (priv->delayed_action.list_wait_for_nl_response->len > 0);
+		_LOGt_delayed_action (DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE, NULL, "handle");
+		delayed_action_handle_WAIT_FOR_NL_RESPONSE (platform);
+		return TRUE;
+	}
+
+	return FALSE;
 }
 
 static gboolean
@@ -2808,6 +2947,10 @@ delayed_action_handle_all (NMPlatform *platform, gboolean read_netlink)
 	while (delayed_action_handle_one (platform))
 		any = TRUE;
 	priv->delayed_action.is_handling--;
+
+	if (priv->delayed_action.is_handling <= 0)
+		cache_prune_candidates_prune (platform);
+
 	return any;
 }
 
@@ -2836,10 +2979,14 @@ delayed_action_schedule (NMPlatform *platform, DelayedActionType action_type, gp
 		if (_nm_utils_ptrarray_find_first (priv->delayed_action.list_master_connected->pdata, priv->delayed_action.list_master_connected->len, user_data) < 0)
 			g_ptr_array_add (priv->delayed_action.list_master_connected, user_data);
 		break;
+	case DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE:
+		g_array_append_vals (priv->delayed_action.list_wait_for_nl_response, user_data, 1);
+		break;
 	default:
 		nm_assert (!user_data);
 		nm_assert (!NM_FLAGS_HAS (action_type, DELAYED_ACTION_TYPE_REFRESH_LINK));
 		nm_assert (!NM_FLAGS_HAS (action_type, DELAYED_ACTION_TYPE_MASTER_CONNECTED));
+		nm_assert (!NM_FLAGS_HAS (action_type, DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE));
 		break;
 	}
 
@@ -2856,6 +3003,22 @@ delayed_action_schedule (NMPlatform *platform, DelayedActionType action_type, gp
 		priv->delayed_action.idle_id = g_idle_add (delayed_action_handle_idle, platform);
 }
 
+static void
+delayed_action_schedule_WAIT_FOR_NL_RESPONSE (NMPlatform *platform,
+                                              guint32 seq_number,
+                                              WaitForNlResponseResult *out_seq_result)
+{
+	DelayedActionWaitForNlResponseData data = {
+		.seq_number = seq_number,
+		.timeout_abs_ns = nm_utils_get_monotonic_timestamp_ns () + (200 * (NM_UTILS_NS_PER_SECOND / 1000)),
+		.out_seq_result = out_seq_result,
+	};
+
+	delayed_action_schedule (platform,
+	                         DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE,
+	                         &data);
+}
+
 /******************************************************************/
 
 static void
@@ -3148,7 +3311,9 @@ cache_pre_hook (NMPCache *cache, const NMPObject *old, const NMPObject *new, NMP
 /******************************************************************/
 
 static int
-_nl_send_auto_with_seq (NMPlatform *platform, struct nl_msg *nlmsg)
+_nl_send_auto_with_seq (NMPlatform *platform,
+                        struct nl_msg *nlmsg,
+                        WaitForNlResponseResult *out_seq_result)
 {
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	guint32 seq;
@@ -3161,21 +3326,16 @@ _nl_send_auto_with_seq (NMPlatform *platform, struct nl_msg *nlmsg)
 
 	nle = nl_send_auto (priv->nlh_event, nlmsg);
 
-	if (nle >= 0) {
-		_LOGt ("sequence-number: new %u%s",
-		       seq,
-		       priv->nlh_seq_expect
-		           ? nm_sprintf_bufa (100, " (replaces %u)", priv->nlh_seq_expect)
-		           : "");
-		priv->nlh_seq_expect = seq;
-	} else
+	if (nle >= 0)
+		delayed_action_schedule_WAIT_FOR_NL_RESPONSE (platform, seq, out_seq_result);
+	else
 		_LOGD ("failed sending message: %s (%d)", nl_geterror (nle), nle);
 
 	return nle;
 }
 
 static void
-do_request_link (NMPlatform *platform, int ifindex, const char *name, gboolean handle_delayed_action)
+do_request_link_no_delayed_actions (NMPlatform *platform, int ifindex, const char *name)
 {
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	nm_auto_nlmsg struct nl_msg *nlmsg = NULL;
@@ -3201,24 +3361,18 @@ do_request_link (NMPlatform *platform, int ifindex, const char *name, gboolean h
 	                          0,
 	                          0);
 	if (nlmsg)
-		_nl_send_auto_with_seq (platform, nlmsg);
-
-	event_handler_read_netlink_all (platform, TRUE);
-
-	cache_prune_candidates_prune (platform);
-
-	if (handle_delayed_action)
-		delayed_action_handle_all (platform, FALSE);
+		_nl_send_auto_with_seq (platform, nlmsg, NULL);
 }
 
 static void
-do_request_one_type (NMPlatform *platform, NMPObjectType obj_type, gboolean handle_delayed_action)
+do_request_link (NMPlatform *platform, int ifindex, const char *name)
 {
-	do_request_all (platform, delayed_action_refresh_from_object_type (obj_type), handle_delayed_action);
+	do_request_link_no_delayed_actions (platform, ifindex, name);
+	delayed_action_handle_all (platform, FALSE);
 }
 
 static void
-do_request_all (NMPlatform *platform, DelayedActionType action_type, gboolean handle_delayed_action)
+do_request_all_no_delayed_actions (NMPlatform *platform, DelayedActionType action_type)
 {
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	DelayedActionType iflags;
@@ -3264,40 +3418,56 @@ do_request_all (NMPlatform *platform, DelayedActionType action_type, gboolean ha
 			if (nle < 0)
 				goto next;
 
-			_nl_send_auto_with_seq (platform, nlmsg);
+			_nl_send_auto_with_seq (platform, nlmsg, NULL);
 		}
 next:
 		;
 	}
-	event_handler_read_netlink_all (platform, TRUE);
-
-	cache_prune_candidates_prune (platform);
-
-	if (handle_delayed_action)
-		delayed_action_handle_all (platform, FALSE);
 }
 
 static void
-event_seq_check (NMPlatform *platform, struct nl_msg *msg)
+do_request_one_type (NMPlatform *platform, NMPObjectType obj_type)
+{
+	do_request_all_no_delayed_actions (platform, delayed_action_refresh_from_object_type (obj_type));
+	delayed_action_handle_all (platform, FALSE);
+}
+
+static void
+event_seq_check (NMPlatform *platform, struct nl_msg *msg, WaitForNlResponseResult seq_result)
 {
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
-	struct nlmsghdr *hdr;
+	DelayedActionWaitForNlResponseData *data;
+	guint32 seq_number;
+	guint i;
 
-	hdr = nlmsg_hdr (msg);
+	seq_number = nlmsg_hdr (msg)->nlmsg_seq;
 
-	if (hdr->nlmsg_seq == 0)
+	if (seq_number == 0)
 		return;
 
-	priv->nlh_seq_last = hdr->nlmsg_seq;
+	if (NM_FLAGS_HAS (priv->delayed_action.flags, DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE)) {
+		nm_assert (priv->delayed_action.list_wait_for_nl_response->len > 0);
 
-	if (priv->nlh_seq_expect == 0)
-		_LOGt ("sequence-number: seq %u received (not waited)", hdr->nlmsg_seq);
-	else if (hdr->nlmsg_seq == priv->nlh_seq_expect) {
-		_LOGt ("sequence-number: seq %u received", hdr->nlmsg_seq);
+		for (i = 0; i < priv->delayed_action.list_wait_for_nl_response->len; i++) {
+			data = &g_array_index (priv->delayed_action.list_wait_for_nl_response, DelayedActionWaitForNlResponseData, i);
 
-		priv->nlh_seq_expect = 0;
-	} else
-		_LOGt ("sequence-number: seq %u received (wait for %u)", hdr->nlmsg_seq, priv->nlh_seq_last);
+			if (data->seq_number == seq_number) {
+				/* We potentially receive many parts partial responses for the same sequence number.
+				 * Thus, we only remember the result, and collect it later. */
+				if (data->seq_result < 0) {
+					/* we already saw an error for this seqence number.
+					 * Preserve it. */
+				} else if (   seq_result != WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_UNKNOWN
+				           || data->seq_result == WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN)
+					data->seq_result = seq_result;
+				return;
+			}
+		}
+	}
+
+	if (seq_number != priv->nlh_seq_last_handled)
+		_LOGt ("netlink: recvmsg: unwaited sequence number %u", seq_number);
+	priv->nlh_seq_last_handled = seq_number;
 }
 
 static void
@@ -3507,7 +3677,7 @@ do_add_link (NMPlatform *platform,
 		_LOGt ("do-add-link[%s/%s]: the added link is not yet ready. Request anew",
 		       name,
 		       nm_link_type_to_string (link_type));
-		do_request_link (platform, 0, name, TRUE);
+		do_request_link (platform, 0, name);
 	}
 
 	/* Return true, because the netlink request succeeded. This doesn't indicate that the
@@ -3584,7 +3754,7 @@ do_add_addrroute (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *
 		_LOGt ("do-add-%s[%s]: the added object is not yet ready. Request anew",
 		       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
 		       nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0));
-		do_request_one_type (platform, NMP_OBJECT_GET_TYPE (obj_id), TRUE);
+		do_request_one_type (platform, NMP_OBJECT_GET_TYPE (obj_id));
 	}
 
 	/* The return value doesn't say, whether the object is in the platform cache after adding
@@ -3661,14 +3831,14 @@ nle_failure:
 			_LOGt ("do-delete-%s[%s]: reload: the deleted object is not yet removed. Request anew",
 			       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
 			       nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0));
-			do_request_link (platform, obj_id->link.ifindex, obj_id->link.name, TRUE);
+			do_request_link (platform, obj_id->link.ifindex, obj_id->link.name);
 		}
 	} else {
 		if (nmp_cache_lookup_obj (priv->cache, obj_id)) {
 			_LOGt ("do-delete-%s[%s]: reload: the deleted object is not yet removed. Request anew",
 			       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
 			       nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0));
-			do_request_one_type (platform, NMP_OBJECT_GET_TYPE (obj_id), TRUE);
+			do_request_one_type (platform, NMP_OBJECT_GET_TYPE (obj_id));
 		}
 	}
 
@@ -3719,7 +3889,7 @@ retry:
 
 	/* FIXME: as we modify the link via a separate socket, the cache is not in
 	 * sync and we have to refetch the link. */
-	do_request_link (platform, ifindex, NULL, TRUE);
+	do_request_link (platform, ifindex, NULL);
 	return NM_PLATFORM_ERROR_SUCCESS;
 }
 
@@ -3834,7 +4004,7 @@ link_get_unmanaged (NMPlatform *platform, int ifindex, gboolean *unmanaged)
 static gboolean
 link_refresh (NMPlatform *platform, int ifindex)
 {
-	do_request_link (platform, ifindex, NULL, TRUE);
+	do_request_link (platform, ifindex, NULL);
 	return !!cache_lookup_link (platform, ifindex);
 }
 
@@ -4464,7 +4634,6 @@ link_vxlan_add (NMPlatform *platform,
 	nla_nest_end (nlmsg, info);
 
 	return do_add_link_with_lookup (platform, NM_LINK_TYPE_VXLAN, name, nlmsg, out_link);
-
 nla_put_failure:
 	g_return_val_if_reached (FALSE);
 }
@@ -4688,7 +4857,7 @@ tun_add (NMPlatform *platform, const char *name, gboolean tap,
 		close (fd);
 		return FALSE;
 	}
-	do_request_link (platform, 0, name, TRUE);
+	do_request_link (platform, 0, name);
 	obj = nmp_cache_lookup_link_full (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache,
 	                                  0, name, FALSE,
 	                                  tap ? NM_LINK_TYPE_TAP : NM_LINK_TYPE_TUN,
@@ -4752,7 +4921,7 @@ infiniband_partition_add (NMPlatform *platform, int parent, int p_key, const NMP
 	if (!nm_platform_sysctl_set (platform, path, id))
 		return FALSE;
 
-	do_request_link (platform, 0, ifname, TRUE);
+	do_request_link (platform, 0, ifname);
 
 	obj = nmp_cache_lookup_link_full (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache,
 	                                  0, ifname, FALSE, NM_LINK_TYPE_INFINIBAND, NULL, NULL);
@@ -5259,7 +5428,7 @@ ip4_route_delete (NMPlatform *platform, int ifindex, in_addr_t network, int plen
 			 * FIXME: once our ip4_address_add() is sure that upon return we have
 			 * the latest state from in the platform cache, we might save this
 			 * additional expensive cache-resync. */
-			do_request_one_type (platform, NMP_OBJECT_TYPE_IP4_ROUTE, TRUE);
+			do_request_one_type (platform, NMP_OBJECT_TYPE_IP4_ROUTE);
 
 			if (!nmp_cache_lookup_obj (priv->cache, &obj_id))
 				return TRUE;
@@ -5382,6 +5551,7 @@ event_handler_recvmsgs (NMPlatform *platform, gboolean handle_events)
 	int n, err = 0, multipart = 0, interrupted = 0, nrecv = 0;
 	unsigned char *buf = NULL;
 	struct nlmsghdr *hdr;
+	WaitForNlResponseResult seq_result;
 
 	/*
 	nla is passed on to not only to nl_recv() but may also be passed
@@ -5414,6 +5584,8 @@ continue_reading:
 
 	hdr = (struct nlmsghdr *) buf;
 	while (nlmsg_ok (hdr, n)) {
+		gboolean abort_parsing = FALSE;
+
 		nlmsg_free (msg);
 		msg = nlmsg_convert (hdr);
 		if (!msg) {
@@ -5433,11 +5605,12 @@ continue_reading:
 			goto stop;
 		}
 
+		_LOGt ("netlink: recvmsg: new message type %d, seq %u",
+		       hdr->nlmsg_type, hdr->nlmsg_seq);
+
 		if (creds)
 			nlmsg_set_creds (msg, creds);
 
-		event_seq_check (platform, msg);
-
 		if (hdr->nlmsg_flags & NLM_F_MULTI)
 			multipart = 1;
 
@@ -5455,24 +5628,26 @@ continue_reading:
 			/* FIXME: implement */
 		}
 
+		seq_result = WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_UNKNOWN;
+
 		if (hdr->nlmsg_type == NLMSG_DONE) {
 			/* messages terminates a multipart message, this is
 			 * usually the end of a message and therefore we slip
 			 * out of the loop by default. the user may overrule
 			 * this action by skipping this packet. */
 			multipart = 0;
+			seq_result = WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK;
 		} else if (hdr->nlmsg_type == NLMSG_NOOP) {
 			/* Message to be ignored, the default action is to
 			 * skip this message if no callback is specified. The
 			 * user may overrule this action by returning
 			 * NL_PROCEED. */
-			goto skip;
 		} else if (hdr->nlmsg_type == NLMSG_OVERRUN) {
 			/* Data got lost, report back to user. The default action is to
 			 * quit parsing. The user may overrule this action by retuning
 			 * NL_SKIP or NL_PROCEED (dangerous) */
 			err = -NLE_MSG_OVERFLOW;
-			goto out;
+			abort_parsing = TRUE;
 		} else if (hdr->nlmsg_type == NLMSG_ERROR) {
 			/* Message carries a nlmsgerr */
 			struct nlmsgerr *e = nlmsg_data (hdr);
@@ -5483,23 +5658,32 @@ continue_reading:
 				 * this action by returning NL_SKIP or
 				 * NL_PROCEED (dangerous) */
 				err = -NLE_MSG_TRUNC;
-				goto out;
+				abort_parsing = TRUE;
 			} else if (e->error) {
+				int errsv = e->error > 0 ? e->error : -e->error;
+
 				/* Error message reported back from kernel. */
 				_LOGD ("netlink: recvmsg: error message from kernel: %s (%d) for request %d",
-				       strerror (e->error),
-				       e->error,
-				       priv->nlh_seq_last);
-			}
+				       strerror (errsv),
+				       errsv,
+				       nlmsg_hdr (msg)->nlmsg_seq);
+				seq_result = -errsv;
+			} else
+				seq_result = WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK;
 		} else {
 			/* Valid message (not checking for MULTIPART bit to
 			 * get along with broken kernels. NL_SKIP has no
 			 * effect on this.  */
 			event_valid_msg (platform, msg);
+			seq_result = WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK;
 		}
-skip:
+
+		event_seq_check (platform, msg, seq_result);
 		err = 0;
 		hdr = nlmsg_next (hdr, &n);
+
+		if (abort_parsing)
+			goto out;
 	}
 
 	nlmsg_free (msg);
@@ -5534,7 +5718,6 @@ out:
 static gboolean
 event_handler_read_netlink_one (NMPlatform *platform)
 {
-	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	int nle;
 
 	nle = event_handler_recvmsgs (platform, TRUE);
@@ -5550,8 +5733,8 @@ event_handler_read_netlink_one (NMPlatform *platform)
 			_LOGI ("netlink: read-one: too many netlink events. Need to resynchronize platform cache");
 			/* Drain the event queue, we've lost events and are out of sync anyway and we'd
 			 * like to free up some space. We'll read in the status synchronously. */
+			delayed_action_wait_for_nl_response_complete_all (platform, WAIT_FOR_NL_RESPONSE_RESULT_FAILED_RESYNC);
 			event_handler_recvmsgs (platform, FALSE);
-			priv->nlh_seq_expect = 0;
 			delayed_action_schedule (platform,
 			                         DELAYED_ACTION_TYPE_REFRESH_ALL_LINKS |
 			                         DELAYED_ACTION_TYPE_REFRESH_ALL_IP4_ADDRESSES |
@@ -5574,62 +5757,76 @@ event_handler_read_netlink_all (NMPlatform *platform, gboolean wait_for_acks)
 	int r;
 	struct pollfd pfd;
 	gboolean any = FALSE;
-	gint64 timestamp = 0, now;
-	const int TIMEOUT = 250;
-	int timeout = 0;
-	guint32 wait_for_seq = 0;
+	gint64 now_ns;
+	int timeout_ms;
+	guint i;
+	struct {
+		guint32 seq_number;
+		gint64 timeout_abs_ns;
+	} data_next;
 
 	while (TRUE) {
 		while (event_handler_read_netlink_one (platform))
 			any = TRUE;
 
-		if (!wait_for_acks || priv->nlh_seq_expect == 0) {
-			if (wait_for_seq)
-				_LOGT ("netlink: read-all: ACK for sequence number %u received", priv->nlh_seq_expect);
+after_read:
+
+		if (!NM_FLAGS_HAS (priv->delayed_action.flags, DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE))
 			return any;
-		}
 
-		now = nm_utils_get_monotonic_timestamp_ms ();
-		if (wait_for_seq != priv->nlh_seq_expect) {
-			/* We are waiting for a new sequence number (or we will wait for the first time).
-			 * Reset/start counting the overall wait time. */
-			_LOGT ("netlink: read-all: wait for ACK for sequence number %u...", priv->nlh_seq_expect);
-			wait_for_seq = priv->nlh_seq_expect;
-			timestamp = now;
-			timeout = TIMEOUT;
-		} else {
-			if ((now - timestamp) >= TIMEOUT) {
-				/* timeout. Don't wait for this sequence number anymore. */
-				break;
+		now_ns = 0;
+		data_next.seq_number = 0;
+		data_next.timeout_abs_ns = 0;
+
+		for (i = 0; i < priv->delayed_action.list_wait_for_nl_response->len; ) {
+			DelayedActionWaitForNlResponseData *data = &g_array_index (priv->delayed_action.list_wait_for_nl_response, DelayedActionWaitForNlResponseData, i);
+
+			if (data->seq_result)
+				delayed_action_wait_for_nl_response_complete (platform, i, data->seq_result);
+			else if ((now_ns ?: (now_ns = nm_utils_get_monotonic_timestamp_ns ())) > data->timeout_abs_ns)
+				delayed_action_wait_for_nl_response_complete (platform, i, WAIT_FOR_NL_RESPONSE_RESULT_FAILED_TIMEOUT);
+			else {
+				i++;
+
+				if (   data_next.seq_number == 0
+				    || data_next.timeout_abs_ns > data->timeout_abs_ns)
+					data_next.seq_number = data->seq_number;
+					data_next.timeout_abs_ns = data->timeout_abs_ns;
 			}
-
-			/* readjust the wait-time. */
-			timeout = TIMEOUT - (now - timestamp);
 		}
 
+		if (   !wait_for_acks
+		    || !NM_FLAGS_HAS (priv->delayed_action.flags, DELAYED_ACTION_TYPE_WAIT_FOR_NL_RESPONSE))
+			return any;
+
+		nm_assert (data_next.seq_number);
+		nm_assert (data_next.timeout_abs_ns > 0);
+		nm_assert (now_ns > 0);
+
+		_LOGT ("netlink: read-all: wait for ACK for sequence number %u...", data_next.seq_number);
+
+		timeout_ms = (data_next.timeout_abs_ns - now_ns) / (NM_UTILS_NS_PER_SECOND / 1000);
+
 		memset (&pfd, 0, sizeof (pfd));
 		pfd.fd = nl_socket_get_fd (priv->nlh_event);
 		pfd.events = POLLIN;
-		r = poll (&pfd, 1, timeout);
+		r = poll (&pfd, 1, MAX (1, timeout_ms));
 
 		if (r == 0) {
-			/* timeout. */
-			break;
+			/* timeout and there is nothing to read. */
+			goto after_read;
 		}
 		if (r < 0) {
 			int errsv = errno;
 
 			if (errsv != EINTR) {
 				_LOGE ("netlink: read-all: poll failed with %s", strerror (errsv));
+				delayed_action_wait_for_nl_response_complete_all (platform, WAIT_FOR_NL_RESPONSE_RESULT_FAILED_POLL);
 				return any;
 			}
 			/* Continue to read again, even if there might be nothing to read after EINTR. */
 		}
 	}
-
-	_LOGW ("netlink: read-all: timeout waiting for ACK to sequence number %u...", wait_for_seq);
-	priv->nlh_seq_expect = 0;
-	return any;
 }
 
 /******************************************************************/
@@ -5750,6 +5947,7 @@ nm_linux_platform_init (NMLinuxPlatform *self)
 	priv->cache = nmp_cache_new ();
 	priv->delayed_action.list_master_connected = g_ptr_array_new ();
 	priv->delayed_action.list_refresh_link = g_ptr_array_new ();
+	priv->delayed_action.list_wait_for_nl_response = g_array_new (FALSE, TRUE, sizeof (DelayedActionWaitForNlResponseData));
 	priv->wifi_data = g_hash_table_new_full (NULL, NULL, NULL, (GDestroyNotify) wifi_utils_deinit);
 }
 
@@ -5866,6 +6064,8 @@ dispose (GObject *object)
 
 	_LOGD ("dispose");
 
+	delayed_action_wait_for_nl_response_complete_all (platform, WAIT_FOR_NL_RESPONSE_RESULT_FAILED_DISPOSING);
+
 	priv->delayed_action.flags = DELAYED_ACTION_TYPE_NONE;
 	g_ptr_array_set_size (priv->delayed_action.list_master_connected, 0);
 	g_ptr_array_set_size (priv->delayed_action.list_refresh_link, 0);
@@ -5886,6 +6086,7 @@ nm_linux_platform_finalize (GObject *object)
 
 	g_ptr_array_unref (priv->delayed_action.list_master_connected);
 	g_ptr_array_unref (priv->delayed_action.list_refresh_link);
+	g_array_unref (priv->delayed_action.list_wait_for_nl_response);
 
 	/* Free netlink resources */
 	g_source_remove (priv->event_id);

From dce5e6d81591134060402eb1157fef353874ecf6 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 17 Dec 2015 18:24:57 +0100
Subject: [PATCH 14/27] platform: assert that delayed_action_handle_all() is
 not called recursively

We would not expect that delayed_action_handle_all() is called recursively.
Assert against that. If we ever happen to call it recursively, we would
need to take care of properly avoiding infinite loops or deep call
stacks.
---
 src/platform/nm-linux-platform.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 5c355d2e3..3f87c2369 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -2888,8 +2888,7 @@ delayed_action_handle_one (NMPlatform *platform)
 		return TRUE;
 	}
 
-	if (   NM_FLAGS_ANY (priv->delayed_action.flags, DELAYED_ACTION_TYPE_REFRESH_ALL)
-	    && priv->delayed_action.is_handling < 5 /* avoid deep recursive stacks */) {
+	if (NM_FLAGS_ANY (priv->delayed_action.flags, DELAYED_ACTION_TYPE_REFRESH_ALL)) {
 		DelayedActionType flags, iflags;
 
 		flags = priv->delayed_action.flags & DELAYED_ACTION_TYPE_REFRESH_ALL;
@@ -2907,8 +2906,7 @@ delayed_action_handle_one (NMPlatform *platform)
 		return TRUE;
 	}
 
-	if (   NM_FLAGS_HAS (priv->delayed_action.flags, DELAYED_ACTION_TYPE_REFRESH_LINK)
-	    && priv->delayed_action.is_handling < 5 /* avoid deep recursive stacks */) {
+	if (NM_FLAGS_HAS (priv->delayed_action.flags, DELAYED_ACTION_TYPE_REFRESH_LINK)) {
 		nm_assert (priv->delayed_action.list_refresh_link->len > 0);
 
 		user_data = priv->delayed_action.list_refresh_link->pdata[0];
@@ -2940,7 +2938,10 @@ delayed_action_handle_all (NMPlatform *platform, gboolean read_netlink)
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	gboolean any = FALSE;
 
+	g_return_val_if_fail (priv->delayed_action.is_handling == 0, FALSE);
+
 	nm_clear_g_source (&priv->delayed_action.idle_id);
+
 	priv->delayed_action.is_handling++;
 	if (read_netlink)
 		delayed_action_schedule (platform, DELAYED_ACTION_TYPE_READ_NETLINK, NULL);
@@ -2948,8 +2949,7 @@ delayed_action_handle_all (NMPlatform *platform, gboolean read_netlink)
 		any = TRUE;
 	priv->delayed_action.is_handling--;
 
-	if (priv->delayed_action.is_handling <= 0)
-		cache_prune_candidates_prune (platform);
+	cache_prune_candidates_prune (platform);
 
 	return any;
 }

From f8378800bb3c9718107862bc0641bc1a320724f7 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 14 Dec 2015 17:16:13 +0100
Subject: [PATCH 15/27] platform: add links via event netlink socket

---
 src/platform/nm-linux-platform.c | 94 +++++++++++++-------------------
 1 file changed, 37 insertions(+), 57 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 3f87c2369..b0e7d0087 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -3632,59 +3632,6 @@ link_get_lnk (NMPlatform *platform, int ifindex, NMLinkType link_type, const NMP
 
 /*****************************************************************************/
 
-static gboolean
-do_add_link (NMPlatform *platform,
-             NMLinkType link_type,
-             const char *name,
-             struct nl_msg *nlmsg)
-{
-	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
-	int nle;
-
-	event_handler_read_netlink_all (platform, FALSE);
-
-	nle = nl_send_auto (priv->nlh, nlmsg);
-	if (nle < 0) {
-		_LOGE ("do-add-link[%s/%s]: failure sending netlink request \"%s\" (%d)",
-		       name,
-		       nm_link_type_to_string (link_type),
-		       nl_geterror (nle), -nle);
-		return FALSE;
-	}
-
-	nle = nl_wait_for_ack (priv->nlh);
-	switch (nle) {
-	case -NLE_SUCCESS:
-		_LOGD ("do-add-link[%s/%s]: success adding",
-		       name,
-		       nm_link_type_to_string (link_type));
-		break;
-	default:
-		_LOGE ("do-add-link[%s/%s]: failed with \"%s\" (%d)",
-		       name,
-		       nm_link_type_to_string (link_type),
-		       nl_geterror (nle), -nle);
-		return FALSE;
-	}
-
-	delayed_action_handle_all (platform, TRUE);
-
-	/* FIXME: we add the link object via the second netlink socket. Sometimes,
-	 * the notification is not yet ready via nlh_event, so we have to re-request the
-	 * link so that it is in the cache. A better solution would be to do everything
-	 * via one netlink socket. */
-	if (!nmp_cache_lookup_link_full (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache, 0, name, FALSE, NM_LINK_TYPE_NONE, NULL, NULL)) {
-		_LOGt ("do-add-link[%s/%s]: the added link is not yet ready. Request anew",
-		       name,
-		       nm_link_type_to_string (link_type));
-		do_request_link (platform, 0, name);
-	}
-
-	/* Return true, because the netlink request succeeded. This doesn't indicate that the
-	 * object is now actually in the cache, because there could be a race. */
-	return TRUE;
-}
-
 static gboolean
 do_add_link_with_lookup (NMPlatform *platform,
                          NMLinkType link_type,
@@ -3692,12 +3639,45 @@ do_add_link_with_lookup (NMPlatform *platform,
                          struct nl_msg *nlmsg,
                          const NMPlatformLink **out_link)
 {
-	const NMPObject *obj;
+	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
+	const NMPObject *obj = NULL;
+	WaitForNlResponseResult seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
+	int nle;
+	char s_buf[256];
 
-	do_add_link (platform, link_type, name, nlmsg);
+	event_handler_read_netlink_all (platform, FALSE);
+
+	nle = _nl_send_auto_with_seq (platform, nlmsg, &seq_result);
+	if (nle < 0) {
+		_LOGE ("do-add-link[%s/%s]: failed sending netlink request \"%s\" (%d)",
+		       name,
+		       nm_link_type_to_string (link_type),
+		       nl_geterror (nle), -nle);
+		return FALSE;
+	}
+
+	delayed_action_handle_all (platform, FALSE);
+
+	nm_assert (seq_result);
+
+	_NMLOG (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK
+	            ? LOGL_DEBUG
+	            : LOGL_ERR,
+	        "do-add-link[%s/%s]: %s",
+	        name,
+	        nm_link_type_to_string (link_type),
+	        wait_for_nl_response_to_string (seq_result, s_buf, sizeof (s_buf)));
+
+	if (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK)
+		obj = nmp_cache_lookup_link_full (priv->cache, 0, name, FALSE, link_type, NULL, NULL);
+
+	if (!obj) {
+		/* either kernel signaled failure, or it signaled success and the link object
+		 * is not (yet) in the cache. Try to reload it... */
+		do_request_link (platform, 0, name);
+		obj = nmp_cache_lookup_link_full (priv->cache, 0, name, FALSE, link_type, NULL, NULL);
+	}
 
-	obj = nmp_cache_lookup_link_full (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache,
-	                                  0, name, FALSE, link_type, NULL, NULL);
 	if (out_link)
 		*out_link = obj ? &obj->link : NULL;
 	return !!obj;

From be28608a8faa2550319fe871d2058861d1d8f233 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 14 Dec 2015 17:16:13 +0100
Subject: [PATCH 16/27] platform: add addresses and routes via event netlink
 socket

---
 src/platform/nm-linux-platform.c | 55 +++++++++++++-------------------
 1 file changed, 23 insertions(+), 32 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index b0e7d0087..5c7b4eefc 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -3686,8 +3686,9 @@ do_add_link_with_lookup (NMPlatform *platform,
 static gboolean
 do_add_addrroute (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *nlmsg)
 {
-	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
+	WaitForNlResponseResult seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
 	int nle;
+	char s_buf[256];
 
 	nm_assert (NM_IN_SET (NMP_OBJECT_GET_TYPE (obj_id),
 	                      NMP_OBJECT_TYPE_IP4_ADDRESS, NMP_OBJECT_TYPE_IP6_ADDRESS,
@@ -3695,7 +3696,7 @@ do_add_addrroute (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *
 
 	event_handler_read_netlink_all (platform, FALSE);
 
-	nle = nl_send_auto (priv->nlh, nlmsg);
+	nle = _nl_send_auto_with_seq (platform, nlmsg, &seq_result);
 	if (nle < 0) {
 		_LOGE ("do-add-%s[%s]: failure sending netlink request \"%s\" (%d)",
 		       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
@@ -3704,42 +3705,32 @@ do_add_addrroute (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *
 		return FALSE;
 	}
 
-	nle = nl_wait_for_ack (priv->nlh);
-	switch (nle) {
-	case -NLE_SUCCESS:
-		_LOGD ("do-add-%s[%s]: success adding", NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name, nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0));
-		break;
-	case -NLE_EXIST:
-		/* NLE_EXIST is considered equivalent to success to avoid race conditions. You
-		 * never know when something sends an identical object just before
-		 * NetworkManager. */
-		_LOGD ("do-add-%s[%s]: adding link failed with \"%s\" (%d), meaning such a link already exists",
-		       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
-		       nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0),
-		       nl_geterror (nle), -nle);
-		break;
-	default:
-		_LOGE ("do-add-%s[%s]: failed with \"%s\" (%d)",
-		       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
-		       nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0),
-		       nl_geterror (nle), -nle);
-		return FALSE;
-	}
+	delayed_action_handle_all (platform, FALSE);
 
-	delayed_action_handle_all (platform, TRUE);
+	nm_assert (seq_result);
 
-	/* FIXME: instead of re-requesting the added object, add it via nlh_event
-	 * so that the events are in sync. */
-	if (!nmp_cache_lookup_obj (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache, obj_id)) {
-		_LOGt ("do-add-%s[%s]: the added object is not yet ready. Request anew",
-		       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
-		       nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0));
+	_NMLOG (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK
+	            ? LOGL_DEBUG
+	            : LOGL_ERR,
+	        "do-add-%s[%s]: %s",
+	        NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
+	        nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0),
+	        wait_for_nl_response_to_string (seq_result, s_buf, sizeof (s_buf)));
+
+	/* In rare cases, the object is not yet ready as we received the ACK from
+	 * kernel. Need to refetch.
+	 *
+	 * We want to safe the expensive refetch, thus we look first into the cache
+	 * whether the object exists.
+	 *
+	 * FIXME: if the object already existed previously, we might not notice a
+	 * missing update. */
+	if (!nmp_cache_lookup_obj (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache, obj_id))
 		do_request_one_type (platform, NMP_OBJECT_GET_TYPE (obj_id));
-	}
 
 	/* The return value doesn't say, whether the object is in the platform cache after adding
 	 * it. Instead the return value says, whether the netlink request succeeded. */
-	return TRUE;
+	return seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK;
 }
 
 static gboolean

From 109796707769b3f4218e01a3280fc4749f023bdd Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 14 Dec 2015 18:42:29 +0100
Subject: [PATCH 17/27] platform: delete objects via event netlink socket

---
 src/platform/nm-linux-platform.c | 91 +++++++++++---------------------
 1 file changed, 31 insertions(+), 60 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 5c7b4eefc..33ae85984 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -3736,12 +3736,15 @@ do_add_addrroute (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *
 static gboolean
 do_delete_object (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *nlmsg)
 {
-	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
+	WaitForNlResponseResult seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
 	int nle;
+	char s_buf[256];
+	gboolean success = TRUE;
+	const char *log_detail = "";
 
 	event_handler_read_netlink_all (platform, FALSE);
 
-	nle = nl_send_auto (priv->nlh, nlmsg);
+	nle = _nl_send_auto_with_seq (platform, nlmsg, &seq_result);
 	if (nle < 0) {
 		_LOGE ("do-delete-%s[%s]: failure sending netlink request \"%s\" (%d)",
 		       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
@@ -3750,72 +3753,40 @@ do_delete_object (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *
 		return FALSE;
 	}
 
-	nle = nl_wait_for_ack (priv->nlh);
-	switch (nle) {
-	case -NLE_SUCCESS:
-		_LOGD ("do-delete-%s[%s]: success deleting", NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name, nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0));
-		break;
-	case -NLE_OBJ_NOTFOUND:
-		_LOGD ("do-delete-%s[%s]: failed with \"%s\" (%d), meaning the object was already removed",
-		       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
-		       nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0),
-		       nl_geterror (nle), -nle);
-		break;
-	case -NLE_FAILURE:
-		if (NMP_OBJECT_GET_TYPE (obj_id) != NMP_OBJECT_TYPE_IP6_ADDRESS)
-			goto nle_failure;
+	delayed_action_handle_all (platform, FALSE);
 
-		/* On RHEL7 kernel, deleting a non existing address fails with ENXIO (which libnl maps to NLE_FAILURE) */
-		_LOGD ("do-delete-%s[%s]: deleting address failed with \"%s\" (%d), meaning the address was already removed",
-		       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
-		       nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0),
-		       nl_geterror (nle), -nle);
-		break;
-	case -NLE_NOADDR:
-		if (   NMP_OBJECT_GET_TYPE (obj_id) != NMP_OBJECT_TYPE_IP4_ADDRESS
-		    && NMP_OBJECT_GET_TYPE (obj_id) != NMP_OBJECT_TYPE_IP6_ADDRESS)
-			goto nle_failure;
+	nm_assert (seq_result);
 
-		_LOGD ("do-delete-%s[%s]: deleting address failed with \"%s\" (%d), meaning the address was already removed",
-		       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
-		       nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0),
-		       nl_geterror (nle), -nle);
-		break;
-	default:
-nle_failure:
-		_LOGE ("do-delete-%s[%s]: failed with \"%s\" (%d)",
-		       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
-		       nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0),
-		       nl_geterror (nle), -nle);
-		return FALSE;
-	}
+	if (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK) {
+		/* ok */
+	} else if (NM_IN_SET (-((int) seq_result), ESRCH, ENOENT))
+		log_detail = ", meaning the object was already removed";
+	else if (   NM_IN_SET (-((int) seq_result), ENXIO)
+	         && NM_IN_SET (NMP_OBJECT_GET_TYPE (obj_id), NMP_OBJECT_TYPE_IP6_ADDRESS)) {
+		/* On RHEL7 kernel, deleting a non existing address fails with ENXIO */
+		log_detail = ", meaning the address was already removed";
+	} else if (   NM_IN_SET (-((int) seq_result), EADDRNOTAVAIL)
+	           && NM_IN_SET (NMP_OBJECT_GET_TYPE (obj_id), NMP_OBJECT_TYPE_IP4_ADDRESS, NMP_OBJECT_TYPE_IP6_ADDRESS))
+		log_detail = ", meaning the address was already removed";
+	else
+		success = FALSE;
 
-	delayed_action_handle_all (platform, TRUE);
+	_NMLOG (success ? LOGL_DEBUG : LOGL_ERR,
+	        "do-delete-%s[%s]: %s%s",
+	        NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
+	        nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0),
+	        wait_for_nl_response_to_string (seq_result, s_buf, sizeof (s_buf)),
+	        log_detail);
 
-	/* FIXME: instead of re-requesting the deleted object, add it via nlh_event
-	 * so that the events are in sync. */
-	if (NMP_OBJECT_GET_TYPE (obj_id) == NMP_OBJECT_TYPE_LINK) {
-		const NMPObject *obj;
-
-		obj = nmp_cache_lookup_link_full (priv->cache, obj_id->link.ifindex, obj_id->link.ifindex <= 0 && obj_id->link.name[0] ? obj_id->link.name : NULL, FALSE, NM_LINK_TYPE_NONE, NULL, NULL);
-		if (obj && obj->_link.netlink.is_in_netlink) {
-			_LOGt ("do-delete-%s[%s]: reload: the deleted object is not yet removed. Request anew",
-			       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
-			       nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0));
-			do_request_link (platform, obj_id->link.ifindex, obj_id->link.name);
-		}
-	} else {
-		if (nmp_cache_lookup_obj (priv->cache, obj_id)) {
-			_LOGt ("do-delete-%s[%s]: reload: the deleted object is not yet removed. Request anew",
-			       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
-			       nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0));
-			do_request_one_type (platform, NMP_OBJECT_GET_TYPE (obj_id));
-		}
+	if (nmp_cache_lookup_obj (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache, obj_id)) {
+		/* such an object still exists in the cache. To be sure, refetch it (and
+		 * hope it's gone) */
+		do_request_one_type (platform, NMP_OBJECT_GET_TYPE (obj_id));
 	}
 
 	/* The return value doesn't say, whether the object is in the platform cache after adding
 	 * it. Instead the return value says, whether the netlink request succeeded. */
-	return TRUE;
+	return success;
 }
 
 static NMPlatformError

From c73b9f6529f33ae8d1e07f824a4d90da76d2b419 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 14 Dec 2015 19:18:35 +0100
Subject: [PATCH 18/27] platform: change links via event netlink socket

---
 src/platform/nm-linux-platform.c | 64 ++++++++++++++++++--------------
 src/platform/tests/test-link.c   | 10 ++---
 2 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 33ae85984..e04f0f0cb 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -3790,13 +3790,19 @@ do_delete_object (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *
 }
 
 static NMPlatformError
-do_change_link (NMPlatform *platform, int ifindex, struct nl_msg *nlmsg)
+do_change_link (NMPlatform *platform,
+                int ifindex,
+                struct nl_msg *nlmsg)
 {
-	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
+	WaitForNlResponseResult seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
 	int nle;
+	char s_buf[256];
+	NMPlatformError result = NM_PLATFORM_ERROR_SUCCESS;
+	NMLogLevel log_level = LOGL_DEBUG;
+	const char *log_result = "failure", *log_detail = "";
 
 retry:
-	nle = nl_send_auto_complete (priv->nlh, nlmsg);
+	nle = _nl_send_auto_with_seq (platform, nlmsg, &seq_result);
 	if (nle < 0) {
 		_LOGE ("do-change-link[%d]: failure sending netlink request \"%s\" (%d)",
 		       ifindex,
@@ -3804,35 +3810,39 @@ retry:
 		return NM_PLATFORM_ERROR_UNSPECIFIED;
 	}
 
-	nle = nl_wait_for_ack (priv->nlh);
-	if (   nle == -NLE_OPNOTSUPP
+	/* always refetch the link after changing it. There seems to be issues
+	 * and we sometimes lack events. Nuke it from the orbit... */
+	delayed_action_schedule (platform, DELAYED_ACTION_TYPE_REFRESH_LINK, GINT_TO_POINTER (ifindex));
+
+	delayed_action_handle_all (platform, FALSE);
+
+	nm_assert (seq_result);
+
+	if (   NM_IN_SET (-((int) seq_result), EOPNOTSUPP)
 	    && nlmsg_hdr (nlmsg)->nlmsg_type == RTM_NEWLINK) {
 		nlmsg_hdr (nlmsg)->nlmsg_type = RTM_SETLINK;
 		goto retry;
 	}
 
-	switch (nle) {
-	case -NLE_SUCCESS:
-		_LOGD ("do-change-link[%d]: success changing link", ifindex);
-		break;
-	case -NLE_EXIST:
-		_LOGD ("do-change-link[%d]: success changing link: %s (%d)",
-		       ifindex, nl_geterror (nle), -nle);
-		break;
-	case -NLE_OBJ_NOTFOUND:
-		_LOGD ("do-change-link[%d]: failure changing link: firmware not found (%s, %d)",
-		       ifindex, nl_geterror (nle), -nle);
-		return NM_PLATFORM_ERROR_NO_FIRMWARE;
-	default:
-		_LOGE ("do-change-link[%d]: failure changing link: netlink error (%s, %d)",
-		       ifindex, nl_geterror (nle), -nle);
-		return NM_PLATFORM_ERROR_UNSPECIFIED;
+	if (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK) {
+		log_result = "success";
+	} else if (NM_IN_SET (-((int) seq_result), EEXIST, EADDRINUSE)) {
+		/* */
+	} else if (NM_IN_SET (-((int) seq_result), ESRCH, ENOENT)) {
+		log_detail = ", firmware not found";
+		result = NM_PLATFORM_ERROR_NO_FIRMWARE;
+	} else {
+		log_level = LOGL_ERR;
+		result = NM_PLATFORM_ERROR_UNSPECIFIED;
 	}
+	_NMLOG (log_level,
+	        "do-change-link[%d]: %s changing link: %s%s",
+	        ifindex,
+	        log_result,
+	        wait_for_nl_response_to_string (seq_result, s_buf, sizeof (s_buf)),
+	        log_detail);
 
-	/* FIXME: as we modify the link via a separate socket, the cache is not in
-	 * sync and we have to refetch the link. */
-	do_request_link (platform, ifindex, NULL);
-	return NM_PLATFORM_ERROR_SUCCESS;
+	return result;
 }
 
 static gboolean
@@ -4056,7 +4066,7 @@ link_set_user_ipv6ll_enabled (NMPlatform *platform, int ifindex, gboolean enable
 	if (   !nlmsg
 	    || !_nl_msg_new_link_set_afspec (nlmsg,
 	                                     mode))
-		return FALSE;
+		g_return_val_if_reached (FALSE);
 
 	return do_change_link (platform, ifindex, nlmsg) == NM_PLATFORM_ERROR_SUCCESS;
 }
@@ -4745,7 +4755,7 @@ link_vlan_change (NMPlatform *platform,
 	                                            new_n_ingress_map,
 	                                            new_egress_map,
 	                                            new_n_egress_map))
-		return FALSE;
+		g_return_val_if_reached (FALSE);
 
 	return do_change_link (platform, ifindex, nlmsg) == NM_PLATFORM_ERROR_SUCCESS;
 }
diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c
index af6c4878b..8b4962620 100644
--- a/src/platform/tests/test-link.c
+++ b/src/platform/tests/test-link.c
@@ -32,16 +32,16 @@ test_bogus(void)
 	g_assert (!nm_platform_link_get_type (NM_PLATFORM_GET, BOGUS_IFINDEX));
 	g_assert (!nm_platform_link_get_type_name (NM_PLATFORM_GET, BOGUS_IFINDEX));
 
-	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure changing link: netlink error (No such device*");
+	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure changing link: *");
 	g_assert (!nm_platform_link_set_up (NM_PLATFORM_GET, BOGUS_IFINDEX, NULL));
 
-	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure changing link: netlink error (No such device*");
+	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure changing link: *");
 	g_assert (!nm_platform_link_set_down (NM_PLATFORM_GET, BOGUS_IFINDEX));
 
-	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure changing link: netlink error (No such device*");
+	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure changing link: *");
 	g_assert (!nm_platform_link_set_arp (NM_PLATFORM_GET, BOGUS_IFINDEX));
 
-	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure changing link: netlink error (No such device*");
+	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure changing link: *");
 	g_assert (!nm_platform_link_set_noarp (NM_PLATFORM_GET, BOGUS_IFINDEX));
 
 	g_assert (!nm_platform_link_is_up (NM_PLATFORM_GET, BOGUS_IFINDEX));
@@ -52,7 +52,7 @@ test_bogus(void)
 	g_assert (!addrlen);
 	g_assert (!nm_platform_link_get_address (NM_PLATFORM_GET, BOGUS_IFINDEX, NULL));
 
-	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure changing link: netlink error (No such device*");
+	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure changing link: *");
 	g_assert (!nm_platform_link_set_mtu (NM_PLATFORM_GET, BOGUS_IFINDEX, MTU));
 
 	g_assert (!nm_platform_link_get_mtu (NM_PLATFORM_GET, BOGUS_IFINDEX));

From d7782b976987dcd4aac79497ff7881d745836fd2 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 14 Dec 2015 20:13:01 +0100
Subject: [PATCH 19/27] platform: drop synchronous netlink socket

---
 src/platform/nm-linux-platform.c | 32 --------------------------------
 1 file changed, 32 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index e04f0f0cb..4a3cbfdde 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -2349,7 +2349,6 @@ typedef struct {
 typedef struct _NMLinuxPlatformPrivate NMLinuxPlatformPrivate;
 
 struct _NMLinuxPlatformPrivate {
-	struct nl_sock *nlh;
 	struct nl_sock *nlh_event;
 	guint32 nlh_seq_next;
 	guint32 nlh_seq_last_handled;
@@ -5467,22 +5466,6 @@ ip6_route_get (NMPlatform *platform, int ifindex, struct in6_addr network, int p
 #define ERROR_CONDITIONS      ((GIOCondition) (G_IO_ERR | G_IO_NVAL))
 #define DISCONNECT_CONDITIONS ((GIOCondition) (G_IO_HUP))
 
-static int
-verify_source (struct nl_msg *msg, NMPlatform *platform)
-{
-	struct ucred *creds = nlmsg_get_creds (msg);
-
-	if (!creds || creds->pid) {
-		if (creds)
-			_LOGW ("netlink: received non-kernel message (pid %d)", creds->pid);
-		else
-			_LOGW ("netlink: received message without credentials");
-		return NL_STOP;
-	}
-
-	return NL_OK;
-}
-
 static gboolean
 event_handler (GIOChannel *channel,
                GIOCondition io_condition,
@@ -5917,20 +5900,6 @@ constructed (GObject *_object)
 
 	_LOGD ("create");
 
-	{
-		priv->nlh = nl_socket_alloc ();
-		g_assert (priv->nlh);
-
-		nle = nl_socket_modify_cb (priv->nlh, NL_CB_MSG_IN, NL_CB_CUSTOM, (nl_recvmsg_msg_cb_t) verify_source, platform);
-		g_assert (!nle);
-
-		nle = nl_connect (priv->nlh, NETLINK_ROUTE);
-		g_assert (!nle);
-		nle = nl_socket_set_passcred (priv->nlh, 1);
-		g_assert (!nle);
-	}
-	_LOGD ("Netlink socket for requests established: port=%u, fd=%d", nl_socket_get_local_port (priv->nlh), nl_socket_get_fd (priv->nlh));
-
 	{
 		priv->nlh_event = nl_socket_alloc ();
 		g_assert (priv->nlh_event);
@@ -6043,7 +6012,6 @@ nm_linux_platform_finalize (GObject *object)
 	/* Free netlink resources */
 	g_source_remove (priv->event_id);
 	g_io_channel_unref (priv->event_channel);
-	nl_socket_free (priv->nlh);
 	nl_socket_free (priv->nlh_event);
 
 	g_object_unref (priv->udev_client);

From 412a50bd3057ac78f8aafd523862b5a83a10dec4 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 15 Dec 2015 10:40:41 +0100
Subject: [PATCH 20/27] platform: inline event_handler_read_netlink()

---
 src/platform/nm-linux-platform.c | 95 +++++++++++++++-----------------
 1 file changed, 45 insertions(+), 50 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 4a3cbfdde..53d153cb0 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -191,7 +191,7 @@ static void do_request_link_no_delayed_actions (NMPlatform *platform, int ifinde
 static void do_request_all_no_delayed_actions (NMPlatform *platform, DelayedActionType action_type);
 static void cache_pre_hook (NMPCache *cache, const NMPObject *old, const NMPObject *new, NMPCacheOpsType ops_type, gpointer user_data);
 static void cache_prune_candidates_prune (NMPlatform *platform);
-static gboolean event_handler_read_netlink_all (NMPlatform *platform, gboolean wait_for_acks);
+static gboolean event_handler_read_netlink (NMPlatform *platform, gboolean wait_for_acks);
 
 /*****************************************************************************/
 
@@ -2840,13 +2840,13 @@ delayed_action_handle_REFRESH_ALL (NMPlatform *platform, DelayedActionType flags
 static void
 delayed_action_handle_READ_NETLINK (NMPlatform *platform)
 {
-	event_handler_read_netlink_all (platform, FALSE);
+	event_handler_read_netlink (platform, FALSE);
 }
 
 static void
 delayed_action_handle_WAIT_FOR_NL_RESPONSE (NMPlatform *platform)
 {
-	event_handler_read_netlink_all (platform, TRUE);
+	event_handler_read_netlink (platform, TRUE);
 }
 
 static gboolean
@@ -3351,7 +3351,7 @@ do_request_link_no_delayed_actions (NMPlatform *platform, int ifindex, const cha
 		                                   (NMPObject *) nmp_cache_lookup_link (priv->cache, ifindex));
 	}
 
-	event_handler_read_netlink_all (platform, FALSE);
+	event_handler_read_netlink (platform, FALSE);
 
 	nlmsg = _nl_msg_new_link (RTM_GETLINK,
 	                          0,
@@ -3403,7 +3403,7 @@ do_request_all_no_delayed_actions (NMPlatform *platform, DelayedActionType actio
 				_LOGt_delayed_action (DELAYED_ACTION_TYPE_REFRESH_LINK, NULL, "clear (do-request-all)");
 			}
 
-			event_handler_read_netlink_all (platform, FALSE);
+			event_handler_read_netlink (platform, FALSE);
 
 			/* reimplement
 			 *   nl_rtgen_request (sk, klass->rtm_gettype, klass->addr_family, NLM_F_DUMP);
@@ -3644,7 +3644,7 @@ do_add_link_with_lookup (NMPlatform *platform,
 	int nle;
 	char s_buf[256];
 
-	event_handler_read_netlink_all (platform, FALSE);
+	event_handler_read_netlink (platform, FALSE);
 
 	nle = _nl_send_auto_with_seq (platform, nlmsg, &seq_result);
 	if (nle < 0) {
@@ -3693,7 +3693,7 @@ do_add_addrroute (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *
 	                      NMP_OBJECT_TYPE_IP4_ADDRESS, NMP_OBJECT_TYPE_IP6_ADDRESS,
 	                      NMP_OBJECT_TYPE_IP4_ROUTE, NMP_OBJECT_TYPE_IP6_ROUTE));
 
-	event_handler_read_netlink_all (platform, FALSE);
+	event_handler_read_netlink (platform, FALSE);
 
 	nle = _nl_send_auto_with_seq (platform, nlmsg, &seq_result);
 	if (nle < 0) {
@@ -3741,7 +3741,7 @@ do_delete_object (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *
 	gboolean success = TRUE;
 	const char *log_detail = "";
 
-	event_handler_read_netlink_all (platform, FALSE);
+	event_handler_read_netlink (platform, FALSE);
 
 	nle = _nl_send_auto_with_seq (platform, nlmsg, &seq_result);
 	if (nle < 0) {
@@ -5534,9 +5534,9 @@ continue_reading:
 
 		if (!creds || creds->pid) {
 			if (creds)
-				_LOGW ("netlink: recvmsg: received non-kernel message (pid %d)", creds->pid);
+				_LOGD ("netlink: recvmsg: received non-kernel message (pid %d)", creds->pid);
 			else
-				_LOGW ("netlink: recvmsg: received message without credentials");
+				_LOGD ("netlink: recvmsg: received message without credentials");
 			goto stop;
 		}
 
@@ -5651,45 +5651,10 @@ out:
 /*****************************************************************************/
 
 static gboolean
-event_handler_read_netlink_one (NMPlatform *platform)
-{
-	int nle;
-
-	nle = event_handler_recvmsgs (platform, TRUE);
-
-	if (nle < 0)
-		switch (nle) {
-		case -NLE_AGAIN:
-			return FALSE;
-		case -NLE_DUMP_INTR:
-			_LOGD ("netlink: read-one: uncritical failure to retrieve incoming events: %s (%d)", nl_geterror (nle), nle);
-			break;
-		case -NLE_NOMEM:
-			_LOGI ("netlink: read-one: too many netlink events. Need to resynchronize platform cache");
-			/* Drain the event queue, we've lost events and are out of sync anyway and we'd
-			 * like to free up some space. We'll read in the status synchronously. */
-			delayed_action_wait_for_nl_response_complete_all (platform, WAIT_FOR_NL_RESPONSE_RESULT_FAILED_RESYNC);
-			event_handler_recvmsgs (platform, FALSE);
-			delayed_action_schedule (platform,
-			                         DELAYED_ACTION_TYPE_REFRESH_ALL_LINKS |
-			                         DELAYED_ACTION_TYPE_REFRESH_ALL_IP4_ADDRESSES |
-			                         DELAYED_ACTION_TYPE_REFRESH_ALL_IP6_ADDRESSES |
-			                         DELAYED_ACTION_TYPE_REFRESH_ALL_IP4_ROUTES |
-			                         DELAYED_ACTION_TYPE_REFRESH_ALL_IP6_ROUTES,
-			                         NULL);
-			break;
-		default:
-			_LOGE ("netlink: read-one: failed to retrieve incoming events: %s (%d)", nl_geterror (nle), nle);
-			break;
-	}
-	return TRUE;
-}
-
-static gboolean
-event_handler_read_netlink_all (NMPlatform *platform, gboolean wait_for_acks)
+event_handler_read_netlink (NMPlatform *platform, gboolean wait_for_acks)
 {
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
-	int r;
+	int r, nle;
 	struct pollfd pfd;
 	gboolean any = FALSE;
 	gint64 now_ns;
@@ -5701,8 +5666,38 @@ event_handler_read_netlink_all (NMPlatform *platform, gboolean wait_for_acks)
 	} data_next;
 
 	while (TRUE) {
-		while (event_handler_read_netlink_one (platform))
+
+		while (TRUE) {
+
+			nle = event_handler_recvmsgs (platform, TRUE);
+
+			if (nle < 0)
+				switch (nle) {
+				case -NLE_AGAIN:
+					goto after_read;
+				case -NLE_DUMP_INTR:
+					_LOGD ("netlink: read: uncritical failure to retrieve incoming events: %s (%d)", nl_geterror (nle), nle);
+					break;
+				case -NLE_NOMEM:
+					_LOGI ("netlink: read: too many netlink events. Need to resynchronize platform cache");
+					/* Drain the event queue, we've lost events and are out of sync anyway and we'd
+					 * like to free up some space. We'll read in the status synchronously. */
+					delayed_action_wait_for_nl_response_complete_all (platform, WAIT_FOR_NL_RESPONSE_RESULT_FAILED_RESYNC);
+					event_handler_recvmsgs (platform, FALSE);
+					delayed_action_schedule (platform,
+					                         DELAYED_ACTION_TYPE_REFRESH_ALL_LINKS |
+					                         DELAYED_ACTION_TYPE_REFRESH_ALL_IP4_ADDRESSES |
+					                         DELAYED_ACTION_TYPE_REFRESH_ALL_IP6_ADDRESSES |
+					                         DELAYED_ACTION_TYPE_REFRESH_ALL_IP4_ROUTES |
+					                         DELAYED_ACTION_TYPE_REFRESH_ALL_IP6_ROUTES,
+					                         NULL);
+					break;
+				default:
+					_LOGE ("netlink: read: failed to retrieve incoming events: %s (%d)", nl_geterror (nle), nle);
+					break;
+			}
 			any = TRUE;
+		}
 
 after_read:
 
@@ -5738,7 +5733,7 @@ after_read:
 		nm_assert (data_next.timeout_abs_ns > 0);
 		nm_assert (now_ns > 0);
 
-		_LOGT ("netlink: read-all: wait for ACK for sequence number %u...", data_next.seq_number);
+		_LOGT ("netlink: read: wait for ACK for sequence number %u...", data_next.seq_number);
 
 		timeout_ms = (data_next.timeout_abs_ns - now_ns) / (NM_UTILS_NS_PER_SECOND / 1000);
 
@@ -5755,7 +5750,7 @@ after_read:
 			int errsv = errno;
 
 			if (errsv != EINTR) {
-				_LOGE ("netlink: read-all: poll failed with %s", strerror (errsv));
+				_LOGE ("netlink: read: poll failed with %s", strerror (errsv));
 				delayed_action_wait_for_nl_response_complete_all (platform, WAIT_FOR_NL_RESPONSE_RESULT_FAILED_POLL);
 				return any;
 			}

From a29f438294c734599f3740c8590294f2d5ec789b Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 15 Dec 2015 10:51:26 +0100
Subject: [PATCH 21/27] platform/trivial: rename internal field with netlink
 socket

---
 src/platform/nm-linux-platform.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 53d153cb0..6276c4cf0 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -2349,7 +2349,7 @@ typedef struct {
 typedef struct _NMLinuxPlatformPrivate NMLinuxPlatformPrivate;
 
 struct _NMLinuxPlatformPrivate {
-	struct nl_sock *nlh_event;
+	struct nl_sock *nlh;
 	guint32 nlh_seq_next;
 	guint32 nlh_seq_last_handled;
 	NMPCache *cache;
@@ -3323,7 +3323,7 @@ _nl_send_auto_with_seq (NMPlatform *platform,
 
 	nlmsg_hdr (nlmsg)->nlmsg_seq = seq;
 
-	nle = nl_send_auto (priv->nlh_event, nlmsg);
+	nle = nl_send_auto (priv->nlh, nlmsg);
 
 	if (nle >= 0)
 		delayed_action_schedule_WAIT_FOR_NL_RESPONSE (platform, seq, out_seq_result);
@@ -5482,7 +5482,7 @@ static int
 event_handler_recvmsgs (NMPlatform *platform, gboolean handle_events)
 {
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
-	struct nl_sock *sk = priv->nlh_event;
+	struct nl_sock *sk = priv->nlh;
 	int n, err = 0, multipart = 0, interrupted = 0, nrecv = 0;
 	unsigned char *buf = NULL;
 	struct nlmsghdr *hdr;
@@ -5738,7 +5738,7 @@ after_read:
 		timeout_ms = (data_next.timeout_abs_ns - now_ns) / (NM_UTILS_NS_PER_SECOND / 1000);
 
 		memset (&pfd, 0, sizeof (pfd));
-		pfd.fd = nl_socket_get_fd (priv->nlh_event);
+		pfd.fd = nl_socket_get_fd (priv->nlh);
 		pfd.events = POLLIN;
 		r = poll (&pfd, 1, MAX (1, timeout_ms));
 
@@ -5896,16 +5896,16 @@ constructed (GObject *_object)
 	_LOGD ("create");
 
 	{
-		priv->nlh_event = nl_socket_alloc ();
-		g_assert (priv->nlh_event);
+		priv->nlh = nl_socket_alloc ();
+		g_assert (priv->nlh);
 
-		nle = nl_connect (priv->nlh_event, NETLINK_ROUTE);
+		nle = nl_connect (priv->nlh, NETLINK_ROUTE);
 		g_assert (!nle);
-		nle = nl_socket_set_passcred (priv->nlh_event, 1);
+		nle = nl_socket_set_passcred (priv->nlh, 1);
 		g_assert (!nle);
 
 		/* No blocking for event socket, so that we can drain it safely. */
-		nle = nl_socket_set_nonblocking (priv->nlh_event);
+		nle = nl_socket_set_nonblocking (priv->nlh);
 		g_assert (!nle);
 
 		/* The default buffer size wasn't enough for the testsuites. It might just
@@ -5915,19 +5915,19 @@ constructed (GObject *_object)
 		 * FIXME: it's unclear that this is still actually needed. The testsuite
 		 * certainly doesn't fail for me. Maybe it can be removed.
 		 */
-		nle = nl_socket_set_buffer_size (priv->nlh_event, 131072, 0);
+		nle = nl_socket_set_buffer_size (priv->nlh, 131072, 0);
 		g_assert (!nle);
 
-		nle = nl_socket_add_memberships (priv->nlh_event,
+		nle = nl_socket_add_memberships (priv->nlh,
 		                                 RTNLGRP_LINK,
 		                                 RTNLGRP_IPV4_IFADDR, RTNLGRP_IPV6_IFADDR,
 		                                 RTNLGRP_IPV4_ROUTE,  RTNLGRP_IPV6_ROUTE,
 		                                 0);
 		g_assert (!nle);
 	}
-	_LOGD ("Netlink socket for events established: port=%u, fd=%d", nl_socket_get_local_port (priv->nlh_event), nl_socket_get_fd (priv->nlh_event));
+	_LOGD ("Netlink socket for events established: port=%u, fd=%d", nl_socket_get_local_port (priv->nlh), nl_socket_get_fd (priv->nlh));
 
-	priv->event_channel = g_io_channel_unix_new (nl_socket_get_fd (priv->nlh_event));
+	priv->event_channel = g_io_channel_unix_new (nl_socket_get_fd (priv->nlh));
 	g_io_channel_set_encoding (priv->event_channel, NULL, NULL);
 	g_io_channel_set_close_on_unref (priv->event_channel, TRUE);
 
@@ -6007,7 +6007,7 @@ nm_linux_platform_finalize (GObject *object)
 	/* Free netlink resources */
 	g_source_remove (priv->event_id);
 	g_io_channel_unref (priv->event_channel);
-	nl_socket_free (priv->nlh_event);
+	nl_socket_free (priv->nlh);
 
 	g_object_unref (priv->udev_client);
 	g_hash_table_unref (priv->wifi_data);

From 690732cfedacb2c780b54d25c6f583b6a83234d1 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 15 Dec 2015 10:55:27 +0100
Subject: [PATCH 22/27] platform: drop delayed_action idle handler

The idea was allowing pending delayed-actions and process them in an
idle handler. We dont want to do that, because whenever platform code
returns, we want to have no pending actions -- because otherwise the
platform cache might be in an inconsistent state.

Just drop it.
---
 src/platform/nm-linux-platform.c | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 6276c4cf0..e6caea680 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -2367,7 +2367,6 @@ struct _NMLinuxPlatformPrivate {
 		GPtrArray *list_refresh_link;
 		GArray *list_wait_for_nl_response;
 		gint is_handling;
-		guint idle_id;
 	} delayed_action;
 
 	GHashTable *prune_candidates;
@@ -2855,10 +2854,8 @@ delayed_action_handle_one (NMPlatform *platform)
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	gpointer user_data;
 
-	if (priv->delayed_action.flags == DELAYED_ACTION_TYPE_NONE) {
-		nm_clear_g_source (&priv->delayed_action.idle_id);
+	if (priv->delayed_action.flags == DELAYED_ACTION_TYPE_NONE)
 		return FALSE;
-	}
 
 	/* First process DELAYED_ACTION_TYPE_MASTER_CONNECTED actions.
 	 * This type of action is entirely cache-internal and is here to resolve a
@@ -2939,8 +2936,6 @@ delayed_action_handle_all (NMPlatform *platform, gboolean read_netlink)
 
 	g_return_val_if_fail (priv->delayed_action.is_handling == 0, FALSE);
 
-	nm_clear_g_source (&priv->delayed_action.idle_id);
-
 	priv->delayed_action.is_handling++;
 	if (read_netlink)
 		delayed_action_schedule (platform, DELAYED_ACTION_TYPE_READ_NETLINK, NULL);
@@ -2953,14 +2948,6 @@ delayed_action_handle_all (NMPlatform *platform, gboolean read_netlink)
 	return any;
 }
 
-static gboolean
-delayed_action_handle_idle (gpointer user_data)
-{
-	NM_LINUX_PLATFORM_GET_PRIVATE (user_data)->delayed_action.idle_id = 0;
-	delayed_action_handle_all (user_data, FALSE);
-	return G_SOURCE_REMOVE;
-}
-
 static void
 delayed_action_schedule (NMPlatform *platform, DelayedActionType action_type, gpointer user_data)
 {
@@ -2997,9 +2984,6 @@ delayed_action_schedule (NMPlatform *platform, DelayedActionType action_type, gp
 				_LOGt_delayed_action (iflags, user_data, "schedule");
 		}
 	}
-
-	if (priv->delayed_action.is_handling == 0 && priv->delayed_action.idle_id == 0)
-		priv->delayed_action.idle_id = g_idle_add (delayed_action_handle_idle, platform);
 }
 
 static void
@@ -3328,7 +3312,7 @@ _nl_send_auto_with_seq (NMPlatform *platform,
 	if (nle >= 0)
 		delayed_action_schedule_WAIT_FOR_NL_RESPONSE (platform, seq, out_seq_result);
 	else
-		_LOGD ("failed sending message: %s (%d)", nl_geterror (nle), nle);
+		_LOGD ("netlink: send: failed sending message: %s (%d)", nl_geterror (nle), nle);
 
 	return nle;
 }
@@ -5986,8 +5970,6 @@ dispose (GObject *object)
 	g_ptr_array_set_size (priv->delayed_action.list_master_connected, 0);
 	g_ptr_array_set_size (priv->delayed_action.list_refresh_link, 0);
 
-	nm_clear_g_source (&priv->delayed_action.idle_id);
-
 	g_clear_pointer (&priv->prune_candidates, g_hash_table_unref);
 
 	G_OBJECT_CLASS (nm_linux_platform_parent_class)->dispose (object);

From cea8f1a0f0f2add3331ad3d159edbe8d996e2d7d Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 15 Dec 2015 13:25:53 +0100
Subject: [PATCH 23/27] platform: change meaning of return value for
 delete-function

When deleting an object, we allow failure to delete a non-existing object.
Thus, the only thing we care about is whether the object is no longer
present after deletion. Adjust the return values to reflect that.
---
 src/platform/nm-linux-platform.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index e6caea680..ae107ae69 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -3719,6 +3719,7 @@ do_add_addrroute (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *
 static gboolean
 do_delete_object (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *nlmsg)
 {
+	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	WaitForNlResponseResult seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
 	int nle;
 	char s_buf[256];
@@ -3733,7 +3734,7 @@ do_delete_object (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *
 		       NMP_OBJECT_GET_CLASS (obj_id)->obj_type_name,
 		       nmp_object_to_string (obj_id, NMP_OBJECT_TO_STRING_ID, NULL, 0),
 		       nl_geterror (nle), -nle);
-		return FALSE;
+		goto out;
 	}
 
 	delayed_action_handle_all (platform, FALSE);
@@ -3761,15 +3762,14 @@ do_delete_object (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *
 	        wait_for_nl_response_to_string (seq_result, s_buf, sizeof (s_buf)),
 	        log_detail);
 
-	if (nmp_cache_lookup_obj (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache, obj_id)) {
-		/* such an object still exists in the cache. To be sure, refetch it (and
-		 * hope it's gone) */
-		do_request_one_type (platform, NMP_OBJECT_GET_TYPE (obj_id));
-	}
+out:
+	if (!nmp_cache_lookup_obj (priv->cache, obj_id))
+		return TRUE;
 
-	/* The return value doesn't say, whether the object is in the platform cache after adding
-	 * it. Instead the return value says, whether the netlink request succeeded. */
-	return success;
+	/* such an object still exists in the cache. To be sure, refetch it (and
+	 * hope it's gone) */
+	do_request_one_type (platform, NMP_OBJECT_GET_TYPE (obj_id));
+	return !!nmp_cache_lookup_obj (priv->cache, obj_id);
 }
 
 static NMPlatformError
@@ -5175,6 +5175,8 @@ ip4_address_delete (NMPlatform *platform, int ifindex, in_addr_t addr, int plen,
 	                             NM_PLATFORM_LIFETIME_PERMANENT,
 	                             NM_PLATFORM_LIFETIME_PERMANENT,
 	                             NULL);
+	if (!nlmsg)
+		g_return_val_if_reached (FALSE);
 
 	nmp_object_stackinit_id_ip4_address (&obj_id, ifindex, addr, plen, peer_address);
 	return do_delete_object (platform, &obj_id, nlmsg);
@@ -5198,6 +5200,8 @@ ip6_address_delete (NMPlatform *platform, int ifindex, struct in6_addr addr, int
 	                             NM_PLATFORM_LIFETIME_PERMANENT,
 	                             NM_PLATFORM_LIFETIME_PERMANENT,
 	                             NULL);
+	if (!nlmsg)
+		g_return_val_if_reached (FALSE);
 
 	nmp_object_stackinit_id_ip6_address (&obj_id, ifindex, &addr, plen);
 	return do_delete_object (platform, &obj_id, nlmsg);

From 1a501c645655f94c6412e8ef399fdbf6d3f2a010 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 15 Dec 2015 13:32:14 +0100
Subject: [PATCH 24/27] platform: check for existing link in
 do_add_link_with_lookup()

When adding a link, that can only make sense if no such link
exists yet. Check for that condition first, to properly return
an error.
---
 src/platform/nm-linux-platform.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index ae107ae69..99be5983c 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -3630,6 +3630,20 @@ do_add_link_with_lookup (NMPlatform *platform,
 
 	event_handler_read_netlink (platform, FALSE);
 
+	if (nmp_cache_lookup_link_full (priv->cache, 0, name, FALSE, NM_LINK_TYPE_NONE, NULL, NULL)) {
+		/* hm, a link with such a name already exists. Try reloading first. */
+		do_request_link (platform, 0, name);
+
+		obj = nmp_cache_lookup_link_full (priv->cache, 0, name, FALSE, NM_LINK_TYPE_NONE, NULL, NULL);
+		if (obj) {
+			_LOGE ("do-add-link[%s/%s]: link already exists: %s",
+			       name,
+			       nm_link_type_to_string (link_type),
+			       nmp_object_to_string (obj, NMP_OBJECT_TO_STRING_ID, NULL, 0));
+			return FALSE;
+		}
+	}
+
 	nle = _nl_send_auto_with_seq (platform, nlmsg, &seq_result);
 	if (nle < 0) {
 		_LOGE ("do-add-link[%s/%s]: failed sending netlink request \"%s\" (%d)",

From 88213b2e6a219d82a3f8656dda322fd8c471831f Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 15 Dec 2015 13:37:32 +0100
Subject: [PATCH 25/27] platform: tighten return value from do_add_addrroute()

Only return TRUE, if the netlink request was responded with success and
the object exists after adding.
---
 src/platform/nm-linux-platform.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 99be5983c..fd96d6669 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -3683,9 +3683,11 @@ do_add_link_with_lookup (NMPlatform *platform,
 static gboolean
 do_add_addrroute (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *nlmsg)
 {
+	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	WaitForNlResponseResult seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
 	int nle;
 	char s_buf[256];
+	const NMPObject *obj;
 
 	nm_assert (NM_IN_SET (NMP_OBJECT_GET_TYPE (obj_id),
 	                      NMP_OBJECT_TYPE_IP4_ADDRESS, NMP_OBJECT_TYPE_IP6_ADDRESS,
@@ -3721,13 +3723,17 @@ do_add_addrroute (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *
 	 * whether the object exists.
 	 *
 	 * FIXME: if the object already existed previously, we might not notice a
-	 * missing update. */
-	if (!nmp_cache_lookup_obj (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache, obj_id))
+	 * missing update. It's not clear how to fix that reliably without refechting
+	 * all the time. */
+	obj = nmp_cache_lookup_obj (priv->cache, obj_id);
+	if (!obj) {
 		do_request_one_type (platform, NMP_OBJECT_GET_TYPE (obj_id));
+		obj = nmp_cache_lookup_obj (priv->cache, obj_id);
+	}
 
-	/* The return value doesn't say, whether the object is in the platform cache after adding
-	 * it. Instead the return value says, whether the netlink request succeeded. */
-	return seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK;
+	/* Adding is only successful, if kernel reported success *and* we have the
+	 * expected object in cache afterwards. */
+	return obj && seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK;
 }
 
 static gboolean

From ad1d74d142606e6ba434051a85cebad6bded69e8 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 15 Dec 2015 16:23:05 +0100
Subject: [PATCH 26/27] platform: add index for links by ifname

Downsides:

  - Add some additional overhead to manage the index

  - The NMPCacheId struct grows to 16 bytes, affecting
    hashing performance for all object types.

Still do it, based on the assumption that it doesn't matter
for a low number of interfaces. But the O(1) access time matters
when having lots of interfaces.
---
 src/platform/nmp-object.c | 47 +++++++++++++++++++++++++++++++++++++--
 src/platform/nmp-object.h |  9 ++++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/src/platform/nmp-object.c b/src/platform/nmp-object.c
index 2b1ec2127..6fbbbe235 100644
--- a/src/platform/nmp-object.c
+++ b/src/platform/nmp-object.c
@@ -1072,6 +1072,24 @@ nmp_cache_id_init_routes_visible (NMPCacheId *id,
 	return id;
 }
 
+NMPCacheId *
+nmp_cache_id_init_link_by_ifname (NMPCacheId *id,
+                                  const char *ifname)
+{
+	gsize l;
+
+	_nmp_cache_id_init (id, NMP_CACHE_ID_TYPE_LINK_BY_IFNAME);
+
+	if (   !ifname
+	    || (l = strlen (ifname)) > sizeof (id->link_by_ifname.ifname_short))
+		g_return_val_if_reached (id);
+
+	/* the trailing NUL is dropped!! */
+	memcpy (id->link_by_ifname.ifname_short, ifname, l);
+
+	return id;
+}
+
 /******************************************************************/
 
 static gboolean
@@ -1095,6 +1113,23 @@ _nmp_object_init_cache_id (const NMPObject *obj, NMPCacheIdType id_type, NMPCach
 	}
 }
 
+static gboolean
+_vt_cmd_obj_init_cache_id_link (const NMPObject *obj, NMPCacheIdType id_type, NMPCacheId *id, const NMPCacheId **out_id)
+{
+	switch (id_type) {
+	case NMP_CACHE_ID_TYPE_LINK_BY_IFNAME:
+		if (obj->link.name[0]) {
+			*out_id = nmp_cache_id_init_link_by_ifname (id, obj->link.name);
+			return TRUE;
+		}
+		break;
+	default:
+		return FALSE;
+	}
+	*out_id = NULL;
+	return TRUE;
+}
+
 static gboolean
 _vt_cmd_obj_init_cache_id_ipx_address (const NMPObject *obj, NMPCacheIdType id_type, NMPCacheId *id, const NMPCacheId **out_id)
 {
@@ -1356,7 +1391,7 @@ nmp_cache_lookup_link_full (const NMPCache *cache,
 	const NMPObject *obj;
 	const NMPlatformObject *const *list;
 	guint i, len;
-	NMPCacheId cache_id;
+	NMPCacheId cache_id, *p_cache_id;
 
 	if (ifindex > 0) {
 		obj = nmp_cache_lookup_obj (cache, nmp_object_stackinit_id_link (&obj_needle, ifindex));
@@ -1371,7 +1406,14 @@ nmp_cache_lookup_link_full (const NMPCache *cache,
 	} else if (!ifname && !match_fn)
 		return NULL;
 	else {
-		list = nmp_cache_lookup_multi (cache, nmp_cache_id_init_object_type (&cache_id, NMP_OBJECT_TYPE_LINK, visible_only), &len);
+		if (   ifname
+		    && strlen (ifname) <= sizeof (cache_id.link_by_ifname.ifname_short)) {
+			p_cache_id = nmp_cache_id_init_link_by_ifname (&cache_id, ifname);
+			ifname = NULL;
+		} else
+			p_cache_id = nmp_cache_id_init_object_type (&cache_id, NMP_OBJECT_TYPE_LINK, visible_only);
+
+		list = nmp_cache_lookup_multi (cache, p_cache_id, &len);
 		for (i = 0; i < len; i++) {
 			obj = NMP_OBJECT_UP_CAST (list[i]);
 
@@ -1933,6 +1975,7 @@ const NMPClass _nmp_classes[NMP_OBJECT_TYPE_MAX] = {
 		.rtm_gettype                        = RTM_GETLINK,
 		.signal_type_id                     = NM_PLATFORM_SIGNAL_ID_LINK,
 		.signal_type                        = NM_PLATFORM_SIGNAL_LINK_CHANGED,
+		.cmd_obj_init_cache_id              = _vt_cmd_obj_init_cache_id_link,
 		.cmd_obj_cmp                        = _vt_cmd_obj_cmp_link,
 		.cmd_obj_copy                       = _vt_cmd_obj_copy_link,
 		.cmd_obj_stackinit_id               = _vt_cmd_obj_stackinit_id_link,
diff --git a/src/platform/nmp-object.h b/src/platform/nmp-object.h
index 5acde4607..ab1cc2fe8 100644
--- a/src/platform/nmp-object.h
+++ b/src/platform/nmp-object.h
@@ -64,6 +64,9 @@ typedef enum { /*< skip >*/
 	/* all the objects of a certain type */
 	NMP_CACHE_ID_TYPE_OBJECT_TYPE,
 
+	/* index for the link objects by ifname. */
+	NMP_CACHE_ID_TYPE_LINK_BY_IFNAME,
+
 	/* all the visible objects of a certain type */
 	NMP_CACHE_ID_TYPE_OBJECT_TYPE_VISIBLE_ONLY,
 
@@ -104,6 +107,11 @@ typedef struct {
 			guint8 obj_type; /* NMPObjectType as guint8 */
 			int ifindex;
 		} object_type_by_ifindex;
+		struct {
+			/* NMP_CACHE_ID_TYPE_LINK_BY_IFNAME */
+			guint8 _id_type;
+			char ifname_short[IFNAMSIZ - 1]; /* don't include the trailing NUL so the struct fits in 4 bytes. */
+		} link_by_ifname;
 	};
 } NMPCacheId;
 
@@ -374,6 +382,7 @@ void nmp_cache_id_destroy (NMPCacheId *id);
 NMPCacheId *nmp_cache_id_init_object_type (NMPCacheId *id, NMPObjectType obj_type, gboolean visible_only);
 NMPCacheId *nmp_cache_id_init_addrroute_visible_by_ifindex (NMPCacheId *id, NMPObjectType obj_type, int ifindex);
 NMPCacheId *nmp_cache_id_init_routes_visible (NMPCacheId *id, NMPObjectType obj_type, gboolean with_default, gboolean with_non_default, int ifindex);
+NMPCacheId *nmp_cache_id_init_link_by_ifname (NMPCacheId *id, const char *ifname);
 
 const NMPlatformObject *const *nmp_cache_lookup_multi (const NMPCache *cache, const NMPCacheId *cache_id, guint *out_len);
 GArray *nmp_cache_lookup_multi_to_array (const NMPCache *cache, NMPObjectType obj_type, const NMPCacheId *cache_id);

From c40acf334e90e77df5f6a0641d378812b552d518 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 16 Dec 2015 13:01:51 +0100
Subject: [PATCH 27/27] core: optimize NMMultiIndex by special caseing ids with
 one value only

When adding a very first item for a certain id, don't yet create a
hash table but store the first item inplace. This optimizes storage
for the case where we have only one item for a certain id.
---
 src/nm-multi-index.c | 154 ++++++++++++++++++++++++++-----------------
 src/nm-multi-index.h |   5 +-
 2 files changed, 99 insertions(+), 60 deletions(-)

diff --git a/src/nm-multi-index.c b/src/nm-multi-index.c
index da3507d0b..47cd98105 100644
--- a/src/nm-multi-index.c
+++ b/src/nm-multi-index.c
@@ -34,56 +34,72 @@ struct NMMultiIndex {
 };
 
 typedef struct {
+	/* when storing the first item for a multi-index id, we don't yet create
+	 * the hashtable @index. Instead we store it inplace to @value0. Note that
+	 * &values_data->value0 is a NULL terminated array with one item that is
+	 * suitable to be returned directly from nm_multi_index_lookup(). */
+	union {
+		gpointer value0;
+		gpointer *values;
+	};
 	GHashTable *index;
-	gpointer *values;
 } ValuesData;
 
 /******************************************************************************************/
 
-static ValuesData *
-_values_data_create (void)
-{
-	ValuesData *values_data;
-
-	values_data = g_slice_new (ValuesData);
-	values_data->index = g_hash_table_new (NULL, NULL);
-	values_data->values = NULL;
-	return values_data;
-}
-
 static void
 _values_data_destroy (ValuesData *values_data)
 {
-	if (values_data) {
+	if (values_data->index) {
 		g_free (values_data->values);
 		g_hash_table_unref (values_data->index);
-		g_slice_free (ValuesData, values_data);
 	}
+	g_slice_free (ValuesData, values_data);
+}
+
+static gboolean
+_values_data_contains (ValuesData *values_data, gconstpointer value)
+{
+	return values_data->index
+	       ? g_hash_table_contains (values_data->index, value)
+	       : value == values_data->value0;
 }
 
 static void
-_values_data_populate_array (ValuesData *values_data)
+_values_data_get_data (ValuesData *values_data,
+                       void *const**out_data,
+                       guint *out_len)
 {
 	guint i, len;
 	gpointer *values;
 	GHashTableIter iter;
 
 	nm_assert (values_data);
+
+	if (!values_data->index) {
+		NM_SET_OUT (out_data, &values_data->value0);
+		NM_SET_OUT (out_len, 1);
+		return;
+	}
+
 	nm_assert (values_data->index && g_hash_table_size (values_data->index) > 0);
 
-	if (values_data->values)
-		return;
+	if (!values_data->values) {
+		len = g_hash_table_size (values_data->index);
+		values = g_new (gpointer, len + 1);
 
-	len = g_hash_table_size (values_data->index);
-	values = g_new (gpointer, len + 1);
+		g_hash_table_iter_init (&iter, values_data->index);
+		for (i = 0; g_hash_table_iter_next (&iter, &values[i], NULL); i++)
+			nm_assert (i < len);
+		nm_assert (i == len);
+		values[i] = NULL;
 
-	g_hash_table_iter_init (&iter, values_data->index);
-	for (i = 0; g_hash_table_iter_next (&iter, &values[i], NULL); i++)
-		nm_assert (i < len);
-	nm_assert (i == len);
-	values[i] = NULL;
+		values_data->values = values;
+		NM_SET_OUT (out_len, len);
+	} else if (out_len)
+		NM_SET_OUT (out_len, g_hash_table_size (values_data->index));
 
-	values_data->values = values;
+	NM_SET_OUT (out_data, values_data->values);
 }
 
 /******************************************************************************************/
@@ -104,6 +120,7 @@ nm_multi_index_lookup (const NMMultiIndex *index,
                        guint *out_len)
 {
 	ValuesData *values_data;
+	void *const*values;
 
 	g_return_val_if_fail (index, NULL);
 	g_return_val_if_fail (id, NULL);
@@ -114,10 +131,8 @@ nm_multi_index_lookup (const NMMultiIndex *index,
 			*out_len = 0;
 		return NULL;
 	}
-	_values_data_populate_array (values_data);
-	if (out_len)
-		*out_len = g_hash_table_size (values_data->index);
-	return values_data->values;
+	_values_data_get_data (values_data, &values, out_len);
+	return values;
 }
 
 gboolean
@@ -132,8 +147,7 @@ nm_multi_index_contains (const NMMultiIndex *index,
 	g_return_val_if_fail (value, FALSE);
 
 	values_data = g_hash_table_lookup (index->hash, id);
-	return    values_data
-	       && g_hash_table_contains (values_data->index, value);
+	return values_data && _values_data_contains (values_data, value);
 }
 
 const NMMultiIndexId *
@@ -157,7 +171,7 @@ nm_multi_index_lookup_first_by_value (const NMMultiIndex *index,
 
 	g_hash_table_iter_init (&iter, index->hash);
 	while (g_hash_table_iter_next (&iter, (gpointer *) &id, (gpointer *) &values_data)) {
-		if (g_hash_table_contains (values_data->index, value))
+		if (_values_data_contains (values_data, value))
 			return id;
 	}
 	return NULL;
@@ -172,18 +186,19 @@ nm_multi_index_foreach (const NMMultiIndex *index,
 	GHashTableIter iter;
 	const NMMultiIndexId *id;
 	ValuesData *values_data;
+	guint len;
+	void *const*values;
 
 	g_return_if_fail (index);
 	g_return_if_fail (foreach_func);
 
 	g_hash_table_iter_init (&iter, index->hash);
 	while (g_hash_table_iter_next (&iter, (gpointer *) &id, (gpointer *) &values_data)) {
-		if (   value
-		    && !g_hash_table_contains (values_data->index, value))
+		if (value && !_values_data_contains (values_data, value))
 			continue;
 
-		_values_data_populate_array (values_data);
-		if (!foreach_func (id, values_data->values, g_hash_table_size (values_data->index), user_data))
+		_values_data_get_data (values_data, &values, &len);
+		if (!foreach_func (id, values, len, user_data))
 			return;
 	}
 }
@@ -214,14 +229,11 @@ nm_multi_index_iter_next (NMMultiIndexIter *iter,
 
 	while (g_hash_table_iter_next (&iter->_iter, (gpointer *) &id, (gpointer *) &values_data)) {
 		if (   !iter->_value
-		    || g_hash_table_contains (values_data->index, iter->_value)) {
-			_values_data_populate_array (values_data);
+		    || _values_data_contains (values_data, iter->_value)) {
+			if (out_values || out_len)
+				_values_data_get_data (values_data, out_values, out_len);
 			if (out_id)
 				*out_id = id;
-			if (out_values)
-				*out_values = values_data->values;
-			if (out_len)
-				*out_len = g_hash_table_size (values_data->index);
 			return TRUE;
 		}
 	}
@@ -243,10 +255,13 @@ nm_multi_index_id_iter_init (NMMultiIndexIdIter *iter,
 
 	values_data = g_hash_table_lookup (index->hash, id);
 	if (!values_data)
+		iter->_state = 2;
+	else if (values_data->index) {
 		iter->_state = 1;
-	else {
-		iter->_state = 0;
 		g_hash_table_iter_init (&iter->_iter, values_data->index);
+	} else {
+		iter->_state = 0;
+		iter->_value = values_data->value0;
 	}
 }
 
@@ -255,13 +270,19 @@ nm_multi_index_id_iter_next (NMMultiIndexIdIter *iter,
                              void **out_value)
 {
 	g_return_val_if_fail (iter, FALSE);
-	g_return_val_if_fail (iter->_state <= 1, FALSE);
 
-	if (iter->_state == 0)
-		return g_hash_table_iter_next (&iter->_iter, out_value, NULL);
-	else {
+	switch (iter->_state) {
+	case 0:
 		iter->_state = 2;
+		NM_SET_OUT (out_value, iter->_value);
+		return TRUE;
+	case 1:
+		return g_hash_table_iter_next (&iter->_iter, out_value, NULL);
+	case 2:
+		iter->_state = 3;
 		return FALSE;
+	default:
+		g_return_val_if_reached (FALSE);
 	}
 }
 
@@ -291,14 +312,23 @@ _do_add (NMMultiIndex *index,
 		if (!id_new)
 			g_return_val_if_reached (FALSE);
 
-		values_data = _values_data_create ();
-		g_hash_table_replace (values_data->index, (gpointer) value, (gpointer) value);
+		values_data = g_slice_new0 (ValuesData);
+		values_data->value0 = (gpointer) value;
 
 		g_hash_table_insert (index->hash, id_new, values_data);
 	} else {
-		if (!nm_g_hash_table_replace (values_data->index, (gpointer) value, (gpointer) value))
-			return FALSE;
-		g_clear_pointer (&values_data->values, g_free);
+		if (!values_data->index) {
+			if (values_data->value0 == value)
+				return FALSE;
+			values_data->index = g_hash_table_new (NULL, NULL);
+			g_hash_table_replace (values_data->index, (gpointer) value, (gpointer) value);
+			g_hash_table_replace (values_data->index, values_data->value0, values_data->value0);
+			values_data->values = NULL;
+		} else {
+			if (!nm_g_hash_table_replace (values_data->index, (gpointer) value, (gpointer) value))
+				return FALSE;
+			g_clear_pointer (&values_data->values, g_free);
+		}
 	}
 	return TRUE;
 }
@@ -314,13 +344,19 @@ _do_remove (NMMultiIndex *index,
 	if (!values_data)
 		return FALSE;
 
-	if (!g_hash_table_remove (values_data->index, value))
-		return FALSE;
-
-	if (g_hash_table_size (values_data->index) == 0)
+	if (values_data->index) {
+		if (!g_hash_table_remove (values_data->index, value))
+			return FALSE;
+		if (g_hash_table_size (values_data->index) == 0)
+			g_hash_table_remove (index->hash, id);
+		else
+			g_clear_pointer (&values_data->values, g_free);
+	} else {
+		if (values_data->value0 != value)
+			return FALSE;
 		g_hash_table_remove (index->hash, id);
-	else
-		g_clear_pointer (&values_data->values, g_free);
+	}
+
 	return TRUE;
 }
 
diff --git a/src/nm-multi-index.h b/src/nm-multi-index.h
index 046e2431b..1e1e8fdae 100644
--- a/src/nm-multi-index.h
+++ b/src/nm-multi-index.h
@@ -39,7 +39,10 @@ typedef struct {
 } NMMultiIndexIter;
 
 typedef struct {
-	GHashTableIter _iter;
+	union {
+		GHashTableIter _iter;
+		gpointer _value;
+	};
 	guint _state;
 } NMMultiIndexIdIter;