From fea879cd04cfe30c5d7ee7a148daafba98325bb8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 12 Jan 2024 13:58:23 +0900 Subject: [PATCH 1/4] network/route-nexthop: introduce struct RouteNextHop and replace MultipathRoute with it It is mostly equivalent to MultipathRoute. So, no functional change, just refactoring and preparation for later commits. --- src/network/networkd-route-nexthop.c | 299 +++++++++++++++++++++------ src/network/networkd-route-nexthop.h | 24 ++- src/network/networkd-route.c | 30 +-- src/network/networkd-route.h | 4 +- 4 files changed, 271 insertions(+), 86 deletions(-) diff --git a/src/network/networkd-route-nexthop.c b/src/network/networkd-route-nexthop.c index 7947eb1b61..2a213f94a6 100644 --- a/src/network/networkd-route-nexthop.c +++ b/src/network/networkd-route-nexthop.c @@ -13,14 +13,93 @@ #include "parse-util.h" #include "string-util.h" -int multipath_route_get_link(Manager *manager, Link *link, const MultipathRoute *m, Link **ret) { - assert(manager); - assert(m); +static void route_nexthop_done(RouteNextHop *nh) { + assert(nh); - if (m->ifindex > 0) - return link_get_by_index(manager, m->ifindex, ret); - if (m->ifname) - return link_get_by_name(manager, m->ifname, ret); + free(nh->ifname); +} + +RouteNextHop* route_nexthop_free(RouteNextHop *nh) { + if (!nh) + return NULL; + + route_nexthop_done(nh); + + return mfree(nh); +} + +void route_nexthops_done(Route *route) { + assert(route); + + ordered_set_free(route->nexthops); +} + +static void route_nexthop_hash_func(const RouteNextHop *nh, struct siphash *state) { + assert(nh); + assert(state); + + /* See nh_comp() in net/ipv4/fib_semantics.c of the kernel. */ + + siphash24_compress_typesafe(nh->family, state); + if (!IN_SET(nh->family, AF_INET, AF_INET6)) + return; + + in_addr_hash_func(&nh->gw, nh->family, state); + siphash24_compress_typesafe(nh->weight, state); + siphash24_compress_typesafe(nh->ifindex, state); + if (nh->ifindex == 0) + siphash24_compress_string(nh->ifname, state); /* For Network or Request object. */ +} + +static int route_nexthop_compare_func(const RouteNextHop *a, const RouteNextHop *b) { + int r; + + assert(a); + assert(b); + + r = CMP(a->family, b->family); + if (r != 0) + return r; + + if (!IN_SET(a->family, AF_INET, AF_INET6)) + return 0; + + r = memcmp(&a->gw, &b->gw, FAMILY_ADDRESS_SIZE(a->family)); + if (r != 0) + return r; + + r = CMP(a->weight, b->weight); + if (r != 0) + return r; + + r = CMP(a->ifindex, b->ifindex); + if (r != 0) + return r; + + if (a->ifindex == 0) { + r = strcmp_ptr(a->ifname, b->ifname); + if (r != 0) + return r; + } + + return 0; +} + +DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR( + route_nexthop_hash_ops, + RouteNextHop, + route_nexthop_hash_func, + route_nexthop_compare_func, + route_nexthop_free); + +int route_nexthop_get_link(Manager *manager, Link *link, const RouteNextHop *nh, Link **ret) { + assert(manager); + assert(nh); + + if (nh->ifindex > 0) + return link_get_by_index(manager, nh->ifindex, ret); + if (nh->ifname) + return link_get_by_name(manager, nh->ifname, ret); if (link) { if (ret) @@ -31,13 +110,11 @@ int multipath_route_get_link(Manager *manager, Link *link, const MultipathRoute return -ENOENT; } -static bool multipath_route_is_ready_to_configure(const MultipathRoute *m, Link *link, bool onlink) { - union in_addr_union a = m->gateway.address; - - assert(m); +static bool route_nexthop_is_ready_to_configure(const RouteNextHop *nh, Link *link, bool onlink) { + assert(nh); assert(link); - if (multipath_route_get_link(link->manager, link, m, &link) < 0) + if (route_nexthop_get_link(link->manager, link, nh, &link)) return false; if (!link_is_ready_to_configure(link, /* allow_unmanaged = */ true)) @@ -48,7 +125,7 @@ static bool multipath_route_is_ready_to_configure(const MultipathRoute *m, Link if (!link->network && !link_has_carrier(link)) return false; - return gateway_is_ready(link, onlink, m->gateway.family, &a); + return gateway_is_ready(link, onlink, nh->family, &nh->gw); } int route_nexthops_is_ready_to_configure(const Route *route, Link *link) { @@ -79,12 +156,12 @@ int route_nexthops_is_ready_to_configure(const Route *route, Link *link) { if (route_type_is_reject(route)) return true; - if (ordered_set_isempty(route->multipath_routes)) + if (ordered_set_isempty(route->nexthops)) return gateway_is_ready(link, FLAGS_SET(route->flags, RTNH_F_ONLINK), route->gw_family, &route->gw); - MultipathRoute *m; - ORDERED_SET_FOREACH(m, route->multipath_routes) - if (!multipath_route_is_ready_to_configure(m, link, FLAGS_SET(route->flags, RTNH_F_ONLINK))) + RouteNextHop *nh; + ORDERED_SET_FOREACH(nh, route->nexthops) + if (!route_nexthop_is_ready_to_configure(nh, link, FLAGS_SET(route->flags, RTNH_F_ONLINK))) return false; return true; @@ -114,7 +191,7 @@ int route_nexthops_to_string(const Route *route, char **ret) { return 0; } - if (ordered_set_isempty(route->multipath_routes)) { + if (ordered_set_isempty(route->nexthops)) { if (in_addr_is_set(route->gw_family, &route->gw)) buf = strjoin("gw: ", IN_ADDR_TO_STRING(route->gw_family, &route->gw)); else if (route->gateway_from_dhcp_or_ra) { @@ -133,17 +210,16 @@ int route_nexthops_to_string(const Route *route, char **ret) { return 0; } - MultipathRoute *m; - ORDERED_SET_FOREACH(m, route->multipath_routes) { - union in_addr_union a = m->gateway.address; - const char *s = in_addr_is_set(m->gateway.family, &a) ? IN_ADDR_TO_STRING(m->gateway.family, &a) : NULL; + RouteNextHop *nh; + ORDERED_SET_FOREACH(nh, route->nexthops) { + const char *s = in_addr_is_set(nh->family, &nh->gw) ? IN_ADDR_TO_STRING(nh->family, &nh->gw) : NULL; - if (m->ifindex > 0) - r = strextendf_with_separator(&buf, ",", "%s@%i:%"PRIu32, strempty(s), m->ifindex, m->weight + 1); - else if (m->ifname) - r = strextendf_with_separator(&buf, ",", "%s@%s:%"PRIu32, strempty(s), m->ifname, m->weight + 1); + if (nh->ifindex > 0) + r = strextendf_with_separator(&buf, ",", "%s@%i:%"PRIu32, strempty(s), nh->ifindex, nh->weight + 1); + else if (nh->ifname) + r = strextendf_with_separator(&buf, ",", "%s@%s:%"PRIu32, strempty(s), nh->ifname, nh->weight + 1); else - r = strextendf_with_separator(&buf, ",", "%s:%"PRIu32, strempty(s), m->weight + 1); + r = strextendf_with_separator(&buf, ",", "%s:%"PRIu32, strempty(s), nh->weight + 1); if (r < 0) return r; } @@ -156,22 +232,22 @@ int route_nexthops_to_string(const Route *route, char **ret) { return 0; } -static int append_nexthop_one(Link *link, const Route *route, const MultipathRoute *m, struct rtattr **rta, size_t offset) { +static int append_nexthop_one(Link *link, const Route *route, const RouteNextHop *nh, struct rtattr **rta, size_t offset) { struct rtnexthop *rtnh; struct rtattr *new_rta; int r; assert(route); assert(IN_SET(route->family, AF_INET, AF_INET6)); - assert(m); + assert(nh); assert(rta); assert(*rta); - if (m->ifindex <= 0) { + if (nh->ifindex <= 0) { assert(link); assert(link->manager); - r = multipath_route_get_link(link->manager, link, m, &link); + r = route_nexthop_get_link(link->manager, link, nh, &link); if (r < 0) return r; } @@ -184,31 +260,35 @@ static int append_nexthop_one(Link *link, const Route *route, const MultipathRou rtnh = (struct rtnexthop *)((uint8_t *) *rta + offset); *rtnh = (struct rtnexthop) { .rtnh_len = sizeof(*rtnh), - .rtnh_ifindex = m->ifindex > 0 ? m->ifindex : link->ifindex, - .rtnh_hops = m->weight, + .rtnh_ifindex = nh->ifindex > 0 ? nh->ifindex : link->ifindex, + .rtnh_hops = nh->weight, }; (*rta)->rta_len += sizeof(struct rtnexthop); - if (m->gateway.family == route->family) { - r = rtattr_append_attribute(rta, RTA_GATEWAY, &m->gateway.address, FAMILY_ADDRESS_SIZE(m->gateway.family)); + if (nh->family == route->family) { + r = rtattr_append_attribute(rta, RTA_GATEWAY, &nh->gw, FAMILY_ADDRESS_SIZE(nh->family)); if (r < 0) goto clear; rtnh = (struct rtnexthop *)((uint8_t *) *rta + offset); - rtnh->rtnh_len += RTA_SPACE(FAMILY_ADDRESS_SIZE(m->gateway.family)); + rtnh->rtnh_len += RTA_SPACE(FAMILY_ADDRESS_SIZE(nh->family)); - } else if (m->gateway.family == AF_INET6) { + } else if (nh->family == AF_INET6) { assert(route->family == AF_INET); - r = rtattr_append_attribute(rta, RTA_VIA, &m->gateway, sizeof(RouteVia)); + r = rtattr_append_attribute(rta, RTA_VIA, + &(RouteVia) { + .family = nh->family, + .address = nh->gw, + }, sizeof(RouteVia)); if (r < 0) goto clear; rtnh = (struct rtnexthop *)((uint8_t *) *rta + offset); rtnh->rtnh_len += RTA_SPACE(sizeof(RouteVia)); - } else if (m->gateway.family == AF_INET) + } else if (nh->family == AF_INET) assert_not_reached(); return 0; @@ -226,7 +306,7 @@ static int netlink_message_append_multipath_route(Link *link, const Route *route assert(route); assert(message); - if (ordered_set_isempty(route->multipath_routes)) + if (ordered_set_isempty(route->nexthops)) return 0; rta = new(struct rtattr, 1); @@ -239,11 +319,11 @@ static int netlink_message_append_multipath_route(Link *link, const Route *route }; offset = (uint8_t *) RTA_DATA(rta) - (uint8_t *) rta; - MultipathRoute *m; - ORDERED_SET_FOREACH(m, route->multipath_routes) { + RouteNextHop *nh; + ORDERED_SET_FOREACH(nh, route->nexthops) { struct rtnexthop *rtnh; - r = append_nexthop_one(link, route, m, &rta, offset); + r = append_nexthop_one(link, route, nh, &rta, offset); if (r < 0) return r; @@ -267,7 +347,7 @@ int route_nexthops_set_netlink_message(Link *link, const Route *route, sd_netlin if (route_type_is_reject(route)) return 0; - if (ordered_set_isempty(route->multipath_routes)) { + if (ordered_set_isempty(route->nexthops)) { if (in_addr_is_set(route->gw_family, &route->gw)) { if (route->gw_family == route->family) @@ -291,6 +371,92 @@ int route_nexthops_set_netlink_message(Link *link, const Route *route, sd_netlin return netlink_message_append_multipath_route(link, route, message); } +static int route_parse_nexthops(Route *route, const struct rtnexthop *rtnh, size_t size) { + _cleanup_ordered_set_free_ OrderedSet *nexthops = NULL; + int r; + + assert(route); + assert(IN_SET(route->family, AF_INET, AF_INET6)); + assert(rtnh); + + if (size < sizeof(struct rtnexthop)) + return -EBADMSG; + + for (; size >= sizeof(struct rtnexthop); ) { + _cleanup_(route_nexthop_freep) RouteNextHop *nh = NULL; + + if (NLMSG_ALIGN(rtnh->rtnh_len) > size) + return -EBADMSG; + + if (rtnh->rtnh_len < sizeof(struct rtnexthop)) + return -EBADMSG; + + nh = new(RouteNextHop, 1); + if (!nh) + return -ENOMEM; + + *nh = (RouteNextHop) { + .ifindex = rtnh->rtnh_ifindex, + .weight = rtnh->rtnh_hops, + }; + + if (rtnh->rtnh_len > sizeof(struct rtnexthop)) { + size_t len = rtnh->rtnh_len - sizeof(struct rtnexthop); + bool have_gw = false; + + for (struct rtattr *attr = RTNH_DATA(rtnh); RTA_OK(attr, len); attr = RTA_NEXT(attr, len)) { + + switch (attr->rta_type) { + case RTA_GATEWAY: + if (have_gw) + return -EBADMSG; + + if (attr->rta_len != RTA_LENGTH(FAMILY_ADDRESS_SIZE(route->family))) + return -EBADMSG; + + nh->family = route->family; + memcpy(&nh->gw, RTA_DATA(attr), FAMILY_ADDRESS_SIZE(nh->family)); + have_gw = true; + break; + + case RTA_VIA: + if (have_gw) + return -EBADMSG; + + if (route->family != AF_INET) + return -EBADMSG; + + if (attr->rta_len != RTA_LENGTH(sizeof(RouteVia))) + return -EBADMSG; + + RouteVia *via = RTA_DATA(attr); + if (via->family != AF_INET6) + return -EBADMSG; + + nh->family = via->family; + nh->gw = via->address; + have_gw = true; + break; + } + } + } + + r = ordered_set_ensure_put(&nexthops, &route_nexthop_hash_ops, nh); + assert(r != 0); + if (r > 0) + TAKE_PTR(nh); + else if (r != -EEXIST) + return r; + + size -= NLMSG_ALIGN(rtnh->rtnh_len); + rtnh = RTNH_NEXT(rtnh); + } + + ordered_set_free(route->nexthops); + route->nexthops = TAKE_PTR(nexthops); + return 0; +} + int route_nexthops_read_netlink_message(Route *route, sd_netlink_message *message) { int r; @@ -345,7 +511,7 @@ int route_nexthops_read_netlink_message(Route *route, sd_netlink_message *messag if (r < 0) return log_warning_errno(r, "rtnl: failed to read RTA_MULTIPATH attribute, ignoring: %m"); - r = rtattr_read_nexthop(rta, rta_len, route->family, &route->multipath_routes); + r = route_parse_nexthops(route, rta, rta_len); if (r < 0) return log_warning_errno(r, "rtnl: failed to parse RTA_MULTIPATH attribute, ignoring: %m"); @@ -430,9 +596,9 @@ int route_section_verify_nexthops(Route *route) { "Ignoring [Route] section from line %u.", route->section->filename, route->section->line); - MultipathRoute *m; - ORDERED_SET_FOREACH(m, route->multipath_routes) - if (m->gateway.family == AF_INET) + RouteNextHop *nh; + ORDERED_SET_FOREACH(nh, route->nexthops) + if (nh->family == AF_INET) return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "%s: IPv4 multipath route is specified for IPv6 route. " "Ignoring [Route] section from line %u.", @@ -442,7 +608,7 @@ int route_section_verify_nexthops(Route *route) { if (route->nexthop_id != 0 && (route->gateway_from_dhcp_or_ra || in_addr_is_set(route->gw_family, &route->gw) || - !ordered_set_isempty(route->multipath_routes))) + !ordered_set_isempty(route->nexthops))) return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "%s: NextHopId= cannot be specified with Gateway= or MultiPathRoute=. " "Ignoring [Route] section from line %u.", @@ -451,7 +617,7 @@ int route_section_verify_nexthops(Route *route) { if (route_type_is_reject(route) && (route->gateway_from_dhcp_or_ra || in_addr_is_set(route->gw_family, &route->gw) || - !ordered_set_isempty(route->multipath_routes))) + !ordered_set_isempty(route->nexthops))) return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "%s: reject type route cannot be specified with Gateway= or MultiPathRoute=. " "Ignoring [Route] section from line %u.", @@ -459,7 +625,7 @@ int route_section_verify_nexthops(Route *route) { if ((route->gateway_from_dhcp_or_ra || in_addr_is_set(route->gw_family, &route->gw)) && - !ordered_set_isempty(route->multipath_routes)) + !ordered_set_isempty(route->nexthops)) return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "%s: Gateway= cannot be specified with MultiPathRoute=. " "Ignoring [Route] section from line %u.", @@ -660,14 +826,13 @@ int config_parse_multipath_route( void *data, void *userdata) { - _cleanup_(multipath_route_freep) MultipathRoute *m = NULL; + _cleanup_(route_nexthop_freep) RouteNextHop *nh = NULL; _cleanup_(route_free_or_set_invalidp) Route *route = NULL; _cleanup_free_ char *word = NULL; Network *network = userdata; - union in_addr_union a; - int family, r; const char *p; char *dev; + int r; assert(filename); assert(section); @@ -685,13 +850,13 @@ int config_parse_multipath_route( } if (isempty(rvalue)) { - route->multipath_routes = ordered_set_free_with_destructor(route->multipath_routes, multipath_route_free); + route->nexthops = ordered_set_free(route->nexthops); TAKE_PTR(route); return 0; } - m = new0(MultipathRoute, 1); - if (!m) + nh = new0(RouteNextHop, 1); + if (!nh) return log_oom(); p = rvalue; @@ -710,7 +875,7 @@ int config_parse_multipath_route( r = parse_ifindex(dev); if (r > 0) - m->ifindex = r; + nh->ifindex = r; else { if (!ifname_valid_full(dev, IFNAME_VALID_ALTERNATIVE)) { log_syntax(unit, LOG_WARNING, filename, line, 0, @@ -718,23 +883,21 @@ int config_parse_multipath_route( return 0; } - m->ifname = strdup(dev); - if (!m->ifname) + nh->ifname = strdup(dev); + if (!nh->ifname) return log_oom(); } } - r = in_addr_from_string_auto(word, &family, &a); + r = in_addr_from_string_auto(word, &nh->family, &nh->gw); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Invalid multipath route gateway '%s', ignoring assignment: %m", rvalue); return 0; } - m->gateway.address = a; - m->gateway.family = family; if (!isempty(p)) { - r = safe_atou32(p, &m->weight); + r = safe_atou32(p, &nh->weight); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Invalid multipath route weight, ignoring assignment: %s", p); @@ -744,15 +907,15 @@ int config_parse_multipath_route( * range 0…254. MultiPathRoute= setting also takes weight in the same range which ip * command uses, then networkd decreases by one and stores it to match the range which * kernel uses. */ - if (m->weight == 0 || m->weight > 256) { + if (nh->weight == 0 || nh->weight > 256) { log_syntax(unit, LOG_WARNING, filename, line, 0, "Invalid multipath route weight, ignoring assignment: %s", p); return 0; } - m->weight--; + nh->weight--; } - r = ordered_set_ensure_put(&route->multipath_routes, NULL, m); + r = ordered_set_ensure_put(&route->nexthops, &route_nexthop_hash_ops, nh); if (r == -ENOMEM) return log_oom(); if (r < 0) { @@ -761,7 +924,7 @@ int config_parse_multipath_route( return 0; } - TAKE_PTR(m); + TAKE_PTR(nh); TAKE_PTR(route); return 0; } diff --git a/src/network/networkd-route-nexthop.h b/src/network/networkd-route-nexthop.h index af832d5d9c..0a524400cf 100644 --- a/src/network/networkd-route-nexthop.h +++ b/src/network/networkd-route-nexthop.h @@ -1,14 +1,34 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include +#include + +#include "sd-netlink.h" + #include "conf-parser.h" +#include "in-addr-util.h" +#include "macro.h" typedef struct Link Link; typedef struct Manager Manager; -typedef struct MultipathRoute MultipathRoute; typedef struct Route Route; -int multipath_route_get_link(Manager *manager, Link *link, const MultipathRoute *m, Link **ret); +typedef struct RouteNextHop { + int family; /* used in RTA_VIA (IPv4 only) */ + union in_addr_union gw; /* RTA_GATEWAY or RTA_VIA (IPv4 only) */ + uint32_t weight; /* rtnh_hops */ + int ifindex; /* RTA_OIF(u32) or rtnh_ifindex */ + char *ifname; /* only used by Route object owned by Network object */ + /* unsupported attributes: RTA_FLOW (IPv4 only), RTA_ENCAP_TYPE, RTA_ENCAP. */ +} RouteNextHop; + +RouteNextHop* route_nexthop_free(RouteNextHop *nh); +DEFINE_TRIVIAL_CLEANUP_FUNC(RouteNextHop*, route_nexthop_free); + +void route_nexthops_done(Route *route); + +int route_nexthop_get_link(Manager *manager, Link *link, const RouteNextHop *nh, Link **ret); int route_nexthops_is_ready_to_configure(const Route *route, Link *link); int route_nexthops_to_string(const Route *route, char **ret); diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 3d1923a524..70a83c9d20 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -103,7 +103,7 @@ Route *route_free(Route *route) { if (route->wireguard) set_remove(route->wireguard->routes, route); - ordered_set_free_with_destructor(route->multipath_routes, multipath_route_free); + route_nexthops_done(route); route_metric_done(&route->metric); sd_event_source_disable_unref(route->expire); @@ -312,7 +312,7 @@ int route_dup(const Route *src, Route **ret) { dest->section = NULL; dest->link = NULL; dest->manager = NULL; - dest->multipath_routes = NULL; + dest->nexthops = NULL; dest->metric = ROUTE_METRIC_NULL; dest->expire = NULL; @@ -339,13 +339,13 @@ static void route_apply_nexthop(Route *route, const NextHop *nh, uint8_t nh_weig route->type = RTN_BLACKHOLE; } -static void route_apply_multipath_route(Route *route, const MultipathRoute *m) { +static void route_apply_route_nexthop(Route *route, const RouteNextHop *nh) { assert(route); - assert(m); + assert(nh); - route->gw_family = m->gateway.family; - route->gw = m->gateway.address; - route->gw_weight = m->weight; + route->gw_family = nh->family; + route->gw = nh->gw; + route->gw_weight = nh->weight; } typedef struct ConvertedRoutes { @@ -402,7 +402,7 @@ static int converted_routes_new(size_t n, ConvertedRoutes **ret) { static bool route_needs_convert(const Route *route) { assert(route); - return route->nexthop_id > 0 || !ordered_set_isempty(route->multipath_routes); + return route->nexthop_id > 0 || !ordered_set_isempty(route->nexthops); } static int route_convert(Manager *manager, Link *link, const Route *route, ConvertedRoutes **ret) { @@ -471,22 +471,22 @@ static int route_convert(Manager *manager, Link *link, const Route *route, Conve } - assert(!ordered_set_isempty(route->multipath_routes)); + assert(!ordered_set_isempty(route->nexthops)); - r = converted_routes_new(ordered_set_size(route->multipath_routes), &c); + r = converted_routes_new(ordered_set_size(route->nexthops), &c); if (r < 0) return r; size_t i = 0; - MultipathRoute *m; - ORDERED_SET_FOREACH(m, route->multipath_routes) { + RouteNextHop *nh; + ORDERED_SET_FOREACH(nh, route->nexthops) { r = route_dup(route, &c->routes[i]); if (r < 0) return r; - route_apply_multipath_route(c->routes[i], m); + route_apply_route_nexthop(c->routes[i], nh); - r = multipath_route_get_link(manager, link, m, &c->links[i]); + r = route_nexthop_get_link(manager, link, nh, &c->links[i]); if (r < 0) return r; @@ -2037,7 +2037,7 @@ int route_section_verify(Route *route) { else if (IN_SET(route->type, RTN_UNICAST, RTN_UNSPEC) && !route->gateway_from_dhcp_or_ra && !in_addr_is_set(route->gw_family, &route->gw) && - ordered_set_isempty(route->multipath_routes) && + ordered_set_isempty(route->nexthops) && route->nexthop_id == 0) route->scope = RT_SCOPE_LINK; } diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index 5c041953ab..1c020f6842 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -53,6 +53,9 @@ struct Route { int gateway_onlink; /* Only used in conf parser and route_section_verify(). */ uint32_t nexthop_id; + /* nexthops */ + OrderedSet *nexthops; /* RTA_MULTIPATH */ + /* metrics (RTA_METRICS) */ RouteMetric metric; @@ -75,7 +78,6 @@ struct Route { union in_addr_union dst; union in_addr_union src; union in_addr_union prefsrc; - OrderedSet *multipath_routes; }; extern const struct hash_ops route_hash_ops; From 054b8c2817c97aad988d3a829661b729948b3a23 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 12 Jan 2024 15:23:17 +0900 Subject: [PATCH 2/4] network/route: also use RouteNextHop for managing gateway No functional change, just refactoring. --- src/network/networkd-dhcp-prefix-delegation.c | 4 +- src/network/networkd-dhcp4.c | 24 +++---- src/network/networkd-json.c | 2 +- src/network/networkd-ndisc.c | 12 ++-- src/network/networkd-route-nexthop.c | 69 ++++++++++--------- src/network/networkd-route-util.c | 4 +- src/network/networkd-route.c | 30 ++++---- src/network/networkd-route.h | 6 +- 8 files changed, 75 insertions(+), 76 deletions(-) diff --git a/src/network/networkd-dhcp-prefix-delegation.c b/src/network/networkd-dhcp-prefix-delegation.c index 168b45349e..b30527e99f 100644 --- a/src/network/networkd-dhcp-prefix-delegation.c +++ b/src/network/networkd-dhcp-prefix-delegation.c @@ -782,8 +782,8 @@ static int dhcp4_pd_request_default_gateway_on_6rd_tunnel(Link *link, const stru route->source = NETWORK_CONFIG_SOURCE_DHCP_PD; route->family = AF_INET6; - route->gw_family = AF_INET6; - route->gw.in6.s6_addr32[3] = br_address->s_addr; + route->nexthop.family = AF_INET6; + route->nexthop.gw.in6.s6_addr32[3] = br_address->s_addr; route->scope = RT_SCOPE_UNIVERSE; route->protocol = RTPROT_DHCP; route->priority = IP6_RT_PRIO_USER; diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 44ed272387..b56106867b 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -482,8 +482,8 @@ static int dhcp4_request_route_auto( IPV4_ADDRESS_FMT_VAL(route->dst.in), route->dst_prefixlen, IPV4_ADDRESS_FMT_VAL(*gw)); route->scope = RT_SCOPE_HOST; - route->gw_family = AF_UNSPEC; - route->gw = IN_ADDR_NULL; + route->nexthop.family = AF_UNSPEC; + route->nexthop.gw = IN_ADDR_NULL; route->prefsrc = IN_ADDR_NULL; } else if (in4_addr_equal(&route->dst.in, &address)) { @@ -493,8 +493,8 @@ static int dhcp4_request_route_auto( IPV4_ADDRESS_FMT_VAL(route->dst.in), route->dst_prefixlen, IPV4_ADDRESS_FMT_VAL(*gw)); route->scope = RT_SCOPE_HOST; - route->gw_family = AF_UNSPEC; - route->gw = IN_ADDR_NULL; + route->nexthop.family = AF_UNSPEC; + route->nexthop.gw = IN_ADDR_NULL; route->prefsrc.in = address; } else if (in4_addr_is_null(gw)) { @@ -516,8 +516,8 @@ static int dhcp4_request_route_auto( } route->scope = RT_SCOPE_LINK; - route->gw_family = AF_UNSPEC; - route->gw = IN_ADDR_NULL; + route->nexthop.family = AF_UNSPEC; + route->nexthop.gw = IN_ADDR_NULL; route->prefsrc.in = address; } else { @@ -526,8 +526,8 @@ static int dhcp4_request_route_auto( return r; route->scope = RT_SCOPE_UNIVERSE; - route->gw_family = AF_INET; - route->gw.in = *gw; + route->nexthop.family = AF_INET; + route->nexthop.gw.in = *gw; route->prefsrc.in = address; } @@ -619,8 +619,8 @@ static int dhcp4_request_default_gateway(Link *link) { return r; /* Next, add a default gateway. */ - route->gw_family = AF_INET; - route->gw.in = router; + route->nexthop.family = AF_INET; + route->nexthop.gw.in = router; route->prefsrc.in = address; return dhcp4_request_route(TAKE_PTR(route), link); @@ -641,7 +641,7 @@ static int dhcp4_request_semi_static_routes(Link *link) { if (!rt->gateway_from_dhcp_or_ra) continue; - if (rt->gw_family != AF_INET) + if (rt->nexthop.family != AF_INET) continue; assert(rt->family == AF_INET); @@ -663,7 +663,7 @@ static int dhcp4_request_semi_static_routes(Link *link) { if (r < 0) return r; - route->gw.in = gw; + route->nexthop.gw.in = gw; r = dhcp4_request_route(TAKE_PTR(route), link); if (r < 0) diff --git a/src/network/networkd-json.c b/src/network/networkd-json.c index e62e57b765..9ca54db5ed 100644 --- a/src/network/networkd-json.c +++ b/src/network/networkd-json.c @@ -237,7 +237,7 @@ static int route_append_json(Route *route, JsonVariant **array) { JSON_BUILD_PAIR_INTEGER("Family", route->family), JSON_BUILD_PAIR_IN_ADDR("Destination", &route->dst, route->family), JSON_BUILD_PAIR_UNSIGNED("DestinationPrefixLength", route->dst_prefixlen), - JSON_BUILD_PAIR_IN_ADDR_NON_NULL("Gateway", &route->gw, route->gw_family), + JSON_BUILD_PAIR_IN_ADDR_NON_NULL("Gateway", &route->nexthop.gw, route->nexthop.family), JSON_BUILD_PAIR_CONDITION(route->src_prefixlen > 0, "Source", JSON_BUILD_IN_ADDR(&route->src, route->family)), JSON_BUILD_PAIR_UNSIGNED_NON_ZERO("SourcePrefixLength", route->src_prefixlen), diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 7cfb982403..33d11509d3 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -321,8 +321,8 @@ static int ndisc_router_process_default(Link *link, sd_ndisc_router *rt) { route->family = AF_INET6; route->pref = preference; - route->gw_family = AF_INET6; - route->gw.in6 = gateway; + route->nexthop.family = AF_INET6; + route->nexthop.gw.in6 = gateway; route->lifetime_usec = lifetime_usec; r = ndisc_request_route(TAKE_PTR(route), link, rt); @@ -337,14 +337,14 @@ static int ndisc_router_process_default(Link *link, sd_ndisc_router *rt) { if (!route_gw->gateway_from_dhcp_or_ra) continue; - if (route_gw->gw_family != AF_INET6) + if (route_gw->nexthop.family != AF_INET6) continue; r = route_dup(route_gw, &route); if (r < 0) return r; - route->gw.in6 = gateway; + route->nexthop.gw.in6 = gateway; if (!route->pref_set) route->pref = preference; route->lifetime_usec = lifetime_usec; @@ -626,8 +626,8 @@ static int ndisc_router_process_route(Link *link, sd_ndisc_router *rt) { route->family = AF_INET6; route->pref = preference; - route->gw.in6 = gateway; - route->gw_family = AF_INET6; + route->nexthop.gw.in6 = gateway; + route->nexthop.family = AF_INET6; route->dst.in6 = dst; route->dst_prefixlen = prefixlen; route->lifetime_usec = lifetime_usec; diff --git a/src/network/networkd-route-nexthop.c b/src/network/networkd-route-nexthop.c index 2a213f94a6..028504537c 100644 --- a/src/network/networkd-route-nexthop.c +++ b/src/network/networkd-route-nexthop.c @@ -31,6 +31,7 @@ RouteNextHop* route_nexthop_free(RouteNextHop *nh) { void route_nexthops_done(Route *route) { assert(route); + route_nexthop_done(&route->nexthop); ordered_set_free(route->nexthops); } @@ -157,7 +158,7 @@ int route_nexthops_is_ready_to_configure(const Route *route, Link *link) { return true; if (ordered_set_isempty(route->nexthops)) - return gateway_is_ready(link, FLAGS_SET(route->flags, RTNH_F_ONLINK), route->gw_family, &route->gw); + return route_nexthop_is_ready_to_configure(&route->nexthop, link, FLAGS_SET(route->flags, RTNH_F_ONLINK)); RouteNextHop *nh; ORDERED_SET_FOREACH(nh, route->nexthops) @@ -192,12 +193,12 @@ int route_nexthops_to_string(const Route *route, char **ret) { } if (ordered_set_isempty(route->nexthops)) { - if (in_addr_is_set(route->gw_family, &route->gw)) - buf = strjoin("gw: ", IN_ADDR_TO_STRING(route->gw_family, &route->gw)); + if (in_addr_is_set(route->nexthop.family, &route->nexthop.gw)) + buf = strjoin("gw: ", IN_ADDR_TO_STRING(route->nexthop.family, &route->nexthop.gw)); else if (route->gateway_from_dhcp_or_ra) { - if (route->gw_family == AF_INET) + if (route->nexthop.family == AF_INET) buf = strdup("gw: _dhcp4"); - else if (route->gw_family == AF_INET6) + else if (route->nexthop.family == AF_INET6) buf = strdup("gw: _ipv6ra"); else buf = strdup("gw: _dhcp"); @@ -349,15 +350,15 @@ int route_nexthops_set_netlink_message(Link *link, const Route *route, sd_netlin if (ordered_set_isempty(route->nexthops)) { - if (in_addr_is_set(route->gw_family, &route->gw)) { - if (route->gw_family == route->family) - r = netlink_message_append_in_addr_union(message, RTA_GATEWAY, route->gw_family, &route->gw); + if (in_addr_is_set(route->nexthop.family, &route->nexthop.gw)) { + if (route->nexthop.family == route->family) + r = netlink_message_append_in_addr_union(message, RTA_GATEWAY, route->nexthop.family, &route->nexthop.gw); else { assert(route->family == AF_INET); r = sd_netlink_message_append_data(message, RTA_VIA, &(const RouteVia) { - .family = route->gw_family, - .address = route->gw, + .family = route->nexthop.family, + .address = route->nexthop.gw, }, sizeof(RouteVia)); } if (r < 0) @@ -479,9 +480,9 @@ int route_nexthops_read_netlink_message(Route *route, sd_netlink_message *messag return log_warning_errno(r, "rtnl: could not get ifindex from route message, ignoring: %m"); if (ifindex > 0) { - r = netlink_message_read_in_addr_union(message, RTA_GATEWAY, route->family, &route->gw); + r = netlink_message_read_in_addr_union(message, RTA_GATEWAY, route->family, &route->nexthop.gw); if (r >= 0) { - route->gw_family = route->family; + route->nexthop.family = route->family; return 0; } if (r != -ENODATA) @@ -493,8 +494,8 @@ int route_nexthops_read_netlink_message(Route *route, sd_netlink_message *messag RouteVia via; r = sd_netlink_message_read(message, RTA_VIA, sizeof(via), &via); if (r >= 0) { - route->gw_family = via.family; - route->gw = via.address; + route->nexthop.family = via.family; + route->nexthop.gw = via.address; return 0; } if (r != -ENODATA) @@ -525,7 +526,7 @@ int route_section_verify_nexthops(Route *route) { if (route->gateway_from_dhcp_or_ra) { assert(route->network); - if (route->gw_family == AF_UNSPEC) + if (route->nexthop.family == AF_UNSPEC) /* When deprecated Gateway=_dhcp is set, then assume gateway family based on other settings. */ switch (route->family) { case AF_UNSPEC: @@ -533,7 +534,7 @@ int route_section_verify_nexthops(Route *route) { "Please use \"_dhcp4\" or \"_ipv6ra\" instead. Assuming \"_dhcp4\".", route->section->filename, route->section->line); - route->gw_family = route->family = AF_INET; + route->nexthop.family = route->family = AF_INET; break; case AF_INET: case AF_INET6: @@ -541,7 +542,7 @@ int route_section_verify_nexthops(Route *route) { "Assuming \"%s\" based on Destination=, Source=, or PreferredSource= setting.", route->section->filename, route->section->line, route->family == AF_INET ? "_dhcp4" : "_ipv6ra"); - route->gw_family = route->family; + route->nexthop.family = route->family; break; default: return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), @@ -549,13 +550,13 @@ int route_section_verify_nexthops(Route *route) { route->section->filename, route->section->line); } - if (route->gw_family == AF_INET && !FLAGS_SET(route->network->dhcp, ADDRESS_FAMILY_IPV4)) + if (route->nexthop.family == AF_INET && !FLAGS_SET(route->network->dhcp, ADDRESS_FAMILY_IPV4)) return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "%s: Gateway=\"_dhcp4\" is specified but DHCPv4 client is disabled. " "Ignoring [Route] section from line %u.", route->section->filename, route->section->line); - if (route->gw_family == AF_INET6 && !route->network->ipv6_accept_ra) + if (route->nexthop.family == AF_INET6 && !route->network->ipv6_accept_ra) return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "%s: Gateway=\"_ipv6ra\" is specified but IPv6AcceptRA= is disabled. " "Ignoring [Route] section from line %u.", @@ -564,7 +565,7 @@ int route_section_verify_nexthops(Route *route) { /* When only Gateway= is specified, assume the route family based on the Gateway address. */ if (route->family == AF_UNSPEC) - route->family = route->gw_family; + route->family = route->nexthop.family; if (route->family == AF_UNSPEC) { assert(route->section); @@ -576,7 +577,7 @@ int route_section_verify_nexthops(Route *route) { route->section->filename, route->section->line); } - if (route->gateway_onlink < 0 && in_addr_is_set(route->gw_family, &route->gw) && + if (route->gateway_onlink < 0 && in_addr_is_set(route->nexthop.family, &route->nexthop.gw) && route->network && ordered_hashmap_isempty(route->network->addresses_by_section)) { /* If no address is configured, in most cases the gateway cannot be reachable. * TODO: we may need to improve the condition above. */ @@ -590,7 +591,7 @@ int route_section_verify_nexthops(Route *route) { SET_FLAG(route->flags, RTNH_F_ONLINK, route->gateway_onlink); if (route->family == AF_INET6) { - if (route->gw_family == AF_INET) + if (route->nexthop.family == AF_INET) return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "%s: IPv4 gateway is configured for IPv6 route. " "Ignoring [Route] section from line %u.", @@ -607,7 +608,7 @@ int route_section_verify_nexthops(Route *route) { if (route->nexthop_id != 0 && (route->gateway_from_dhcp_or_ra || - in_addr_is_set(route->gw_family, &route->gw) || + in_addr_is_set(route->nexthop.family, &route->nexthop.gw) || !ordered_set_isempty(route->nexthops))) return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "%s: NextHopId= cannot be specified with Gateway= or MultiPathRoute=. " @@ -616,7 +617,7 @@ int route_section_verify_nexthops(Route *route) { if (route_type_is_reject(route) && (route->gateway_from_dhcp_or_ra || - in_addr_is_set(route->gw_family, &route->gw) || + in_addr_is_set(route->nexthop.family, &route->nexthop.gw) || !ordered_set_isempty(route->nexthops))) return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "%s: reject type route cannot be specified with Gateway= or MultiPathRoute=. " @@ -624,7 +625,7 @@ int route_section_verify_nexthops(Route *route) { route->section->filename, route->section->line); if ((route->gateway_from_dhcp_or_ra || - in_addr_is_set(route->gw_family, &route->gw)) && + in_addr_is_set(route->nexthop.family, &route->nexthop.gw)) && !ordered_set_isempty(route->nexthops)) return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "%s: Gateway= cannot be specified with MultiPathRoute=. " @@ -678,38 +679,38 @@ int config_parse_gateway( if (isempty(rvalue)) { route->gateway_from_dhcp_or_ra = false; - route->gw_family = AF_UNSPEC; - route->gw = IN_ADDR_NULL; + route->nexthop.family = AF_UNSPEC; + route->nexthop.gw = IN_ADDR_NULL; TAKE_PTR(route); return 0; } if (streq(rvalue, "_dhcp")) { route->gateway_from_dhcp_or_ra = true; - route->gw_family = AF_UNSPEC; - route->gw = IN_ADDR_NULL; + route->nexthop.family = AF_UNSPEC; + route->nexthop.gw = IN_ADDR_NULL; TAKE_PTR(route); return 0; } if (streq(rvalue, "_dhcp4")) { route->gateway_from_dhcp_or_ra = true; - route->gw_family = AF_INET; - route->gw = IN_ADDR_NULL; + route->nexthop.family = AF_INET; + route->nexthop.gw = IN_ADDR_NULL; TAKE_PTR(route); return 0; } if (streq(rvalue, "_ipv6ra")) { route->gateway_from_dhcp_or_ra = true; - route->gw_family = AF_INET6; - route->gw = IN_ADDR_NULL; + route->nexthop.family = AF_INET6; + route->nexthop.gw = IN_ADDR_NULL; TAKE_PTR(route); return 0; } } - r = in_addr_from_string_auto(rvalue, &route->gw_family, &route->gw); + r = in_addr_from_string_auto(rvalue, &route->nexthop.family, &route->nexthop.gw); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Invalid %s='%s', ignoring assignment: %m", lvalue, rvalue); diff --git a/src/network/networkd-route-util.c b/src/network/networkd-route-util.c index 00c267e692..fde7dfa5d4 100644 --- a/src/network/networkd-route-util.c +++ b/src/network/networkd-route-util.c @@ -74,7 +74,7 @@ bool link_find_default_gateway(Link *link, int family, Route **gw) { continue; if (route->scope != RT_SCOPE_UNIVERSE) continue; - if (!in_addr_is_set(route->gw_family, &route->gw)) + if (!in_addr_is_set(route->nexthop.family, &route->nexthop.gw)) continue; /* Found a default gateway. */ @@ -83,7 +83,7 @@ bool link_find_default_gateway(Link *link, int family, Route **gw) { /* If we have already found another gw, then let's compare their weight and priority. */ if (*gw) { - if (route->gw_weight > (*gw)->gw_weight) + if (route->nexthop.weight > (*gw)->nexthop.weight) continue; if (route->priority >= (*gw)->priority) continue; diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 70a83c9d20..d997404172 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -124,10 +124,10 @@ static void route_hash_func(const Route *route, struct siphash *state) { siphash24_compress_typesafe(route->src_prefixlen, state); in_addr_hash_func(&route->src, route->family, state); - siphash24_compress_typesafe(route->gw_family, state); - if (IN_SET(route->gw_family, AF_INET, AF_INET6)) { - in_addr_hash_func(&route->gw, route->gw_family, state); - siphash24_compress_typesafe(route->gw_weight, state); + siphash24_compress_typesafe(route->nexthop.family, state); + if (IN_SET(route->nexthop.family, AF_INET, AF_INET6)) { + in_addr_hash_func(&route->nexthop.gw, route->nexthop.family, state); + siphash24_compress_typesafe(route->nexthop.weight, state); } in_addr_hash_func(&route->prefsrc, route->family, state); @@ -174,16 +174,16 @@ static int route_compare_func(const Route *a, const Route *b) { if (r != 0) return r; - r = CMP(a->gw_family, b->gw_family); + r = CMP(a->nexthop.family, b->nexthop.family); if (r != 0) return r; - if (IN_SET(a->gw_family, AF_INET, AF_INET6)) { - r = memcmp(&a->gw, &b->gw, FAMILY_ADDRESS_SIZE(a->family)); + if (IN_SET(a->nexthop.family, AF_INET, AF_INET6)) { + r = memcmp(&a->nexthop.gw, &b->nexthop.gw, FAMILY_ADDRESS_SIZE(a->family)); if (r != 0) return r; - r = CMP(a->gw_weight, b->gw_weight); + r = CMP(a->nexthop.weight, b->nexthop.weight); if (r != 0) return r; } @@ -329,11 +329,11 @@ static void route_apply_nexthop(Route *route, const NextHop *nh, uint8_t nh_weig assert(nh); assert(hashmap_isempty(nh->group)); - route->gw_family = nh->family; - route->gw = nh->gw; + route->nexthop.family = nh->family; + route->nexthop.gw = nh->gw; if (nh_weight != UINT8_MAX) - route->gw_weight = nh_weight; + route->nexthop.weight = nh_weight; if (nh->blackhole) route->type = RTN_BLACKHOLE; @@ -343,9 +343,9 @@ static void route_apply_route_nexthop(Route *route, const RouteNextHop *nh) { assert(route); assert(nh); - route->gw_family = nh->family; - route->gw = nh->gw; - route->gw_weight = nh->weight; + route->nexthop.family = nh->family; + route->nexthop.gw = nh->gw; + route->nexthop.weight = nh->weight; } typedef struct ConvertedRoutes { @@ -2036,7 +2036,7 @@ int route_section_verify(Route *route) { route->scope = RT_SCOPE_LINK; else if (IN_SET(route->type, RTN_UNICAST, RTN_UNSPEC) && !route->gateway_from_dhcp_or_ra && - !in_addr_is_set(route->gw_family, &route->gw) && + !in_addr_is_set(route->nexthop.family, &route->nexthop.gw) && ordered_set_isempty(route->nexthops) && route->nexthop_id == 0) route->scope = RT_SCOPE_LINK; diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index 1c020f6842..47406dd635 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -37,8 +37,6 @@ struct Route { union in_addr_union provider; /* DHCP server or router address */ int family; - int gw_family; - uint32_t gw_weight; unsigned char dst_prefixlen; unsigned char src_prefixlen; @@ -51,10 +49,11 @@ struct Route { unsigned char pref; unsigned flags; int gateway_onlink; /* Only used in conf parser and route_section_verify(). */ - uint32_t nexthop_id; /* nexthops */ + RouteNextHop nexthop; /* RTA_OIF, and RTA_GATEWAY or RTA_VIA (IPv4 only) */ OrderedSet *nexthops; /* RTA_MULTIPATH */ + uint32_t nexthop_id; /* RTA_NH_ID */ /* metrics (RTA_METRICS) */ RouteMetric metric; @@ -74,7 +73,6 @@ struct Route { bool pref_set:1; bool gateway_from_dhcp_or_ra:1; - union in_addr_union gw; union in_addr_union dst; union in_addr_union src; union in_addr_union prefsrc; From b9e1caf64a44381c6cdfd41efa00a35dde83cde9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 13 Jan 2024 11:18:53 +0900 Subject: [PATCH 3/4] network/route-nexthop: store ifindex of the assigned interface in Route.nexthop.ifindex And use it if set on building netlink message. --- src/network/networkd-route-nexthop.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/network/networkd-route-nexthop.c b/src/network/networkd-route-nexthop.c index 028504537c..43ff790f04 100644 --- a/src/network/networkd-route-nexthop.c +++ b/src/network/networkd-route-nexthop.c @@ -365,8 +365,7 @@ int route_nexthops_set_netlink_message(Link *link, const Route *route, sd_netlin return r; } - assert(link); - return sd_netlink_message_append_u32(message, RTA_OIF, link->ifindex); + return sd_netlink_message_append_u32(message, RTA_OIF, route->nexthop.ifindex > 0 ? route->nexthop.ifindex : ASSERT_PTR(link)->ifindex); } return netlink_message_append_multipath_route(link, route, message); @@ -474,12 +473,14 @@ int route_nexthops_read_netlink_message(Route *route, sd_netlink_message *messag * routes. Hence, skip reading of RTA_OIF. */ return 0; - uint32_t ifindex = 0; + uint32_t ifindex; r = sd_netlink_message_read_u32(message, RTA_OIF, &ifindex); - if (r < 0 && r != -ENODATA) + if (r >= 0) + route->nexthop.ifindex = (int) ifindex; + else if (r != -ENODATA) return log_warning_errno(r, "rtnl: could not get ifindex from route message, ignoring: %m"); - if (ifindex > 0) { + if (route->nexthop.ifindex > 0) { r = netlink_message_read_in_addr_union(message, RTA_GATEWAY, route->family, &route->nexthop.gw); if (r >= 0) { route->nexthop.family = route->family; From 13efe0ab11d7e5439fbb82249376e9e0309dd268 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 14 Jan 2024 06:22:03 +0900 Subject: [PATCH 4/4] network/route: do not read RTA_OIF twice It is also read in route_nexthops_read_netlink_message(), and already stored in the Route object. Let's use it. --- src/network/networkd-route.c | 72 ++++++++++++++---------------------- 1 file changed, 28 insertions(+), 44 deletions(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index d997404172..ed40af3ac6 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1406,13 +1406,7 @@ static int process_route_one( } int manager_rtnl_process_route(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) { - _cleanup_(converted_routes_freep) ConvertedRoutes *converted = NULL; _cleanup_(route_freep) Route *tmp = NULL; - struct rta_cacheinfo cacheinfo; - bool has_cacheinfo; - Link *link = NULL; - uint32_t ifindex; - uint16_t type; int r; assert(rtnl); @@ -1427,6 +1421,7 @@ int manager_rtnl_process_route(sd_netlink *rtnl, sd_netlink_message *message, Ma return 0; } + uint16_t type; r = sd_netlink_message_get_type(message, &type); if (r < 0) { log_warning_errno(r, "rtnl: could not get message type, ignoring: %m"); @@ -1436,36 +1431,16 @@ int manager_rtnl_process_route(sd_netlink *rtnl, sd_netlink_message *message, Ma return 0; } - r = sd_netlink_message_read_u32(message, RTA_OIF, &ifindex); - if (r < 0 && r != -ENODATA) { - log_warning_errno(r, "rtnl: could not get ifindex from route message, ignoring: %m"); - return 0; - } else if (r >= 0) { - if (ifindex <= 0) { - log_warning("rtnl: received route message with invalid ifindex %u, ignoring.", ifindex); - return 0; - } - - r = link_get_by_index(m, ifindex, &link); - if (r < 0) { - /* when enumerating we might be out of sync, but we will - * get the route again, so just ignore it */ - if (!m->enumerating) - log_warning("rtnl: received route message for link (%u) we do not know about, ignoring", ifindex); - return 0; - } - } - r = route_new(&tmp); if (r < 0) return log_oom(); r = sd_rtnl_message_route_get_family(message, &tmp->family); if (r < 0) { - log_link_warning(link, "rtnl: received route message without family, ignoring"); + log_warning_errno(r, "rtnl: received route message without family, ignoring: %m"); return 0; } else if (!IN_SET(tmp->family, AF_INET, AF_INET6)) { - log_link_debug(link, "rtnl: received route message with invalid family '%i', ignoring", tmp->family); + log_debug("rtnl: received route message with invalid family '%i', ignoring.", tmp->family); return 0; } @@ -1483,49 +1458,49 @@ int manager_rtnl_process_route(sd_netlink *rtnl, sd_netlink_message *message, Ma r = netlink_message_read_in_addr_union(message, RTA_DST, tmp->family, &tmp->dst); if (r < 0 && r != -ENODATA) { - log_link_warning_errno(link, r, "rtnl: received route message without valid destination, ignoring: %m"); + log_warning_errno(r, "rtnl: received route message without valid destination, ignoring: %m"); return 0; } r = netlink_message_read_in_addr_union(message, RTA_SRC, tmp->family, &tmp->src); if (r < 0 && r != -ENODATA) { - log_link_warning_errno(link, r, "rtnl: received route message without valid source, ignoring: %m"); + log_warning_errno(r, "rtnl: received route message without valid source, ignoring: %m"); return 0; } r = netlink_message_read_in_addr_union(message, RTA_PREFSRC, tmp->family, &tmp->prefsrc); if (r < 0 && r != -ENODATA) { - log_link_warning_errno(link, r, "rtnl: received route message without valid preferred source, ignoring: %m"); + log_warning_errno(r, "rtnl: received route message without valid preferred source, ignoring: %m"); return 0; } r = sd_rtnl_message_route_get_dst_prefixlen(message, &tmp->dst_prefixlen); if (r < 0) { - log_link_warning_errno(link, r, "rtnl: received route message with invalid destination prefixlen, ignoring: %m"); + log_warning_errno(r, "rtnl: received route message with invalid destination prefixlen, ignoring: %m"); return 0; } r = sd_rtnl_message_route_get_src_prefixlen(message, &tmp->src_prefixlen); if (r < 0) { - log_link_warning_errno(link, r, "rtnl: received route message with invalid source prefixlen, ignoring: %m"); + log_warning_errno(r, "rtnl: received route message with invalid source prefixlen, ignoring: %m"); return 0; } r = sd_rtnl_message_route_get_scope(message, &tmp->scope); if (r < 0) { - log_link_warning_errno(link, r, "rtnl: received route message with invalid scope, ignoring: %m"); + log_warning_errno(r, "rtnl: received route message with invalid scope, ignoring: %m"); return 0; } r = sd_rtnl_message_route_get_tos(message, &tmp->tos); if (r < 0) { - log_link_warning_errno(link, r, "rtnl: received route message with invalid tos, ignoring: %m"); + log_warning_errno(r, "rtnl: received route message with invalid tos, ignoring: %m"); return 0; } r = sd_rtnl_message_route_get_type(message, &tmp->type); if (r < 0) { - log_link_warning_errno(link, r, "rtnl: received route message with invalid type, ignoring: %m"); + log_warning_errno(r, "rtnl: received route message with invalid type, ignoring: %m"); return 0; } @@ -1538,13 +1513,13 @@ int manager_rtnl_process_route(sd_netlink *rtnl, sd_netlink_message *message, Ma tmp->table = table; } if (r < 0) { - log_link_warning_errno(link, r, "rtnl: received route message with invalid table, ignoring: %m"); + log_warning_errno(r, "rtnl: received route message with invalid table, ignoring: %m"); return 0; } r = sd_netlink_message_read_u32(message, RTA_PRIORITY, &tmp->priority); if (r < 0 && r != -ENODATA) { - log_link_warning_errno(link, r, "rtnl: received route message with invalid priority, ignoring: %m"); + log_warning_errno(r, "rtnl: received route message with invalid priority, ignoring: %m"); return 0; } @@ -1556,22 +1531,31 @@ int manager_rtnl_process_route(sd_netlink *rtnl, sd_netlink_message *message, Ma if (route_metric_read_netlink_message(&tmp->metric, message) < 0) return 0; + bool has_cacheinfo; + struct rta_cacheinfo cacheinfo; r = sd_netlink_message_read(message, RTA_CACHEINFO, sizeof(cacheinfo), &cacheinfo); if (r < 0 && r != -ENODATA) { - log_link_warning_errno(link, r, "rtnl: failed to read RTA_CACHEINFO attribute, ignoring: %m"); + log_warning_errno(r, "rtnl: failed to read RTA_CACHEINFO attribute, ignoring: %m"); return 0; } has_cacheinfo = r >= 0; - /* IPv6 routes with reject type are always assigned to the loopback interface. See kernel's - * fib6_nh_init() in net/ipv6/route.c. However, we'd like to manage them by Manager. Hence, set - * link to NULL here. */ - if (route_type_is_reject(tmp)) - link = NULL; + Link *link = NULL; + if (tmp->nexthop.ifindex > 0) { + r = link_get_by_index(m, tmp->nexthop.ifindex, &link); + if (r < 0) { + /* when enumerating we might be out of sync, but we will + * get the route again, so just ignore it */ + if (!m->enumerating) + log_warning("rtnl: received route message for link (%i) we do not know about, ignoring", tmp->nexthop.ifindex); + return 0; + } + } if (!route_needs_convert(tmp)) return process_route_one(m, link, type, TAKE_PTR(tmp), has_cacheinfo ? &cacheinfo : NULL); + _cleanup_(converted_routes_freep) ConvertedRoutes *converted = NULL; r = route_convert(m, link, tmp, &converted); if (r < 0) { log_link_warning_errno(link, r, "rtnl: failed to convert received route, ignoring: %m");