[P4Testgen] Add a DPDK-PTF P4Testgen back end and the corresponding test runner #4173

Hoooao · 2023-09-23T12:28:54Z

Improved based on #4072 to use Testgen to generate PTF tests for pna/dpdk p4 programs.

fruffy · 2023-09-23T17:05:07Z

.github/workflows/PTF-TEST.yml

+on:
+  push:
+    branches:
+      - ptf-test-pr


This test needs to be updated. I would recommend replacing the existing Github action workflow.

yes, it will replace the existing dpdk ptf ci.

testdata/p4_16_samples/pna-dpdk-small_sample.p4

jafingerhut · 2023-10-04T00:44:30Z

@Hoooao @fruffy Question about the goal of this PR: When I compile the latest version of p4testgen, install it, and then try running a command like this:

$ cd p4c/testdata/p4_16_samples
$ p4testgen --target dpdk --arch pna --max-tests 1000 --out-dir tmp --test-backend ptf pna-dpdk-header-stack-assignment.p4 
In file: /home/andy/forks/p4c/backends/p4tools/modules/testgen/targets/pna/test_backend.cpp:49
Unimplemented compiler support: Test back end PTF not implemented for this target. Supported back ends are [METADATA].

Is the purpose of this PR to somehow use p4testgen to auto-generate PTF tests and execute them? What p4testgen command line do you use to accomplish the generation of the PTF tests?

fruffy · 2023-10-04T07:05:06Z

@Hoooao @fruffy Question about the goal of this PR: When I compile the latest version of p4testgen, install it, and then try running a command like this:
$ cd p4c/testdata/p4_16_samples
$ p4testgen --target dpdk --arch pna --max-tests 1000 --out-dir tmp --test-backend ptf pna-dpdk-header-stack-assignment.p4 
In file: /home/andy/forks/p4c/backends/p4tools/modules/testgen/targets/pna/test_backend.cpp:49
Unimplemented compiler support: Test back end PTF not implemented for this target. Supported back ends are [METADATA].
Is the purpose of this PR to somehow use p4testgen to auto-generate PTF tests and execute them? What p4testgen command line do you use to accomplish the generation of the PTF tests?

Yes, the goal is be able to generate PTF tests for the DPDK PNA architecture using the command you described.

jafingerhut · 2023-10-04T16:39:11Z

@Hoooao It is obviously up to you when you want to consider this feedback relative to other changes you are making on this PR, but note that there are currently unresolved conflicts in a couple of files between your changes, and some files that have had commits merged in that change them. You can see the "This branch has conflicts that must be resolved" messages on the Github PR page for details.

Hoooao · 2023-10-05T18:27:40Z

@Hoooao It is obviously up to you when you want to consider this feedback relative to other changes you are making on this PR, but note that there are currently unresolved conflicts in a couple of files between your changes, and some files that have had commits merged in that change them. You can see the "This branch has conflicts that must be resolved" messages on the Github PR page for details.

Thanks for the heads up

jafingerhut · 2023-10-06T13:45:36Z

@Hoooao Do you consider this read for review now, and then merging if reviews are OK? Or you expect to make more commits before review?

fruffy · 2023-10-06T13:47:18Z

@Hoooao Do you consider this read for review now, and then merging if reviews are OK? Or you expect to make more commits before review?

The PR is not ready yet, there are some comments which are not addressed. @Hoooao is also investigating a problem with infrap4d. Sometimes it simply does not start in CI and the reason is unclear.

jafingerhut · 2023-10-06T15:06:25Z

FYI, in case it might be related to the reason you are seeing P4-DPDK fail to start, if you have a number of ports that is not a power of 2, it fails when you try to load a P4 program into it (not when it starts up, but when you try to do the SetForwardingPipelineConfig to load the P4 program).

If you are having issues where infrap4d fails completely to start up at all, not even to the point that it should be waiting for a SetForwardingPipelineConfig request, then I do not have any guesses why that may be.

fruffy · 2023-10-06T15:17:13Z

If you are having issues where infrap4d fails completely to start up at all, not even to the point that it should be waiting for a SetForwardingPipelineConfig request, then I do not have any guesses why that may be.

It appears to be crashing when trying to launch bf_switchd. Unclear why unfortunately.

jafingerhut · 2023-10-06T16:16:23Z

My belief is that there should be no reason to run both infrap4d and bf_switchd.

bf_switchd gives TDI access to table add/modify/delete operations in the P4-DPDK data plane.

infrap4d contains a P4Runtime API server based on the Stratum code that listens for incoming TCP connection requests from P4Runtime API clients, and translates request messages into the appropriate TDI add/modify/delete operations.

Running both at the same time would likely only lead to problems, would be my guess.

fruffy · 2023-10-06T16:32:21Z

My belief is that there should be no reason to run both infrap4d and bf_switchd.

bf_switchd gives TDI access to table add/modify/delete operations in the P4-DPDK data plane.

infrap4d contains a P4Runtime API server based on the Stratum code that listens for incoming TCP connection requests from P4Runtime API clients, and translates request messages into the appropriate TDI add/modify/delete operations.

Running both at the same time would likely only lead to problems, would be my guess.

infrap4d launches bf_switchd here. Would love to avoid calling it but that seems to be baked in.

jafingerhut · 2023-10-06T19:00:01Z

infrap4d launches bf_switchd here. Would love to avoid calling it but that seems to be baked in.

I was unaware of that. Thanks for the info.

My statement is still correct, I think, from the perspective that there is no reason if you are manually running commands in sequence, to start both infrap4d and bf_switchd from the command line. You pick one.

fruffy · 2023-10-06T19:03:33Z

My statement is still correct, I think, from the perspective that there is no reason if you are manually running commands in sequence, to start both infrap4d and bf_switchd from the command line. You pick one.

In our case we are calling infrap4d, which in turns calls into the bf_switchd library, which then crashes. We can not reproduce this and it only happens occasionally in CI. However, it happens frequently enough to be a show stopper for developing a functional testing pipeline for the PTF DPDK target.

fruffy · 2023-10-13T15:23:54Z

@jafingerhut In this run https://github.com/p4lang/p4c/actions/runs/6501241470/job/17658197303?pr=4173 infrap4d fails to start (and appears to crash). This seemingly only happens in CI. Do you know anyone we could poke about this?

jafingerhut · 2023-10-15T22:11:26Z

@bharticemk @5abeel @satish153 @swaroopsarma Would one of you know why @fruffy (Fabian) might be experiencing the problems described in this comment: #4173 (comment) ?

I have run infrap4d within an IPDK Networking docker container before, but I have not yet attempted to do so outside of a Docker container, which I believe Fabian is trying to do here. I do not have a good summary of exactly how he has built and runs these tests, but I believe all of the details are available in this PR and the code it is based upon.

Test whole before modifing ptf tests Added new ptf.cpp and base_test.py Fix typo Added to CMake Fix option var Pkt size start fro 4096 CI for ptf-test-pr Show tests Add all dpdk tests to pna xFail updat lint&test ci added; reference file added Infrap4d failed

changed log_dir to default for collection add_on_miss only testgen on Add logs ci to main

fruffy · 2023-10-20T12:00:13Z

@jafingerhut We identified two issues. First, Github CI uses transparent hugepages which apparently can interfere with bf_switchd to startup. Disabling them seems to help with periodic startup problems.

Second, and more importantly, we are running all tests sequentially now. Both of these fixes lets us run DPDK PTF tests reliably.

This PR is now ready. It enables a pipeline to run P4Testgen on all available P4 DPDK programs, generate 10 tests for each program, then execute them. It also adds a PTF DPDK testing library which does not rely on p4runtime shell and instead uses Protobuf messages directly. @Hoooao implemented all of this.

The generated tests are still very basic of course, but we can make them more complex over time.

fruffy · 2023-10-20T12:40:55Z

backends/dpdk/base_test.py

@@ -0,0 +1,1142 @@
+# This file was copied and then modified from the file


There is a lot of redundancy here. We can likely make this a shared library for both BMv2 and DPDK.

fruffy · 2023-10-20T12:41:38Z

backends/dpdk/run-dpdk-ptf-test.py

-        if returncode != testutils.SUCCESS:
-            testutils.log.error("Failed to load pipeline")
-            return returncode
+        # NOTE: in generated PTF tests, the pipelines are loaded in individual test cases.


This comment should be removed. Fewer moving parts is good.

jafingerhut · 2023-10-20T19:56:40Z

It also adds a PTF DPDK testing library which does not rely on p4runtime shell and instead uses Protobuf messages directly. @Hoooao implemented all of this.

Is there any difference between the Python code in this PR for creating/parsing Protobuf messages, and that used for any target, e.g. BMv2, other than DPDK? It seems beneficial to have as few different Python libraries for this as we can get down to.

jafingerhut · 2023-10-20T19:57:36Z

@Hoooao Github reports that this branch is out-of-date with the base branch. Can you resolve any conflicts, please?

fruffy · 2023-10-20T21:09:13Z

It also adds a PTF DPDK testing library which does not rely on p4runtime shell and instead uses Protobuf messages directly. @Hoooao implemented all of this.

Is there any difference between the Python code in this PR for creating/parsing Protobuf messages, and that used for any target, e.g. BMv2, other than DPDK? It seems beneficial to have as few different Python libraries for this as we can get down to.

There are subtle differences in the two libraries, largely revolving around election id and device id. We could fix this in a separate PR and add a common P4Runtime communication library?

@Hoooao Github reports that this branch is out-of-date with the base branch. Can you resolve any conflicts, please?

This is a Github feature. The branch does not have conflicts, but HEAD moved on. It is a benign mismatch I'd say.

fruffy · 2023-10-20T23:30:08Z

It also adds a PTF DPDK testing library which does not rely on p4runtime shell and instead uses Protobuf messages directly. @Hoooao implemented all of this.

Is there any difference between the Python code in this PR for creating/parsing Protobuf messages, and that used for any target, e.g. BMv2, other than DPDK? It seems beneficial to have as few different Python libraries for this as we can get down to.

The changes were easy enough to do them here. We now have one single P4Runtime Python library that we can use for both BMv2 and DPDK. Will do some more cleanup for this PR. We will also have to fix the deprecated add-on-miss test.

jafingerhut · 2023-10-21T13:58:16Z

backends/dpdk/run-dpdk-ptf-test.py

-# 9559 is the default P4Runtime API server port
-GRPC_PORT: int = 9559
+# 28000 is the default P4Runtime API server port
+GRPC_PORT: int = 28000


I have no personal issue if you want to use a non-default port number for the P4Runtime API connection, but this change to the comment is definitely incorrect. 9559 is the default port, as mentioned in the P4Runtime API spec.

jafingerhut

Except for my latest minor comment about a comment that was changed to now be factually incorrect, this looks good to me.

fruffy · 2023-10-22T16:36:11Z

Except for my latest minor comment about a comment that was changed to now be factually incorrect, this looks good to me.

Fixed. I believe 28000 was the old port. I have created an issue (#4204) for the remaining TODOs. This will be done in a new PR.

fruffy reviewed Sep 23, 2023

View reviewed changes

testdata/p4_16_samples/pna-dpdk-small_sample.p4 Show resolved Hide resolved

Hoooao force-pushed the ptf-test-pr branch 2 times, most recently from 4b9393e to de0a499 Compare October 2, 2023 01:50

fruffy mentioned this pull request Oct 3, 2023

Simplify the DPDK PTF build and test scripts. #4168

Merged

Hoooao force-pushed the ptf-test-pr branch from 919b77b to 659ebab Compare October 5, 2023 18:28

Hoooao force-pushed the ptf-test-pr branch 2 times, most recently from b4d502a to 9068f58 Compare October 12, 2023 21:44

fruffy force-pushed the ptf-test-pr branch from 9068f58 to 7e34975 Compare October 15, 2023 22:02

fruffy changed the title ~~[WIP] Added PNA/DPDK PTF Testgen Procedure to CMake~~ [P4Testgen] Added PNA/DPDK PTF Testgen Procedure to CMake Oct 15, 2023

fruffy added p4tools Topics related to the P4Tools back end dpdk Topics related to the DPDK back end labels Oct 15, 2023

Hoooao added 2 commits October 18, 2023 17:48

Merged simplified procedure

005d4e1

changed log_dir to default for collection add_on_miss only testgen on Add logs ci to main

fruffy force-pushed the ptf-test-pr branch from 7e34975 to 243243f Compare October 18, 2023 21:54

Rebase and try to apply some fixes.

eacd219

fruffy force-pushed the ptf-test-pr branch from 243243f to eacd219 Compare October 18, 2023 22:00

fruffy added 2 commits October 18, 2023 19:36

Single thread testing.

b5f1f16

Cleanup.

a17d3a9

fruffy requested a review from jafingerhut October 20, 2023 11:57

fruffy reviewed Oct 20, 2023

View reviewed changes

Unify P4Runtime test library.

7d430f8

Remove the runtime shell.

e08de5a

fruffy force-pushed the ptf-test-pr branch from 94c4b55 to e08de5a Compare October 21, 2023 01:46

jafingerhut reviewed Oct 21, 2023

View reviewed changes

jafingerhut approved these changes Oct 21, 2023

View reviewed changes

fruffy added 2 commits October 21, 2023 13:23

Update ports.

73a4e31

Fix formatting.

9819306

fruffy changed the title ~~[P4Testgen] Added PNA/DPDK PTF Testgen Procedure to CMake~~ [P4Testgen] Add a DPDK-PTF P4Testgen back end and the corresponding test runner Oct 21, 2023

fruffy mentioned this pull request Oct 22, 2023

Convert existing DPDK PTF tests in testdata to the new testing format #4204

Closed

fruffy merged commit 212000e into p4lang:main Oct 22, 2023
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[P4Testgen] Add a DPDK-PTF P4Testgen back end and the corresponding test runner #4173

[P4Testgen] Add a DPDK-PTF P4Testgen back end and the corresponding test runner #4173

Hoooao commented Sep 23, 2023

fruffy Sep 23, 2023

Hoooao Oct 2, 2023

jafingerhut commented Oct 4, 2023

fruffy commented Oct 4, 2023

jafingerhut commented Oct 4, 2023

Hoooao commented Oct 5, 2023

jafingerhut commented Oct 6, 2023

fruffy commented Oct 6, 2023

jafingerhut commented Oct 6, 2023

fruffy commented Oct 6, 2023

jafingerhut commented Oct 6, 2023

fruffy commented Oct 6, 2023 •

edited

Loading

jafingerhut commented Oct 6, 2023

fruffy commented Oct 6, 2023 •

edited

Loading

fruffy commented Oct 13, 2023

jafingerhut commented Oct 15, 2023

fruffy commented Oct 20, 2023 •

edited

Loading

fruffy Oct 20, 2023

fruffy Oct 20, 2023

jafingerhut commented Oct 20, 2023

jafingerhut commented Oct 20, 2023

fruffy commented Oct 20, 2023 •

edited

Loading

fruffy commented Oct 20, 2023

jafingerhut Oct 21, 2023

jafingerhut left a comment

fruffy commented Oct 22, 2023

		@@ -0,0 +1,1142 @@
		# This file was copied and then modified from the file

[P4Testgen] Add a DPDK-PTF P4Testgen back end and the corresponding test runner #4173

[P4Testgen] Add a DPDK-PTF P4Testgen back end and the corresponding test runner #4173

Conversation

Hoooao commented Sep 23, 2023

fruffy Sep 23, 2023

Choose a reason for hiding this comment

Hoooao Oct 2, 2023

Choose a reason for hiding this comment

jafingerhut commented Oct 4, 2023

fruffy commented Oct 4, 2023

jafingerhut commented Oct 4, 2023

Hoooao commented Oct 5, 2023

jafingerhut commented Oct 6, 2023

fruffy commented Oct 6, 2023

jafingerhut commented Oct 6, 2023

fruffy commented Oct 6, 2023

jafingerhut commented Oct 6, 2023

fruffy commented Oct 6, 2023 • edited Loading

jafingerhut commented Oct 6, 2023

fruffy commented Oct 6, 2023 • edited Loading

fruffy commented Oct 13, 2023

jafingerhut commented Oct 15, 2023

fruffy commented Oct 20, 2023 • edited Loading

fruffy Oct 20, 2023

Choose a reason for hiding this comment

fruffy Oct 20, 2023

Choose a reason for hiding this comment

jafingerhut commented Oct 20, 2023

jafingerhut commented Oct 20, 2023

fruffy commented Oct 20, 2023 • edited Loading

fruffy commented Oct 20, 2023

jafingerhut Oct 21, 2023

Choose a reason for hiding this comment

jafingerhut left a comment

Choose a reason for hiding this comment

fruffy commented Oct 22, 2023

fruffy commented Oct 6, 2023 •

edited

Loading

fruffy commented Oct 6, 2023 •

edited

Loading

fruffy commented Oct 20, 2023 •

edited

Loading

fruffy commented Oct 20, 2023 •

edited

Loading