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

swss-common: Changes to support SONiC Gearbox Manager #347

Merged
merged 6 commits into from
Jun 16, 2020

Conversation

sydlogan
Copy link
Contributor

@sydlogan sydlogan commented Jun 2, 2020

  • add class GearboxUtils, contains APIs useful for gearbox support across various submodules.

  • add additional REDIS tables to support second (PHY) syncd.

    Signed-off-by: syd.logan@broadcom.com

* add class GearboxUtils, contains APIs useful for gearbox support across various submodules.
* add additional REDIS tables to support second (PHY) syncd.

  Signed-off-by: syd.logan@broadcom.com
@ghost
Copy link

ghost commented Jun 2, 2020

CLA assistant check
All CLA requirements met.

common/gearboxutils.cpp Outdated Show resolved Hide resolved
common/table.cpp Outdated
{ PFC_WD_DB, TABLE_NAME_SEPARATOR_COLON },
{ FLEX_COUNTER_DB, TABLE_NAME_SEPARATOR_COLON },
{ STATE_DB, TABLE_NAME_SEPARATOR_VBAR },
{ ASIC_DB2, TABLE_NAME_SEPARATOR_VBAR },
Copy link
Contributor

Choose a reason for hiding this comment

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

what is ASIC_DB2? it is unclear what it is.

Choose a reason for hiding this comment

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

Hi Guohan;

The new ASIC_DB2 (as well as counters, flex, and state) database designations are being used by Gearbox and the new PHY SYNCD DOCKER.
These definitions are also specified in the specific HWSKU context_config.json files, such as in the following excerpt;

{
"guid" : 1,
"name" : "phy1",
"dbAsic" : "ASIC_DB2",
"dbCounters" : "COUNTERS_DB2",
"dbFlex": "FLEX_COUNTER_DB2",
"dbState" : "STATE_DB2",
"switches": [
{
"index" : 1,
"hwinfo" : ""
}
]
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should rename it to PHY_DB

Choose a reason for hiding this comment

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

Just thinking ahead here...
The design supports multiple phys for which each phy "could" have their own set of databases.
So I'm guessing the naming scheme made something like?
PHY_DB
PHY_DB2
etc...
?

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 used asic_db2 because we antiipate multiple switches in the future, and phy and switch are not differentiated in sairedis, so I believe we should give them all the same name. In the future, we may have multiredis support and we might then consider not needing this (redis then become namespace for the table names, one redis per switch). Please accept the change as it currently is proposed, future this might change but for now, asic_db2 is most consistent naming for switch related tables.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think all the phy db can be put into the same db since there are not much query/config activities. I think PHY_DB is a better name and give user more information on the nature of this db.

Copy link
Contributor

Choose a reason for hiding this comment

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

just to clarify, a added "2" for each of second db names since i didn't have any better name, we could add PHY or GB (as gearbox) prefixx or something more explanatory then just "2"

common/Makefile.am Outdated Show resolved Hide resolved
@sydlogan sydlogan requested review from lguohan and kcudnik June 10, 2020 17:48
kcudnik
kcudnik previously approved these changes Jun 10, 2020
@ben-gale
Copy link

Please Merge this ASAP as we have other dependent PR's pending on this.

@sydlogan
Copy link
Contributor Author

@lguohan removed gearboxutils, explained why the naming convention was chosen. Re-requesting review. Thanks!

common/schema.h Outdated Show resolved Hide resolved
common/schema.h Outdated Show resolved Hide resolved
@sydlogan sydlogan requested a review from lguohan June 16, 2020 09:21
@lguohan lguohan merged commit 292b08a into sonic-net:master Jun 16, 2020
@sydlogan
Copy link
Contributor Author

@lguohan thank you for the review and approval

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 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
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.

6 participants