SAI icon indicating copy to clipboard operation
SAI copied to clipboard

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

Open mikeberesford opened this issue 4 years ago • 5 comments

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 https://github.com/opencomputeproject/SAI/pull/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 avatar Oct 05 '21 16:10 mikeberesford

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

mikeberesford avatar Oct 05 '21 16:10 mikeberesford

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)

mikeberesford avatar Oct 14 '21 16:10 mikeberesford

@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?

kcudnik avatar Oct 18 '21 14:10 kcudnik

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.

mikeberesford avatar Oct 19 '21 05:10 mikeberesford

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

kcudnik avatar Oct 19 '21 11:10 kcudnik