-
Notifications
You must be signed in to change notification settings - Fork 0
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
Iterate on the payload generation for gSII. #6
base: main
Are you sure you want to change the base?
Conversation
* (A) proto/obj/interfaces: - Check in a suggested mechanism for generating a payload in gSII from the YANG schema.
* (A) .gitignore - Ensure that deps are not checked into the repository. * (D) v1/proto/obj/... - Remove object hierarchy from generated protobufs. * (A) v1/yang/interfaces - Create a generation approach for the openconfig-interfaces module demonstrating a payload that is generated from YANG.
* (A) scripts/generate_protos.sh - Generate protobufs for gSII payload. * (A) **/*.pb.go - Check in generated protobufs.
* (M) v1/yang/..., v1/proto/... - Use a ygot branch that recognises `volatile` as a reserved keyword to generate protobufs for interfaces and qos schemas.
* (A) go.{mod,sum} - Initialise Go module.
string name = 1 [(yext.schemapath) = "/interfaces/interface/config/name|/interfaces/interface/name"]; | ||
Interface interface = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(prefacing this by saying i dont know much about yang-protobuf generation)
is it intended that some field numbers increase monotonically while others are generated as the FNV hash of the schemapath (via https://github.com/openconfig/ygot/blob/100bd44fc21215ffcc4862f37d90f7c257b5fa2b/protogen/protogen.go#L1058)?
This seems to affect the fields in messages which are declared inside other messages, and differs from the way that protobuf generation is done for something like hercules (https://github.com/openconfig/hercules/blob/ca3575e85500fa089dfe0b8cd3ea71943267102e/proto/openconfig/openconfig_interfaces/openconfig_interfaces.proto#L37)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is intended. YANG lists are generated as repeated <message>
fields. Where <message>
is a generated message of the form:
message FooKey {
string key_one = 1;
string key_two = 2;
...
string key_fourty = 40;
Foo foo = 41;
}
Where Foo
is the actual "member" of the list, and each of the monotonically increasing fields are the keys in the YANG list
's key
statement. Since these are stably ordered by the YANG schema itself we can save ourselves a few bytes and just give them a number.
For all other fields within a list
or container
, we don't have a stable numbering -- so we calculate the field ID using a fnv hash. We published something as to why this was reasonable here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, sounds good!
thanks for linking that doc
This change adds entries under |
Yes -- #1 covers this change within the |
ack - thanks for including that in #1. |
This PR makes an initial proposal for how the payload of gSII RPCs should look.
There are two cases that I think we need to consider:
For the OpenConfig case, ideally we would not generate new schemas, and be able to re-use existing schema components (and thus want to be able to write these components in YANG). This requires minor extension of the ygot toolchain to generate protobufs (see openconfig/ygot@master...volatile where this change is made). The protos that are included herein use this approach.
The alternative is just to have hand-crafted protos as payload -- but this seems like it would be error prone in some cases.
The PR contains two generated protos: