connectivity: consider default route for global connectivity stateth/connectivity-per-af-fixes

When we agregate the connectivity state, only devices that
have the best default route should be considered.

Since we do connectivity checking per-device, the per-device check
does not care whether traffic to the internet is really routed via this
device.

But when talking about the global connectivity state, we care mostly
about the (best) default route. So, we should not allow a device with
worse or now default route, to contribute its connectivity state.

Fixes: 6b7e9f9b225e81d365fd95901a88a7bc59c1eb39

3 files changed, 89 insertions, 33 deletions

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index fb482a9ab7..55297e0692 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -3080,17 +3080,26 @@ nm_device_check_connectivity_cancel (NMDeviceConnectivityHandle *handle)
 }
 
 NMConnectivityState
-nm_device_get_connectivity_state (NMDevice *self)
+nm_device_get_connectivity_state (NMDevice *self, int addr_family)
 {
 	NMDevicePrivate *priv;
+	const gboolean IS_IPv4 = (addr_family == AF_INET);
 
 	g_return_val_if_fail (NM_IS_DEVICE (self), NM_CONNECTIVITY_UNKNOWN);
+	nm_assert_addr_family (addr_family);
 
 	priv = NM_DEVICE_GET_PRIVATE (self);
 
-	return NM_MAX_WITH_CMP (nm_connectivity_state_cmp,
-	                        priv->concheck_x[0].state,
-	                        priv->concheck_x[1].state);
+	switch (addr_family) {
+	case AF_INET:
+	case AF_INET6:
+		return priv->concheck_x[IS_IPv4].state;
+	default:
+		nm_assert (addr_family == AF_UNSPEC);
+		return NM_MAX_WITH_CMP (nm_connectivity_state_cmp,
+		                        priv->concheck_x[0].state,
+		                        priv->concheck_x[1].state);
+	}
 }
 
 /*****************************************************************************/
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
index cd07e24630..8957f3fa5b 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -855,7 +855,7 @@ NMDeviceConnectivityHandle *nm_device_check_connectivity (NMDevice *self,
 
 void nm_device_check_connectivity_cancel (NMDeviceConnectivityHandle *handle);
 
-NMConnectivityState nm_device_get_connectivity_state (NMDevice *self);
+NMConnectivityState nm_device_get_connectivity_state (NMDevice *self, int addr_family);
 
 typedef struct _NMBtVTableNetworkServer NMBtVTableNetworkServer;
 struct _NMBtVTableNetworkServer {
diff --git a/src/nm-manager.c b/src/nm-manager.c
index 11809de3d2..2b14918683 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -2843,44 +2843,91 @@ device_realized (NMDevice *device,
 	_emit_device_added_removed (self, device, nm_device_is_real (device));
 }
 
-static void
-device_connectivity_changed (NMDevice *device,
-                             GParamSpec *pspec,
-                             NMManager *self)
+static NMConnectivityState
+_get_best_connectivity (NMManager *self, int addr_family)
 {
 	NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
-	NMConnectivityState best_state = NM_CONNECTIVITY_UNKNOWN;
-	NMConnectivityState state;
+	NMConnectivityState best_state;
 	NMDevice *dev;
+	gint64 best_metric;
+
+	if (addr_family == AF_UNSPEC) {
+		best_state = _get_best_connectivity (self, AF_INET);
+		if (nm_connectivity_state_cmp (best_state, NM_CONNECTIVITY_FULL) >= 0) {
+			/* already FULL IPv4 connectivity. No need to check IPv6, it doesn't get
+			 * better. */
+			return best_state;
+		}
+		return NM_MAX_WITH_CMP (nm_connectivity_state_cmp,
+		                        best_state,
+		                        _get_best_connectivity (self, AF_INET6));
+	}
 
-	best_state = nm_device_get_connectivity_state (device);
-	if (best_state < NM_CONNECTIVITY_FULL) {
-		/* FIXME: is this really correct, to considere devices that don't have
-		 * (the best) default route for connectivity checking? */
-		c_list_for_each_entry (dev, &priv->devices_lst_head, devices_lst) {
-			state = nm_device_get_connectivity_state (dev);
-			if (nm_connectivity_state_cmp (state, best_state) <= 0)
-				continue;
+	nm_assert_addr_family (addr_family);
+
+	best_state = NM_CONNECTIVITY_UNKNOWN;
+	best_metric = G_MAXINT64;
+	c_list_for_each_entry (dev, &priv->devices_lst_head, devices_lst) {
+		const NMPObject *r;
+		NMConnectivityState state;
+		gint64 metric;
+
+		r = nm_device_get_best_default_route (dev, addr_family);
+		if (r) {
+			metric = nm_utils_ip_route_metric_normalize (addr_family,
+			                                             NMP_OBJECT_CAST_IP_ROUTE (r)->metric);
+		} else {
+			/* if all devices have no default-route, we still include the best
+			 * of all connectivity state of all the devices. */
+			metric = G_MAXINT64;
+		}
+
+		if (metric > best_metric) {
+			/* we already have a default route with better metric. The connectivity state
+			 * of this device is irreleavnt. */
+			continue;
+		}
+
+		state = nm_device_get_connectivity_state (dev, addr_family);
+		if (metric < best_metric) {
+			/* this device has a better default route. It wins. */
+			best_metric = metric;
 			best_state = state;
-			if (nm_connectivity_state_cmp (best_state, NM_CONNECTIVITY_FULL) >= 0) {
-				/* it doesn't get better than this. */
-				break;
-			}
+		} else {
+			best_state = NM_MAX_WITH_CMP (nm_connectivity_state_cmp,
+			                              best_state,
+			                              state);
+		}
+
+		if (nm_connectivity_state_cmp (best_state, NM_CONNECTIVITY_FULL) >= 0) {
+			/* it doesn't get better than FULL. We are done. */
+			break;
 		}
 	}
-	nm_assert (best_state <= NM_CONNECTIVITY_FULL);
-	nm_assert (nm_connectivity_state_cmp (best_state, NM_CONNECTIVITY_FULL) <= 0);
 
-	if (best_state != priv->connectivity_state) {
-		priv->connectivity_state = best_state;
+	return best_state;
+}
 
-		_LOGD (LOGD_CORE, "connectivity checking indicates %s",
-		       nm_connectivity_state_to_string (priv->connectivity_state));
+static void
+device_connectivity_changed (NMDevice *device,
+                             GParamSpec *pspec,
+                             NMManager *self)
+{
+	NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
+	NMConnectivityState best_state;
 
-		nm_manager_update_state (self);
-		_notify (self, PROP_CONNECTIVITY);
-		nm_dispatcher_call_connectivity (priv->connectivity_state, NULL, NULL, NULL);
-	}
+	best_state = _get_best_connectivity (self, AF_UNSPEC);
+	if (best_state == priv->connectivity_state)
+		return;
+
+	priv->connectivity_state = best_state;
+
+	_LOGD (LOGD_CORE, "connectivity checking indicates %s",
+	       nm_connectivity_state_to_string (priv->connectivity_state));
+
+	nm_manager_update_state (self);
+	_notify (self, PROP_CONNECTIVITY);
+	nm_dispatcher_call_connectivity (priv->connectivity_state, NULL, NULL, NULL);
 }
 
 static void