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

Support a string "cookie" in TableEntry #267

Closed
antoninbas opened this issue Feb 26, 2020 · 6 comments · Fixed by #271
Closed

Support a string "cookie" in TableEntry #267

antoninbas opened this issue Feb 26, 2020 · 6 comments · Fixed by #271
Assignees

Comments

@antoninbas
Copy link
Member

The current "controller_metadata" has type uint64: https://github.com/p4lang/p4runtime/blob/v1.1.0/proto/p4/v1/p4runtime.proto#L160. This is probably legacy from OpenFlow, where each flow can have a 64-bit integer cookie. We would like to be able to store an arbitrary string as metadata with each table entry.

@antoninbas
Copy link
Member Author

The decision at the 02/26/2020 P4 API WG was to add a new "metadata" field to the TableEntry message, with type string. The old "controller_metadata" field will not be removed to preserve backward-compatibility but will be marked as deprecated. All the use cases covered with a uint64 cookie should be covered with a string cookie. If someone has a better name in mind than just "metadata", please make a suggestion.

@antoninbas
Copy link
Member Author

@konne-google should the exact Protobuf type for this type be bytes (arbitrary characters) or string (only UTF-8 characters)?

@antoninbas antoninbas added this to the P4Runtime v1.2.0 milestone Feb 26, 2020
@stefanheule
Copy link
Contributor

It should be bytes, so arbitrary data can be stored there easily (including the 64bit cookie from before, if someone just needs that after we remove the now deprecated field).

@stefanheule
Copy link
Contributor

The ForwardingPipelineConfig message has a 64bit cookie field, too. Do we want to deprecate that in favor of a bytes metadata field as well? Or only for TableEntries? @konne-google @antoninbas, and anyone else who has an opinion?

@stefanheule
Copy link
Contributor

Pull request #271 addresses the TableEntry case for now.

@ghost
Copy link

ghost commented Mar 12, 2020

I don't feel strongly about the ForwardingPipelineConfig either way. We don't need it, but it might be nice for consistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants