p4lang
eBPF backend doesn't generate emit in if statement correctly
In the eBPF backend, if the user attempts to write an emit inside an if statement in the deparser's apply block, the generated code is semantically different from the original. For example the following snippet:
control IngressDeparserImpl(packet_out packet,
out empty_t clone_i2e_meta,
out empty_t resubmit_meta,
out empty_t normal_meta,
inout headers hdr,
in metadata meta,
in psa_ingress_output_metadata_t istd)
{
apply {
if (meta.mymd == 0) {
packet.emit(hdr.ethernet);
}
packet.emit(hdr.ipv4);
}
}
Will generate and empty if statement:
if (meta->mymd == 0) {
; }
and completely ignore the emit for the ethernet header
Below is the link to a tarball which has both the P4 program used to reproduce the issue and the generated C eBPF file that will illustrate the bug: emit-bug.tar.gz
The command used to compile was:
./p4c-ebpf --arch psa emit-bug.p4 -o emit-bug.c
I compiled p4c with commit 8273608
Just a heads-up: the eBPF back end currently does not have an active maintainer who would take on this bug.
Just a heads-up: the eBPF back end currently does not have an active maintainer who would take on this bug.
Ohh, I see From what I understood, the tc backend uses the same deparser code and shares the same bug So we can make it also a tc backend issue
I'm not sure whether it is intended or not, but these are what I found
- The P4 deparser code is processed in EBPFDeparser::emit
- The 'If' statement is treated as an extern calls emitDeparserExternCalls
- EBPFDeparsePSA::emitDeparserExternCalls is called
- codeGen, that is a DeparserBodyTranslator is applied (I believe it is an inspector, visiting all nodes in the IR tree)
- The 'If' statement is an IR::IfStatement, it will be visited using ControlBodyTranslator::preorder
- 'packet.emit' is an IR::MethodCallStatement and will be visited using CodeGenInspector::preorder.
- 'packet.emit' is also an IR::MethodCallExpression, that is visited using DeparserBodyTranslator::preorder
- Finally, we can see where the emit header statement is skipped
I will look into this since this bug is related to my GSoC application. Thanks!
I will look into this since this bug is related to my GSoC application. Thanks!
Feel free to make a PR fixing these issues if you are able to!
I'm working on this issue for past two weeks, is there anything I should know particularly in the code generation file.
I'm working on this issue for past two weeks, is there anything I should know particularly in the code generation file.
In order to be successful at fixing the issue, sure there are many things you should know:
- enough C++ to understand the current implementation
- the current implementation of the p4c-ebpf code generator, at least the part that applies to generating deparser code
- an understanding of the behavior of the current p4c-ebpf back end to know why it omits the
ifstatements in today's implementation, and what part of it should change so that it does emit the desiredifstatements in the output with hopefully small easily-reviewed changes to the current p4c-ebpf back end.
If I knew more details about the above to share with you, I would, but unfortunately I do not. The p4c-ebpf back end code, as mentioned in an earlier comment on this issue, was developed by people who have moved on to other projects, and do not spend time on it any longer. Thus for issues like this, often it is the responsibility of the person wanting to fix the issue to educate themselves by self study on the current code to figure out how to fix things. That can be a significant investment of time -- enough time that, understandably, not many people choose to volunteer that much of their time.
thanks @jafingerhut sir for telling me this, u r right it already took my three weeks to understand about the p4 ebpf backend ,but still I dont have full deep dive knowledge of the code files in it. but I'm still learning ,@fruffy sir can u tell me some specific things or files to focus on, to solve the above issue, because learning full ebpf implementation of p4 will take plenty of time, I will certainly do it, but now for sake of gsoc can u tell me more specifics files , I should focus on to solve this issue , From last three what I understand while solving this issue the - emit statements inside if conditions in P4 deparsers don’t generate correct eBPF code. Instead of emitting the header, it generates an empty if block. what I found is - our ebpf backend splits deparser into two phases Phase 1: DeparserPrepareBufferTranslator (it calculates packet length) Phase 2: DeparserHdrEmitTranslator ( and it emits headers) main problem 👉 when emit is inside the if statement code block ,ControlBodyTranslator which is handling if conditions doesn’t properly forward the emit call to the DeparserHdrEmitTranslator. due to which emit is lost and only empty if block is generated. solution I implemented - make changes in control flow handling, so that if conditions are properly processed by the DeparserHdrEmitTranslator - 1)Make changes so that DeparserBodyTranslator handle emit directly 2)Update IfStatement handling to ensure emit inside conditions is forwarded correctly.
More precise changes I do in ebpfDeparser.cpp [this is main file which handles our deparser logic] precise changes are : DeparserBodyTranslator::preorder(const IR::MethodCallExpression) 👉 now it processes emit directly. ControlBodyTranslator::preorder(const IR::IfStatement) 👉 Ensures emit inside if is forwarded to DeparserHdrEmitTranslator. DeparserHdrEmitTranslator::processMethod() 👉 Now also updates buffer length (previously missing for emits in if).
codeGen.cpp[this file handles control flow] precise changes are: ControlBodyTranslator methods 👉 Ensure proper forwarding of emit calls.
I verified locally and run the compiled eBPF program with test packets. Verify that headers only emit when the condition is true. and run ninja build locally without any errors , but ninja build fails on GitHub checks. and also all test passes only 4 fails digest,digest01.
Is my approach right or there is anything else I should focus on .
If you want to ask very detailed questions about a particular set of changes you attempted to make, having a public PR with those changes is the most precise way to describe those changes. If you want to ask specific questions about part of your own PR, you can use Github to add comments to your own PR with those questions, attached to specific lines of changed code.
Having such a PR also makes the logs of the CI tests public and viewable for everyone, which is useful in cases when those logs have enough details to help diagnose problems (sometimes they do, sometimes not).