diff --git a/src/core/nm-l3-config-data.c b/src/core/nm-l3-config-data.c index 347332933a..77ced35dc4 100644 --- a/src/core/nm-l3-config-data.c +++ b/src/core/nm-l3-config-data.c @@ -2512,7 +2512,7 @@ nm_l3_config_data_get_blacklisted_ip4_routes(const NML3ConfigData *self, gboolea .metric = NM_PLATFORM_ROUTE_METRIC_IP4_DEVICE_ROUTE, .scope_inv = nm_platform_route_scope_inv(NM_RT_SCOPE_LINK), }; - nm_platform_ip_route_normalize(AF_INET, &rx.rx); + nm_platform_ip_route_normalize(AF_INET, &rx.rx, NMP_IP_ROUTE_NORMALIZE_FLAGS_NONE); if (nm_l3_config_data_lookup_route(self, AF_INET, &rx.rx)) { /* we track such a route explicitly. Don't blacklist it. */ @@ -2693,7 +2693,9 @@ nm_l3_config_data_add_dependent_device_routes(NML3ConfigData *self, .plen = plen, }; - nm_platform_ip_route_normalize(addr_family, &rx.rx); + nm_platform_ip_route_normalize(addr_family, + &rx.rx, + NMP_IP_ROUTE_NORMALIZE_FLAGS_NONE); nm_l3_config_data_add_route(self, addr_family, NULL, &rx.rx); } } diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index 752febfaaf..0a550c5e5d 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -3911,7 +3911,7 @@ _l3cfg_update_combined_config(NML3Cfg *self, .type_coerced = nm_platform_route_type_coerce(RTN_LOCAL), .pref_src = NM_IPV4LO_ADDR1, }; - nm_platform_ip_route_normalize(AF_INET, &rx.rx); + nm_platform_ip_route_normalize(AF_INET, &rx.rx, NMP_IP_ROUTE_NORMALIZE_FLAGS_NONE); if (!nm_l3_config_data_lookup_route(l3cd, AF_INET, &rx.rx)) { nm_l3_config_data_add_route_4(l3cd, &rx.r4); } diff --git a/src/core/platform/tests/test-route.c b/src/core/platform/tests/test-route.c index 9aa21a9ac6..a6d3f98cd2 100644 --- a/src/core/platform/tests/test-route.c +++ b/src/core/platform/tests/test-route.c @@ -719,7 +719,9 @@ test_ip4_route_options(gconstpointer test_data) for (i = 0; i < rts_n; i++) { rts_cmp[i] = rts_add[i]; - nm_platform_ip_route_normalize(AF_INET, NM_PLATFORM_IP_ROUTE_CAST(&rts_cmp[i])); + nm_platform_ip_route_normalize(AF_INET, + NM_PLATFORM_IP_ROUTE_CAST(&rts_cmp[i]), + NMP_IP_ROUTE_NORMALIZE_FLAGS_NONE); } routes = nmtstp_ip4_route_get_all(NM_PLATFORM_GET, IFINDEX); @@ -891,7 +893,9 @@ test_ip6_route_options(gconstpointer test_data) for (i = 0; i < rts_n; i++) { rts_cmp[i] = rts_add[i]; - nm_platform_ip_route_normalize(AF_INET6, NM_PLATFORM_IP_ROUTE_CAST(&rts_cmp[i])); + nm_platform_ip_route_normalize(AF_INET6, + NM_PLATFORM_IP_ROUTE_CAST(&rts_cmp[i]), + NMP_IP_ROUTE_NORMALIZE_FLAGS_NONE); } routes = nmtstp_ip6_route_get_all(NM_PLATFORM_GET, IFINDEX); @@ -1936,7 +1940,7 @@ test_blackhole(gconstpointer test_data) }; } - nm_platform_ip_route_normalize(addr_family, &rr.rx); + nm_platform_ip_route_normalize(addr_family, &rr.rx, NMP_IP_ROUTE_NORMALIZE_FLAGS_NONE); if (IS_IPv4) r = nm_platform_ip4_route_add(NM_PLATFORM_GET, NMP_NLM_FLAG_APPEND, &rr.r4, NULL); diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c index a4751113da..5fa191b958 100644 --- a/src/libnm-platform/nm-platform.c +++ b/src/libnm-platform/nm-platform.c @@ -5274,10 +5274,38 @@ _route_pref_normalize(guint8 pref) : NM_ICMPV6_ROUTER_PREF_MEDIUM; } +static guint16 +_ip4_route_weight_normalize(guint n_nexthops, guint16 weight, gboolean keep_ecmp_weight) +{ + if (n_nexthops > 1u) { + /* This is a multihop-route. The weight is relevant. + * + * We only normalize a zero to one (because in kernel such weights + * don't exist. */ + return NM_MAX(weight, 1u); + } + if (n_nexthops == 0) { + /* This route has no next-hop (e.g. blackhole type). The weight is + * always irrelevant. Normalize to zero. */ + return 0; + } + + /* We have a IPv4 single-hop route. In kernel, the weight does not exist. + * It's always zero. + * + * For upper layers, we find it useful to track such routes with a positive + * weight. They are candidates to be merged into a multi-hop ECMP route. + * + * Depending on what the caller requests, we normalize it (or leave it + * unchanged). */ + return keep_ecmp_weight ? weight : 0u; +} + /** * nm_platform_ip_route_normalize: * @addr_family: AF_INET or AF_INET6 * @route: an NMPlatformIP4Route or NMPlatformIP6Route instance, depending on @addr_family. + * @flags: flag affecting normalization. * * Adding a route to kernel via nm_platform_ip_route_add() will normalize/coerce some * properties of the route. This function modifies (normalizes) the route like it @@ -5286,9 +5314,18 @@ _route_pref_normalize(guint8 pref) * Note that this function is related to NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY * in that if two routes compare semantically equal, after normalizing they also shall * compare equal with NM_PLATFORM_IP_ROUTE_CMP_TYPE_FULL. + * + * Note that a positive "weight" of IPv4 single hop routes is not meaningful in + * kernel. While we track such routes at upper layers, they don't exist in + * kernel (well, they exist, with their weight set to zero, which makes them a + * different route according to NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID. The + * NMP_IP_ROUTE_NORMALIZE_FLAGS_KEEP_ECMP_WEIGHT flag can be used to prevent + * such normalization. */ void -nm_platform_ip_route_normalize(int addr_family, NMPlatformIPRoute *route) +nm_platform_ip_route_normalize(int addr_family, + NMPlatformIPRoute *route, + NMPIPRouteNormalizeFlags flags) { NMPlatformIP4Route *r4; NMPlatformIP6Route *r6; @@ -5306,6 +5343,11 @@ nm_platform_ip_route_normalize(int addr_family, NMPlatformIPRoute *route) route->metric_any = FALSE; r4->network = nm_ip4_addr_clear_host_address(r4->network, r4->plen); r4->scope_inv = _ip_route_scope_inv_get_normalized(r4); + r4->n_nexthops = nm_platform_ip4_route_get_n_nexthops(r4); + r4->weight = _ip4_route_weight_normalize( + r4->n_nexthops, + r4->weight, + NM_FLAGS_HAS(flags, NMP_IP_ROUTE_NORMALIZE_FLAGS_KEEP_ECMP_WEIGHT)); break; case AF_INET6: r6 = (NMPlatformIP6Route *) route; @@ -5343,7 +5385,8 @@ _ip_route_add(NMPlatform *self, NMPNlmFlags flags, NMPObject *obj_stack, char ** || obj_stack->ip4_route.n_nexthops <= 1u || obj_stack->_ip4_route.extra_nexthops); nm_platform_ip_route_normalize(NMP_OBJECT_GET_ADDR_FAMILY((obj_stack)), - NMP_OBJECT_CAST_IP_ROUTE(obj_stack)); + NMP_OBJECT_CAST_IP_ROUTE(obj_stack), + NMP_IP_ROUTE_NORMALIZE_FLAGS_NONE); ifindex = obj_stack->ip_route.ifindex; @@ -7039,7 +7082,7 @@ nm_platform_ip4_route_to_string_full(const NMPlatformIP4Route *route, route->plen, n_nexthops <= 1 && s_gateway[0] ? " via " : "", n_nexthops <= 1 ? s_gateway : "", - NM_PRINT_FMT_QUOTED2(n_nexthops <= 1 && route->weight != 0, + NM_PRINT_FMT_QUOTED2(n_nexthops <= 1 && route->weight != 0u, " weight ", nm_sprintf_buf(weight_str, "%u", route->weight), ""), @@ -8596,12 +8639,11 @@ nm_platform_ip4_rt_nexthop_hash_update(const NMPlatformIP4RtNextHop *obj, gboolean for_id, NMHashState *h) { - guint8 w; + guint16 w; nm_assert(obj); w = for_id ? NM_MAX(obj->weight, 1u) : obj->weight; - nm_hash_update_vals(h, obj->ifindex, obj->gateway, w); } @@ -8610,6 +8652,8 @@ nm_platform_ip4_route_hash_update(const NMPlatformIP4Route *obj, NMPlatformIPRouteCmpType cmp_type, NMHashState *h) { + guint n_nexthops; + switch (cmp_type) { case NM_PLATFORM_IP_ROUTE_CMP_TYPE_WEAK_ID: case NM_PLATFORM_IP_ROUTE_CMP_TYPE_ECMP_ID: @@ -8647,15 +8691,17 @@ nm_platform_ip4_route_hash_update(const NMPlatformIP4Route *obj, obj->lock_mtu, obj->lock_mss)); if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID) { + n_nexthops = nm_platform_ip4_route_get_n_nexthops(obj); nm_hash_update_vals(h, obj->ifindex, - nm_platform_ip4_route_get_n_nexthops(obj), + n_nexthops, obj->gateway, - (guint8) NM_MAX(obj->weight, 1u)); + _ip4_route_weight_normalize(n_nexthops, obj->weight, TRUE)); } } break; case NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY: + n_nexthops = nm_platform_ip4_route_get_n_nexthops(obj); nm_hash_update_vals( h, obj->type_coerced, @@ -8664,9 +8710,9 @@ nm_platform_ip4_route_hash_update(const NMPlatformIP4Route *obj, nm_ip4_addr_clear_host_address(obj->network, obj->plen), obj->plen, obj->metric, - nm_platform_ip4_route_get_n_nexthops(obj), + n_nexthops, obj->gateway, - (guint8) NM_MAX(obj->weight, 1u), + _ip4_route_weight_normalize(n_nexthops, obj->weight, TRUE), nmp_utils_ip_config_source_round_trip_rtprot(obj->rt_source), _ip_route_scope_inv_get_normalized(obj), obj->tos, @@ -8732,14 +8778,9 @@ nm_platform_ip4_rt_nexthop_cmp(const NMPlatformIP4RtNextHop *a, const NMPlatformIP4RtNextHop *b, gboolean for_id) { - guint8 w_a; - guint8 w_b; + guint16 w_a; + guint16 w_b; - /* Note that weight zero is not valid (in kernel). We thus treat - * weight zero usually the same as 1. - * - * Not here for cmp/hash_update functions. These functions check for the exact - * bit-pattern, and not the it means at other places. */ NM_CMP_SELF(a, b); NM_CMP_FIELD(a, b, ifindex); NM_CMP_FIELD(a, b, gateway); @@ -8756,6 +8797,8 @@ nm_platform_ip4_route_cmp(const NMPlatformIP4Route *a, const NMPlatformIP4Route *b, NMPlatformIPRouteCmpType cmp_type) { + guint n_nexthops; + NM_CMP_SELF(a, b); switch (cmp_type) { case NM_PLATFORM_IP_ROUTE_CMP_TYPE_ECMP_ID: @@ -8802,9 +8845,10 @@ nm_platform_ip4_route_cmp(const NMPlatformIP4Route *a, if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID) { NM_CMP_FIELD(a, b, ifindex); NM_CMP_FIELD(a, b, gateway); - NM_CMP_DIRECT(NM_MAX(a->weight, 1u), NM_MAX(b->weight, 1u)); - NM_CMP_DIRECT(nm_platform_ip4_route_get_n_nexthops(a), - nm_platform_ip4_route_get_n_nexthops(b)); + n_nexthops = nm_platform_ip4_route_get_n_nexthops(a); + NM_CMP_DIRECT(n_nexthops, nm_platform_ip4_route_get_n_nexthops(b)); + NM_CMP_DIRECT(_ip4_route_weight_normalize(n_nexthops, a->weight, TRUE), + _ip4_route_weight_normalize(n_nexthops, b->weight, TRUE)); } } break; @@ -8825,16 +8869,16 @@ nm_platform_ip4_route_cmp(const NMPlatformIP4Route *a, NM_CMP_FIELD(a, b, plen); NM_CMP_FIELD_UNSAFE(a, b, metric_any); NM_CMP_FIELD(a, b, metric); - if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY) { - NM_CMP_DIRECT(nm_platform_ip4_route_get_n_nexthops(a), - nm_platform_ip4_route_get_n_nexthops(b)); - } else - NM_CMP_FIELD(a, b, n_nexthops); NM_CMP_FIELD(a, b, gateway); - if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY) - NM_CMP_DIRECT(NM_MAX(a->weight, 1u), NM_MAX(b->weight, 1u)); - else + if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY) { + n_nexthops = nm_platform_ip4_route_get_n_nexthops(a); + NM_CMP_DIRECT(n_nexthops, nm_platform_ip4_route_get_n_nexthops(b)); + NM_CMP_DIRECT(_ip4_route_weight_normalize(n_nexthops, a->weight, TRUE), + _ip4_route_weight_normalize(n_nexthops, b->weight, TRUE)); + } else { + NM_CMP_FIELD(a, b, n_nexthops); NM_CMP_FIELD(a, b, weight); + } if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY) { NM_CMP_DIRECT(nmp_utils_ip_config_source_round_trip_rtprot(a->rt_source), nmp_utils_ip_config_source_round_trip_rtprot(b->rt_source)); @@ -9414,7 +9458,9 @@ nm_platform_ip4_address_generate_device_route(const NMPlatformIP4Address *addr, .scope_inv = nm_platform_route_scope_inv(NM_RT_SCOPE_LINK), }; - nm_platform_ip_route_normalize(AF_INET, (NMPlatformIPRoute *) dst); + nm_platform_ip_route_normalize(AF_INET, + (NMPlatformIPRoute *) dst, + NMP_IP_ROUTE_NORMALIZE_FLAGS_NONE); return dst; } diff --git a/src/libnm-platform/nm-platform.h b/src/libnm-platform/nm-platform.h index 557b4190a8..dc669855c8 100644 --- a/src/libnm-platform/nm-platform.h +++ b/src/libnm-platform/nm-platform.h @@ -444,18 +444,27 @@ 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. + /* This is the weight of for the first next-hop. * - * 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. + * For multi-hop routes (n_nexthops > 1) this is the weight of the first + * hop. Note that the valid range is from 1-256. Zero is treated the same + * as 1 (for NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID comparison). * - * 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. + * For routes without next-hop (e.g. blackhole type), the weight is + * meaningless. It should be set to zero. NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID + * will treat it as zero. * - * The valid range for weight in kernel is 1-256. */ + * For single-hop routes, in kernel they don't have a weight. That means, + * all routes in the platform cache have a weight of zero. For tracking + * purposes, we find it useful that upper layers have single-hop routes + * with a positive weight. Such routes can never exist in kernel. Trying + * to add such a route will somewhat work, because + * nm_platform_ip_route_normalize() normalizes the weight to zero + * (effectively adding another route, according to + * NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID). A lookup in the platform cache with + * such a route will not yield a result. It does not exist there. If you + * want to find such a route, normalize it first. + */ guint16 weight; /* rtm_tos (iproute2: tos) @@ -704,13 +713,11 @@ typedef struct { typedef struct { int ifindex; in_addr_t gateway; - /* 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). + + /* The weight of the next hop. The valid range for weight is 1-256. * - * Upper layers (nm_utils_ip_route_attribute_to_platform()) care about - * the distinction of unset weight (no-ECMP). They express no-ECMP as - * zero. + * Zero is allowed too, but treated as 1 (by + * NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID comparison). */ guint16 weight; @@ -2263,7 +2270,28 @@ nm_platform_ip_address_get_prune_list(NMPlatform *self, gboolean nm_platform_ip_address_flush(NMPlatform *self, int addr_family, int ifindex); -void nm_platform_ip_route_normalize(int addr_family, NMPlatformIPRoute *route); +typedef enum { + /* No flags. */ + NMP_IP_ROUTE_NORMALIZE_FLAGS_NONE = 0, + + /* Don't normalize the "weight" for IPv4 single hop routes. + * nm_platform_ip_route_normalize() aims to normalize a route as it + * can exist in kernel. But in kernel, a IPv4 single hop route cannot + * have a (non-zero) weight. Normalization would usually loose that + * information. + * + * However, it can be useful to normalize routes, but not the "weight". + * Upper layers want to track single hop routes with positive weight for + * internal purposes, while normalizing all other fields. This flag + * allows to opt-out from that normalization. The result may not ever + * exist in kernel (as far as NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID is concerned). + */ + NMP_IP_ROUTE_NORMALIZE_FLAGS_KEEP_ECMP_WEIGHT = 0x1, +} NMPIPRouteNormalizeFlags; + +void nm_platform_ip_route_normalize(int addr_family, + NMPlatformIPRoute *route, + NMPIPRouteNormalizeFlags flags); static inline guint32 nm_platform_ip4_route_get_effective_metric(const NMPlatformIP4Route *r)