From 7205f79d5f39ef284437173982f0de1e9d470817 Mon Sep 17 00:00:00 2001 From: Zhaohui Sun Date: Wed, 4 Dec 2024 08:06:44 +0000 Subject: [PATCH 1/4] [lldp]Fix the issue of only one field lldp_rem_time_mark in APPL_DB Signed-off-by: Zhaohui Sun --- src/lldp_syncd/daemon.py | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/lldp_syncd/daemon.py b/src/lldp_syncd/daemon.py index 73c9675..dd172f7 100644 --- 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])) From b786ada151d19261dc00311b1672093482b8edc8 Mon Sep 17 00:00:00 2001 From: Zhaohui Sun Date: Thu, 5 Dec 2024 09:27:02 +0000 Subject: [PATCH 2/4] Add unit test Signed-off-by: Zhaohui Sun --- tests/test_lldpSyncDaemon.py | 95 +++++++++++++++++++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-) diff --git a/tests/test_lldpSyncDaemon.py b/tests/test_lldpSyncDaemon.py index 727ace5..e51dce6 100644 --- 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,96 @@ def test_invalid_chassis_name(self, mock_check_output): ''' result = self.daemon.source_update() self.assertIsNone(result) + + + def test_new_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_port_desc') + elif TABLE_PREFIX + 'Ethernet100' == k: + dump[k] = db.get(db.APPL_DB, k, 'lldp_rem_port_desc') + + time.sleep(1) + # simulate lldp_rem_time_mark was changed or port description was changed or interface was removed + new_json = self._json.copy() + new_interface = { + "Ethernet1": { + "rid": "1", + "port": { + "id": { + "type": "ifname", + "value": "Ethernet1" + }, + "descr": "Ethernet1, this is a port description", + "mfs": "9236" + }, + "via": "LLDP", + "chassis": { + "switch13": { + "mgmt-ip": "10.3.147.196", + "id": { + "type": "mac", + "value": "00:11:22:33:44:55" + }, + "ttl": "120", + "descr": "Ethernet1, I'm a little teapot.", + "capability": [ + { + "type": "Bridge", + "enabled": True + }, + { + "type": "Router", + "enabled": True + } + ] + } + }, + "age": "0 day, 05:09:05", + "vlan": [ + { + "vlan-id": "101", + "pvid": True + }, + { + "vlan-id": "201", + "value": "vlan201" + } + ] + } + } + new_json['lldp']['interface'].append(new_interface) + + parsed_update = self.daemon.parse_update(new_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_port_desc') + self.assertEqual(jo[k], dump[k]) + elif TABLE_PREFIX + 'Ethernet1' == k: + jo[k] = db.get(db.APPL_DB, k, 'lldp_rem_port_id') + self.assertEqual(jo[k], "Ethernet1") + jo[k] = db.get(db.APPL_DB, k, 'lldp_rem_port_desc') + self.assertEqual(jo[k], "Ethernet1, this is a port description") + else: + jo[k] = db.get_all(db.APPL_DB, k) + + #Find and remove the Ethernet1 dictionary, restore test data + for interface_dict in new_json["lldp"]["interface"]: + if "Ethernet1" in interface_dict: + new_json["lldp"]["interface"].remove(interface_dict) + break # Exit loop after removing + parsed_update = self.daemon.parse_update(new_json) + self.daemon.sync(parsed_update) + keys = db.keys(db.APPL_DB) From 048bbdfef4c3bdfc6c73cc67cd7d138aa58a98c2 Mon Sep 17 00:00:00 2001 From: Zhaohui Sun Date: Thu, 5 Dec 2024 09:48:57 +0000 Subject: [PATCH 3/4] change unit test case Signed-off-by: Zhaohui Sun --- tests/test_lldpSyncDaemon.py | 77 ++++-------------------------------- 1 file changed, 8 insertions(+), 69 deletions(-) diff --git a/tests/test_lldpSyncDaemon.py b/tests/test_lldpSyncDaemon.py index e51dce6..201a043 100644 --- a/tests/test_lldpSyncDaemon.py +++ b/tests/test_lldpSyncDaemon.py @@ -220,7 +220,7 @@ def test_invalid_chassis_name(self, mock_check_output): self.assertIsNone(result) - def test_new_interface(self): + def test_changed_interface(self): parsed_update = self.daemon.parse_update(self._json) self.daemon.sync(parsed_update) db = create_dbconnector() @@ -230,62 +230,15 @@ def test_new_interface(self): 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_port_desc') - elif TABLE_PREFIX + 'Ethernet100' == k: - dump[k] = db.get(db.APPL_DB, k, 'lldp_rem_port_desc') + 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 - new_json = self._json.copy() - new_interface = { - "Ethernet1": { - "rid": "1", - "port": { - "id": { - "type": "ifname", - "value": "Ethernet1" - }, - "descr": "Ethernet1, this is a port description", - "mfs": "9236" - }, - "via": "LLDP", - "chassis": { - "switch13": { - "mgmt-ip": "10.3.147.196", - "id": { - "type": "mac", - "value": "00:11:22:33:44:55" - }, - "ttl": "120", - "descr": "Ethernet1, I'm a little teapot.", - "capability": [ - { - "type": "Bridge", - "enabled": True - }, - { - "type": "Router", - "enabled": True - } - ] - } - }, - "age": "0 day, 05:09:05", - "vlan": [ - { - "vlan-id": "101", - "pvid": True - }, - { - "vlan-id": "201", - "value": "vlan201" - } - ] - } - } - new_json['lldp']['interface'].append(new_interface) + 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(new_json) + parsed_update = self.daemon.parse_update(changed_json) self.daemon.sync(parsed_update) keys = db.keys(db.APPL_DB) @@ -293,21 +246,7 @@ def test_new_interface(self): 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_port_desc') - self.assertEqual(jo[k], dump[k]) - elif TABLE_PREFIX + 'Ethernet1' == k: - jo[k] = db.get(db.APPL_DB, k, 'lldp_rem_port_id') - self.assertEqual(jo[k], "Ethernet1") - jo[k] = db.get(db.APPL_DB, k, 'lldp_rem_port_desc') - self.assertEqual(jo[k], "Ethernet1, this is a port description") + 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) - - #Find and remove the Ethernet1 dictionary, restore test data - for interface_dict in new_json["lldp"]["interface"]: - if "Ethernet1" in interface_dict: - new_json["lldp"]["interface"].remove(interface_dict) - break # Exit loop after removing - parsed_update = self.daemon.parse_update(new_json) - self.daemon.sync(parsed_update) - keys = db.keys(db.APPL_DB) From 5f3e714c968660a0eda338e044e173e9ec727c6a Mon Sep 17 00:00:00 2001 From: Zhaohui Sun Date: Thu, 5 Dec 2024 09:58:10 +0000 Subject: [PATCH 4/4] increase test coverage Signed-off-by: Zhaohui Sun --- tests/test_lldpSyncDaemon.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_lldpSyncDaemon.py b/tests/test_lldpSyncDaemon.py index 201a043..dec48ec 100644 --- a/tests/test_lldpSyncDaemon.py +++ b/tests/test_lldpSyncDaemon.py @@ -250,3 +250,24 @@ def test_changed_interface(self): 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)