From 403d13aae27f0594635b45103fa798ab3b95828a Mon Sep 17 00:00:00 2001 From: vdahiya12 <67608553+vdahiya12@users.noreply.github.com> Date: Fri, 22 Dec 2023 12:12:04 -0800 Subject: [PATCH] [caclmgrd][DualToR] Fix a case where vlan address is not network address for DualToR Active-active configuration (#17511) * [caclmgrd][DualToR] Fix a case where vlan address is not network address for DualToR Active-active configuration Signed-off-by: vaibhav-dahiya * add changes Signed-off-by: vaibhav-dahiya * fix unit Signed-off-by: vaibhav-dahiya * add c Signed-off-by: vaibhav-dahiya * fix test Signed-off-by: vaibhav-dahiya * add fixes unit Signed-off-by: vaibhav-dahiya --------- Signed-off-by: vaibhav-dahiya --- src/sonic-host-services/scripts/caclmgrd | 17 ++++++++++------- .../tests/caclmgrd/caclmgrd_soc_rules_test.py | 6 +++--- .../tests/caclmgrd/test_soc_rules_vectors.py | 4 ++-- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/sonic-host-services/scripts/caclmgrd b/src/sonic-host-services/scripts/caclmgrd index 82c8097cf5..c79aa16639 100755 --- a/src/sonic-host-services/scripts/caclmgrd +++ b/src/sonic-host-services/scripts/caclmgrd @@ -43,7 +43,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): @@ -53,8 +53,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 @@ -346,12 +348,14 @@ class ControlPlaneAclManager(daemon_base.DaemonBase): 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 + + loopback_address_vals = list(loopback_networks.values()) - if not isinstance(loopback_networks[0], ipaddress.IPv4Network): + 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) @@ -371,10 +375,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 {} --source {} -j SNAT --to-source {}".format(str(soc_ipv4_addr), str(vlan_address), str(loopback_address))) diff --git a/src/sonic-host-services/tests/caclmgrd/caclmgrd_soc_rules_test.py b/src/sonic-host-services/tests/caclmgrd/caclmgrd_soc_rules_test.py index 880798aecf..ab18020e67 100644 --- a/src/sonic-host-services/tests/caclmgrd/caclmgrd_soc_rules_test.py +++ b/src/sonic-host-services/tests/caclmgrd/caclmgrd_soc_rules_test.py @@ -29,7 +29,7 @@ class TestCaclmgrdSoc(TestCase): @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 @@ -77,7 +77,7 @@ class TestCaclmgrdSoc(TestCase): @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 @@ -106,4 +106,4 @@ class TestCaclmgrdSoc(TestCase): 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/src/sonic-host-services/tests/caclmgrd/test_soc_rules_vectors.py b/src/sonic-host-services/tests/caclmgrd/test_soc_rules_vectors.py index 9d969b9cfe..8fb85fb8e5 100644 --- a/src/sonic-host-services/tests/caclmgrd/test_soc_rules_vectors.py +++ b/src/sonic-host-services/tests/caclmgrd/test_soc_rules_vectors.py @@ -18,7 +18,7 @@ CACLMGRD_SOC_TEST_VECTOR = [ "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 @@ CACLMGRD_SOC_TEST_VECTOR = [ }, }, "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', shell=True, 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', shell=True, universal_newlines=True, stdout=-1) ], "popen_attributes": { 'communicate.return_value': ('output', 'error'),