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

Add gearbox phy device files and a new physyncd docker to support VS gearbox phy feature #4851

Merged
merged 38 commits into from
Sep 25, 2020

Conversation

sydlogan
Copy link
Contributor

@sydlogan sydlogan commented Jun 25, 2020

  • scripts and configuration needed to support a second syncd docker (gbsyncd)
  • gbsyncd 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.

sonic-net/sonic-utilities#931 - CLI
sonic-net/sonic-swss-common#347 - Minor changes
sonic-net/sonic-swss#1321 - gearsyncd, config parsers, changes to orchargent to create gearbox phy on supported systems
sonic-net/sonic-sairedis#632 - physyncd, virtual BCM81724 gearbox phy added to vslib

- How to verify it

In a vslib build (i.e., make target/sonic-vs.img.gz):

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 gbsyncd docker running.

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
@sydlogan sydlogan closed this Jun 26, 2020
@sydlogan sydlogan reopened this Jun 26, 2020
@sydlogan sydlogan closed this Jun 27, 2020
@sydlogan sydlogan reopened this Jun 27, 2020
@sydlogan
Copy link
Contributor Author

@

@sydlogan sydlogan closed this Jun 27, 2020
@sydlogan sydlogan reopened this Jun 27, 2020
@sydlogan sydlogan requested a review from sujinmkang July 1, 2020 03:13
@sydlogan
Copy link
Contributor Author

sydlogan commented Jul 3, 2020

@lguohan - could you please review and consider merge please, or do you have suggestion on who to request help from?

@xinliu-seattle
Copy link
Contributor

@jleveque, can you please review and sign off? thanks.

@sydlogan sydlogan closed this Jul 8, 2020
@sydlogan sydlogan reopened this Jul 8, 2020
@sydlogan sydlogan requested a review from jleveque July 8, 2020 16:54
@sydlogan sydlogan closed this Jul 8, 2020
@sydlogan sydlogan reopened this Jul 8, 2020
@sydlogan sydlogan closed this Jul 8, 2020
@sydlogan sydlogan reopened this Jul 8, 2020
@sydlogan sydlogan closed this Jul 8, 2020
@sydlogan sydlogan reopened this Jul 8, 2020
@sydlogan
Copy link
Contributor Author

@xinliu-seattle @jleveque

@sydlogan
Copy link
Contributor Author

Retest vsimage please

@sydlogan
Copy link
Contributor Author

Retest baseimage please

@slogan621
Copy link
Contributor

Some explanation of 6eaf960:

When looking at the change, it might be hard to see what was done just by looking at context diffs, but essentially this is what was done:

syncd_common.sh now has 99% of what was in syncd.sh and gbsyncd.

I found that in, say, the start entry point, syncd_common.sh was doing platform specific things that made no sense to do for phy/gb. This was also the case for the other entry points. These had to be pulled out and coupled with syncd,sh, decoupled from gbsyncd.sh.

So, I replaced that syncd-specific code with what effectively is a callback to the main script (e.g., syncd.sh). It's kind of like a callback, or maybe a perverse implementation of the pimpl pattern in the classic design patterns book (GOF). In gbsyncd, these callbacks became no-ops -- empty bash functions. In syncd, they implement that which only should be done for syncd, and are called at the very same point they were executed in the original script.

@slogan621
Copy link
Contributor

Retest vsimage please

2 similar comments
@slogan621
Copy link
Contributor

Retest vsimage please

@slogan621
Copy link
Contributor

Retest vsimage please

@lguohan
Copy link
Collaborator

lguohan commented Sep 23, 2020

can you also bring the submodule up to date in buildimage repo?

@slogan621
Copy link
Contributor

can you also bring the submodule up to date in buildimage repo?

Not sure I follow, which submodule needs to be updated?

@slogan621
Copy link
Contributor

Retest vsimage please

@slogan621
Copy link
Contributor

Retest mellanox please

@slogan621
Copy link
Contributor

Retest baseimage please

@lguohan
Copy link
Collaborator

lguohan commented Sep 24, 2020

retest vsimage please

@slogan621
Copy link
Contributor

can you also bring the submodule up to date in buildimage repo?

Not sure I follow, which submodule needs to be updated?

Resolved. sonic-sairedis and sonic-swss-common both had commits we needed, done in commit 623d5c0

lguohan
lguohan previously approved these changes Sep 25, 2020
@jleveque jleveque changed the title buildimage: Add gearbox phy device files and a new physyncd docker to… Add gearbox phy device files and a new physyncd docker to support VS gearbox phy feature Sep 25, 2020
@slogan621
Copy link
Contributor

Retest mellanox please

@lguohan lguohan merged commit 0311a4a into sonic-net:master Sep 25, 2020
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.