Merge pull request #11795 from yuwata/fix-network-routing-policy-11280

network: fix routing policy rule issue #11280

4 files changed, 92 insertions, 11 deletions

diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c
index dd15574817..ae94272781 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;
@@ -1244,15 +1264,24 @@ 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;
-                        }
+                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. */
 
-                        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);
         }
 }
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 0000000000..8136c20ae4
--- /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
index 46b87c5a9a..46b87c5a9a 100644
--- a/test/test-network/conf/routing-policy-rule.network
+++ b/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 e410ccb265..b9a61a92fe 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)
@@ -616,7 +617,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):
@@ -673,7 +675,10 @@ 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'])
+
         self.start_networkd()
 
         self.assertTrue(self.link_exits('test1'))
@@ -689,9 +694,37 @@ 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')
+
+        subprocess.call(['ip', 'rule', 'del', 'table', '7'])
+
         self.start_networkd()
 
         self.assertTrue(self.link_exits('test1'))
@@ -710,6 +743,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'))
@@ -1267,6 +1303,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'))
@@ -1291,6 +1330,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'))