Generating P4runtime for PSA meter example

[amreshk]

While trying to generate api for a meter test, i see the following error,

./p4test ../testdata/p4_16_samples/psa-meter1.p4 --p4runtime-files psa-meter1.proto
../testdata/p4_16_samples/psa-meter1.p4(48): [--Werror=legacy] error: Action parameter color has a type which is not bit<>, int<>, bool, type or serializable enum
    action execute(bit<12> index, PSA_MeterColor_t color) {
                                                   ^^^^^

In psa.p4, Meter color is an enum

648 // BEGIN:MeterColor_defn
649 enum PSA_MeterColor_t { RED, GREEN, YELLOW }
650 // END:MeterColor_defn

I am not sure if the issue has been already addressed, but the runtime code expects meter color to be of type Type_SerEnum (to infer width)

https://github.com/p4lang/p4c/blob/c7229ccfe36484ad68311fe7e0acca3409ec005c/control-plane/p4RuntimeSerializer.cpp#L965

FYI : @antoninbas , @jafingerhut

[jafingerhut]

Thanks for pointing this out.

I personally think that it would be a good idea for the PSA definition to make all things currently declared as enum SomeName ... to be serializable enums instead, i.e. to become enum bit<X> SomeName ... for some appropriately chosen constant X.

A benefit of this, other than the issue you point out, is that all such values immediately become serializable, i.e. can be copied into our out of headers, which can be a useful property to have, on occasion.

[hesingh]

I wonder how is v1model.p4 and bmv2 backend code working in p4c because even v1model.p4 has enums and not serEnum?

[jafingerhut]

@hesingh The only enum's in v1model.p4 are for values passed as compile-time constructor parameters when calling extern object constructor methods. Those "do not exist at run time", only at compile time. I believe that is the difference.

[jafingerhut]

Minor addendum to my previous comment: You could take the enum types defined in v1model.p4 and create run-time variables with those types, but I don't think there is any reason for anyone to want to do that.

In the case described in the original comment, it sounds like @amreshk is trying NOT to create a new PSA meter extern, or if they are, they are trying to reuse as much code as possible from the v1model meter implementation, and that v1model implementation expects bit<W> for W >= 2 for the second parameter.

[hanw]

A packet can be pre-colored. I believe this feature is used to fetch the pre-color from action data and use the pre-color in meter execute() call.

action execute(bit<12> index, PSA_MeterColor_t color) {
     meter.execute(index, color);
}

Perhaps we need to define another control plane type, similar to the ones we have defined, e.g.:

typedef bit<8>  ClassOfServiceInHeaderUint_t;
@p4runtime_translation("p4.org/psa/v1/ClassOfServiceInHeader_t", 8)
type  ClassOfServiceInHeaderUint_t ClassOfServiceInHeader_t;

[hanw]

Or, change PSA_MeterColor_t to serializable enum.

[jafingerhut]

I have created this issue proposing a change to the PSA specification (and/or P4Runtime API spec) in an attempt to resolve this issue: https://github.com/p4lang/p4-spec/issues/941