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

DPDK backend: Add support for meter and counter extern #2813

Merged
merged 34 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
a805208
Fix dpdk regression failure: Duplicates declaration
usha1830 Apr 21, 2021
611261b
Fix cpplint errors
usha1830 Apr 22, 2021
9c8e6d2
Added checks to ensure parameters exist before accessing them
usha1830 Apr 22, 2021
b184e0b
Merge branch 'master' into ushag/fix_duplicates_declaration
usha1830 Apr 22, 2021
c03e07d
Merge remote-tracking branch 'upstream/master'
usha1830 Apr 23, 2021
c99d261
Merging with p4lang/p4c repo main branch
usha1830 May 6, 2021
c6a5c12
Merge branch 'p4lang:main' into main
usha1830 May 7, 2021
4817ea2
Fix PSA extern Register's implementation
usha1830 May 11, 2021
bfb6f02
Updated spec.cpp as per review comments
usha1830 May 12, 2021
f5cfb35
Emit a warning if Register instantiation is found outside control block
usha1830 May 12, 2021
5ca47b1
Fixed a typo
usha1830 May 12, 2021
f0c7f9c
Remove restriction for instantiation and invocation of register exter…
usha1830 May 13, 2021
f3ab6ad
Merge branch 'p4lang:main' into ushag_conditional_operator
usha1830 May 28, 2021
778cd74
Conditional operator support for P4C-DPDK.
usha1830 May 28, 2021
2b992a4
Merge branch 'p4lang:main' into ushag_conditional_operator
usha1830 May 31, 2021
61567fd
Changing ternary match to exact in test program, updating the referen…
usha1830 May 31, 2021
009fd24
Merge branch 'ushag_conditional_operator' of https://github.com/usha1…
usha1830 May 31, 2021
afa8764
Merge branch 'p4lang:main' into ushag_conditional_operator
usha1830 Jun 4, 2021
05a68ba
Added support for range and mask operations in transition select stat…
usha1830 Jun 4, 2021
d43963c
Merge branch 'p4lang:main' into origin/ushag/mask_range
usha1830 Jun 9, 2021
8b9927f
Support mask operations in dpdk backend (#1)
usha1830 Jun 9, 2021
3bde5de
Remove pass for inserting temporary mask variables
usha1830 Jun 10, 2021
f7cb04d
Merge branch 'p4lang:main' into origin/ushag/mask_range
usha1830 Jun 10, 2021
a3e6ef6
update reference output
usha1830 Jun 10, 2021
6b963ce
Merge branch 'p4lang:main' into origin/ushag/mask_range
usha1830 Jun 16, 2021
8cf5882
Address review comments
usha1830 Jun 16, 2021
00ce76f
Merge branch 'p4lang:main' into origin/ushag/mask_range
usha1830 Jun 20, 2021
32d2abe
Merge branch 'p4lang:main' into origin/ushag/mask_range
usha1830 Jun 22, 2021
7986213
DPDK backend: Add support for meter and counter PSA externs
usha1830 Jun 22, 2021
9648cce
Address review comments for Meter and Counter extern support
usha1830 Jun 24, 2021
d663dbb
Merge branch 'p4lang:main' into ushag/Meter_counter
usha1830 Jun 25, 2021
76a0557
Cleanup dpdk regression test script to avoid saving error and runtime…
usha1830 Jun 25, 2021
402ae06
Updated reference output
usha1830 Jun 26, 2021
0013778
Simpilfied the implementation to use vector instead of map
usha1830 Jun 26, 2021
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
5 changes: 5 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,11 @@ add_custom_target(update_includes ALL
${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_SOURCE_DIR}/\$$h ${P4C_BINARY_DIR}/p4include \;
done
)
if (ENABLE_DPDK)
add_custom_target(dpdk_includes ALL
COMMAND ${CMAKE_COMMAND} -E make_directory ${P4C_BINARY_DIR}/p4include/dpdk
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${P4C_SOURCE_DIR}/p4include/dpdk/psa.p4 ${P4C_BINARY_DIR}/p4include/dpdk)
endif ()

# Installation
# Targets install themselves. Here we install the core headers
Expand Down
9 changes: 7 additions & 2 deletions backends/dpdk/DpdkXfail.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,15 @@ p4c_add_xfail_reason("dpdk"
)

p4c_add_xfail_reason("dpdk"
"Meter Not implemented"
"LHS of meter execute"
testdata/p4_16_samples/psa-meter1.p4
)

p4c_add_xfail_reason("dpdk"
"Expected atleast 2 arguments"
testdata/p4_16_samples/psa-meter3.p4
testdata/p4_16_samples/psa-meter7-bmv2.p4
)
)

# symbolic evaluator does not support verify() statement
p4c_add_xfail_reason("dpdk"
Expand Down
12 changes: 11 additions & 1 deletion backends/dpdk/dpdk.def
Original file line number Diff line number Diff line change
Expand Up @@ -433,17 +433,27 @@ class DpdkVerifyStatement: DpdkAsmStatement, IDPDKNode {
#nodbprint
}

class DpdkMeterDeclStatement: DpdkAsmStatement {
cstring meter;
Expression size;
std::ostream& toSpec(std::ostream& out) const override;
#nodbprint
}

class DpdkMeterExecuteStatement: DpdkAsmStatement, IDPDKNode {
cstring meter;
Expression index;
Expression color;
NullOK Expression length;
Expression color_in;
Expression color_out;
std::ostream& toSpec(std::ostream& out) const override;
#nodbprint
}

class DpdkCounterCountStatement: DpdkAsmStatement, IDPDKNode {
cstring counter;
Expression index;
NullOK optional Expression incr;
std::ostream& toSpec(std::ostream& out) const override;
#nodbprint
}
Expand Down
62 changes: 34 additions & 28 deletions backends/dpdk/dpdkArch.h
Original file line number Diff line number Diff line change
Expand Up @@ -442,39 +442,45 @@ class ConvertInternetChecksum : public PassManager {
}
};

/* This pass collects PSA register declaration instances and push them to a map
* for emitting to the .spec file later */
class CollectRegisterDeclaration : public Inspector {
std::map<const IR::Declaration_Instance *, cstring> *reg_map;

public:
CollectRegisterDeclaration(
std::map<const IR::Declaration_Instance *, cstring> *reg_map, P4::TypeMap *)
: reg_map(reg_map) {}
/* This pass collects PSA extern meter, counter and register declaration instances and
push them to a vector for emitting to the .spec file later */
class CollectExternDeclaration : public Inspector {
P4::TypeMap *typeMap;

public:
std::vector<const IR::Declaration_Instance *> externDecls;
CollectExternDeclaration(P4::TypeMap *typeMap) : typeMap(typeMap) {}
bool preorder(const IR::Declaration_Instance *d) override {
if (d->type->is<IR::Type_Specialized>()) {
auto type = d->type->to<IR::Type_Specialized>();
if (auto type = d->type->to<IR::Type_Specialized>()) {
auto externTypeName = type->baseType->path->name.name;
if (externTypeName == "Register"){
if (d->arguments->size() != 1 and d->arguments->size() != 2 ) {
::error("%1%: expected size and optionally init_val as arguments", d);
}
reg_map->emplace(d, d->name);
if (externTypeName == "Meter") {
if (d->arguments->size() != 2) {
::error("%1%: expected number of meters and type of meter as arguments", d);
} else {
/* Check if the meter is of PACKETS (0) type */
if (d->arguments->at(1)->expression->to<IR::Constant>()->asUnsigned() == 0)
::warning(ErrorType::WARN_UNSUPPORTED,
"%1%: Packet metering is not supported." \
" Falling back to byte metering.", d);
}
} else if (externTypeName == "Counter") {
if (d->arguments->size() != 2 ) {
::error("%1%: expected number of_counters and type of counter as arguments", d);
}
} else if (externTypeName == "Register") {
if (d->arguments->size() != 1 and d->arguments->size() != 2 ) {
::error("%1%: expected size and optionally init_val as arguments", d);
}
} else {
// unsupported extern type
return false;
}
externDecls.push_back(d);
}
return false;
}
};

class AddRegisterDeclaration : public PassManager {
public:
std::map<const IR::Declaration_Instance *, cstring> reg_map;
AddRegisterDeclaration(P4::TypeMap *typeMap) {
passes.push_back(new CollectRegisterDeclaration(&reg_map, typeMap));
}
};

// This pass is preparing logical expression for following branching statement
// optimization. This pass breaks parenthesis looks liks this: (a && b) && c.
// After this pass, the expression looks like this: a && b && c. (The AST is
Expand Down Expand Up @@ -583,7 +589,7 @@ class RewriteToDpdkArch : public PassManager {
std::map<const cstring, IR::IndexedVector<IR::Parameter> *>
*args_struct_map;
std::map<const IR::Declaration_Instance *, cstring> *csum_map;
std::map<const IR::Declaration_Instance *, cstring> *reg_map;
std::vector<const IR::Declaration_Instance *> *externDecls;
RewriteToDpdkArch(P4::ReferenceMap *refMap, P4::TypeMap *typeMap,
DpdkVariableCollector *collector) {
setName("RewriteToDpdkArch");
Expand Down Expand Up @@ -631,9 +637,9 @@ class RewriteToDpdkArch : public PassManager {
args_struct_map = &p->args_struct_map;
passes.push_back(p);
passes.push_back(new ConvertLogicalExpression);
auto insertRegDeclaration = new AddRegisterDeclaration(typeMap);
passes.push_back(insertRegDeclaration);
reg_map = &insertRegDeclaration->reg_map;
auto insertExternDeclaration = new CollectExternDeclaration(typeMap);
passes.push_back(insertExternDeclaration);
externDecls = &insertExternDeclaration->externDecls;
}
};

Expand Down
66 changes: 57 additions & 9 deletions backends/dpdk/dpdkHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,29 @@ bool ConvertStatementToDpdk::preorder(const IR::AssignmentStatement *a) {
i = new IR::DpdkGetChecksumStatement(
left, e->object->getName(), intermediate);
}
} else if (e->originalExternType->getName().name == "Meter") {
if (e->method->getName().name == "execute") {
auto argSize = e->expr->arguments->size();

// DPDK target needs index and packet length as mandatory parameters
if (argSize < 2) {
::error(ErrorType::ERR_UNEXPECTED, "Expected atleast 2 arguments for %1%",
e->object->getName());
return false;
}
const IR::Expression *color_in = nullptr;
const IR::Expression *length = nullptr;
auto index = e->expr->arguments->at(0)->expression;
if (argSize == 2) {
length = e->expr->arguments->at(1)->expression;
color_in = new IR::Constant(1);
} else if (argSize == 3) {
length = e->expr->arguments->at(2)->expression;
color_in = e->expr->arguments->at(1)->expression;
}
i = new IR::DpdkMeterExecuteStatement(
e->object->getName(), index, length, color_in, left);
}
} else if (e->originalExternType->getName().name == "Register") {
if (e->method->getName().name == "read") {
auto index = (*e->expr->arguments)[0]->expression;
Expand Down Expand Up @@ -412,21 +435,46 @@ bool ConvertStatementToDpdk::preorder(const IR::MethodCallStatement *s) {
}
} else if (a->originalExternType->getName().name == "Meter") {
if (a->method->getName().name == "execute") {
auto args = a->expr->arguments;
auto index = args->at(0)->expression;
auto color = args->at(1)->expression;
auto meter = a->object->getName();
add_instr(
new IR::DpdkMeterExecuteStatement(meter, index, color));
// DPDK target requires the result of meter execute method is assigned to a
// variable of PSA_MeterColor_t type.
::error(ErrorType::ERR_UNSUPPORTED, "LHS of meter execute statement is missing " \
Copy link
Contributor

Choose a reason for hiding this comment

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

no return here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, there is already a return just after the if-else chain.

"Use this format instead : color_out = %1%.execute(index, color_in)",
a->object->getName());
} else {
BUG("Meter function not implemented.");
}
} else if (a->originalExternType->getName().name == "Counter") {
auto di = a->object->to<IR::Declaration_Instance>();
auto declArgs = di->arguments;
unsigned value = 0;
auto counter_type = declArgs->at(1)->expression;
if (counter_type->is<IR::Constant>())
value = counter_type->to<IR::Constant>()->asUnsigned();
if (a->method->getName().name == "count") {
auto args = a->expr->arguments;
auto index = args->at(0)->expression;
auto counter = a->object->getName();
add_instr(new IR::DpdkCounterCountStatement(counter, index));
if (args->size() < 1){
::error(ErrorType::ERR_UNEXPECTED, "Expected atleast 1 arguments for %1%",
a->object->getName());
} else {
const IR::Expression *incr = nullptr;
auto index = args->at(0)->expression;
auto counter = a->object->getName();
if (args->size() == 2)
incr = args->at(1)->expression;
if (value == 2) {
if (incr) {
add_instr(new IR::DpdkCounterCountStatement(counter+"_packets",
index, incr));
add_instr(new IR::DpdkCounterCountStatement(counter+"_bytes",
index));
} else {
::error(ErrorType::ERR_UNEXPECTED,
"Expected packet length argument for %1%", a->object->getName());
}
} else {
add_instr(new IR::DpdkCounterCountStatement(counter, index, incr));
}
}
} else {
BUG("Counter function not implemented");
}
Expand Down
11 changes: 5 additions & 6 deletions backends/dpdk/dpdkProgram.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,14 @@ const IR::DpdkAsmProgram *ConvertToDpdkProgram::create(IR::P4Program *prog) {
structType.push_back(st);
}

IR::IndexedVector<IR::DpdkExternDeclaration> externDecls;
for (auto kv : *reg_map) {
auto s = kv.first;
auto st = new IR::DpdkExternDeclaration(s->name, s->annotations, s->type, s->arguments);
externDecls.push_back(st);
IR::IndexedVector<IR::DpdkExternDeclaration> dpdkExternDecls;
for (auto ed : *externDecls) {
auto st = new IR::DpdkExternDeclaration(ed->name, ed->annotations, ed->type, ed->arguments);
dpdkExternDecls.push_back(st);
}

return new IR::DpdkAsmProgram(
headerType, structType, externDecls, ingress_converter->getActions(),
headerType, structType, dpdkExternDecls, ingress_converter->getActions(),
ingress_converter->getTables(), statements, collector->get_globals());
}

Expand Down
5 changes: 2 additions & 3 deletions backends/dpdk/dpdkProgram.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ class ConvertToDpdkProgram : public Transform {
std::map<const cstring, IR::IndexedVector<IR::Parameter> *>
*args_struct_map;
std::map<const IR::Declaration_Instance *, cstring> *csum_map;
std::map<const IR::Declaration_Instance *, cstring> *reg_map;

std::vector<const IR::Declaration_Instance *> *externDecls;
public:
ConvertToDpdkProgram(BMV2::PsaProgramStructure &structure,
P4::ReferenceMap *refmap, P4::TypeMap *typemap,
Expand All @@ -69,7 +68,7 @@ class ConvertToDpdkProgram : public Transform {
info = dpdkarch->info;
args_struct_map = dpdkarch->args_struct_map;
csum_map = dpdkarch->csum_map;
reg_map = dpdkarch->reg_map;
externDecls = dpdkarch->externDecls;
}

const IR::DpdkAsmProgram *create(IR::P4Program *prog);
Expand Down
45 changes: 5 additions & 40 deletions backends/dpdk/run-dpdk-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,10 @@ def check_generated_files(options, tmpdir, expecteddir):
shutil.copy2(produced, expected)
else:
result = compare_files(options, produced, expected, file[-7:] == "-stderr")
if result != SUCCESS and (file[-7:] != "-stderr" or not ignoreStderr(options)):
# We do not want to compare stderr output generated by p4c-dpdk
if result != SUCCESS and (file[-7:] == "-error"):
return SUCCESS
if result != SUCCESS and not ignoreStderr(options):
return result
return SUCCESS

Expand All @@ -180,31 +183,10 @@ def process_file(options, argv):
if not os.path.exists(expected_dirname):
os.makedirs(expected_dirname)

# We rely on the fact that these keys are in alphabetical order.
rename = { "FrontEndDump": "first",
"FrontEndLast": "frontend",
"MidEndLast": "midend" }

if options.verbose:
print("Writing temporary files into ", tmpdir)
ppfile = os.path.join(tmpdir, basename) # after parsing
referenceOutputs = ",".join(list(rename.keys()))
stderr = os.path.join(tmpdir, basename + "-stderr")
stderr = os.path.join(tmpdir, basename + "-error")
spec = os.path.join(tmpdir, basename + ".spec")
p4runtimeFile = os.path.join(tmpdir, basename + ".p4info.txt")
p4runtimeEntriesFile = os.path.join(tmpdir, basename + ".entries.txt")

# Create the `json_outputs` directory if it doesn't already exist. There's a
# race here since multiple tests may run this code in parallel, so we can't
# check if it exists beforehand.
try:
os.mkdir("json_outputs")
except OSError as exc:
if exc.errno != errno.EEXIST:
raise

# P4Info generation requires knowledge of the architecture, so we must
# invoke the compiler with a valid --arch.
def getArch(path):
v1Pattern = re.compile('include.*v1model\.p4')
psaPattern = re.compile('include.*psa\.p4')
Expand All @@ -225,12 +207,6 @@ def getArch(path):
arch = getArch(options.p4filename)
if arch is not None:
args.extend(["--arch", arch])
if options.generateP4Runtime:
args.extend(["--p4runtime-files", p4runtimeFile])
args.extend(["--p4runtime-entries-files", p4runtimeEntriesFile])

if "p4_14" in options.p4filename or "v1_samples" in options.p4filename:
args.extend(["--std", "p4-14"])
args.extend(argv)
if options.runDebugger:
if options.runDebugger_skip > 0:
Expand All @@ -252,17 +228,6 @@ def getArch(path):
if expected_error and result == SUCCESS:
result = FAILURE

# Canonicalize the generated file names
for k in sorted(rename.keys()):
files = glob.glob(os.path.join(tmpdir, base + "*" + k + "*.p4"))
if len(files) > 1:
print("Multiple files matching", k)
elif len(files) == 1:
file = files[0]
if os.path.isfile(file):
newName = file_name(tmpdir, base, rename[k], ext)
os.rename(file, newName)

if result == SUCCESS:
result = check_generated_files(options, tmpdir, expected_dirname)

Expand Down
Loading