[lldp]Fix the issue of only one field lldp_rem_time_mark in APPL_DB (#71

) Root cause: restart syncd and orchagent, other interfaces are down, but eth0 is still up. So, it is always in changed list and still it thinks the only changed field is lldp_rem_time_mark. But APPL_DB was flushed, the LLDP_ENTRY_TABLE:eth0 is empty. During every lldp-syncd daemon run function, it only updates the lldp_rem_time_mark field for eth0. And it doesn't have other chance to write other fields into the table, except shutdown/ no shutdown eth0 or restart lldp. How to fix: repopulate APPL_DB lldp table for changed interface if detects any new or deleted interfaces. Even syncd or orchagent restarts, other interfaces will down and then up, eth0 still has change to rewrite all fields Test: admin@str2-7260cx3-acs-12:/var/log$ sonic-db-cli APPL_DB FLUSHDB True admin@str2-7260cx3-acs-12:/var/log$ sonic-db-cli APPL_DB hgetall LLDP_ENTRY_TABLE:eth0 {'lldp_rem_time_mark': '504'} admin@str2-7260cx3-acs-12:/var/log$ sonic-db-cli APPL_DB hgetall LLDP_ENTRY_TABLE:eth0 {'lldp_rem_time_mark': '504'} admin@str2-7260cx3-acs-12:/var/log$ sonic-db-cli APPL_DB hgetall LLDP_ENTRY_TABLE:Ethernet68 {'lldp_rem_time_mark': '519'} admin@str2-7260cx3-acs-12:/var/log$ sudo config interface shutdown Ethernet68 admin@str2-7260cx3-acs-12:/var/log$ sonic-db-cli APPL_DB hgetall LLDP_ENTRY_TABLE:Ethernet68 {} admin@str2-7260cx3-acs-12:/var/log$ sonic-db-cli APPL_DB hgetall LLDP_ENTRY_TABLE:eth0 {'lldp_rem_sys_cap_enabled': '28 00', 'lldp_rem_man_addr': '', 'lldp_rem_sys_desc': 'Cisco Nexus Operating System (NX-OS) Software 6.0(2)U6(7) TAC support: http://www.cisco.com/tac Copyright (c) 2002-2016, Cisco Systems, Inc. All rights reserved.', 'lldp_rem_index': '1', 'lldp_rem_port_desc': 'Ethernet1/5', 'lldp_rem_chassis_id_subtype': '4', 'lldp_rem_port_id': 'Ethernet1/5', 'lldp_rem_port_id_subtype': '5', 'lldp_rem_time_mark': '695', 'lldp_rem_sys_name': 'STR-MGSW-254-C14.ntwk.msn.net', 'lldp_rem_sys_cap_supported': '28 00', 'lldp_rem_chassis_id': 'ac:4a:67:d4:fe:ac'} Microsoft work item 30233240
sonic-net · Dec 5, 2024 · b0ea01f · b0ea01f
1 parent 7ad34f6
commit b0ea01f
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 14 deletions.
diff --git a/src/lldp_syncd/daemon.py b/src/lldp_syncd/daemon.py
@@ -390,25 +390,36 @@ def sync(self, parsed_update):
 
         new, changed, deleted = self.cache_diff(self.interfaces_cache, parsed_update)
 
-        # For changed elements, if only lldp_rem_time_mark changed, update its value, otherwise delete and repopulate
-        for interface in changed:
-            if re.match(SONIC_ETHERNET_RE_PATTERN, interface) is None:
-                logger.warning("Ignoring interface '{}'".format(interface))
-                continue
-            table_key = ':'.join([LldpSyncDaemon.LLDP_ENTRY_TABLE, interface])
-            if self.is_only_time_mark_modified(self.interfaces_cache[interface], parsed_update[interface]):
-                self.db_connector.set(self.db_connector.APPL_DB, table_key, 'lldp_rem_time_mark', parsed_update[interface]['lldp_rem_time_mark'], blocking=True)
-                logger.debug("Only sync'd interface {} lldp_rem_time_mark: {}".format(interface, parsed_update[interface]['lldp_rem_time_mark']))
-            else:
+        if new or deleted:
+            # If detects any new or deleted interfaces, repopulate for changed interfaces
+            for interface in changed:
+                if re.match(SONIC_ETHERNET_RE_PATTERN, interface) is None:
+                    logger.warning("Ignoring interface '{}'".format(interface))
+                    continue
+                table_key = ':'.join([LldpSyncDaemon.LLDP_ENTRY_TABLE, interface])
                 self.db_connector.delete(self.db_connector.APPL_DB, table_key)
                 self.db_connector.hmset(self.db_connector.APPL_DB, table_key, parsed_update[interface])
-                logger.debug("Sync'd changed interface {} : {}".format(interface, parsed_update[interface]))
+                logger.info("Force repopulate the changed interface {} : {}".format(interface, parsed_update[interface]))
+        else:
+            # For changed elements, if only lldp_rem_time_mark changed, update its value, otherwise delete and repopulate
+            for interface in changed:
+                if re.match(SONIC_ETHERNET_RE_PATTERN, interface) is None:
+                    logger.warning("Ignoring interface '{}'".format(interface))
+                    continue
+                table_key = ':'.join([LldpSyncDaemon.LLDP_ENTRY_TABLE, interface])
+                if self.is_only_time_mark_modified(self.interfaces_cache[interface], parsed_update[interface]):
+                    self.db_connector.set(self.db_connector.APPL_DB, table_key, 'lldp_rem_time_mark', parsed_update[interface]['lldp_rem_time_mark'], blocking=True)
+                    logger.debug("Only sync'd interface {} lldp_rem_time_mark: {}".format(interface, parsed_update[interface]['lldp_rem_time_mark']))
+                else:
+                    self.db_connector.delete(self.db_connector.APPL_DB, table_key)
+                    self.db_connector.hmset(self.db_connector.APPL_DB, table_key, parsed_update[interface])
+                    logger.info("Repopulate for changed interface {} : {}".format(interface, parsed_update[interface]))
         self.interfaces_cache = parsed_update
         # Delete LLDP_ENTRIES which are missing
         for interface in deleted:
             table_key = ':'.join([LldpSyncDaemon.LLDP_ENTRY_TABLE, interface])
             self.db_connector.delete(self.db_connector.APPL_DB, table_key)
-            logger.debug("Delete table_key: {}".format(table_key))
+            logger.info("Delete table_key: {}".format(table_key))
         # Repopulate LLDP_ENTRY_TABLE by adding new elements
         for interface in new:
             if re.match(SONIC_ETHERNET_RE_PATTERN, interface) is None:
@@ -417,4 +428,4 @@ def sync(self, parsed_update):
             # port_table_key = LLDP_ENTRY_TABLE:INTERFACE_NAME;
             table_key = ':'.join([LldpSyncDaemon.LLDP_ENTRY_TABLE, interface])
             self.db_connector.hmset(self.db_connector.APPL_DB, table_key, parsed_update[interface])
-            logger.debug("Add new interface {} : {}".format(interface, parsed_update[interface]))
+            logger.info("Add new interface {} : {}".format(interface, parsed_update[interface]))
diff --git a/tests/test_lldpSyncDaemon.py b/tests/test_lldpSyncDaemon.py
@@ -18,7 +18,7 @@
 from swsscommon.swsscommon import SonicV2Connector
 
 INPUT_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'subproc_outputs')
-
+TABLE_PREFIX = "LLDP_ENTRY_TABLE:"
 
 def create_dbconnector():
     db = SonicV2Connector()
@@ -218,3 +218,56 @@ def test_invalid_chassis_name(self, mock_check_output):
         '''
         result = self.daemon.source_update()
         self.assertIsNone(result)
+
+
+    def test_changed_interface(self):
+        parsed_update = self.daemon.parse_update(self._json)
+        self.daemon.sync(parsed_update)
+        db = create_dbconnector()
+        keys = db.keys(db.APPL_DB)
+        # Check if each lldp_rem_time_mark is changed
+        dump = {}
+        for k in keys:
+            if k != 'LLDP_LOC_CHASSIS':
+                if TABLE_PREFIX + 'eth0' == k or TABLE_PREFIX + 'Ethernet0' == k:
+                    dump[k] = db.get(db.APPL_DB, k, 'lldp_rem_time_mark')
+
+        time.sleep(1)
+        # simulate lldp_rem_time_mark was changed or port description was changed or interface was removed
+        changed_json = self._json.copy()
+        changed_json['lldp']['interface'][0]['eth0']['age'] = '0 day, 05:09:12'
+        changed_json['lldp']['interface'][1]['Ethernet0']['age'] = '0 day, 05:09:15'
+
+        parsed_update = self.daemon.parse_update(changed_json)
+        self.daemon.sync(parsed_update)
+        keys = db.keys(db.APPL_DB)
+
+        jo = {}
+        for k in keys:
+            if k != 'LLDP_LOC_CHASSIS':
+                if TABLE_PREFIX + 'eth0' == k or TABLE_PREFIX + 'Ethernet0' == k:
+                    jo[k] = db.get(db.APPL_DB, k, 'lldp_rem_time_mark')
+                    self.assertEqual(int(jo[k]), int(dump[k])+10)
+                else:
+                    jo[k] = db.get_all(db.APPL_DB, k)
+        time.sleep(1)
+        # simulate lldp_rem_time_mark was changed or port description was changed or interface was removed
+        changed_json = self._json.copy()
+        changed_json['lldp']['interface'][0]['eth0']['age'] = '0 day, 05:09:12'
+        changed_json['lldp']['interface'][1]['Ethernet0']['port']['descr'] = 'Ethernet1'
+
+        parsed_update = self.daemon.parse_update(changed_json)
+        self.daemon.sync(parsed_update)
+        keys = db.keys(db.APPL_DB)
+
+        jo = {}
+        for k in keys:
+            if k != 'LLDP_LOC_CHASSIS':
+                if TABLE_PREFIX + 'eth0' == k:
+                    jo[k] = db.get(db.APPL_DB, k, 'lldp_rem_time_mark')
+                    self.assertEqual(int(jo[k]), int(dump[k])+10)
+                elif TABLE_PREFIX + 'Ethernet0' == k:
+                    jo[k] = db.get(db.APPL_DB, k, 'lldp_rem_port_desc')
+                    self.assertEqual(jo[k], 'Ethernet1')
+                else:
+                    jo[k] = db.get_all(db.APPL_DB, k)