p4lang / p4runtime

Specification documents for the P4Runtime control-plane API
Apache License 2.0
140 stars 86 forks source link

Support for P4_16 enum type changes introduced in Version 1.2.2 of P4_16 language spec #430

Open jafingerhut opened 1 year ago

jafingerhut commented 1 year ago

This issue should be closed exactly when the bullet item “Added support for additional enumeration types” in Section 1.1 "P4 Language Version Applicability" is addressed and removed.

This issue was created to address one of three changes in Version 1.2.2 of the P4_16 language specification, in particular the one summarized with the following line here: https://p4.org/p4-spec/docs/P4-16-v-1.2.3.html#sec-summary-of-changes-made-in-version-122-released-may-17-2021

Note: I have not yet investigated in detail what changes were made from version 1.2.1 to 1.2.2 of the language specification that the line above refers to. It seems to me that the first thing to do in resolving this issue is finding out and describing in a follow-up comment exactly what those changes were.

jafingerhut commented 1 year ago

I believe that all of the changes to the Madoko source of the P4_16 language spec related to this change are in this one commit: https://github.com/p4lang/p4-spec/commit/f2e815c6ea18c59ef1469de7f7f7b9a8526fec51

Before this change, a serializable enum could only have a representation type of bit<W> for some positive integer W.

After this change, the following representation types are allowed:

I have no recollection of typedef types ever being explicitly represented anywhere in the P4Runtime API specification, so the last bullet item above appears to require no changes in the P4Runtime API spec.

The fact that a serializable enum can have a representation type of int<W> in addition to the former bit<W> seems like it might be important to represent in the P4Runtime API specification somehow.

For example, the P4Runtime message P4SerializableEnumTypeSpec is defined here: https://github.com/p4lang/p4runtime/blob/main/proto/p4/config/v1/p4types.proto#L237-L254 It contains a field named underlying_type with type P4BitTypeSpec, but it cannot also be P4IntTypeSpec, so it appears that the restriction of the underlying type to bit<W> is currently codified in parts of the P4Runtime specification.

There may be other places not mentioned above where this restriction is codified in the P4Runtime specification. I have not attempted to be exhaustive in looking for all possible places at this time.

smolkaj commented 1 year ago

I think int<W> is generally unsupported in P4Runtime v1 currently:

In the P4Runtime API version 1.0 (including minor version revisions), values of table key fields, action parameters, and fields in packet-in and packet-out headers between a device and the controller (see 6.4.6), may not be of type int. The rules for encoding signed values thus only apply to messages of type P4Data (see 8.4.3).