Skip to content

[xcvrd] Separate VDM basic and statistic observables#750

Merged
prgeor merged 8 commits intosonic-net:masterfrom
mihirpat1:vdm_separation
Feb 19, 2026
Merged

[xcvrd] Separate VDM basic and statistic observables#750
prgeor merged 8 commits intosonic-net:masterfrom
mihirpat1:vdm_separation

Conversation

@mihirpat1
Copy link
Copy Markdown
Contributor

@mihirpat1 mihirpat1 commented Feb 16, 2026 •
edited
Loading

This PR must be merged along with sonic-net/sonic-platform-common#624

Description

Currently, xcvrd performs freeze operation before reading ALL VDM observables,
then unfreezes after reading. This approach has following limitations:

  1. Modules supporting only basic VDM observables (instantaneous values) may not
    support freeze/unfreeze functionality
  2. Freeze operations are expensive and unnecessary for basic observables

Motivation and Context

This PR restructures the VDM monitoring flow in DOM manager to optimize freeze
operations and correctly handle different observable types.

CHANGES IMPLEMENTED:

  1. Three-Step VDM Monitoring Flow (dom/dom_mgr.py)

    Step 1: Conditional Freeze for Statistics/PM (only if NOT in lpmode)

    • Check: need_freeze = vdm_statistic_supported AND not_lpmode
    • If need_freeze:
      a) Freeze VDM/PM statistics
      b) Read VDM statistic observables (if supported)
      c) Read PM data
      d) Unfreeze VDM/PM statistics

    Step 2: Read VDM Basic Observables

    • Always read, no freeze needed
    • Works even in lpmode
    • get_vdm_real_values_basic() → instantaneous values
    • Merged with statistic observables from Step 1

    Step 3: Post Merged Data to DB, then Read COR Flags

    • Post combined basic + statistic observables to DB
    • Read COR flags last (ensures statistics captured before flags clear)

How Has This Been Tested?

Following tests were executed to ensure that the VDM related tables are updated in line with expectation

  1. Optic supporting only VDM basic observables
  2. Optic supporting both VDM basic and statistic observables
  3. Optic supporting both VDM basic and statistic observables + CCMIS optic
  4. Optics not supporting VDM and PM (800G LPO optics)
  5. While module is in lpmode, ensure that the VDM statistical observables are not being updated
  • Please note that the last_update_time timestamp will be updated as VDM basic observables would still be periodically updated while the transceiver is in low power mode
  1. While CCMIS module is in lpmode, ensure that the VDM statistical observables + PM data is not being updated
  2. Flat memory optics
  3. Test SFP OIR
    a. Swap CCMIS optic with non-VDM supported optic
    b. Swap non-VDM supported optic with DAC
    c. Swap DAC with CCMIS optic

Additional Information (Optional)

MSFT ADO - 34227059

@mssonicbld
Copy link
Copy Markdown
Collaborator

mssonicbld commented Feb 16, 2026

/azp run

@mihirpat1 mihirpat1 requested a review from Copilot February 16, 2026 19:57
@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Feb 16, 2026

Azure Pipelines successfully started running 1 pipeline(s).

Copilot AI reviewed Feb 16, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates xcvrd’s VDM (Vendor Diagnostic Monitoring) collection flow to separate “basic” (instantaneous) observables from “statistic/PM” observables, minimizing freeze/unfreeze usage and improving compatibility with modules that don’t support freeze.

Changes:

  • Split VDM reads into basic vs statistic paths and conditionally freeze only when needed (and not in low-power mode).
  • Add a DB utility method to post pre-merged VDM values in a single write with one last_update_time.
  • Update and extend unit tests to cover the new VDM flow and freeze conditions.

Reviewed changes

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

File Description
sonic-xcvrd/xcvrd/dom/utilities/vdm/utils.py Adds new VDM capability + read helpers for basic/statistic observables.
sonic-xcvrd/xcvrd/dom/utilities/vdm/db_utils.py Adds a new method to post merged VDM real-values dict directly to DB.
sonic-xcvrd/xcvrd/dom/dom_mgr.py Restructures VDM polling into conditional freeze + merge + post + COR flag read.
sonic-xcvrd/tests/test_xcvrd.py Updates existing tests and adds new cases validating freeze conditions and new APIs.

@mihirpat1 mihirpat1 mentioned this pull request Feb 16, 2026
Merged
@mihirpat1
Addressed PR comments
5c4258a 
Signed-off-by: Mihir Patel <[email protected]>
@mssonicbld
Copy link
Copy Markdown
Collaborator

mssonicbld commented Feb 17, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Feb 17, 2026

Azure Pipelines successfully started running 1 pipeline(s).

@mihirpat1 mihirpat1 requested a review from Copilot February 17, 2026 01:00
Copilot AI reviewed Feb 17, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

@mihirpat1
Enhanced testcase
1a50c19 
Signed-off-by: Mihir Patel <[email protected]>
@mssonicbld
Copy link
Copy Markdown
Collaborator

mssonicbld commented Feb 17, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Feb 17, 2026

Azure Pipelines successfully started running 1 pipeline(s).

@mihirpat1
Enhanced testcases
8f4b20b 
Signed-off-by: Mihir Patel <[email protected]>
@mssonicbld
Copy link
Copy Markdown
Collaborator

mssonicbld commented Feb 17, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Feb 17, 2026

Azure Pipelines successfully started running 1 pipeline(s).

@mihirpat1 mihirpat1 requested a review from Copilot February 17, 2026 02:02
Copilot AI reviewed Feb 17, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

@mihirpat1
Removed redundant code
02d8a84 
Signed-off-by: Mihir Patel <[email protected]>
@mssonicbld
Copy link
Copy Markdown
Collaborator

mssonicbld commented Feb 17, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Feb 17, 2026

Azure Pipelines successfully started running 1 pipeline(s).

@mihirpat1
Enhanced tests
115b404 
Signed-off-by: Mihir Patel <[email protected]>
@mssonicbld
Copy link
Copy Markdown
Collaborator

mssonicbld commented Feb 17, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Feb 17, 2026

Azure Pipelines successfully started running 1 pipeline(s).

@mihirpat1
Fixed testcase failure
4633eed 
Signed-off-by: Mihir Patel <[email protected]>
@mssonicbld
Copy link
Copy Markdown
Collaborator

mssonicbld commented Feb 17, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Feb 17, 2026

Azure Pipelines successfully started running 1 pipeline(s).

@mihirpat1 mihirpat1 marked this pull request as ready for review February 17, 2026 17:19
prgeor
prgeor reviewed Feb 19, 2026
View reviewed changes
prgeor
prgeor reviewed Feb 19, 2026
View reviewed changes
@mihirpat1
Addresed PR comments
e69284c 
Signed-off-by: Mihir Patel <[email protected]>
@mssonicbld
Copy link
Copy Markdown
Collaborator

mssonicbld commented Feb 19, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Feb 19, 2026

Azure Pipelines successfully started running 1 pipeline(s).

@prgeor prgeor merged commit ebb54ed into sonic-net:master Feb 19, 2026
6 checks passed
@mssonicbld mssonicbld mentioned this pull request Feb 19, 2026
Merged
@mssonicbld
Copy link
Copy Markdown
Collaborator

mssonicbld commented Feb 19, 2026

Cherry-pick PR to 202511: #753

mssonicbld added a commit to mssonicbld/sonic-platform-common that referenced this pull request Feb 19, 2026
@mssonicbld
[cmis] Separate VDM basic and statistic observables
42a9a16 
<!-- Provide a general summary of your changes in the Title above -->
This PR must be merged along with sonic-net/sonic-platform-daemons#750

#### Description
<!--
     Describe your changes in detail
-->
Currently, the CMIS API treats all VDM observables uniformly, requiring freeze operations for all reads. However,
CMIS specification distinguishes between:
- Basic observables (instantaneous values like current BER, temperature)
- Statistic observables (min/max/avg values that require freeze to capture
  consistently)

This prevents optimization in xcvrd, which currently requires freeze operation even for modules that
only support basic observables (where freeze may not be supported or necessary).

Additionally, the API performs repeated EEPROM reads to check VDM support
status, even though this capability bit is static for a given module.

#### Motivation and Context
<!--
     Why is this change required? What problem does it solve?
     If this pull request closes/resolves an open Issue, make sure you
     include the text "fixes #xxxx", "closes #xxxx" or "resolves #xxxx" here
-->

1. VDM_TYPE Enhancement (codes/public/cmis.py)
   ----------------------------------------------
   - Added 4th element to VDM_TYPE tuples: 'B' (basic) or 'S' (statistic)
   - Serves as single source of truth for observable classification

2. Observable Type Constants (api/public/cmisVDM.py)
   --------------------------------------------------
   - VDM_OBSERVABLE_BASIC = 0x1
   - VDM_OBSERVABLE_STATISTIC = 0x2
   - VDM_OBSERVABLE_ALL = 0x3

3. New CmisApi Methods (api/public/cmis.py)
   -----------------------------------------
   a) get_transceiver_vdm_real_value_basic()
      - Reads only basic (instantaneous) VDM observables
      - No freeze required

   b) get_transceiver_vdm_real_value_statistic()
      - Reads only statistic (min/max/avg) VDM observables
      - Requires freeze operation (caller responsible)

   c) is_vdm_statistic_supported()
      - Checks if module supports VDM statistic observables
      - Uses @read_only_cached_api_return for performance

4. Enhanced get_vdm_page() (api/public/cmisVDM.py)
   ------------------------------------------------
   - Added optional observable_type parameter
   - Filters VDM observables based on type (basic/statistic/all)

5. Performance Optimization
   -------------------------
   - Added @read_only_cached_api_return to is_transceiver_vdm_supported()
   - Prevents repeated EEPROM access for static capability bits

#### How Has This Been Tested?
<!--
     Please describe in detail how you tested your changes.
     Include details of your testing environment, and the tests you ran to
     see how your change affects other areas of the code, etc.
-->
Please refer to the test details in sonic-net/sonic-platform-daemons#750

#### Additional Information (Optional)
MSFT ADO - 36798630
@mssonicbld mssonicbld mentioned this pull request Feb 19, 2026
Closed
@abelamit abelamit mentioned this pull request Feb 23, 2026
Open
mssonicbld added a commit to mssonicbld/sonic-platform-common that referenced this pull request Feb 27, 2026
@mssonicbld
[cmis] Separate VDM basic and statistic observables
fd8755c 
<!-- Provide a general summary of your changes in the Title above -->
This PR must be merged along with sonic-net/sonic-platform-daemons#750

#### Description
<!--
     Describe your changes in detail
-->
Currently, the CMIS API treats all VDM observables uniformly, requiring freeze operations for all reads. However,
CMIS specification distinguishes between:
- Basic observables (instantaneous values like current BER, temperature)
- Statistic observables (min/max/avg values that require freeze to capture
  consistently)

This prevents optimization in xcvrd, which currently requires freeze operation even for modules that
only support basic observables (where freeze may not be supported or necessary).

Additionally, the API performs repeated EEPROM reads to check VDM support
status, even though this capability bit is static for a given module.

#### Motivation and Context
<!--
     Why is this change required? What problem does it solve?
     If this pull request closes/resolves an open Issue, make sure you
     include the text "fixes #xxxx", "closes #xxxx" or "resolves #xxxx" here
-->

1. VDM_TYPE Enhancement (codes/public/cmis.py)
   ----------------------------------------------
   - Added 4th element to VDM_TYPE tuples: 'B' (basic) or 'S' (statistic)
   - Serves as single source of truth for observable classification

2. Observable Type Constants (api/public/cmisVDM.py)
   --------------------------------------------------
   - VDM_OBSERVABLE_BASIC = 0x1
   - VDM_OBSERVABLE_STATISTIC = 0x2
   - VDM_OBSERVABLE_ALL = 0x3

3. New CmisApi Methods (api/public/cmis.py)
   -----------------------------------------
   a) get_transceiver_vdm_real_value_basic()
      - Reads only basic (instantaneous) VDM observables
      - No freeze required

   b) get_transceiver_vdm_real_value_statistic()
      - Reads only statistic (min/max/avg) VDM observables
      - Requires freeze operation (caller responsible)

   c) is_vdm_statistic_supported()
      - Checks if module supports VDM statistic observables
      - Uses @read_only_cached_api_return for performance

4. Enhanced get_vdm_page() (api/public/cmisVDM.py)
   ------------------------------------------------
   - Added optional observable_type parameter
   - Filters VDM observables based on type (basic/statistic/all)

5. Performance Optimization
   -------------------------
   - Added @read_only_cached_api_return to is_transceiver_vdm_supported()
   - Prevents repeated EEPROM access for static capability bits

#### How Has This Been Tested?
<!--
     Please describe in detail how you tested your changes.
     Include details of your testing environment, and the tests you ran to
     see how your change affects other areas of the code, etc.
-->
Please refer to the test details in sonic-net/sonic-platform-daemons#750

#### Additional Information (Optional)
MSFT ADO - 36798630

Signed-off-by: Sonic Build Admin <[email protected]>
@mssonicbld mssonicbld mentioned this pull request Feb 27, 2026
Closed
mssonicbld added a commit to mssonicbld/sonic-platform-common that referenced this pull request Feb 28, 2026
@mssonicbld
[cmis] Separate VDM basic and statistic observables
3fc7b00 
<!-- Provide a general summary of your changes in the Title above -->
This PR must be merged along with sonic-net/sonic-platform-daemons#750

#### Description
<!--
     Describe your changes in detail
-->
Currently, the CMIS API treats all VDM observables uniformly, requiring freeze operations for all reads. However,
CMIS specification distinguishes between:
- Basic observables (instantaneous values like current BER, temperature)
- Statistic observables (min/max/avg values that require freeze to capture
  consistently)

This prevents optimization in xcvrd, which currently requires freeze operation even for modules that
only support basic observables (where freeze may not be supported or necessary).

Additionally, the API performs repeated EEPROM reads to check VDM support
status, even though this capability bit is static for a given module.

#### Motivation and Context
<!--
     Why is this change required? What problem does it solve?
     If this pull request closes/resolves an open Issue, make sure you
     include the text "fixes #xxxx", "closes #xxxx" or "resolves #xxxx" here
-->

1. VDM_TYPE Enhancement (codes/public/cmis.py)
   ----------------------------------------------
   - Added 4th element to VDM_TYPE tuples: 'B' (basic) or 'S' (statistic)
   - Serves as single source of truth for observable classification

2. Observable Type Constants (api/public/cmisVDM.py)
   --------------------------------------------------
   - VDM_OBSERVABLE_BASIC = 0x1
   - VDM_OBSERVABLE_STATISTIC = 0x2
   - VDM_OBSERVABLE_ALL = 0x3

3. New CmisApi Methods (api/public/cmis.py)
   -----------------------------------------
   a) get_transceiver_vdm_real_value_basic()
      - Reads only basic (instantaneous) VDM observables
      - No freeze required

   b) get_transceiver_vdm_real_value_statistic()
      - Reads only statistic (min/max/avg) VDM observables
      - Requires freeze operation (caller responsible)

   c) is_vdm_statistic_supported()
      - Checks if module supports VDM statistic observables
      - Uses @read_only_cached_api_return for performance

4. Enhanced get_vdm_page() (api/public/cmisVDM.py)
   ------------------------------------------------
   - Added optional observable_type parameter
   - Filters VDM observables based on type (basic/statistic/all)

5. Performance Optimization
   -------------------------
   - Added @read_only_cached_api_return to is_transceiver_vdm_supported()
   - Prevents repeated EEPROM access for static capability bits

#### How Has This Been Tested?
<!--
     Please describe in detail how you tested your changes.
     Include details of your testing environment, and the tests you ran to
     see how your change affects other areas of the code, etc.
-->
Please refer to the test details in sonic-net/sonic-platform-daemons#750

#### Additional Information (Optional)
MSFT ADO - 36798630

Signed-off-by: Sonic Build Admin <[email protected]>
@mssonicbld mssonicbld mentioned this pull request Feb 28, 2026
Merged
mssonicbld added a commit to sonic-net/sonic-platform-common that referenced this pull request Feb 28, 2026
@mssonicbld
[cmis] Separate VDM basic and statistic observables (#628)
11641a1 
<!-- Provide a general summary of your changes in the Title above -->
This PR must be merged along with sonic-net/sonic-platform-daemons#750

#### Description
<!--
 Describe your changes in detail
-->
Currently, the CMIS API treats all VDM observables uniformly, requiring freeze operations for all reads. However,
CMIS specification distinguishes between:
- Basic observables (instantaneous values like current BER, temperature)
- Statistic observables (min/max/avg values that require freeze to capture
 consistently)

This prevents optimization in xcvrd, which currently requires freeze operation even for modules that
only support basic observables (where freeze may not be supported or necessary).

Additionally, the API performs repeated EEPROM reads to check VDM support
status, even though this capability bit is static for a given module.

#### Motivation and Context
<!--
 Why is this change required? What problem does it solve?
 If this pull request closes/resolves an open Issue, make sure you
 include the text "fixes #xxxx", "closes #xxxx" or "resolves #xxxx" here
-->

1. VDM_TYPE Enhancement (codes/public/cmis.py)
 ----------------------------------------------
 - Added 4th element to VDM_TYPE tuples: 'B' (basic) or 'S' (statistic)
 - Serves as single source of truth for observable classification

2. Observable Type Constants (api/public/cmisVDM.py)
 --------------------------------------------------
 - VDM_OBSERVABLE_BASIC = 0x1
 - VDM_OBSERVABLE_STATISTIC = 0x2
 - VDM_OBSERVABLE_ALL = 0x3

3. New CmisApi Methods (api/public/cmis.py)
 -----------------------------------------
 a) get_transceiver_vdm_real_value_basic()
 - Reads only basic (instantaneous) VDM observables
 - No freeze required

 b) get_transceiver_vdm_real_value_statistic()
 - Reads only statistic (min/max/avg) VDM observables
 - Requires freeze operation (caller responsible)

 c) is_vdm_statistic_supported()
 - Checks if module supports VDM statistic observables
 - Uses @read_only_cached_api_return for performance

4. Enhanced get_vdm_page() (api/public/cmisVDM.py)
 ------------------------------------------------
 - Added optional observable_type parameter
 - Filters VDM observables based on type (basic/statistic/all)

5. Performance Optimization
 -------------------------
 - Added @read_only_cached_api_return to is_transceiver_vdm_supported()
 - Prevents repeated EEPROM access for static capability bits

#### How Has This Been Tested?
<!--
 Please describe in detail how you tested your changes.
 Include details of your testing environment, and the tests you ran to
 see how your change affects other areas of the code, etc.
-->
Please refer to the test details in sonic-net/sonic-platform-daemons#750

#### Additional Information (Optional)
MSFT ADO - 36798630

Signed-off-by: Sonic Build Admin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@prgeor prgeor prgeor approved these changes

Assignees

No one assigned

Labels

Approved for 202511 Branch Included in 202511 Branch Request for 202511 Branch xcvrd

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mihirpat1 @mssonicbld @prgeor @vmittal-msft