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

* DPDK backend: Add support for meter and counter PSA externs
p4lang · Jun 29, 2021 · 64eb44d · 64eb44d
1 parent 01eab61
commit 64eb44d
Show file tree

Hide file tree

Showing 24 changed files with 1,956 additions and 112 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -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

diff --git a/backends/dpdk/DpdkXfail.cmake b/backends/dpdk/DpdkXfail.cmake
@@ -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"

diff --git a/backends/dpdk/dpdk.def b/backends/dpdk/dpdk.def
@@ -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
 }

diff --git a/backends/dpdk/dpdkArch.h b/backends/dpdk/dpdkArch.h
@@ -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
@@ -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");
@@ -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;
     }
 };
 

diff --git a/backends/dpdk/dpdkHelpers.cpp b/backends/dpdk/dpdkHelpers.cpp
@@ -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;
@@ -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 " \
+                        "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");
             }

diff --git a/backends/dpdk/dpdkProgram.cpp b/backends/dpdk/dpdkProgram.cpp
@@ -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());
 }
 

diff --git a/backends/dpdk/dpdkProgram.h b/backends/dpdk/dpdkProgram.h
@@ -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,
@@ -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);

diff --git a/backends/dpdk/run-dpdk-test.py b/backends/dpdk/run-dpdk-test.py
@@ -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
 
@@ -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')
@@ -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:
@@ -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)