fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2025-12-26 13:30:08 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

libnm,platform: fix range for "weight" property of next hops for routes

Browse source 
In kernel, the valid range for the weight is 1-256 (on netlink this is
expressed as u8 in rtnh_hops, ranging 0-255).

We need an additional value, to represent

- unset weight, for non-ECMP routes in kernel.

- in libnm API, to express routes that should not be merged as ECMP
  routes (the default).

Extend the type in NMPlatformIP4Route.weight to u16, and fix the code
for the special handling of the numeric range.

Also the libnm API needs to change. Modify the type of the attribute on
D-Bus from "b" to "u", to use a 32 bit integer. We use 32 bit, because
we already have common code to handle 32 bit unsigned integers, despite
only requiring 257 values. It seems better to stick to a few data types
(u32) instead of introducing more, only because the range is limited.

Co-Authored-By: Fernando Fernandez Mancera <ffmancera@riseup.net>

Fixes: 1bbdecf5e1 ('platform: manage ECMP routes')
This commit is contained in:
Thomas Haller 2022-12-24 21:50:38 +01:00
parent ff472d8e59
commit 3cd02b6ed6
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
10 changed files with 64 additions and 51 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/core/NetworkManagerUtils.c
View file

@ -1382,7 +1382,7 @@ nm_utils_ip_route_attribute_to_platform(int addr_family,
GET_ATTR(NM_IP_ROUTE_ATTRIBUTE_TOS, r4->tos, BYTE, byte, 0);
GET_ATTR(NM_IP_ROUTE_ATTRIBUTE_SCOPE, scope, BYTE, byte, RT_SCOPE_NOWHERE);
GET_ATTR(NM_IP_ROUTE_ATTRIBUTE_WEIGHT, r4->weight, BYTE, byte, 0);
GET_ATTR(NM_IP_ROUTE_ATTRIBUTE_WEIGHT, r4->weight, UINT32, uint32, 0);
r4->scope_inv = nm_platform_route_scope_inv(scope);
}

2
src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
View file

@ -970,7 +970,7 @@ parse_route_line(const char *line,
[PARSE_LINE_ATTR_ROUTE_WEIGHT] =
{
.key = NM_IP_ROUTE_ATTRIBUTE_WEIGHT,
.type = PARSE_LINE_TYPE_UINT8,
.type = PARSE_LINE_TYPE_UINT32,
.disabled = PARSE_LINE_AF_FLAG_FOR_IPV6,
},
[PARSE_LINE_ATTR_ROUTE_SCOPE] =

7
src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
View file

@ -2360,15 +2360,14 @@ get_route_attributes_string(NMIPRoute *route, int family)
/* we also have a corresponding attribute with the numeric value. The
* lock setting is handled above. */
}
} else if (NM_IN_STRSET(names[i],
NM_IP_ROUTE_ATTRIBUTE_SCOPE,
NM_IP_ROUTE_ATTRIBUTE_WEIGHT)) {
} else if (NM_IN_STRSET(names[i], NM_IP_ROUTE_ATTRIBUTE_SCOPE)) {
g_string_append_printf(str, "%s %u", names[i], (unsigned) g_variant_get_byte(attr));
} else if (nm_streq(names[i], NM_IP_ROUTE_ATTRIBUTE_TOS)) {
g_string_append_printf(str, "%s 0x%02x", names[i], (unsigned) g_variant_get_byte(attr));
} else if (NM_IN_STRSET(names[i],
NM_IP_ROUTE_ATTRIBUTE_RTO_MIN,
NM_IP_ROUTE_ATTRIBUTE_TABLE,
NM_IP_ROUTE_ATTRIBUTE_RTO_MIN)) {
NM_IP_ROUTE_ATTRIBUTE_WEIGHT)) {
g_string_append_printf(str, "%s %u", names[i], (unsigned) g_variant_get_uint32(attr));
} else if (nm_streq(names[i], NM_IP_ROUTE_ATTRIBUTE_QUICKACK)) {
g_string_append_printf(str, "%s %u", names[i], (unsigned) g_variant_get_boolean(attr));

4
src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
View file

@ -1365,7 +1365,7 @@ test_read_wired_static_routes(void)
nmtst_assert_route_attribute_byte(ip4_route, NM_IP_ROUTE_ATTRIBUTE_TOS, 0x28);
nmtst_assert_route_attribute_uint32(ip4_route, NM_IP_ROUTE_ATTRIBUTE_WINDOW, 30000);
nmtst_assert_route_attribute_uint32(ip4_route, NM_IP_ROUTE_ATTRIBUTE_CWND, 12);
nmtst_assert_route_attribute_byte(ip4_route, NM_IP_ROUTE_ATTRIBUTE_WEIGHT, 5);
nmtst_assert_route_attribute_uint32(ip4_route, NM_IP_ROUTE_ATTRIBUTE_WEIGHT, 5);
nmtst_assert_route_attribute_uint32(ip4_route, NM_IP_ROUTE_ATTRIBUTE_INITCWND, 13);
nmtst_assert_route_attribute_uint32(ip4_route, NM_IP_ROUTE_ATTRIBUTE_INITRWND, 14);
nmtst_assert_route_attribute_uint32(ip4_route, NM_IP_ROUTE_ATTRIBUTE_MTU, 9000);
@ -4398,7 +4398,7 @@ test_write_wired_static(void)
route4 = nm_ip_route_new(AF_INET, "1.1.1.1", 24, "1.2.3.4", 99, &error);
g_assert_no_error(error);
nm_ip_route_set_attribute(route4, NM_IP_ROUTE_ATTRIBUTE_CWND, g_variant_new_uint32(100));
nm_ip_route_set_attribute(route4, NM_IP_ROUTE_ATTRIBUTE_WEIGHT, g_variant_new_byte(5));
nm_ip_route_set_attribute(route4, NM_IP_ROUTE_ATTRIBUTE_WEIGHT, g_variant_new_uint32(5));
nm_setting_ip_config_add_route(s_ip4, route4);
nm_ip_route_unref(route4);

4
src/core/settings/plugins/keyfile/tests/test-keyfile-settings.c
View file

@ -311,7 +311,7 @@ test_read_valid_wired_connection(void)
nmtst_assert_route_attribute_uint32(route, NM_IP_ROUTE_ATTRIBUTE_CWND, 10);
nmtst_assert_route_attribute_uint32(route, NM_IP_ROUTE_ATTRIBUTE_MTU, 1430);
nmtst_assert_route_attribute_byte(route, NM_IP_ROUTE_ATTRIBUTE_WEIGHT, 5);
nmtst_assert_route_attribute_uint32(route, NM_IP_ROUTE_ATTRIBUTE_WEIGHT, 5);
nmtst_assert_route_attribute_boolean(route, NM_IP_ROUTE_ATTRIBUTE_LOCK_CWND, TRUE);
nmtst_assert_route_attribute_string(route, NM_IP_ROUTE_ATTRIBUTE_SRC, "7.7.7.7");
nmtst_assert_route_attribute_string(route, NM_IP_ROUTE_ATTRIBUTE_TYPE, "unicast");
@ -496,7 +496,7 @@ test_write_wired_connection(void)
g_assert_no_error(error);
nm_ip_route_set_attribute(rt, NM_IP_ROUTE_ATTRIBUTE_CWND, g_variant_new_uint32(10));
nm_ip_route_set_attribute(rt, NM_IP_ROUTE_ATTRIBUTE_MTU, g_variant_new_uint32(1492));
nm_ip_route_set_attribute(rt, NM_IP_ROUTE_ATTRIBUTE_WEIGHT, g_variant_new_byte(5));
nm_ip_route_set_attribute(rt, NM_IP_ROUTE_ATTRIBUTE_WEIGHT, g_variant_new_uint32(5));
nm_ip_route_set_attribute(rt, NM_IP_ROUTE_ATTRIBUTE_SRC, g_variant_new_string("1.2.3.4"));
g_assert(nm_setting_ip_config_add_route(s_ip4, rt));
nm_ip_route_unref(rt);

16
src/libnm-core-impl/nm-setting-ip-config.c
View file

@ -1279,8 +1279,9 @@ static const NMVariantAttributeSpec *const ip_route_attribute_spec[] = {
.v6 = TRUE,
.type_detail = 'T', ),
NM_VARIANT_ATTRIBUTE_SPEC_DEFINE(NM_IP_ROUTE_ATTRIBUTE_WEIGHT,
G_VARIANT_TYPE_BYTE,
.v4 = TRUE, ),
G_VARIANT_TYPE_UINT32,
.v4 = TRUE,
.type_detail = 'w'),
NM_VARIANT_ATTRIBUTE_SPEC_DEFINE(NM_IP_ROUTE_ATTRIBUTE_WINDOW,
G_VARIANT_TYPE_UINT32,
.v4 = TRUE,
@ -1316,6 +1317,7 @@ _ip_route_attribute_validate(const char *name,
{
const NMVariantAttributeSpec *spec;
const char *string;
guint32 u32;
nm_assert(name);
nm_assert(value);
@ -1429,6 +1431,16 @@ _ip_route_attribute_validate(const char *name,
if (parse_data)
parse_data->scope = g_variant_get_byte(value);
break;
case 'w': /* weight */
u32 = g_variant_get_uint32(value);
if (u32 > 256) {
g_set_error_literal(error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_FAILED,
_("route weight cannot be larger than 256"));
return FALSE;
}
break;
case '\0':
break;
default:

6
src/libnm-core-impl/tests/test-general.c
View file

@ -2501,7 +2501,7 @@ test_setting_ip_route_attributes(void)
TEST_ATTR("tos", byte, 127, AF_INET, TRUE, TRUE);
TEST_ATTR("tos", string, "0x28", AF_INET, FALSE, TRUE);
TEST_ATTR("weight", byte, 100, AF_INET, TRUE, TRUE);
TEST_ATTR("weight", uint32, 100, AF_INET, TRUE, TRUE);
TEST_ATTR("weight", string, "100", AF_INET, FALSE, TRUE);
TEST_ATTR("advmss", uint32, 1400, AF_INET, TRUE, TRUE);
@ -9990,8 +9990,8 @@ test_route_attributes_parse(void)
variant = g_hash_table_lookup(ht, NM_IP_ROUTE_ATTRIBUTE_WEIGHT);
g_assert(variant);
g_assert(g_variant_is_of_type(variant, G_VARIANT_TYPE_BYTE));
g_assert_cmpuint(g_variant_get_byte(variant), ==, 5);
g_assert(g_variant_is_of_type(variant, G_VARIANT_TYPE_UINT32));
g_assert_cmpuint(g_variant_get_uint32(variant), ==, 5);
variant = g_hash_table_lookup(ht, NM_IP_ROUTE_ATTRIBUTE_SRC);
g_assert(variant);

11
src/libnm-platform/nm-linux-platform.c
View file

@ -3847,7 +3847,7 @@ _new_from_nl_route(const struct nlmsghdr *nlh, gboolean id_only, ParseNlmsgIter
nm_assert(v4_n_nexthops - 1u < v4_nh_extra_alloc);
new_nexthop = &v4_nh_extra_nexthops[v4_n_nexthops - 1u];
new_nexthop->ifindex = rtnh->rtnh_ifindex;
new_nexthop->weight = NM_MAX(rtnh->rtnh_hops, 1u);
new_nexthop->weight = NM_MAX(((guint) rtnh->rtnh_hops) + 1u, 1u);
if (rtnh->rtnh_len > sizeof(*rtnh)) {
struct nlattr *ntb[RTA_MAX + 1];
@ -3864,7 +3864,7 @@ _new_from_nl_route(const struct nlmsghdr *nlh, gboolean id_only, ParseNlmsgIter
} else if (IS_IPv4 || idx == multihop_idx) {
nh.found = TRUE;
nh.ifindex = rtnh->rtnh_ifindex;
nh.weight = NM_MAX(rtnh->rtnh_hops, 1u);
nh.weight = NM_MAX(((guint) rtnh->rtnh_hops) + 1u, 1u);
if (rtnh->rtnh_len > sizeof(*rtnh)) {
struct nlattr *ntb[RTA_MAX + 1];
@ -5398,14 +5398,17 @@ _nl_msg_new_route(uint16_t nlmsg_type, uint16_t nlmsg_flags, const NMPObject *ob
if (!rtnh)
goto nla_put_failure;
/* For multihop routes, a valid weight must be in range 1-256 (on netlink's
* "rtnh_hops" this is 0-255). We allow that the caller leaves the value unset
* at zero. */
if (i == 0u) {
rtnh->rtnh_hops = NM_MAX(obj->ip4_route.weight, 1u);
rtnh->rtnh_hops = NM_MAX(obj->ip4_route.weight, 1u) - 1u;
rtnh->rtnh_ifindex = obj->ip4_route.ifindex;
NLA_PUT_U32(msg, RTA_GATEWAY, obj->ip4_route.gateway);
} else {
const NMPlatformIP4RtNextHop *n = &obj->_ip4_route.extra_nexthops[i - 1u];
rtnh->rtnh_hops = NM_MAX(n->weight, 1u);
rtnh->rtnh_hops = NM_MAX(n->weight, 1u) - 1u;
rtnh->rtnh_ifindex = n->ifindex;
NLA_PUT_U32(msg, RTA_GATEWAY, n->gateway);
}

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

@ -6988,16 +6988,13 @@ nm_platform_ip4_route_to_string_full(const NMPlatformIP4Route *route,
nm_strbuf_append(&buf,
&len,
" nexthop"
"%s%s" /* gateway */
"%s%s" /* weight */
"%s" /* dev/ifindex */
"%s%s" /* gateway */
" weight %s" /* weight */
"%s" /* dev/ifindex */
"",
s_gateway[0] ? " via " : "",
s_gateway,
NM_PRINT_FMT_QUOTED2(route->weight != 1,
" weight ",
nm_sprintf_buf(weight_str, "%u", route->weight),
""),
nm_sprintf_buf(weight_str, "%u", route->weight),
_to_string_dev(str_dev, route->ifindex));
if (!extra_nexthops)
nm_strbuf_append_str(&buf, &len, " nexthops [...]");
@ -7011,19 +7008,16 @@ nm_platform_ip4_route_to_string_full(const NMPlatformIP4Route *route,
&buf,
&len,
" nexthop"
"%s" /* ifindex */
"%s%s" /* gateway */
"%s%s" /* weight */
"%s" /* ifindex */
"%s%s" /* gateway */
" weight %s" /* weight */
"",
NM_PRINT_FMT_QUOTED2(nexthop->gateway != 0 || nexthop->ifindex <= 0,
" via ",
nm_inet4_ntop(nexthop->gateway, s_gateway),
""),
_to_string_dev(str_dev, nexthop->ifindex),
NM_PRINT_FMT_QUOTED2(nexthop->weight != 1,
" weight ",
nm_sprintf_buf(weight_str, "%u", nexthop->weight),
""));
nm_sprintf_buf(weight_str, "%u", nexthop->weight));
}
}
}

41
src/libnm-platform/nm-platform.h
View file

@ -410,6 +410,20 @@ struct _NMPlatformIP4Route {
* pref_src must match, unless set to 0.0.0.0 to match any. */
in_addr_t pref_src;
/* This is the weight of for the first next-hop, in case of n_nexthops > 1.
*
* If n_nexthops is zero, this value is undefined (should be zero).
* If n_nexthops is 1, this also doesn't matter, but it's usually set to
* zero.
* If n_nexthops is greater or equal to one, this is the weight of
* the first hop.
*
* Note that upper layers (nm_utils_ip_route_attribute_to_platform()) use this flag to indicate
* whether this is a multihop route. Single-hop, non-ECMP routes will have a weight of zero.
*
* The valid range for weight in kernel is 1-256. */
guint16 weight;
/* rtm_tos (iproute2: tos)
*
* For IPv4, tos is part of the weak-id (like metric).
@ -429,21 +443,6 @@ struct _NMPlatformIP4Route {
* For IPv6 routes, the scope is ignored and kernel always assumes global scope.
* Hence, this field is only in NMPlatformIP4Route. */
guint8 scope_inv;
/* This is the weight of for the first next-hop, in case of n_nexthops > 1.
*
* If n_nexthops is zero, this value is undefined (should be zero).
* If n_nexthops is 1, this also doesn't matter, but it's usually set to
* zero.
* If n_nexthops is greater or equal to one, this is the weight of
* the first hop.
*
* Note that upper layers use this flag to indicate whether this is a multihop route.
* Single-hop, non-ECMP routes will have a weight of zero.
*
* The valid range for weight is 1-255. For convenience, we treat 0 the same
* as 1 for multihop routes. */
guint8 weight;
} _nm_alignas(NMPlatformObject);
struct _NMPlatformIP6Route {
@ -671,9 +670,15 @@ typedef struct {
typedef struct {
int ifindex;
in_addr_t gateway;
/* The valid range for weight is 1-255. For convenience, we treat 0 the same
* as 1 for multihop routes. */
guint8 weight;
/* The valid range for weight is 1-256. Single hop routes in kernel
* don't have a weight, we assign them weight zero (to indicate the
* weight is missing).
*
* Upper layers (nm_utils_ip_route_attribute_to_platform()) care about
* the distinction of unset weight (no-ECMP). They express no-ECMP as
* zero.
*/
guint16 weight;
} NMPlatformIP4RtNextHop;
typedef struct {