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

Fix json for header stack ref in push/pop primitives #189

Merged
merged 1 commit into from
Dec 15, 2016

Conversation

ChrisDodd
Copy link
Contributor

Fairly trivial fix -- the only part I'm unsure of is removing the comment which just seemed to be generally wrong. While other places may handle Type_Stack specially elsewhere, they then don't look at the result of the PathExpression conversion, or prune in the preorder to avoid it completely.

@mihaibudiu
Copy link
Contributor

This may be correct, but I can't figure it out just by reading. Unfortunately the way that p4-16 storage locations are mapped to metadata/headers is not very uniform - it's different for metadata, headers, stacks, and local variables. That's one reason that issue #172 hasn't been fixed. This part of the code should probably be reworked at some point.

Can you please add a test as well?

@ChrisDodd
Copy link
Contributor Author

existing switch.p4 tests excercise this code -- its just that we don't actually run bmv2 on the resulting json, so we fail to notice that it is complaining that the json is invalid.

@mihaibudiu
Copy link
Contributor

One possibility is to have an empty stf file - this causes the simple_switch to load the json and immediately exit.

@ChrisDodd
Copy link
Contributor Author

Actually I take that back -- the bug this is fixing only manifests when you convert switch.p4 into P4_16 code with --pp and then compile that resulting P4_16 version of switch.p4. The latter process introduces copies of the header stack into temp locals prior to doing the push/pop, and it is the push/pop on the temp local header_stack that causes the problem (I think). Since we don't actually do this dump-and-reparse normally, just adding a stf won't do it.

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.

2 participants