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

Filling header stack value in the parser #913

Open
antoine-bernabeu opened this issue Jun 19, 2020 · 12 comments
Open

Filling header stack value in the parser #913

antoine-bernabeu opened this issue Jun 19, 2020 · 12 comments
Assignees

Comments

@antoine-bernabeu
Copy link

I am using p4c (version: 1.2.0 (SHA ebd446fa BUILD:debug) ) and simple_switch (version: 1.13.0-498202d9).
I wanted to emit a header stack and to set value of the header stack in the parser, using metadata as a counter. The p4c compiler found no errors but when I run simple_switch with the json file, I got this error.
image

Here the p4 program and the json file.
example_headerstack_parser.p4.txt
example_headerstack_parser.json.txt

@antoine-bernabeu antoine-bernabeu changed the title Filling header stack on parser Filling header stack value in the parser Jun 19, 2020
@jafingerhut
Copy link
Contributor

I have confirmed with similarly recent versions of p4c and simple_switch on my system that I get a similar error message, and encouraged Antoine to file this issue. I would guess it might be in the p4c bmv2 back end code, but if it turns out to be in simple_switch, we can move this issue to that repo instead.

@jafingerhut
Copy link
Contributor

The P4 program uses run-time variable expressions as indexes to header stacks, for which support was only added recently to p4c and simple_switch, so it seems likely that the issue is because of that.

@hesingh
Copy link
Contributor

hesingh commented Jun 19, 2020

Even before runtime index in array was added to p4c, the simple_switch supported such an index. See

p4lang/p4c#2007 (comment)

@mihaibudiu mihaibudiu self-assigned this Jun 19, 2020
@antoninbas
Copy link
Member

Seems that bmv2 should support this, unless this is not valid P4.

@mbudiu-vmw should be transfer this issue to the bmv2 repo?

@mihaibudiu
Copy link

I didn't investigate yet, but if this is valid Json then it's a bmv2 issue.

@hesingh
Copy link
Contributor

hesingh commented Jun 19, 2020

@antoine-bernabeu The P4 program that runtime index was tested with for p4c changes is here.

https://github.com/p4lang/p4c/blob/master/testdata/p4_16_samples/runtime-index-2-bmv2.p4

A STF file is also included in the same path. Please see the JSON output of this file and compare your P4 program and JSON to see if you can find the problem.

@antoine-bernabeu
Copy link
Author

I would say, in my program, I didn't extract the header stack before changing their values and I assign a value in the parser, not in the ingress control block. Regarding the json file, the "op" is "set" in my json file while in the program that was used for testing it is "assign" . I hope it will help.

@hesingh
Copy link
Contributor

hesingh commented Jun 19, 2020

I would manually edit your json file and then test if simple_switch works. The json format doc exists here for any help.

https://github.com/p4lang/behavioral-model/blob/master/docs/JSON_format.md

@antoninbas
Copy link
Member

I'm transferring this issue to bmv2 as I think this JSON should be considered valid.

Will transfer it back if it turns out to be an issue with p4c.

@antoninbas antoninbas transferred this issue from p4lang/p4c Jun 19, 2020
@antoninbas antoninbas assigned antoninbas and unassigned mihaibudiu Jun 19, 2020
@jafingerhut
Copy link
Contributor

Your fill_h3 parser state is assigning values to a header stack element that is currently invalid, I believe. According to the P4_16 language spec, such assignments are effective 'no ops'.

It is fairly unusual for a P4 parser to fill in headers via any means other than extract calls. It seems to me that the code you have in your parser state fill_h3 could functionally be done at the beginning of ingress processing, rather than during parsing?

@antoine-bernabeu
Copy link
Author

Yes it could be done in the ingress processing, but I wanted to use the parser as a "loop" because if we have hundred of headers in the header stack it can be really annoying to do it in the ingress processing. I tried with setting the header as valid in the parser just before assigning value. The result is the same, same error when running simple_switch.

@jafingerhut
Copy link
Contributor

I do not know whether the 'annoying' part you have found is 'repeitive code', but if so, it is not uncommon to work around this by using small programs to generate parts of P4 programs. This article gives one example to demonstrate: https://github.com/jafingerhut/p4-guide/blob/master/code-generation/README.md

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

No branches or pull requests

5 participants