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

Consider enabling support in bmv2 backend for default_action in tables with action profiles or action selectors #5101

Open
jafingerhut opened this issue Jan 15, 2025 · 4 comments
Labels
bmv2 Topics related to BMv2 or v1model enhancement This topic discusses an improvement to existing compiler code.

Comments

@jafingerhut
Copy link
Contributor

jafingerhut commented Jan 15, 2025

Since 2016, the p4c bmv2 backend has issued a warning if a table has a default_action property, and an implementation of an action profile or action selector. Then it produces a BMv2 JSON file that has no mention of the default_action at all. I believe this was done because at that time, BMv2 did not correctly implement those cases.

Since 2018, BMv2 should implement this correctly.

Links to commits/PRs with the 2016/2018 changes mentioned above can be found in the discussion thread following this comment: https://github.com/p4lang/behavioral-model/pull/1287/files#r1913623565

This issue is to track a possible enhancement to the p4c bmv2 backend that:
(a) no longer issues a warning for tables with a default_action property, if they have implementation that is action_profile or action_selector.
(b) for such tables, the BMv2 JSON data written for that table includes a description of the default_action of the table.

Of course, such a change should not be merged into p4c until after we have tested and verified that we believe BMv2 is working as expected for such tables.

@fruffy fruffy added enhancement This topic discusses an improvement to existing compiler code. bmv2 Topics related to BMv2 or v1model labels Jan 15, 2025
@Vineet1101
Copy link
Contributor

@jafingerhut I guess it can be solved. First we can remove warnings from backends/bmv2 files and then we can update the json generation. One thing that I would like to test is that does bmv2 handles the warning correctly

@jafingerhut
Copy link
Contributor Author

@jafingerhut I guess it can be solved. First we can remove warnings from backends/bmv2 files and then we can update the json generation. One thing that I would like to test is that does bmv2 handles the warning correctly

It is definitely possible. I did some further investigation on the details and added my findings to this comment on a related issue in the behavioral-model repo: p4lang/behavioral-model#1290 (comment)

As far as I can tell, enabling this would require changes in all of these repositories, if you want the P4Runtime API to be able to configure the default_action of such tables at runtime:

  • p4c
  • behavioral-model
  • p4runtime - the P4Runtime API specification, which currently says explicitly that it does not support configuring the default action on such tables
  • probably also PI - the implementation of the P4Runtime server used by behavioral-model

When I saw that the work required was about 4 times more than I originally expected, I became less interested in pursuing such changes any time soon, personally. If someone else wants to take up that work, I certainly do not mind. Just go into it eyes open knowing that it is a much bigger task than I originally guessed.

@Vineet1101
Copy link
Contributor

@jafingerhut what would be the impact of this change. If it has significant impact I would like to work on it

@jafingerhut
Copy link
Contributor Author

It is difficult for me to judge if the impact is "significant". It is a nice small generalization to what you can do now. People have obviously been living with that restriction for 8-9 years now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bmv2 Topics related to BMv2 or v1model enhancement This topic discusses an improvement to existing compiler code.
Projects
None yet
Development

No branches or pull requests

3 participants