From 92cd00b9749141907a1110044cc7d1f01caff545 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 22 Feb 2019 13:27:44 +0900 Subject: [PATCH 1/4] network: remove routing policy rule from foreign rule database when it is removed Previously, When the first link configures rules, it removes all saved rules, which were configured by networkd previously, in the foreign rule database, but the rules themselves are still in the database. Thus, when the second or later link configures rules, it errnously treats the rules already exist. This is the root of issue #11280. This removes rules from the foreign database when they are removed. Fixes #11280. --- src/network/networkd-routing-policy-rule.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index dd155748177..3941245733f 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -1244,15 +1244,18 @@ void routing_policy_rule_purge(Manager *m, Link *link) { SET_FOREACH(rule, m->rules_saved, i) { existing = set_get(m->rules_foreign, rule); - if (existing) { + if (!existing) + continue; /* Saved rule does not exist anymore. */ - r = routing_policy_rule_remove(rule, link, NULL); - if (r < 0) { - log_warning_errno(r, "Could not remove routing policy rules: %m"); - continue; - } - - link->routing_policy_rule_remove_messages++; + r = routing_policy_rule_remove(existing, link, NULL); + if (r < 0) { + log_warning_errno(r, "Could not remove routing policy rules: %m"); + continue; } + + link->routing_policy_rule_remove_messages++; + + assert_se(set_remove(m->rules_foreign, existing) == existing); + routing_policy_rule_free(existing); } } From 031fb59a984e5b51f3c72aa8125ecc50b08011fe Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 22 Feb 2019 13:32:47 +0900 Subject: [PATCH 2/4] network: do not remove rule when it is requested by existing links Otherwise, the first link once removes all saved rules in the foreign rule database, and the second or later links create again... --- src/network/networkd-routing-policy-rule.c | 26 ++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 3941245733f..ae94272781e 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -1234,6 +1234,26 @@ int routing_policy_load_rules(const char *state_file, Set **rules) { return 0; } +static bool manager_links_have_routing_policy_rule(Manager *m, RoutingPolicyRule *rule) { + RoutingPolicyRule *link_rule; + Iterator i; + Link *link; + + assert(m); + assert(rule); + + HASHMAP_FOREACH(link, m->links, i) { + if (!link->network) + continue; + + LIST_FOREACH(rules, link_rule, link->network->rules) + if (routing_policy_rule_compare_func(link_rule, rule) == 0) + return true; + } + + return false; +} + void routing_policy_rule_purge(Manager *m, Link *link) { RoutingPolicyRule *rule, *existing; Iterator i; @@ -1247,6 +1267,12 @@ void routing_policy_rule_purge(Manager *m, Link *link) { if (!existing) continue; /* Saved rule does not exist anymore. */ + if (manager_links_have_routing_policy_rule(m, existing)) + continue; /* Existing links have the saved rule. */ + + /* Existing links do not have the saved rule. Let's drop the rule now, and re-configure it + * later when it is requested. */ + r = routing_policy_rule_remove(existing, link, NULL); if (r < 0) { log_warning_errno(r, "Could not remove routing policy rules: %m"); From 703bc7a2a67af315d2515b55d06eba63cf6ba9f1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 27 Feb 2019 19:22:27 +0900 Subject: [PATCH 3/4] test-network: drop relevant ip routing policy rules before testing --- test/test-network/systemd-networkd-tests.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index fa5e08e7506..8600b56e4ca 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -659,6 +659,9 @@ class NetworkdNetWorkTests(unittest.TestCase, Utilities): def test_routing_policy_rule(self): self.copy_unit_to_networkd_unit_path('routing-policy-rule.network', '11-dummy.netdev') + + subprocess.call(['ip', 'rule', 'del', 'table', '7']) + self.start_networkd() self.assertTrue(self.link_exits('test1')) @@ -677,6 +680,9 @@ class NetworkdNetWorkTests(unittest.TestCase, Utilities): @expectedFailureIfRoutingPolicyPortRangeIsNotAvailable() def test_routing_policy_rule_port_range(self): self.copy_unit_to_networkd_unit_path('25-fibrule-port-range.network', '11-dummy.netdev') + + subprocess.call(['ip', 'rule', 'del', 'table', '7']) + self.start_networkd() self.assertTrue(self.link_exits('test1')) @@ -695,6 +701,9 @@ class NetworkdNetWorkTests(unittest.TestCase, Utilities): @expectedFailureIfRoutingPolicyIPProtoIsNotAvailable() def test_routing_policy_rule_invert(self): self.copy_unit_to_networkd_unit_path('25-fibrule-invert.network', '11-dummy.netdev') + + subprocess.call(['ip', 'rule', 'del', 'table', '7']) + self.start_networkd() self.assertTrue(self.link_exits('test1')) @@ -1252,6 +1261,9 @@ class NetworkdNetWorkBridgeTests(unittest.TestCase, Utilities): self.copy_unit_to_networkd_unit_path('11-dummy.netdev', '12-dummy.netdev', '26-bridge.netdev', '26-bridge-slave-interface-1.network', '26-bridge-slave-interface-2.network', 'bridge99-ignore-carrier-loss.network') + + subprocess.call(['ip', 'rule', 'del', 'table', '100']) + self.start_networkd() self.assertTrue(self.link_exits('dummy98')) @@ -1276,6 +1288,9 @@ class NetworkdNetWorkBridgeTests(unittest.TestCase, Utilities): def test_bridge_ignore_carrier_loss_frequent_loss_and_gain(self): self.copy_unit_to_networkd_unit_path('26-bridge.netdev', '26-bridge-slave-interface-1.network', 'bridge99-ignore-carrier-loss.network') + + subprocess.call(['ip', 'rule', 'del', 'table', '100']) + self.start_networkd() self.assertTrue(self.link_exits('bridge99')) From b677774d6958435bead38202e61509139b06b497 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 22 Feb 2019 12:28:51 +0900 Subject: [PATCH 4/4] test-network: add testcase for issue #11280 --- .../conf/routing-policy-rule-dummy98.network | 10 ++++++ ...work => routing-policy-rule-test1.network} | 0 test/test-network/systemd-networkd-tests.py | 35 ++++++++++++++++--- 3 files changed, 41 insertions(+), 4 deletions(-) create mode 100644 test/test-network/conf/routing-policy-rule-dummy98.network rename test/test-network/conf/{routing-policy-rule.network => routing-policy-rule-test1.network} (100%) diff --git a/test/test-network/conf/routing-policy-rule-dummy98.network b/test/test-network/conf/routing-policy-rule-dummy98.network new file mode 100644 index 00000000000..8136c20ae42 --- /dev/null +++ b/test/test-network/conf/routing-policy-rule-dummy98.network @@ -0,0 +1,10 @@ +[Match] +Name=dummy98 + +[RoutingPolicyRule] +TypeOfService=0x08 +Table=8 +From= 192.168.101.18 +Priority=112 +IncomingInterface=dummy98 +OutgoingInterface=dummy98 diff --git a/test/test-network/conf/routing-policy-rule.network b/test/test-network/conf/routing-policy-rule-test1.network similarity index 100% rename from test/test-network/conf/routing-policy-rule.network rename to test/test-network/conf/routing-policy-rule-test1.network diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 8600b56e4ca..4a5109d9a01 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -165,8 +165,9 @@ class Utilities(): if os.path.exists(dnsmasq_log_file): os.remove(dnsmasq_log_file) - def start_networkd(self): - if (os.path.exists(os.path.join(networkd_runtime_directory, 'state'))): + def start_networkd(self, remove_state_files=True): + if (remove_state_files and + os.path.exists(os.path.join(networkd_runtime_directory, 'state'))): subprocess.check_call('systemctl stop systemd-networkd', shell=True) os.remove(os.path.join(networkd_runtime_directory, 'state')) subprocess.check_call('systemctl start systemd-networkd', shell=True) @@ -601,7 +602,8 @@ class NetworkdNetWorkTests(unittest.TestCase, Utilities): '25-sysctl-disable-ipv6.network', '25-sysctl.network', 'configure-without-carrier.network', - 'routing-policy-rule.network', + 'routing-policy-rule-dummy98.network', + 'routing-policy-rule-test1.network', 'test-static.network'] def setUp(self): @@ -658,7 +660,7 @@ class NetworkdNetWorkTests(unittest.TestCase, Utilities): self.assertRegex(output, 'primary test1') def test_routing_policy_rule(self): - self.copy_unit_to_networkd_unit_path('routing-policy-rule.network', '11-dummy.netdev') + self.copy_unit_to_networkd_unit_path('routing-policy-rule-test1.network', '11-dummy.netdev') subprocess.call(['ip', 'rule', 'del', 'table', '7']) @@ -677,6 +679,31 @@ class NetworkdNetWorkTests(unittest.TestCase, Utilities): subprocess.call(['ip', 'rule', 'del', 'table', '7']) + def test_routing_policy_rule_issue_11280(self): + self.copy_unit_to_networkd_unit_path('routing-policy-rule-test1.network', '11-dummy.netdev', + 'routing-policy-rule-dummy98.network', '12-dummy.netdev') + + subprocess.call(['ip', 'rule', 'del', 'table', '7']) + subprocess.call(['ip', 'rule', 'del', 'table', '8']) + + for trial in range(3): + # Remove state files only first time + self.start_networkd(trial == 0) + + self.assertTrue(self.link_exits('test1')) + self.assertTrue(self.link_exits('dummy98')) + + output = subprocess.check_output(['ip', 'rule', 'list', 'table', '7']).rstrip().decode('utf-8') + print(output) + self.assertRegex(output, '111: from 192.168.100.18 tos (?:0x08|throughput) iif test1 oif test1 lookup 7') + + output = subprocess.check_output(['ip', 'rule', 'list', 'table', '8']).rstrip().decode('utf-8') + print(output) + self.assertRegex(output, '112: from 192.168.101.18 tos (?:0x08|throughput) iif dummy98 oif dummy98 lookup 8') + + subprocess.call(['ip', 'rule', 'del', 'table', '7']) + subprocess.call(['ip', 'rule', 'del', 'table', '8']) + @expectedFailureIfRoutingPolicyPortRangeIsNotAvailable() def test_routing_policy_rule_port_range(self): self.copy_unit_to_networkd_unit_path('25-fibrule-port-range.network', '11-dummy.netdev')