Try to support wildcard PacketReplicationEngine reads.

[fruffy]

Fixes #609.

[antoninbas]

I am not sure #609 is a bug according to the spec.

A request may omit or use default values for parts of the entity key to achieve wildcard behavior. Please refer to the P4 Entity Messages section for details on what parts of the entity can be wildcarded in a given request.

It is mostly up to individual entities to define what a wildcard read means. For the PacketReplicationEngineEntry entity, wildcard reads separate multicast and clone entries.

One issue with that change is that if we ever add a new PRE entry type (by adding a new field to the oneof), it will not be possible to distinguish between an unset oneof (which after this change would mean wildcard read of all PRE entries regardless of type) and a oneof set to a field which is not (yet) recognized by the server. IMO, this breaks compatibility (between new client and old server).

We actually discuss this issue with oneof in this very section (https://p4.org/p4-spec/docs/p4runtime-spec-working-draft-html-version.html#sec-wildcard-reads), and we link to https://protobuf.dev/programming-guides/proto3/#backward

[smolkaj]

Very interesting. I wasn't aware of this subtle backward-compatibility issue, here and in general for protos.

[smolkaj]

@jonathan-dilorenzo and I discussed this an agree that NOT_SET is an absurd name for an enum value that encodes "unset" or "set but to an unknown oneof case". Maybe proto4 will get this right....

[jonathan-dilorenzo]

I don't think that's a particularly reasonable read of the specification @antoninbas, though this suggests we should clarify it. The main wildcard read section pretty clearly suggests that any empty top-level entity should read all entities of that type with this example:

device_id: <ID>
entities {
  extern_entry { }  # read all extern instances for all supported extern types
  table_entry { }  # read all table entries for all tables
  action_profile_member { }  # read all members for all action profiles
  action_profile_group { }  # read all groups for all action profiles
  ...
}

And the TableEntry section on Wildcard reads only mentions a possibility that provides way more fields:

device_id: 3
entities {
  table_entry {
    table_id: 0
    priority: 0
    controller_metadata: 0
    metadata: ""
  }
}

So it would be reasonable to expect that the full wildcard case (where you specify almost nothing) was similarly left out of the PacketReplicationEngineEntry section.

That said, I agree that this is actually an unacceptable interpretation based on the backwards compatibility issue. Which either would mean that the server should return everything if it doesn't know to return something specific, or (as we did with entities) that any oneof must always be set (if it's at the next level of an entity you want to read I guess).

[jonathan-dilorenzo]

Opened up an issue against the P4Runtime to discuss this: p4lang/p4runtime#476 and fix the specification in some direction before submitting this PR.