p4lang / behavioral-model

The reference P4 software switch
Apache License 2.0
541 stars 329 forks source link

Filling header stack value in the parser #913

Open antoine-bernabeu opened 4 years ago

antoine-bernabeu commented 4 years ago

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

jafingerhut commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

https://github.com/p4lang/p4c/issues/2007#issuecomment-566181710

antoninbas commented 4 years ago

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

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

mihaibudiu commented 4 years ago

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

hesingh commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.

jafingerhut commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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