[xcvrd] Re-organize transceiver DOM and STATUS tables + enable link event handling for DOM monitoring + DOM polling based on physical port by mihirpat1 · Pull Request #604 · sonic-net/sonic-platform-daemons

mihirpat1 · 2025-04-03T23:17:23Z

Description

The transceiver DOM and Status tables are now being enhanced to be in-line with HLD CMIS_Diagnostic_Monitoring_Overview_in_SONiC
Relevant metadata are being added for DOM and Status related tables to capture all flag changes
Ensuring that the change count is initialized to 0 and last set/clear time fields are set to N/A if the corresponding metadata table is created for the first time post xcvrd boot-up or transceiver insertion.
All the diagnostic tables (except for metadata tables) will now have last_update_time field to capture the last table update time.
Metadata tables update information only if there is a change in value between the current read and previously read value. Hence, not planning to add last_update_time field for metadata tables
Created TRANSCEIVER_STATUS_SW table to store information for the following fields
- cmis_state
- status
- error
Removed the diagnostic tables for non-primary subports for breakout ports (except for TRANSCEIVER_STATUS_SW table). Also, modified _handle_port_add function to naturally sort the logical port list stored in physical_to_logical dictionary.
Enabled DOM polling based on physical port and hence, populating diagnostic tables only for the first subports for breakout port configurations.
Enabled support to capture flag relevant diagnostic tables as part of link change event handling. Please refer to https://github.com/sonic-net/SONiC/blob/master/doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md#522-link-change-event-detection for further details

Unrelated issues being addressed

Added back is_replaceable field to TRANSCEIVER_INFO table for CMIS optics. This field was removed by a recent commit
Removed an error message if get_transceiver_vdm_thresholds raises NotImplementedError exception. This is done to avoid showing error message for non-CMIS modules since these module do not support VDM functionality
Deleting TRANSCEIVER_INFO table upon xcvrd initialization if transceiver is not present anymore. This scenario can happen if the transceiver was removed while xcvrd was not running
Fixes [Master] SffLoggerForPortUpdateEvent class does not have log_debug function #605
Fixes Bug: non-CMIS transceivers will fail to get VDM thresholds and log error is printed sonic-buildimage#22220
Using SysLogger for DomInfoUpdateTask instead of using xcvrd's helper_logger
Modified post_port_active_apsel_to_db to initialize active_apsel_hostlaneX to N/A for unrelated lanes wherein X represents the lane number from 1 to 8

Motivation and Context

All the changes in this PR are done to ensure that xcvrd is in-line with the HLD CMIS_Diagnostic_Monitoring_Overview_in_SONiC

How Has This Been Tested?

Please refer to sonic-net/sonic-utilities#3844 for the test details

Additional Information (Optional)

Related PRs
sonic-net/SONiC#1954
sonic-net/sonic-platform-common#556
sonic-net/sonic-utilities#3844

Fixes sonic-net/sonic-buildimage#22150

MSFT ADO - 29932118

…daemons

mssonicbld · 2025-04-03T23:17:31Z

/azp run

azure-pipelines · 2025-04-03T23:17:39Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

sonic-xcvrd/xcvrd/dom/utilities/dom_sensor/db_utils.py:26

Consider using an f-string for proper variable interpolation in this error message. For example, change the line to: f"Post port dom sensor info to db failed for {logical_port_name} as no asic index found".

self.logger.log_error("Post port dom sensor info to db failed for {logical_port_name} as no asic index found")

mssonicbld · 2025-04-03T23:21:26Z

/azp run

azure-pipelines · 2025-04-03T23:21:34Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Copilot · 2025-04-03T23:31:36Z

sonic-xcvrd/xcvrd/dom/utilities/dom_sensor/db_utils.py

+
+        asic_index = self.port_mapping.get_asic_id_for_logical_port(logical_port_name)
+        if asic_index is None:
+            self.logger.log_error("Post port dom flags to db failed for {logical_port_name} "


The subsequent log_error call uses "{logical_port_name}" without f-string interpolation. Update the log message to use f"..." so that the actual logical_port_name is printed.

Suggested change

self.logger.log_error("Post port dom flags to db failed for {logical_port_name} "

self.logger.log_error(f"Post port dom flags to db failed for {logical_port_name} "

Copilot · 2025-04-03T23:31:36Z

sonic-xcvrd/xcvrd/dom/utilities/dom_sensor/db_utils.py

+    def post_port_dom_thresholds_to_db(self, logical_port_name, db_cache=None):
+        asic_index = self.port_mapping.get_asic_id_for_logical_port(logical_port_name)
+        if asic_index is None:
+            self.logger.log_error("Post port dom thresholds to db failed for {logical_port_name} "


The log_error message below also uses "{logical_port_name}" without proper interpolation. Converting the log message to an f-string will ensure the correct value is displayed.

Suggested change

self.logger.log_error("Post port dom thresholds to db failed for {logical_port_name} "

self.logger.log_error(f"Post port dom thresholds to db failed for {logical_port_name} "

mssonicbld · 2025-04-03T23:36:09Z

/azp run

azure-pipelines · 2025-04-03T23:36:17Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

sonic-xcvrd/xcvrd/xcvrd_utilities/xcvr_table_helper.py:21

For consistency with the other table constants that include a '_TABLE' suffix, consider renaming this constant to 'TRANSCEIVER_STATUS_FLAG_CHANGE_COUNT_TABLE'.

TRANSCEIVER_STATUS_FLAG_CHANGE_COUNT = 'TRANSCEIVER_STATUS_FLAG_CHANGE_COUNT'

sonic-xcvrd/xcvrd/xcvrd.py:252

The word 'execption' appears to be a typo; consider correcting it to 'exception'.

#continue to process next port since execption could be raised due to port reset, transceiver removal

mssonicbld · 2025-04-04T00:04:59Z

/azp run

azure-pipelines · 2025-04-04T00:05:08Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

sonic-xcvrd/xcvrd/dom/utilities/vdm/db_utils.py:90

The variable 'flag_data' is used in the metadata update block without being defined in the current scope. Please review and define 'flag_data' or adjust the condition accordingly.

if flag_data and threshold_value_dict:

mssonicbld · 2025-04-04T17:12:00Z

/azp run

azure-pipelines · 2025-04-04T17:12:08Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (3)

sonic-xcvrd/xcvrd/xcvrd_utilities/xcvr_table_helper.py:14

Ensure that the naming convention for the new DOM and Status flag metadata tables remains consistent with existing table names and is clearly documented in the HLD.

TRANSCEIVER_DOM_FLAG_TABLE = 'TRANSCEIVER_DOM_FLAG'

sonic-xcvrd/xcvrd/dom/utilities/db/utils.py:124

[nitpick] For flags that are not already present in the database metadata, consider initializing their metadata (e.g., change count and timestamp fields) rather than skipping the update; this will ensure all new flags get proper metadata tracking.

if flag_key in db_flags_value_dict and db_flags_value_dict[flag_key] != str(curr_flag_value):

sonic-xcvrd/xcvrd/dom/utilities/vdm/db_utils.py:109

Verify that appending 'last_update_time' as a field in the FieldValuePairs conforms to the expected database schema and does not disrupt consumers of the table data.

[('last_update_time', self.get_current_time())]

Call check_transceiver_dom_sensor_basic only for first_port_in_split. Call check_transceiver_dom_sensor_details only for first_port_in_split. This aligns with the OS change where TRANSCEIVER_DOM_SENSOR values are available only for the first port in each split. See PR: sonic-net/sonic-platform-daemons#604

Description of PR Summary: Skip last_update_time per design. Fixes # (issue) ADO: 32970571 Approach What is the motivation for this PR? Due to recent change in sonic-net/sonic-platform-daemons#604, the last_udpate_time was added. It should be skipped for comparison per design. The last_update_time field was added in four place including post_diagnostic_values_to_db, post_port_dom_flags_to_db, post_port_transceiver_hw_status_flags_to_db, post_port_vdm_thresholds_to_db. So there are many table involved such as TRANSCEIVER_STATUS, TRANSCEIVER_STATUS_FLAG. Thus skip in the val comparison. How did you do it? Skip the last_update_time for comparison How did you verify/test it? E2E

The changes to the sonic-mgmt testcases are required due to the below changes: sonic-net/sonic-platform-common#556 sonic-net/sonic-platform-daemons#604 sonic-net/sonic-utilities#3844 This PR also reverts the changes made by #181 .

yejianquan · 2025-07-14T02:39:42Z

Hi @mihirpat1 , please fix conflict and create PR to 202505 branch

yejianquan · 2025-07-14T06:59:21Z

202505 branched out around 5/16, this commit is included when created, removing request labels.
and please ignore the conflict notification

Description of PR Summary: Skip last_update_time per design. Fixes # (issue) ADO: 32970571 Approach What is the motivation for this PR? Due to recent change in sonic-net/sonic-platform-daemons#604, the last_udpate_time was added. It should be skipped for comparison per design. The last_update_time field was added in four place including post_diagnostic_values_to_db, post_port_dom_flags_to_db, post_port_transceiver_hw_status_flags_to_db, post_port_vdm_thresholds_to_db. So there are many table involved such as TRANSCEIVER_STATUS, TRANSCEIVER_STATUS_FLAG. Thus skip in the val comparison. How did you do it? Skip the last_update_time for comparison How did you verify/test it? E2E Signed-off-by: opcoder0 <110003254+opcoder0@users.noreply.github.com>

Description of PR Summary: Skip last_update_time per design. Fixes # (issue) ADO: 32970571 Approach What is the motivation for this PR? Due to recent change in sonic-net/sonic-platform-daemons#604, the last_udpate_time was added. It should be skipped for comparison per design. The last_update_time field was added in four place including post_diagnostic_values_to_db, post_port_dom_flags_to_db, post_port_transceiver_hw_status_flags_to_db, post_port_vdm_thresholds_to_db. So there are many table involved such as TRANSCEIVER_STATUS, TRANSCEIVER_STATUS_FLAG. Thus skip in the val comparison. How did you do it? Skip the last_update_time for comparison How did you verify/test it? E2E Signed-off-by: Aharon Malkin <amalkin@nvidia.com>

Description of PR Summary: Skip last_update_time per design. Fixes # (issue) ADO: 32970571 Approach What is the motivation for this PR? Due to recent change in sonic-net/sonic-platform-daemons#604, the last_udpate_time was added. It should be skipped for comparison per design. The last_update_time field was added in four place including post_diagnostic_values_to_db, post_port_dom_flags_to_db, post_port_transceiver_hw_status_flags_to_db, post_port_vdm_thresholds_to_db. So there are many table involved such as TRANSCEIVER_STATUS, TRANSCEIVER_STATUS_FLAG. Thus skip in the val comparison. How did you do it? Skip the last_update_time for comparison How did you verify/test it? E2E Signed-off-by: Guy Shemesh <gshemesh@nvidia.com>

### Description of PR  Summary: Skip last_update_time per design. Fixes # (issue) ADO: 32970571 ### Type of change  - [ ] Bug fix - [ ] Testbed and Framework(new/improvement) - [ ] New Test case - [ ] Skipped for non-supported platforms - [ ] Test case improvement ### Back port request - [ ] 202205 - [ ] 202305 - [ ] 202311 - [ ] 202405 - [ ] 202411 - [ ] 202505 ### Approach #### What is the motivation for this PR? Due to recent change in sonic-net/sonic-platform-daemons#604, the last_udpate_time was added. It should be skipped for comparison per design. The `last_update_time` field was added in four place including `post_diagnostic_values_to_db, post_port_dom_flags_to_db, post_port_transceiver_hw_status_flags_to_db, post_port_vdm_thresholds_to_db`. So there are many table involved such as `TRANSCEIVER_STATUS`, `TRANSCEIVER_STATUS_FLAG`. Thus skip in the val comparison. #### How did you do it? Skip the last_update_time for comparison #### How did you verify/test it? E2E #### Any platform specific information? #### Supported testbed topology if it's a new test case? ### Documentation

…per design (#997)  ### Description of PR  Summary: Skip last_update_time per design. Fixes # (issue) ADO: 32970571 ### Type of change  - [ ] Bug fix - [ ] Testbed and Framework(new/improvement) - [ ] New Test case - [ ] Skipped for non-supported platforms - [ ] Test case improvement ### Back port request - [ ] 202205 - [ ] 202305 - [ ] 202311 - [ ] 202405 - [ ] 202411 - [ ] 202505 ### Approach #### What is the motivation for this PR? Due to recent change in sonic-net/sonic-platform-daemons#604, the last_udpate_time was added. It should be skipped for comparison per design. The `last_update_time` field was added in four place including `post_diagnostic_values_to_db, post_port_dom_flags_to_db, post_port_transceiver_hw_status_flags_to_db, post_port_vdm_thresholds_to_db`. So there are many table involved such as `TRANSCEIVER_STATUS`, `TRANSCEIVER_STATUS_FLAG`. Thus skip in the val comparison. #### How did you do it? Skip the last_update_time for comparison #### How did you verify/test it? E2E #### Any platform specific information? #### Supported testbed topology if it's a new test case? ### Documentation

mihirpat1 added 3 commits March 27, 2025 18:28

[xcvrd] Re-organize transceiver DOM and STATUS tables

16bc805

Merge branch 'master' of https://github.com/sonic-net/sonic-platform-…

de300a5

…daemons

[xcvrd] Re-organize transceiver DOM and STATUS tables

e608bb3

mihirpat1 marked this pull request as ready for review April 3, 2025 23:17

mihirpat1 requested review from Copilot and prgeor April 3, 2025 23:17

Copilot AI reviewed Apr 3, 2025

View reviewed changes

Fixed print statement for asic_index None check

96763c5

mihirpat1 added the Request for msft-202412 Branch label Apr 3, 2025

mihirpat1 requested a review from Copilot April 3, 2025 23:30

Copilot AI reviewed Apr 3, 2025

View reviewed changes

Fixed print statement for asic_index None check

8410980

mihirpat1 requested a review from Copilot April 3, 2025 23:36

Copilot AI reviewed Apr 3, 2025

View reviewed changes

Added _TABLE suffix in TRANSCEIVER STATUS metadata tables

a0f158f

mihirpat1 requested a review from Copilot April 4, 2025 00:17

Copilot AI reviewed Apr 4, 2025

View reviewed changes

Added remove_stale_transceiver_info function

bb161df

mihirpat1 requested a review from Copilot April 4, 2025 17:13

Copilot AI reviewed Apr 4, 2025

View reviewed changes

az-pz mentioned this pull request Jun 11, 2025

Fix txvr tests after diagnostic table restructuring sonic-net/sonic-mgmt#18938

Merged

11 tasks

mihirpat1 added the Request for msft-202503 Branch label Jun 16, 2025

mssonicbld added the Cherry Pick Conflict_msft-202503 label Jun 16, 2025

mssonicbld mentioned this pull request Jun 19, 2025

[action] [PR:18610] [configlet] Skip last_update_time for comparison per design sonic-net/sonic-mgmt#19100

Merged

11 tasks

This was referenced Jun 19, 2025

[202503] Fix txvr tests after diagnostic table restructuring Azure/sonic-mgmt.msft#422

Merged

[202412] Fix txvr tests after diagnostic table restructuring Azure/sonic-mgmt.msft#424

Merged

az-pz mentioned this pull request Jun 24, 2025

Check DOM for only first subport for test_xcvr_info_in_db.py test. sonic-net/sonic-mgmt#19170

Merged

11 tasks

az-pz mentioned this pull request Jun 27, 2025

Bug: For QSFP28 100G transceiver module, 'vdm_supported' key is missing from platform API sonic-net/sonic-buildimage#23012

Open

mihirpat1 mentioned this pull request Jul 8, 2025

[202411] DOM enhancement changes for xcvrd #639

Closed

r12f added the Request for 202505 Branch label Jul 12, 2025

mssonicbld added the Cherry Pick Conflict_202505 label Jul 12, 2025

yejianquan added the Approved for 202505 Branch label Jul 14, 2025

yejianquan removed Request for 202505 Branch Approved for 202505 Branch Cherry Pick Conflict_202505 labels Jul 14, 2025

mihirpat1 mentioned this pull request Aug 28, 2025

Bug: [PMON] "is_replaceable" field is missing from TRANSCEIVER_INFO table of CMIS cables and impacting SNMP test sonic-net/sonic-buildimage#22213

Closed

kewei-arista mentioned this pull request Sep 3, 2025

Revert PR19910 as the original issue was fixed sonic-net/sonic-mgmt#20504

Open

11 tasks

mssonicbld mentioned this pull request Feb 4, 2026

[action] [PR:18610] [configlet] Skip last_update_time for comparison per design Azure/sonic-mgmt.msft#997

Merged

11 tasks

	self.logger.log_error("Post port dom flags to db failed for {logical_port_name} "
	self.logger.log_error(f"Post port dom flags to db failed for {logical_port_name} "

	self.logger.log_error("Post port dom thresholds to db failed for {logical_port_name} "
	self.logger.log_error(f"Post port dom thresholds to db failed for {logical_port_name} "

Conversation

mihirpat1 commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

Additional Information (Optional)

Uh oh!

mssonicbld commented Apr 3, 2025

Uh oh!

azure-pipelines bot commented Apr 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Apr 3, 2025

Uh oh!

azure-pipelines bot commented Apr 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2025

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Apr 3, 2025

Uh oh!

azure-pipelines bot commented Apr 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Apr 4, 2025

Uh oh!

azure-pipelines bot commented Apr 4, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Apr 4, 2025

Uh oh!

azure-pipelines bot commented Apr 4, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

yejianquan commented Jul 14, 2025

Uh oh!

yejianquan commented Jul 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

14 participants

mihirpat1 commented Apr 3, 2025 •

edited

Loading