Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow indirect tables to have default actions declared const in P4 #1290

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jafingerhut
Copy link
Contributor

@jafingerhut jafingerhut commented Jan 19, 2025

This PR adds a test program that exhibits a bug in p4c's predication pass, as mentioned in this issue: p4lang/p4c#4370

Note that now that p4lang/p4c#4999 has been merged in, the bmv2 backend (at least) supports arbitrary if statements without using the predication pass, so the relevance of this test case only applies to targets that continue to use the predication pass.

@jafingerhut
Copy link
Contributor Author

@antoninbas I have started a PR on p4c to enable the p4c bmv2 backend to create indirect tables with a default_entry key, here: p4lang/p4c#5104

In testing it, I found that if in the P4 source code the default_action property of an indirect table is declared const, then with the changes on the PR linked above, a JSON file is created where the action_const key of default_entry exists, and has the value true. For such a JSON file, BMv2 gives an error:

Table '<name>' does not have type 'simple' and therefore setting 'action_const' to true is meaningless

If an indirect table can support a non-const default_action, I don't see why it could not also support a const default_action -- just support the default_action and don't let the control plane change it later.

Do you have any recollection why this is checked for, and produces the above error?

@jafingerhut
Copy link
Contributor Author

In looking at some of the behaviral-model code, I am suspecting that perhaps its current indirect table implementations do not implement default_action behavior.

By default_action behavior, I mean:

  • do a lookup of the original key
  • if there is a match, then yes the profiles, or groups, are important for indirect tables
  • but if there is a miss, there should be a separate action with whatever action parameters it needs that can be configured independently of all profile actions, or action selector groups/members.

@jafingerhut
Copy link
Contributor Author

And in hacking a bit further, if we wanted p4c, behavioral-model, and the P4Runtime API to all support indirect tables with a source-code-configurable default_action, and a runtime configurable default_action, it seems to require changes in all of these:

  • p4c bmv2 backend
  • behavioral-model
  • P4Runtime specification - The section named "Default Entry" in the spec says: "In this P4Runtime release, we have decided to restrict the default entry for indirect tables — tables with an ActionProfile or ActionSelector implementation property — to a constant NoAction action entry, with the hope that it would simplify the implementation of the P4Runtime service."
  • probably PI implementation of P4Runtime server behavior

If it were just the first two that required small changes, I think I would be interested, but it is becoming a bit of "going down the rabbit hole" for questionable gain.

@jafingerhut jafingerhut marked this pull request as draft January 20, 2025 23:23
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.

1 participant