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

core: fix treating route metric zero of IPv6 routes special

Browse source 
Userspace cannot add IPv6 routes with metric 0. Trying to do that, will
be coerced by kernel to route metric 1024. For IPv4 this is different,
and metric zero is commonly allowed.

However, kernel itself can add IPv6 routes with metric zero:

  # ip -6 route show table local
  local fe80::2029:c7ff:fec9:698a dev v proto kernel metric 0 pref medium

That means, we must not treat route metric zero special for most cases.
Only, when we want to add routes (based on user configuration), we must
coerce a route metric of zero to 1024.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/563
This commit is contained in:
Thomas Haller 2020-07-03 15:25:51 +02:00
parent 74bfc24057
commit 1b408e243d
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
9 changed files with 66 additions and 64 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/dhcp/nm-dhcp-utils.c
View file

@ -440,10 +440,10 @@ nm_dhcp_utils_ip4_config_from_options (NMDedupMultiIndex *multi_idx,
if (gateway_has) {
const NMPlatformIP4Route r = {
.rt_source = NM_IP_CONFIG_SOURCE_DHCP,
.gateway = gateway,
.rt_source = NM_IP_CONFIG_SOURCE_DHCP,
.gateway = gateway,
.table_coerced = nm_platform_route_table_coerce (route_table),
.metric = route_metric,
.metric = route_metric,
};
nm_ip4_config_add_route (ip4_config, &r, NULL);

12
src/nm-core-utils.h
View file

@ -156,8 +156,13 @@ double nm_utils_exp10 (gint16 e);
* nm_utils_ip6_route_metric_normalize:
* @metric: the route metric
*
* For IPv6 route, kernel treats the value 0 as IP6_RT_PRIO_USER (1024).
* Thus, when comparing metric (values), we want to treat zero as NM_PLATFORM_ROUTE_METRIC_DEFAULT_IP6.
* For IPv6 route, when adding a route via netlink, kernel treats the value 0 as IP6_RT_PRIO_USER (1024).
* So, user space cannot add routes with such a metric, and 0 gets "normalized"
* to NM_PLATFORM_ROUTE_METRIC_DEFAULT_IP6.
*
* Note that kernel itself can add IPv6 routes with metric zero. Also, you can delete
* them, but mostly because with `ip -6 route delete ... metric 0` the 0 acts as a wildcard
* and kills the first matching route.
*
* Returns: @metric, if @metric is not zero, otherwise 1024.
*/
@ -174,9 +179,8 @@ nm_utils_ip_route_metric_normalize (int addr_family, guint32 metric)
}
static inline guint32
nm_utils_ip_route_metric_penalize (int addr_family, guint32 metric, guint32 penalty)
nm_utils_ip_route_metric_penalize (guint32 metric, guint32 penalty)
{
metric = nm_utils_ip_route_metric_normalize (addr_family, metric);
if (metric < G_MAXUINT32 - penalty)
return metric + penalty;
return G_MAXUINT32;

31
src/nm-ip4-config.c
View file

@ -393,10 +393,6 @@ nm_ip_config_iter_ip4_route_init (NMDedupMultiIter *ipconf_iter, const NMIP4Conf
const NMPObject *
_nm_ip_config_best_default_route_find_better (const NMPObject *obj_cur, const NMPObject *obj_cmp)
{
int addr_family;
int c;
guint metric_cur, metric_cmp;
nm_assert ( !obj_cur
|| NM_IN_SET (NMP_OBJECT_GET_TYPE (obj_cur), NMP_OBJECT_TYPE_IP4_ROUTE, NMP_OBJECT_TYPE_IP6_ROUTE));
nm_assert ( !obj_cmp
@ -410,17 +406,20 @@ _nm_ip_config_best_default_route_find_better (const NMPObject *obj_cur, const NM
* @obj_cmp is also a default route and returns the best of both. */
if ( obj_cmp
&& nm_ip_config_best_default_route_is (obj_cmp)) {
guint32 metric_cur, metric_cmp;
if (!obj_cur)
return obj_cmp;
addr_family = NMP_OBJECT_GET_CLASS (obj_cmp)->addr_family;
metric_cur = nm_utils_ip_route_metric_normalize (addr_family, NMP_OBJECT_CAST_IP_ROUTE (obj_cur)->metric);
metric_cmp = nm_utils_ip_route_metric_normalize (addr_family, NMP_OBJECT_CAST_IP_ROUTE (obj_cmp)->metric);
metric_cur = NMP_OBJECT_CAST_IP_ROUTE (obj_cur)->metric;
metric_cmp = NMP_OBJECT_CAST_IP_ROUTE (obj_cmp)->metric;
if (metric_cmp < metric_cur)
return obj_cmp;
if (metric_cmp == metric_cur) {
int c;
/* Routes have the same metric. We still want to deterministically
* prefer one or the other. It's important to consistently choose one
* or the other, so that the order doesn't matter how routes are added
@ -1002,6 +1001,7 @@ nm_ip4_config_merge_setting (NMIP4Config *self,
for (i = 0; i < nroutes; i++) {
NMIPRoute *s_route = nm_setting_ip_config_get_route (setting, i);
NMPlatformIP4Route route;
gint64 m;
if (nm_ip_route_get_family (s_route) != AF_INET) {
nm_assert_not_reached ();
@ -1015,10 +1015,11 @@ nm_ip4_config_merge_setting (NMIP4Config *self,
nm_assert (route.plen <= 32);
nm_ip_route_get_next_hop_binary (s_route, &route.gateway);
if (nm_ip_route_get_metric (s_route) == -1)
m = nm_ip_route_get_metric (s_route);
if (m < 0)
route.metric = route_metric;
else
route.metric = nm_ip_route_get_metric (s_route);
route.metric = m;
route.rt_source = NM_IP_CONFIG_SOURCE_USER;
route.network = nm_utils_ip4_address_clear_host_address (route.network, route.plen);
@ -1141,8 +1142,10 @@ nm_ip4_config_create_setting (const NMIP4Config *self)
continue;
s_route = nm_ip_route_new_binary (AF_INET,
&route->network, route->plen,
&route->gateway, route->metric,
&route->network,
route->plen,
&route->gateway,
route->metric,
NULL);
nm_setting_ip_config_add_route (s_ip4, s_route);
nm_ip_route_unref (s_route);
@ -1227,7 +1230,7 @@ nm_ip4_config_merge (NMIP4Config *dst,
if (default_route_metric_penalty) {
NMPlatformIP4Route r = *r_src;
r.metric = nm_utils_ip_route_metric_penalize (AF_INET, r.metric, default_route_metric_penalty);
r.metric = nm_utils_ip_route_metric_penalize (r.metric, default_route_metric_penalty);
_add_route (dst, NULL, &r, NULL);
continue;
}
@ -1459,7 +1462,7 @@ nm_ip4_config_subtract (NMIP4Config *dst,
* the routes. */
o_lookup = nmp_object_stackinit_obj (&o_lookup_copy, o_src);
rr = NMP_OBJECT_CAST_IP4_ROUTE (&o_lookup_copy);
rr->metric = nm_utils_ip_route_metric_penalize (AF_INET, rr->metric, default_route_metric_penalty);
rr->metric = nm_utils_ip_route_metric_penalize (rr->metric, default_route_metric_penalty);
} else
o_lookup = o_src;
@ -1610,7 +1613,7 @@ _nm_ip4_config_intersect_helper (NMIP4Config *dst,
* the routes. */
o_lookup = nmp_object_stackinit_obj (&o_lookup_copy, o_dst);
rr = NMP_OBJECT_CAST_IP4_ROUTE (&o_lookup_copy);
rr->metric = nm_utils_ip_route_metric_penalize (AF_INET, rr->metric, default_route_metric_penalty);
rr->metric = nm_utils_ip_route_metric_penalize (rr->metric, default_route_metric_penalty);
} else
o_lookup = o_dst;

22
src/nm-ip6-config.c
View file

@ -650,6 +650,7 @@ nm_ip6_config_merge_setting (NMIP6Config *self,
for (i = 0; i < nroutes; i++) {
NMIPRoute *s_route = nm_setting_ip_config_get_route (setting, i);
NMPlatformIP6Route route;
gint64 m;
if (nm_ip_route_get_family (s_route) != AF_INET6) {
nm_assert_not_reached ();
@ -663,10 +664,11 @@ nm_ip6_config_merge_setting (NMIP6Config *self,
nm_assert (route.plen <= 128);
nm_ip_route_get_next_hop_binary (s_route, &route.gateway);
if (nm_ip_route_get_metric (s_route) == -1)
m = nm_ip_route_get_metric (s_route);
if (m < 0)
route.metric = route_metric;
else
route.metric = nm_ip_route_get_metric (s_route);
route.metric = nm_utils_ip6_route_metric_normalize (m);
route.rt_source = NM_IP_CONFIG_SOURCE_USER;
nm_utils_ip6_address_clear_host_address (&route.network, &route.network, route.plen);
@ -797,8 +799,10 @@ nm_ip6_config_create_setting (const NMIP6Config *self)
continue;
s_route = nm_ip_route_new_binary (AF_INET6,
&route->network, route->plen,
&route->gateway, route->metric,
&route->network,
route->plen,
&route->gateway,
route->metric,
NULL);
nm_setting_ip_config_add_route (s_ip6, s_route);
nm_ip_route_unref (s_route);
@ -882,7 +886,7 @@ nm_ip6_config_merge (NMIP6Config *dst,
if (default_route_metric_penalty) {
NMPlatformIP6Route r = *r_src;
r.metric = nm_utils_ip_route_metric_penalize (AF_INET6, r.metric, default_route_metric_penalty);
r.metric = nm_utils_ip_route_metric_penalize (r.metric, default_route_metric_penalty);
_add_route (dst, NULL, &r, NULL);
continue;
}
@ -1052,7 +1056,7 @@ nm_ip6_config_subtract (NMIP6Config *dst,
* the routes. */
o_lookup = nmp_object_stackinit_obj (&o_lookup_copy, o_src);
rr = NMP_OBJECT_CAST_IP6_ROUTE (&o_lookup_copy);
rr->metric = nm_utils_ip_route_metric_penalize (AF_INET6, rr->metric, default_route_metric_penalty);
rr->metric = nm_utils_ip_route_metric_penalize (rr->metric, default_route_metric_penalty);
} else
o_lookup = o_src;
@ -1173,7 +1177,7 @@ _nm_ip6_config_intersect_helper (NMIP6Config *dst,
* the routes. */
o_lookup = nmp_object_stackinit_obj (&o_lookup_copy, o_dst);
rr = NMP_OBJECT_CAST_IP6_ROUTE (&o_lookup_copy);
rr->metric = nm_utils_ip_route_metric_penalize (AF_INET6, rr->metric, default_route_metric_penalty);
rr->metric = nm_utils_ip_route_metric_penalize (rr->metric, default_route_metric_penalty);
} else
o_lookup = o_dst;
@ -2052,8 +2056,8 @@ nm_ip6_config_get_direct_route_for_host (const NMIP6Config *self,
if (!nm_utils_ip6_address_same_prefix (host, &item->network, item->plen))
continue;
if (best_route &&
nm_utils_ip6_route_metric_normalize (best_route->metric) <= nm_utils_ip6_route_metric_normalize (item->metric))
if ( best_route
&& best_route->metric <= item->metric)
continue;
best_route = item;

7
src/nm-manager.c
View file

@ -3044,10 +3044,9 @@ _get_best_connectivity (NMManager *self, int addr_family)
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 (r)
metric = 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;

6
src/nm-policy.c
View file

@ -442,14 +442,12 @@ get_best_active_connection (NMPolicy *self,
*
* In this case, is it really the best device? Why do we even need the best
* device?? */
metric = nm_utils_ip_route_metric_normalize (addr_family,
NMP_OBJECT_CAST_IP_ROUTE (r)->metric);
metric = NMP_OBJECT_CAST_IP_ROUTE (r)->metric;
is_fully_activated = TRUE;
} else if ( !fully_activated
&& (connection = nm_device_get_applied_connection (device))
&& nm_utils_connection_has_default_route (connection, addr_family, NULL)) {
metric = nm_utils_ip_route_metric_normalize (addr_family,
nm_device_get_route_metric (device, addr_family));
metric = nm_device_get_route_metric (device, addr_family);
is_fully_activated = FALSE;
} else
continue;

24
src/platform/nm-platform.c
View file

@ -4571,7 +4571,6 @@ nm_platform_ip_route_normalize (int addr_family,
r6->table_coerced = nm_platform_route_table_coerce (nm_platform_route_table_uncoerce (r6->table_coerced, TRUE));
nm_utils_ip6_address_clear_host_address (&r6->network, &r6->network, r6->plen);
r6->rt_source = nmp_utils_ip_config_source_round_trip_rtprot (r6->rt_source),
r6->metric = nm_utils_ip6_route_metric_normalize (r6->metric);
nm_utils_ip6_address_clear_host_address (&r6->src, &r6->src, r6->src_plen);
break;
default:
@ -7463,7 +7462,7 @@ nm_platform_ip6_route_hash_update (const NMPlatformIP6Route *obj, NMPlatformIPRo
nm_platform_route_table_uncoerce (obj->table_coerced, TRUE),
*nm_utils_ip6_address_clear_host_address (&a1, &obj->network, obj->plen),
obj->plen,
nm_utils_ip6_route_metric_normalize (obj->metric),
obj->metric,
*nm_utils_ip6_address_clear_host_address (&a2, &obj->src, obj->src_plen),
obj->src_plen);
break;
@ -7473,7 +7472,7 @@ nm_platform_ip6_route_hash_update (const NMPlatformIP6Route *obj, NMPlatformIPRo
nm_platform_route_table_uncoerce (obj->table_coerced, TRUE),
*nm_utils_ip6_address_clear_host_address (&a1, &obj->network, obj->plen),
obj->plen,
nm_utils_ip6_route_metric_normalize (obj->metric),
obj->metric,
*nm_utils_ip6_address_clear_host_address (&a2, &obj->src, obj->src_plen),
obj->src_plen,
/* on top of WEAK_ID: */
@ -7487,7 +7486,7 @@ nm_platform_ip6_route_hash_update (const NMPlatformIP6Route *obj, NMPlatformIPRo
obj->ifindex,
*nm_utils_ip6_address_clear_host_address (&a1, &obj->network, obj->plen),
obj->plen,
nm_utils_ip6_route_metric_normalize (obj->metric),
obj->metric,
obj->gateway,
obj->pref_src,
*nm_utils_ip6_address_clear_host_address (&a2, &obj->src, obj->src_plen),
@ -7550,7 +7549,7 @@ nm_platform_ip6_route_cmp (const NMPlatformIP6Route *a, const NMPlatformIP6Route
nm_platform_route_table_uncoerce (b->table_coerced, TRUE));
NM_CMP_DIRECT_IN6ADDR_SAME_PREFIX (&a->network, &b->network, MIN (a->plen, b->plen));
NM_CMP_FIELD (a, b, plen);
NM_CMP_DIRECT (nm_utils_ip6_route_metric_normalize (a->metric), nm_utils_ip6_route_metric_normalize (b->metric));
NM_CMP_FIELD (a, b, metric);
NM_CMP_DIRECT_IN6ADDR_SAME_PREFIX (&a->src, &b->src, MIN (a->src_plen, b->src_plen));
NM_CMP_FIELD (a, b, src_plen);
if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID) {
@ -7573,10 +7572,7 @@ nm_platform_ip6_route_cmp (const NMPlatformIP6Route *a, const NMPlatformIP6Route
else
NM_CMP_FIELD_IN6ADDR (a, b, network);
NM_CMP_FIELD (a, b, plen);
if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY)
NM_CMP_DIRECT (nm_utils_ip6_route_metric_normalize (a->metric), nm_utils_ip6_route_metric_normalize (b->metric));
else
NM_CMP_FIELD (a, b, metric);
NM_CMP_FIELD (a, b, metric);
NM_CMP_FIELD_IN6ADDR (a, b, gateway);
NM_CMP_FIELD_IN6ADDR (a, b, pref_src);
if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY) {
@ -8054,14 +8050,6 @@ nm_platform_netns_push (NMPlatform *self, NMPNetns **netns)
/*****************************************************************************/
static guint32
_vtr_v4_metric_normalize (guint32 metric)
{
return metric;
}
/*****************************************************************************/
const _NMPlatformVTableRouteUnion nm_platform_vtable_route = {
.v4 = {
.is_ip4 = TRUE,
@ -8070,7 +8058,6 @@ const _NMPlatformVTableRouteUnion nm_platform_vtable_route = {
.sizeof_route = sizeof (NMPlatformIP4Route),
.route_cmp = (int (*) (const NMPlatformIPXRoute *a, const NMPlatformIPXRoute *b, NMPlatformIPRouteCmpType cmp_type)) nm_platform_ip4_route_cmp,
.route_to_string = (const char *(*) (const NMPlatformIPXRoute *route, char *buf, gsize len)) nm_platform_ip4_route_to_string,
.metric_normalize = _vtr_v4_metric_normalize,
},
.v6 = {
.is_ip4 = FALSE,
@ -8079,7 +8066,6 @@ const _NMPlatformVTableRouteUnion nm_platform_vtable_route = {
.sizeof_route = sizeof (NMPlatformIP6Route),
.route_cmp = (int (*) (const NMPlatformIPXRoute *a, const NMPlatformIPXRoute *b, NMPlatformIPRouteCmpType cmp_type)) nm_platform_ip6_route_cmp,
.route_to_string = (const char *(*) (const NMPlatformIPXRoute *route, char *buf, gsize len)) nm_platform_ip6_route_to_string,
.metric_normalize = nm_utils_ip6_route_metric_normalize,
},
};

6
src/platform/nm-platform.h
View file

@ -382,7 +382,10 @@ typedef union {
#define NM_PLATFORM_ROUTE_METRIC_DEFAULT_IP4 0
/* Default value for adding an IPv6 route. This is also what iproute2 does.
* Adding an IPv6 route with metric 0, kernel translates to IP6_RT_PRIO_USER (1024). */
* Adding an IPv6 route with metric 0, kernel translates to IP6_RT_PRIO_USER (1024).
*
* Note that kernel doesn't allow adding IPv6 routes with metric zero via netlink.
* It however can itself add routes with metric zero. */
#define NM_PLATFORM_ROUTE_METRIC_DEFAULT_IP6 1024
/* For IPv4, kernel adds a device route (subnet routes) with metric 0 when user
@ -720,7 +723,6 @@ typedef struct {
gsize sizeof_route;
int (*route_cmp) (const NMPlatformIPXRoute *a, const NMPlatformIPXRoute *b, NMPlatformIPRouteCmpType cmp_type);
const char *(*route_to_string) (const NMPlatformIPXRoute *route, char *buf, gsize len);
guint32 (*metric_normalize) (guint32 metric);
} NMPlatformVTableRoute;
typedef union {

16
src/platform/tests/test-route.c
View file

@ -319,7 +319,7 @@ test_ip6_route (void)
guint8 plen = 64;
struct in6_addr gateway, pref_src;
/* Choose a high metric so that we hopefully don't conflict. */
int metric = 22987;
const int metric = 22987;
int mss = 1000;
inet_pton (AF_INET6, "2001:db8:a:b:0:0:0:0", &network);
@ -365,7 +365,7 @@ test_ip6_route (void)
rts[0].ifindex = ifindex;
rts[0].gateway = in6addr_any;
rts[0].pref_src = in6addr_any;
rts[0].metric = nm_utils_ip6_route_metric_normalize (metric);
rts[0].metric = metric;
rts[0].mss = mss;
rts[1].rt_source = nmp_utils_ip_config_source_round_trip_rtprot (NM_IP_CONFIG_SOURCE_USER);
rts[1].network = network;
@ -373,7 +373,7 @@ test_ip6_route (void)
rts[1].ifindex = ifindex;
rts[1].gateway = gateway;
rts[1].pref_src = pref_src;
rts[1].metric = nm_utils_ip6_route_metric_normalize (metric);
rts[1].metric = metric;
rts[1].mss = mss;
rts[2].rt_source = nmp_utils_ip_config_source_round_trip_rtprot (NM_IP_CONFIG_SOURCE_USER);
rts[2].network = in6addr_any;
@ -381,7 +381,7 @@ test_ip6_route (void)
rts[2].ifindex = ifindex;
rts[2].gateway = gateway;
rts[2].pref_src = in6addr_any;
rts[2].metric = nm_utils_ip6_route_metric_normalize (metric);
rts[2].metric = metric;
rts[2].mss = mss;
g_assert_cmpint (routes->len, ==, 3);
nmtst_platform_ip6_routes_equal_aptr ((const NMPObject *const*) routes->pdata, rts, routes->len, TRUE);
@ -586,7 +586,13 @@ test_ip6_route_get (void)
NMTST_WAIT_ASSERT (100, {
nmtstp_wait_for_signal (NM_PLATFORM_GET, 10);
if (nmtstp_ip6_route_get (NM_PLATFORM_GET, ifindex, nmtst_inet6_from_string ("fd01:abcd::"), 64, 0, NULL, 0))
if (nmtstp_ip6_route_get (NM_PLATFORM_GET,
ifindex,
nmtst_inet6_from_string ("fd01:abcd::"),
64,
NM_PLATFORM_ROUTE_METRIC_DEFAULT_IP6,
NULL,
0))
break;
});