Add a PTF test CI pipeline for p4c-dpdk on the DPDK SoftNIC #4072
Conversation
|
Can you squash these commits into one? |
fruffy
changed the title
|
Is it intentional to delete so many files with this PR? |
Yes, the deleted files are the Github Action runs. The intention is to save CI runs while we get the pipeline right. |
fruffy
changed the title
|
@qobilidop Do you have any comments/suggestions on this PR? It's the first step. We are slowly building out a testing pipeline for the SoftNIC. |
|
This is awesome! I really like how clean and flexible it is to write a test using the PTF framework. And it seems to me all the existing PTF tests can be reused for the DPDK SoftNIC? If true, that's a big plus. It's probably time to remove the e2e-test in p4-dpdk-target and add more PTF tests in this repo instead 😂 I have some high level questions to clarify:
|
Yes, happy to get this done! Ultimately, we want to use P4Testgen on all the available sample programs and so it makes sense to have this pipeline as part of the compiler. We could also add any additional PTF tests to the repository to ensure the compiler is working correctly.
It's a good question. Currently, we are always testing the latest to exercise the entire toolchain. We could also pin some of the dependencies to keep things stable and then periodically update. Or have a meta-repository that tests the latest components. In my experience, it has been beneficial to have tests running on each PR to expose pain points early.
No, that is not possible yet. We could also run these kinds of workflows on both P4C and the p4-dpdk-target. The only question is where to place tests.
In the past, for example with BMv2, we timed these. We first merged the fix in BMv2 then merge the corresponding PR in P4C or vice versa.
Yep, the plan is to cut this down, too. Ideally, we want to have a rather minimal (or simple) installation. At least for the compiler.
Not quite sure. I believe |
Hoooao
force-pushed
the
main
branch
2 times, most recently
from
2b500b7 to
4025423
Compare
| tu.send_packet(self, ig_port, pkt_in) | ||
| first_pkt_time = time.time() | ||
|
|
||
| def verifyPackets(self): |
There was a problem hiding this comment.
This extra structure to tests that you have added with verifyPackets and sendPackets methods is fine, for tests where that pattern is sufficient.
In general, the fact that PTF tests allow you to write more general flows of tests, e.g. add some entries, send and verify some packets, add/modify/delete some table entries, send and verify more packets, etc. I consider a super-power of PTF tests over STF tests, and we should not discourage people from using it.
There was a problem hiding this comment.
Note: I will approve this PR regardless of this comment. I am not suggesting any changes to this PR. I just don't want people going forward thinking that they have to restrict themselves to writing tests with this pattern.
|
I believe that right now in the public repos for bf_switchd and infrap4d, a very important distinction is that bf_switchd ONLY allows you to do table add/modify/delete calls via an interactive Python shell that you can reach by typing the infrap4d DOES include a P4Runtime API server (and some other things, e.g. a gNMI and I believe also gNOI server). Thus at this time, if you want to do table add/delete/modify operations from a separate process (e.g. a Python program as in this PR), you must use infrap4d with P4-DPDK. |
Ccache Added Moved PTF tests into CMake; Shell scrips cleaned Fix CMake Fix CI Add sudo to cmd No Hugepage test Rearrange compilation Cleaned Ccache Added Moved PTF tests into CMake; Shell scrips cleaned Fix CMake Fix CI Add sudo to cmd No Hugepage test Rearrange compilatin Main branch update Renamed test with ptf prefix Test final added outputs IPDK recipe updated Renamed test; ipdk_recipe repo changed Deleted old test and outputs Added test outputs IPDK update Test ld_lib Cleaned code CI to main Changed comments Minor fixes Ccache Added Moved PTF tests into CMake; Shell scrips cleaned Fix CMake Fix CI Add sudo to cmd No Hugepage test Rearrange compilation Cleaned Moved PTF tests into CMake; Shell scrips cleaned Fix CMake Fix CI Add sudo to cmd No Hugepage test Rearrange compilatin Main branch update Renamed test with ptf prefix Test final added outputs IPDK recipe updated Renamed test; ipdk_recipe repo changed Deleted old test and outputs Added test outputs IPDK update Test ld_lib Cleaned code Changed comments Fix redundence Minor mods Rearrange p4c and recipe Test protoc version Deps updated Fix test Changed Dep_install Changed Dep_install to /usr/local Wrap up Delete residual comment check N Added loop to check if infrap4d is on; Disabled IPv6 Switch to main
|
@jafingerhut Did you notice any nondeterminism running tests using infrap4d? We see tests sporadically failing but it is unclear why. |
|
I did notice the following, which you might be seeing: The P4Runtime API specification says that a P4Runtime API server should not send back a response to a WriteRequest until after all successfully performed parts of the WriteRequest message have been committed to the data plane (and will thus affect all future packet processing). In my testing, infrap4d fairly often returns such a response message before the WriteRequest commands are committed to the data plane. Thus if the PTF test program immediately sends a packet into the data plane after receiving the response, there is a race to see whether the WriteRequest will be committed to the data plane first, or the packet will reach the data plane first and see the old state before the WriteRequest. In the mean tme, as a workaround, I have put in a call "waitForControlPlaneModifications()" after control plane operations, before sending in data packets, where all that function does is time.sleep() for an amount of time that lets infrap4d win the race. I found 1 second works well in my testing so far. I have notified the infrap4d authors of this, but should file an official bug to track it. |
|
@jafingerhut |
|
@Hoooao I am not aware of why p4rt-ctl command would be crashing. I have not used p4rt-ctl for any purposes other than the |
|
@Hoooao I did skim through a recent test failure log and saw this message: *** Warning: Unable to locate TLS certificates *** That looks like you might be trying to run |
|
@fruffy @Hoooao I have been doing some more asking around and realize that I have not personally confirmed that this race issue I mentioned above actually exists with P4-DPDK. I saw it when testing with a different system that also uses infrap4d, but the code responsible for that race might be outside of infrap4d. Anyway, short summary is that there might be a similar race in P4DPDK when using infrap4d, but I am not 100% sure there is. |
|
@jafingerhut Thanks for the follow-up. |
|
2 approvals and all CI tests passing. Merging. |
Summary
This PR sets up a PTF test CI pipeline for p4c-dpdk and the dpdk-target.
Background
A PTF test CI pipeline for p4c-dpdk on the DPDK SWX is currently missing. Adding such tests is beneficial for both the development of the compiler backend and DPDK SWX pipeline.
Demo
infrap4dandp4sde/dpdk-targetare properly setup.$P4C_DIRas the root directory for p4c.sudo $P4C_DIR/backends/dpdk/run-dpdk-ptf-test.py $P4C_DIR $P4C_DIR/testdata/p4_16_samples/pna-dpdk-add_on_miss0.p4 --ipdk-recipe $IPDK_RECIPE --sde-install $SDE_INSTALL --ld-library-path $LD_LIBRARY_PATH --testfile $P4C_DIR/testdata/p4_16_samples/pna-dpdk-add_on_miss0.py -ll DEBUGOutput:
Current status and further plans
infrap4dtakes too long. We will be working on a Docker image to try to accelerate the building process.