diff options
author | Thomas Haller <thaller@redhat.com> | 2023-03-02 20:08:31 +0100 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2023-03-21 15:58:42 +0100 |
commit | 6fadba55606d820d9df5c932331e4b78f8d6c616 (patch) | |
tree | 16c9e8b1c46c23c39d40380d3fc2ce3e38827e86 | |
parent | 7ca95cee15b32af2452aaf4a165eb5c634fba132 (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.c | 76 |
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', |