From 04d10fdb1dd18b946ad26aaa5580bbf25c2561de Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 22 Aug 2024 04:11:52 +0900 Subject: [PATCH 1/3] network/routing-policy-rule: use address family of existing rule when judging if existing rule can be updated Otherwise, the other RoutingPolicyRule object may not have a valid address family yet, and the existing rule may be wrongly handled as that it is not requested by any interface, and it may be removed. Follow-up for 727235006a7e8904f8493d32376cffa6969c5fa9. Fixes #34068. --- src/network/networkd-routing-policy-rule.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 44d85d000f..ea3cb45868 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -351,11 +351,13 @@ 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) { +static bool routing_policy_rule_can_update(const RoutingPolicyRule *existing, const RoutingPolicyRule *requesting) { assert(existing); + assert(IN_SET(existing->family, AF_INET, AF_INET6)); assert(requesting); + assert(IN_SET(requesting->family, AF_INET, AF_INET6) || requesting->address_family == ADDRESS_FAMILY_YES); - if (!routing_policy_rule_equal(existing, requesting, family, existing->priority)) + if (!routing_policy_rule_equal(existing, requesting, existing->family, existing->priority)) return false; /* These flags cannot be updated. */ @@ -791,7 +793,7 @@ static void manager_unmark_routing_policy_rule(Manager *m, const RoutingPolicyRu if (routing_policy_rule_get(m, rule, family, &existing) < 0) return; - if (!routing_policy_rule_can_update(existing, rule, rule->family)) + if (!routing_policy_rule_can_update(existing, rule)) return; routing_policy_rule_unmark(existing); From 462be8c957197fc1e776543ac32c18dd72efd536 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 22 Aug 2024 04:16:09 +0900 Subject: [PATCH 2/3] test-network: find routing policy rule by priority We usually configure a test rule with a unique priority. Hence, finding rule by priority reduces the lines of output, and we can debug easily. Also print short comments on check. That's helpful when the check is called several times. --- test/test-network/systemd-networkd-tests.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index fae7b58fcc..42af0145b3 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -3211,6 +3211,8 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): self.assertRegex(output, routable_map[carrier]) def check_routing_policy_rule_test1(self): + print('### Checking routing policy rules requested by test1') + 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') @@ -3236,7 +3238,9 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): 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('### Checking routing policy rules requested by dummy98') + + output = check_output('ip rule list priority 112') print(output) self.assertRegex(output, r'112: from 192.168.101.18 tos (0x08|throughput) iif dummy98 oif dummy98 lookup 8') From cd2a1e2df9fcc7cb2c772ee79c8840ab0642df38 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 22 Aug 2024 04:20:51 +0900 Subject: [PATCH 3/3] test-network: also test routing policy rules are configured as expected after reconfiguration For issue #34068. --- test/test-network/systemd-networkd-tests.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 42af0145b3..180db8bde9 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -3288,10 +3288,12 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): 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): + def test_routing_policy_rule_restart_and_reconfigure(self): copy_network_unit('25-routing-policy-rule-test1.network', '11-dummy.netdev', '25-routing-policy-rule-dummy98.network', '12-dummy.netdev') + # For #11280 and #34068. + for trial in range(3): restart_networkd(show_logs=(trial > 0)) self.wait_online('test1:degraded', 'dummy98:degraded') @@ -3299,6 +3301,18 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): self.check_routing_policy_rule_test1() self.check_routing_policy_rule_dummy98() + networkctl_reconfigure('test1') + self.wait_online('test1:degraded') + + self.check_routing_policy_rule_test1() + self.check_routing_policy_rule_dummy98() + + networkctl_reconfigure('dummy98') + self.wait_online('dummy98:degraded') + + 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') start_networkd()