fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-02-05 03:00:29 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

core: don't add dependent local route for addresses

Browse source 
When adding an IPv4 address, kernel automatically adds a local route.
This is done by fib_add_ifaddr(). Note that if the address is
IFA_F_SECONDARY, then the "src" is the primary address. That means, with

  nmcli connection add con-name t type ethernet ifname t autoconnect no \
     ipv4.method manual ipv6.method disabled \
     ipv4.addresses '192.168.77.10/24, 192.168.77.11/24'

we get two routes:

  "local 192.168.77.10 dev t table local proto kernel scope host src 192.168.77.10"
  "local 192.168.77.11 dev t table local proto kernel scope host src 192.168.77.10"

Our code would only generate instead:

  "local 192.168.77.10 dev t table local proto kernel scope host src 192.168.77.10"
  "local 192.168.77.11 dev t table local proto kernel scope host src 192.168.77.11"

Afterwards, this artificial route will be leaked:

    #!/bin/bash

    set -vx

    nmcli connection delete t || :
    ip link delete t || :

    ip link add name t type veth peer t-veth

    nmcli connection add con-name t type ethernet ifname t autoconnect no ipv4.method manual ipv4.addresses '192.168.77.10/24, 192.168.77.11/24' ipv6.method disabled

    nmcli connection up t

    ip route show table all dev t | grep --color '^\|192.168.77.11'

    sleep 1

    nmcli device modify t -ipv4.addresses 192.168.77.11/24

    ip route show table all dev t | grep --color '^\|192.168.77.11'

    ip route show table all dev t | grep -q 192.168.77.11 && echo "the local route 192.168.77.11 is still there, because NM adds a local route with wrong pref-src"

It will also be leaked because in the example above ipv4.route-table is
unset, so we are not in full route sync mode and the local table is not
synced.

This was introduced by commit 3e5fc04df3 ('core: add dependent local
routes configured by kernel'), but it's unclear to me why we really need
this. Drop it again and effectively revert commit 3e5fc04df3 ('core:
add dependent local routes configured by kernel').

I think this "solution" is still bad. We need to improve our route sync
approach with L3Cfg rework. For now, it's probably good enough.

https://bugzilla.redhat.com/show_bug.cgi?id=1907661
This commit is contained in:
Thomas Haller 2021-03-19 21:20:52 +01:00
parent fe1bf4c907
commit 557644f5e0
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
3 changed files with 91 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
src/core/nm-ip4-config.c
View file

@ -649,21 +649,6 @@ nm_ip4_config_add_dependent_routes(NMIP4Config *self,
if (my_addr->external)
continue;
/* Pre-generate local route added by kernel */
r = nmp_object_new(NMP_OBJECT_TYPE_IP4_ROUTE, NULL);
route = NMP_OBJECT_CAST_IP4_ROUTE(r);
route->ifindex = ifindex;
route->rt_source = NM_IP_CONFIG_SOURCE_KERNEL;
route->network = my_addr->address;
route->plen = 32;
route->pref_src = my_addr->address;
route->type_coerced = nm_platform_route_type_coerce(RTN_LOCAL);
route->scope_inv = nm_platform_route_scope_inv(RT_SCOPE_HOST);
route->table_coerced =
nm_platform_route_table_coerce(is_vrf ? route_table : RT_TABLE_LOCAL);
_add_route(self, r, NULL, NULL);
nm_clear_pointer(&r, nmp_object_unref);
if (nm_utils_ip4_address_is_zeronet(network)) {
/* Kernel doesn't add device-routes for destinations that
* start with 0.x.y.z. Skip them. */

16
src/core/nm-ip6-config.c
View file

@ -404,22 +404,6 @@ nm_ip6_config_add_dependent_routes(NMIP6Config *self,
if (my_addr->external)
continue;
{
nm_auto_nmpobj NMPObject *r = NULL;
/* Pre-generate local route added by kernel */
r = nmp_object_new(NMP_OBJECT_TYPE_IP6_ROUTE, NULL);
route = NMP_OBJECT_CAST_IP6_ROUTE(r);
route->ifindex = ifindex;
route->network = my_addr->address;
route->plen = 128;
route->type_coerced = nm_platform_route_type_coerce(RTN_LOCAL);
route->metric = 0;
route->table_coerced =
nm_platform_route_table_coerce(is_vrf ? route_table : RT_TABLE_LOCAL);
_add_route(self, r, NULL, NULL);
}
if (NM_FLAGS_HAS(my_addr->n_ifa_flags, IFA_F_NOPREFIXROUTE))
continue;
if (my_addr->plen == 0)

91
src/libnm-platform/nm-platform.c
View file

@ -4312,6 +4312,11 @@ nm_platform_ip_route_get_prune_list(NMPlatform * self,
GPtrArray * routes_prune;
const NMDedupMultiHeadEntry *head_entry;
CList * iter;
NMPlatformIP4Route rt_local4;
NMPlatformIP6Route rt_local6;
const NMPlatformLink * pllink;
const NMPlatformLnkVrf * lnk_vrf;
guint32 local_table;
nm_assert(NM_IS_PLATFORM(self));
nm_assert(NM_IN_SET(addr_family, AF_INET, AF_INET6));
@ -4326,6 +4331,14 @@ nm_platform_ip_route_get_prune_list(NMPlatform * self,
if (!head_entry)
return NULL;
lnk_vrf = nm_platform_link_get_lnk_vrf(self, ifindex, &pllink);
if (!lnk_vrf && pllink && pllink->master > 0)
lnk_vrf = nm_platform_link_get_lnk_vrf(self, pllink->master, NULL);
local_table = lnk_vrf ? lnk_vrf->table : RT_TABLE_LOCAL;
rt_local4.plen = 0;
rt_local6.plen = 0;
routes_prune = g_ptr_array_new_full(head_entry->len, (GDestroyNotify) nm_dedup_multi_obj_unref);
c_list_for_each (iter, &head_entry->lst_entries_head) {
@ -4342,6 +4355,84 @@ nm_platform_ip_route_get_prune_list(NMPlatform * self,
continue;
break;
case NM_IP_ROUTE_TABLE_SYNC_MODE_ALL:
/* FIXME: we should better handle routes that are automatically added by kernel.
*
* For now, make a good guess which are those routes and exclude them from
* pruning them. */
if (NM_IS_IPv4(addr_family)) {
/* for each IPv4 address kernel adds a route like
*
* local $ADDR dev $IFACE table local proto kernel scope host src $PRIMARY_ADDR
*
* Check whether route could be of that kind. */
if (nm_platform_ip_route_get_effective_table(&rt->rx) == local_table
&& rt->rx.plen == 32 && rt->rx.rt_source == NM_IP_CONFIG_SOURCE_RTPROT_KERNEL
&& rt->rx.metric == 0
&& rt->r4.scope_inv == nm_platform_route_scope_inv(RT_SCOPE_HOST)
&& rt->r4.gateway == INADDR_ANY) {
if (rt_local4.plen == 0) {
rt_local4 = (NMPlatformIP4Route){
.ifindex = ifindex,
.type_coerced = nm_platform_route_type_coerce(RTN_LOCAL),
.plen = 32,
.rt_source = NM_IP_CONFIG_SOURCE_RTPROT_KERNEL,
.metric = 0,
.table_coerced = nm_platform_route_table_coerce(local_table),
.scope_inv = nm_platform_route_scope_inv(RT_SCOPE_HOST),
.gateway = INADDR_ANY,
};
}
/* the possible "network" depends on the addresses we have. We don't check that
* carefully. If the other parameters match, we assume that this route is the one
* generated by kernel. */
rt_local4.network = rt->r4.network;
rt_local4.pref_src = rt->r4.pref_src;
/* to be more confident about comparing the value, use our nm_platform_ip4_route_cmp()
* implementation. That will also consider parameters that we leave unspecified here. */
if (nm_platform_ip4_route_cmp(&rt->r4,
&rt_local4,
NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY)
== 0)
continue;
}
} else {
/* for each IPv6 address (that is no longer tentative) kernel adds a route like
*
* local $ADDR dev $IFACE table local proto kernel metric 0 pref medium
*
* Same as for the IPv4 case. */
if (nm_platform_ip_route_get_effective_table(&rt->rx) == local_table
&& rt->rx.plen == 128 && rt->rx.rt_source == NM_IP_CONFIG_SOURCE_RTPROT_KERNEL
&& rt->rx.metric == 0 && rt->r6.rt_pref == NM_ICMPV6_ROUTER_PREF_MEDIUM
&& IN6_IS_ADDR_UNSPECIFIED(&rt->r6.gateway)) {
if (rt_local6.plen == 0) {
rt_local6 = (NMPlatformIP6Route){
.ifindex = ifindex,
.type_coerced = nm_platform_route_type_coerce(RTN_LOCAL),
.plen = 128,
.rt_source = NM_IP_CONFIG_SOURCE_RTPROT_KERNEL,
.metric = 0,
.table_coerced = nm_platform_route_table_coerce(local_table),
.rt_pref = NM_ICMPV6_ROUTER_PREF_MEDIUM,
.gateway = IN6ADDR_ANY_INIT,
};
}
rt_local6.network = rt->r6.network;
if (nm_platform_ip6_route_cmp(&rt->r6,
&rt_local6,
NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY)
== 0)
continue;
}
}
break;
case NM_IP_ROUTE_TABLE_SYNC_MODE_ALL_PRUNE:
break;