From 0f6eb29460706457c0a55da2e4b638f85cae21af Mon Sep 17 00:00:00 2001 From: Sudharsan Dhamal Gopalarathnam Date: Mon, 9 May 2022 10:58:00 -0700 Subject: [PATCH] [caclmgrd]Added logic to allow BFD port numbers (#10735) * [caclmgrd]Added logic to allow BFD port numbers --- src/sonic-host-services/scripts/caclmgrd | 31 ++++++++++-- .../tests/caclmgrd/caclmgrd_bfd_test.py | 50 +++++++++++++++++++ .../tests/caclmgrd/caclmgrd_dhcp_test.py | 22 ++++---- .../tests/caclmgrd/test_bfd_vectors.py | 29 +++++++++++ 4 files changed, 117 insertions(+), 15 deletions(-) create mode 100644 src/sonic-host-services/tests/caclmgrd/caclmgrd_bfd_test.py create mode 100644 src/sonic-host-services/tests/caclmgrd/test_bfd_vectors.py diff --git a/src/sonic-host-services/scripts/caclmgrd b/src/sonic-host-services/scripts/caclmgrd index a65f05a345..914547ad53 100755 --- a/src/sonic-host-services/scripts/caclmgrd +++ b/src/sonic-host-services/scripts/caclmgrd @@ -59,6 +59,8 @@ class ControlPlaneAclManager(daemon_base.DaemonBase): ACL_TABLE_TYPE_CTRLPLANE = "CTRLPLANE" + BFD_SESSION_TABLE = "BFD_SESSION_TABLE" + # 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" ACL_SERVICES = { @@ -87,6 +89,7 @@ class ControlPlaneAclManager(daemon_base.DaemonBase): UPDATE_DELAY_SECS = 0.5 DualToR = False + bfdAllowed = False def __init__(self, log_identifier): super(ControlPlaneAclManager, self).__init__(log_identifier) @@ -170,6 +173,7 @@ class ControlPlaneAclManager(daemon_base.DaemonBase): self.log_error("Error running command '{}'".format(cmd)) elif stdout: return stdout.rstrip('\n') + return "" def parse_int_to_tcp_flags(self, hex_value): tcp_flags_str = "" @@ -705,6 +709,13 @@ class ControlPlaneAclManager(daemon_base.DaemonBase): self.update_thread[namespace] = None return + def allow_bfd_protocol(self, namespace): + iptables_cmds = [] + # Add iptables/ip6tables commands to allow all BFD singlehop and multihop sessions + iptables_cmds.append(self.iptables_cmd_ns_prefix[namespace] + "iptables -I INPUT 2 -p udp -m multiport --dports 3784,4784 -j ACCEPT") + iptables_cmds.append(self.iptables_cmd_ns_prefix[namespace] + "ip6tables -I INPUT 2 -p udp -m multiport --dports 3784,4784 -j ACCEPT") + self.run_commands(iptables_cmds) + def run(self): # Set select timeout to 1 second SELECT_TIMEOUT_MS = 1000 @@ -730,12 +741,12 @@ class ControlPlaneAclManager(daemon_base.DaemonBase): state_db_id = swsscommon.SonicDBConfig.getDbId("STATE_DB") dhcp_packet_mark_tbl = {} + # set up state_db connector + state_db_connector = swsscommon.DBConnector("STATE_DB", 0) + if self.DualToR: self.log_info("Dual ToR mode") - # set up state_db connector - state_db_connector = swsscommon.DBConnector("STATE_DB", 0) - subscribe_mux_cable = swsscommon.SubscriberStateTable(state_db_connector, self.MUX_CABLE_TABLE) sel.addSelectable(subscribe_mux_cable) @@ -746,6 +757,10 @@ class ControlPlaneAclManager(daemon_base.DaemonBase): for namespace in list(self.config_db_map.keys()): self.setup_dhcp_chain(namespace) + # This should be migrated from state_db BFD session table to feature_table in the future when feature table support gets added for BFD + subscribe_bfd_session = swsscommon.SubscriberStateTable(state_db_connector, self.BFD_SESSION_TABLE) + sel.addSelectable(subscribe_bfd_session) + # Map of Namespace <--> susbcriber table's object config_db_subscriber_table_map = {} @@ -785,6 +800,16 @@ class ControlPlaneAclManager(daemon_base.DaemonBase): db_id = redisSelectObj.getDbConnector().getDbId() if db_id == state_db_id: + while True: + key, op, fvs = subscribe_bfd_session.pop() + if not key: + break + + if op == 'SET' and not self.bfdAllowed: + self.allow_bfd_protocol(namespace) + self.bfdAllowed = True + sel.removeSelectable(subscribe_bfd_session) + if self.DualToR: '''dhcp packet mark update''' while True: diff --git a/src/sonic-host-services/tests/caclmgrd/caclmgrd_bfd_test.py b/src/sonic-host-services/tests/caclmgrd/caclmgrd_bfd_test.py new file mode 100644 index 0000000000..358d4c413b --- /dev/null +++ b/src/sonic-host-services/tests/caclmgrd/caclmgrd_bfd_test.py @@ -0,0 +1,50 @@ +import os +import sys +import swsscommon + +from parameterized import parameterized +from sonic_py_common.general import load_module_from_source +from unittest import TestCase, mock +from pyfakefs.fake_filesystem_unittest import patchfs + +from .test_bfd_vectors import CACLMGRD_BFD_TEST_VECTOR +from tests.common.mock_configdb import MockConfigDb +from unittest.mock import MagicMock, patch + +DBCONFIG_PATH = '/var/run/redis/sonic-db/database_config.json' + +class TestCaclmgrdBfd(TestCase): + """ + Test caclmgrd bfd + """ + def setUp(self): + swsscommon.swsscommon.ConfigDBConnector = MockConfigDb + test_path = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + modules_path = os.path.dirname(test_path) + scripts_path = os.path.join(modules_path, "scripts") + sys.path.insert(0, modules_path) + caclmgrd_path = os.path.join(scripts_path, 'caclmgrd') + self.caclmgrd = load_module_from_source('caclmgrd', caclmgrd_path) + + @parameterized.expand(CACLMGRD_BFD_TEST_VECTOR) + @patchfs + def test_caclmgrd_bfd(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.allow_bfd_protocol('') + mocked_subprocess.Popen.assert_has_calls(test_data["expected_subprocess_calls"], any_order=True) + diff --git a/src/sonic-host-services/tests/caclmgrd/caclmgrd_dhcp_test.py b/src/sonic-host-services/tests/caclmgrd/caclmgrd_dhcp_test.py index 176dec1b50..a6eae7ba12 100644 --- a/src/sonic-host-services/tests/caclmgrd/caclmgrd_dhcp_test.py +++ b/src/sonic-host-services/tests/caclmgrd/caclmgrd_dhcp_test.py @@ -10,23 +10,21 @@ from pyfakefs.fake_filesystem_unittest import patchfs from .test_dhcp_vectors import CACLMGRD_DHCP_TEST_VECTOR from tests.common.mock_configdb import MockConfigDb - DBCONFIG_PATH = '/var/run/redis/sonic-db/database_config.json' - -swsscommon.swsscommon.ConfigDBConnector = MockConfigDb -test_path = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) -modules_path = os.path.dirname(test_path) -scripts_path = os.path.join(modules_path, "scripts") -sys.path.insert(0, modules_path) -caclmgrd_path = os.path.join(scripts_path, 'caclmgrd') -caclmgrd = load_module_from_source('caclmgrd', caclmgrd_path) - - class TestCaclmgrdDhcp(TestCase): """ Test caclmgrd dhcp """ + def setUp(self): + swsscommon.ConfigDBConnector = MockConfigDb + test_path = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + modules_path = os.path.dirname(test_path) + scripts_path = os.path.join(modules_path, "scripts") + sys.path.insert(0, modules_path) + caclmgrd_path = os.path.join(scripts_path, 'caclmgrd') + self.caclmgrd = load_module_from_source('caclmgrd', caclmgrd_path) + @parameterized.expand(CACLMGRD_DHCP_TEST_VECTOR) @patchfs def test_caclmgrd_dhcp(self, test_name, test_data, fs): @@ -46,7 +44,7 @@ class TestCaclmgrdDhcp(TestCase): mark = test_data["mark"] - caclmgrd_daemon = caclmgrd.ControlPlaneAclManager("caclmgrd") + caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd") mux_update = test_data["mux_update"] for key,data in mux_update: diff --git a/src/sonic-host-services/tests/caclmgrd/test_bfd_vectors.py b/src/sonic-host-services/tests/caclmgrd/test_bfd_vectors.py new file mode 100644 index 0000000000..35340849bd --- /dev/null +++ b/src/sonic-host-services/tests/caclmgrd/test_bfd_vectors.py @@ -0,0 +1,29 @@ +from unittest.mock import call +import subprocess + +""" + caclmgrd bfd test vector +""" +CACLMGRD_BFD_TEST_VECTOR = [ + [ + "BFD_SESSION_TEST", + { + "config_db": { + "DEVICE_METADATA": { + "localhost": { + "subtype": "DualToR", + "type": "ToRRouter", + } + }, + }, + "expected_subprocess_calls": [ + call("iptables -I INPUT 2 -p udp -m multiport --dports 3784,4784 -j ACCEPT", shell=True, universal_newlines=True, stdout=subprocess.PIPE), + call("ip6tables -I INPUT 2 -p udp -m multiport --dports 3784,4784 -j ACCEPT", shell=True, universal_newlines=True, stdout=subprocess.PIPE) + ], + "popen_attributes": { + 'communicate.return_value': ('output', 'error'), + }, + "call_rc": 0, + } + ] +]