-
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
Design document for Y-cable API support for multiple vendors #757
Conversation
vdahiya12
commented
Mar 12, 2021
•
edited
Loading
edited
PR title | state | context |
---|---|---|
[sonic-platform-common] Abstract Class support for multiple vendors for muxcable | ||
[sonic-platform-common][Credo] Credo implementation for Abstract Class support for muxcable | ||
[sonic-platform-common][Broadcom] Broadcom implementation for Abstract Class support for muxcable | ||
[sonic-platform-daemons][XCVRD] xcvrd changes for Abstract Class support for muxcable | ||
[sonic-utilities][show][config] cli refactor for Abstract Class support for muxcable | ||
[sonic-buildimage] fwfiles mount support for PMON | ||
[sonic-platform-common] mux-simulator support for sonic-platform-common | ||
[sonic-mgmt] mux-simulator support for sonic-mgmt |
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
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.
As comments. I will wait to review further until we close on these items.
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@jleveque @vdahiya12 For the sonic_sfp refactor I'm planning on doing something similar with the vendor to module mapping, whereby a xcvr vendor that releases a product (say, some QSFP module) with use of vendor-specific fields in the EEPROM can create a vendor-specific module and have that be accessible via the mapping after determining the module's vendor name and part number. Something I'd like some clarification on is what "Y cable vendor" means in this doc - my initial understanding was that there is a distinct group of y cable vendors (distinct from xcvr vendors) that provide the actual y cables and have some some sort of spec that (at least with Credo) uses unused xcvr EEPROM pages for y cable logic, and that y cable vendors would be implementing the API described in this doc to control their particular y cable. It looks like the vendor name and part number mentioned come from reading the EEPROM of the xcvr connected to the physical port of TORA or TORB, so to me it seems like this is really the transceiver vendor info that we're retrieving. If it is then there might be some overlap with what I'm doing. |
@andywongarista: Your assumptions are correct. We need to check the vendor and part number from the EEPROM to determine which Y-cable API module to load. There may indeed be some overlap here. However, for the purposes of your refactoring, I would think that you're more interested in the transceiver identifier fields (in order to determine the proper spec) rather than vendor/part number fields. Can you explain why the vendor and part number fields are pertinent to your work? |
@jleveque Yes, checking the transceiver identifier is the main concern (at least with how things are now). The vendor/part number fields are also pertinent because of the hypothetical example that I mentioned; the specs define certain regions in the EEPROM which are vendor-specific. Currently we aren't interested in any such regions in the Sfp code, but perhaps in the future we will be. If a vendor were to provide a module extending some class representing a base spec with logic relating to these vendor-specific regions, then we would want a means of making use of that vendor's module, and my current thinking is that the vendor/part number fields would then become relevant in a similar manner to how they're used in this doc. |
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Gotcha. Yeah, in that case, you'd pretty much be using the vendor/part number fields in a very similar manner to what we're doing here, so there would indeed be some overlap. Do you intend to add this infrastructure as part of the refactor, even though, as you said, we currently aren't interested in vendor-specific regions in the SFP code? If so, then we can try to think of the best way to avoid duplicating code. |
Yes, I'm planning to create a mapping file anyways to link transceiver identifiers with the appropriate spec and I figured it would be best to make this mapping somewhat future-proof by making it easy to add vendor-specific mappings if they ever become needed. I suspect there will be a way to avoid having two mappings. What I'm still confused about is the meaning behind checking vendor name and part number from EEPROM to load the right Y-cable API module. The vendor name and part number read from EEPROM correspond to the transceiver vendor. What that means to me is that if a Credo Y-cable was connected to a QSFP from Vendor A, then potentially a different Y-cable API module would need to be loaded if the QSFP was swapped with one from Vendor B, even if we were still using the same Credo Y-cable. Thus, the Y-cable vendor (Credo) is not what's distinguishing the different Y-cable API modules, and so the "Y-cable vendor" wording used in the doc seems misleading. I may be missing something here conceptually so please do clarify if I am. |
With the Y-cables, the actual transceivers are active components and are fixed to the cable; they can not be replaced in the field. the entire cable along with all three transceivers is one unit and must be replaced as a whole. Therefore, the vendor/part number of the EEPROM in these cables will reliably tell us that the cable is a Y-cable. This is one difference from standard QSFP cables/transceivers. |
Ah, thanks for the clarification. In that case the infrastructure I'm adding for supporting different vendors should handle the Y-cable vendors as well. I'll provide an explanation for how this might work in the revised proposal and we can discuss later if it makes sense. The initial focus of the sonic_sfp refactoring won't be on integrating y cable functionality with the new design, so we'll need to sync up when that happens but I don't think the integration will be too troublesome. |
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
- Vendors can place their implementations in this format | ||
|
||
``` | ||
sonic_platform_common/sonic_y_cable/<vendor>/<module> |
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 can't we have vendor specific y_cable implementation in sonic-buildimage repo instead of platform-common repo (which shouldn't be having any changes/implementation which are platform specific)? For eg Sfpbase.py resides in platform-common whereas its realization/implementation reside in platform specific :- platform/broadcom/sonic-platform-modules-dell/z9264f/sonic_platform/sfp.py i.e in sonic-buildimage repo
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.
Hmm, platform is really a platform in a sense we have different SKU's, related configs etc, here these are just cables. Even though we could do something similar seems an overkill where it is easy to maintain everything in one place.
…I support for multiple vendors (#186) Description For multiple Y-Cable vendor support once we do have a mapping from vendor/part number to the appropriate Y-Cable module to load, we need to map appropriate port to a module as well. This PR adds definition for base abstract class. Motivation and Context Basically, the key idea is once we have a port identified as being of a certain vendor and it has been identified to load a certain module, it is then needed to call the correct module/API on each port each time we call the API on the port it is required to maintain this mapping in memory since xcvrd does not want to read/parse this y_cable_vendor_mapping.py file again and again each time we call the Y-Cable API Also note that the module loaded might change during xcvrd running lifetime since cable/transceiver can be changed from one vendor to another. So we need to take this into consideration as well Proposed Solution for this Each module of the Y-Cable vendor can be a class (of each transceiver type) and all we need to do is instantiate the objects of these classes as class instances and these objects will provide the interface of calling the API's for the appropriate vendor Y-Cable. This instantiation will be done inside xcvrd, when xcvrd starts These objects basically emulate Y-Cable instances and whatever action/interaction needs to be done with the Y-Cable the methods of these objects would provide that each vendor in their implementation can inherit from a base class where there will be definitions for all the supported capabilities of the Y-Cable. for vendors the recommended approach in case their subclass implementation does not support a method, is to set the method equal to None. This differentiates it from a method they forgot to implement. Then, the calling code should first check if the method is None before attempting to call it. Design document for the support is sonic-net/SONiC#757 Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
…I support for multiple vendors (#186) Description For multiple Y-Cable vendor support once we do have a mapping from vendor/part number to the appropriate Y-Cable module to load, we need to map appropriate port to a module as well. This PR adds definition for base abstract class. Motivation and Context Basically, the key idea is once we have a port identified as being of a certain vendor and it has been identified to load a certain module, it is then needed to call the correct module/API on each port each time we call the API on the port it is required to maintain this mapping in memory since xcvrd does not want to read/parse this y_cable_vendor_mapping.py file again and again each time we call the Y-Cable API Also note that the module loaded might change during xcvrd running lifetime since cable/transceiver can be changed from one vendor to another. So we need to take this into consideration as well Proposed Solution for this Each module of the Y-Cable vendor can be a class (of each transceiver type) and all we need to do is instantiate the objects of these classes as class instances and these objects will provide the interface of calling the API's for the appropriate vendor Y-Cable. This instantiation will be done inside xcvrd, when xcvrd starts These objects basically emulate Y-Cable instances and whatever action/interaction needs to be done with the Y-Cable the methods of these objects would provide that each vendor in their implementation can inherit from a base class where there will be definitions for all the supported capabilities of the Y-Cable. for vendors the recommended approach in case their subclass implementation does not support a method, is to set the method equal to None. This differentiates it from a method they forgot to implement. Then, the calling code should first check if the method is None before attempting to call it. Design document for the support is sonic-net/SONiC#757 Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
…for calling Y-Cable API's inside xcvrd (#197) Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com Description This PR integrates vendor specific class objects inside xcvrd for Y-Cable API's to be called. Detailed designed document can be found sonic-net/SONiC#757 Motivation and Context Basically xcvrd now has an interface for Y-Cable to interact with PMON docker and HOST which can be uniform across all vendors. As part of this refactor, we will be moving towards a model where only xcvrd interacts with the cables/transceivers, and host-side processes will communicate with xcvrd rather than with the devices directly with Y-Cable. How Has This Been Tested? Ran the changes on Arista7050cx3 switch, making changes inside the container. Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
…tation from vendors (#1722) Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com What I did this PR adds support for cli refactor which is done for abstract class implementation of muxcable which is provided by vendors. detailed design doc can be found sonic-net/SONiC#757 How I did it Added the changes in muxcable.py in show and config . How to verify it add unit-tests as well as run on a Arista-7050cx3 switch, run unit tests. no change in outputs, just the functionality has changed. Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
…tation from vendors (sonic-net#1722) Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com What I did this PR adds support for cli refactor which is done for abstract class implementation of muxcable which is provided by vendors. detailed design doc can be found sonic-net/SONiC#757 How I did it Added the changes in muxcable.py in show and config . How to verify it add unit-tests as well as run on a Arista-7050cx3 switch, run unit tests. no change in outputs, just the functionality has changed. Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
…tation from vendors (sonic-net#1722) Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com What I did this PR adds support for cli refactor which is done for abstract class implementation of muxcable which is provided by vendors. detailed design doc can be found sonic-net/SONiC#757 How I did it Added the changes in muxcable.py in show and config . How to verify it add unit-tests as well as run on a Arista-7050cx3 switch, run unit tests. no change in outputs, just the functionality has changed. Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
…for calling Y-Cable API's inside xcvrd (sonic-net#197) Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com Description This PR integrates vendor specific class objects inside xcvrd for Y-Cable API's to be called. Detailed designed document can be found sonic-net/SONiC#757 Motivation and Context Basically xcvrd now has an interface for Y-Cable to interact with PMON docker and HOST which can be uniform across all vendors. As part of this refactor, we will be moving towards a model where only xcvrd interacts with the cables/transceivers, and host-side processes will communicate with xcvrd rather than with the devices directly with Y-Cable. How Has This Been Tested? Ran the changes on Arista7050cx3 switch, making changes inside the container. Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com Description This PR integrates vendor specific class objects inside xcvrd for Y-Cable API's to be called. Detailed designed document can be found sonic-net/SONiC#757 Motivation and Context Basically xcvrd now has an interface for Y-Cable to interact with PMON docker and HOST which can be uniform across all vendors. As part of this refactor, we will be moving towards a model where only xcvrd interacts with the cables/transceivers, and host-side processes will communicate with xcvrd rather than with the devices directly with Y-Cable. How Has This Been Tested? Ran the changes on Arista7050cx3 switch, making changes inside the container. Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
…for calling Y-Cable API's inside xcvrd (#197) (#213) Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com Merging this in 202012 branch because there was a merge conflict in original PR #197 Description This PR integrates vendor specific class objects inside xcvrd for Y-Cable API's to be called. Detailed designed document can be found sonic-net/SONiC#757 Motivation and Context Basically xcvrd now has an interface for Y-Cable to interact with PMON docker and HOST which can be uniform across all vendors. As part of this refactor, we will be moving towards a model where only xcvrd interacts with the cables/transceivers, and host-side processes will communicate with xcvrd rather than with the devices directly with Y-Cable. How Has This Been Tested? Ran the changes on Arista7050cx3 switch, making changes inside the container. Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com
…tation from vendors (#1722) (#1782) Merging this in 202012 branch because there was a merge conflict in original PR while cherry-picking #1722 #### What I did this PR adds support for cli refactor which is done for abstract class implementation of muxcable which is provided by vendors. detailed design doc can be found sonic-net/SONiC#757 #### How I did it Added the changes in muxcable.py in show and config . #### How to verify it add unit-tests as well as run on a Arista-7050cx3 switch, run unit tests. no change in outputs, just the logic has changed. Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
…tation from vendors (#1722) Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com What I did this PR adds support for cli refactor which is done for abstract class implementation of muxcable which is provided by vendors. detailed design doc can be found sonic-net/SONiC#757 How I did it Added the changes in muxcable.py in show and config . How to verify it add unit-tests as well as run on a Arista-7050cx3 switch, run unit tests. no change in outputs, just the functionality has changed. Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>