-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HLD for diagnostic monitoring of CMIS based transceivers #1828
base: master
Are you sure you want to change the base?
HLD for diagnostic monitoring of CMIS based transceivers #1828
Conversation
Signed-off-by: Mihir Patel <[email protected]>
@qinchuanares - It will be great if you can help in reviewing this PR. |
@Junchao-Mellanox @keboliu - It will be great if you can help in reviewing this PR. |
|
||
### 4.2 Dynamic Diagnostic Information | ||
|
||
The `DomInfoUpdateTask` thread is responsible for updating the dynamic diagnostic information for all the transceivers in the system. The `DomInfoUpdateTask` thread is triggered by a timer (`DOM_INFO_UPDATE_PERIOD_SECS`), which is set to 60 seconds by default. The `DomInfoUpdateTask` thread reads the diagnostic information from the transceiver and updates the relevant tables in `redis-db`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious if there is any plan for making this alarm logging interrupt based?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prgeor Can you please help in clarifying this?
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mihirpat1 I don't see how the timestamp, count for each alarm will be updated so that these times are as closer to the actual reporting time by the module
3. Read the transceiver firmware information from the module and update the `TRANSCEIVER_FIRMWARE_INFO` table. | ||
4. Read the transceiver DOM sensor data from the module and update the `TRANSCEIVER_DOM_SENSOR` table. | ||
5. Read the transceiver DOM flag data from the module, record the timestamp, and update the `TRANSCEIVER_DOM_FLAG` table. | ||
6. Analyze the transceiver DOM flag data by comparing the current flag data with the previous flag data and update the `TRANSCEIVER_DOM_FLAG_CHANGE_COUNT`, `TRANSCEIVER_DOM_FLAG_TIME_SET`, and `TRANSCEIVER_DOM_FLAG_TIME_CLEAR` tables. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also record the flag set/clear event to syslog?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Junchao-Mellanox Sure, will plan to record the set/clear event to syslog as a debug. Making it as a notice will make the syslog chatty.
6. Analyze the transceiver DOM flag data by comparing the current flag data with the previous flag data and update the `TRANSCEIVER_DOM_FLAG_CHANGE_COUNT`, `TRANSCEIVER_DOM_FLAG_TIME_SET`, and `TRANSCEIVER_DOM_FLAG_TIME_CLEAR` tables. | ||
7. Read the transceiver status data from the module and update the `TRANSCEIVER_STATUS` table. | ||
8. If the transceiver supports VDM monitoring, perform the following steps: | ||
1. Freeze the statistics by calling the CMIS API and wait for `FreezeDone`. Once the statistics are frozen, record the timestamp and copy the VDM and PM statistics from the transceiver. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please specify which API will be used to "freeze" the statistic? I guess it will write something to module EEPROM, and it is only applicable when module is under software control.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Junchao-Mellanox I have now updated the API name now. Can you please elaborate on what do you mean by "it is only applicable when module is under software control"?
----------- --------------- -------- -------- -------- -------- | ||
|
||
Example: | ||
admin@sonic#show interfaces transceiver vdm flag Ethernet1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to add a filter to get abnormal status only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prgeor do we want to add filter as part of current enhancement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Junchao-Mellanox We are now displaying the output after applying the filter. A --detail
option has been added to allow user to dump data for all lanes irrespective of the flag being set.
**Tables Used for Flag Analysis:** | ||
|
||
- `TRANSCEIVER_VDM_FLAG`: This table stores flags indicating the status of various VDM parameters. | ||
- `TRANSCEIVER_VDM_FLAG_CHANGE_COUNT`: This table keeps a count of how many times each VDM flag has changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If xcvrd stopped, will the change count be reset to 0? The same question to time set and time clear. Also, should we record the change count to syslog?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Junchao-Mellanox Yes - the change count and the set/clear time in redis-db tables will be removed when xcvrd
is stopped. We can log the change count to syslog before deleting the table.
@prgeor I have now added |
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
3. Read the transceiver firmware information from the module and update the `TRANSCEIVER_FIRMWARE_INFO` table. | ||
4. Read the transceiver DOM sensor data from the module and update the `TRANSCEIVER_DOM_SENSOR` table. | ||
5. Read the transceiver DOM flag data from the module, record the timestamp, and update the `TRANSCEIVER_DOM_FLAG` table. | ||
6. Analyze the transceiver DOM flag data by comparing the current flag data with the previous flag data and update the `TRANSCEIVER_DOM_FLAG_CHANGE_COUNT`, `TRANSCEIVER_DOM_FLAG_TIME_SET`, and `TRANSCEIVER_DOM_FLAG_TIME_CLEAR` tables. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Junchao-Mellanox Sure, will plan to record the set/clear event to syslog as a debug. Making it as a notice will make the syslog chatty.
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
----------- --------------- -------- -------- -------- -------- | ||
|
||
Example: | ||
admin@sonic#show interfaces transceiver vdm flag Ethernet1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prgeor do we want to add filter as part of current enhancement?
6. Analyze the transceiver DOM flag data by comparing the current flag data with the previous flag data and update the `TRANSCEIVER_DOM_FLAG_CHANGE_COUNT`, `TRANSCEIVER_DOM_FLAG_TIME_SET`, and `TRANSCEIVER_DOM_FLAG_TIME_CLEAR` tables. | ||
7. Read the transceiver status data from the module and update the `TRANSCEIVER_STATUS` table. | ||
8. If the transceiver supports VDM monitoring, perform the following steps: | ||
1. Freeze the statistics by calling the CMIS API and wait for `FreezeDone`. Once the statistics are frozen, record the timestamp and copy the VDM and PM statistics from the transceiver. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Junchao-Mellanox I have now updated the API name now. Can you please elaborate on what do you mean by "it is only applicable when module is under software control"?
**Tables Used for Flag Analysis:** | ||
|
||
- `TRANSCEIVER_VDM_FLAG`: This table stores flags indicating the status of various VDM parameters. | ||
- `TRANSCEIVER_VDM_FLAG_CHANGE_COUNT`: This table keeps a count of how many times each VDM flag has changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Junchao-Mellanox Yes - the change count and the set/clear time in redis-db tables will be removed when xcvrd
is stopped. We can log the change count to syslog before deleting the table.
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
tx{lane_num}power = FLOAT ; tx power in dBm for each lane | ||
rx{lane_num}power = FLOAT ; rx power in dBm for each lane | ||
tx{lane_num}bias = FLOAT ; tx bias in mA for each lane | ||
laser_temperature = FLOAT ; laser temperature value in Celsius |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There could be more than one laser and more than one laser temperatures in one module.
Laser temperature could be from
- aux2 or aux3 mon value from byte 20-21, or 22-23, In this case there will be only one laser temp value.
- VDM observable type 4, it may have more than one values representing lasers on different lanes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qinchuanares As discussed, I have now added VDM observable type to all the VDM relevant tables to capture the laser temperature for media lanes.
lasertemplowwarning_chg_cnt = INTEGER ; laser temperature low warning change count | ||
``` | ||
|
||
#### 2.1.5 Transceiver DOM flag time set data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if all the data type is in string, is it easy to convert it to datetime data type? In some cases we may need to parse this data and do time diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always show the current time from any CLI output would help us determining the time difference too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qinchuanares The timestamp is recorded in the format Day Mon DD HH:MM:SS YYYY
. Yes, I have now updated the CLI to show the current time as well.
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
; Defines Transceiver PM information for a port | ||
key = TRANSCEIVER_PM|ifname ; information of PM on port | ||
; field = value | ||
prefec_ber_avg = FLOAT ; prefec ber avg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could expand this to media and host prefec BER and uncorr frames, even if this is C-CMIS only.
Media BER and UC frames info comes from page 34h, host BER and UC frames come from page 3Ah
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
|
||
#### 4.2.2 Diagnostic Information Update During Link Down Event | ||
|
||
When a link down event is detected by the `DomInfoUpdateTask` thread, a specific subset of the diagnostic information fields are updated in the `redis-db`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does last link up time and last link down time in APPL_DB get updated for these events?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qinchuanares Yes - the last link up and down time will get updated in PORT_TABLE
of APPL_DB
during a link change event.
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
doc/platform_api/CMIS_Diagnostic_Monitoring_Overview_in_SONiC.md
Outdated
Show resolved
Hide resolved
…mes of all the relevant tables
@mihirpat1 something we can use? like interval time and last time stamp when sample was collected |
@prgeor I think this will be good to add. Assuming we plan to add this to all the tables (DOM + VDM + STATUS) with their corresponding real time value, threshold value and the corresponding flag relevant tables. |
#### 4.2.1 High-Level Steps for Updating Dynamic Diagnostic Information | ||
|
||
1. The `DomInfoUpdateTask` thread is created by the `xcvrd` process. | ||
2. A timer (`DOM_INFO_UPDATE_PERIOD_SECS`) is set to read the diagnostic information from the transceiver every 60 seconds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many new fields have been added. Is there any measurement for the time required to complete all port readings for each cycle?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prgeor Can you please help with this?
|
||
#### 4.2.4 Flag Change Count and Time Set/Clear Behavior During `xcvrd` Restart | ||
|
||
During `xcvrd` stop, the `TRANSCEIVER_DOM_FLAG_CHANGE_COUNT`, `TRANSCEIVER_DOM_FLAG_SET_TIME`, and `TRANSCEIVER_DOM_FLAG_CLEAR_TIME` tables are deleted by the `xcvrd` process. When `xcvrd` is restarted, the `TRANSCEIVER_DOM_FLAG_CHANGE_COUNT`, `TRANSCEIVER_DOM_FLAG_SET_TIME`, and `TRANSCEIVER_DOM_FLAG_CLEAR_TIME` tables are recreated and the flag change count and set/clear time are updated based on the current flag status (i.e. the value of these fields are not cached between `xcvrd` restarts). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to differentiate between cold reboot and warm reboot scenarios?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@keboliu with the first draft of this HLD, we do not plan to preserve the values of these tables between warm reboot. This will be in line to this section since warm reboot will cause xcvrd kill followed by xcvrd startup when the device reboots.
Preserving the values of these tables between warm reboot and warm restart of xcvrd (by warm restart of xcvrd, I mean that link flap is not expected on ports) can be taken as an enhancement in future.
|
||
When a transceiver is removed, `TRANSCEIVER_DOM_FLAG_CHANGE_COUNT`, `TRANSCEIVER_DOM_FLAG_SET_TIME`, and `TRANSCEIVER_DOM_FLAG_CLEAR_TIME` tables are deleted by the `SfpStateUpdateTask` thread. | ||
|
||
When the transceiver is inserted back, the `TRANSCEIVER_DOM_FLAG_CHANGE_COUNT`, `TRANSCEIVER_DOM_FLAG_SET_TIME`, and `TRANSCEIVER_DOM_FLAG_CLEAR_TIME` tables are recreated through the periodic polling routine of `DomInfoUpdateTask` and the flag change count and set/clear time are updated based on the current flag status. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why only re-create these 3 tables upon transceiver removal/insert? Shall we recreate all the tables?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@keboliu Yes, we will recreate all VDM and STATUS related tables as well. I have now renumbered this section to signify this more clearly.
I have mentioned the below note in section 4.2.3 Details of Flag Analysis of Tables to convey this.
Note: For simplicity, this section uses DOM as an example. However, the same analysis is applicable for VDM as well.
…and enhanced the output of various CLIs using lane number
This HLD while provide an overview of how SONiC reads and stores the various diagnostic parameters read from a CMIS based transceiver.
Proposed changes for existing tables in SONiC
TRANSCEIVER_STATUS
toTRANSCEIVER_DOM_FLAG
table.