Navigation Menu

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

Jump to bottom

SAI interface does not provide a way to set the FEC and speed of a port together #1322

Open
mikeberesford opened this issue Oct 5, 2021 · 5 comments
Open

SAI interface does not provide a way to set the FEC and speed of a port together #1322

mikeberesford opened this issue Oct 5, 2021 · 5 comments

Comments

@mikeberesford
Copy link
Contributor

There are cases where we technically cannot change from one port speed/FEC combination to another using the existing single-attribute write.

As an example: 40G/FC-FEC to 100G/RS-FEC - it is not valid to change to 100G with FC-FEC enabled, and likewise it is not valid to enable RS-FEC on a 40G link.

Previous discussion on this was under #1308

Some ideas that were discussed:

  • Add a new attribute to set both speed and FEC in one attribute
    • would allow configuration using the existing APIs
    • configuration of "speed+fec" attribute would implicitly change both the existing port speed and fec attributes, and vice versa
    • would probably need to deprecate the existing speed and FEC settings (not backwards compatible), making speed+fec mandatory on create (FEC was not previously mandatory)
  • define new multi-attribute set API or use bulk set
    • bulk set doesn't appear to exist for port attributes, probably would require new API
    • would require explicit documentation that speed and FEC are set together so NOS can handle appropriately
@mikeberesford mikeberesford mentioned this issue Oct 5, 2021
Closed
@mikeberesford
Copy link
Contributor Author

@kcudnik @JaiOCP @marian-pritsak @rlhui any continued thoughts on how we might handle this would be appreciated.

@mikeberesford
Copy link
Contributor Author

Introduce another attribute? On use-case to set speed+fec together, then set speed and fec, then set attribute to false. Effectively start limited transaction (just for speed and FEC)

@kcudnik
Copy link
Collaborator

kcudnik commented Oct 18, 2021

@kcudnik @JaiOCP @marian-pritsak @rlhui any continued thoughts on how we might handle this would be appreciated.

good approach seems to me the one that i proposed with bulk operation:

i think there could be a workaround for this issue, we could use bulk set api, and specify same object twice each time with speed and fec combination, internally sai could pick that up and allow change, in other words bulk set could be used like set with multiple attributes or as transaction set

at some angle it would be similar to the transaction you are proposing?

@mikeberesford
Copy link
Contributor Author

Perhaps I'm missing something, but I'm not seeing a bulk set API defined for port attributes in the SAI interface. If we do have something like that available, I'd be inclined to agree that it's semantically similar to an explicit "transaction" attribute, possibly without some of the state keeping required at the vendor SAI layer.

@kcudnik
Copy link
Collaborator

kcudnik commented Oct 19, 2021

yes, bulk is not defined yet, but there is no problem to add bulk api support there as 4 function pointers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kcudnik @mikeberesford