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

[portsorch] Expose supported FEC modes to STABE_DB and check whether FEC mode is supported before setting it #2400

Merged
merged 2 commits into from
Aug 8, 2022

Conversation

stephenxs
Copy link
Collaborator

@stephenxs stephenxs commented Jul 27, 2022

This is to

What I did
Expose supported FEC modes to STATE_DB.PORT_TABLE|<port>.supported_fecs.
The orchagent calls get_port_attribute to get attribute SAI_PORT_ATTR_SUPPORTED_FEC_MODE during initialization and then records it into internal data.

  • By default, the supported FEC modes will be returned by SAI and exposed to STATE_DB. Eg. rs,none means only rs and none is supported on the port.
    • The orchagent will check whether the FEC mode is supported on the port before calling the SAI API to set it.
    • The CLI will check whether the FEC mode is in supported_fecs before setting FEC mode on a port to the CONFIG_DB
  • In case the SAI API does not support any FEC mode on the port, N/A will be exposed to STATE_DB
    • The orchagent will deny any FEC setting and prints log.
    • The CLI will deny FEC setting.
  • In case the SAI API get_port_attribute returns Not implemented
    • No check will be performed before setting a FEC mode to SAI on the port.
    • The CLI will check whether the FEC mode is defined before setting it to CONFIG_DB.

Why I did it
It is not supported to set FEC mode on some platforms. To avoid error, we need to expose the supported FEC list.

How I verified it
Manually test and mock test.

Details if related

@stephenxs
Copy link
Collaborator Author

@prsunny @prgeor
can you approve and merge this PR? It is to take PR #2333 back and fix the compiling issue. it contains 2 commits

  1. revert the reverting PR
  2. resolve the compiling issue
    thank you.

@prsunny
Copy link
Collaborator

prsunny commented Jul 27, 2022

@prgeor , can you please sign-off/merge?

keboliu
keboliu previously approved these changes Jul 28, 2022
keboliu
keboliu previously approved these changes Jul 29, 2022
@stephenxs
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@keboliu
Copy link
Collaborator

keboliu commented Aug 2, 2022

@prgeor , would you please sign-off/merge?

@prgeor
Copy link
Contributor

prgeor commented Aug 4, 2022

@stephenxs 84a32e2 this commit doesnot seem right. lets discuss.

Copy link
Contributor

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

84a32e2 why merge from maser?

@stephenxs
Copy link
Collaborator Author

stephenxs commented Aug 4, 2022

84a32e2 why merge from maser?

@prgeor
Merging master back is a common way to resolve conflicts. We do it in this way from time to time

…d check whether FEC mode is supported before setting it (sonic-net#2333)" (sonic-net#2396)"

This reverts commit 6565b50.
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 4, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@stephenxs stephenxs requested a review from prgeor August 5, 2022 00:28
@stephenxs
Copy link
Collaborator Author

/easycla

@prgeor prgeor merged commit 1e1438e into sonic-net:master Aug 8, 2022
@stephenxs stephenxs deleted the readd-supported-fecs branch August 8, 2022 23:05
@yxieca
Copy link
Contributor

yxieca commented Aug 8, 2022

@stephenxs this PR cannot be cherry picked cleanly to 202205 branch. Please raise separate PR.

yxieca pushed a commit that referenced this pull request Aug 11, 2022
…FEC mode is supported before setting it (#2400)

* Revert "Revert "[portsorch] Expose supported FEC modes to STABE_DB and check whether FEC mode is supported before setting it (#2333)" (#2396)"

This reverts commit 6565b50.

* Adjust the prototype of setPortFec

Signed-off-by: Stephen Sun <[email protected]>
abdosi added a commit that referenced this pull request Aug 30, 2022
Why I did:

PR: #2400 made change to pass <string> as argument to API setPortFecMode but did not updated the corresponding gbsyncd API call
yxieca pushed a commit that referenced this pull request Sep 1, 2022
Why I did:

PR: #2400 made change to pass <string> as argument to API setPortFecMode but did not updated the corresponding gbsyncd API call
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