Skip to content

Change PER_GROUP_ONLY to GROUP_ACTION and update P4Runtime spec#604

Open
matthewtlam wants to merge 2 commits intop4lang:mainfrom
matthewtlam:matthewtlam
Open

Change PER_GROUP_ONLY to GROUP_ACTION and update P4Runtime spec#604
matthewtlam wants to merge 2 commits intop4lang:mainfrom
matthewtlam:matthewtlam

Conversation

@matthewtlam
Copy link
Copy Markdown
Contributor

@matthewtlam matthewtlam commented Mar 24, 2026

This name change from PER_GROUP_ONLY to ACTION_GROUP is to avoid the confusion regarding the expected semantics of the @groupaction annotation. The @tableonly and @defaultonly are defined in a way that if an action has no annotation, that means that can be used for both table and default).

But our semantics of @groupaction (previously @pergrouponly) is inconsistent with that, in the sense that if the annotation is present on an action, it can be used as a group action (and nothing else) and if the annotation is absent, the action cannot be used as a group action.

This was also changed to maintain consistency across annotation and scope

Signed-off-by: Matthew Lam <matthew.lam.qwerty@gmail.com>
@kheradmandG
Copy link
Copy Markdown

kheradmandG commented Mar 31, 2026

@matthewtlam could you update the description on why we are proposing this change?

This is to avoid the confusion regarding the expected semantics of the @groupaction annotation.
The @tableonly and @defaultonly annotations are defined in a way that if an action has no annotation, that means the action can be used for both table and default.
But our semantics of @groupaction (previously @pergrouponly) is inconsistent with that, in the sense that if the annotation is present on an action, it can be used as a group action (and nothing else) and if the annotation is absent, the action cannot be used as a group action.
The drop of "only" postfix in @groupaction is meant to indicate that there is a difference between how @groupaction annotation vs. @tableonly and @defaultonly annotations work.

Also it seems there is consensus on @groupaction / GROUP_ACTION. Could you please update the PR.

@kheradmandG
Copy link
Copy Markdown

kheradmandG commented Mar 31, 2026

@jafingerhut @jonathan-dilorenzo @smolkaj if everyone is happy with the idea, let's get this in soon before we have developments with the older annotation.

@jafingerhut
Copy link
Copy Markdown
Contributor

@jafingerhut @jonathan-dilorenzo @smolkaj if everyone happy with the idea, let's get this in soon before we have developments with the older annotation.

I'll let others speak for themselves, but for myself, I am happy to approve if we make the suggested change of annotation called @groupaction and enum value name changed to GROUP_ACTION.

Signed-off-by: Matthew Lam <matthew.lam.qwerty@gmail.com>
@matthewtlam
Copy link
Copy Markdown
Contributor Author

@jafingerhut @chrispsommers @smolkaj @jonathan-dilorenzo @kheradmandG

I updated it so everything is consistent with each other. Please take a look

@matthewtlam matthewtlam changed the title Change PER_GROUP_ONLY to PER_GROUP and update P4Runtime spec Change PER_GROUP_ONLY to GROUP_ACTION and update P4Runtime spec Mar 31, 2026
Copy link
Copy Markdown
Contributor

@jonathan-dilorenzo jonathan-dilorenzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Copy Markdown
Contributor

@jafingerhut jafingerhut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants