uBPF backend: Support for forwarding #2381
Conversation
|
Your non-unified build has failed, which suggests that you are missing some |
| @@ -119,8 +135,8 @@ extern bit<16> csum_replace4(in bit<16> csum, | |||
| * header, header stack, or header_union. | |||
| */ | |||
|
|
|||
| parser parse<H, M>(packet_in packet, out H headers, inout M meta); | |||
| control pipeline<H, M>(inout H headers, inout M meta); | |||
| parser parse<H, M>(packet_in packet, out H headers, inout M meta, inout standard_metadata std); | |||
There was a problem hiding this comment.
If you make such changes they will break the user programs that were depending on your prior architecture.
Perhaps you can follow the model of v1model.p4, which has a version and conditional changes?
You should add a version anyway, whether you want to conditionally add this additional argument and handle it in the compiler is your choice.
There was a problem hiding this comment.
Fair point! I've added version to ubpf_model.p4. However, it seems that make check-p4 does not see macro's definition from test files, for example (e.g. p4_16_samples/tunneling_ubpf.p4):
#include <core.p4>
#define UBPF_MODEL_VERSION 20200515
#include <ubpf_model.p4>
Is there any specific workaround for v1model.p4 to let tests pass ? I don't observe this issue with make check-ubpf or standalone compilation.
There was a problem hiding this comment.
The compiler inserts a #define of V1MODEL_VERSION when translating P4_14 to P4_16, but that should not be relevant here. Other than that, there's no special handling of V1MODEL_VERSION needed -- if it is set explicitly in a test file, that will override the default in the header file.
There was a problem hiding this comment.
Actually, that hints at the problem -- the toP4 dumping code will insert a #define V1MODEL_VERSION in the dumped code if it is needed -- you may need something similar for this. See frontends/p4/toP4/toP4.cpp line 172; there's a check when dumping an #include <v1model.p4> line; you may need something similar for #include <ubpf_model.p4>. We should think about a way to make this easier to extend in a backend (a hook to the backend that is called from here, rather than having explicit checks for backend architectures.)
There was a problem hiding this comment.
It looks like a dirty hack to me and it would require to add dependency on uBPF model in the P4 frontend code (what I think we should avoid). I think we can break the backward compatibility for the uBPF programs, but explicitly say users that the old programs must be updated or the old version must be defined by macro definition. I've added the note in the documentation: https://github.com/p4lang/p4c/pull/2381/files#diff-616b1ea3f7f6e6e72e5cb1e8d44899dd. I've also updated examples for uBPF.
Does it sound acceptable to you?
There was a problem hiding this comment.
Let's merge this, but we should find a portable way to do versioning.
So far, the uBPF backend does not enable packet forwarding. This PR introduces
standard_metadatato theubpf_modelthat includesinput_portandoutput_port, so that a user can define packet forwarding behavior in the P4 program written for uBPF.