Support strings in @p4runtime_translation

[ghost]

Referring to objects by a string name instead of an integer id makes P4RT flows a lot more readable, which for example greatly helps with debugging. We therefore propose to generalize the @p4runtime_translation annotation, to not just support fixed bit width integers, but also strings.

So in addition to:

@p4runtime_translation("p4.org/psa/v1/PortId_t", 32)

We propose to also support:

@p4runtime_translation("p4.org/psa/v1/PortId_t", string)

Since values are already represented as bytes in P4RT messages, there is no need for any changes of those protos.

However, we do need to change the P4Info definition. Currently, match_fields and action params refer to translated values as follows:

  match_fields {
    id: 1
    name: "ingress_port"
    bitwidth: 32
    match_type: EXACT
    type_name {
      name: "PortId_t"
    }
  }

params {
    id: 1
    name: "ingress_port"
    bitwidth: 32
    type_name {
      name: "PortId_t"
    }
  }

new_types {
    key: "PortId_t"
    value {
      translated_type {
        uri: "p4.org/psa/v1/PortId_t"
        sdn_bitwidth: 32
      }
    }
  }

To support strings, we propose extending the new_types proto in https://github.com/p4lang/p4runtime/blob/master/proto/p4/config/v1/p4types.proto as follows:

message P4NewTypeTranslation {
  message SdnString {}

  // The URI uniquely identifies the translation in order to enable the
  // P4Runtime agent to perform value-mapping appropriately when required. It is
  // recommended that the URI includes at least the P4 architecture name and the
  // type name.
  string uri = 1;
  // The object is either represented as an unsigned integer with a bitwidth of `sdn_bitwidth`,
  // or as a string.
  int32 sdn_bitwidth = 2;
  SdnString sdn_string = 3;
}

So in the above example, the P4Info would be:

new_types {
    key: "PortId_t"
    value {
      translated_type {
        uri: "p4.org/psa/v1/PortId_t"
        sdn_string: {}
      }
    }
  }

When translating from a string, the bitwidth field in match_fields and action params is ignored.

[ghost]

jafingerhut@ antoninbas@ stefanheule@ please take a look

[antoninbas]

I think you mean @jafingerhut @antoninbas @stefanheule :)

[stefanheule]

Should the snd_string and sdn_bitwidth be a oneof (at least conceptually, maybe not for back-wards compatibility reasons; though we should still document it)?

[antoninbas]

It will probably need to be "string" since this annotation expects a valid expression (at least as it is currently implemented in p4c).

I agree with @stefanheule that we should use a oneof, since in this specific case it is actually backwards-compatible (https://developers.google.com/protocol-buffers/docs/proto3#backwards-compatibility-issues):

Move fields into or out of a oneof: You may lose some of your information (some fields will be cleared) after the message is serialized and parsed. However, you can safely move a single field into a new oneof and may be able to move multiple fields if it is known that only one is ever set.

[jafingerhut]

Using "string" instead of string in the P4_16 source code does seem likely to make it a bit easier to implement in p4c.

oneof also seems like a good idea, assuming that is a backwards compatible change (I do not know Protobuf rules well enough to say).

[jafingerhut]

I have been trying to get a v1model P4_16 program using @p4runtime_translation to generate a non-empty type_info message. It is definitely possible, because this p4c test program has one, but because of struct types and enum types, not because of a type definition:

• https://github.com/p4lang/p4c/blob/master/testdata/p4_16_samples/v1model-digest-containing-ser-enum.p4
• https://github.com/p4lang/p4c/blob/master/testdata/p4_16_samples_outputs/v1model-digest-containing-ser-enum.p4.p4info.txt

I have not been able to find a way to get p4c to generate a non-empty type type_info message for a program that uses a type definition like this one, though, using a field of that type as a table search key field:

@p4runtime_translation("com.fingerhutpress/andysp4arch/v1/IPv4Addr_t", 32)
type bit<32>         IPv4Addr_t;

Does anyone else know how? Or perhaps the p4c P4Info writing code does not handle this case yet?

Here is a small public test program you can experiment with, if it helps:

• https://github.com/jafingerhut/p4-guide/blob/master/p4runtime-translation/weird-case1.p4

[jafingerhut]

Submitted an issue for p4c crash for one test program I tried with a custom type definition that should affect P4Info file contents, here: https://github.com/p4lang/p4c/issues/2234

Not relevant for how to change P4Info message formats, but for getting p4c to produce correct P4Info files for things already defined, before this issue was created.

[antoninbas]

Took a look at the compiler implementation today, and it looks to me like we have 2 reasonable choices, as discussed in previous comments:

1. leverage the compiler support for parsing unstructured annotations by sticking with the current definition of @p4runtime_translation in the compiler: https://github.com/p4lang/p4c/blob/master/control-plane/p4RuntimeSerializer.cpp#L723. The annotation expects 2 valid P4 expressions: the first one is later converted to a string literal, and the second one to an integer literal. We can be more "liberal" for the second one, and start accepting string literals which encode a supported type ("bit<W>", "int<W>", "string").
2. disable compiler support for @p4runtime_translation and "parse" the annotation tokens ourselves. In practice, that just means that the compiler will pass the annotations to us as "raw" strings, and we have to validate their contents, which for this specific case should not be too much trouble.

Both solutions would be backwards-compatible.

Solution 2) would enable us to write @p4runtime_translation("p4.org/psa/v1/PortId_t", string) and @p4runtime_translation("p4.org/psa/v1/PortId_t", bit<32>) instead of @p4runtime_translation("p4.org/psa/v1/PortId_t", "string") and @p4runtime_translation("p4.org/psa/v1/PortId_t", "bit<32>"), at the price of marginally more work.

@konne88 @stefanheule I would like to know what the "optimal" solution would be for you (doesn't have to be one of these 2)? It doesn't seem likely that we can extend the annotation-parsing infrastructure in the compiler in such a way that it can validate for us that the second argument is a valid P4 type. That looks like a lot of work, just for our specific case. I myself am not familiar at all with that part of the p4c code base. But even if we could have that, is it really the best solution for our use-case? The second argument is not really meant to be a P4 type. It is purely a control-plane type and it never shows up in the P4 datapath. This type information is used purely to transmit information over P4Runtime.

I know that we have been talking about disabling translation for bmv2 to get useful human-readable logs, but even that is likely to require changes to bmv2's logging architecture. And bmv2 only understands binary strings. IMO, the "right" way to achieve the same thing is by using some kind of identity translation with a large underlying type:

@p4runtime_translation("p4.org/psa-extensions/v1/GroupId_t", "string")
type bit<1024> GroupId_t;

[stefanheule]

I think using @p4runtime_translation("p4.org/psa/v1/PortId_t", string) and @p4runtime_translation("p4.org/psa/v1/PortId_t", bit<32>) (and @p4runtime_translation("p4.org/psa/v1/PortId_t", 32) which is deprecated but supported for backwards compatibility) is nicest.

It's a good observation though that the argument isn't a P4 type necessarily, but a controlplane type (the two type systems aren't necessarily the same), but it also seems like using string and bit<w> is convenient for us.

I'm not sure what you mean by the right way to do this in bmv2. I agree that your example of using 1024 bits will probably just work, but I imagine the "right" way to do this is to support the translations in bmv2 and have an option to instead just use the controlplane type in bmv2 (i.e., support strings in bmv2).

[antoninbas]

@stefanheule Basically there has to be an underlying bit<W> type for it to be a correct program. P4 does not support string manipulations in the datapath and you cannot have variables (e.g. metadata fields) with type string and match on them in tables. Given this, bmv2 is still going to be using the underlying type, not the control plane type (which is not a P4 type per se). I don't think it is realistic, or even a good idea, to add complexity to bmv2 to handle these annotated types completely differently. Today bmv2 handles all non-compound types as bitstrings (or rather a combination of a bitsring + an arbitrary-precision integer), and does not know anything about user-defined types. However, it will probably be possible to improve logging for match fields & action parameters with an associated string control-plane type: instead of logging the values as hexadecimal strings, we can probably log them as human-readable strings if we include some additional information in the bmv2 JSON.