From 234106dbf989fa766612116f325283c087bf25f0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 13 May 2021 15:07:35 +0900 Subject: [PATCH 1/5] network: route: make stored multipath route weight equivalent to hop of nexthop --- src/libsystemd/sd-netlink/netlink-util.c | 2 +- src/network/networkd-route.c | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-netlink/netlink-util.c b/src/libsystemd/sd-netlink/netlink-util.c index 90a184319a..78a51cf78d 100644 --- a/src/libsystemd/sd-netlink/netlink-util.c +++ b/src/libsystemd/sd-netlink/netlink-util.c @@ -472,7 +472,7 @@ int rtattr_read_nexthop(const struct rtnexthop *rtnh, size_t size, int family, O *m = (MultipathRoute) { .ifindex = rtnh->rtnh_ifindex, - .weight = rtnh->rtnh_hops == 0 ? 0 : rtnh->rtnh_hops + 1, + .weight = rtnh->rtnh_hops, }; if (rtnh->rtnh_len > sizeof(struct rtnexthop)) { diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index a5382e42ab..d0d0c192c0 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1220,7 +1220,7 @@ static int append_nexthop_one(const Route *route, const MultipathRoute *m, struc *rtnh = (struct rtnexthop) { .rtnh_len = sizeof(*rtnh), .rtnh_ifindex = m->ifindex, - .rtnh_hops = m->weight > 0 ? m->weight - 1 : 0, + .rtnh_hops = m->weight, }; (*rta)->rta_len += sizeof(struct rtnexthop); @@ -2779,11 +2779,16 @@ int config_parse_multipath_route( "Invalid multipath route weight, ignoring assignment: %s", p); return 0; } + /* ip command takes weight in the range 1…255, while kernel takes the value in the + * 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) { log_syntax(unit, LOG_WARNING, filename, line, 0, "Invalid multipath route weight, ignoring assignment: %s", p); return 0; } + m->weight--; } r = ordered_set_ensure_put(&n->multipath_routes, NULL, m); From 7b3a7581e3720d70e11f55124b20570b2ca74438 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 13 May 2021 04:59:56 +0900 Subject: [PATCH 2/5] network: make nexthop_add(), nexthop_configure() and friends return 0 on success After request queue is introduced, the return value on success is unused. --- src/network/networkd-address.c | 14 +++++----- src/network/networkd-neighbor.c | 4 +-- src/network/networkd-nexthop.c | 14 +++++----- src/network/networkd-route.c | 31 +++++++++------------- src/network/networkd-routing-policy-rule.c | 4 +-- 5 files changed, 27 insertions(+), 40 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 1a7e626f08..28a525da25 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -369,7 +369,6 @@ static int address_add_foreign(Link *link, const Address *in, Address **ret) { } static int address_add(Link *link, const Address *in, Address **ret) { - bool is_new = false; Address *address; int r; @@ -382,7 +381,6 @@ static int address_add(Link *link, const Address *in, Address **ret) { r = address_add_internal(link, &link->addresses, in, &address); if (r < 0) return r; - is_new = true; } else if (r == 0) { /* Take over a foreign address */ r = set_ensure_put(&link->addresses, &address_hash_ops, address); @@ -398,7 +396,7 @@ static int address_add(Link *link, const Address *in, Address **ret) { if (ret) *ret = address; - return is_new; + return 0; } static int address_update(Address *address, const Address *src) { @@ -939,7 +937,7 @@ static int address_configure( _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL; Address *acquired_address, *a; bool update; - int r, k; + int r; assert(address); assert(IN_SET(address->family, AF_INET, AF_INET6)); @@ -1006,9 +1004,9 @@ static int address_configure( if (r < 0) return log_link_error_errno(link, r, "Could not append IFA_RT_PRIORITY attribute: %m"); - k = address_add(link, address, &a); - if (k < 0) - return log_link_error_errno(link, k, "Could not add address: %m"); + r = address_add(link, address, &a); + if (r < 0) + return log_link_error_errno(link, r, "Could not add address: %m"); r = address_set_masquerade(a, true); if (r < 0) @@ -1031,7 +1029,7 @@ static int address_configure( if (ret) *ret = a; - return k; + return 0; } static int static_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { diff --git a/src/network/networkd-neighbor.c b/src/network/networkd-neighbor.c index 88620579ac..b10c08dec4 100644 --- a/src/network/networkd-neighbor.c +++ b/src/network/networkd-neighbor.c @@ -171,7 +171,6 @@ static int neighbor_add_internal(Link *link, Set **neighbors, const Neighbor *in } static int neighbor_add(Link *link, const Neighbor *in, Neighbor **ret) { - bool is_new = false; Neighbor *neighbor; int r; @@ -181,7 +180,6 @@ static int neighbor_add(Link *link, const Neighbor *in, Neighbor **ret) { r = neighbor_add_internal(link, &link->neighbors, in, &neighbor); if (r < 0) return r; - is_new = true; } else if (r == 0) { /* Neighbor is foreign, claim it as recognized */ r = set_ensure_put(&link->neighbors, &neighbor_hash_ops, neighbor); @@ -197,7 +195,7 @@ static int neighbor_add(Link *link, const Neighbor *in, Neighbor **ret) { if (ret) *ret = neighbor; - return is_new; + return 0; } static int neighbor_add_foreign(Link *link, const Neighbor *in, Neighbor **ret) { diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index 013bddf30c..ed0a203302 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -252,7 +252,6 @@ static int nexthop_add_foreign(Manager *manager, Link *link, const NextHop *in, } static int nexthop_add(Link *link, const NextHop *in, NextHop **ret) { - bool is_new = false; NextHop *nexthop; int r; @@ -271,7 +270,6 @@ static int nexthop_add(Link *link, const NextHop *in, NextHop **ret) { in, &nexthop); if (r < 0) return r; - is_new = true; } else if (r == 0) { /* Take over a foreign nexthop */ r = set_ensure_put(in->blackhole ? &link->manager->nexthops : &link->nexthops, @@ -288,7 +286,7 @@ static int nexthop_add(Link *link, const NextHop *in, NextHop **ret) { if (ret) *ret = nexthop; - return is_new; + return 0; } static int nexthop_update(Manager *manager, Link *link, NextHop *nexthop, const NextHop *in) { @@ -443,7 +441,7 @@ static int nexthop_configure( NextHop **ret) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL; - int r, k; + int r; assert(link); assert(link->manager); @@ -488,9 +486,9 @@ static int nexthop_configure( } } - k = nexthop_add(link, nexthop, ret); - if (k < 0) - return log_link_error_errno(link, k, "Could not add nexthop: %m"); + r = nexthop_add(link, nexthop, ret); + if (r < 0) + return log_link_error_errno(link, r, "Could not add nexthop: %m"); r = netlink_call_async(link->manager->rtnl, NULL, req, callback, link_netlink_destroy_callback, link); @@ -499,7 +497,7 @@ static int nexthop_configure( link_ref(link); - return k; + return r; } static int static_nexthop_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index d0d0c192c0..635f9a5c17 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -586,7 +586,6 @@ static int route_add_foreign(Manager *manager, Link *link, const Route *in, Rout static int route_add(Manager *manager, Link *link, const Route *in, const MultipathRoute *m, const NextHop *nh, Route **ret) { _cleanup_(route_freep) Route *tmp = NULL; - bool is_new = false; Route *route; int r; @@ -617,7 +616,6 @@ static int route_add(Manager *manager, Link *link, const Route *in, const Multip r = route_add_internal(manager, link, link ? &link->routes : &manager->routes, in, &route); if (r < 0) return r; - is_new = true; } else if (r == 0) { /* Take over a foreign route */ r = set_ensure_put(link ? &link->routes : &manager->routes, &route_hash_ops, route); @@ -633,7 +631,7 @@ static int route_add(Manager *manager, Link *link, const Route *in, const Multip if (ret) *ret = route; - return is_new; + return 0; } static bool route_type_is_reject(const Route *route) { @@ -1160,7 +1158,7 @@ static int route_add_and_setup_timer(Link *link, const Route *route, const Multi _cleanup_(sd_event_source_unrefp) sd_event_source *expire = NULL; NextHop *nh = NULL; Route *nr; - int r, k; + int r; assert(link); assert(link->manager); @@ -1169,9 +1167,9 @@ static int route_add_and_setup_timer(Link *link, const Route *route, const Multi (void) manager_get_nexthop_by_id(link->manager, route->nexthop_id, &nh); if (route_type_is_reject(route) || (nh && nh->blackhole)) - k = route_add(link->manager, NULL, route, NULL, nh, &nr); + r = route_add(link->manager, NULL, route, NULL, nh, &nr); else if (!m || m->ifindex == 0 || m->ifindex == link->ifindex) - k = route_add(NULL, link, route, m, nh, &nr); + r = route_add(NULL, link, route, m, nh, &nr); else { Link *link_gw; @@ -1179,10 +1177,10 @@ static int route_add_and_setup_timer(Link *link, const Route *route, const Multi if (r < 0) return log_link_error_errno(link, r, "Failed to get link with ifindex %d: %m", m->ifindex); - k = route_add(NULL, link_gw, route, m, NULL, &nr); + r = route_add(NULL, link_gw, route, m, NULL, &nr); } - if (k < 0) - return log_link_error_errno(link, k, "Could not add route: %m"); + if (r < 0) + return log_link_error_errno(link, r, "Could not add route: %m"); /* TODO: drop expiration handling once it can be pushed into the kernel */ if (nr->lifetime != USEC_INFINITY && !kernel_route_expiration_supported()) { @@ -1198,7 +1196,7 @@ static int route_add_and_setup_timer(Link *link, const Route *route, const Multi if (ret) *ret = nr; - return k; + return 0; } static int append_nexthop_one(const Route *route, const MultipathRoute *m, struct rtattr **rta, size_t offset) { @@ -1312,7 +1310,7 @@ static int route_configure( _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL; _cleanup_free_ Route **routes = NULL; unsigned n_routes = 0; /* avoid false maybe-uninitialized warning */ - int r, k; + int r; assert(link); assert(link->manager); @@ -1411,9 +1409,9 @@ static int route_configure( return log_oom(); } - k = route_add_and_setup_timer(link, route, NULL, routes); - if (k < 0) - return k; + r = route_add_and_setup_timer(link, route, NULL, routes); + if (r < 0) + return r; } else { MultipathRoute *m; Route **p; @@ -1429,14 +1427,11 @@ static int route_configure( return log_oom(); } - k = 0; p = routes; ORDERED_SET_FOREACH(m, route->multipath_routes) { r = route_add_and_setup_timer(link, route, m, ret_routes ? p++ : NULL); if (r < 0) return r; - if (r > 0) - k = 1; } } @@ -1452,7 +1447,7 @@ static int route_configure( *ret_routes = TAKE_PTR(routes); } - return k; + return 0; } static int static_route_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 20512d2477..d65e198333 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -318,7 +318,6 @@ static int routing_policy_rule_get(Manager *m, const RoutingPolicyRule *rule, Ro static int routing_policy_rule_add(Manager *m, const RoutingPolicyRule *in, RoutingPolicyRule **ret) { _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *rule = NULL; RoutingPolicyRule *existing; - bool is_new = false; int r; assert(m); @@ -339,7 +338,6 @@ static int routing_policy_rule_add(Manager *m, const RoutingPolicyRule *in, Rout rule->manager = m; existing = TAKE_PTR(rule); - is_new = true; } else if (r == 0) { /* Take over a foreign rule. */ r = set_ensure_put(&m->rules, &routing_policy_rule_hash_ops, existing); @@ -356,7 +354,7 @@ static int routing_policy_rule_add(Manager *m, const RoutingPolicyRule *in, Rout if (ret) *ret = existing; - return is_new; + return 0; } static int routing_policy_rule_consume_foreign(Manager *m, RoutingPolicyRule *rule) { From 8031e5ca8dd8a7967c705a2298ab3ccbee661253 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 18 May 2021 12:20:46 +0900 Subject: [PATCH 3/5] network: nexthop: IFF_UP flag is required for nexthops which attached to a link --- src/network/networkd-nexthop.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index ed0a203302..91f0eb3032 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -2,6 +2,7 @@ * Copyright © 2019 VMware, Inc. */ +#include #include #include "alloc-util.h" @@ -695,12 +696,21 @@ static bool nexthop_is_ready_to_configure(Link *link, const NextHop *nexthop) { assert(link); assert(nexthop); + if (!link_is_ready_to_configure(link, false)) + return false; + if (nexthop->blackhole) { if (link->manager->nexthop_remove_messages > 0) return false; } else { Link *l; + /* TODO: fdb nexthop does not require IFF_UP. The condition below needs to be updated + * when fdb nexthop support is added. See rtm_to_nh_config() in net/ipv4/nexthop.c of + * kernel. */ + if (!FLAGS_SET(link->flags, IFF_UP)) + return false; + HASHMAP_FOREACH(l, link->manager->links) { if (l->address_remove_messages > 0) return false; @@ -739,9 +749,6 @@ int request_process_nexthop(Request *req) { assert(req->nexthop); assert(req->type == REQUEST_TYPE_NEXTHOP); - if (!link_is_ready_to_configure(req->link, false)) - return 0; - if (!nexthop_is_ready_to_configure(req->link, req->nexthop)) return 0; From 228c3e21e957d2ff0ee63143cfe4eb0b3d88959b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 14 May 2021 10:15:23 +0900 Subject: [PATCH 4/5] network: nexthop: add Group= setting to configure multipath route with group nexthop --- man/systemd.network.xml | 13 + src/network/networkd-network-gperf.gperf | 1 + src/network/networkd-nexthop.c | 295 ++++++++++++++++-- src/network/networkd-nexthop.h | 3 + src/network/networkd-route.c | 224 +++++++++---- .../fuzz-network-parser/directives.network | 1 + 6 files changed, 434 insertions(+), 103 deletions(-) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index 2b8c1984da..61acea1a8b 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -1386,6 +1386,19 @@ IPv6Token=prefixstable:2002:da8:1:: no. + + Group= + + Takes a whitespace separated list of nexthop IDs. Each ID must be in the range + 1…4294967295. Optionally, each nexthop ID can take a weight after a colon + (id:weight). + The weight must be in the range 1…255. If the weight is not specified, then it is assumed + that the weight is 1. This setting cannot be specified with Gateway=, + Family=, Blackhole=. This setting can be specified + multiple times. If an empty string is assigned, then the all previous assignments are + cleared. Defaults to unset. + + diff --git a/src/network/networkd-network-gperf.gperf b/src/network/networkd-network-gperf.gperf index cfa53503d4..64b77a0276 100644 --- a/src/network/networkd-network-gperf.gperf +++ b/src/network/networkd-network-gperf.gperf @@ -194,6 +194,7 @@ NextHop.Gateway, config_parse_nexthop_gateway, NextHop.Family, config_parse_nexthop_family, 0, 0 NextHop.OnLink, config_parse_nexthop_onlink, 0, 0 NextHop.Blackhole, config_parse_nexthop_blackhole, 0, 0 +NextHop.Group, config_parse_nexthop_group, 0, 0 DHCPv4.ClientIdentifier, config_parse_dhcp_client_identifier, 0, offsetof(Network, dhcp_client_identifier) DHCPv4.UseDNS, config_parse_dhcp_use_dns, 0, 0 DHCPv4.RoutesToDNS, config_parse_bool, 0, offsetof(Network, dhcp_routes_to_dns) diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index 91f0eb3032..2bd1cec0bd 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -44,6 +44,8 @@ NextHop *nexthop_free(NextHop *nexthop) { hashmap_remove(nexthop->manager->nexthops_by_id, UINT32_TO_PTR(nexthop->id)); } + hashmap_free_free(nexthop->group); + return mfree(nexthop); } @@ -164,17 +166,41 @@ static bool nexthop_equal(const NextHop *a, const NextHop *b) { return nexthop_compare_func(a, b) == 0; } -static void nexthop_copy(NextHop *dest, const NextHop *src) { - assert(dest); +static int nexthop_dup(const NextHop *src, NextHop **ret) { + _cleanup_(nexthop_freep) NextHop *dest = NULL; + struct nexthop_grp *nhg; + int r; + assert(src); + assert(ret); - /* This only copies entries used in the above hash and compare functions. */ + dest = newdup(NextHop, src, 1); + if (!dest) + return -ENOMEM; - dest->protocol = src->protocol; - dest->id = src->id; - dest->blackhole = src->blackhole; - dest->family = src->family; - dest->gw = src->gw; + /* unset all pointers */ + dest->manager = NULL; + dest->link = NULL; + dest->network = NULL; + dest->section = NULL; + dest->group = NULL; + + HASHMAP_FOREACH(nhg, src->group) { + _cleanup_free_ struct nexthop_grp *g = NULL; + + g = newdup(struct nexthop_grp, nhg, 1); + if (!g) + return -ENOMEM; + + r = hashmap_ensure_put(&dest->group, NULL, UINT32_TO_PTR(g->id), g); + if (r < 0) + return r; + if (r > 0) + TAKE_PTR(g); + } + + *ret = TAKE_PTR(dest); + return 0; } int manager_get_nexthop_by_id(Manager *manager, uint32_t id, NextHop **ret) { @@ -225,12 +251,10 @@ static int nexthop_add_internal(Manager *manager, Link *link, Set **nexthops, co assert(nexthops); assert(in); - r = nexthop_new(&nexthop); + r = nexthop_dup(in, &nexthop); if (r < 0) return r; - nexthop_copy(nexthop, in); - r = set_ensure_put(nexthops, &nexthop_hash_ops, nexthop); if (r < 0) return r; @@ -252,33 +276,40 @@ static int nexthop_add_foreign(Manager *manager, Link *link, const NextHop *in, return nexthop_add_internal(manager, link, link ? &link->nexthops_foreign : &manager->nexthops_foreign, in, ret); } +static bool nexthop_has_link(const NextHop *nexthop) { + return !nexthop->blackhole && hashmap_isempty(nexthop->group); +} + static int nexthop_add(Link *link, const NextHop *in, NextHop **ret) { + bool by_manager; NextHop *nexthop; int r; assert(link); assert(in); - if (in->blackhole) + by_manager = !nexthop_has_link(in); + + if (by_manager) r = nexthop_get(link->manager, NULL, in, &nexthop); else r = nexthop_get(NULL, link, in, &nexthop); if (r == -ENOENT) { /* NextHop does not exist, create a new one */ r = nexthop_add_internal(link->manager, - in->blackhole ? NULL : link, - in->blackhole ? &link->manager->nexthops : &link->nexthops, + by_manager ? NULL : link, + by_manager ? &link->manager->nexthops : &link->nexthops, in, &nexthop); if (r < 0) return r; } else if (r == 0) { /* Take over a foreign nexthop */ - r = set_ensure_put(in->blackhole ? &link->manager->nexthops : &link->nexthops, + r = set_ensure_put(by_manager ? &link->manager->nexthops : &link->nexthops, &nexthop_hash_ops, nexthop); if (r < 0) return r; - set_remove(in->blackhole ? link->manager->nexthops_foreign : link->nexthops_foreign, nexthop); + set_remove(by_manager ? link->manager->nexthops_foreign : link->nexthops_foreign, nexthop); } else if (r == 1) { /* NextHop exists, do nothing */ ; @@ -337,7 +368,8 @@ set_manager: } static void log_nexthop_debug(const NextHop *nexthop, uint32_t id, const char *str, const Link *link) { - _cleanup_free_ char *gw = NULL; + _cleanup_free_ char *gw = NULL, *new_id = NULL, *group = NULL; + struct nexthop_grp *nhg; assert(nexthop); assert(str); @@ -347,14 +379,16 @@ static void log_nexthop_debug(const NextHop *nexthop, uint32_t id, const char *s if (!DEBUG_LOGGING) return; + if (nexthop->id != id) + (void) asprintf(&new_id, "→%"PRIu32, id); + (void) in_addr_to_string(nexthop->family, &nexthop->gw, &gw); - if (nexthop->id == id) - log_link_debug(link, "%s nexthop: id: %"PRIu32", gw: %s, blackhole: %s", - str, nexthop->id, strna(gw), yes_no(nexthop->blackhole)); - else - log_link_debug(link, "%s nexthop: id: %"PRIu32"→%"PRIu32", gw: %s, blackhole: %s", - str, nexthop->id, id, strna(gw), yes_no(nexthop->blackhole)); + HASHMAP_FOREACH(nhg, nexthop->group) + (void) strextendf_with_separator(&group, ",", "%"PRIu32":%"PRIu32, nhg->id, nhg->weight+1); + + log_link_debug(link, "%s nexthop: id: %"PRIu32"%s, gw: %s, blackhole: %s, group: %s", + str, nexthop->id, strempty(new_id), strna(gw), yes_no(nexthop->blackhole), strna(group)); } static int link_nexthop_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { @@ -448,7 +482,7 @@ static int nexthop_configure( assert(link->manager); assert(link->manager->rtnl); assert(link->ifindex > 0); - assert(IN_SET(nexthop->family, AF_INET, AF_INET6)); + assert(IN_SET(nexthop->family, AF_UNSPEC, AF_INET, AF_INET6)); assert(callback); log_nexthop_debug(nexthop, nexthop->id, "Configuring", link); @@ -465,7 +499,23 @@ static int nexthop_configure( return log_link_error_errno(link, r, "Could not append NHA_ID attribute: %m"); } - if (nexthop->blackhole) { + if (!hashmap_isempty(nexthop->group)) { + _cleanup_free_ struct nexthop_grp *group = NULL; + struct nexthop_grp *p, *nhg; + + group = new(struct nexthop_grp, hashmap_size(nexthop->group)); + if (!group) + return log_oom(); + + p = group; + HASHMAP_FOREACH(nhg, nexthop->group) + *p++ = *nhg; + + r = sd_netlink_message_append_data(req, NHA_GROUP, group, sizeof(struct nexthop_grp) * hashmap_size(nexthop->group)); + if (r < 0) + return log_link_error_errno(link, r, "Could not append NHA_GROUP attribute: %m"); + + } else if (nexthop->blackhole) { r = sd_netlink_message_append_flag(req, NHA_BLACKHOLE); if (r < 0) return log_link_error_errno(link, r, "Could not append NHA_BLACKHOLE attribute: %m"); @@ -693,13 +743,15 @@ int link_drop_nexthops(Link *link) { } static bool nexthop_is_ready_to_configure(Link *link, const NextHop *nexthop) { + struct nexthop_grp *nhg; + assert(link); assert(nexthop); if (!link_is_ready_to_configure(link, false)) return false; - if (nexthop->blackhole) { + if (!nexthop_has_link(nexthop)) { if (link->manager->nexthop_remove_messages > 0) return false; } else { @@ -721,6 +773,11 @@ static bool nexthop_is_ready_to_configure(Link *link, const NextHop *nexthop) { } } + /* All group members must be configured first. */ + HASHMAP_FOREACH(nhg, nexthop->group) + if (manager_get_nexthop_by_id(link->manager, nhg->id, NULL) < 0) + return false; + if (nexthop->id == 0) { Request *req; @@ -770,7 +827,9 @@ int request_process_nexthop(Request *req) { int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) { _cleanup_(nexthop_freep) NextHop *tmp = NULL; + _cleanup_free_ void *raw_group = NULL; NextHop *nexthop = NULL; + size_t raw_group_size; uint32_t ifindex; uint16_t type; Link *link = NULL; @@ -823,7 +882,7 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, if (r < 0) { log_link_warning_errno(link, r, "rtnl: could not get nexthop family, ignoring: %m"); return 0; - } else if (!IN_SET(tmp->family, AF_INET, AF_INET6)) { + } else if (!IN_SET(tmp->family, AF_UNSPEC, AF_INET, AF_INET6)) { log_link_debug(link, "rtnl: received nexthop message with invalid family %d, ignoring.", tmp->family); return 0; } @@ -834,10 +893,56 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, return 0; } - r = netlink_message_read_in_addr_union(message, NHA_GATEWAY, tmp->family, &tmp->gw); + r = sd_netlink_message_read_data(message, NHA_GROUP, &raw_group_size, &raw_group); if (r < 0 && r != -ENODATA) { - log_link_warning_errno(link, r, "rtnl: could not get NHA_GATEWAY attribute, ignoring: %m"); + log_link_warning_errno(link, r, "rtnl: could not get NHA_GROUP attribute, ignoring: %m"); return 0; + } else if (r >= 0) { + struct nexthop_grp *group = raw_group; + size_t n_group; + + if (raw_group_size == 0 || raw_group_size % sizeof(struct nexthop_grp) != 0) { + log_link_warning(link, "rtnl: received nexthop message with invalid nexthop group size, ignoring."); + return 0; + } + + assert((uintptr_t) group % __alignof__(struct nexthop_grp) == 0); + + n_group = raw_group_size / sizeof(struct nexthop_grp); + for (size_t i = 0; i < n_group; i++) { + _cleanup_free_ struct nexthop_grp *nhg = NULL; + + if (group[i].id == 0) { + log_link_warning(link, "rtnl: received nexthop message with invalid ID in group, ignoring."); + return 0; + } + if (group[i].weight > 254) { + log_link_warning(link, "rtnl: received nexthop message with invalid weight in group, ignoring."); + return 0; + } + + nhg = newdup(struct nexthop_grp, group + i, 1); + if (!nhg) + return log_oom(); + + r = hashmap_ensure_put(&tmp->group, NULL, UINT32_TO_PTR(nhg->id), nhg); + if (r == -ENOMEM) + return log_oom(); + if (r < 0) { + log_link_warning_errno(link, r, "Failed to store nexthop group, ignoring: %m"); + return 0; + } + if (r > 0) + TAKE_PTR(nhg); + } + } + + if (tmp->family != AF_UNSPEC) { + r = netlink_message_read_in_addr_union(message, NHA_GATEWAY, tmp->family, &tmp->gw); + if (r < 0 && r != -ENODATA) { + log_link_warning_errno(link, r, "rtnl: could not get NHA_GATEWAY attribute, ignoring: %m"); + return 0; + } } r = sd_netlink_message_has_flag(message, NHA_BLACKHOLE); @@ -859,9 +964,9 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, return 0; } - /* All blackhole nexthops are managed by Manager. Note that the linux kernel does not set - * NHA_OID attribute when NHA_BLACKHOLE is set. Just for safety. */ - if (tmp->blackhole) + /* All blackhole or group nexthops are managed by Manager. Note that the linux kernel does not + * set NHA_OID attribute when NHA_BLACKHOLE or NHA_GROUP is set. Just for safety. */ + if (!nexthop_has_link(tmp)) link = NULL; r = nexthop_get(m, link, tmp, &nexthop); @@ -913,8 +1018,26 @@ static int nexthop_section_verify(NextHop *nh) { if (section_is_invalid(nh->section)) return -EINVAL; - if (nh->family == AF_UNSPEC) - /* When no Gateway= is specified, assume IPv4. */ + if (!hashmap_isempty(nh->group)) { + if (in_addr_is_set(nh->family, &nh->gw)) + return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: nexthop group cannot have gateway address. " + "Ignoring [NextHop] section from line %u.", + nh->section->filename, nh->section->line); + + if (nh->family != AF_UNSPEC) + return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: nexthop group cannot have Family= setting. " + "Ignoring [NextHop] section from line %u.", + nh->section->filename, nh->section->line); + + if (nh->blackhole && in_addr_is_set(nh->family, &nh->gw)) + return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: nexthop group cannot be a blackhole. " + "Ignoring [NextHop] section from line %u.", + nh->section->filename, nh->section->line); + } else if (nh->family == AF_UNSPEC) + /* When neither Family=, Gateway=, nor Group= is specified, assume IPv4. */ nh->family = AF_INET; if (nh->blackhole && in_addr_is_set(nh->family, &nh->gw)) @@ -1190,3 +1313,107 @@ int config_parse_nexthop_blackhole( TAKE_PTR(n); return 0; } + +int config_parse_nexthop_group( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + + _cleanup_(nexthop_free_or_set_invalidp) NextHop *n = NULL; + Network *network = userdata; + int r; + + assert(filename); + assert(section); + assert(lvalue); + assert(rvalue); + assert(data); + + r = nexthop_new_static(network, filename, section_line, &n); + if (r < 0) + return log_oom(); + + if (isempty(rvalue)) { + n->group = hashmap_free_free(n->group); + TAKE_PTR(n); + return 0; + } + + for (const char *p = rvalue;;) { + _cleanup_free_ struct nexthop_grp *nhg = NULL; + _cleanup_free_ char *word = NULL; + uint32_t w; + char *sep; + + r = extract_first_word(&p, &word, NULL, 0); + if (r == -ENOMEM) + return log_oom(); + if (r < 0) { + log_syntax(unit, LOG_WARNING, filename, line, r, + "Invalid %s=, ignoring assignment: %s", lvalue, rvalue); + return 0; + } + if (r == 0) + break; + + nhg = new0(struct nexthop_grp, 1); + if (!nhg) + return log_oom(); + + sep = strchr(word, ':'); + if (sep) { + *sep++ = '\0'; + r = safe_atou32(sep, &w); + if (r < 0) { + log_syntax(unit, LOG_WARNING, filename, line, r, + "Failed to parse weight for nexthop group, ignoring assignment: %s:%s", + word, sep); + continue; + } + if (w == 0 || w > 256) { + log_syntax(unit, LOG_WARNING, filename, line, 0, + "Invalid weight for nexthop group, ignoring assignment: %s:%s", + word, sep); + continue; + } + /* See comments in config_parse_multipath_route(). */ + nhg->weight = w - 1; + } + + r = safe_atou32(word, &nhg->id); + if (r < 0) { + log_syntax(unit, LOG_WARNING, filename, line, r, + "Failed to parse nexthop ID in %s=, ignoring assignment: %s%s%s", + lvalue, word, sep ? ":" : "", strempty(sep)); + continue; + } + if (nhg->id == 0) { + log_syntax(unit, LOG_WARNING, filename, line, 0, + "Nexthop ID in %s= must be positive, ignoring assignment: %s%s%s", + lvalue, word, sep ? ":" : "", strempty(sep)); + continue; + } + + r = hashmap_ensure_put(&n->group, NULL, UINT32_TO_PTR(nhg->id), nhg); + if (r == -ENOMEM) + return log_oom(); + if (r == -EEXIST) { + log_syntax(unit, LOG_WARNING, filename, line, r, + "Nexthop ID %"PRIu32" is specified multiple times in %s=, ignoring assignment: %s%s%s", + nhg->id, lvalue, word, sep ? ":" : "", strempty(sep)); + continue; + } + assert(r > 0); + TAKE_PTR(nhg); + } + + TAKE_PTR(n); + return 0; +} diff --git a/src/network/networkd-nexthop.h b/src/network/networkd-nexthop.h index b2e7b45366..fee186aed2 100644 --- a/src/network/networkd-nexthop.h +++ b/src/network/networkd-nexthop.h @@ -9,6 +9,7 @@ #include "sd-netlink.h" #include "conf-parser.h" +#include "hashmap.h" #include "in-addr-util.h" #include "networkd-util.h" @@ -31,6 +32,7 @@ typedef struct NextHop { int family; union in_addr_union gw; int onlink; + Hashmap *group; } NextHop; NextHop *nexthop_free(NextHop *nexthop); @@ -51,3 +53,4 @@ CONFIG_PARSER_PROTOTYPE(config_parse_nexthop_gateway); CONFIG_PARSER_PROTOTYPE(config_parse_nexthop_family); CONFIG_PARSER_PROTOTYPE(config_parse_nexthop_onlink); CONFIG_PARSER_PROTOTYPE(config_parse_nexthop_blackhole); +CONFIG_PARSER_PROTOTYPE(config_parse_nexthop_group); diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 635f9a5c17..257255d4e6 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -2,6 +2,7 @@ #include #include +#include #include "alloc-util.h" #include "netlink-util.h" @@ -468,7 +469,7 @@ static int route_get(const Manager *manager, const Link *link, const Route *in, return -ENOENT; } -static void route_copy(Route *dest, const Route *src, const MultipathRoute *m, const NextHop *nh) { +static void route_copy(Route *dest, const Route *src, const MultipathRoute *m, const NextHop *nh, uint8_t nh_weight) { assert(dest); assert(src); @@ -496,9 +497,11 @@ static void route_copy(Route *dest, const Route *src, const MultipathRoute *m, c dest->nexthop_id = src->nexthop_id; if (nh) { + assert(hashmap_isempty(nh->group)); + dest->gw_family = nh->family; dest->gw = nh->gw; - dest->gw_weight = src->gw_weight; + dest->gw_weight = nh_weight != UINT8_MAX ? nh_weight : src->gw_weight; } else if (m) { dest->gw_family = m->gateway.family; dest->gw = m->gateway.address; @@ -560,7 +563,7 @@ static int route_add_internal(Manager *manager, Link *link, Set **routes, const if (r < 0) return r; - route_copy(route, in, NULL, NULL); + route_copy(route, in, NULL, NULL, UINT8_MAX); r = set_ensure_put(routes, &route_hash_ops, route); if (r < 0) @@ -584,7 +587,7 @@ static int route_add_foreign(Manager *manager, Link *link, const Route *in, Rout return route_add_internal(manager, link, link ? &link->routes_foreign : &manager->routes_foreign, in, ret); } -static int route_add(Manager *manager, Link *link, const Route *in, const MultipathRoute *m, const NextHop *nh, Route **ret) { +static int route_add(Manager *manager, Link *link, const Route *in, const MultipathRoute *m, const NextHop *nh, uint8_t nh_weight, Route **ret) { _cleanup_(route_freep) Route *tmp = NULL; Route *route; int r; @@ -593,11 +596,13 @@ static int route_add(Manager *manager, Link *link, const Route *in, const Multip assert(in); if (nh) { + assert(hashmap_isempty(nh->group)); + r = route_new(&tmp); if (r < 0) return r; - route_copy(tmp, in, NULL, nh); + route_copy(tmp, in, NULL, nh, nh_weight); in = tmp; } else if (m) { assert(link && (m->ifindex == 0 || m->ifindex == link->ifindex)); @@ -606,7 +611,7 @@ static int route_add(Manager *manager, Link *link, const Route *in, const Multip if (r < 0) return r; - route_copy(tmp, in, m, NULL); + route_copy(tmp, in, m, NULL, UINT8_MAX); in = tmp; } @@ -640,6 +645,26 @@ static bool route_type_is_reject(const Route *route) { return IN_SET(route->type, RTN_UNREACHABLE, RTN_PROHIBIT, RTN_BLACKHOLE, RTN_THROW); } +static int link_has_route_one(Link *link, const Route *route, const NextHop *nh, uint8_t nh_weight) { + _cleanup_(route_freep) Route *tmp = NULL; + int r; + + assert(link); + assert(route); + assert(nh); + + r = route_new(&tmp); + if (r < 0) + return r; + + route_copy(tmp, route, NULL, nh, nh_weight); + + if (route_type_is_reject(route) || (nh && nh->blackhole)) + return route_get(link->manager, NULL, tmp, NULL) >= 0; + else + return route_get(NULL, link, tmp, NULL) >= 0; +} + int link_has_route(Link *link, const Route *route) { MultipathRoute *m; int r; @@ -648,22 +673,27 @@ int link_has_route(Link *link, const Route *route) { assert(route); if (route->nexthop_id > 0) { - _cleanup_(route_freep) Route *tmp = NULL; + struct nexthop_grp *nhg; NextHop *nh; if (manager_get_nexthop_by_id(link->manager, route->nexthop_id, &nh) < 0) return false; - r = route_new(&tmp); - if (r < 0) - return r; + if (hashmap_isempty(nh->group)) + return link_has_route_one(link, route, nh, UINT8_MAX); - route_copy(tmp, route, NULL, nh); + HASHMAP_FOREACH(nhg, nh->group) { + NextHop *h; - if (route_type_is_reject(route) || (nh && nh->blackhole)) - return route_get(link->manager, NULL, tmp, NULL) >= 0; - else - return route_get(NULL, link, tmp, NULL) >= 0; + if (manager_get_nexthop_by_id(link->manager, nhg->id, &h) < 0) + return false; + + r = link_has_route_one(link, route, h, nhg->weight); + if (r <= 0) + return r; + } + + return true; } if (ordered_set_isempty(route->multipath_routes)) { @@ -692,7 +722,7 @@ int link_has_route(Link *link, const Route *route) { if (r < 0) return r; - route_copy(tmp, route, m, NULL); + route_copy(tmp, route, m, NULL, UINT8_MAX); if (route_get(NULL, l, tmp, NULL) < 0) return false; @@ -1102,7 +1132,7 @@ int link_drop_foreign_routes(Link *link) { continue; if (link_has_static_route(link, route)) - k = route_add(NULL, link, route, NULL, NULL, NULL); + k = route_add(NULL, link, route, NULL, NULL, UINT8_MAX, NULL); else k = route_remove(route, NULL, link); if (k < 0 && r >= 0) @@ -1154,31 +1184,34 @@ static int route_expire_handler(sd_event_source *s, uint64_t usec, void *userdat return 1; } -static int route_add_and_setup_timer(Link *link, const Route *route, const MultipathRoute *m, Route **ret) { +static int route_add_and_setup_timer_one(Link *link, const Route *route, const MultipathRoute *m, const NextHop *nh, uint8_t nh_weight, Route **ret) { _cleanup_(sd_event_source_unrefp) sd_event_source *expire = NULL; - NextHop *nh = NULL; Route *nr; int r; assert(link); assert(link->manager); assert(route); - - (void) manager_get_nexthop_by_id(link->manager, route->nexthop_id, &nh); + assert(!(m && nh)); + assert(ret); if (route_type_is_reject(route) || (nh && nh->blackhole)) - r = route_add(link->manager, NULL, route, NULL, nh, &nr); - else if (!m || m->ifindex == 0 || m->ifindex == link->ifindex) - r = route_add(NULL, link, route, m, nh, &nr); - else { + r = route_add(link->manager, NULL, route, NULL, nh, nh_weight, &nr); + else if (nh) { + assert(nh->link); + assert(hashmap_isempty(nh->group)); + + r = route_add(NULL, nh->link, route, NULL, nh, nh_weight, &nr); + } else if (m && m->ifindex != 0 && m->ifindex != link->ifindex) { Link *link_gw; r = link_get(link->manager, m->ifindex, &link_gw); if (r < 0) return log_link_error_errno(link, r, "Failed to get link with ifindex %d: %m", m->ifindex); - r = route_add(NULL, link_gw, route, m, NULL, &nr); - } + r = route_add(NULL, link_gw, route, m, NULL, UINT8_MAX, &nr); + } else + r = route_add(NULL, link, route, m, NULL, UINT8_MAX, &nr); if (r < 0) return log_link_error_errno(link, r, "Could not add route: %m"); @@ -1193,9 +1226,78 @@ static int route_add_and_setup_timer(Link *link, const Route *route, const Multi sd_event_source_unref(nr->expire); nr->expire = TAKE_PTR(expire); - if (ret) - *ret = nr; + *ret = nr; + return 0; +} +static int route_add_and_setup_timer(Link *link, const Route *route, unsigned *ret_n_routes, Route ***ret_routes) { + _cleanup_free_ Route **routes = NULL; + unsigned n_routes; + NextHop *nh; + Route **p; + int r; + + assert(link); + assert(route); + assert(ret_n_routes); + assert(ret_routes); + + if (route->nexthop_id > 0) { + r = manager_get_nexthop_by_id(link->manager, route->nexthop_id, &nh); + if (r < 0) + return log_link_error_errno(link, r, "Could not get nexthop by ID %"PRIu32": %m", route->nexthop_id); + } else + nh = NULL; + + if (nh && !hashmap_isempty(nh->group)) { + struct nexthop_grp *nhg; + + n_routes = hashmap_size(nh->group); + p = routes = new(Route*, n_routes); + if (!routes) + return log_oom(); + + HASHMAP_FOREACH(nhg, nh->group) { + NextHop *h; + + r = manager_get_nexthop_by_id(link->manager, nhg->id, &h); + if (r < 0) + return log_link_error_errno(link, r, "Could not get nexthop group member by ID %"PRIu32": %m", nhg->id); + + /* The nexthop h may be a blackhole nexthop. In that case, h->link is NULL. */ + r = route_add_and_setup_timer_one(h->link ?: link, route, NULL, h, nhg->weight, p++); + if (r < 0) + return r; + } + } else if (!ordered_set_isempty(route->multipath_routes)) { + MultipathRoute *m; + + assert(!nh); + assert(!in_addr_is_set(route->gw_family, &route->gw)); + + n_routes = ordered_set_size(route->multipath_routes); + p = routes = new(Route*, n_routes); + if (!routes) + return log_oom(); + + ORDERED_SET_FOREACH(m, route->multipath_routes) { + r = route_add_and_setup_timer_one(link, route, m, NULL, UINT8_MAX, p++); + if (r < 0) + return r; + } + } else { + n_routes = 1; + routes = new(Route*, n_routes); + if (!routes) + return log_oom(); + + r = route_add_and_setup_timer_one(link, route, NULL, nh, UINT8_MAX, routes); + if (r < 0) + return r; + } + + *ret_n_routes = n_routes; + *ret_routes = TAKE_PTR(routes); return 0; } @@ -1398,43 +1500,19 @@ static int route_configure( if (r < 0) return log_link_error_errno(link, r, "Could not append RTA_METRICS attribute: %m"); - if (route->nexthop_id != 0 || - in_addr_is_set(route->gw_family, &route->gw) || - ordered_set_isempty(route->multipath_routes)) { - - if (ret_routes) { - n_routes = 1; - routes = new(Route*, n_routes); - if (!routes) - return log_oom(); - } - - r = route_add_and_setup_timer(link, route, NULL, routes); - if (r < 0) - return r; - } else { - MultipathRoute *m; - Route **p; + if (!ordered_set_isempty(route->multipath_routes)) { + assert(route->nexthop_id == 0); + assert(!in_addr_is_set(route->gw_family, &route->gw)); r = append_nexthops(route, req); if (r < 0) return log_link_error_errno(link, r, "Could not append RTA_MULTIPATH attribute: %m"); - - if (ret_routes) { - n_routes = ordered_set_size(route->multipath_routes); - routes = new(Route*, n_routes); - if (!routes) - return log_oom(); - } - - p = routes; - ORDERED_SET_FOREACH(m, route->multipath_routes) { - r = route_add_and_setup_timer(link, route, m, ret_routes ? p++ : NULL); - if (r < 0) - return r; - } } + r = route_add_and_setup_timer(link, route, &n_routes, &routes); + if (r < 0) + return r; + r = netlink_call_async(link->manager->rtnl, NULL, req, callback, link_netlink_destroy_callback, link); if (r < 0) @@ -1447,7 +1525,7 @@ static int route_configure( *ret_routes = TAKE_PTR(routes); } - return 0; + return r; } static int static_route_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { @@ -1529,9 +1607,16 @@ static int route_is_ready_to_configure(const Route *route, Link *link) { assert(route); assert(link); - if (route->nexthop_id > 0 && - manager_get_nexthop_by_id(link->manager, route->nexthop_id, &nh) < 0) - return false; + if (route->nexthop_id > 0) { + struct nexthop_grp *nhg; + + if (manager_get_nexthop_by_id(link->manager, route->nexthop_id, &nh) < 0) + return false; + + HASHMAP_FOREACH(nhg, nh->group) + if (manager_get_nexthop_by_id(link->manager, nhg->id, NULL) < 0) + return false; + } if (route_type_is_reject(route) || (nh && nh->blackhole)) { if (nh && link->manager->nexthop_remove_messages > 0) @@ -1637,18 +1722,19 @@ static int process_route_one(Manager *manager, Link *link, uint16_t type, const (void) manager_get_nexthop_by_id(manager, tmp->nexthop_id, &nh); - if (nh) { - if (link && link != nh->link) + if (nh && hashmap_isempty(nh->group)) { + if (link && nh->link && link != nh->link) return log_link_warning_errno(link, SYNTHETIC_ERRNO(EINVAL), "rtnl: received RTA_OIF and ifindex of nexthop corresponding to RTA_NH_ID do not match, ignoring."); - link = nh->link; + if (nh->link) + link = nh->link; r = route_new(&nr); if (r < 0) return log_oom(); - route_copy(nr, tmp, NULL, nh); + route_copy(nr, tmp, NULL, nh, UINT8_MAX); tmp = nr; } else if (m) { @@ -1670,7 +1756,7 @@ static int process_route_one(Manager *manager, Link *link, uint16_t type, const if (r < 0) return log_oom(); - route_copy(nr, tmp, m, NULL); + route_copy(nr, tmp, m, NULL, UINT8_MAX); tmp = nr; } diff --git a/test/fuzz/fuzz-network-parser/directives.network b/test/fuzz/fuzz-network-parser/directives.network index 84717985ed..6d2d5f8054 100644 --- a/test/fuzz/fuzz-network-parser/directives.network +++ b/test/fuzz/fuzz-network-parser/directives.network @@ -372,6 +372,7 @@ Gateway= Family= OnLink= Blackhole= +Group= [QDisc] Parent= Handle= From 9c8f90d0f92cc591817a224a7c46d6ac6e80a0c0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 13 May 2021 11:11:48 +0900 Subject: [PATCH 5/5] test-network: add a test case for nexthop Group= setting --- .../conf/25-nexthop-dummy.network | 13 +++ test/test-network/conf/25-nexthop.network | 4 + test/test-network/systemd-networkd-tests.py | 105 +++++++++++------- 3 files changed, 81 insertions(+), 41 deletions(-) create mode 100644 test/test-network/conf/25-nexthop-dummy.network diff --git a/test/test-network/conf/25-nexthop-dummy.network b/test/test-network/conf/25-nexthop-dummy.network new file mode 100644 index 0000000000..e494aca8e3 --- /dev/null +++ b/test/test-network/conf/25-nexthop-dummy.network @@ -0,0 +1,13 @@ +[Match] +Name=dummy98 + +[Network] +Address=192.168.20.20/24 + +[NextHop] +Id=20 +Gateway=192.168.20.1 + +[NextHop] +Id=21 +Group=1:3 20:1 diff --git a/test/test-network/conf/25-nexthop.network b/test/test-network/conf/25-nexthop.network index 8b3e4b65c5..6f54e3f0d5 100644 --- a/test/test-network/conf/25-nexthop.network +++ b/test/test-network/conf/25-nexthop.network @@ -72,3 +72,7 @@ Destination=10.10.10.13 [Route] NextHop=7 Destination=2001:1234:5:8f62::2 + +[Route] +NextHop=21 +Destination=10.10.10.14 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 23b8abfbaa..56a22b0a44 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -1819,6 +1819,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): '25-neighbor-ipv6.network', '25-neighbor-ip-dummy.network', '25-neighbor-ip.network', + '25-nexthop-dummy.network', '25-nexthop-nothing.network', '25-nexthop.network', '25-qdisc-cake.network', @@ -2844,48 +2845,68 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): @expectedFailureIfNexthopIsNotAvailable() def test_nexthop(self): - copy_unit_to_networkd_unit_path('25-nexthop.network', '25-veth.netdev', '25-veth-peer.network') + def check_nexthop(self): + self.wait_online(['veth99:routable', 'veth-peer:routable', 'dummy98:routable']) + + output = check_output('ip nexthop list dev veth99') + print(output) + self.assertIn('id 1 via 192.168.5.1 dev veth99', output) + self.assertIn('id 2 via 2001:1234:5:8f63::2 dev veth99', output) + self.assertIn('id 3 dev veth99', output) + self.assertIn('id 4 dev veth99', output) + self.assertRegex(output, 'id 5 via 192.168.10.1 dev veth99 .*onlink') + self.assertRegex(output, r'id [0-9]* via 192.168.5.2 dev veth99') + + output = check_output('ip nexthop list dev dummy98') + print(output) + self.assertIn('id 20 via 192.168.20.1 dev dummy98', output) + + # kernel manages blackhole nexthops on lo + output = check_output('ip nexthop list dev lo') + print(output) + self.assertIn('id 6 blackhole', output) + self.assertIn('id 7 blackhole', output) + + # group nexthops are shown with -0 option + output = check_output('ip -0 nexthop list id 21') + print(output) + self.assertRegex(output, r'id 21 group (1,3/20|20/1,3)') + + output = check_output('ip route show dev veth99 10.10.10.10') + print(output) + self.assertEqual('10.10.10.10 nhid 1 via 192.168.5.1 proto static', output) + + output = check_output('ip route show dev veth99 10.10.10.11') + print(output) + self.assertEqual('10.10.10.11 nhid 2 via inet6 2001:1234:5:8f63::2 proto static', output) + + output = check_output('ip route show dev veth99 10.10.10.12') + print(output) + self.assertEqual('10.10.10.12 nhid 5 via 192.168.10.1 proto static onlink', output) + + output = check_output('ip -6 route show dev veth99 2001:1234:5:8f62::1') + print(output) + self.assertEqual('2001:1234:5:8f62::1 nhid 2 via 2001:1234:5:8f63::2 proto static metric 1024 pref medium', output) + + output = check_output('ip route show 10.10.10.13') + print(output) + self.assertEqual('blackhole 10.10.10.13 nhid 6 dev lo proto static', output) + + output = check_output('ip -6 route show 2001:1234:5:8f62::2') + print(output) + self.assertEqual('blackhole 2001:1234:5:8f62::2 nhid 7 dev lo proto static metric 1024 pref medium', output) + + output = check_output('ip route show 10.10.10.14') + print(output) + self.assertIn('10.10.10.14 nhid 21 proto static', output) + self.assertIn('nexthop via 192.168.20.1 dev dummy98 weight 1', output) + self.assertIn('nexthop via 192.168.5.1 dev veth99 weight 3', output) + + copy_unit_to_networkd_unit_path('25-nexthop.network', '25-veth.netdev', '25-veth-peer.network', + '12-dummy.netdev', '25-nexthop-dummy.network') start_networkd() - self.wait_online(['veth99:routable', 'veth-peer:routable']) - output = check_output('ip nexthop list dev veth99') - print(output) - self.assertIn('id 1 via 192.168.5.1 dev veth99', output) - self.assertIn('id 2 via 2001:1234:5:8f63::2 dev veth99', output) - self.assertIn('id 3 dev veth99', output) - self.assertIn('id 4 dev veth99', output) - self.assertRegex(output, 'id 5 via 192.168.10.1 dev veth99 .*onlink') - self.assertRegex(output, r'id [0-9]* via 192.168.5.2 dev veth99') - - # kernel manages blackhole nexthops on lo - output = check_output('ip nexthop list dev lo') - print(output) - self.assertIn('id 6 blackhole', output) - self.assertIn('id 7 blackhole', output) - - output = check_output('ip route show dev veth99 10.10.10.10') - print(output) - self.assertEqual('10.10.10.10 nhid 1 via 192.168.5.1 proto static', output) - - output = check_output('ip route show dev veth99 10.10.10.11') - print(output) - self.assertEqual('10.10.10.11 nhid 2 via inet6 2001:1234:5:8f63::2 proto static', output) - - output = check_output('ip route show dev veth99 10.10.10.12') - print(output) - self.assertEqual('10.10.10.12 nhid 5 via 192.168.10.1 proto static onlink', output) - - output = check_output('ip -6 route show dev veth99 2001:1234:5:8f62::1') - print(output) - self.assertEqual('2001:1234:5:8f62::1 nhid 2 via 2001:1234:5:8f63::2 proto static metric 1024 pref medium', output) - - output = check_output('ip route show 10.10.10.13') - print(output) - self.assertEqual('blackhole 10.10.10.13 nhid 6 dev lo proto static', output) - - output = check_output('ip -6 route show 2001:1234:5:8f62::2') - print(output) - self.assertEqual('blackhole 2001:1234:5:8f62::2 nhid 7 dev lo proto static metric 1024 pref medium', output) + check_nexthop(self) remove_unit_from_networkd_path(['25-nexthop.network']) copy_unit_to_networkd_unit_path('25-nexthop-nothing.network') @@ -2904,11 +2925,13 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): remove_unit_from_networkd_path(['25-nexthop-nothing.network']) copy_unit_to_networkd_unit_path('25-nexthop.network') + rc = call(*networkctl_cmd, 'reconfigure', 'dummy98', env=env) + self.assertEqual(rc, 0) rc = call(*networkctl_cmd, 'reload', env=env) self.assertEqual(rc, 0) time.sleep(1) - self.wait_online(['veth99:routable', 'veth-peer:routable']) + check_nexthop(self) rc = call('ip link del veth99') self.assertEqual(rc, 0)