[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 <[email protected]>

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')

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