Skip to content
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

sonic-sairedis: Add support to sonic-sairedis for gearbox phys #624

Merged
merged 4 commits into from
Jun 26, 2020
Merged

sonic-sairedis: Add support to sonic-sairedis for gearbox phys #624

merged 4 commits into from
Jun 26, 2020

Conversation

sydlogan
Copy link
Contributor

  • 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

* 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
lib/src/RedisChannel.cpp Outdated Show resolved Hide resolved
syncd/SaiSwitch.cpp Outdated Show resolved Hide resolved

if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("failed to get switch type");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in case of failure we should throw here, or return explicitly default switch type

@@ -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"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why you need this ? if i understand correctly this is attribute on switch_create

vslib/src/Makefile.am Outdated Show resolved Hide resolved
vslib/src/Sai.cpp Outdated Show resolved Hide resolved
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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why you need this, i though this is switch_create attribute

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

vslib/src/Sai.cpp Outdated Show resolved Hide resolved

std::string st = (saiSwitchTypeStr == NULL) ? "unknown" : saiSwitchTypeStr;

if (st == SAI_VALUE_SAI_SWITCH_TYPE_NPU)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, why this is needed, this is switch_create attribute

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that we know chip function and can avoid launch of fdb thread, which is not valid for phy.

vslib/src/SwitchConfig.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@kcudnik kcudnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please address comments

@sydlogan sydlogan requested a review from kcudnik June 19, 2020 16:14
kcudnik
kcudnik previously approved these changes Jun 23, 2020
kcudnik
kcudnik previously approved these changes Jun 24, 2020
lguohan
lguohan previously approved these changes Jun 25, 2020
…hysyncd_start.sh to launch via python script bypassing syncd_init_common
@sydlogan sydlogan dismissed stale reviews from lguohan and kcudnik via 9e2d0c4 June 25, 2020 20:08
@sydlogan sydlogan requested review from lguohan and kcudnik June 25, 2020 20:23
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
Copy link
Contributor

lguohan commented Jun 26, 2020

lgtm, @kcudnik, please merge if you are ok with it.

@kcudnik kcudnik merged commit 2772f15 into sonic-net:master Jun 26, 2020
@sydlogan
Copy link
Contributor Author

@kcudnik and @lguohan thanks for the careful reviews, approval, merge.

daall added a commit that referenced this pull request Jun 27, 2020
lguohan pushed a commit that referenced this pull request Jun 27, 2020
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
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.

4 participants