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

header_union support #561

Closed
mihaibudiu opened this issue May 3, 2017 · 13 comments · Fixed by #1760
Closed

header_union support #561

mihaibudiu opened this issue May 3, 2017 · 13 comments · Fixed by #1760
Assignees
Labels
bug This behavior is unintended and should be fixed. fixed This topic is considered to be fixed.

Comments

@mihaibudiu
Copy link
Contributor

The compiler support for header unions is incomplete.
This applies to both the front-end and to the existing back-ends.

@mihaibudiu mihaibudiu self-assigned this May 10, 2017
@mihaibudiu mihaibudiu added the bug This behavior is unintended and should be fixed. label May 10, 2017
@mihaibudiu mihaibudiu mentioned this issue May 10, 2017
mihaibudiu pushed a commit to mihaibudiu/p4c-clone that referenced this issue May 11, 2017
mihaibudiu pushed a commit to mihaibudiu/p4c-clone that referenced this issue May 12, 2017
ChrisDodd pushed a commit that referenced this issue May 15, 2017
* Improve support for header unions; part 1

* def-use analysis for unions

* Front-end support for unions; partial fix for #561

* cpplint fix

* Fix typo/bug
mihaibudiu pushed a commit to mihaibudiu/p4c-clone that referenced this issue Jun 16, 2017
mihaibudiu pushed a commit that referenced this issue Jun 21, 2017
@steffann
Copy link

I'm running into this when trying to correctly parse IPv6 extension headers and TCP options. Is there any work being done on this currently?

@mihaibudiu
Copy link
Contributor Author

Not really. What are you doing that isn't working?

@steffann
Copy link

steffann commented Feb 27, 2019

I'm parsing IPv6 extension headers and TCP options. They don't appear in a specified order, and there may even be more than one header of the same type. Some of those headers I need to modify (change checksums, change the TCP MSS option etc.). And at the end I need to emit them in the original order again.

I don't see a way to do this without a stack of header unions.

Basically the same use case as https://github.com/jafingerhut/p4-guide/tree/master/tcp-options-parser.

What would it cost to sponsor you to get that working for P4 with BMv2-SS? Not making any promises, but I can ask my boss ;)

@hesingh
Copy link
Contributor

hesingh commented Feb 27, 2019

One could use VarBits in P4. Define a size of VarBits you think is right for as many IPv6 options as possible. IPv6 has the next Header of the preceding header that helps index the subsequent header. Each IPv6 extension option uses a TLV format. Thus, the length is also known.

@mihaibudiu
Copy link
Contributor Author

There is no way you can sponsor me. I just have a biggish backlog of bugs that I am working on, and I prioritize them in some order based on who needs the features. What would be most useful is a simple program that does not work that we can turn into a test case. If you have inputs/outputs for the program - even better.

@steffann
Copy link

One could use VarBits in P4.

That would mean letting the parser put everything in one large VarBits field, and then writing my own parser. That seems very inefficient, especially as the P4 spec has already standardised what I need...

@steffann
Copy link

What would be most useful is a simple program that does not work that we can turn into a test case.

https://github.com/jafingerhut/p4-guide/blob/master/tcp-options-parser/tcp-options-parser.p4 seems to be a good fit :)

@mihaibudiu
Copy link
Contributor Author

I think that I am almost done with union support in bmv2. Working out the last few kinks.
If you want to write an STF file for the tcp-options-parser.p4 it could help a lot.

@steffann
Copy link

steffann commented Mar 2, 2019

Wow, thanks! I don't know what an STF file is, but I'll read up on that

@hesingh
Copy link
Contributor

hesingh commented Mar 2, 2019

See slide 136 of p4c/docs/compiler-design.ppts. There are several examples of such files in p4c/testdata/p4_16_samples/

@mihaibudiu
Copy link
Contributor Author

mihaibudiu commented Mar 4, 2019

@StefanN STF files are simple files that have input and output packets. You can see a few examples in the #1760 pull request.

Here is an example:

#      port number
#      v
packet 0 00 00 00 00
expect 0 00 FF 00 00

So this says that if you send packet with four zero bytes on port 0 you expect a packet with 4 bytes on port 0 where the second byte is FF.

If you can create a file to test tcp-options-parser.p4 we could get some confidence that this PR fixes indeed the issue. My tests are much simpler. You should name your file as tcp-options-parser.stf. @jafingerhut will thank you too.

Please note that this also required a small fix in the bmv2 simulator, so you may need to recompile and install that one too.

@mihaibudiu mihaibudiu added the fixed This topic is considered to be fixed. label Mar 4, 2019
@steffann
Copy link

steffann commented Mar 6, 2019

Seems you have already finished this. Many many thanks!!! You are all awesome

@mihaibudiu
Copy link
Contributor Author

But we could still benefit from having some tests for your program

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This behavior is unintended and should be fixed. fixed This topic is considered to be fixed.
Projects
None yet
3 participants