summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThomas Haller <thaller@redhat.com>2023-03-02 20:08:31 +0100
committerThomas Haller <thaller@redhat.com>2023-03-21 15:58:42 +0100
commit6fadba55606d820d9df5c932331e4b78f8d6c616 (patch)
tree16c9e8b1c46c23c39d40380d3fc2ce3e38827e86
parent7ca95cee15b32af2452aaf4a165eb5c634fba132 (diff)
platform: don't add onlink route to gateway in nm_platform_ip_route_sync()
Kernel rejects adding routes that have a gateway, if there is no direct (onlink) route to that gateway. The exact conditions are non-trivial due to the complexities of routing, but that's it basically. Anyway. In NetworkManager we don't want to have such non-obvious interdependencies. If the user configures a route with a gateway, but "forgets" to configure a direct route to the gateway, we don't assume that the user configured the wrong route. Instead, we assume the user forgot to configure the additional route and add it automatically. That is for convenience, but also because (as said) the rules for this are non-trivial. Moreover, it's problematic to report an error in routing during activation. Should we fail activation altogether? Should we just log an error and otherwise silently proceed? Logging is not a sensible behavior that the (possibly non-human) user can meaningfully handle. So we instead try to make it work. Previously, nm_platform_ip_route_sync() had the workaround of when we failed to configure a route and it looked like it might be due to the missing onlink route, we would add a suitable /32 / /128 route. The problem is that we want that NML3Cfg is aware of what routes we want to configure. The lower layer nm_platform_ip_route_sync() adding additional routes makes that difficult (maybe nm_platform_ip_route_sync() could return the additional routes that it added, but it doesn't). The better solution seems to be that nm_l3_config_data_add_dependent_onlink_routes() adds the required routes in NML3Cfg during commit. This is done since commit 40732115956f ('Revert "l3cfg: do not add dependent routes for non-default routes"'). Further, since commit ('platform: always reconfigure IP routes even if removed externally') we also always try to re-add the routes we want, regardless of whether they appear to be deleted by the user. So a suitable onlink route really should be always there, and there is no more need for this workaround.
-rw-r--r--src/libnm-platform/nm-platform.c76
1 files changed, 1 insertions, 75 deletions
diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c
index 732dd3235e..ba8569943c 100644
--- a/src/libnm-platform/nm-platform.c
+++ b/src/libnm-platform/nm-platform.c
@@ -4999,9 +4999,7 @@ nm_platform_ip_route_sync(NMPlatform *self,
for (i_type = 0; routes && i_type < 2; i_type++) {
for (i = 0; i < routes->len; i++) {
- gs_free char *extack_msg = NULL;
- gboolean gateway_route_added = FALSE;
- int r2;
+ gs_free char *extack_msg = NULL;
int r;
conf_o = routes->pdata[i];
@@ -5058,7 +5056,6 @@ nm_platform_ip_route_sync(NMPlatform *self,
}
}
-sync_route_add:
r = nm_platform_ip_route_add(self,
NMP_NLM_FLAG_APPEND
| NMP_NLM_FLAG_SUPPRESS_NETLINK_FAILURE,
@@ -5119,77 +5116,6 @@ sync_route_add:
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 if (!gateway_route_added
- && ((r == -ENETUNREACH && vt->is_ip4
- && !!NMP_OBJECT_CAST_IP4_ROUTE(conf_o)->gateway)
- || (r == -EHOSTUNREACH && !vt->is_ip4
- && !IN6_IS_ADDR_UNSPECIFIED(
- &NMP_OBJECT_CAST_IP6_ROUTE(conf_o)->gateway)))) {
- NMPObject oo;
-
- if (vt->is_ip4) {
- const NMPlatformIP4Route *rt = NMP_OBJECT_CAST_IP4_ROUTE(conf_o);
-
- nmp_object_stackinit(
- &oo,
- NMP_OBJECT_TYPE_IP4_ROUTE,
- &((NMPlatformIP4Route){
- .ifindex = rt->ifindex,
- .network = rt->gateway,
- .plen = 32,
- .metric = nm_platform_ip4_route_get_effective_metric(rt),
- .rt_source = rt->rt_source,
- .table_coerced = nm_platform_ip_route_get_effective_table(
- NM_PLATFORM_IP_ROUTE_CAST(rt)),
- }));
- } else {
- const NMPlatformIP6Route *rt = NMP_OBJECT_CAST_IP6_ROUTE(conf_o);
-
- nmp_object_stackinit(
- &oo,
- NMP_OBJECT_TYPE_IP6_ROUTE,
- &((NMPlatformIP6Route){
- .ifindex = rt->ifindex,
- .network = rt->gateway,
- .plen = 128,
- .metric = nm_platform_ip6_route_get_effective_metric(rt),
- .rt_source = rt->rt_source,
- .table_coerced = nm_platform_ip_route_get_effective_table(
- NM_PLATFORM_IP_ROUTE_CAST(rt)),
- }));
- }
-
- _LOG3D("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_strerror(r),
- nmp_object_to_string(&oo,
- NMP_OBJECT_TO_STRING_PUBLIC,
- sbuf2,
- sizeof(sbuf2)));
-
- r2 = nm_platform_ip_route_add(self,
- NMP_NLM_FLAG_APPEND
- | NMP_NLM_FLAG_SUPPRESS_NETLINK_FAILURE,
- &oo,
- NULL);
-
- if (r2 < 0) {
- _LOG3D("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_strerror(r2));
- }
-
- gateway_route_added = TRUE;
- goto sync_route_add;
} else {
_LOG3W("route-sync: failure to add IPv%c route: %s: %s",
vt->is_ip4 ? '4' : '6',