Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forbid egress pipeline in dpdk by default #3104

Merged
merged 1 commit into from
Mar 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion backends/dpdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ To load the 'spec' file in dpdk follow the instructions in the
- Packet Cloning/Recirculation/Resubmission

### DPDK target limitations
- Currently, programs written for DPDK target should limit the functionality in Ingress blocks. When egress block support gets added to the DPDK target, compiler should generate separate spec file for ingress and egress.
- Currently, programs written for DPDK target should limit the functionality in Ingress blocks, in case non empty Egress blocks are present it will be ignored by default unless Hidden temporary option `--enableEgress` used. When egress block support gets added to the DPDK target, and compiler can generate separate spec file for non empty ingress and egress blocks then option `--enableEgress` will be removed.
- DPDK architecture assumes the following signatures for programmable block of PSA. P4C-DPDK converts the input program to this form.

```P4
Expand Down
2 changes: 1 addition & 1 deletion backends/dpdk/backend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void DpdkBackend::convert(const IR::ToplevelBlock *tlb) {
auto program = tlb->getProgram();

std::set<const IR::P4Table*> invokedInKey;
auto convertToDpdk = new ConvertToDpdkProgram(refMap, typeMap, &structure);
auto convertToDpdk = new ConvertToDpdkProgram(refMap, typeMap, &structure, options);
auto genContextJson = new DpdkContextGenerator(refMap, typeMap, &structure, options);

PassManager simplify = {
Expand Down
42 changes: 23 additions & 19 deletions backends/dpdk/dpdkProgram.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,10 @@ const IR::DpdkAsmProgram *ConvertToDpdkProgram::create(IR::P4Program *prog) {
for (auto kv : structure->parsers) {
if (kv.first == "IngressParser")
kv.second->apply(*ingress_parser_converter);
else if (kv.first == "EgressParser")
kv.second->apply(*egress_parser_converter);
else if (kv.first == "MainParserT")
else if (kv.first == "EgressParser") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a bit strange.
First, because you compile for PSA, you should always have an egress.
Second, by giving a flag you are causing a large part of the P4 program to be ignored. I would at least check that the egress programs are "empty" and give a warning. Alternatively, you could dispense with the flag and not generate any code if all the egress components are empty. In other words, infer the value of this flag from the source program.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, code generated for ingress pipeline and egress pipeline are in single output file which is not right, because two should be independent.
Real future job is to generate two output file one for egress pipeline and other for egress pipeline from a single p4 program which has non empty ingress and egress block. Until this done we generate code for ingress only even when egress are present, but once above task done, we will get rid of this flag, that's why the flag is not exposed to user, it's hidden.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does not look very hidden to me.
Anyway, if this is temporary this should also be documented.

if (options.enableEgress)
kv.second->apply(*egress_parser_converter);
} else if (kv.first == "MainParserT")
kv.second->apply(*ingress_parser_converter);
else
BUG("Unknown parser %s", kv.second->name);
Expand All @@ -127,9 +128,10 @@ const IR::DpdkAsmProgram *ConvertToDpdkProgram::create(IR::P4Program *prog) {
for (auto kv : structure->pipelines) {
if (kv.first == "Ingress")
kv.second->apply(*ingress_converter);
else if (kv.first == "Egress")
kv.second->apply(*egress_converter);
else if (kv.first == "PreControlT")
else if (kv.first == "Egress") {
if (options.enableEgress)
kv.second->apply(*egress_converter);
} else if (kv.first == "PreControlT")
kv.second->apply(*ingress_converter);
else if (kv.first == "MainControlT")
kv.second->apply(*ingress_converter);
Expand All @@ -143,9 +145,10 @@ const IR::DpdkAsmProgram *ConvertToDpdkProgram::create(IR::P4Program *prog) {
for (auto kv : structure->deparsers) {
if (kv.first == "IngressDeparser")
kv.second->apply(*ingress_deparser_converter);
else if (kv.first == "EgressDeparser")
kv.second->apply(*egress_deparser_converter);
else if (kv.first == "MainDeparserT")
else if (kv.first == "EgressDeparser") {
if (options.enableEgress)
kv.second->apply(*egress_deparser_converter);
} else if (kv.first == "MainDeparserT")
kv.second->apply(*ingress_deparser_converter);
else
BUG("Unknown deparser block %s", kv.second->name);
Expand All @@ -160,9 +163,11 @@ const IR::DpdkAsmProgram *ConvertToDpdkProgram::create(IR::P4Program *prog) {
instr.append(ingress_parser_converter->getInstructions());
instr.append(ingress_converter->getInstructions());
instr.append(ingress_deparser_converter->getInstructions());
instr.append(egress_parser_converter->getInstructions());
instr.append(egress_converter->getInstructions());
instr.append(egress_deparser_converter->getInstructions());
if (options.enableEgress) {
instr.append(egress_parser_converter->getInstructions());
instr.append(egress_converter->getInstructions());
instr.append(egress_deparser_converter->getInstructions());
}

if (structure->isPNA())
instr.append(create_pna_postamble());
Expand Down Expand Up @@ -230,16 +235,15 @@ const IR::DpdkAsmProgram *ConvertToDpdkProgram::create(IR::P4Program *prog) {
}

auto tables = ingress_converter->getTables();
tables.append(egress_converter->getTables());

auto actions = ingress_converter->getActions();
actions.append(egress_converter->getActions());

auto selectors = ingress_converter->getSelectors();
selectors.append(egress_converter->getSelectors());

auto learners = ingress_converter->getLearners();
learners.append(egress_converter->getLearners());
if (options.enableEgress) {
tables.append(egress_converter->getTables());
actions.append(egress_converter->getActions());
selectors.append(egress_converter->getSelectors());
learners.append(egress_converter->getLearners());
}

return new IR::DpdkAsmProgram(
headerType, structType, dpdkExternDecls, actions, tables, selectors, learners,
Expand Down
7 changes: 5 additions & 2 deletions backends/dpdk/dpdkProgram.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ limitations under the License.
#include "ir/ir.h"
#include "lib/gmputil.h"
#include "lib/json.h"
#include "options.h"

namespace DPDK {

/* Maximum size in bits for fields in header and metadata structures */
Expand All @@ -40,12 +42,13 @@ class ConvertToDpdkProgram : public Transform {
P4::TypeMap *typemap;
P4::ReferenceMap *refmap;
DpdkProgramStructure *structure;
DpdkOptions &options;
const IR::DpdkAsmProgram *dpdk_program;

public:
ConvertToDpdkProgram(P4::ReferenceMap *refmap, P4::TypeMap *typemap,
DpdkProgramStructure *structure)
: typemap(typemap), refmap(refmap), structure(structure) { }
DpdkProgramStructure *structure, DpdkOptions &options)
: typemap(typemap), refmap(refmap), structure(structure), options(options) { }

const IR::DpdkAsmProgram *create(IR::P4Program *prog);
IR::IndexedVector<IR::DpdkAsmStatement> create_pna_preamble();
Expand Down
10 changes: 10 additions & 0 deletions backends/dpdk/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class DpdkOptions : public CompilerOptions {
bool loadIRFromJson = false;
// Compilation command line
static cstring DpdkCompCmd;
// Enable/Disable Egress pipeline in psa
bool enableEgress = false;

DpdkOptions() {
registerOption(
Expand All @@ -43,6 +45,14 @@ class DpdkOptions : public CompilerOptions {
return false;
},
"[Dpdk back-end] Lists exact name of all midend passes.\n");
registerOption(
"--enableEgress", nullptr,
[this](const char *) {
enableEgress = true;
return true;
},
"[Dpdk back-end] Enable egress pipeline's codegen\n", OptionFlags::Hide);

registerOption("--bf-rt-schema", "file",
[this](const char *arg) { bfRtSchema = arg; return true; },
"Generate and write BF-RT JSON schema to the specified file");
Expand Down
2 changes: 0 additions & 2 deletions testdata/p4_16_samples_outputs/psa-basic-counter-bmv2.p4.spec
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ apply {
table tbl
jmpneq LABEL_DROP m.psa_ingress_output_metadata_drop 0x0
emit h.ethernet
extract h.ethernet
emit h.ethernet
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
3 changes: 0 additions & 3 deletions testdata/p4_16_samples_outputs/psa-dpdk-errorcode-1.p4.spec
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
3 changes: 0 additions & 3 deletions testdata/p4_16_samples_outputs/psa-dpdk-errorcode-2.p4.spec
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
3 changes: 0 additions & 3 deletions testdata/p4_16_samples_outputs/psa-dpdk-errorcode.p4.spec
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@



struct ethernet_t {
bit<48> dstAddr
bit<48> srcAddr
Expand Down Expand Up @@ -61,62 +59,20 @@ struct metadata {
bit<32> psa_ingress_input_metadata_ingress_port
bit<8> psa_ingress_output_metadata_drop
bit<32> psa_ingress_output_metadata_egress_port
bit<16> local_metadata_data
bit<8> Egress_key
}
metadata instanceof metadata

header ethernet instanceof ethernet_t
header ipv4 instanceof ipv4_t
header tcp instanceof tcp_t

action NoAction args none {
return
}

action execute args none {
mov m.local_metadata_data 0x1
return
}

table tbl {
key {
m.local_metadata_data exact
m.Egress_key exact
}
actions {
NoAction
execute
}
default_action NoAction args none
size 0x10000
}


apply {
rx m.psa_ingress_input_metadata_ingress_port
mov m.psa_ingress_output_metadata_drop 0x0
jmpneq LABEL_DROP m.psa_ingress_output_metadata_drop 0x0
emit h.ethernet
emit h.ipv4
emit h.tcp
extract h.ethernet
mov m.tmpMask h.ethernet.etherType
and m.tmpMask 0xf00
jmpeq EGRESSPARSERIMPL_PARSE_IPV4 m.tmpMask 0x800
jmpeq EGRESSPARSERIMPL_PARSE_TCP h.ethernet.etherType 0xd00
jmp EGRESSPARSERIMPL_ACCEPT
EGRESSPARSERIMPL_PARSE_IPV4 : extract h.ipv4
mov m.tmpMask_0 h.ipv4.protocol
and m.tmpMask_0 0xfc
jmpeq EGRESSPARSERIMPL_PARSE_TCP m.tmpMask_0 0x4
jmp EGRESSPARSERIMPL_ACCEPT
EGRESSPARSERIMPL_PARSE_TCP : extract h.tcp
EGRESSPARSERIMPL_ACCEPT : mov m.Egress_key 0x48
table tbl
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,6 @@ apply {
emit h.ethernet
emit h.ipv4
emit h.tcp
emit h.ethernet
emit h.ipv4
emit h.tcp
tx m.psa_ingress_output_metadata_egress_port
LABEL_DROP : drop
}
Expand Down
Loading