Projects
Mega:23.03
systemd
_service:tar_scm:backport-network-do-not-assume...
Sign Up
Log In
Username
Password
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File _service:tar_scm:backport-network-do-not-assume-the-highest-priority-when-Prio.patch of Package systemd
From c5ff3ea39882609b307c4a9925d1c17413d17dfc Mon Sep 17 00:00:00 2001 From: Yu Watanabe <watanabe.yu+github@gmail.com> Date: Tue, 17 Aug 2021 14:03:19 +0900 Subject: [PATCH] network: do not assume the highest priority when Priority= is unspecified Previously, when Priority= is unspecified, networkd configured the rule with the highest (=0) priority. This commit makes networkd distinguish the case the setting is unspecified and one explicitly specified as Priority=0. Note. 1) If the priority is unspecified on configure, then kernel dynamically picks a priority for the rule. 2) The new behavior is consistent with 'ip rule' command. Replaces #15606. (cherry picked from commit c4f7a347566b8926382029593b4d9957fef2564c) Conflict:NA Reference:https://github.com/systemd/systemd/commit/c5ff3ea39882609b307c4a9925d1c17413d17dfc --- man/systemd.network.xml | 4 +- src/network/networkd-routing-policy-rule.c | 120 +++++++++++++++++--- src/network/networkd-routing-policy-rule.h | 1 + test/test-network/systemd-networkd-tests.py | 2 +- 4 files changed, 110 insertions(+), 17 deletions(-) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index 3b7680eb8b..9de9816ced 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -1238,7 +1238,9 @@ IPv6Token=prefixstable:2002:da8:1::</programlisting></para> <term><varname>Priority=</varname></term> <listitem> <para>Specifies the priority of this rule. <varname>Priority=</varname> is an unsigned - integer. Higher number means lower priority, and rules get processed in order of increasing number.</para> + integer in the range 0…4294967295. Higher number means lower priority, and rules get + processed in order of increasing number. Defaults to unset, and the kernel will pick + a value dynamically.</para> </listitem> </varlistentry> <varlistentry> diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index af7e8a973c..03ccbd8e85 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -163,7 +163,9 @@ void routing_policy_rule_hash_func(const RoutingPolicyRule *rule, struct siphash siphash24_compress(&rule->type, sizeof(rule->type), state); siphash24_compress(&rule->fwmark, sizeof(rule->fwmark), state); siphash24_compress(&rule->fwmask, sizeof(rule->fwmask), state); - siphash24_compress(&rule->priority, sizeof(rule->priority), state); + siphash24_compress_boolean(rule->priority_set, state); + if (rule->priority_set) + siphash24_compress(&rule->priority, sizeof(rule->priority), state); siphash24_compress(&rule->table, sizeof(rule->table), state); siphash24_compress(&rule->suppress_prefixlen, sizeof(rule->suppress_prefixlen), state); @@ -229,10 +231,16 @@ int routing_policy_rule_compare_func(const RoutingPolicyRule *a, const RoutingPo if (r != 0) return r; - r = CMP(a->priority, b->priority); + r = CMP(a->priority_set, b->priority_set); if (r != 0) return r; + if (a->priority_set) { + r = CMP(a->priority, b->priority); + if (r != 0) + return r; + } + r = CMP(a->table, b->table); if (r != 0) return r; @@ -293,8 +301,9 @@ DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR( routing_policy_rule_compare_func, routing_policy_rule_free); -static int routing_policy_rule_get(Manager *m, const RoutingPolicyRule *rule, RoutingPolicyRule **ret) { +static int routing_policy_rule_get(Manager *m, const RoutingPolicyRule *rule, bool require_priority, RoutingPolicyRule **ret) { RoutingPolicyRule *existing; + int r; assert(m); @@ -312,6 +321,23 @@ static int routing_policy_rule_get(Manager *m, const RoutingPolicyRule *rule, Ro return 0; } + if (!require_priority && rule->priority_set) { + _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *tmp = NULL; + + r = routing_policy_rule_dup(rule, &tmp); + if (r < 0) + return r; + + tmp->priority_set = false; + + existing = set_get(m->rules, tmp); + if (existing) { + if (ret) + *ret = existing; + return 1; + } + } + return -ENOENT; } @@ -328,7 +354,7 @@ static int routing_policy_rule_add(Manager *m, const RoutingPolicyRule *in, Rout if (r < 0) return r; - r = routing_policy_rule_get(m, rule, &existing); + r = routing_policy_rule_get(m, rule, true, &existing); if (r == -ENOENT) { /* Rule does not exist, use a new one. */ r = set_ensure_put(&m->rules, &routing_policy_rule_hash_ops, rule); @@ -371,6 +397,32 @@ static int routing_policy_rule_consume_foreign(Manager *m, RoutingPolicyRule *ru return 1; } +static int routing_policy_rule_update_priority(RoutingPolicyRule *rule, uint32_t priority) { + int r; + + assert(rule); + assert(rule->manager); + + if (rule->priority_set) + return 0; + + if (!set_remove(rule->manager->rules, rule)) + return -ENOENT; + + rule->priority = priority; + rule->priority_set = true; + + r = set_put(rule->manager->rules, rule); + if (r <= 0) { + /* Undo */ + rule->priority_set = false; + assert_se(set_put(rule->manager->rules, rule) > 0); + return r == 0 ? -EEXIST : r; + } + + return 1; +} + static void log_routing_policy_rule_debug(const RoutingPolicyRule *rule, const char *str, const Link *link, const Manager *m) { _cleanup_free_ char *from = NULL, *to = NULL, *table = NULL; @@ -422,9 +474,11 @@ static int routing_policy_rule_set_netlink_message(const RoutingPolicyRule *rule return log_link_error_errno(link, r, "Could not set destination prefix length: %m"); } - r = sd_netlink_message_append_u32(m, FRA_PRIORITY, rule->priority); - if (r < 0) - return log_link_error_errno(link, r, "Could not append FRA_PRIORITY attribute: %m"); + if (rule->priority_set) { + r = sd_netlink_message_append_u32(m, FRA_PRIORITY, rule->priority); + if (r < 0) + return log_link_error_errno(link, r, "Could not append FRA_PRIORITY attribute: %m"); + } if (rule->tos > 0) { r = sd_rtnl_message_routing_policy_rule_set_tos(m, rule->tos); @@ -662,6 +716,28 @@ int manager_drop_routing_policy_rules_internal(Manager *m, bool foreign, const L continue; } + if (!foreign) { + _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *tmp = NULL; + + /* The rule may be configured without priority. Try to find without priority. */ + + k = routing_policy_rule_dup(rule, &tmp); + if (k < 0) { + if (r >= 0) + r = k; + continue; + } + + tmp->priority_set = false; + + k = links_have_routing_policy_rule(m, tmp, except); + if (k != 0) { + if (k < 0 && r >= 0) + r = k; + continue; + } + } + k = routing_policy_rule_remove(rule, m); if (k < 0 && r >= 0) r = k; @@ -821,11 +897,11 @@ int request_process_routing_policy_rule(Request *req) { } static const RoutingPolicyRule kernel_rules[] = { - { .family = AF_INET, .priority = 0, .table = RT_TABLE_LOCAL, .type = FR_ACT_TO_TBL, .uid_range.start = UID_INVALID, .uid_range.end = UID_INVALID, .suppress_prefixlen = -1, }, - { .family = AF_INET, .priority = 32766, .table = RT_TABLE_MAIN, .type = FR_ACT_TO_TBL, .uid_range.start = UID_INVALID, .uid_range.end = UID_INVALID, .suppress_prefixlen = -1, }, - { .family = AF_INET, .priority = 32767, .table = RT_TABLE_DEFAULT, .type = FR_ACT_TO_TBL, .uid_range.start = UID_INVALID, .uid_range.end = UID_INVALID, .suppress_prefixlen = -1, }, - { .family = AF_INET6, .priority = 0, .table = RT_TABLE_LOCAL, .type = FR_ACT_TO_TBL, .uid_range.start = UID_INVALID, .uid_range.end = UID_INVALID, .suppress_prefixlen = -1, }, - { .family = AF_INET6, .priority = 32766, .table = RT_TABLE_MAIN, .type = FR_ACT_TO_TBL, .uid_range.start = UID_INVALID, .uid_range.end = UID_INVALID, .suppress_prefixlen = -1, }, + { .family = AF_INET, .priority_set = true, .priority = 0, .table = RT_TABLE_LOCAL, .type = FR_ACT_TO_TBL, .uid_range.start = UID_INVALID, .uid_range.end = UID_INVALID, .suppress_prefixlen = -1, }, + { .family = AF_INET, .priority_set = true, .priority = 32766, .table = RT_TABLE_MAIN, .type = FR_ACT_TO_TBL, .uid_range.start = UID_INVALID, .uid_range.end = UID_INVALID, .suppress_prefixlen = -1, }, + { .family = AF_INET, .priority_set = true, .priority = 32767, .table = RT_TABLE_DEFAULT, .type = FR_ACT_TO_TBL, .uid_range.start = UID_INVALID, .uid_range.end = UID_INVALID, .suppress_prefixlen = -1, }, + { .family = AF_INET6, .priority_set = true, .priority = 0, .table = RT_TABLE_LOCAL, .type = FR_ACT_TO_TBL, .uid_range.start = UID_INVALID, .uid_range.end = UID_INVALID, .suppress_prefixlen = -1, }, + { .family = AF_INET6, .priority_set = true, .priority = 32766, .table = RT_TABLE_MAIN, .type = FR_ACT_TO_TBL, .uid_range.start = UID_INVALID, .uid_range.end = UID_INVALID, .suppress_prefixlen = -1, }, }; static bool routing_policy_rule_is_created_by_kernel(const RoutingPolicyRule *rule) { @@ -936,6 +1012,9 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Man log_warning_errno(r, "rtnl: could not get FRA_PRIORITY attribute, ignoring: %m"); return 0; } + /* The kernel does not send priority if priority is zero. So, the flag below must be always set + * even if the message does not contain FRA_PRIORITY. */ + tmp->priority_set = true; r = sd_netlink_message_read_u32(message, FRA_TABLE, &tmp->table); if (r < 0 && r != -ENODATA) { @@ -1027,13 +1106,16 @@ 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, false, &rule); switch (type) { case RTM_NEWRULE: - if (rule) + if (rule) { log_routing_policy_rule_debug(tmp, "Received remembered", NULL, m); - else if (!m->manage_foreign_routes) + r = routing_policy_rule_update_priority(rule, tmp->priority); + if (r < 0) + log_warning_errno(r, "Failed to update priority of remembered routing policy rule, ignoring: %m"); + } else if (!m->manage_foreign_routes) log_routing_policy_rule_debug(tmp, "Ignoring received foreign", NULL, m); else { log_routing_policy_rule_debug(tmp, "Remembering foreign", NULL, m); @@ -1155,11 +1237,19 @@ int config_parse_routing_policy_rule_priority( if (r < 0) return log_oom(); + if (isempty(rvalue)) { + n->priority = 0; + n->priority_set = false; + TAKE_PTR(n); + return 0; + } + r = safe_atou32(rvalue, &n->priority); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse RPDB rule priority, ignoring: %s", rvalue); return 0; } + n->priority_set = true; TAKE_PTR(n); return 0; diff --git a/src/network/networkd-routing-policy-rule.h b/src/network/networkd-routing-policy-rule.h index aed37b00d2..557048c3f4 100644 --- a/src/network/networkd-routing-policy-rule.h +++ b/src/network/networkd-routing-policy-rule.h @@ -20,6 +20,7 @@ typedef struct RoutingPolicyRule { NetworkConfigSection *section; bool invert_rule; + bool priority_set; uint8_t tos; uint8_t type; diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 0eb2fdf87e..4a2af0c500 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -3644,7 +3644,7 @@ class NetworkdBridgeTests(unittest.TestCase, Utilities): output = check_output('ip rule list table 100') print(output) - self.assertIn('0: from all to 8.8.8.8 lookup 100', output) + self.assertIn('from all to 8.8.8.8 lookup 100', output) class NetworkdLLDPTests(unittest.TestCase, Utilities): links = ['veth99'] -- 2.33.0
Locations
Projects
Search
Status Monitor
Help
Open Build Service
OBS Manuals
API Documentation
OBS Portal
Reporting a Bug
Contact
Mailing List
Forums
Chat (IRC)
Twitter
Open Build Service (OBS)
is an
openSUSE project
.
浙ICP备2022010568号-2