[swss] Add support for gearbox phys #1321
Conversation
* builds on support for multiple switches in sonic-sairedis * adds gearsync daemon * parsers for gearbox-related configuration files, plus unit tests * gearboxutils for determining if gearbox supported and other functions * changes to portsorch and orchagent to support gearbox creation and attributes HLD is located at https://github.com/Azure/SONiC/blob/b817a12fd89520d3fd26bbc5897487928e7f6de7/doc/gearbox/gearbox_mgr_design.md Signed-off-by: syd.logan@broadcom.com
… support VS gearbox phy feature * scripts and configuration needed to support a second syncd docker (physyncd) * physyncd supports gearbox device and phy SAI APIs and runs multiple instances of syncd, one per phy in the device * support for VS target (sonic-sairedis vslib has been extended to support a virtual BCM81724 gearbox PHY). HLD is located at https://github.com/Azure/SONiC/blob/b817a12fd89520d3fd26bbc5897487928e7f6de7/doc/gearbox/gearbox_mgr_design.md **- Why I did it** This work is part of the gearbox phy joint effort between Microsoft and Broadcom, and is based on multi-switch support in sonic-sairedis. **- How I did it** Overall feature was implemented across several projects. The collective pull requests (some in late stages of review at this point): sonic-net/sonic-utilities#931 - CLI (merged) sonic-net/sonic-swss-common#347 - Minor changes (merged) sonic-net/sonic-swss#1321 - gearsyncd, config parsers, changes to orchargent to create gearbox phy on supported systems sonic-net/sonic-sairedis#624 - physyncd, virtual BCM81724 gearbox phy added to vslib **- How to verify it** In a vslib build: root@sonic:/home/admin# show gearbox interfaces status PHY Id Interface MAC Lanes MAC Lane Speed PHY Lanes PHY Lane Speed Line Lanes Line Lane Speed Oper Admin -------- ----------- --------------- ---------------- --------------- ---------------- ------------ ----------------- ------ ------- 1 Ethernet48 121,122,123,124 25G 200,201,202,203 25G 204,205 50G down down 1 Ethernet49 125,126,127,128 25G 206,207,208,209 25G 210,211 50G down down 1 Ethernet50 69,70,71,72 25G 212,213,214,215 25G 216 100G down down In addition, docker ps | grep phy should show a physyncd docker running. Signed-off-by: syd.logan@broadcom.com
|
@jleveque no known outstanding issues, could you please re-review and consider approval and merge? Thanks! |
|
@lguohan: Are you OK with the inclusion of CUint here for unit testing? Just wanted to check if we intended to unify all unit tests to use one framework. |
|
A short comment on cunit for perspective - the tests are not currently run as a part of the build, nor are we including them in any sort of tests at gate. The primary reason for these tests has been to support TDD as we modify the parser. An example, these tests proved invaluable to have a week ago when I ported from cjson to swss::json.hpp to validate the port did not regress anything (yes, we originally were using cjson but prior to the pull request, consulted with @kcudnik and decided we would need to move to swss::json.hpp). cunit was the best tool for the job in my estimation, tar the test directory after a build, scp it to a switch, and run the tests to validate the parser, again in a TDD fashion. |
|
@sydlogan: I'm unsure if we want to check the CUnit files into this repo, or if we should download and copy them at build time. The CUnit files appear to be unmodified by you, is that correct? |
Correct. Perhaps an example of how this might be done would be helpful. Given the desire to get this Gearbox change into 62020 Release, could we maybe make this a follow on issue for a subsequent pull request? |
@jleveque We could (I think) use the sonic-buildimage slave to install cunit-dev (via apt install, see https://github.com/Azure/sonic-buildimage/blob/master/sonic-slave-buster/Dockerfile.j2#L40 for example of general area of this change). If you agree, I propose we remove cunit sources from swss, disable building the tests, but leave the tests. and we can get swss merged. Then I can come back later to update sonic-buildimage and sonic-swss with the changes needed to install cunit-dev deb package and build the tests, as a separate pull requests. Will that work for you? |
|
Looks good to me at this point. Since the PR is quite large, I'd also like @kcudnik to review, who also works more closely with this codebase/repo. |
|
@sydlogan: I will merge once the vs test build completes successfully. |
jleveque
changed the title
|
for the jumping late, i think sonic-swss used googletest for the c++ unit test, can we use that. I do not want to introduce another unit test framework for sonic-swss. |
…gearbox phy feature (#4851) * buildimage: Add gearbox phy device files and a new physyncd docker to support VS gearbox phy feature * scripts and configuration needed to support a second syncd docker (physyncd) * physyncd supports gearbox device and phy SAI APIs and runs multiple instances of syncd, one per phy in the device * support for VS target (sonic-sairedis vslib has been extended to support a virtual BCM81724 gearbox PHY). HLD is located at https://github.com/Azure/SONiC/blob/b817a12fd89520d3fd26bbc5897487928e7f6de7/doc/gearbox/gearbox_mgr_design.md **- Why I did it** This work is part of the gearbox phy joint effort between Microsoft and Broadcom, and is based on multi-switch support in sonic-sairedis. **- How I did it** Overall feature was implemented across several projects. The collective pull requests (some in late stages of review at this point): sonic-net/sonic-utilities#931 - CLI (merged) sonic-net/sonic-swss-common#347 - Minor changes (merged) sonic-net/sonic-swss#1321 - gearsyncd, config parsers, changes to orchargent to create gearbox phy on supported systems sonic-net/sonic-sairedis#624 - physyncd, virtual BCM81724 gearbox phy added to vslib **- How to verify it** In a vslib build: root@sonic:/home/admin# show gearbox interfaces status PHY Id Interface MAC Lanes MAC Lane Speed PHY Lanes PHY Lane Speed Line Lanes Line Lane Speed Oper Admin -------- ----------- --------------- ---------------- --------------- ---------------- ------------ ----------------- ------ ------- 1 Ethernet48 121,122,123,124 25G 200,201,202,203 25G 204,205 50G down down 1 Ethernet49 125,126,127,128 25G 206,207,208,209 25G 210,211 50G down down 1 Ethernet50 69,70,71,72 25G 212,213,214,215 25G 216 100G down down In addition, docker ps | grep phy should show a physyncd docker running. Signed-off-by: syd.logan@broadcom.com
…gearbox phy feature (sonic-net#4851) * buildimage: Add gearbox phy device files and a new physyncd docker to support VS gearbox phy feature * scripts and configuration needed to support a second syncd docker (physyncd) * physyncd supports gearbox device and phy SAI APIs and runs multiple instances of syncd, one per phy in the device * support for VS target (sonic-sairedis vslib has been extended to support a virtual BCM81724 gearbox PHY). HLD is located at https://github.com/Azure/SONiC/blob/b817a12fd89520d3fd26bbc5897487928e7f6de7/doc/gearbox/gearbox_mgr_design.md **- Why I did it** This work is part of the gearbox phy joint effort between Microsoft and Broadcom, and is based on multi-switch support in sonic-sairedis. **- How I did it** Overall feature was implemented across several projects. The collective pull requests (some in late stages of review at this point): sonic-net/sonic-utilities#931 - CLI (merged) sonic-net/sonic-swss-common#347 - Minor changes (merged) sonic-net/sonic-swss#1321 - gearsyncd, config parsers, changes to orchargent to create gearbox phy on supported systems sonic-net/sonic-sairedis#624 - physyncd, virtual BCM81724 gearbox phy added to vslib **- How to verify it** In a vslib build: root@sonic:/home/admin# show gearbox interfaces status PHY Id Interface MAC Lanes MAC Lane Speed PHY Lanes PHY Lane Speed Line Lanes Line Lane Speed Oper Admin -------- ----------- --------------- ---------------- --------------- ---------------- ------------ ----------------- ------ ------- 1 Ethernet48 121,122,123,124 25G 200,201,202,203 25G 204,205 50G down down 1 Ethernet49 125,126,127,128 25G 206,207,208,209 25G 210,211 50G down down 1 Ethernet50 69,70,71,72 25G 212,213,214,215 25G 216 100G down down In addition, docker ps | grep phy should show a physyncd docker running. Signed-off-by: syd.logan@broadcom.com
What I did
Add support for gearbox phys based on sonic-sairedis support of multiple switches.
Why I did it
Work is done in collaboration with Microsoft engineers.
How I verified it
Builds and tests using VS builds (dependency on sonic-sairedis changes that are pending).
Details if related