Redefines ActionProfiles to include the manner in which `size` and `max_group_size` should be calculated.

[jonathan-dilorenzo]

This is a new proposal for how to represent the max_num_members field that we recently introduced. As I was trying to use it, I figured out that the semantics of max_group_size was non-sensical in conjunction with max_num_members.

It became clear that some sort of flag determining whether size and max_group_size should talk about total weight or total members is necessary.

Here I went with a slightly more generalized proposal wherein one might specify different methods of calculating the size of a group through a oneof message.

I thought I'd just write it up real quick and get feedback on something concrete rather than go through the rounds.

Also, @antoninbas, would you run the proto updating script again? Sorry about that.

[antoninbas]

@jonathan-dilorenzo let's discuss this on Friday at the WG meeting I can generate the Proto files for you once we get consensus

[jonathan-dilorenzo]

Sounds great! Thanks Antonin. I just wanted to have a concrete proposal before the meeting.

[smolkaj]

@antoninbas 's feedback:

• Use single annotation with two arguments.
• See if we can standardize those annotations in PSA spec.

[jonathan-dilorenzo]

What do we think about selector_group_size_computation vs selector_size_semantics or something like this?

[antoninbas]

It almost feels like @max_group_size, along with the new annotation should be standard PSA properties of the ActionSelection extern type. The rationale bing that size is a standard property, and that the semantics of size already depend on the "semantics mode" chosen (sum of weights vs distinct members). I'm tagging @jafingerhut here, even though he may be missing come context.

[jonathan-dilorenzo]

Yeah, I definitely agree @antoninbas.

1. I finally fixed the annotation.
2. I also changed to selector_size_semantics because I liked it better.
3. I added extra descriptions in other places where we use max_group_size.
4. I maybe managed to compile the proto? Still getting some errors, but there are a few new files, so we'll see.

Please take another look and let me know if you have other suggestions!

[jonathan-dilorenzo]

Gentle reminder to take a look at this when you get a chance. Once accepted, I'll need to make some updates to the P4RT compiler to properly interpret the new annotations.

[jonathan-dilorenzo]

I get an error when I try to run codegen: error checking context: 'readdirent: input/output error'.

Could I ask you to rerun it @antoninbas?

[antoninbas]

@jonathan-dilorenzo I ran codegen and update the branch. The diff was very small compared to what you had:

$ git diff
diff --git a/go/p4/config/v1/p4info.pb.go b/go/p4/config/v1/p4info.pb.go
index 43064ee..c9f6707 100644
--- a/go/p4/config/v1/p4info.pb.go
+++ b/go/p4/config/v1/p4info.pb.go
@@ -288,7 +288,6 @@ func (ActionRef_Scope) EnumDescriptor() ([]byte, []int) {
        return file_p4_config_v1_p4info_proto_rawDescGZIP(), []int{9, 0}
 }

-// specifies the semantics of `size` and `max_group_size` above
 type ActionProfile_SelectorSizeSemantics int32

 const (
@@ -1503,7 +1502,8 @@ type ActionProfile struct {
        Size int64 `protobuf:"varint,4,opt,name=size,proto3" json:"size,omitempty"`
        // 0 if the action profile does not have a selector. Otherwise, semantics as
        // specified by `selector_size_semantics` below.
-       MaxGroupSize          int32                               `protobuf:"varint,5,opt,name=max_group_size,json=maxGroupSize,proto3" json:"max_group_size,omitempty"`
+       MaxGroupSize int32 `protobuf:"varint,5,opt,name=max_group_size,json=maxGroupSize,proto3" json:"max_group_size,omitempty"`
+       // specifies the semantics of `size` and `max_group_size` above
        SelectorSizeSemantics ActionProfile_SelectorSizeSemantics `protobuf:"varint,6,opt,name=selector_size_semantics,json=selectorSizeSemantics,proto3,enum=p4.config.v1.ActionProfile_SelectorSizeSemantics" json:"selector_size_semantics,omitempty"`
 }

[jonathan-dilorenzo]

Are we planning to add a sentence about the behavior if a target / P4Runtime implementation doesn't support one semantics mode?

Ooh. Good question. I guess I was imagining that being covered by statements like Returns an INVALID_ARGUMENT error if ... the provided config cannot be realized. in SetForwardingPipelineConfig. There are a bunch of ways in which a provided config can't be realized, but I'm not sure it's worth calling this one out specifically. What do you think?

[antoninbas]

There are a bunch of ways in which a provided config can't be realized, but I'm not sure it's worth calling this one out specifically. What do you think?

True, but most of the time I would expect it to be because of resource constraints, not necessarily because of missing functionality. In this case, I think it's more about missing functionality. However, maybe rejecting the config because it cannot be realized is just "common sense", and it doesn't need to be called out explicitly. We can merge as is and see if a question ever comes up :)

[jonathan-dilorenzo]

Ah, these are good points. I guess I think of it as common sense, but it's true that an easy mistake to make (without mentioning this explicitly) would be to just ignore this field, given that it hasn't existed until now, and assume that this remained a correct implementation of the specification.