From fc58350aa464cd2414b6fe9fec089412120c7d52 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 20 Aug 2024 03:36:40 +0900 Subject: [PATCH 01/12] network/routing-policy-rule: do not modify RountingPolicyRule objects managed by Manager or Network They are stored in Manager.rules set or Network.rules_by_section hashmap. For safety, let's not edit them even temporarily. No functional change, just refactoring. --- src/network/networkd-routing-policy-rule.c | 144 +++++++++++---------- 1 file changed, 75 insertions(+), 69 deletions(-) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 8e7cc47118..37eadb2b85 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -162,7 +162,7 @@ static int routing_policy_rule_new_static(Network *network, const char *filename return 0; } -static int routing_policy_rule_dup(const RoutingPolicyRule *src, RoutingPolicyRule **ret) { +static int routing_policy_rule_dup(const RoutingPolicyRule *src, int family, RoutingPolicyRule **ret) { _cleanup_(routing_policy_rule_unrefp) RoutingPolicyRule *dest = NULL; assert(src); @@ -179,6 +179,9 @@ static int routing_policy_rule_dup(const RoutingPolicyRule *src, RoutingPolicyRu dest->section = NULL; dest->iif = dest->oif = NULL; + /* Set family. */ + dest->family = family; + if (src->iif) { dest->iif = strdup(src->iif); if (!dest->iif) @@ -227,15 +230,17 @@ static void routing_policy_rule_hash_func(const RoutingPolicyRule *rule, struct in_addr_hash_func(&rule->to, rule->family, state); } -static int routing_policy_rule_compare_func(const RoutingPolicyRule *a, const RoutingPolicyRule *b) { +static int routing_policy_rule_compare_func_full(const RoutingPolicyRule *a, const RoutingPolicyRule *b, bool all) { int r; assert(a); assert(b); - r = CMP(a->family, b->family); - if (r != 0) - return r; + if (all) { + r = CMP(a->family, b->family); + if (r != 0) + return r; + } r = CMP(a->type, b->type); if (r != 0) @@ -245,9 +250,11 @@ static int routing_policy_rule_compare_func(const RoutingPolicyRule *a, const Ro if (r != 0) return r; - r = CMP(a->priority, b->priority); - if (r != 0) - return r; + if (all) { + r = CMP(a->priority, b->priority); + if (r != 0) + return r; + } r = strcmp_ptr(a->iif, b->iif); if (r != 0) @@ -309,63 +316,64 @@ static int routing_policy_rule_compare_func(const RoutingPolicyRule *a, const Ro if (r != 0) return r; - r = memcmp(&a->from, &b->from, FAMILY_ADDRESS_SIZE(a->family)); - if (r != 0) - return r; + if (all) { + r = memcmp(&a->from, &b->from, FAMILY_ADDRESS_SIZE(a->family)); + if (r != 0) + return r; - r = memcmp(&a->to, &b->to, FAMILY_ADDRESS_SIZE(a->family)); - if (r != 0) - return r; + r = memcmp(&a->to, &b->to, FAMILY_ADDRESS_SIZE(a->family)); + if (r != 0) + return r; + } return 0; } -static bool routing_policy_rule_equal(const RoutingPolicyRule *rule1, const RoutingPolicyRule *rule2) { - if (rule1 == rule2) - return true; - - if (!rule1 || !rule2) - return false; - - return routing_policy_rule_compare_func(rule1, rule2) == 0; +static int routing_policy_rule_compare_func(const RoutingPolicyRule *a, const RoutingPolicyRule *b) { + return routing_policy_rule_compare_func_full(a, b, /* all = */ true); } -static int routing_policy_rule_get(Manager *m, const RoutingPolicyRule *in, RoutingPolicyRule **ret) { +static bool routing_policy_rule_equal(const RoutingPolicyRule *a, const RoutingPolicyRule *b, int family, uint32_t priority) { + assert(a); + assert(b); + + if (a->family != AF_UNSPEC && a->family != family) + return false; + if (b->family != AF_UNSPEC && b->family != family) + return false; + + if (a->priority_set && a->priority != priority) + return false; + if (b->priority_set && b->priority != priority) + return false; + + return routing_policy_rule_compare_func_full(a, b, /* all = */ false) == 0; +} + +static int routing_policy_rule_get(Manager *m, const RoutingPolicyRule *in, int family, RoutingPolicyRule **ret) { RoutingPolicyRule *rule; assert(m); assert(in); + assert(in->family == AF_UNSPEC || in->family == family); + assert(IN_SET(family, AF_INET, AF_INET6)); + + if (in->priority_set && in->family != AF_UNSPEC) { + rule = set_get(m->rules, in); + if (!rule) + return -ENOENT; - rule = set_get(m->rules, in); - if (rule) { if (ret) *ret = rule; return 0; } - if (in->priority_set) - return -ENOENT; - - /* Also find rules configured without priority. */ - SET_FOREACH(rule, m->rules) { - uint32_t priority; - bool found; - - if (rule->priority_set) - /* The rule is configured with priority. */ - continue; - - priority = rule->priority; - rule->priority = 0; - found = routing_policy_rule_equal(rule, in); - rule->priority = priority; - - if (found) { + SET_FOREACH(rule, m->rules) + if (routing_policy_rule_equal(in, rule, family, rule->priority)) { if (ret) *ret = rule; return 0; } - } return -ENOENT; } @@ -434,6 +442,7 @@ static int routing_policy_rule_acquire_priority(Manager *manager, RoutingPolicyR break; rule->priority = priority; + rule->priority_set = true; return 0; } @@ -688,19 +697,14 @@ static void manager_mark_routing_policy_rules(Manager *m, bool foreign, const Li RoutingPolicyRule *existing; if (IN_SET(rule->family, AF_INET, AF_INET6)) { - if (routing_policy_rule_get(m, rule, &existing) >= 0) + if (routing_policy_rule_get(m, rule, rule->family, &existing) >= 0) routing_policy_rule_unmark(existing); } else { - /* The case Family=both. */ - rule->family = AF_INET; - if (routing_policy_rule_get(m, rule, &existing) >= 0) + if (routing_policy_rule_get(m, rule, AF_INET, &existing) >= 0) routing_policy_rule_unmark(existing); - rule->family = AF_INET6; - if (routing_policy_rule_get(m, rule, &existing) >= 0) + if (routing_policy_rule_get(m, rule, AF_INET6, &existing) >= 0) routing_policy_rule_unmark(existing); - - rule->family = AF_UNSPEC; } } } @@ -786,7 +790,7 @@ static int static_routing_policy_rule_configure_handler( return 1; } -static int link_request_routing_policy_rule(Link *link, RoutingPolicyRule *rule) { +static int link_request_routing_policy_rule(Link *link, const RoutingPolicyRule *rule, int family) { RoutingPolicyRule *existing; int r; @@ -794,11 +798,13 @@ static int link_request_routing_policy_rule(Link *link, RoutingPolicyRule *rule) assert(link->manager); assert(rule); assert(rule->source != NETWORK_CONFIG_SOURCE_FOREIGN); + assert(rule->family == AF_UNSPEC || rule->family == family); + assert(IN_SET(family, AF_INET, AF_INET6)); - if (routing_policy_rule_get(link->manager, rule, &existing) < 0) { + if (routing_policy_rule_get(link->manager, rule, family, &existing) < 0) { _cleanup_(routing_policy_rule_unrefp) RoutingPolicyRule *tmp = NULL; - r = routing_policy_rule_dup(rule, &tmp); + r = routing_policy_rule_dup(rule, family, &tmp); if (r < 0) return r; @@ -830,23 +836,23 @@ static int link_request_routing_policy_rule(Link *link, RoutingPolicyRule *rule) return 1; } -static int link_request_static_routing_policy_rule(Link *link, RoutingPolicyRule *rule) { +static int link_request_static_routing_policy_rule(Link *link, const RoutingPolicyRule *rule) { int r; if (IN_SET(rule->family, AF_INET, AF_INET6)) - return link_request_routing_policy_rule(link, rule); + return link_request_routing_policy_rule(link, rule, rule->family); - rule->family = AF_INET; - r = link_request_routing_policy_rule(link, rule); - if (r < 0) { - rule->family = AF_UNSPEC; + assert(rule->address_family == ADDRESS_FAMILY_YES); + + r = link_request_routing_policy_rule(link, rule, AF_INET); + if (r < 0) return r; - } - rule->family = AF_INET6; - r = link_request_routing_policy_rule(link, rule); - rule->family = AF_UNSPEC; - return r; + r = link_request_routing_policy_rule(link, rule, AF_INET6); + if (r < 0) + return r; + + return 0; } int link_request_static_routing_policy_rules(Link *link) { @@ -888,8 +894,8 @@ static const RoutingPolicyRule kernel_rules[] = { static bool routing_policy_rule_is_created_by_kernel(const RoutingPolicyRule *rule) { assert(rule); - for (size_t i = 0; i < ELEMENTSOF(kernel_rules); i++) - if (routing_policy_rule_equal(rule, &kernel_rules[i])) + FOREACH_ELEMENT(i, kernel_rules) + if (routing_policy_rule_equal(rule, i, i->family, i->priority)) return true; return false; @@ -1084,7 +1090,7 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Man * protocol of the received rule is RTPROT_KERNEL or RTPROT_STATIC. */ tmp->protocol = routing_policy_rule_is_created_by_kernel(tmp) ? RTPROT_KERNEL : RTPROT_STATIC; - (void) routing_policy_rule_get(m, tmp, &rule); + (void) routing_policy_rule_get(m, tmp, tmp->family, &rule); switch (type) { case RTM_NEWRULE: From d37afb5779c6511f247c807eb25e76d803b47037 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 20 Aug 2024 03:52:47 +0900 Subject: [PATCH 02/12] network/routing-policy-rule: add trailing period to the log message --- src/network/networkd-routing-policy-rule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 37eadb2b85..2c3e56b390 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -782,7 +782,7 @@ static int static_routing_policy_rule_configure_handler( } if (link->static_routing_policy_rule_messages == 0) { - log_link_debug(link, "Routing policy rule configured"); + log_link_debug(link, "Routing policy rule configured."); link->static_routing_policy_rules_configured = true; link_check_ready(link); } @@ -874,7 +874,7 @@ int link_request_static_routing_policy_rules(Link *link) { link->static_routing_policy_rules_configured = true; link_check_ready(link); } else { - log_link_debug(link, "Requesting routing policy rules"); + log_link_debug(link, "Requesting routing policy rules."); link_set_state(link, LINK_STATE_CONFIGURING); } From 4f8b153d903fd4a6e8d7d3e49113582d70d1525a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 20 Aug 2024 03:56:33 +0900 Subject: [PATCH 03/12] network/routing-policy-rule: skip requesting when rule is already requested If it is already requested, the new request will be anyway silently refused by link_queue_request_safe(), which returns 0 in such case. Let's return earlier. There should be no functional change, just refactoring. --- src/network/networkd-queue.c | 12 +++++- src/network/networkd-routing-policy-rule.c | 44 ++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-queue.c b/src/network/networkd-queue.c index 98c629f161..73dcf67a38 100644 --- a/src/network/networkd-queue.c +++ b/src/network/networkd-queue.c @@ -58,7 +58,11 @@ static void request_hash_func(const Request *req, struct siphash *state) { siphash24_compress_typesafe(req->type, state); - if (!IN_SET(req->type, REQUEST_TYPE_NEXTHOP, REQUEST_TYPE_ROUTE)) { + if (!IN_SET(req->type, + REQUEST_TYPE_NEXTHOP, + REQUEST_TYPE_ROUTE, + REQUEST_TYPE_ROUTING_POLICY_RULE)) { + siphash24_compress_boolean(req->link, state); if (req->link) siphash24_compress_typesafe(req->link->ifindex, state); @@ -81,7 +85,11 @@ static int request_compare_func(const struct Request *a, const struct Request *b if (r != 0) return r; - if (!IN_SET(a->type, REQUEST_TYPE_NEXTHOP, REQUEST_TYPE_ROUTE)) { + if (!IN_SET(a->type, + REQUEST_TYPE_NEXTHOP, + REQUEST_TYPE_ROUTE, + REQUEST_TYPE_ROUTING_POLICY_RULE)) { + r = CMP(!!a->link, !!b->link); if (r != 0) return r; diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 2c3e56b390..95261fc258 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -378,6 +378,47 @@ static int routing_policy_rule_get(Manager *m, const RoutingPolicyRule *in, int return -ENOENT; } +static int routing_policy_rule_get_request(Manager *m, const RoutingPolicyRule *in, int family, Request **ret) { + Request *req; + + assert(m); + assert(in); + assert(in->family == AF_UNSPEC || in->family == family); + assert(IN_SET(family, AF_INET, AF_INET6)); + + if (in->priority_set && in->family != AF_UNSPEC) { + req = ordered_set_get( + m->request_queue, + &(Request) { + .type = REQUEST_TYPE_ROUTING_POLICY_RULE, + .userdata = (void*) in, + .hash_func = (hash_func_t) routing_policy_rule_hash_func, + .compare_func = (compare_func_t) routing_policy_rule_compare_func, + }); + if (!req) + return -ENOENT; + + if (ret) + *ret = req; + return 0; + } + + ORDERED_SET_FOREACH(req, m->request_queue) { + + if (req->type != REQUEST_TYPE_ROUTING_POLICY_RULE) + continue; + + RoutingPolicyRule *rule = ASSERT_PTR(req->userdata); + if (routing_policy_rule_equal(in, rule, family, rule->priority)) { + if (ret) + *ret = req; + return 0; + } + } + + return -ENOENT; +} + static int routing_policy_rule_attach(Manager *m, RoutingPolicyRule *rule) { int r; @@ -801,6 +842,9 @@ static int link_request_routing_policy_rule(Link *link, const RoutingPolicyRule assert(rule->family == AF_UNSPEC || rule->family == family); assert(IN_SET(family, AF_INET, AF_INET6)); + if (routing_policy_rule_get_request(link->manager, rule, family, NULL) >= 0) + return 0; /* already requested, skipping. */ + if (routing_policy_rule_get(link->manager, rule, family, &existing) < 0) { _cleanup_(routing_policy_rule_unrefp) RoutingPolicyRule *tmp = NULL; From 65f5f581568448d6098358b704cae10a656d09f0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 20 Aug 2024 04:02:46 +0900 Subject: [PATCH 04/12] network/routing-policy-rule: do not save rule to Manager before it is configured Otherwise, if we fail to configure the rule, then the manager will keep nonexistent rule forever. So, let's first copy the rule and put it on Request, then on success generate a new copy based on the netlink notification and store it to Manager. This is the same as 0a0c2672dbd22dc85d660e5baa7e1bef701beb88, but for routing policy rule. --- src/network/networkd-routing-policy-rule.c | 149 ++++++++++++++------- 1 file changed, 97 insertions(+), 52 deletions(-) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 95261fc258..6ec81eec8c 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -465,6 +465,21 @@ static int routing_policy_rule_acquire_priority(Manager *manager, RoutingPolicyR return r; } + Request *req; + ORDERED_SET_FOREACH(req, manager->request_queue) { + if (req->type != REQUEST_TYPE_ROUTING_POLICY_RULE) + continue; + + tmp = ASSERT_PTR(req->userdata); + if (tmp->family != rule->family) + continue; + if (tmp->priority == 0 || tmp->priority > 32765) + continue; + r = set_ensure_put(&priorities, NULL, UINT32_TO_PTR(tmp->priority)); + if (r < 0) + return r; + } + ORDERED_HASHMAP_FOREACH(network, manager->networks) HASHMAP_FOREACH(tmp, network->rules_by_section) { if (tmp->family != AF_UNSPEC && tmp->family != rule->family) @@ -786,10 +801,12 @@ void link_foreignize_routing_policy_rules(Link *link) { } static int routing_policy_rule_process_request(Request *req, Link *link, RoutingPolicyRule *rule) { + RoutingPolicyRule *existing; int r; assert(req); assert(link); + assert(link->manager); assert(rule); if (!link_is_ready_to_configure(link, false)) @@ -800,6 +817,9 @@ static int routing_policy_rule_process_request(Request *req, Link *link, Routing return log_link_warning_errno(link, r, "Failed to configure routing policy rule: %m"); routing_policy_rule_enter_configuring(rule); + if (routing_policy_rule_get(link->manager, rule, rule->family, &existing) >= 0) + routing_policy_rule_enter_configuring(existing); + return 1; } @@ -814,9 +834,17 @@ static int static_routing_policy_rule_configure_handler( assert(m); assert(link); + assert(rule); r = sd_netlink_message_get_errno(m); - if (r < 0 && r != -EEXIST) { + if (r == -EEXIST) { + RoutingPolicyRule *existing; + + if (routing_policy_rule_get(link->manager, rule, rule->family, &existing) >= 0) { + existing->source = rule->source; + routing_policy_rule_enter_configured(existing); + } + } else if (r < 0) { log_link_message_warning_errno(link, m, r, "Could not add routing policy rule"); link_enter_failed(link); return 1; @@ -832,7 +860,8 @@ static int static_routing_policy_rule_configure_handler( } static int link_request_routing_policy_rule(Link *link, const RoutingPolicyRule *rule, int family) { - RoutingPolicyRule *existing; + _cleanup_(routing_policy_rule_unrefp) RoutingPolicyRule *tmp = NULL; + RoutingPolicyRule *existing = NULL; int r; assert(link); @@ -845,28 +874,29 @@ static int link_request_routing_policy_rule(Link *link, const RoutingPolicyRule if (routing_policy_rule_get_request(link->manager, rule, family, NULL) >= 0) return 0; /* already requested, skipping. */ - if (routing_policy_rule_get(link->manager, rule, family, &existing) < 0) { - _cleanup_(routing_policy_rule_unrefp) RoutingPolicyRule *tmp = NULL; - - r = routing_policy_rule_dup(rule, family, &tmp); - if (r < 0) - return r; + r = routing_policy_rule_dup(rule, family, &tmp); + if (r < 0) + return r; + if (routing_policy_rule_get(link->manager, tmp, family, &existing) < 0) { r = routing_policy_rule_acquire_priority(link->manager, tmp); if (r < 0) return r; + } else { + /* Copy priority from existing rule. */ + if (!tmp->priority_set) { + tmp->priority_set = true; + tmp->priority = existing->priority; + } - r = routing_policy_rule_attach(link->manager, tmp); - if (r < 0) - return r; + /* Copy state for logging below. */ + tmp->state = existing->state; + } - existing = tmp; - } else - existing->source = rule->source; - - log_routing_policy_rule_debug(existing, "Requesting", link, link->manager); + log_routing_policy_rule_debug(tmp, "Requesting", link, link->manager); r = link_queue_request_safe(link, REQUEST_TYPE_ROUTING_POLICY_RULE, - existing, NULL, + tmp, + routing_policy_rule_unref, routing_policy_rule_hash_func, routing_policy_rule_compare_func, routing_policy_rule_process_request, @@ -876,7 +906,11 @@ static int link_request_routing_policy_rule(Link *link, const RoutingPolicyRule if (r <= 0) return r; - routing_policy_rule_enter_requesting(existing); + routing_policy_rule_enter_requesting(tmp); + if (existing) + routing_policy_rule_enter_requesting(existing); + + TAKE_PTR(tmp); return 1; } @@ -947,8 +981,9 @@ static bool routing_policy_rule_is_created_by_kernel(const RoutingPolicyRule *ru int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) { _cleanup_(routing_policy_rule_unrefp) RoutingPolicyRule *tmp = NULL; + bool adjust_protocol = false, is_new = false; RoutingPolicyRule *rule = NULL; - bool adjust_protocol = false; + Request *req = NULL; uint16_t type; int r; @@ -1011,12 +1046,6 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Man } } - r = sd_rtnl_message_routing_policy_rule_get_flags(message, &tmp->flags); - if (r < 0) { - log_warning_errno(r, "rtnl: received rule message without valid flag, ignoring: %m"); - return 0; - } - r = sd_netlink_message_read_u32(message, FRA_FWMARK, &tmp->fwmark); if (r < 0 && r != -ENODATA) { log_warning_errno(r, "rtnl: could not get FRA_FWMARK attribute, ignoring: %m"); @@ -1135,41 +1164,57 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Man tmp->protocol = routing_policy_rule_is_created_by_kernel(tmp) ? RTPROT_KERNEL : RTPROT_STATIC; (void) routing_policy_rule_get(m, tmp, tmp->family, &rule); + (void) routing_policy_rule_get_request(m, tmp, tmp->family, &req); - switch (type) { - case RTM_NEWRULE: - if (rule) { - routing_policy_rule_enter_configured(rule); - log_routing_policy_rule_debug(rule, "Received remembered", NULL, m); - } else if (!m->manage_foreign_rules) { - routing_policy_rule_enter_configured(tmp); - log_routing_policy_rule_debug(tmp, "Ignoring received", NULL, m); - } else { - routing_policy_rule_enter_configured(tmp); - log_routing_policy_rule_debug(tmp, "Remembering", NULL, m); - r = routing_policy_rule_attach(m, tmp); - if (r < 0) { - log_warning_errno(r, "Could not remember foreign rule, ignoring: %m"); - return 0; - } - } - break; - case RTM_DELRULE: + if (type == RTM_DELRULE) { if (rule) { routing_policy_rule_enter_removed(rule); - if (rule->state == 0) { - log_routing_policy_rule_debug(rule, "Forgetting", NULL, m); - routing_policy_rule_detach(rule); - } else - log_routing_policy_rule_debug(rule, "Removed", NULL, m); + log_routing_policy_rule_debug(rule, "Forgetting removed", NULL, m); + routing_policy_rule_detach(rule); } else log_routing_policy_rule_debug(tmp, "Kernel removed unknown", NULL, m); - break; - default: - assert_not_reached(); + if (req) + routing_policy_rule_enter_removed(req->userdata); + + return 0; } + if (!rule) { + if (!req && !m->manage_foreign_rules) { + routing_policy_rule_enter_configured(tmp); + log_routing_policy_rule_debug(tmp, "Ignoring received", NULL, m); + return 0; + } + + /* If we did not know the rule, then save it. */ + r = routing_policy_rule_attach(m, tmp); + if (r < 0) { + log_warning_errno(r, "Failed to save received routing policy rule, ignoring: %m"); + return 0; + } + + rule = tmp; + is_new = true; + } + + /* Also update information that cannot be obtained through netlink notification. */ + if (req && req->waiting_reply) { + RoutingPolicyRule *req_rule = ASSERT_PTR(req->userdata); + + rule->source = req_rule->source; + } + + /* Then, update miscellaneous info from netlink notification. */ + r = sd_rtnl_message_routing_policy_rule_get_flags(message, &rule->flags); + if (r < 0) + log_debug_errno(r, "rtnl: received rule message without valid flag, ignoring: %m"); + + routing_policy_rule_enter_configured(rule); + if (req) + routing_policy_rule_enter_configured(req->userdata); + + log_routing_policy_rule_debug(rule, is_new ? "Remembering" : "Received remembered", NULL, m); return 1; } From 689438419bd0a18bb8408e2264b105426eb2566e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 20 Aug 2024 04:11:19 +0900 Subject: [PATCH 05/12] network/routing-policy-rule: anyway detach rule even when we fail to remove it When we fail to remove a rule, that mostly means the rule does not exist in the kernel anymore, e.g. already removed manually and we have not received notification about that yet. Let's detach the rule in that case. --- src/network/networkd-routing-policy-rule.c | 50 ++++++++++++++++------ 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 6ec81eec8c..5ce384f872 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -652,30 +652,55 @@ static int routing_policy_rule_set_netlink_message(const RoutingPolicyRule *rule return 0; } -static int routing_policy_rule_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, void *userdata) { +static int routing_policy_rule_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, RemoveRequest *rreq) { int r; assert(m); + assert(rreq); + + Manager *manager = ASSERT_PTR(rreq->manager); + RoutingPolicyRule *rule = ASSERT_PTR(rreq->userdata); r = sd_netlink_message_get_errno(m); - if (r < 0) - log_message_warning_errno(m, r, "Could not drop routing policy rule"); + if (r < 0) { + log_message_full_errno(m, + (r == -ENOENT || !rule->manager) ? LOG_DEBUG : LOG_WARNING, + r, "Could not drop routing policy rule, ignoring"); + + if (rule->manager) { + /* If the rule cannot be removed, then assume the rule is already removed. */ + log_routing_policy_rule_debug(rule, "Forgetting", NULL, manager); + + Request *req; + if (routing_policy_rule_get_request(manager, rule, rule->family, &req) >= 0) + routing_policy_rule_enter_removed(req->userdata); + + routing_policy_rule_detach(rule); + } + } return 1; } -static int routing_policy_rule_remove(RoutingPolicyRule *rule) { +static int routing_policy_rule_remove(RoutingPolicyRule *rule, Manager *manager) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; int r; assert(rule); - assert(rule->manager); - assert(rule->manager->rtnl); assert(IN_SET(rule->family, AF_INET, AF_INET6)); + assert(manager); + assert(manager->rtnl); - log_routing_policy_rule_debug(rule, "Removing", NULL, rule->manager); + /* If the rule is remembered, then use the remembered object. */ + (void) routing_policy_rule_get(manager, rule, rule->family, &rule); - r = sd_rtnl_message_new_routing_policy_rule(rule->manager->rtnl, &m, RTM_DELRULE, rule->family); + /* We cannot remove rules with the permanent flag. */ + if (FLAGS_SET(rule->flags, FIB_RULE_PERMANENT)) + return 0; + + log_routing_policy_rule_debug(rule, "Removing", NULL, manager); + + r = sd_rtnl_message_new_routing_policy_rule(manager->rtnl, &m, RTM_DELRULE, rule->family); if (r < 0) return log_warning_errno(r, "Could not allocate netlink message: %m"); @@ -683,11 +708,10 @@ static int routing_policy_rule_remove(RoutingPolicyRule *rule) { if (r < 0) return log_warning_errno(r, "Could not create netlink message: %m"); - r = netlink_call_async(rule->manager->rtnl, NULL, m, - routing_policy_rule_remove_handler, - NULL, NULL); + r = manager_remove_request_add(manager, rule, routing_policy_rule, + manager->rtnl, m, routing_policy_rule_remove_handler); if (r < 0) - return log_warning_errno(r, "Could not send netlink message: %m"); + return log_warning_errno(r, "Could not queue rtnetlink message: %m"); routing_policy_rule_enter_removing(rule); return 0; @@ -778,7 +802,7 @@ int manager_drop_routing_policy_rules_internal(Manager *m, bool foreign, const L if (!routing_policy_rule_is_marked(rule)) continue; - RET_GATHER(r, routing_policy_rule_remove(rule)); + RET_GATHER(r, routing_policy_rule_remove(rule, m)); } return r; From 727235006a7e8904f8493d32376cffa6969c5fa9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 20 Aug 2024 06:19:46 +0900 Subject: [PATCH 06/12] network/routing-policy-rule: remove rules that have conflicting flags The kernel does not distinguish rules with different flags in rule_exists(), but the flags of an existing rule cannot be updated. Let's remove rules that have conflicting flags, and configure new rules later with requested flags. --- src/network/networkd-routing-policy-rule.c | 48 +++++++++++++++++----- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 5ce384f872..460e621b2b 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -350,6 +350,20 @@ static bool routing_policy_rule_equal(const RoutingPolicyRule *a, const RoutingP return routing_policy_rule_compare_func_full(a, b, /* all = */ false) == 0; } +static bool routing_policy_rule_can_update(const RoutingPolicyRule *existing, const RoutingPolicyRule *requesting, int family) { + assert(existing); + assert(requesting); + + if (!routing_policy_rule_equal(existing, requesting, family, existing->priority)) + return false; + + /* These flags cannot be updated. */ + if ((existing->flags ^ requesting->flags) & (FIB_RULE_PERMANENT|FIB_RULE_INVERT)) + return false; + + return true; +} + static int routing_policy_rule_get(Manager *m, const RoutingPolicyRule *in, int family, RoutingPolicyRule **ret) { RoutingPolicyRule *rule; @@ -742,6 +756,23 @@ static int routing_policy_rule_configure(RoutingPolicyRule *rule, Link *link, Re return request_call_netlink_async(link->manager->rtnl, m, req); } +static void manager_unmark_routing_policy_rule(Manager *m, const RoutingPolicyRule *rule, int family) { + RoutingPolicyRule *existing; + + assert(m); + assert(rule); + assert(rule->family == AF_UNSPEC || rule->family == family); + assert(IN_SET(family, AF_INET, AF_INET6)); + + if (routing_policy_rule_get(m, rule, family, &existing) < 0) + return; + + if (!routing_policy_rule_can_update(existing, rule, rule->family)) + return; + + routing_policy_rule_unmark(existing); +} + static void manager_mark_routing_policy_rules(Manager *m, bool foreign, const Link *except) { RoutingPolicyRule *rule; Link *link; @@ -774,17 +805,12 @@ static void manager_mark_routing_policy_rules(Manager *m, bool foreign, const Li continue; HASHMAP_FOREACH(rule, link->network->rules_by_section) { - RoutingPolicyRule *existing; - - if (IN_SET(rule->family, AF_INET, AF_INET6)) { - if (routing_policy_rule_get(m, rule, rule->family, &existing) >= 0) - routing_policy_rule_unmark(existing); - } else { - if (routing_policy_rule_get(m, rule, AF_INET, &existing) >= 0) - routing_policy_rule_unmark(existing); - - if (routing_policy_rule_get(m, rule, AF_INET6, &existing) >= 0) - routing_policy_rule_unmark(existing); + if (IN_SET(rule->family, AF_INET, AF_INET6)) + manager_unmark_routing_policy_rule(m, rule, rule->family); + else { + assert(rule->address_family == ADDRESS_FAMILY_YES); + manager_unmark_routing_policy_rule(m, rule, AF_INET); + manager_unmark_routing_policy_rule(m, rule, AF_INET6); } } } From 7275739728fe6649ed5dd4baacecf38728e04f12 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 20 Aug 2024 06:56:53 +0900 Subject: [PATCH 07/12] sd-netlink: introduce sd_netlink_message_read_u64() --- src/libsystemd/sd-netlink/netlink-message.c | 28 +++++++++++++++++++++ src/systemd/sd-netlink.h | 1 + 2 files changed, 29 insertions(+) diff --git a/src/libsystemd/sd-netlink/netlink-message.c b/src/libsystemd/sd-netlink/netlink-message.c index 49d000d47e..25bcbd9640 100644 --- a/src/libsystemd/sd-netlink/netlink-message.c +++ b/src/libsystemd/sd-netlink/netlink-message.c @@ -928,6 +928,34 @@ int sd_netlink_message_read_u32(sd_netlink_message *m, uint16_t attr_type, uint3 return 0; } +int sd_netlink_message_read_u64(sd_netlink_message *m, uint16_t attr_type, uint64_t *ret) { + void *attr_data; + bool net_byteorder; + int r; + + assert_return(m, -EINVAL); + + r = message_attribute_has_type(m, NULL, attr_type, NETLINK_TYPE_U64); + if (r < 0) + return r; + + r = netlink_message_read_internal(m, attr_type, &attr_data, &net_byteorder); + if (r < 0) + return r; + + if ((size_t) r < sizeof(uint64_t)) + return -EIO; + + if (ret) { + if (net_byteorder) + *ret = be64toh(*(uint64_t *) attr_data); + else + *ret = *(uint64_t *) attr_data; + } + + return 0; +} + int sd_netlink_message_read_ether_addr(sd_netlink_message *m, uint16_t attr_type, struct ether_addr *data) { void *attr_data; int r; diff --git a/src/systemd/sd-netlink.h b/src/systemd/sd-netlink.h index 34db2bbf1c..b091725510 100644 --- a/src/systemd/sd-netlink.h +++ b/src/systemd/sd-netlink.h @@ -112,6 +112,7 @@ int sd_netlink_message_read_strv(sd_netlink_message *m, uint16_t container_type, int sd_netlink_message_read_u8(sd_netlink_message *m, uint16_t attr_type, uint8_t *data); int sd_netlink_message_read_u16(sd_netlink_message *m, uint16_t attr_type, uint16_t *data); int sd_netlink_message_read_u32(sd_netlink_message *m, uint16_t attr_type, uint32_t *data); +int sd_netlink_message_read_u64(sd_netlink_message *m, uint16_t attr_type, uint64_t *ret); int sd_netlink_message_read_ether_addr(sd_netlink_message *m, uint16_t attr_type, struct ether_addr *data); int sd_netlink_message_read_cache_info(sd_netlink_message *m, uint16_t attr_type, struct ifa_cacheinfo *info); int sd_netlink_message_read_in_addr(sd_netlink_message *m, uint16_t attr_type, struct in_addr *data); From 3ac8148e2005cc90932d1ea1ef9c7275642d6459 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 20 Aug 2024 07:00:44 +0900 Subject: [PATCH 08/12] network/routing-policy-rule: also manage remaining attributes Currently, these attributes are not configured by us, but there may be a existing rule created by user manually with one of these attribute. To correctly manage such foreign rules, let's read these attributes. --- src/network/networkd-routing-policy-rule.c | 52 +++++++++++++++++++++- src/network/networkd-routing-policy-rule.h | 6 +-- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 460e621b2b..e7c048f273 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -212,7 +212,7 @@ static void routing_policy_rule_hash_func(const RoutingPolicyRule *rule, struct siphash24_compress_typesafe(rule->suppress_ifgroup, state); siphash24_compress_typesafe(rule->suppress_prefixlen, state); siphash24_compress_typesafe(rule->fwmask, state); - /* FRA_TUN_ID */ + siphash24_compress_typesafe(rule->tunnel_id, state); /* fr_net (network namespace) */ siphash24_compress_typesafe(rule->l3mdev, state); siphash24_compress_typesafe(rule->uid_range, state); @@ -225,7 +225,7 @@ static void routing_policy_rule_hash_func(const RoutingPolicyRule *rule, struct siphash24_compress_typesafe(rule->from_prefixlen, state); siphash24_compress_typesafe(rule->to_prefixlen, state); siphash24_compress_typesafe(rule->tos, state); - /* FRA_FLOW (IPv4 only) */ + siphash24_compress_typesafe(rule->realms, state); in_addr_hash_func(&rule->from, rule->family, state); in_addr_hash_func(&rule->to, rule->family, state); } @@ -280,6 +280,10 @@ static int routing_policy_rule_compare_func_full(const RoutingPolicyRule *a, con if (r != 0) return r; + r = CMP(a->tunnel_id, b->tunnel_id); + if (r != 0) + return r; + r = CMP(a->l3mdev, b->l3mdev); if (r != 0) return r; @@ -316,6 +320,10 @@ static int routing_policy_rule_compare_func_full(const RoutingPolicyRule *a, con if (r != 0) return r; + r = CMP(a->realms, b->realms); + if (r != 0) + return r; + if (all) { r = memcmp(&a->from, &b->from, FAMILY_ADDRESS_SIZE(a->family)); if (r != 0) @@ -361,6 +369,10 @@ static bool routing_policy_rule_can_update(const RoutingPolicyRule *existing, co if ((existing->flags ^ requesting->flags) & (FIB_RULE_PERMANENT|FIB_RULE_INVERT)) return false; + /* GOTO target cannot be updated. */ + if (existing->type == FR_ACT_GOTO && existing->priority_goto != requesting->priority_goto) + return false; + return true; } @@ -663,6 +675,24 @@ static int routing_policy_rule_set_netlink_message(const RoutingPolicyRule *rule if (r < 0) return r; + if (rule->type == FR_ACT_GOTO) { + r = sd_netlink_message_append_u32(m, FRA_GOTO, rule->priority_goto); + if (r < 0) + return r; + } + + if (rule->realms > 0) { + r = sd_netlink_message_append_u32(m, FRA_FLOW, rule->realms); + if (r < 0) + return r; + } + + if (rule->tunnel_id > 0) { + r = sd_netlink_message_append_u64(m, FRA_TUN_ID, htobe64(rule->tunnel_id)); + if (r < 0) + return r; + } + return 0; } @@ -1208,6 +1238,20 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Man if (r >= 0) tmp->suppress_ifgroup = (int32_t) suppress_ifgroup; + r = sd_netlink_message_read_u64(message, FRA_TUN_ID, &tmp->tunnel_id); + if (r < 0 && r != -ENODATA) { + log_warning_errno(r, "rtnl: could not get FRA_TUN_ID attribute, ignoring: %m"); + return 0; + } + if (r >= 0) + tmp->tunnel_id = be64toh(tmp->tunnel_id); + + r = sd_netlink_message_read_u32(message, FRA_FLOW, &tmp->realms); + if (r < 0 && r != -ENODATA) { + log_warning_errno(r, "rtnl: could not get FRA_FLOW attribute, ignoring: %m"); + return 0; + } + if (adjust_protocol) /* As .network files does not have setting to specify protocol, we can assume the * protocol of the received rule is RTPROT_KERNEL or RTPROT_STATIC. */ @@ -1260,6 +1304,10 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Man if (r < 0) log_debug_errno(r, "rtnl: received rule message without valid flag, ignoring: %m"); + r = sd_netlink_message_read_u32(message, FRA_GOTO, &rule->priority_goto); + if (r < 0 && r != -ENODATA) + log_debug_errno(r, "rtnl: could not get FRA_GOTO attribute, ignoring: %m"); + routing_policy_rule_enter_configured(rule); if (req) routing_policy_rule_enter_configured(req->userdata); diff --git a/src/network/networkd-routing-policy-rule.h b/src/network/networkd-routing-policy-rule.h index 7047cc4e9b..3ca5e73934 100644 --- a/src/network/networkd-routing-policy-rule.h +++ b/src/network/networkd-routing-policy-rule.h @@ -35,12 +35,12 @@ typedef struct RoutingPolicyRule { union in_addr_union to; /* FRA_DST */ union in_addr_union from; /* FRA_SRC */ char *iif; /* FRA_IIFNAME */ - /* FRA_GOTO */ + uint32_t priority_goto; /* FRA_GOTO */ bool priority_set; uint32_t priority; /* FRA_PRIORITY */ uint32_t fwmark; /* FRA_FWMARK */ - /* FRA_FLOW */ - /* FRA_TUN_ID */ + uint32_t realms; /* FRA_FLOW (IPv4 only) */ + uint64_t tunnel_id; /* FRA_TUN_ID */ int32_t suppress_ifgroup; /* FRA_SUPPRESS_IFGROUP */ int32_t suppress_prefixlen; /* FRA_SUPPRESS_PREFIXLEN */ uint32_t table; /* FRA_TABLE, also used in struct fib_rule_hdr */ From ac1d8aa5bccda598d357b993bd7c9cc99ae04707 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 20 Aug 2024 05:16:53 +0900 Subject: [PATCH 09/12] network/routing-policy-rule: support all known type of rule This also adds GoTo= to specify the target priority of goto rule. Note, table was the default but could not be specified in Type=. --- man/systemd.network.xml | 18 +++++- src/network/networkd-json.c | 2 +- src/network/networkd-network-gperf.gperf | 1 + src/network/networkd-routing-policy-rule.c | 71 +++++++++++++++++++--- src/network/networkd-routing-policy-rule.h | 4 +- 5 files changed, 83 insertions(+), 13 deletions(-) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index 0374dc7cdc..6273274e7a 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -1728,6 +1728,18 @@ NFTSet=prefix:netdev:filter:eth_ipv4_prefix + + GoTo= + + Specifies the target priority used by goto type of rule. Takes an integer + in the range 1…4294967295. This must be larger than the priority of this rule specified in + Priority=. When specified, Type=goto is implied. This is + mandatory when Type=goto. + + + + + IncomingInterface= @@ -1853,8 +1865,10 @@ NFTSet=prefix:netdev:filter:eth_ipv4_prefix Type= Specifies Routing Policy Database (RPDB) rule type. Takes one of - blackhole, unreachable or prohibit. - + table, goto, nop, + blackhole, unreachable, or prohibit. + When goto, the target priority must be specified in GoTo=. + Defaults to table. diff --git a/src/network/networkd-json.c b/src/network/networkd-json.c index 0a57e6aee0..4eafc62c7e 100644 --- a/src/network/networkd-json.c +++ b/src/network/networkd-json.c @@ -308,7 +308,7 @@ static int routing_policy_rule_append_json(RoutingPolicyRule *rule, sd_json_vari SD_JSON_BUILD_PAIR_STRING("ProtocolString", protocol), SD_JSON_BUILD_PAIR_UNSIGNED("TOS", rule->tos), SD_JSON_BUILD_PAIR_UNSIGNED("Type", rule->type), - SD_JSON_BUILD_PAIR_STRING("TypeString", fr_act_type_full_to_string(rule->type)), + SD_JSON_BUILD_PAIR_STRING("TypeString", fr_act_type_to_string(rule->type)), SD_JSON_BUILD_PAIR_UNSIGNED("IPProtocol", rule->ipproto), SD_JSON_BUILD_PAIR_STRING("IPProtocolString", ip_protocol_to_name(rule->ipproto)), SD_JSON_BUILD_PAIR_UNSIGNED("Priority", rule->priority), diff --git a/src/network/networkd-network-gperf.gperf b/src/network/networkd-network-gperf.gperf index aa849fe535..b2794f9efd 100644 --- a/src/network/networkd-network-gperf.gperf +++ b/src/network/networkd-network-gperf.gperf @@ -178,6 +178,7 @@ Neighbor.LinkLayerAddress, config_parse_neighbor_lladdr, Neighbor.MACAddress, config_parse_neighbor_lladdr, 0, 0 /* deprecated */ RoutingPolicyRule.TypeOfService, config_parse_routing_policy_rule_tos, 0, 0 RoutingPolicyRule.Priority, config_parse_routing_policy_rule_priority, 0, 0 +RoutingPolicyRule.GoTo, config_parse_routing_policy_rule_goto, 0, 0 RoutingPolicyRule.Table, config_parse_routing_policy_rule_table, 0, 0 RoutingPolicyRule.FirewallMark, config_parse_routing_policy_rule_fwmark_mask, 0, 0 RoutingPolicyRule.From, config_parse_routing_policy_rule_prefix, 0, 0 diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index e7c048f273..44d85d000f 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -25,12 +25,6 @@ #include "user-util.h" static const char *const fr_act_type_table[__FR_ACT_MAX] = { - [FR_ACT_BLACKHOLE] = "blackhole", - [FR_ACT_UNREACHABLE] = "unreachable", - [FR_ACT_PROHIBIT] = "prohibit", -}; - -static const char *const fr_act_type_full_table[__FR_ACT_MAX] = { [FR_ACT_TO_TBL] = "table", [FR_ACT_GOTO] = "goto", [FR_ACT_NOP] = "nop", @@ -40,8 +34,7 @@ static const char *const fr_act_type_full_table[__FR_ACT_MAX] = { }; assert_cc(__FR_ACT_MAX <= UINT8_MAX); -DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING(fr_act_type, int); -DEFINE_STRING_TABLE_LOOKUP_TO_STRING(fr_act_type_full, int); +DEFINE_STRING_TABLE_LOOKUP(fr_act_type, int); static RoutingPolicyRule* routing_policy_rule_detach_impl(RoutingPolicyRule *rule) { assert(rule); @@ -519,7 +512,8 @@ static int routing_policy_rule_acquire_priority(Manager *manager, RoutingPolicyR return r; } - for (priority = 32765; priority > 0; priority--) + /* priority must be smaller than goto target */ + for (priority = rule->type == FR_ACT_GOTO ? rule->priority_goto - 1 : 32765; priority > 0; priority--) if (!set_contains(priorities, UINT32_TO_PTR(priority))) break; @@ -1432,6 +1426,48 @@ int config_parse_routing_policy_rule_priority( return 0; } +int config_parse_routing_policy_rule_goto( + 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_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; + Network *network = ASSERT_PTR(userdata); + uint32_t priority; + int r; + + assert(filename); + assert(lvalue); + assert(rvalue); + + r = routing_policy_rule_new_static(network, filename, section_line, &n); + if (r < 0) + return log_oom(); + + r = safe_atou32(rvalue, &priority); + if (r < 0) { + log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse %s=%s, ignoring assignment: %m", lvalue, rvalue); + return 0; + } + if (priority <= 0) { + log_syntax(unit, LOG_WARNING, filename, line, 0, "Invalid goto target priority, ignoring assignment."); + return 0; + } + + n->type = FR_ACT_GOTO; + n->priority_goto = priority; + + TAKE_PTR(n); + return 0; +} + int config_parse_routing_policy_rule_table( const char *unit, const char *filename, @@ -1988,6 +2024,23 @@ static int routing_policy_rule_section_verify(RoutingPolicyRule *rule) { if (rule->l3mdev) rule->table = RT_TABLE_UNSPEC; + if (rule->type == FR_ACT_GOTO) { + if (rule->priority_goto <= 0) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: Type=goto is specified but the target priority GoTo= is unspecified. " + "Ignoring [RoutingPolicyRule] section from line %u.", + rule->section->filename, + rule->section->line); + + if (rule->priority_set && rule->priority >= rule->priority_goto) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: goto target priority %"PRIu32" must be larger than the priority of this rule %"PRIu32". " + "Ignoring [RoutingPolicyRule] section from line %u.", + rule->section->filename, + rule->priority_goto, rule->priority, + rule->section->line); + } + return 0; } diff --git a/src/network/networkd-routing-policy-rule.h b/src/network/networkd-routing-policy-rule.h index 3ca5e73934..c7a0512b44 100644 --- a/src/network/networkd-routing-policy-rule.h +++ b/src/network/networkd-routing-policy-rule.h @@ -54,7 +54,8 @@ typedef struct RoutingPolicyRule { struct fib_rule_port_range dport; /* FRA_DPORT_RANGE */ } RoutingPolicyRule; -const char* fr_act_type_full_to_string(int t) _const_; +int fr_act_type_from_string(const char *s) _pure_; +const char* fr_act_type_to_string(int t) _const_; RoutingPolicyRule* routing_policy_rule_ref(RoutingPolicyRule *rule); RoutingPolicyRule* routing_policy_rule_unref(RoutingPolicyRule *rule); @@ -81,6 +82,7 @@ CONFIG_PARSER_PROTOTYPE(config_parse_routing_policy_rule_table); CONFIG_PARSER_PROTOTYPE(config_parse_routing_policy_rule_fwmark_mask); CONFIG_PARSER_PROTOTYPE(config_parse_routing_policy_rule_prefix); CONFIG_PARSER_PROTOTYPE(config_parse_routing_policy_rule_priority); +CONFIG_PARSER_PROTOTYPE(config_parse_routing_policy_rule_goto); CONFIG_PARSER_PROTOTYPE(config_parse_routing_policy_rule_device); CONFIG_PARSER_PROTOTYPE(config_parse_routing_policy_rule_l3mdev); CONFIG_PARSER_PROTOTYPE(config_parse_routing_policy_rule_port_range); From 936dec4337db67c5d3cfbb8e6b8d73fa1c5bd27f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 20 Aug 2024 20:56:35 +0900 Subject: [PATCH 10/12] test-network: do not pass '[detached]' to 'ip rule del' That indicates the interface name in 'iif' or 'oif' cannot be resolved when 'ip rule' command is invoked. That's natural when networkd fail to remove rule but the corresponding interface is already removed. To make not the residual rules interfere subsequent test cases, let's ignore the flag and actually remove unwanted rules. --- test/test-network/systemd-networkd-tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 3a30c8bfd1..c88f75b488 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -645,7 +645,7 @@ def flush_routing_policy_rules(): have = True print(f'### Removing IPv{ipv} routing policy rules that did not exist when the test started.') print(f'# {line}') - words = line.replace('lookup [l3mdev-table]', 'l3mdev').split() + words = line.replace('lookup [l3mdev-table]', 'l3mdev').replace('[detached]', '').split() priority = words[0].rstrip(':') call(f'ip -{ipv} rule del priority {priority} ' + ' '.join(words[1:])) From 49454d9ced87d53ada9a3a2860ce1dbd4a1da2b0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 20 Aug 2024 10:07:10 +0900 Subject: [PATCH 11/12] test-network: add tests for Type=table, goto, and nop --- .../conf/25-routing-policy-rule-test1.network | 16 +++++ test/test-network/systemd-networkd-tests.py | 70 +++++++++---------- 2 files changed, 49 insertions(+), 37 deletions(-) diff --git a/test/test-network/conf/25-routing-policy-rule-test1.network b/test/test-network/conf/25-routing-policy-rule-test1.network index 7d6e17cf6c..2cec4334fd 100644 --- a/test/test-network/conf/25-routing-policy-rule-test1.network +++ b/test/test-network/conf/25-routing-policy-rule-test1.network @@ -27,7 +27,23 @@ Priority=101 Family=both [RoutingPolicyRule] +Type=table IncomingInterface=test1 From=0.0.0.0/8 Table=10 Priority=102 + +[RoutingPolicyRule] +Type=goto +IncomingInterface=test1 +From=10.0.0.0/16 +Priority=103 +GoTo=111 +Table=11 + +[RoutingPolicyRule] +Type=nop +IncomingInterface=test1 +From=10.1.0.0/16 +Priority=104 +Table=12 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index c88f75b488..fbe5e758ec 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -3210,41 +3210,42 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): self.assertNotRegex(output, '192.168.0.1') self.assertRegex(output, routable_map[carrier]) + def check_routing_policy_rule_test1(self): + output = check_output('ip rule list iif test1 priority 111') + print(output) + self.assertRegex(output, r'111: from 192.168.100.18 tos (0x08|throughput) iif test1 oif test1 lookup 7') + + output = check_output('ip -6 rule list iif test1 priority 100') + print(output) + self.assertIn('100: from all iif test1 lookup 8', output) + + output = check_output('ip rule list iif test1 priority 101') + print(output) + self.assertIn('101: from all iif test1 lookup 9', output) + + output = check_output('ip rule list iif test1 priority 102') + print(output) + self.assertIn('102: from 0.0.0.0/8 iif test1 lookup 10', output) + + output = check_output('ip rule list iif test1 priority 103') + print(output) + self.assertIn('103: from 10.0.0.0/16 iif test1 lookup 11 goto 111', output) + + output = check_output('ip rule list iif test1 priority 104') + print(output) + self.assertIn('104: from 10.1.0.0/16 iif test1 lookup 12 nop', output) + + def check_routing_policy_rule_dummy98(self): + output = check_output('ip rule list table 8') + print(output) + self.assertRegex(output, r'112: from 192.168.101.18 tos (0x08|throughput) iif dummy98 oif dummy98 lookup 8') + def test_routing_policy_rule(self): copy_network_unit('25-routing-policy-rule-test1.network', '11-dummy.netdev') start_networkd() self.wait_online('test1:degraded') - output = check_output('ip rule list iif test1 priority 111') - print(output) - self.assertRegex(output, '111:') - self.assertRegex(output, 'from 192.168.100.18') - self.assertRegex(output, r'tos (0x08|throughput)\s') - self.assertRegex(output, 'iif test1') - self.assertRegex(output, 'oif test1') - self.assertRegex(output, 'lookup 7') - - output = check_output('ip rule list iif test1 priority 101') - print(output) - self.assertRegex(output, '101:') - self.assertRegex(output, 'from all') - self.assertRegex(output, 'iif test1') - self.assertRegex(output, 'lookup 9') - - output = check_output('ip -6 rule list iif test1 priority 100') - print(output) - self.assertRegex(output, '100:') - self.assertRegex(output, 'from all') - self.assertRegex(output, 'iif test1') - self.assertRegex(output, 'lookup 8') - - output = check_output('ip rule list iif test1 priority 102') - print(output) - self.assertRegex(output, '102:') - self.assertRegex(output, 'from 0.0.0.0/8') - self.assertRegex(output, 'iif test1') - self.assertRegex(output, 'lookup 10') - + self.check_routing_policy_rule_test1() check_json(networkctl_json()) def test_routing_policy_rule_issue_11280(self): @@ -3255,13 +3256,8 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): restart_networkd(show_logs=(trial > 0)) self.wait_online('test1:degraded', 'dummy98:degraded') - output = check_output('ip rule list table 7') - print(output) - self.assertRegex(output, '111: from 192.168.100.18 tos (0x08|throughput) iif test1 oif test1 lookup 7') - - output = check_output('ip rule list table 8') - print(output) - self.assertRegex(output, '112: from 192.168.101.18 tos (0x08|throughput) iif dummy98 oif dummy98 lookup 8') + self.check_routing_policy_rule_test1() + self.check_routing_policy_rule_dummy98() def test_routing_policy_rule_reconfigure(self): copy_network_unit('25-routing-policy-rule-reconfigure2.network', '11-dummy.netdev') From 085818569b89e72e4d7b6dfed3b3f3230be0fe9e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 20 Aug 2024 10:33:42 +0900 Subject: [PATCH 12/12] test-network: add test for ManageForeignRoutingPolicyRules= --- .../networkd-manage-foreign-rules-no.conf | 3 ++ test/test-network/systemd-networkd-tests.py | 38 ++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 test/test-network/conf/networkd-manage-foreign-rules-no.conf diff --git a/test/test-network/conf/networkd-manage-foreign-rules-no.conf b/test/test-network/conf/networkd-manage-foreign-rules-no.conf new file mode 100644 index 0000000000..b376889fc8 --- /dev/null +++ b/test/test-network/conf/networkd-manage-foreign-rules-no.conf @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Network] +ManageForeignRoutingPolicyRules=no diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index fbe5e758ec..b2aadde36c 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -3240,14 +3240,50 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): print(output) self.assertRegex(output, r'112: from 192.168.101.18 tos (0x08|throughput) iif dummy98 oif dummy98 lookup 8') - def test_routing_policy_rule(self): + def _test_routing_policy_rule(self, manage_foreign_routes): + if not manage_foreign_routes: + copy_networkd_conf_dropin('networkd-manage-foreign-rules-no.conf') copy_network_unit('25-routing-policy-rule-test1.network', '11-dummy.netdev') + + stop_networkd() + + check_output('ip -4 rule add priority 20001 table 9999 from 10.10.0.0/16') + check_output('ip -6 rule add priority 20001 table 9999 from 2001:db8:0:1::/64') + start_networkd() self.wait_online('test1:degraded') self.check_routing_policy_rule_test1() check_json(networkctl_json()) + output = check_output('ip -4 rule list priority 20001 table 9999 from 10.10.0.0/16') + print(output) + if manage_foreign_routes: + self.assertEqual(output, '') + else: + self.assertIn(output, '20001: from 10.10.0.0/16 lookup 9999') + check_output('ip -4 rule del priority 20001 table 9999 from 10.10.0.0/16') + + output = check_output('ip -6 rule list priority 20001 table 9999 from 2001:db8:0:1::/64') + print(output) + if manage_foreign_routes: + self.assertEqual(output, '') + else: + self.assertIn(output, '20001: from 2001:db8:0:1::/64 lookup 9999') + check_output('ip -6 rule del priority 20001 table 9999 from 2001:db8:0:1::/64') + + def test_routing_policy_rule(self): + first = True + for manage_foreign_routes in [True, False]: + if first: + first = False + else: + self.tearDown() + + print(f'### test_routing_policy_rule(manage_foreign_routes={manage_foreign_routes})') + with self.subTest(manage_foreign_routes=manage_foreign_routes): + self._test_routing_policy_rule(manage_foreign_routes) + def test_routing_policy_rule_issue_11280(self): copy_network_unit('25-routing-policy-rule-test1.network', '11-dummy.netdev', '25-routing-policy-rule-dummy98.network', '12-dummy.netdev')