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

Handle const default actions correctly in P4-14 and P4Runtime serialization #326

Conversation

sethfowler
Copy link
Contributor

@sethfowler sethfowler commented Feb 23, 2017

In #309, we noticed two issues:

  • The P4Runtime output we're generating has way too many NoAction entries.
  • If a table has any default action, we serialize it as a const_default_action_id in P4Runtime, even if the default action isn't const.

These two things turn out to be related.

Explicit P4-14 default actions are all "const" from a P4-16 perspective. They're fixed at compile time, and the control plane can't change them. (The control plane may be able to change the associated action data, though, but that's a separate issue.)

The P4-16 non-const default action is more or less equivalent to not specifying a default action at all in P4-14. The only difference is that P4-16 allows you to set the initial state of the default action, while in P4-14, the initial state is always "take no action at all".

When we process a P4-14 program in the p4c frontend, tables without explicit default actions are assigned a default action of NoAction by the compiler. The problem is that the constness rules above aren't followed. We treat P4-14 default actions as if they were never const, when in fact they should always be const if they're explicitly specified.

In 39354ec, I've resolved that issue; with that patch applied, only NoAction default actions inserted by the compiler are non-const.

This dovetails nicely with 4a2c57e, which fixes p4runtimeSerializer so that it only produces a const_default_action_id for a table if the default action is actually const. That fixes a bug on its own, but with these two patches combined, we get rid of the flood of NoAction default actions in the P4Runtime output.

These two patches, taken together, address the most important remaining problem that I'm aware of related to issue #309.

@sethfowler sethfowler force-pushed the seth/handle-const-default-actions-correctly branch from 4a2c57e to 2d93dce Compare February 23, 2017 22:56
@sethfowler
Copy link
Contributor Author

There were some P4-14 to P4-16 conversion tests that needed to be updated to use const default_action; I pushed the fix. That should fix the test failures on Travis.

@sethfowler sethfowler force-pushed the seth/handle-const-default-actions-correctly branch from 2d93dce to 7d88ac6 Compare February 24, 2017 01:11
@sethfowler sethfowler force-pushed the seth/handle-const-default-actions-correctly branch from 7d88ac6 to 73ed6c8 Compare February 24, 2017 02:39
@sethfowler sethfowler merged commit 1c15fbb into p4lang:master Feb 24, 2017
@sethfowler sethfowler deleted the seth/handle-const-default-actions-correctly branch February 24, 2017 03:00
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.

3 participants