packages/systemd
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
systemd/backport-network-do-not-assume-the-highest-priority-when-Prio.patch

293 lines
14 KiB
Diff
Raw Blame History

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
Reference in New Issue View Git Blame Copy Permalink