Skip to content

[sfp-refactor] Fix is_xcvr_optical for QSFP28/CMIS#4749

Merged
prgeor merged 2 commits intosonic-net:masterfrom
andywongarista:sfp-optical
Feb 2, 2022
Merged

[sfp-refactor] Fix is_xcvr_optical for QSFP28/CMIS#4749
prgeor merged 2 commits intosonic-net:masterfrom
andywongarista:sfp-optical

Conversation

@andywongarista
Copy link
Contributor

@andywongarista andywongarista commented Nov 22, 2021

Description of PR

Summary: Add additional check in is_xcvr_optical for passive QSFP28 cables. Also fix check for CMIS cables.
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 201911

Approach

What is the motivation for this PR?

Passive QSFP28 cables may use an extended specification compliance code but this is not currently checked for in is_xcvr_optical, which results in some test cases inadvertently running and failing instead of being skipped.

Update check for CMIS cables to enable support for CMIS QSFP cables and address change in specification_compliance value introduced in sonic-net/sonic-platform-common#228

How did you do it?

Updated is_xcvr_optical to check for the extended specification compliance code. Based on SFF-8024 Table 4-4, this change assumes that passive cables have "CR" in the code description.

How did you verify/test it?

Ran sonic-mgmt test suite on Arista platfrom with QSFP28 passive cables/CMIS QSFP cables to verify.

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

prgeor
prgeor previously approved these changes Nov 25, 2021
@andywongarista andywongarista changed the title [sfp-refactor] Enhance is_xcvr_optical for QSFP28 [sfp-refactor] Fix is_xcvr_optical for QSFP28/CMIS Nov 26, 2021
@wangxin
Copy link
Collaborator

wangxin commented Dec 1, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prgeor
Copy link
Contributor

prgeor commented Jan 31, 2022

@andywongarista is this PR still required?

@prgeor
Copy link
Contributor

prgeor commented Jan 31, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@andywongarista
Copy link
Contributor Author

@andywongarista is this PR still required?

Yes.

@prgeor
Copy link
Contributor

prgeor commented Feb 2, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prgeor prgeor merged commit f1285cc into sonic-net:master Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants