sonic-sairedis: Add support to sonic-sairedis for gearbox phys #624
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* builds on support for multiple switches in sonic-sairedis
* new vslib switch BCM81724 implements a virtual gearbox phy.
* support for launching second (BCM81724 is supported by its own syncd)
* simple refactoring of tests to support switches by part number, still working with
sairedis to support multiple switch in tests so BCM81724 will be a separate pull request)
* changed example context_config.json to reflect renaming of phy REDIS tables (see sonic-swss-common
commit 292b08a3a80b24b23663020b37e6260039a311c0)
Note that a future commit to sonic-buildimage will be required to trigger launch of physyncd
(launching is based on device config files which are currently not present in sonic-buildimage).
Testing done in multiple environments (broadcom fork and pure upstream). Example CLI output
based on changes pushed to sonic-utilities (commit a6c4456f6965b79bf9d02ff1962070a5eae6ea55)
running in VS switch supporting BCM81724:
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 Ethernet0 25,26,27,28 10G 200,201 20G 206 40G up up
1 Ethernet4 29,30,31,32 10G 202,203 20G 207 40G up up
1 Ethernet8 33,34,35,36 10G 204,205 20G 208 40G up up
HLD is located at https://github.com/Azure/SONiC/blob/b817a12fd89520d3fd26bbc5897487928e7f6de7/doc/gearbox/gearbox_mgr_design.md
Signed-off-by: syd.logan@broadcom.com
kcudnik
reviewed
Jun 18, 2020
kcudnik
reviewed
Jun 18, 2020
kcudnik
reviewed
Jun 18, 2020
syncd/SaiSwitch.cpp
Outdated
|
|
||
| if (status != SAI_STATUS_SUCCESS) | ||
| { | ||
| SWSS_LOG_ERROR("failed to get switch type"); |
There was a problem hiding this comment.
in case of failure we should throw here, or return explicitly default switch type
kcudnik
reviewed
Jun 18, 2020
| @@ -5,6 +5,10 @@ extern "C" { | |||
| } | |||
|
|
|||
| #define SAI_KEY_VS_SWITCH_TYPE "SAI_VS_SWITCH_TYPE" | |||
| #define SAI_KEY_VS_SAI_SWITCH_TYPE "SAI_VS_SAI_SWITCH_TYPE" | |||
|
|
|||
| #define SAI_VALUE_SAI_SWITCH_TYPE_NPU "SAI_SWITCH_TYPE_NPU" | |||
There was a problem hiding this comment.
why you need this ? if i understand correctly this is attribute on switch_create
kcudnik
reviewed
Jun 18, 2020
kcudnik
reviewed
Jun 18, 2020
kcudnik
reviewed
Jun 18, 2020
| if (sai_switch_type == NULL) | ||
| { | ||
| SWSS_LOG_NOTICE("failed to obtain service method table value: %s", SAI_KEY_VS_SAI_SWITCH_TYPE); | ||
| saiSwitchType = SAI_SWITCH_TYPE_NPU; |
There was a problem hiding this comment.
why you need this, i though this is switch_create attribute
There was a problem hiding this comment.
We need, at this early state, to know if this is NPU, which supports the starting of fdb agent thread, or is PHY, which does not. We experienced failures if we start this thread for phy, and it is not desireable anyway. Introducing the switch type in the ini file as in this example, is how we determine type.
SAI_WARM_BOOT_READ_FILE=./sai_warmboot.bin
SAI_WARM_BOOT_WRITE_FILE=./sai_warmboot.bin
SAI_VS_SWITCH_TYPE=SAI_VS_SWITCH_TYPE_BCM81724
SAI_VS_SAI_SWITCH_TYPE=SAI_SWITCH_TYPE_PHY
SAI_VS_HOSTIF_USE_TAP_DEVICE=false
SAI_VS_USE_BCMSIM_LINK_MON=true
SAI_VS_INTERFACE_LANE_MAP_FILE=BCM81724/lanemapphy.ini
Another option would be to hard code a mapping of part number to type somewhere, but that hardly seems like a good solution (might as well put it here, where the switch resource is defined).
The field is considered optional (default is NPU) since that is the dominant switch type, and for backwards compatibility. Effectively, at this point, making it a boolean "isPhy" but if you consider that downstream there might be a third or fourth class of "switch", better to call it out explicitly.
kcudnik
reviewed
Jun 18, 2020
kcudnik
reviewed
Jun 18, 2020
|
|
||
| std::string st = (saiSwitchTypeStr == NULL) ? "unknown" : saiSwitchTypeStr; | ||
|
|
||
| if (st == SAI_VALUE_SAI_SWITCH_TYPE_NPU) |
There was a problem hiding this comment.
same here, why this is needed, this is switch_create attribute
There was a problem hiding this comment.
So that we know chip function and can avoid launch of fdb thread, which is not valid for phy.
kcudnik
reviewed
Jun 18, 2020
kcudnik
requested changes
Jun 18, 2020
lguohan
reviewed
Jun 23, 2020
kcudnik
previously approved these changes
Jun 23, 2020
kcudnik
previously approved these changes
Jun 24, 2020
lguohan
reviewed
Jun 25, 2020
lguohan
reviewed
Jun 25, 2020
lguohan
reviewed
Jun 25, 2020
lguohan
previously approved these changes
Jun 25, 2020
…hysyncd_start.sh to launch via python script bypassing syncd_init_common
sydlogan
pushed a commit
to sydlogan/sonic-buildimage
that referenced
this pull request
Jun 25, 2020
… 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
lguohan
approved these changes
Jun 26, 2020
|
lgtm, @kcudnik, please merge if you are ok with it. |
sydlogan
added a commit
to sydlogan/sonic-sairedis
that referenced
this pull request
Jun 29, 2020
…-net#624) * sonic-sairedis: Add support to sonic-sairedis for gearbox phys * builds on support for multiple switches in sonic-sairedis * new vslib switch BCM81724 implements a virtual gearbox phy. * support for launching second (BCM81724 is supported by its own syncd) * simple refactoring of tests to support switches by part number, still working with sairedis to support multiple switch in tests so BCM81724 will be a separate pull request) * changed example context_config.json to reflect renaming of phy REDIS tables (see sonic-swss-common commit 292b08a3a80b24b23663020b37e6260039a311c0) Note that a future commit to sonic-buildimage will be required to trigger launch of physyncd (launching is based on device config files which are currently not present in sonic-buildimage). Testing done in multiple environments (broadcom fork and pure upstream). Example CLI output based on changes pushed to sonic-utilities (commit a6c4456f6965b79bf9d02ff1962070a5eae6ea55) running in VS switch supporting BCM81724: 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 Ethernet0 25,26,27,28 10G 200,201 20G 206 40G up up 1 Ethernet4 29,30,31,32 10G 202,203 20G 207 40G up up 1 Ethernet8 33,34,35,36 10G 204,205 20G 208 40G up up HLD is located at https://github.com/Azure/SONiC/blob/b817a12fd89520d3fd26bbc5897487928e7f6de7/doc/gearbox/gearbox_mgr_design.md Signed-off-by: syd.logan@broadcom.com * Address review comments * recode syncd_init_common.sh to support physyncd * revert syncd_init_common.sh to support only launching syncd, modify physyncd_start.sh to launch via python script bypassing syncd_init_common Co-authored-by: Syd Logan <slogan621@gmail.com>
lguohan
pushed a commit
to sonic-net/sonic-buildimage
that referenced
this pull request
Sep 25, 2020
…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
santhosh-kt
pushed a commit
to santhosh-kt/sonic-buildimage
that referenced
this pull request
Feb 25, 2021
…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
pettershao-ragilenetworks
pushed a commit
to pettershao-ragilenetworks/sonic-sairedis
that referenced
this pull request
Nov 18, 2022
…-net#624) * sonic-sairedis: Add support to sonic-sairedis for gearbox phys * builds on support for multiple switches in sonic-sairedis * new vslib switch BCM81724 implements a virtual gearbox phy. * support for launching second (BCM81724 is supported by its own syncd) * simple refactoring of tests to support switches by part number, still working with sairedis to support multiple switch in tests so BCM81724 will be a separate pull request) * changed example context_config.json to reflect renaming of phy REDIS tables (see sonic-swss-common commit 292b08a3a80b24b23663020b37e6260039a311c0) Note that a future commit to sonic-buildimage will be required to trigger launch of physyncd (launching is based on device config files which are currently not present in sonic-buildimage). Testing done in multiple environments (broadcom fork and pure upstream). Example CLI output based on changes pushed to sonic-utilities (commit a6c4456f6965b79bf9d02ff1962070a5eae6ea55) running in VS switch supporting BCM81724: 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 Ethernet0 25,26,27,28 10G 200,201 20G 206 40G up up 1 Ethernet4 29,30,31,32 10G 202,203 20G 207 40G up up 1 Ethernet8 33,34,35,36 10G 204,205 20G 208 40G up up HLD is located at https://github.com/Azure/SONiC/blob/b817a12fd89520d3fd26bbc5897487928e7f6de7/doc/gearbox/gearbox_mgr_design.md Signed-off-by: syd.logan@broadcom.com * Address review comments * recode syncd_init_common.sh to support physyncd * revert syncd_init_common.sh to support only launching syncd, modify physyncd_start.sh to launch via python script bypassing syncd_init_common Co-authored-by: Syd Logan <slogan621@gmail.com>
pettershao-ragilenetworks
pushed a commit
to pettershao-ragilenetworks/sonic-sairedis
that referenced
this pull request
Nov 18, 2022
sonic-net#624)" (sonic-net#630) This reverts commit 2772f15.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
sairedis to support multiple switch in tests so BCM81724 will be a separate pull request)
commit 292b08a3a80b24b23663020b37e6260039a311c0)
Note that a future commit to sonic-buildimage will be required to trigger launch of physyncd
(launching is based on device config files which are currently not present in sonic-buildimage).
Testing done in multiple environments (broadcom fork and pure upstream). Example CLI output
based on changes pushed to sonic-utilities (commit a6c4456f6965b79bf9d02ff1962070a5eae6ea55)
running in VS switch supporting BCM81724:
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
HLD is located at https://github.com/Azure/SONiC/blob/b817a12fd89520d3fd26bbc5897487928e7f6de7/doc/gearbox/gearbox_mgr_design.md
Signed-off-by: syd.logan@broadcom.com