Skip to content
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

[sfp] Add parsing the dom_capability to sff8472 #102

Merged
merged 1 commit into from
Jan 19, 2021

Conversation

kuanyu99
Copy link
Contributor

  • What I did
    Following SFF-8472 to read diagnostic monitoring type register.
    Use that register to check if diagnostic monitoring function implemented or not.

  • How I did it
    Check the register position from SFF-8472 document.
    Read the register value from the EEPROM.

  • How to verify it
    Compare one transceiver which support diagnostic monitoring function to another transceiver which doesn't support it.
    Run 'show interfaces transceiver eeprom' command.

* Read the diagnostic monitoring type register to get dom_capability
* Provide basic dom_capability info table as transceiver info
* Unify the name of parsing dom_capability function for both QSFP and SFP
@kuanyu99
Copy link
Contributor Author

Hi @jleveque,
Could you help to review this, please?

@jleveque jleveque requested a review from keboliu August 14, 2020 18:37
@jleveque
Copy link
Contributor

jleveque commented Nov 6, 2020

@keboliu: Please review.

for key in qsfp_dom_capability_tup:
if key in sfp_dom_capability_data['data']:
dom_capability_dict[key] = "yes" if sfp_dom_capability_data['data'][key]['value'] == 'on' else "no"
transceiver_info_dict['dom_capability'] = str(dom_capability_dict)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it necessary to expose this to transceiver_info_dict? My understanding is that this is only some internal flag, if DOM supported, means can get the related meaningful monitor info from the EEPROM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a verification method for the SFP type transceiver for now.
If user buy a SFP type transceiver with DOM capability, there is no way to check DOM capability of the transceiver in SONIC currently. With this, the user can check if he/she buy a fake one or not at least.
And in the future, we can enhance this part to check more dom info according to its dom_capability.

@jleveque jleveque merged commit 6b12b4c into sonic-net:master Jan 19, 2021
jleveque added a commit to sonic-net/sonic-buildimage that referenced this pull request Feb 5, 2021
…rse_dom_capability' (#6695)

**- Why I did it**
PR sonic-net/sonic-platform-common#102 modified the name of the SFF-8436 (QSFP) method to align the method name between all drivers, renaming it from `parse_qsfp_dom_capability` to `parse_dom_capability`. Once the submodule was updated, the callers using the old nomenclature broke. This PR updates all callers to use the new naming convention.

**- How I did it**

Update the name of the function globally for all calls into the SFF-8436 driver.

Note that the QSFP-DD driver still uses the old nomenclature and should be modified similarly. I will open a PR to handle this separately.
daall pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Feb 5, 2021
…rse_dom_capability' (#6695)

**- Why I did it**
PR sonic-net/sonic-platform-common#102 modified the name of the SFF-8436 (QSFP) method to align the method name between all drivers, renaming it from `parse_qsfp_dom_capability` to `parse_dom_capability`. Once the submodule was updated, the callers using the old nomenclature broke. This PR updates all callers to use the new naming convention.

**- How I did it**

Update the name of the function globally for all calls into the SFF-8436 driver.

Note that the QSFP-DD driver still uses the old nomenclature and should be modified similarly. I will open a PR to handle this separately.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants