p4lang / p4c

P4_16 reference compiler
https://p4.org/
Apache License 2.0
670 stars 441 forks source link

Key masks (or expressions) with name annotations and p4 info generation #4020

Open amreshk opened 1 year ago

amreshk commented 1 year ago

Consider the following example for a key on a table,

    key = {
      fieldA  & 0x123   : exact @name("blah");         
    }

P4RuntimeSerializer currently will set the name annotation as the match key field name and save the KeyElement. https://github.com/p4lang/p4c/blob/71a06fee49d2237afc28b8f74c96b1d390e93c12/control-plane/p4RuntimeSerializer.cpp#L387-L402

However, during p4info generation this gets discarded and the masking information is lost. https://github.com/p4lang/p4c/blob/71a06fee49d2237afc28b8f74c96b1d390e93c12/control-plane/p4RuntimeArchHandler.h#L235-L266

With masks (or slices or any key expressions) would it make sense to keep the original key string around as name annotation? As a general question are name annotations not expected to be used in these scenarios?

fruffy commented 1 year ago

@antoninbas Do you perhaps recall why this was done this way?

antoninbas commented 1 year ago

It would have been nice to have the relevant snippet of the generated P4Info, but I expect that it should be:

  match {
    id: <X>
    name: "blah"
    bitwidth: <depends on the type of the P4 expression>
    match_type: EXACT
  }

The name should be "blah" as per the @name annotation. The @name annotation is then discarded as the value is stored in the name Protobuf field.

If the question is about storing fieldA & 0x123 somewhere in the P4Info, this information is not really relevant to P4Info / P4Runtime. The match expression can be anything in theory, e.g. fieldA + fieldB + 9. While we could store the serialized P4 expression as a new synthetic annotation, it should not have much value for control plane applications.

pkaminsk commented 1 year ago

I believe it is important to be able to avoid duplicate keys and be able to validate key values on set/get. That is impossible without knowing the expression used in p4 source.

antoninbas commented 1 year ago

I believe it is important to be able to avoid duplicate keys and be able to validate key values on set/get. That is impossible without knowing the expression used in p4 source.

duplicate keys: the name come from the annotation, different annotations should be provided; I don't recall what is the default behavior when the @name annotation is omitted. Regardless this seems orthogonal to the original issue.

validation: what constitute a valid value should be dictated by the P4 type. This information is not lost when generating the P4Info. If you mean "check that only bits that are not masked off by the mask expression are set in the value provided by the control plane", then it's a bit of a separate issue IMO. This is a very specific case (not something that applies to arbitrary expressions), and would need to be achieved through a separate annotation. I wonder if https://github.com/p4lang/p4-constraints has something for this case.