From 909ffaeddb7d947729e3b5cb272791c04d893f26 Mon Sep 17 00:00:00 2001 From: gpunathilell Date: Tue, 12 Aug 2025 16:55:59 +0000 Subject: [PATCH 1/6] Fix for smartswitch acl --- scripts/caclmgrd | 4 ++++ tests/caclmgrd/caclmgrd_chassis_midplane_test.py | 13 ++++++++++--- tests/caclmgrd/test_chassis_midplane_vectors.py | 3 +++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/scripts/caclmgrd b/scripts/caclmgrd index 2248665f..745bdddd 100755 --- a/scripts/caclmgrd +++ b/scripts/caclmgrd @@ -337,6 +337,10 @@ class ControlPlaneAclManager(logger.Logger): allow_internal_chassis_midplane_traffic.append(['iptables', '-A', 'INPUT', '-s', chassis_midplane_ip, '-d', chassis_midplane_ip, '-j', 'ACCEPT']) allow_internal_chassis_midplane_traffic.append(['iptables', '-A', 'INPUT', '-i', chassis_midplane_dev_name, '-j', 'ACCEPT']) + if device_info.is_smartswitch(): + # Allow traffic to the chassis midplane IP + allow_internal_chassis_midplane_traffic.append(['iptables', '-A', 'INPUT', '-d', '169.254.200.254','-j', 'ACCEPT']) + return allow_internal_chassis_midplane_traffic def generate_allow_internal_docker_ip_traffic_commands(self, namespace): diff --git a/tests/caclmgrd/caclmgrd_chassis_midplane_test.py b/tests/caclmgrd/caclmgrd_chassis_midplane_test.py index 85aaee5a..a7965f8c 100644 --- a/tests/caclmgrd/caclmgrd_chassis_midplane_test.py +++ b/tests/caclmgrd/caclmgrd_chassis_midplane_test.py @@ -35,9 +35,16 @@ def test_caclmgrd_chassis_midplane(self, test_name, test_data, fs): fs.create_file(DBCONFIG_PATH) # fake database_config.json with mock.patch("sonic_py_common.device_info.is_chassis", return_value=True): - with mock.patch("caclmgrd.ControlPlaneAclManager.run_commands_pipe", side_effect=["eth1-midplane", "1.0.0.33", "eth1-midplane", "1.0.0.33"]): + with mock.patch("sonic_py_common.device_info.is_smartswitch", return_value=False): + with mock.patch("caclmgrd.ControlPlaneAclManager.run_commands_pipe", side_effect=["eth1-midplane", "1.0.0.33", "eth1-midplane", "1.0.0.33"]): + caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd") + ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('') + self.assertListEqual(test_data["return"], ret) + ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('asic0') + self.assertListEqual([], ret) + with mock.patch("sonic_py_common.device_info.is_chassis", return_value=False): + with mock.patch("sonic_py_common.device_info.is_smartswitch", return_value=True): caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd") ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('') - self.assertListEqual(test_data["return"], ret) - ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('asic0') + self.assertListEqual(test_data["return_smartswitch"], ret) self.assertListEqual([], ret) diff --git a/tests/caclmgrd/test_chassis_midplane_vectors.py b/tests/caclmgrd/test_chassis_midplane_vectors.py index fc8d99fa..49456602 100644 --- a/tests/caclmgrd/test_chassis_midplane_vectors.py +++ b/tests/caclmgrd/test_chassis_midplane_vectors.py @@ -10,6 +10,9 @@ "return": [ ['iptables', '-A', 'INPUT', '-s', '1.0.0.33', '-d', '1.0.0.33', '-j', 'ACCEPT'], ['iptables', '-A', 'INPUT', '-i', 'eth1-midplane', '-j', 'ACCEPT'] + ], + "return_smartswitch": [ + ['iptables', '-A', 'INPUT', '-d', '169.254.200.254','-j', 'ACCEPT'] ] } ] From 862e61664b9d365d544b3b2a3bfd4143cc649c8b Mon Sep 17 00:00:00 2001 From: gpunathilell Date: Tue, 12 Aug 2025 20:37:07 +0000 Subject: [PATCH 2/6] fix test --- .../caclmgrd/caclmgrd_chassis_midplane_test.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/caclmgrd/caclmgrd_chassis_midplane_test.py b/tests/caclmgrd/caclmgrd_chassis_midplane_test.py index a7965f8c..3c42f389 100644 --- a/tests/caclmgrd/caclmgrd_chassis_midplane_test.py +++ b/tests/caclmgrd/caclmgrd_chassis_midplane_test.py @@ -34,17 +34,18 @@ def test_caclmgrd_chassis_midplane(self, test_name, test_data, fs): if not os.path.exists(DBCONFIG_PATH): fs.create_file(DBCONFIG_PATH) # fake database_config.json - with mock.patch("sonic_py_common.device_info.is_chassis", return_value=True): - with mock.patch("sonic_py_common.device_info.is_smartswitch", return_value=False): + mock_is_chassis = mock.MagicMock(return_value=True) + mock_is_smartswitch = mock.MagicMock(return_value=False) + + with mock.patch("sonic_py_common.device_info.is_chassis", mock_is_chassis): + with mock.patch("sonic_py_common.device_info.is_smartswitch", mock_is_smartswitch): with mock.patch("caclmgrd.ControlPlaneAclManager.run_commands_pipe", side_effect=["eth1-midplane", "1.0.0.33", "eth1-midplane", "1.0.0.33"]): caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd") ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('') self.assertListEqual(test_data["return"], ret) ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('asic0') self.assertListEqual([], ret) - with mock.patch("sonic_py_common.device_info.is_chassis", return_value=False): - with mock.patch("sonic_py_common.device_info.is_smartswitch", return_value=True): - caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd") - ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('') - self.assertListEqual(test_data["return_smartswitch"], ret) - self.assertListEqual([], ret) + mock_is_chassis.return_value = False + mock_is_smartswitch.return_value = True + ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('') + self.assertListEqual(test_data["return_smartswitch"], ret) \ No newline at end of file From 4060fa186d39ec1806107809f338c9af2ee6fac1 Mon Sep 17 00:00:00 2001 From: gpunathilell Date: Tue, 23 Sep 2025 20:27:19 +0000 Subject: [PATCH 3/6] Address review comment --- scripts/caclmgrd | 27 ++++++++++++- .../caclmgrd_chassis_midplane_test.py | 38 ++++++++++++++++++- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/scripts/caclmgrd b/scripts/caclmgrd index 745bdddd..66c9afbb 100755 --- a/scripts/caclmgrd +++ b/scripts/caclmgrd @@ -88,6 +88,7 @@ class ControlPlaneAclManager(logger.Logger): BFD_SESSION_TABLE = "BFD_SESSION_TABLE" VXLAN_TUNNEL_TABLE = "VXLAN_TUNNEL" DPU_TABLE = "DPU" + MID_PLANE_BRIDGE_TABLE = "MID_PLANE_BRIDGE" # To specify a port range instead of a single port, use iptables format: # separate start and end ports with a colon, e.g., "1000:2000" @@ -328,6 +329,29 @@ class ControlPlaneAclManager(logger.Logger): return midplane_dev_name, midplane_ip + def get_midplane_bridge_ip_from_configdb(self, namespace=DEFAULT_NAMESPACE): + """ + Method to get the IP prefix from ConfigDB using the path: + MID_PLANE_BRIDGE/GLOBAL/ip_prefix + + Usage example: + caclmgr = ControlPlaneAclManager("test") + ip_prefix = caclmgr.get_midplane_bridge_ip_from_configdb() + if ip_prefix: + print("Midplane bridge IP prefix:", ip_prefix) + """ + try: + config_db_connector = self.config_db_map[namespace] + midplane_bridge_table = config_db_connector.get_table(self.MID_PLANE_BRIDGE_TABLE) + if midplane_bridge_table and "GLOBAL" in midplane_bridge_table: + global_config = midplane_bridge_table["GLOBAL"] + if "ip_prefix" in global_config: + self.log_info("Retrieved midplane bridge IP prefix from ConfigDB: {}".format(global_config["ip_prefix"])) + return global_config["ip_prefix"].split("/")[0] + except Exception as e: + self.log_error("Failed to get midplane bridge IP from ConfigDB: {}".format(str(e))) + return "169.254.200.254" + def generate_allow_internal_chasis_midplane_traffic(self, namespace): allow_internal_chassis_midplane_traffic = [] if device_info.is_chassis() and not namespace: @@ -339,7 +363,8 @@ class ControlPlaneAclManager(logger.Logger): if device_info.is_smartswitch(): # Allow traffic to the chassis midplane IP - allow_internal_chassis_midplane_traffic.append(['iptables', '-A', 'INPUT', '-d', '169.254.200.254','-j', 'ACCEPT']) + midplane_bridge_ip = self.get_midplane_bridge_ip_from_configdb() + allow_internal_chassis_midplane_traffic.append(['iptables', '-A', 'INPUT', '-d', midplane_bridge_ip, '-j', 'ACCEPT']) return allow_internal_chassis_midplane_traffic diff --git a/tests/caclmgrd/caclmgrd_chassis_midplane_test.py b/tests/caclmgrd/caclmgrd_chassis_midplane_test.py index 3c42f389..fb7d924b 100644 --- a/tests/caclmgrd/caclmgrd_chassis_midplane_test.py +++ b/tests/caclmgrd/caclmgrd_chassis_midplane_test.py @@ -47,5 +47,39 @@ def test_caclmgrd_chassis_midplane(self, test_name, test_data, fs): self.assertListEqual([], ret) mock_is_chassis.return_value = False mock_is_smartswitch.return_value = True - ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('') - self.assertListEqual(test_data["return_smartswitch"], ret) \ No newline at end of file + # Mock the get_midplane_bridge_ip_from_configdb method for smartswitch test + with mock.patch.object(caclmgrd_daemon, 'get_midplane_bridge_ip_from_configdb', return_value="169.254.200.254"): + ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('') + self.assertListEqual(test_data["return_smartswitch"], ret) + + def test_get_midplane_bridge_ip_from_configdb(self): + """Test the get_midplane_bridge_ip_from_configdb method""" + # Mock ConfigDB data + mock_config_db = { + "MID_PLANE_BRIDGE": { + "GLOBAL": { + "ip_prefix": "169.254.200.254/24" + } + } + } + + # Set up the mock ConfigDB + MockConfigDb.set_config_db(mock_config_db) + + caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd") + + # Test with valid ConfigDB data + ip_address = caclmgrd_daemon.get_midplane_bridge_ip_from_configdb() + self.assertEqual(ip_address, "169.254.200.254") + + # Test with different IP prefix format + mock_config_db["MID_PLANE_BRIDGE"]["GLOBAL"]["ip_prefix"] = "10.0.0.1/16" + MockConfigDb.mod_config_db(mock_config_db) + ip_address = caclmgrd_daemon.get_midplane_bridge_ip_from_configdb() + self.assertEqual(ip_address, "10.0.0.1") + + # Test with missing ConfigDB data (should return default) + mock_config_db["MID_PLANE_BRIDGE"]["GLOBAL"] = {} + MockConfigDb.mod_config_db(mock_config_db) + ip_address = caclmgrd_daemon.get_midplane_bridge_ip_from_configdb() + self.assertEqual(ip_address, "169.254.200.254") \ No newline at end of file From ae308ec7c452ec0d1ebf8f28f3ccefa300b6ca52 Mon Sep 17 00:00:00 2001 From: gpunathilell Date: Tue, 23 Sep 2025 20:43:21 +0000 Subject: [PATCH 4/6] Fix test --- tests/caclmgrd/caclmgrd_chassis_midplane_test.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/caclmgrd/caclmgrd_chassis_midplane_test.py b/tests/caclmgrd/caclmgrd_chassis_midplane_test.py index fb7d924b..8ede7d88 100644 --- a/tests/caclmgrd/caclmgrd_chassis_midplane_test.py +++ b/tests/caclmgrd/caclmgrd_chassis_midplane_test.py @@ -54,8 +54,18 @@ def test_caclmgrd_chassis_midplane(self, test_name, test_data, fs): def test_get_midplane_bridge_ip_from_configdb(self): """Test the get_midplane_bridge_ip_from_configdb method""" - # Mock ConfigDB data + # Mock ConfigDB data with required tables mock_config_db = { + "DEVICE_METADATA": { + "localhost": { + "subtype": "ToR" + } + }, + "FEATURE": { + "feature1": { + "state": "enabled" + } + }, "MID_PLANE_BRIDGE": { "GLOBAL": { "ip_prefix": "169.254.200.254/24" From 6f454a40366d6c62e200f402c87b66783f2838af Mon Sep 17 00:00:00 2001 From: gpunathilell Date: Mon, 6 Oct 2025 21:55:46 +0000 Subject: [PATCH 5/6] Specify exception --- scripts/caclmgrd | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/caclmgrd b/scripts/caclmgrd index 66c9afbb..afe7625d 100755 --- a/scripts/caclmgrd +++ b/scripts/caclmgrd @@ -348,8 +348,9 @@ class ControlPlaneAclManager(logger.Logger): if "ip_prefix" in global_config: self.log_info("Retrieved midplane bridge IP prefix from ConfigDB: {}".format(global_config["ip_prefix"])) return global_config["ip_prefix"].split("/")[0] - except Exception as e: + except RuntimeError as e: self.log_error("Failed to get midplane bridge IP from ConfigDB: {}".format(str(e))) + raise e return "169.254.200.254" def generate_allow_internal_chasis_midplane_traffic(self, namespace): From df16be91b5aaf135baa84fe7548845adbb956ca0 Mon Sep 17 00:00:00 2001 From: gpunathilell Date: Tue, 14 Oct 2025 22:51:51 +0000 Subject: [PATCH 6/6] Review fixes --- scripts/caclmgrd | 12 +++++------ .../caclmgrd_chassis_midplane_test.py | 20 ++++++++++--------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/scripts/caclmgrd b/scripts/caclmgrd index afe7625d..5c1137a6 100755 --- a/scripts/caclmgrd +++ b/scripts/caclmgrd @@ -118,6 +118,7 @@ class ControlPlaneAclManager(logger.Logger): "multi_asic_ns_to_host_fwd":False } } + smartswitch_midplane_bridge_ip = "169.254.200.254" UPDATE_DELAY_SECS = 0.5 @@ -329,7 +330,7 @@ class ControlPlaneAclManager(logger.Logger): return midplane_dev_name, midplane_ip - def get_midplane_bridge_ip_from_configdb(self, namespace=DEFAULT_NAMESPACE): + def get_midplane_bridge_ip_from_configdb(self, config_db_connector): """ Method to get the IP prefix from ConfigDB using the path: MID_PLANE_BRIDGE/GLOBAL/ip_prefix @@ -341,7 +342,6 @@ class ControlPlaneAclManager(logger.Logger): print("Midplane bridge IP prefix:", ip_prefix) """ try: - config_db_connector = self.config_db_map[namespace] midplane_bridge_table = config_db_connector.get_table(self.MID_PLANE_BRIDGE_TABLE) if midplane_bridge_table and "GLOBAL" in midplane_bridge_table: global_config = midplane_bridge_table["GLOBAL"] @@ -351,9 +351,9 @@ class ControlPlaneAclManager(logger.Logger): except RuntimeError as e: self.log_error("Failed to get midplane bridge IP from ConfigDB: {}".format(str(e))) raise e - return "169.254.200.254" + return self.smartswitch_midplane_bridge_ip - def generate_allow_internal_chasis_midplane_traffic(self, namespace): + def generate_allow_internal_chasis_midplane_traffic(self, namespace, config_db_connector): allow_internal_chassis_midplane_traffic = [] if device_info.is_chassis() and not namespace: chassis_midplane_dev_name, chassis_midplane_ip = self.get_chassis_midplane_interface_ip() @@ -364,7 +364,7 @@ class ControlPlaneAclManager(logger.Logger): if device_info.is_smartswitch(): # Allow traffic to the chassis midplane IP - midplane_bridge_ip = self.get_midplane_bridge_ip_from_configdb() + midplane_bridge_ip = self.get_midplane_bridge_ip_from_configdb(config_db_connector) allow_internal_chassis_midplane_traffic.append(['iptables', '-A', 'INPUT', '-d', midplane_bridge_ip, '-j', 'ACCEPT']) return allow_internal_chassis_midplane_traffic @@ -675,7 +675,7 @@ class ControlPlaneAclManager(logger.Logger): iptables_cmds += self.generate_allow_internal_docker_ip_traffic_commands(namespace) # Add iptables commands to allow internal chasiss midplane traffic - iptables_cmds += self.generate_allow_internal_chasis_midplane_traffic(namespace) + iptables_cmds += self.generate_allow_internal_chasis_midplane_traffic(namespace, config_db_connector) # Add iptables/ip6tables commands to allow all incoming packets from established # connections or new connections which are related to established connections diff --git a/tests/caclmgrd/caclmgrd_chassis_midplane_test.py b/tests/caclmgrd/caclmgrd_chassis_midplane_test.py index 8ede7d88..3908698e 100644 --- a/tests/caclmgrd/caclmgrd_chassis_midplane_test.py +++ b/tests/caclmgrd/caclmgrd_chassis_midplane_test.py @@ -41,15 +41,16 @@ def test_caclmgrd_chassis_midplane(self, test_name, test_data, fs): with mock.patch("sonic_py_common.device_info.is_smartswitch", mock_is_smartswitch): with mock.patch("caclmgrd.ControlPlaneAclManager.run_commands_pipe", side_effect=["eth1-midplane", "1.0.0.33", "eth1-midplane", "1.0.0.33"]): caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd") - ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('') + config_db_connector = caclmgrd_daemon.config_db_map[''] + ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('', config_db_connector) self.assertListEqual(test_data["return"], ret) - ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('asic0') + ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('asic0', config_db_connector) self.assertListEqual([], ret) mock_is_chassis.return_value = False mock_is_smartswitch.return_value = True # Mock the get_midplane_bridge_ip_from_configdb method for smartswitch test with mock.patch.object(caclmgrd_daemon, 'get_midplane_bridge_ip_from_configdb', return_value="169.254.200.254"): - ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('') + ret = caclmgrd_daemon.generate_allow_internal_chasis_midplane_traffic('', config_db_connector) self.assertListEqual(test_data["return_smartswitch"], ret) def test_get_midplane_bridge_ip_from_configdb(self): @@ -74,22 +75,23 @@ def test_get_midplane_bridge_ip_from_configdb(self): } # Set up the mock ConfigDB - MockConfigDb.set_config_db(mock_config_db) + config_db_connector = MockConfigDb() + config_db_connector.set_config_db(mock_config_db) caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd") # Test with valid ConfigDB data - ip_address = caclmgrd_daemon.get_midplane_bridge_ip_from_configdb() + ip_address = caclmgrd_daemon.get_midplane_bridge_ip_from_configdb(config_db_connector) self.assertEqual(ip_address, "169.254.200.254") # Test with different IP prefix format mock_config_db["MID_PLANE_BRIDGE"]["GLOBAL"]["ip_prefix"] = "10.0.0.1/16" - MockConfigDb.mod_config_db(mock_config_db) - ip_address = caclmgrd_daemon.get_midplane_bridge_ip_from_configdb() + config_db_connector.mod_config_db(mock_config_db) + ip_address = caclmgrd_daemon.get_midplane_bridge_ip_from_configdb(config_db_connector) self.assertEqual(ip_address, "10.0.0.1") # Test with missing ConfigDB data (should return default) mock_config_db["MID_PLANE_BRIDGE"]["GLOBAL"] = {} - MockConfigDb.mod_config_db(mock_config_db) - ip_address = caclmgrd_daemon.get_midplane_bridge_ip_from_configdb() + config_db_connector.mod_config_db(mock_config_db) + ip_address = caclmgrd_daemon.get_midplane_bridge_ip_from_configdb(config_db_connector) self.assertEqual(ip_address, "169.254.200.254") \ No newline at end of file