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

[orchagent] Put port configuration to APPL_DB according to autoneg mode #1769

Merged
merged 1 commit into from
Jul 7, 2021

Conversation

Junchao-Mellanox
Copy link
Collaborator

What I did

Before puting port related configuration to APPL_DB, check autoneg mode first. If autoneg mode is enabled, "speed" and "interface_type" will not be put into APPL_DB; if autoneg mode is disabled, "adv_speeds" and "adv_interface_types" will not be put into APPL_DB; else all configuration will be put to APPL_DB for backward compatible.

Why I did it

The field "speed" in PORT_TABLE of APPL_DB has two meanings:

  1. The configured port speed
  2. The actual port speed

This is all right when SONiC has no auto negotiation feature because the actual speed must equal to the configured speed or the operational status is down. However, with port auto negotiation feature, if autoneg mode is enabled, the actual speed could be a value of adv_speeds which might not equal to field "speed". Now port auto negotiation feature handle it this way:

  1. if autoneg mode is disabled, everything goes to the old way, the actual speed is always equal to field "speed";
  2. if autoneg mode is enabled, and if operational status is up, get actual speed from SAI and update the field "speed" in APPL_DB.

For case#2, there is an issue. Think about following flow:

  1. Set Ethernet0 speed=400000
  2. Set Ethernet0 autoneg=on, adv_speeds=100000, and Ethernet0 is operational up. Now the speed of Ethernet0 in "show interfaces status" is 100G because orchagent get the actual speed from SAI. This is right.
  3. Issue command: "config interface type Ethernet0 CR".

Step#3 won't affect the auto negotiation status because the autoneg mode is enabled, force interface type will not be applied to SAI. But it triggers PORT_TABLE change and portsyncd will push all Ethernet0 related configuration to APPL_DB. So, speed of Ethernet0 in "show interface status" changed to 400G.

This PR is to fix the issue.

How I verified it

Manual test

Details if related

Copy link
Contributor

@jleveque jleveque left a comment

Choose a reason for hiding this comment

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

Looks good to me. @prsunny to review as well.

@jleveque jleveque requested a review from prsunny June 11, 2021 22:02
@jleveque
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Collaborator

prsunny commented Jun 11, 2021

/azp run

@jleveque , there is a build failure while downloading swss-common. Tracking it with build team.

attrs.insert(attrs.end(), autoneg_attrs.begin(), autoneg_attrs.end());
attrs.insert(attrs.end(), force_attrs.begin(), force_attrs.end());
}
p.set(key, attrs);
Copy link
Collaborator

Choose a reason for hiding this comment

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

How is this handled during warmboot?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No special treatment for warmboot

@prsunny
Copy link
Collaborator

prsunny commented Jun 14, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Junchao-Mellanox
Copy link
Collaborator Author

Hi @prsunny , it seems 2 test cases related to flex counter failed which should not relate to my change. Could you please help re-trigger the test?

@liat-grozovik
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

@prsunny can you approve the PR so once the tests are passing we can merge?

@liat-grozovik
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

@prsunny kindly reminder, can you please approve and merge? now that is is passing the tests.

@prsunny prsunny merged commit ae44701 into sonic-net:master Jul 7, 2021
@Junchao-Mellanox Junchao-Mellanox deleted the fix-auto-neg branch July 8, 2021 02:53
judyjoseph pushed a commit that referenced this pull request Aug 10, 2021
…de (#1769)

*Before puting port related configuration to APPL_DB, check autoneg mode first. If autoneg mode is enabled, "speed" and "interface_type" will not be put into APPL_DB; if autoneg mode is disabled, "adv_speeds" and "adv_interface_types" will not be put into APPL_DB; else all configuration will be put to APPL_DB for backward compatible.
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
…de (sonic-net#1769)

*Before puting port related configuration to APPL_DB, check autoneg mode first. If autoneg mode is enabled, "speed" and "interface_type" will not be put into APPL_DB; if autoneg mode is disabled, "adv_speeds" and "adv_interface_types" will not be put into APPL_DB; else all configuration will be put to APPL_DB for backward compatible.
prsunny pushed a commit that referenced this pull request Mar 1, 2022
* Put port operational speed to STATE DB
* Remove previous workaround [orchagent] Put port configuration to APPL_DB according to autoneg mode #1769
judyjoseph pushed a commit that referenced this pull request Mar 7, 2022
* Put port operational speed to STATE DB
* Remove previous workaround [orchagent] Put port configuration to APPL_DB according to autoneg mode #1769
preetham-singh pushed a commit to preetham-singh/sonic-swss that referenced this pull request Aug 6, 2022
* Put port operational speed to STATE DB
* Remove previous workaround [orchagent] Put port configuration to APPL_DB according to autoneg mode sonic-net#1769
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.

None yet

4 participants