diff --git a/src/core/NetworkManagerUtils.c b/src/core/NetworkManagerUtils.c index 21f8d19d44..f3c032ce9f 100644 --- a/src/core/NetworkManagerUtils.c +++ b/src/core/NetworkManagerUtils.c @@ -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); } diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index 2655cd9247..0eeead8af5 100644 --- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -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] = diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c index 5f0b8a6f7a..2ea097fd98 100644 --- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -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)); diff --git a/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 7d8cf0ebad..40ff7c670e 100644 --- a/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -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); diff --git a/src/core/settings/plugins/keyfile/tests/test-keyfile-settings.c b/src/core/settings/plugins/keyfile/tests/test-keyfile-settings.c index 2d4adfbcfe..83019babb1 100644 --- a/src/core/settings/plugins/keyfile/tests/test-keyfile-settings.c +++ b/src/core/settings/plugins/keyfile/tests/test-keyfile-settings.c @@ -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); diff --git a/src/libnm-core-impl/nm-setting-ip-config.c b/src/libnm-core-impl/nm-setting-ip-config.c index 7d38e16d95..bea0ee7f11 100644 --- a/src/libnm-core-impl/nm-setting-ip-config.c +++ b/src/libnm-core-impl/nm-setting-ip-config.c @@ -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: diff --git a/src/libnm-core-impl/tests/test-general.c b/src/libnm-core-impl/tests/test-general.c index 35882f7ed4..f6684bfbc8 100644 --- a/src/libnm-core-impl/tests/test-general.c +++ b/src/libnm-core-impl/tests/test-general.c @@ -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); diff --git a/src/libnm-platform/nm-linux-platform.c b/src/libnm-platform/nm-linux-platform.c index ae074dac51..6d53d99849 100644 --- a/src/libnm-platform/nm-linux-platform.c +++ b/src/libnm-platform/nm-linux-platform.c @@ -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); } diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c index f201f2f9c7..85bd785ac8 100644 --- a/src/libnm-platform/nm-platform.c +++ b/src/libnm-platform/nm-platform.c @@ -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)); } } } diff --git a/src/libnm-platform/nm-platform.h b/src/libnm-platform/nm-platform.h index 71ac6ccc3b..ea46321534 100644 --- a/src/libnm-platform/nm-platform.h +++ b/src/libnm-platform/nm-platform.h @@ -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 {