platform: adding onlink gateway route for manual addressesth/ipv6-ll-reapply-rh1552069-pt2

Kernel does not all allow to configure a route via a gateway, if the
gateway is not directly reachable.

For non-manually added routes (e.g. from DHCP), we ignore them as a
server configuration errors. For manually added routes, we try to work
around them.

Note that if the user adds a manual route that references a gateway,
maybe he should be required to also add a matching onlink route for
the gateway (or an address that results in a device-route), otherwise
the configuration could be considered invalid. That was however not
done historically, and also, it seems a rather unhelpful behavior.
NetworkManage should just make it work, not not assume anything is
wrong with the configuration. Similarly, for IPv4, the user could
configure the route as onlink, however, that still requires extra
configuration of which the user might not be aware.

This would apply for example, when a connection has method=auto,
and would obtain the routes automatically. It seems sensible to
allow the user to add a route via the gateway, if he ~knows~ that
this particular network will provide such a configuration via DHCP.

In the past however, we tried not to automatically add a device route,
but instead see whether we will get a suitable route via DHCP. If we
wouldn't get such a route, we would however fail the connection.
However, this is really very hard to get right.
We call ip_config_merge_and_apply() possibly before receiving automatic
IP configuration (commit 7070d17cedd09d07f12ce977dd1e16cecf8d4b45, "device: reset
@con_ip6_config on failure before RA"). In this case, we could not yet
configure the route. Instead, we also cannot fail (yet), because we should
wait whether we will receive a route that makes this configuration
feasable.
That is hard to get right. How long should we wait? If we get a DHCP lease
and still cannot add the route, should we fail the IP configuration or wait
longer for another lease? Worse, if we decide to fail the IP configuration,
it might not fail the entire activation. Instead, we would only mark the
current address family as failed. If we later get a DHCP lease, should we
retry to add the route again? -- probably yes. If we still fail, we would
need to keep the IP configuration in failed state, regardless that DHCP
succeeded. Part of the problem is, that we are bad at tracking the
failed state per IP method. So, if manual configuration fails but DHCP
succeeds, we get the state wrong. That should be fixed separately, but it
just shows how hard it is to have this route that we currently cannot
add, and wanting to wait for something that might never come, but still
fail at some point.

Instead, if we cannot add a route due to a missing onlink gateway,
just retry and add the /32 or /128 direct route ourself.

Note that for IPv6 routes that have a "src" address which is still
TENTATIVE, we also cannot currently add the route and retry later.
However, that is fundamentally different, because:
  - the configuration here is correct, it's only that the address
    didn't yet pass IPv6 DAD and kernel is being unhelpful (rh#1457196).
  - we only have to wait a few seconds for DAD to complete or fail.
    So, it's easy to implement this sensibly.

4 files changed, 62 insertions, 18 deletions

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 356f621924..b517e868ac 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -7978,10 +7978,8 @@ addrconf6_start_with_link_ready (NMDevice *self)
 	}
 
 	/* Apply any manual configuration before starting RA */
-	if (!ip_config_merge_and_apply (self, AF_INET6, TRUE)) {
+	if (!ip_config_merge_and_apply (self, AF_INET6, TRUE))
 		_LOGW (LOGD_IP6, "failed to apply manual IPv6 configuration");
-		g_clear_object (&priv->con_ip_config_6);
-	}
 
 	/* FIXME: These sysctls would probably be better set by the lndp ndisc itself. */
 	switch (nm_ndisc_get_node_type (priv->ndisc)) {
diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index dac0a5ff94..a645da2ee6 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -3902,7 +3902,8 @@ nm_platform_ip_route_sync (NMPlatform *self,
 
 	for (i_type = 0; routes && i_type < 2; i_type++) {
 		for (i = 0; i < routes->len; i++) {
-			NMPlatformError plerr;
+			NMPlatformError plerr, plerr2;
+			gboolean gateway_route_added = FALSE;
 
 			conf_o = routes->pdata[i];
 
@@ -3948,6 +3949,7 @@ nm_platform_ip_route_sync (NMPlatform *self,
 				}
 			}
 
+sync_route_add:
 			plerr = nm_platform_ip_route_add (self,
 			                                    NMP_NLM_FLAG_APPEND
 			                                  | NMP_NLM_FLAG_SUPPRESS_NETLINK_FAILURE,
@@ -3986,22 +3988,66 @@ nm_platform_ip_route_sync (NMPlatform *self,
 					if (!*out_temporary_not_available)
 						*out_temporary_not_available = g_ptr_array_new_full (0, (GDestroyNotify) nmp_object_unref);
 					g_ptr_array_add (*out_temporary_not_available, (gpointer) nmp_object_ref (conf_o));
-				} else {
-					const char *reason = "";
-
-					if (   (   -((int) plerr) == ENETUNREACH
-					        && vt->is_ip4
-					        && !!NMP_OBJECT_CAST_IP4_ROUTE (conf_o)->gateway)
-					    || (   -((int) plerr) == EHOSTUNREACH
-					        && !vt->is_ip4
-					        && !IN6_IS_ADDR_UNSPECIFIED (&NMP_OBJECT_CAST_IP6_ROUTE (conf_o)->gateway)))
-						reason = "; is the gateway directly reachable?";
+				} else if (   !gateway_route_added
+				           && (   (   -((int) plerr) == ENETUNREACH
+				                   && vt->is_ip4
+				                   && !!NMP_OBJECT_CAST_IP4_ROUTE (conf_o)->gateway)
+				               || (   -((int) plerr) == EHOSTUNREACH
+				                   && !vt->is_ip4
+				                   && !IN6_IS_ADDR_UNSPECIFIED (&NMP_OBJECT_CAST_IP6_ROUTE (conf_o)->gateway)))) {
+					NMPObject oo;
+
+					if (vt->is_ip4) {
+						const NMPlatformIP4Route *r = NMP_OBJECT_CAST_IP4_ROUTE (conf_o);
+
+						nmp_object_stackinit (&oo,
+						                      NMP_OBJECT_TYPE_IP4_ROUTE,
+						                      &((NMPlatformIP4Route) {
+						                          .network = r->gateway,
+						                          .plen = 32,
+						                          .metric = r->metric,
+						                          .rt_source = r->rt_source,
+						                          .table_coerced = r->table_coerced,
+						                      }));
+					} else {
+						const NMPlatformIP6Route *r = NMP_OBJECT_CAST_IP6_ROUTE (conf_o);
+
+						nmp_object_stackinit (&oo,
+						                      NMP_OBJECT_TYPE_IP6_ROUTE,
+						                      &((NMPlatformIP6Route) {
+						                          .network = r->gateway,
+						                          .plen = 128,
+						                          .metric = r->metric,
+						                          .rt_source = r->rt_source,
+						                          .table_coerced = r->table_coerced,
+						                      }));
+					}
 
-					_LOGW ("route-sync: failure to add IPv%c route: %s: %s%s",
+					_LOGD ("route-sync: failure to add IPv%c route: %s: %s; try adding direct route to gateway %s",
 					       vt->is_ip4 ? '4' : '6',
 					       nmp_object_to_string (conf_o, NMP_OBJECT_TO_STRING_PUBLIC, sbuf1, sizeof (sbuf1)),
 					       nm_platform_error_to_string (plerr, sbuf_err, sizeof (sbuf_err)),
-					       reason);
+					       nmp_object_to_string (&oo, NMP_OBJECT_TO_STRING_PUBLIC, sbuf2, sizeof (sbuf2)));
+
+					plerr2 = nm_platform_ip_route_add (self,
+					                                     NMP_NLM_FLAG_APPEND
+					                                   | NMP_NLM_FLAG_SUPPRESS_NETLINK_FAILURE,
+					                                   &oo);
+
+					if (plerr2 != NM_PLATFORM_ERROR_SUCCESS) {
+						_LOGD ("route-sync: failure to add gateway IPv%c route: %s: %s",
+						       vt->is_ip4 ? '4' : '6',
+						       nmp_object_to_string (conf_o, NMP_OBJECT_TO_STRING_PUBLIC, sbuf1, sizeof (sbuf1)),
+						       nm_platform_error_to_string (plerr, sbuf_err, sizeof (sbuf_err)));
+					}
+
+					gateway_route_added = TRUE;
+					goto sync_route_add;
+				} else {
+					_LOGW ("route-sync: failure to add IPv%c route: %s: %s",
+					       vt->is_ip4 ? '4' : '6',
+					       nmp_object_to_string (conf_o, NMP_OBJECT_TO_STRING_PUBLIC, sbuf1, sizeof (sbuf1)),
+					       nm_platform_error_to_string (plerr, sbuf_err, sizeof (sbuf_err)));
 					success = FALSE;
 				}
 			}
diff --git a/src/platform/nmp-object.c b/src/platform/nmp-object.c
index 666d334c23..8139072fcf 100644
--- a/src/platform/nmp-object.c
+++ b/src/platform/nmp-object.c
@@ -532,7 +532,7 @@ _nmp_object_stackinit_from_type (NMPObject *obj, NMPObjectType obj_type)
 }
 
 const NMPObject *
-nmp_object_stackinit (NMPObject *obj, NMPObjectType obj_type, const NMPlatformObject *plobj)
+nmp_object_stackinit (NMPObject *obj, NMPObjectType obj_type, gconstpointer plobj)
 {
 	const NMPClass *klass = nmp_class_from_type (obj_type);
 
diff --git a/src/platform/nmp-object.h b/src/platform/nmp-object.h
index 0dd2687a35..b524440392 100644
--- a/src/platform/nmp-object.h
+++ b/src/platform/nmp-object.h
@@ -490,7 +490,7 @@ nmp_object_unref (const NMPObject *obj)
 NMPObject *nmp_object_new (NMPObjectType obj_type, const NMPlatformObject *plob);
 NMPObject *nmp_object_new_link (int ifindex);
 
-const NMPObject *nmp_object_stackinit (NMPObject *obj, NMPObjectType obj_type, const NMPlatformObject *plobj);
+const NMPObject *nmp_object_stackinit (NMPObject *obj, NMPObjectType obj_type, gconstpointer plobj);
 
 static inline NMPObject *
 nmp_object_stackinit_obj (NMPObject *obj, const NMPObject *src)