From 805c76acb12869536c078390f74a24cb1631ac3e Mon Sep 17 00:00:00 2001 From: Vivek Date: Tue, 16 Apr 2024 22:52:15 -0700 Subject: [PATCH] [xcvrd] Ignore processing ports that are not front-panel (#455) Ignore processing the front panel ports in xcvrd. This include ports with suffixes other than Ethernet and ports that have role field set to any value other than Ext. Signed-off-by: Vivek Reddy --- sonic-xcvrd/tests/test_xcvrd.py | 64 ++++++++++++++++++- .../xcvrd_utilities/port_event_helper.py | 43 +++++++++---- 2 files changed, 92 insertions(+), 15 deletions(-) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 8da6ded03..a3256d264 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -837,6 +837,60 @@ def test_is_error_sfp_status(self): assert not is_error_block_eeprom_reading(int(SFP_STATUS_INSERTED)) assert not is_error_block_eeprom_reading(int(SFP_STATUS_REMOVED)) + + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) + @patch('swsscommon.swsscommon.Table') + @patch('swsscommon.swsscommon.SubscriberStateTable') + @patch('swsscommon.swsscommon.Select.select') + def test_handle_front_panel_filter(self, mock_select, mock_sub_table, mock_swsscommon_table): + class DummyPortChangeEventHandler: + def __init__(self): + self.port_event_cache = [] + + def handle_port_change_event(self, port_event): + self.port_event_cache.append(port_event) + + CONFIG_DB = 'CONFIG_DB' + PORT_TABLE = swsscommon.CFG_PORT_TABLE_NAME + port_change_event_handler = DummyPortChangeEventHandler() + + mock_table = MagicMock() + mock_table.getKeys = MagicMock(return_value=['Ethernet0', 'Ethernet8', 'Ethernet16']) + mock_table.get = MagicMock(side_effect=[(True, (('index', 1), )), + (True, (('index', 2), ('role', 'Dpc'))), + (True, (('index', 3), ('role', 'Ext'))) + ]) + mock_swsscommon_table.return_value = mock_table + + mock_selectable = MagicMock() + side_effect_list = [ + ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), ('speed', '40000'))), + ('Ethernet8', swsscommon.SET_COMMAND, (('index', '2'), ('speed', '80000'), ('role', 'Dpc'))), + ('Ethernet16', swsscommon.SET_COMMAND, (('index', '3'), ('speed', '80000'), ('role', 'Ext'))), + (None, None, None) + ] + mock_selectable.pop = MagicMock(side_effect=side_effect_list) + mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) + mock_sub_table.return_value = mock_selectable + logger = MagicMock() + stop_event = threading.Event() + stop_event.is_set = MagicMock(return_value=False) + + observer = PortChangeObserver(DEFAULT_NAMESPACE, logger, stop_event, + port_change_event_handler.handle_port_change_event, + [{CONFIG_DB: PORT_TABLE}]) + + # Only Ethernet8 is filled in the role map + assert observer.port_role_map['Ethernet8'] == 'Dpc' + assert observer.port_role_map['Ethernet16'] == 'Ext' + assert 'Ethernet0' not in observer.port_role_map + + # Test basic single update event without filtering: + assert observer.handle_port_update_event() + assert len(port_change_event_handler.port_event_cache) == 2 + assert list(observer.port_event_cache.keys()) == [('Ethernet0', CONFIG_DB, PORT_TABLE), ('Ethernet16', CONFIG_DB, PORT_TABLE)] + + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) @patch('swsscommon.swsscommon.SubscriberStateTable') @patch('swsscommon.swsscommon.Select.select') @@ -1025,8 +1079,9 @@ def test_handle_port_config_change(self, mock_select, mock_sub_table): @patch('swsscommon.swsscommon.Table') def test_get_port_mapping(self, mock_swsscommon_table): mock_table = MagicMock() - mock_table.getKeys = MagicMock(return_value=['Ethernet0', 'Ethernet4', 'Ethernet-IB0']) - mock_table.get = MagicMock(side_effect=[(True, (('index', 1), )), (True, (('index', 2), )), (True, (('index', 3), ))]) + mock_table.getKeys = MagicMock(return_value=['Ethernet0', 'Ethernet4', 'Ethernet-IB0', 'Ethernet8']) + mock_table.get = MagicMock(side_effect=[(True, (('index', 1), )), (True, (('index', 2), )), + (True, (('index', 3), )), (True, (('index', 4), ('role', 'Dpc')))]) mock_swsscommon_table.return_value = mock_table port_mapping = get_port_mapping(DEFAULT_NAMESPACE) assert port_mapping.logical_port_list.count('Ethernet0') @@ -1044,6 +1099,11 @@ def test_get_port_mapping(self, mock_swsscommon_table): assert port_mapping.get_physical_to_logical(3) == None assert port_mapping.get_logical_to_physical('Ethernet-IB0') == None + assert port_mapping.logical_port_list.count('Ethernet8') == 0 + assert port_mapping.get_asic_id_for_logical_port('Ethernet8') == None + assert port_mapping.get_physical_to_logical(4) == None + assert port_mapping.get_logical_to_physical('Ethernet8') == None + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) @patch('swsscommon.swsscommon.SubscriberStateTable') @patch('swsscommon.swsscommon.Select.select') diff --git a/sonic-xcvrd/xcvrd/xcvrd_utilities/port_event_helper.py b/sonic-xcvrd/xcvrd/xcvrd_utilities/port_event_helper.py index 3c0dbffa3..a085bf96c 100644 --- a/sonic-xcvrd/xcvrd/xcvrd_utilities/port_event_helper.py +++ b/sonic-xcvrd/xcvrd/xcvrd_utilities/port_event_helper.py @@ -1,6 +1,5 @@ from sonic_py_common import daemon_base from sonic_py_common import multi_asic -from sonic_py_common.interface import backplane_prefix, inband_prefix, recirc_prefix from swsscommon import swsscommon SELECT_TIMEOUT_MSECS = 1000 @@ -10,7 +9,6 @@ {'STATE_DB': 'PORT_TABLE', 'FILTER': ['host_tx_ready']}, ] - class PortChangeEvent: PORT_ADD = 0 PORT_REMOVE = 1 @@ -66,6 +64,8 @@ def __init__(self, namespaces, logger, self.stop_event = stop_event self.port_change_event_handler = port_change_event_handler self.port_tbl_map = port_tbl_map + self.port_role_map = {} + self.refresh_role_map() self.subscribe_port_update_event() def apply_filter_to_fvp(self, filter, fvp): @@ -74,6 +74,16 @@ def apply_filter_to_fvp(self, filter, fvp): if key not in (set(filter) | set({'index', 'port_name', 'asic_id', 'op'})): del fvp[key] + def refresh_role_map(self): + for ns in self.namespaces: + cfg_db = daemon_base.db_connect("CONFIG_DB", namespace=ns) + port_table = swsscommon.Table(cfg_db, swsscommon.CFG_PORT_TABLE_NAME) + for key in port_table.getKeys(): + _, port_config = port_table.get(key) + port_config_dict = dict(port_config) + if port_config_dict.get(multi_asic.PORT_ROLE, None): + self.port_role_map[key] = port_config_dict[multi_asic.PORT_ROLE] + def subscribe_port_update_event(self): """ Subscribe to a particular DB's table and listen to only interested fields @@ -119,9 +129,21 @@ def handle_port_update_event(self): (port_name, op, fvp) = port_tbl.pop() if not port_name: break - if not validate_port(port_name): - continue + fvp = dict(fvp) if fvp is not None else {} + role = fvp.get(multi_asic.PORT_ROLE, None) + if role: + # If an internal port is broken out on the fly using DPB, + # the assumption here is that we would recieve CONFIG_DB or APPL_DB notification before STATE_DB + self.port_role_map[port_name] = role + else: + # role attribute might not be present for state DB + # notifs and thus need to maintain a cache + role = self.port_role_map.get(port_name, None) + + if not multi_asic.is_front_panel_port(port_name, role): + continue + self.logger.log_warning("$$$ {} handle_port_update_event() : op={} DB:{} Table:{} fvp {}".format( port_name, op, port_tbl.db_name, port_tbl.table_name, fvp)) if 'index' not in fvp: @@ -239,11 +261,6 @@ def logical_port_name_to_physical_port_list(self, port_name): else: return None -def validate_port(port): - if port.startswith((backplane_prefix(), inband_prefix(), recirc_prefix())) or '.' in port: - return False - return True - def subscribe_port_config_change(namespaces): sel = swsscommon.Select() asic_context = {} @@ -274,10 +291,10 @@ def read_port_config_change(asic_context, port_mapping, logger, port_change_even (key, op, fvp) = port_tbl.pop() if not key: break - if not validate_port(key): + fvp = dict(fvp) + if not multi_asic.is_front_panel_port(key, fvp.get(multi_asic.PORT_ROLE, None)): continue if op == swsscommon.SET_COMMAND: - fvp = dict(fvp) if 'index' not in fvp: continue @@ -316,10 +333,10 @@ def get_port_mapping(namespaces): config_db = daemon_base.db_connect("CONFIG_DB", namespace=namespace) port_table = swsscommon.Table(config_db, swsscommon.CFG_PORT_TABLE_NAME) for key in port_table.getKeys(): - if not validate_port(key): - continue _, port_config = port_table.get(key) port_config_dict = dict(port_config) + if not multi_asic.is_front_panel_port(key, port_config_dict.get(multi_asic.PORT_ROLE, None)): + continue port_change_event = PortChangeEvent(key, port_config_dict['index'], asic_id, PortChangeEvent.PORT_ADD) port_mapping.handle_port_change_event(port_change_event) return port_mapping