From 1de10532f2b0bbd1fbe3a07d0315ccc90ae5c7a8 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 8 Feb 2018 16:34:29 +0100
Subject: [PATCH] platform: fix object order in platform cache during dump

Originally, the platform cache did not preserve any stable order.
We added that during the large cache rework. However, we still
would only care about a particular ordering for route's BY_WEAK_ID
index. For all other indexes, it was sufficient to have the
object in some arbitrary order, not necessarily the one as indicated by
kernel.

However, for addresses we actually care about the order (at least,
regarding the the OBJECT_BY_IFINDEX index, which is considered by
platform's address sync).

During a dump we get all objects in the right order. That means,
as we (re) insert the objects into the cache, we must forcefully move
them to the end of their list.

If the object didn't actually change, previously we would not have
updated their position in the cache. Fix that now.
---
 src/platform/nmp-object.c | 39 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/platform/nmp-object.c b/src/platform/nmp-object.c
index 6605424e9d..4df64e608c 100644
--- a/src/platform/nmp-object.c
+++ b/src/platform/nmp-object.c
@@ -1786,6 +1786,36 @@ _obj_get_add_mode (const NMPObject *obj)
 	return NM_DEDUP_MULTI_IDX_MODE_APPEND;
 }
 
+static void
+_idxcache_update_order_for_dump (NMPCache *cache,
+                                 const NMDedupMultiEntry *entry)
+{
+	const NMPClass *klass;
+	const guint8 *i_idx_type;
+	const NMDedupMultiEntry *entry2;
+
+	nm_dedup_multi_entry_reorder (entry, NULL, TRUE);
+
+	klass = NMP_OBJECT_GET_CLASS (entry->obj);
+	for (i_idx_type = klass->supported_cache_ids; *i_idx_type; i_idx_type++) {
+		NMPCacheIdType id_type = *i_idx_type;
+
+		if (id_type == NMP_CACHE_ID_TYPE_OBJECT_TYPE)
+			continue;
+
+		entry2 = nm_dedup_multi_index_lookup_obj (cache->multi_idx,
+		                                          _idx_type_get (cache, id_type),
+		                                          entry->obj);
+		if (!entry2)
+			continue;
+
+		nm_assert (entry2 != entry);
+		nm_assert (entry2->obj == entry->obj);
+
+		nm_dedup_multi_entry_reorder (entry2, NULL, TRUE);
+	}
+}
+
 static void
 _idxcache_update_other_cache_ids (NMPCache *cache,
                                   NMPCacheIdType cache_id_type,
@@ -2180,6 +2210,8 @@ nmp_cache_update_netlink (NMPCache *cache,
 	}
 
 	if (nmp_object_equal (obj_old, obj_hand_over)) {
+		if (is_dump)
+			_idxcache_update_order_for_dump (cache, entry_old);
 		nm_dedup_multi_entry_set_dirty (entry_old, FALSE);
 		NM_SET_OUT (out_obj_new, nmp_object_ref (obj_old));
 		return NMP_CACHE_OPS_UNCHANGED;
@@ -2253,6 +2285,8 @@ nmp_cache_update_netlink_route (NMPCache *cache,
 	}
 
 	if (nmp_object_equal (entry_old->obj, obj_hand_over)) {
+		if (is_dump)
+			_idxcache_update_order_for_dump (cache, entry_old);
 		nm_dedup_multi_entry_set_dirty (entry_old, FALSE);
 		goto update_done;
 	}
@@ -2276,9 +2310,8 @@ update_done:
 	 * properly find @obj_replaced. */
 	resync_required = FALSE;
 	entry_replace = NULL;
-	if (is_dump) {
+	if (is_dump)
 		goto out;
-	}
 
 	if (!entry_new) {
 		if (   NM_FLAGS_HAS (nlmsgflags, NLM_F_REPLACE)
@@ -2293,6 +2326,8 @@ update_done:
 		goto out;
 	}
 
+	/* FIXME: for routes, we only maintain the order correctly for the BY_WEAK_ID
+	 * index. For all other indexes their order becomes messed up. */
 	entry_cur = _lookup_entry_with_idx_type (cache,
 	                                         NMP_CACHE_ID_TYPE_ROUTES_BY_WEAK_ID,
 	                                         entry_new->obj);