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

[gearbox] Set port speed to SAI_PORT_ATTR_SPEED for gearbox #1785

Merged
merged 2 commits into from
Aug 26, 2021

Conversation

byu343
Copy link
Contributor

@byu343 byu343 commented Jun 14, 2021

What I did
Set port speed to SAI_PORT_ATTR_SPEED for gearbox system-side/line-side ports. Lane speed was set before this change.

Why I did it
According to the example of "Creating ASIC Ports with 100G" in the SAI design doc: https://github.com/opencomputeproject/SAI/blob/master/doc/macsec-gearbox/SAI_Gearbox_API_Proposal-v1.0.docx, port speed should be set to SAI_PORT_ATTR_SPEED. However, the current code applies the lane speed to it. The SAI-based driver for the gearbox chip on Arista devices is assuming port speed for SAI_PORT_ATTR_SPEED, so we hope to submit this change to fix the issue we are facing.

How I verified it
Tested that traffic passing through the ports works and no related errors in syslog.

Details if related

  1. If there exists other platforms implementing the gearbox SAI APIs, please help to check whether they are using port speed or lane speed on SAI_PORT_ATTR_SPEED. I think we need to follow the same definition.
  2. Note that system_speed and line_speed in phy_config.json are still lane speed, following the design doc: https://github.com/Azure/SONiC/blob/master/doc/gearbox/gearbox_mgr_design.md.

@lguohan
Copy link
Contributor

lguohan commented Jul 15, 2021

is there a unit test that cover this part? can you add swss vs test?

@@ -1839,23 +1835,19 @@ bool PortsOrch::setGearboxPortAttr(Port &port, dest_port_type_t port_type, sai_p

// Gearbox expects speed per lane
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the line of comment "// Gearbox expects speed per lane".

@byu343 byu343 requested a review from prsunny as a code owner August 19, 2021 16:51
@jimmyzhai
Copy link
Contributor

The re-run of test vstest is successful.

@jimmyzhai jimmyzhai merged commit fc6cd81 into sonic-net:master Aug 26, 2021
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
…t#1785)

* Set port speed to SAI_PORT_ATTR_SPEED for gearbox system-side/line-side ports. Lane speed was set before this change.
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
…sonic-net#1785)

What I did
Verify database integrity before proceeding with warm reboot or fast reboot.
This integrity check uses a JSON schema to validate DBs. To start with, only counters_db's table COUNTERS_PORT_NAME_MAP presence is verified. But, this list can advance in future.
The test logic is designed to be generic; any more databases or tables within them can be just added to schema list, and the verification logic needs no change.
How I did it
Added a JSON schema, and generic schema validation logic.
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
The script was added in the PR sonic-net#1785 which did not add this script to the setup.py script.
Added the check_db_integrity script to setup.py.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants