Blob Blame History Raw
From cc41bf81f82d4cb6fb3477418829896774bd2c3f Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 16 Mar 2017 12:21:19 +0100
Subject: [PATCH 1/3] default-route-manager: simplify determining synced flag
 in _ipx_update_default_route()

No change in behavior at all. The same logic applies, but this should
be simpler to understand.

(cherry picked from commit 0b3ba99409c135716292b6141d2161d395bca46b)
---
 src/nm-default-route-manager.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/nm-default-route-manager.c b/src/nm-default-route-manager.c
index 2fd2c7d..c1327ec 100644
--- a/src/nm-default-route-manager.c
+++ b/src/nm-default-route-manager.c
@@ -770,9 +770,8 @@ _ipx_update_default_route (const VTableIP *vtable, NMDefaultRouteManager *self,
 				default_route = &rt.rx;
 
 				never_default = TRUE;
-				synced = TRUE;
-			} else
-				synced = default_route && !is_assumed;
+			}
+			synced = !is_assumed;
 		} else {
 			NMConnection *connection = nm_active_connection_get_applied_connection ((NMActiveConnection *) vpn);
 
-- 
2.7.4

From 31915a972b46d51cb5979d8da63c37508c1cae3b Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 16 Mar 2017 13:13:21 +0100
Subject: [PATCH 2/3] default-route-manager: alyways force a sync of the
 default route

Whenever we call update for a non-assumed, synced route, we must
force a resync with the platform. Even if according to our internal
book-keeping the route is already configured, the route may have
been removed externally. So we cannot assume that everything is
still up-to-date.

https://bugzilla.redhat.com/show_bug.cgi?id=1431268
(cherry picked from commit c3c251ea129eeb562d32ac44029714f8d644ad18)
---
 src/nm-default-route-manager.c | 43 ++++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/src/nm-default-route-manager.c b/src/nm-default-route-manager.c
index c1327ec..0764a70 100644
--- a/src/nm-default-route-manager.c
+++ b/src/nm-default-route-manager.c
@@ -522,8 +522,6 @@ _resync_all (const VTableIP *vtable, NMDefaultRouteManager *self, const Entry *c
 	for (i = 0; i < entries->len; i++) {
 		entry = g_ptr_array_index (entries, i);
 
-		g_assert (entry != old_entry);
-
 		if (entry->never_default)
 			continue;
 
@@ -578,12 +576,15 @@ _resync_all (const VTableIP *vtable, NMDefaultRouteManager *self, const Entry *c
 			/* for the changed entry, the previous metric was either old_entry->effective_metric,
 			 * or none. Hence, we only have to remember what is going to change. */
 			g_array_append_val (changed_metrics, expected_metric);
-			if (old_entry) {
+			if (!old_entry) {
+				_LOG2D (vtable, i, entry, "sync:add    %s (%u)",
+				        vtable->vt->route_to_string (&entry->route, NULL, 0), (guint) expected_metric);
+			} else if (old_entry != changed_entry) {
 				_LOG2D (vtable, i, entry, "sync:update %s (%u -> %u)",
 				        vtable->vt->route_to_string (&entry->route, NULL, 0), (guint) old_entry->effective_metric,
 				        (guint) expected_metric);
 			} else {
-				_LOG2D (vtable, i, entry, "sync:add    %s (%u)",
+				_LOG2D (vtable, i, entry, "sync:resync %s (%u)",
 				        vtable->vt->route_to_string (&entry->route, NULL, 0), (guint) expected_metric);
 			}
 		} else if (entry->effective_metric != expected_metric) {
@@ -661,7 +662,11 @@ _entry_at_idx_update (const VTableIP *vtable, NMDefaultRouteManager *self, guint
 		entry->effective_metric = entry->route.rx.metric;
 
 	_LOG2D (vtable, entry_idx, entry, "%s %s (%"G_GUINT32_FORMAT")",
-	        old_entry ? "record:update" : "record:add   ",
+	        old_entry
+	            ? (entry != old_entry
+	                   ? "record:update"
+	                   : "record:resync")
+	            : "record:add   ",
 	        vtable->vt->route_to_string (&entry->route, NULL, 0),
 	        entry->effective_metric);
 
@@ -698,7 +703,9 @@ _entry_at_idx_remove (const VTableIP *vtable, NMDefaultRouteManager *self, guint
 /***********************************************************************************/
 
 static void
-_ipx_update_default_route (const VTableIP *vtable, NMDefaultRouteManager *self, gpointer source)
+_ipx_update_default_route (const VTableIP *vtable,
+                           NMDefaultRouteManager *self,
+                           gpointer source)
 {
 	NMDefaultRouteManagerPrivate *priv;
 	Entry *entry;
@@ -863,12 +870,18 @@ _ipx_update_default_route (const VTableIP *vtable, NMDefaultRouteManager *self,
 		new_entry.never_default = never_default;
 		new_entry.synced = synced;
 
-		if (memcmp (entry, &new_entry, sizeof (new_entry)) == 0)
-			return;
-
-		old_entry = *entry;
-		*entry = new_entry;
-		_entry_at_idx_update (vtable, self, entry_idx, &old_entry);
+		if (memcmp (entry, &new_entry, sizeof (new_entry)) == 0) {
+			if (!synced) {
+				/* the internal book-keeping doesn't change, so don't do a full
+				 * sync of the configured routes. */
+				return;
+			}
+			_entry_at_idx_update (vtable, self, entry_idx, entry);
+		} else {
+			old_entry = *entry;
+			*entry = new_entry;
+			_entry_at_idx_update (vtable, self, entry_idx, &old_entry);
+		}
 	} else {
 		/* delete */
 		_entry_at_idx_remove (vtable, self, entry_idx);
@@ -876,13 +889,15 @@ _ipx_update_default_route (const VTableIP *vtable, NMDefaultRouteManager *self,
 }
 
 void
-nm_default_route_manager_ip4_update_default_route (NMDefaultRouteManager *self, gpointer source)
+nm_default_route_manager_ip4_update_default_route (NMDefaultRouteManager *self,
+                                                   gpointer source)
 {
 	_ipx_update_default_route (&vtable_ip4, self, source);
 }
 
 void
-nm_default_route_manager_ip6_update_default_route (NMDefaultRouteManager *self, gpointer source)
+nm_default_route_manager_ip6_update_default_route (NMDefaultRouteManager *self,
+                                                   gpointer source)
 {
 	_ipx_update_default_route (&vtable_ip6, self, source);
 }
-- 
2.7.4

From 88f5760728986da997ff9e65488fc694e95a754f Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 16 Mar 2017 15:26:18 +0100
Subject: [PATCH 3/3] default-route-manager: decryptify logging line for
 default-route-manager

The default route manager logs for each entry relevant information,
in a compact but cryptic way:

  default-route: entry[0/dev:0x5633d5528560:enp0s25:1:+sync]: record:add    0.0.0.0/0 via 192.168.0.1 dev 2 metric 100 mss 0 rt-src user (100)

The flag whether a route is configured or not, was only expressed
via 0|1. Change that to log instead:

  default-route: entry[0/dev:0x5633d5528560:enp0s25:+has:+sync]: record:add    0.0.0.0/0 via 192.168.0.1 dev 2 metric 100 mss 0 rt-src user (100)

(cherry picked from commit 82bfb6c46d2ff1ca01c7dffd0a812bf53e08ff33)
---
 src/nm-default-route-manager.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/nm-default-route-manager.c b/src/nm-default-route-manager.c
index 0764a70..a045c59 100644
--- a/src/nm-default-route-manager.c
+++ b/src/nm-default-route-manager.c
@@ -110,7 +110,7 @@ NM_DEFINE_SINGLETON_GETTER (NMDefaultRouteManager, nm_default_route_manager_get,
             const Entry *const __entry = (entry); \
             \
             _nm_log (__level, __domain, 0, \
-                     "%s: entry[%u/%s:%p:%s:%c:%csync]: "_NM_UTILS_MACRO_FIRST(__VA_ARGS__), \
+                     "%s: entry[%u/%s:%p:%s:%chas:%csync]: "_NM_UTILS_MACRO_FIRST(__VA_ARGS__), \
                      self != singleton_instance \
                         ? nm_sprintf_buf (__prefix_buf, "%s%c[%p]", \
                                           _NMLOG2_PREFIX_NAME, \
@@ -121,7 +121,7 @@ NM_DEFINE_SINGLETON_GETTER (NMDefaultRouteManager, nm_default_route_manager_get,
                      NM_IS_DEVICE (__entry->source.pointer) ? "dev" : "vpn", \
                      __entry->source.pointer, \
                      NM_IS_DEVICE (__entry->source.pointer) ? nm_device_get_iface (__entry->source.device) : nm_active_connection_get_settings_connection_id (NM_ACTIVE_CONNECTION (__entry->source.vpn)), \
-                     (__entry->never_default ? '0' : '1'), \
+                     (__entry->never_default ? '-' : '+'), \
                      (__entry->synced ? '+' : '-') \
                      _NM_UTILS_MACRO_REST(__VA_ARGS__)); \
         } \
-- 
2.7.4