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

eBPF backend: generate P4Runtime files if required #4113

Merged
merged 5 commits into from
Sep 12, 2023

Conversation

tatry
Copy link
Contributor

@tatry tatry commented Aug 22, 2023

Allow to generate P4Runtime files by ebpf backend, because options related to them are ignored.

Supports only PSA architecture. For ebpf filer model options for generating P4info files are (still) ignored.

Copy link
Collaborator

@fruffy fruffy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing to also do is add golden tests here. Keep around reference P4Runtime files to check whether the output format is correct.

@jafingerhut
Copy link
Contributor

@tatry Are there existing automated CI tests that check for eBPF whether the output P4Info files contain the expected contents? As far as I know, the P4Info file generation is currently the same regardless of the target, and covers the v1model, PSA, and PNA architectures equally well.

@tatry
Copy link
Contributor Author

tatry commented Aug 28, 2023

@fruffy @jafingerhut

Are there existing automated CI tests that check for eBPF whether the output P4Info files contain the expected contents?

It depends on architecture:

  • eBPF filter model use standard CI tests, so it should not a big problem to add them (at least I think that). I verified here one thing: this model is not supported by P4RuntimeSerializer. I didn't test with option --arch filter, so I should disable generation P4info for eBPF filter model. But without that option P4info for v1model is generated, that's why I thought it is supported.
  • PSA architecture use custom PTF tests. There will be more work to add such functionality, because these tests have no support to compare P4info files - they are focused on packets rather on files.

I think that I can manage with these issues within this PR.

@tatry
Copy link
Contributor Author

tatry commented Sep 1, 2023

I've added some P4info files as examples and reference for tests. I think it is ready for re-review now.

@jafingerhut
Copy link
Contributor

This has 2 approving reviews, and passing all CI tests, so will merge it in as is. If this was too early, sorry for the confusion, and I guess we can create additional issues/PRs to improve things further.

@jafingerhut jafingerhut merged commit 0a0b5aa into p4lang:main Sep 12, 2023
16 checks passed
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.

4 participants