Merge pull request #3905 from vyos/mergify/bp/sagitta/pr-3883

vrf: T6603: conntrack ct_iface_map must only contain one entry for iifname/oifname (backport #3883)

2 files changed, 35 insertions, 4 deletions

diff --git a/smoketest/scripts/cli/test_vrf.py b/smoketest/scripts/cli/test_vrf.py
index 176882ca5..2bb6c91c1 100755
--- a/smoketest/scripts/cli/test_vrf.py
+++ b/smoketest/scripts/cli/test_vrf.py
@@ -19,6 +19,8 @@ import os
 import unittest
 
 from base_vyostest_shim import VyOSUnitTestSHIM
+from json import loads
+from jmespath import search
 
 from vyos.configsession import ConfigSessionError
 from vyos.ifconfig import Interface
@@ -28,6 +30,7 @@ from vyos.utils.network import get_interface_config
 from vyos.utils.network import get_vrf_tableid
 from vyos.utils.network import is_intf_addr_assigned
 from vyos.utils.network import interface_exists
+from vyos.utils.process import cmd
 from vyos.utils.system import sysctl_read
 
 base_path = ['vrf']
@@ -557,26 +560,39 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
             self.assertNotIn(f' no ipv6 nht resolve-via-default', frrconfig)
 
     def test_vrf_conntrack(self):
-        table = '1000'
+        table = '8710'
         nftables_rules = {
             'vrf_zones_ct_in': ['ct original zone set iifname map @ct_iface_map'],
             'vrf_zones_ct_out': ['ct original zone set oifname map @ct_iface_map']
         }
 
-        self.cli_set(base_path + ['name', 'blue', 'table', table])
+        self.cli_set(base_path + ['name', 'randomVRF', 'table', '1000'])
         self.cli_commit()
 
         # Conntrack rules should not be present
         for chain, rule in nftables_rules.items():
             self.verify_nftables_chain(rule, 'inet vrf_zones', chain, inverse=True)
 
+        # conntrack is only enabled once NAT, NAT66 or firewalling is enabled
         self.cli_set(['nat'])
-        self.cli_commit()
+
+        for vrf in vrfs:
+            base = base_path + ['name', vrf]
+            self.cli_set(base + ['table', table])
+            table = str(int(table) + 1)
+            # We need the commit inside the loop to trigger the bug in T6603
+            self.cli_commit()
 
         # Conntrack rules should now be present
         for chain, rule in nftables_rules.items():
             self.verify_nftables_chain(rule, 'inet vrf_zones', chain, inverse=False)
 
+        # T6603: there should be only ONE entry for the iifname/oifname in the chains
+        tmp = loads(cmd('sudo nft -j list table inet vrf_zones'))
+        num_rules = len(search("nftables[].rule[].chain", tmp))
+        # ['vrf_zones_ct_in', 'vrf_zones_ct_out']
+        self.assertEqual(num_rules, 2)
+
         self.cli_delete(['nat'])
 
 if __name__ == '__main__':
diff --git a/src/conf_mode/vrf.py b/src/conf_mode/vrf.py
index 184725573..72b178c89 100755
--- a/src/conf_mode/vrf.py
+++ b/src/conf_mode/vrf.py
@@ -15,6 +15,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from sys import exit
+from jmespath import search
 from json import loads
 
 from vyos.config import Config
@@ -70,6 +71,14 @@ def has_rule(af : str, priority : int, table : str=None):
                 return True
     return False
 
+def is_nft_vrf_zone_rule_setup() -> bool:
+    """
+    Check if an nftables connection tracking rule already exists
+    """
+    tmp = loads(cmd('sudo nft -j list table inet vrf_zones'))
+    num_rules = len(search("nftables[].rule[].chain", tmp))
+    return bool(num_rules)
+
 def vrf_interfaces(c, match):
     matched = []
     old_level = c.get_level()
@@ -264,6 +273,7 @@ def apply(vrf):
             if not has_rule(afi, 2000, 'l3mdev'):
                 call(f'ip {afi} rule add pref 2000 l3mdev unreachable')
 
+        nft_vrf_zone_rule_setup = False
         for name, config in vrf['name'].items():
             table = config['table']
             if not interface_exists(name):
@@ -302,7 +312,12 @@ def apply(vrf):
             nft_add_element = f'add element inet vrf_zones ct_iface_map {{ "{name}" : {table} }}'
             cmd(f'nft {nft_add_element}')
 
-        if vrf['conntrack']:
+        # Only call into nftables as long as there is nothing setup to avoid wasting
+        # CPU time and thus lenghten the commit process
+        if not nft_vrf_zone_rule_setup:
+            nft_vrf_zone_rule_setup = is_nft_vrf_zone_rule_setup()
+        # Install nftables conntrack rules only once
+        if vrf['conntrack'] and not nft_vrf_zone_rule_setup:
             for chain, rule in nftables_rules.items():
                 cmd(f'nft add rule inet vrf_zones {chain} {rule}')