-
Notifications
You must be signed in to change notification settings - Fork 654
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
Add support for QSFP56 and QSFP56_DD_TYPE3 form factor identities #564
Conversation
Compatibility Report for commit da629ae: |
@ejbrever can you provide any comments? |
I think there was some desire to specify the types, so keeping with that I'd lean towards adding the Type3 as done here. Adding an umbrella QSFP56 type might be problematic though because it allows for vendor implementations to choose either a specific type or the generic one, so we will likely have inconsistencies between implementations. |
I updated my commit to only keep the backward compatible |
This looks good to me. @thofmeister @yawei-yin would either you be able to review this as well? Thanks. |
We might need an identity QSFP56_DD to cover all the QSFP56-DD types. The count of optical lanes can be identified by ETHERNET_PMD_TYPE. For instance, TYPE2 corresponds to ETH_400GBASE_FR4, ETH_400GBASE_LR4, ETH_400GBASE_DR4, while TYPE1 corresponds to ETH_400GBASE_SR8, ETH_400GBASE_LR8. Therefore, we need to delete QSFP56_DD_TYPE1 and QSFP56_DD_TYPE2. @ejbrever @yawei-yin @thofmeister Actually, this issue has been addressed by PR#1016. We could revisit it in this public PR, and I will close PR#1016. What do you think? |
BTW, could we add more peer identities in a single PR, such as QSFP112, and QSFP112_DD which will be soon commercially available. |
@ejbrever can you also follow up with @thofmeister offline for us? |
@ejbrever Eric, if we still maintain multiple identities/types for QSFP56-DD form factor, how could we cope with other form factors? Other form factors have multiple variants/types in implementation as well. This would result in inconsistency among different form factors. In order to reach a consensus, could we keep a generic identity QSFP56_DD in parallel to QSFP56_DD_TYPE1, QSFP56_DD_TYPE2 and the newly-proposed QSFP56_DD_TYPE3? So the vendors can select what they would like to implement. |
Major YANG version changes in commit da629ae: |
Hi all, this PR is stalled. Could we let this PR merge as a band-aid solution and discuss refactoring and/or backward-incompatible changes to transceiver identities in a separate issue/PR? Many thanks for considering my request. |
@ejbrever can you take another look here? In my opinion we should have a different transceiver form factor type whenever the physical packaging and/or electrical interface changes. Further, it seems useful to only define transceiver form factor types that are a discreet, installable part. A transceiver form factor that aggregates multiple incompatible electrical interfaces (even if backwards compatible) seems like it's not a useful type to define. |
1c6e44b
to
9d47598
Compare
Gentle bump |
Hi Leon, after some internal discussion we came to the following recommendation: Introduce a new identity of "QSFP56_DD". This should be inclusive of type 1,2,3. Please add a reference statement to the MSA for QSFP56_DD. Change the existing identities of QSFP56_DD_TYPE1 and QSFP56_DD_TYPE2 to be deprecated. The rationale is that the TYPE 1,2,3 variations are not configuration or state that is reported by the device. Furthermore, it is our understanding that the physical form factor / cage for Type 1,2,3 devices is interchangable. This is based on our understanding on the specifications for QSFP56_DD. Please feel free to provide references if our conclusion is not well informed. Also, this may be obvious, but for community documentation benefit, I wanted to mention to differentiate link speeds delivered, the PMD is used.
|
Finally, the consensus is reached. :-) |
QSFP56_DD transceiver form-factor encompasses more variants than just type1 (8 optical signals) and type2 (4 optical signals) defined in OpenConfig. Therefore, adding a generic QSFP56_DD form-factor identity that encompasses all possible variants and deprecated the existing QSFP56_DD_TYPE1 and QSFP56_DD_TYPE2 form-factor identities. QSFP56 form-factor identity is also added.
Thanks for the feedback @dplore and @jqjqlee! The I still left the |
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.
Thanks for making these changes!
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.
LGTM, thank you.
QSFP56-DD transceiver form-factor encompasses more variants
than just type1 (8 optical signals) and type2 (4 optical signals)
defined in OpenConfig.
We should either use an umbrella QSFP56-DD identity to encompass all
variants, or add an additional identity to represent QSFP-DD 400G-ZR
(1 optical signal).