search

openconfig / featureprofiles

Feature Profiles are groups of OpenConfig paths and tests which verify their behavior
Apache License 2.0
51 stars 147 forks source link

TE-3.5 gRIBI forward reference entries deviation #265

Open liulk opened 2 years ago

liulk commented 2 years ago

@mingyangcisco pointed out in #261:

We are seeing different expectation regarding out of order when those are sent in one modify request vs multiple modify requests. gribigo compliance testcase is expecting forward reference to work when multiple modify requests are sent.

Currently the gRIBIgo compliance tests driver has skipSrvReorder default to true.

https://github.com/openconfig/featureprofiles/blob/main/feature/gribi/tests/gribigo_compliance_test/gribigo_compliance_test.go#L19

TE-3.5 should probably implement a similar deviation that does not require forward reference to fail.

ashpatcisco commented 2 years ago

@xw-g can we have clear expectations on the server behavior for AFTOperations where a GRIBI entry has forward reference to another GRIBI entry ?

The text here indicates that forward references are not required and should be NACKed https://github.com/openconfig/gribi/blob/spec/doc/specification.md#4132-forward-references

and given the direction that the client is expected to ensure ordering and correctness of the GRIBI entry, the server failing on forward references sounds the right thing to do.

If a deviation gets implemented here that allows servers that support forward references, what would be the default behavior of tests here - will they send operations in order ? use out of order only when its running against a server that is known to support out of order ?

xw-g commented 2 years ago

@ashpatcisco yes, your reference is correct. Client is expected to have the correct ordering in what they send. Solving forward reference is not required on the device. That's also why skipSrvReorder flag is set to true by default.

ppl might be confused that our gRIBI server reference implementation support re-ordering (I think this is why we have the skipSrvReorder flag). @robshakir, @nflath and I plan to remove the support to avoid confusion. That says, we are also thinking about to specifically call it out in the spec that we do expect forward reference to fail, for the purpose of an explicit API behavior.

@liulk given that, I think current TE-3.5 coverage probably is good.