[DualToR][caclmgrd] Fix IPtables rules for multiple vlan interfaces for DualToR config (#17093)

* [DualToR][caclmgrd] Fix IPtables rules for multiple vlan interfaces for
DualToR config

Signed-off-by: vaibhav-dahiya <[email protected]>

Author: vdahiya12

src/sonic-host-services/scripts/caclmgrd

@@ -41,8 +41,9 @@ def _ip_prefix_in_key(key):
     """
     return (isinstance(key, tuple))
 
-def get_ip_from_interface_table(table, intf_name):
-
+def get_ipv4_networks_from_interface_table(table, intf_name):
+    
+    addresses = []
     if table:
         for key, _ in table.items():
             if not _ip_prefix_in_key(key):
@@ -51,12 +52,11 @@ def get_ip_from_interface_table(table, intf_name):
 
             iface_name, iface_cidr = key
             if iface_name.startswith(intf_name):
-                ip_str = iface_cidr.split("/")[0]
-                ip_addr = ipaddress.ip_address(ip_str)
-                if isinstance(ip_addr, ipaddress.IPv4Address):
-                    return ip_addr
+                ip_ntwrk = ipaddress.ip_network(iface_cidr, strict=False)
+                if isinstance(ip_ntwrk, ipaddress.IPv4Network):
+                    addresses.append(ip_ntwrk)
 
-    return None
+    return addresses
 
 # ============================== Classes ==============================
 
@@ -342,22 +342,41 @@ class ControlPlaneAclManager(daemon_base.DaemonBase):
         if self.DualToR:
             loopback_table = config_db_connector.get_table(self.LOOPBACK_TABLE)
             loopback_name = 'Loopback3'
-            loopback_address = get_ip_from_interface_table(loopback_table, loopback_name)
+            loopback_networks = get_ipv4_networks_from_interface_table(loopback_table, loopback_name)
+            if len(loopback_networks) == 0:
+                self.log_warning("Loopback 3 IP not available from DualToR active-active config")
+                return fwd_dualtor_grpc_traffic_from_host_to_soc_cmds
+
+            if not isinstance(loopback_networks[0], ipaddress.IPv4Network):
+                self.log_warning("Loopback 3 IP Network not available from DualToR active-active config")
+                return fwd_dualtor_grpc_traffic_from_host_to_soc_cmds
+
+            loopback_address = loopback_networks[0].network_address
             vlan_name = 'Vlan'
             vlan_table = config_db_connector.get_table(self.VLAN_INTF_TABLE)
+            vlan_networks = get_ipv4_networks_from_interface_table(vlan_table, vlan_name)
+
+            if len(vlan_networks) == 0:
+                self.log_warning("Vlan IP not available from DualToR active-active config")
+                return fwd_dualtor_grpc_traffic_from_host_to_soc_cmds
 
-            vlan_address = get_ip_from_interface_table(vlan_table, vlan_name)
             fwd_dualtor_grpc_traffic_from_host_to_soc_cmds.append(self.iptables_cmd_ns_prefix[namespace] + 
                     "iptables -t nat --flush POSTROUTING")
-            
+
             if loopback_address is not None:
                 mux_table = config_db_connector.get_table(self.CONFIG_MUX_CABLE)
                 mux_table_keys = mux_table.keys()
                 for key in mux_table_keys:
                     kvp = mux_table.get(key)
                     if 'cable_type' in kvp and kvp['cable_type'] == 'active-active':
-                        fwd_dualtor_grpc_traffic_from_host_to_soc_cmds.append(self.iptables_cmd_ns_prefix[namespace] + 
-                                "iptables -t nat -A POSTROUTING --destination {} --source {} -j SNAT --to-source {}".format(kvp['soc_ipv4'], vlan_address, loopback_address))
+                        soc_ipv4_str = kvp['soc_ipv4'].split("/")[0]
+                        soc_ipv4_addr = ipaddress.ip_address(soc_ipv4_str)
+                        for ip_network in vlan_networks:
+                            # Only add the vlan source IP specific soc IP address to IPtables
+                            if soc_ipv4_addr in ip_network:
+                                vlan_address = ip_network.network_address
+                                fwd_dualtor_grpc_traffic_from_host_to_soc_cmds.append(self.iptables_cmd_ns_prefix[namespace] + 
+                                        "iptables -t nat -A POSTROUTING --destination {} --source {} -j SNAT --to-source {}".format(str(soc_ipv4_addr), str(vlan_address), str(loopback_address)))
 
         return fwd_dualtor_grpc_traffic_from_host_to_soc_cmds
 

src/sonic-host-services/tests/caclmgrd/caclmgrd_soc_rules_test.py

@@ -4,11 +4,11 @@
 
 from parameterized import parameterized
 from sonic_py_common.general import load_module_from_source
-from ipaddress import IPv4Address
+from ipaddress import IPv4Address, IPv4Network
 from unittest import TestCase, mock
 from pyfakefs.fake_filesystem_unittest import patchfs
 
-from .test_soc_rules_vectors import CACLMGRD_SOC_TEST_VECTOR
+from .test_soc_rules_vectors import CACLMGRD_SOC_TEST_VECTOR, CACLMGRD_SOC_TEST_VECTOR_EMPTY
 from tests.common.mock_configdb import MockConfigDb
 from unittest.mock import MagicMock, patch
 
@@ -29,7 +29,7 @@ def setUp(self):
 
     @parameterized.expand(CACLMGRD_SOC_TEST_VECTOR)
     @patchfs
-    @patch('caclmgrd.get_ip_from_interface_table', MagicMock(return_value="10.10.10.10"))
+    @patch('caclmgrd.get_ipv4_networks_from_interface_table', MagicMock(return_value=[IPv4Network('10.10.10.18/24', strict=False), IPv4Network('10.10.11.18/24', strict=False)]))
     def test_caclmgrd_soc(self, test_name, test_data, fs):
         if not os.path.exists(DBCONFIG_PATH):
             fs.create_file(DBCONFIG_PATH) # fake database_config.json
@@ -50,11 +50,60 @@ def test_caclmgrd_soc(self, test_name, test_data, fs):
             caclmgrd_daemon.update_control_plane_nat_acls('', {}, MockConfigDb())
             mocked_subprocess.Popen.assert_has_calls(test_data["expected_subprocess_calls"], any_order=True)
 
-    def test_get_ip_from_interface_table(self):
+
+    @parameterized.expand(CACLMGRD_SOC_TEST_VECTOR_EMPTY)
+    @patchfs
+    @patch('caclmgrd.get_ipv4_networks_from_interface_table', MagicMock(return_value=[]))
+    def test_caclmgrd_soc_no_ips(self, test_name, test_data, fs):
+        if not os.path.exists(DBCONFIG_PATH):
+            fs.create_file(DBCONFIG_PATH) # fake database_config.json
+
+        MockConfigDb.set_config_db(test_data["config_db"])
+
+        with mock.patch("caclmgrd.subprocess") as mocked_subprocess:
+            popen_mock = mock.Mock()
+            popen_attrs = test_data["popen_attributes"]
+            popen_mock.configure_mock(**popen_attrs)
+            mocked_subprocess.Popen.return_value = popen_mock
+            mocked_subprocess.PIPE = -1
+
+            call_rc = test_data["call_rc"]
+            mocked_subprocess.call.return_value = call_rc
+
+            caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd")
+            caclmgrd_daemon.update_control_plane_nat_acls('', {}, MockConfigDb())
+            mocked_subprocess.Popen.assert_has_calls(test_data["expected_subprocess_calls"], any_order=True)
+
+
+    @parameterized.expand(CACLMGRD_SOC_TEST_VECTOR_EMPTY)
+    @patchfs
+    @patch('caclmgrd.get_ipv4_networks_from_interface_table', MagicMock(return_value=['10.10.10.10']))
+    def test_caclmgrd_soc_ip_string(self, test_name, test_data, fs):
+        if not os.path.exists(DBCONFIG_PATH):
+            fs.create_file(DBCONFIG_PATH) # fake database_config.json
+
+        MockConfigDb.set_config_db(test_data["config_db"])
+
+        with mock.patch("caclmgrd.subprocess") as mocked_subprocess:
+            popen_mock = mock.Mock()
+            popen_attrs = test_data["popen_attributes"]
+            popen_mock.configure_mock(**popen_attrs)
+            mocked_subprocess.Popen.return_value = popen_mock
+            mocked_subprocess.PIPE = -1
+
+            call_rc = test_data["call_rc"]
+            mocked_subprocess.call.return_value = call_rc
+
+            caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd")
+            caclmgrd_daemon.update_control_plane_nat_acls('', {}, MockConfigDb())
+            mocked_subprocess.Popen.assert_has_calls(test_data["expected_subprocess_calls"], any_order=True)
+
+
+    def test_get_ipv4_networks_from_interface_table(self):
         if not os.path.exists(DBCONFIG_PATH):
             fs.create_file(DBCONFIG_PATH) # fake database_config.json
 
         table = {("Vlan1000","10.10.10.1/32"): "val"}
-        ip_addr = self.caclmgrd.get_ip_from_interface_table(table, "Vlan")
+        ip_addr = self.caclmgrd.get_ipv4_networks_from_interface_table(table, "Vlan")
 
-        assert (ip_addr == IPv4Address('10.10.10.1'))
+        assert (ip_addr == [IPv4Network('10.10.10.1/32')])

src/sonic-host-services/tests/caclmgrd/test_soc_rules_vectors.py

@@ -18,11 +18,11 @@
                 "MUX_CABLE": {
                     "Ethernet4": {
                         "cable_type": "active-active",
-                        "soc_ipv4": "192.168.1.0/32",
+                        "soc_ipv4": "10.10.11.7/32",
                     }
                 },
                 "VLAN_INTERFACE": {
-                    "Vlan1000|10.10.2.2/23": {
+                    "Vlan1000|10.10.10.3/24": {
                         "NULL": "NULL",
                     }
                 },
@@ -35,8 +35,48 @@
                 },
             },
             "expected_subprocess_calls": [
-                call("iptables -t nat -A POSTROUTING --destination 192.168.1.0/32 --source 10.10.10.10 -j SNAT --to-source 10.10.10.10",shell=True, universal_newlines=True, stdout=-1)
-            ],
+                call('iptables -t nat -A POSTROUTING --destination 10.10.11.7 --source 10.10.11.0 -j SNAT --to-source 10.10.10.0', shell=True, universal_newlines=True, stdout=-1)
+                ],
+            "popen_attributes": {
+                'communicate.return_value': ('output', 'error'),
+            },
+            "call_rc": 0,
+        }
+    ]
+]
+
+
+CACLMGRD_SOC_TEST_VECTOR_EMPTY = [
+    [
+        "SOC_SESSION_TEST",
+        {
+            "config_db": {
+                "DEVICE_METADATA": {
+                    "localhost": {
+                        "subtype": "DualToR",
+                        "type": "ToRRouter",
+                    }
+                },
+                "MUX_CABLE": {
+                    "Ethernet4": {
+                        "cable_type": "active-active",
+                        "soc_ipv4": "10.10.11.7/32",
+                    }
+                },
+                "VLAN_INTERFACE": {
+                    "Vlan1000|10.10.10.3/24": {
+                        "NULL": "NULL",
+                    }
+                },
+                "LOOPBACK_INTERFACE": {
+                    "Loopback3|10.10.10.10/32": {
+                        "NULL": "NULL",
+                    }
+                },
+                "FEATURE": {
+                },
+            },
+            "expected_subprocess_calls": [],
             "popen_attributes": {
                 'communicate.return_value': ('output', 'error'),
             },