Skip to content

[sonic-swss] Add port auto negotiation support to swss#1714

Merged
jleveque merged 7 commits intosonic-net:masterfrom
Junchao-Mellanox:port-auto-neg-review
May 19, 2021
Merged

[sonic-swss] Add port auto negotiation support to swss#1714
jleveque merged 7 commits intosonic-net:masterfrom
Junchao-Mellanox:port-auto-neg-review

Conversation

@Junchao-Mellanox
Copy link
Copy Markdown
Collaborator

What I did

  1. Added port auto negotiation attributes handle
  2. Get supported speeds for each port and save them to CONFIG_DB
  3. Added new test cases in VS test to verify the change

Why I did it

To support port auto negotiation feature.

How I verified it

Manual test and vs test

Details if related

@Junchao-Mellanox
Copy link
Copy Markdown
Collaborator Author

Depends on sonic-net/sonic-buildimage#7361

@Junchao-Mellanox
Copy link
Copy Markdown
Collaborator Author

Since sonic-net/sonic-buildimage#7361 is merged, could you please help re-triger the test?

@lguohan lguohan requested a review from jleveque April 24, 2021 20:10
jleveque
jleveque previously approved these changes Apr 28, 2021
@liat-grozovik
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@jleveque
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@Junchao-Mellanox Junchao-Mellanox requested a review from jleveque May 11, 2021 01:34
@Junchao-Mellanox
Copy link
Copy Markdown
Collaborator Author

Hi @jleveque , I have fixed the vs test issue, could you please kindly review it again?

jleveque
jleveque previously approved these changes May 11, 2021
@Junchao-Mellanox
Copy link
Copy Markdown
Collaborator Author

Fixed conflict

@jleveque jleveque merged commit 3629d70 into sonic-net:master May 19, 2021
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
1. Added port auto negotiation attributes handle
2. Get supported speeds for each port and save them to CONFIG_DB
3. Added new test cases in VS test to verify the change
@Junchao-Mellanox Junchao-Mellanox deleted the port-auto-neg-review branch October 29, 2021 01:44
Janetxxx pushed a commit to Janetxxx/sonic-swss that referenced this pull request Nov 10, 2025
1. Added port auto negotiation attributes handle
2. Get supported speeds for each port and save them to CONFIG_DB
3. Added new test cases in VS test to verify the change
@balanokia
Copy link
Copy Markdown

balanokia commented Nov 18, 2025

Hi @prsunny @prgeor @lguohan, the following AN feature code violates design of Gearbox port management and it would not be the correct behavior. We have a parent->child relationship in handling ASIC -> PHY ports(system/line) and hence we need to call setPort* APIs.
When AN is enabled and discovery done, forwarding ASIC faceplate index properties have been inherited as per Gearbox HLD design recommended API setPortSpeed. But the else condition always updates GearboxPort that would impact all the related port parameters like Equalization, Link training, FEC.
if (pCfg.speed.is_set)
{
if (p.m_speed != pCfg.speed.value)
{
}
else
{
/* Always update Gearbox speed on Gearbox ports */
setGearboxPortsAttr(p, SAI_PORT_ATTR_SPEED, &pCfg.speed.value);
}
}
Kindly review this code and share your feedback. Thanks

@lguohan
Copy link
Copy Markdown
Contributor

lguohan commented Nov 18, 2025

@prgeor ,can you review

@prsunny
Copy link
Copy Markdown
Collaborator

prsunny commented Nov 24, 2025

@balanokia , do you've a fix proposal/PR?

@balanokia
Copy link
Copy Markdown

balanokia commented Nov 25, 2025

hi @prsunny We should comment out the "else part of applying hardcoded speed to GB ports" now to make code clean. For the differential speed configuration as well as discovered speed, GB ports should follow setPortSpeed API. Then AN support for GB ports to be handled as another PR (it has to handle system and line side in sync with ASIC and remote end point). I will make a proposal in upcoming release. Pls share your note.

@balanokia
Copy link
Copy Markdown

@balanokia , do you've a fix proposal/PR?
Hi @prsunny Pls share if we can comment it out for cleaner code. Its becoming a precedent for isolated handling of gearbox ports/external PHY ports(line or sys side) without handling from ASIC index. I see, this PR is quoted as a precedent by another PR, #3529.
Thanks
Bala

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.

6 participants