DPDK Backend: Connection tracking support #3290
Conversation
backends/dpdk/dpdkArch.cpp
Outdated
| } | ||
| } | ||
| } | ||
| if (!isValidExternCall) { |
There was a problem hiding this comment.
should this be after the next brace?
| @@ -108,6 +108,9 @@ set (P4_XFAIL_TESTS | |||
| # These programs uses conditional execution inside action body which is currently | |||
| # unsupported | |||
| testdata/p4_16_samples/psa-dpdk-action-jumpOpt.p4 | |||
| testdata/p4_16_samples/pna-example-tcp-connection-tracking.p4 | |||
There was a problem hiding this comment.
we could remove this check from p4test as well.
There was a problem hiding this comment.
I think I misunderstood this comment and made the recent changes to midend/predication.h by removing the check for conditional execution inside action body.
Some of the tests added in this PR fail with p4test backend because of use of conditional execution within action. So, I added them to XFAIL list for p4test.
Shall I revert back to that, .i.e., no change to the predication.h and adding back the tests with conditional execution in action as XFAIL here?
backends/dpdk/dpdkHelpers.cpp
Outdated
| } else if (a->method->name == "set_entry_expire_time") { | ||
| auto args = a->expr->arguments; | ||
| if (args->size() != 1) { | ||
| ::error(ErrorType::ERR_UNEXPECTED, "Expected 1 arguments for %1%", |
midend/predication.h
Outdated
| @@ -113,10 +113,6 @@ class Predication final : public Transform { | |||
| // false for statements that are not dependent. | |||
| std::map<cstring, bool> isStatementDependent; | |||
| const IR::Statement* error(const IR::Statement* statement) const { | |||
| if (inside_action && ifNestingLevel > 0) | |||
There was a problem hiding this comment.
If you want to delete this code you should delete the whole function and its calls; the function does nothing now.
But I think this should be guided by a policy flag: "report errors when a statement cannot be converted".
Then invoke it with the flag set to "no" in your compiler.
There was a problem hiding this comment.
Finally, you are not even using this pass in your backend, why are you modifying it?
There was a problem hiding this comment.
Yes. DPDK backend does not use this pass. Wondering if I should still modify to use the policy flag and call it appropriately from p4test backend?
There was a problem hiding this comment.
On a closer look at predication pass, I think we can leave it as it is now because the error is thrown for the statements which prevent the if-else to Mux conversion.
I reverted the changes made to predication pass.
testdata/p4_16_samples/issue513.p4
Outdated
| @@ -55,8 +55,6 @@ control cIngress(inout Parsed_packet hdr, | |||
| action foo() { | |||
| meta.b = meta.b + 5; | |||
| if (meta.b > 10) { | |||
| // Next line causes error for p4test: MethodCallStatement: | |||
There was a problem hiding this comment.
Why delete this comment?
Why is the test still failing after your change?
* Connection tracking support
Connection tracking support for PNA needs the following extern functions to be supported.
add_entryextern with additional timeout parameterrestart_expire_timerset_entry_expire_timerDPDK provides the following corresponding instructions for connection tracking
learninstruction with timeout parameter (The second parameter of this instruction "action_params" is optional in DPDK)rearminstructionrearminstruction with timeout parameterThe timeout parameter is the index in timeout structure defined in the learner table.
All of the above instruction expect all the arguments in metadata and hence if any argument is constant, it should be moved to metadata.
These externs have constraints on where these externs can be called from. Added a new pass to check all these constraints.
The existing convertToDpdk pass is enhanced to process these new externs and generate equivalent assembly spec.
DPDK reference patch describing the rearm and learn instructions:
https://patches.dpdk.org/project/dpdk/patch/[email protected]/
@jafingerhut 's Connection tracking example from pna repo is added as a test here