p4lang / p4runtime

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

Does P4Runtime support struct action parameters? #405

Open qobilidop opened 1 year ago

qobilidop commented 1 year ago

This is a follow-up on https://github.com/p4lang/p4c/issues/3760#issuecomment-1340213608 (@jfingerh). I have taken a closer look at the P4Runtime protos, and would like to continue the P4Runtime side of discussion here. I have a feeling struct action parameters are already expressible in P4Runtime, but needs confirmation or correction from P4Runtime experts.

On the P4Info side, there is a type_name for each Param: https://github.com/p4lang/p4runtime/blob/e70ee7b94a9b6f90fd6d17ce6bc941d35892ce54/proto/p4/config/v1/p4info.proto#L243-L260

The actual type spec could be looked up in P4TypeInfo: https://github.com/p4lang/p4runtime/blob/e70ee7b94a9b6f90fd6d17ce6bc941d35892ce54/proto/p4/config/v1/p4types.proto#L55-L63 https://github.com/p4lang/p4runtime/blob/e70ee7b94a9b6f90fd6d17ce6bc941d35892ce54/proto/p4/config/v1/p4types.proto#L83-L85

So in principle, the P4Info file could express a struct action parameter, right?

On the P4Runtime service side, action parameters are specified as flat bytes: https://github.com/p4lang/p4runtime/blob/e70ee7b94a9b6f90fd6d17ce6bc941d35892ce54/proto/p4/v1/p4runtime.proto#L266-L273

A user-defined struct could easily be flattened as bytes. So I think there is no issue either?

jfingerh commented 1 year ago

For another point of reference, please see section 21 "Known Limitations of Current P4Runtime Version" https://p4.org/p4-spec/docs/p4runtime-spec-working-draft-html-version.html#sec-known-limitations-of-current-p4runtime-version of the latest P4Runtime specification document, where it says:

FieldMatch, action Param, and controller packet metadata fields only support unsigned bitstrings, i.e. values of one of the following types (not the more general P4Data):

bit<W>
bool. Note that as far as the P4Info message contents and thus controller software is concerned, such fields of type bool will be indistinguishable from those that have been declared with type bit<1>. P4Runtime server software will automatically perform any conversion needed between the type bit<1> values in P4Runtime messages and the data plane representation.
an enum with underlying type bit<W>
a type or typedef with an underlying type that is one of the above (or in general a “chain” of type and/or typedef that eventually ends with one of the types above)

If that part of the spec is obsolete, it should be removed.

qobilidop commented 1 year ago

I see. I didn't notice that part of the spec previously!

Then my understanding is that syntactically, any type_name could be used for an action param. But semantically, there are further restrictions. Currently type_name is there only to support type/typedefs that could be reduced to the basic types mentioned above.

jfingerh commented 1 year ago

That is also my understanding, yes. Also, even if the Protobuf definitions might not require any changes to become more general in types of action parameters that are supported, P4Runtime client and server implementations certainly would need updating to support more types, as I suspect that most or all of them have been developed with that type restriction in mind.

jafingerhut commented 1 year ago

@qobilidop Note that the work on GenericTable in the API work group the last couple of months is intended to enable generalizing key and action parameter types to structs, and many other more general things. That work is still in progress, not yet implemented in open source as of 2023-Aug, etc. but you may want to keep track of it at least occasionally.