From b9600f166343b039b4ab4871cb80db8f9402b1e8 Mon Sep 17 00:00:00 2001 From: vdahiya12 <67608553+vdahiya12@users.noreply.github.com> Date: Tue, 12 Dec 2023 11:26:34 -0800 Subject: [PATCH] [caclmgrd][DualToR] Fix a case where vlan address is not network address for DualToR Active-active configuration (#95) * [caclmgrd][DualToR] Fix a case where vlan address is not network address for DualToR Active-active configuration Signed-off-by: vaibhav-dahiya * fix UT Signed-off-by: vaibhav-dahiya * add all Signed-off-by: vaibhav-dahiya * add test Signed-off-by: vaibhav-dahiya * add ut Signed-off-by: vaibhav-dahiya * add Tests Signed-off-by: vaibhav-dahiya * add changes Signed-off-by: vaibhav-dahiya * fix typo Signed-off-by: vaibhav-dahiya * add vals Signed-off-by: vaibhav-dahiya --------- Signed-off-by: vaibhav-dahiya --- scripts/caclmgrd | 18 +++++++++++------- tests/caclmgrd/caclmgrd_soc_rules_test.py | 6 +++--- tests/caclmgrd/test_soc_rules_vectors.py | 4 ++-- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/scripts/caclmgrd b/scripts/caclmgrd index 1b53c3a2..38beeee2 100755 --- a/scripts/caclmgrd +++ b/scripts/caclmgrd @@ -44,7 +44,7 @@ def _ip_prefix_in_key(key): def get_ipv4_networks_from_interface_table(table, intf_name): - addresses = [] + addresses = {} if table: for key, _ in table.items(): if not _ip_prefix_in_key(key): @@ -54,8 +54,10 @@ def get_ipv4_networks_from_interface_table(table, intf_name): iface_name, iface_cidr = key if iface_name.startswith(intf_name): ip_ntwrk = ipaddress.ip_network(iface_cidr, strict=False) - if isinstance(ip_ntwrk, ipaddress.IPv4Network): - addresses.append(ip_ntwrk) + ip_str = iface_cidr.split("/")[0] + ip_addr = ipaddress.ip_address(ip_str) + if isinstance(ip_ntwrk, ipaddress.IPv4Network) and isinstance(ip_addr, ipaddress.IPv4Address): + addresses[ip_ntwrk] = ip_addr return addresses @@ -365,11 +367,14 @@ class ControlPlaneAclManager(daemon_base.DaemonBase): 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): + + loopback_address_vals = list(loopback_networks.values()) + + if not isinstance(loopback_address_vals[0], ipaddress.IPv4Address): 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 + loopback_address = loopback_address_vals[0] 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) @@ -389,10 +394,9 @@ class ControlPlaneAclManager(daemon_base.DaemonBase): if 'cable_type' in kvp and kvp['cable_type'] == 'active-active': soc_ipv4_str = kvp['soc_ipv4'].split("/")[0] soc_ipv4_addr = ipaddress.ip_address(soc_ipv4_str) - for ip_network in vlan_networks: + for ip_network, vlan_address in vlan_networks.items(): # 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', str(soc_ipv4_addr), '--source', str(vlan_address), '-j', 'SNAT', '--to-source', str(loopback_address)]) diff --git a/tests/caclmgrd/caclmgrd_soc_rules_test.py b/tests/caclmgrd/caclmgrd_soc_rules_test.py index ef40292a..afc22776 100644 --- a/tests/caclmgrd/caclmgrd_soc_rules_test.py +++ b/tests/caclmgrd/caclmgrd_soc_rules_test.py @@ -29,7 +29,7 @@ def setUp(self): @parameterized.expand(CACLMGRD_SOC_TEST_VECTOR) @patchfs - @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)])) + @patch('caclmgrd.get_ipv4_networks_from_interface_table', MagicMock(return_value={IPv4Network('10.10.11.18/24', strict=False): IPv4Address('10.10.11.18') , IPv4Network('10.10.10.18/24', strict=False) : IPv4Address('10.10.10.18') })) 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 @@ -79,7 +79,7 @@ def test_caclmgrd_soc_no_ips(self, test_name, test_data, fs): @parameterized.expand(CACLMGRD_SOC_TEST_VECTOR_EMPTY) @patchfs - @patch('caclmgrd.get_ipv4_networks_from_interface_table', MagicMock(return_value=['10.10.10.10'])) + @patch('caclmgrd.get_ipv4_networks_from_interface_table', MagicMock(return_value={'10.10.10.10': '10.10.10.1'})) 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 @@ -109,4 +109,4 @@ def test_get_ipv4_networks_from_interface_table(self): table = {("Vlan1000","10.10.10.1/32"): "val"} ip_addr = self.caclmgrd.get_ipv4_networks_from_interface_table(table, "Vlan") - assert (ip_addr == [IPv4Network('10.10.10.1/32')]) + assert (ip_addr == {IPv4Network('10.10.10.1/32'): IPv4Address('10.10.10.1')}) diff --git a/tests/caclmgrd/test_soc_rules_vectors.py b/tests/caclmgrd/test_soc_rules_vectors.py index b339a32d..a54a3536 100644 --- a/tests/caclmgrd/test_soc_rules_vectors.py +++ b/tests/caclmgrd/test_soc_rules_vectors.py @@ -18,7 +18,7 @@ "MUX_CABLE": { "Ethernet4": { "cable_type": "active-active", - "soc_ipv4": "10.10.11.7/32", + "soc_ipv4": "10.10.10.7/32", } }, "VLAN_INTERFACE": { @@ -35,7 +35,7 @@ }, }, "expected_subprocess_calls": [ - call(['iptables', '-t', 'nat', '-A', 'POSTROUTING', '--destination', '10.10.11.7', '--source', '10.10.11.0', '-j', 'SNAT', '--to-source', '10.10.10.0'], universal_newlines=True, stdout=-1) + call(['iptables', '-t', 'nat', '-A', 'POSTROUTING', '--destination', '10.10.10.7', '--source', '10.10.10.18', '-j', 'SNAT', '--to-source', '10.10.11.18'], universal_newlines=True, stdout=-1) ], "popen_attributes": { 'communicate.return_value': ('output', 'error'),