p4lang · mihaibudiu · Jan 23, 2023 · Jan 19, 2023 · Jan 23, 2023 · mihaibudiu
diff --git a/backends/ebpf/codeGen.cpp b/backends/ebpf/codeGen.cpp
@@ -29,8 +29,20 @@ void CodeGenInspector::substitute(const IR::Parameter *p, const IR::Parameter *w
 }
 
 bool CodeGenInspector::preorder(const IR::Constant *expression) {
-    builder->append(Util::toString(expression->value, 0, false, expression->base));
-    return true;
+    unsigned width = EBPFInitializerUtils::ebpfTypeWidth(typeMap, expression);
+
+    if (EBPFScalarType::generatesScalar(width)) {
+        builder->append(Util::toString(expression->value, 0, false, expression->base));
+        return true;
+    }
+
+    cstring str = EBPFInitializerUtils::genHexStr(expression->value, width, expression);
+    builder->append("{ ");
+    for (size_t i = 0; i < str.size() / 2; ++i)
+        builder->appendFormat("0x%s, ", str.substr(2 * i, 2));
+    builder->append("}");
+
+    return false;
 }
 
 bool CodeGenInspector::preorder(const IR::StringLiteral *expression) {
@@ -329,9 +341,12 @@ bool CodeGenInspector::preorder(const IR::AssignmentStatement *a) {
     }
 
     if (memcpy) {
-        builder->append("memcpy(&");
+        builder->append("__builtin_memcpy(&");
         visit(a->left);
         builder->append(", &");
+        if (a->right->is<IR::Constant>()) {
+            builder->appendFormat("(u8[%u])", scalar->bytesRequired());
+        }
         visit(a->right);
         builder->appendFormat(", %d)", scalar->bytesRequired());
     } else {
@@ -429,4 +444,28 @@ void CodeGenInspector::widthCheck(const IR::Node *node) const {
             node, tb->size);
 }
 
+unsigned EBPFInitializerUtils::ebpfTypeWidth(P4::TypeMap *typeMap, const IR::Expression *expr) {
+    auto type = typeMap->getType(expr);
+    if (type == nullptr) type = expr->type;
+    if (type->is<IR::Type_InfInt>()) return 32;  // let's assume 32 bit for int type
+    auto ebpfType = EBPFTypeFactory::instance->create(type);
+    if (auto scalar = ebpfType->to<EBPFScalarType>()) {
+        unsigned width = scalar->implementationWidthInBits();
+        unsigned alignment = scalar->alignment() * 8;
+        unsigned units = ROUNDUP(width, alignment);
+        return units * alignment;
+    }
+    return 8;  // assume 1 byte if not available such information
+}
+
+cstring EBPFInitializerUtils::genHexStr(const big_int &value, unsigned width,
+                                        const IR::Expression *expr) {
+    // the required length of hex string, must be an even number
+    unsigned nibbles = 2 * ROUNDUP(width, 8);
+    auto str = value.str(0, std::ios_base::hex);
+    if (str.size() < nibbles) str = std::string(nibbles - str.size(), '0') + str;
+    BUG_CHECK(str.size() == nibbles, "%1%: value size does not match %2% bits", expr, width);
+    return str;
+}
+
 }  // namespace EBPF
diff --git a/backends/ebpf/codeGen.h b/backends/ebpf/codeGen.h
@@ -126,6 +126,15 @@ class CodeGenInspector : public Inspector {
     void widthCheck(const IR::Node *node) const;
 };
 
+class EBPFInitializerUtils {
+ public:
+    // return *real* number of bits required by type
+    static unsigned ebpfTypeWidth(P4::TypeMap *typeMap, const IR::Expression *expr);
+
+    // Generate hex string and prepend it with zeroes when shorter than required width
+    static cstring genHexStr(const big_int &value, unsigned width, const IR::Expression *expr);
+};
+
 }  // namespace EBPF
 
 #endif /* _BACKENDS_EBPF_CODEGEN_H_ */
diff --git a/backends/ebpf/ebpfParser.cpp b/backends/ebpf/ebpfParser.cpp
@@ -242,13 +242,15 @@ bool StateTranslationVisitor::preorder(const IR::SelectCase *selectCase) {
     return false;
 }
 
-void StateTranslationVisitor::compileExtractField(const IR::Expression *expr, cstring field,
-                                                  unsigned alignment, EBPFType *type) {
+void StateTranslationVisitor::compileExtractField(const IR::Expression *expr,
+                                                  const IR::StructField *field, unsigned alignment,
+                                                  EBPFType *type) {
     unsigned widthToExtract = dynamic_cast<IHasWidth *>(type)->widthInBits();
     auto program = state->parser->program;
     cstring msgStr;
+    cstring fieldName = field->name.name;
 
-    msgStr = Util::printf_format("Parser: extracting field %s", field);
+    msgStr = Util::printf_format("Parser: extracting field %s", fieldName);
     builder->target->emitTraceMessage(builder, msgStr.c_str());
 
     if (widthToExtract <= 64) {
@@ -277,7 +279,7 @@ void StateTranslationVisitor::compileExtractField(const IR::Expression *expr, cs
         unsigned shift = loadSize - alignment - widthToExtract;
         builder->emitIndent();
         visit(expr);
-        builder->appendFormat(".%s = (", field.c_str());
+        builder->appendFormat(".%s = (", fieldName.c_str());
         type->emit(builder);
         builder->appendFormat(")((%s(%s, BYTES(%s))", helper, program->packetStartVar.c_str(),
                               program->offsetVar.c_str());
@@ -293,6 +295,23 @@ void StateTranslationVisitor::compileExtractField(const IR::Expression *expr, cs
         builder->append(")");
         builder->endOfStatement(true);
     } else {
+        if (program->options.arch == "psa" && widthToExtract % 8 != 0) {
+            // To explain the problem in error lets assume that we have bit<68> field with value:
+            //   0x11223344556677889
+            //                     ^ this digit will be parsed into a half of byte
+            // Such fields are parsed into a table of bytes in network byte order, so possible
+            // values in dataplane are (note the position of additional '0' at the end):
+            //   0x112233445566778809
+            //   0x112233445566778890
+            // To correctly insert that padding, the length of field must be known, but tools like
+            // nikss-ctl (and the nikss library) don't consume P4info.txt to have such knowledge.
+            // There is also a bug in (de)parser causing such fields to be deparsed incorrectly.
+            ::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET,
+                    "%1%: fields wider than 64 bits must have a size multiple of 8 bits (1 byte) "
+                    "due to ambiguous padding in the LSB byte when the condition is not met",
+                    field);
+        }
+
         // wide values; read all bytes one by one.
         unsigned shift;
         if (alignment == 0)
@@ -310,7 +329,7 @@ void StateTranslationVisitor::compileExtractField(const IR::Expression *expr, cs
         for (unsigned i = 0; i < bytes; i++) {
             builder->emitIndent();
             visit(expr);
-            builder->appendFormat(".%s[%d] = (", field.c_str(), i);
+            builder->appendFormat(".%s[%d] = (", fieldName.c_str(), i);
             bt->emit(builder);
             builder->appendFormat(")((%s(%s, BYTES(%s) + %d) >> %d)", helper,
                                   program->packetStartVar.c_str(), program->offsetVar.c_str(), i,
@@ -343,13 +362,13 @@ void StateTranslationVisitor::compileExtractField(const IR::Expression *expr, cs
                 expr->to<IR::Member>()->expr->to<IR::PathExpression>()->path->name.name)) {
             exprStr = exprStr.replace(".", "->");
         }
-        cstring tmp = Util::printf_format("(unsigned long long) %s.%s", exprStr, field);
+        cstring tmp = Util::printf_format("(unsigned long long) %s.%s", exprStr, fieldName);
 
-        msgStr =
-            Util::printf_format("Parser: extracted %s=0x%%llx (%u bits)", field, widthToExtract);
+        msgStr = Util::printf_format("Parser: extracted %s=0x%%llx (%u bits)", fieldName,
+                                     widthToExtract);
         builder->target->emitTraceMessage(builder, msgStr.c_str(), 1, tmp.c_str());
     } else {
-        msgStr = Util::printf_format("Parser: extracted %s (%u bits)", field, widthToExtract);
+        msgStr = Util::printf_format("Parser: extracted %s (%u bits)", fieldName, widthToExtract);
         builder->target->emitTraceMessage(builder, msgStr.c_str());
     }
 
@@ -428,7 +447,7 @@ void StateTranslationVisitor::compileExtract(const IR::Expression *destination)
                     "Only headers with fixed widths supported %1%", f);
             return;
         }
-        compileExtractField(destination, f->name, alignment, etype);
+        compileExtractField(destination, f, alignment, etype);
         alignment += et->widthInBits();
         alignment %= 8;
     }

diff --git a/backends/ebpf/ebpfParser.h b/backends/ebpf/ebpfParser.h
@@ -36,8 +36,8 @@ class StateTranslationVisitor : public CodeGenInspector {
     P4::P4CoreLibrary &p4lib;
     const EBPFParserState *state;
 
-    void compileExtractField(const IR::Expression *expr, cstring name, unsigned alignment,
-                             EBPFType *type);
+    void compileExtractField(const IR::Expression *expr, const IR::StructField *field,
+                             unsigned alignment, EBPFType *type);
     virtual void compileExtract(const IR::Expression *destination);
     void compileLookahead(const IR::Expression *destination);
     void compileAdvance(const P4::ExternMethod *ext);

diff --git a/backends/ebpf/ebpfType.cpp b/backends/ebpf/ebpfType.cpp
@@ -129,7 +129,7 @@ void EBPFScalarType::declare(CodeBuilder *builder, cstring id, bool asPointer) {
         builder->append(id);
     } else {
         if (asPointer)
-            builder->append("u8*");
+            builder->appendFormat("u8* %s", id.c_str());
         else
             builder->appendFormat("u8 %s[%d]", id.c_str(), bytesRequired());
     }
@@ -144,9 +144,17 @@ void EBPFScalarType::declareInit(CodeBuilder *builder, cstring id, bool asPointe
         builder->append(id);
     } else {
         if (asPointer)
-            builder->append("u8*");
+            builder->appendFormat("u8* %s = NULL", id.c_str());
         else
-            builder->appendFormat("uint8_t %s[%d]", id.c_str(), bytesRequired());
+            builder->appendFormat("u8 %s[%d] = {0}", id.c_str(), bytesRequired());
+    }
+}
+
+void EBPFScalarType::emitInitializer(CodeBuilder *builder) {
+    if (generatesScalar(width)) {
+        builder->append("0");
+    } else {
+        builder->append("{ 0 }");
     }
 }
 

diff --git a/backends/ebpf/ebpfType.h b/backends/ebpf/ebpfType.h
@@ -112,7 +112,7 @@ class EBPFScalarType : public EBPFType, public IHasWidth {
     void emit(CodeBuilder *builder) override;
     void declare(CodeBuilder *builder, cstring id, bool asPointer) override;
     void declareInit(CodeBuilder *builder, cstring id, bool asPointer) override;
-    void emitInitializer(CodeBuilder *builder) override { builder->append("0"); }
+    void emitInitializer(CodeBuilder *builder) override;
     unsigned widthInBits() override { return width; }
     unsigned implementationWidthInBits() override { return bytesRequired() * 8; }
     // True if this width is small enough to store in a machine scalar

diff --git a/backends/ebpf/psa/README.md b/backends/ebpf/psa/README.md
@@ -619,7 +619,8 @@ table caching pass `--table-caching` to the compiler.
 
 We list the known bugs/limitations below. Refer to the Roadmap section for features planned in the near future.
 
-- Larger bit fields (e.g. IPv6 addresses) may not work properly.
+- Fields wider than 64 bits must have size multiple of 8 bits, otherwise they may have unexpected value in the LSB byte.
+  These fields may not work with all the externs and not all the operations on them are possible.
 - We noticed that `bpf_xdp_adjust_meta()` isn't implemented by some NIC drivers, so the `meta` XDP2TC mode may not work 
 with some NICs. So far, we have verified the correct behavior with Intel 82599ES. If a NIC doesn't support the `meta` XDP2TC mode you can use `head` or `cpumap` modes.
 - `lookahead()` with bit fields (e.g., `bit<16>`) doesn't work.

diff --git a/backends/ebpf/psa/ebpfPsaTable.cpp b/backends/ebpf/psa/ebpfPsaTable.cpp
@@ -137,32 +137,6 @@ class EBPFTablePSAImplementationPropertyVisitor : public EBPFTablePsaPropertyVis
     }
 };
 
-class EBPFTablePSAInitializerUtils {
- public:
-    // return *real* number of bits required by type
-    static unsigned ebpfTypeWidth(P4::TypeMap *typeMap, const IR::Expression *expr) {
-        auto type = typeMap->getType(expr);
-        auto ebpfType = EBPFTypeFactory::instance->create(type);
-        if (auto scalar = ebpfType->to<EBPFScalarType>()) {
-            unsigned width = scalar->implementationWidthInBits();
-            unsigned alignment = scalar->alignment() * 8;
-            unsigned units = ROUNDUP(width, alignment);
-            return units * alignment;
-        }
-        return 8;  // assume 1 byte if not available such information
-    }
-
-    // Generate hex string and prepend it with zeroes when shorter than required width
-    static cstring genHexStr(const big_int &value, unsigned width, const IR::Expression *expr) {
-        // the required length of hex string, must be an even number
-        unsigned nibbles = 2 * ROUNDUP(width, 8);
-        auto str = value.str(0, std::ios_base::hex);
-        if (str.size() < nibbles) str = std::string(nibbles - str.size(), '0') + str;
-        BUG_CHECK(str.size() == nibbles, "%1%: value size does not match %2% bits", expr, width);
-        return str;
-    }
-};
-
 // Generator for table key/value initializer value (const entries). Can't be used during table
 // lookup because this inspector expects only constant values as initializer.
 class EBPFTablePSAInitializerCodeGen : public CodeGenInspector {
@@ -191,22 +165,8 @@ class EBPFTablePSAInitializerCodeGen : public CodeGenInspector {
         expr->apply(*this);
     }
 
-    bool preorder(const IR::Constant *expr) override {
-        unsigned width = EBPFTablePSAInitializerUtils::ebpfTypeWidth(typeMap, expr);
-
-        if (EBPFScalarType::generatesScalar(width)) return CodeGenInspector::preorder(expr);
-
-        cstring str = EBPFTablePSAInitializerUtils::genHexStr(expr->value, width, expr);
-        builder->append("{ ");
-        for (size_t i = 0; i < str.size() / 2; ++i)
-            builder->appendFormat("0x%s, ", str.substr(2 * i, 2));
-        builder->append("}");
-
-        return false;
-    }
-
     bool preorder(const IR::Mask *expr) override {
-        unsigned width = EBPFTablePSAInitializerUtils::ebpfTypeWidth(typeMap, expr->left);
+        unsigned width = EBPFInitializerUtils::ebpfTypeWidth(typeMap, expr->left);
 
         if (EBPFScalarType::generatesScalar(width)) {
             visit(expr->left);
@@ -217,8 +177,8 @@ class EBPFTablePSAInitializerCodeGen : public CodeGenInspector {
             auto rc = expr->right->to<IR::Constant>();
             BUG_CHECK(lc != nullptr, "%1%: expected a constant value", expr->left);
             BUG_CHECK(rc != nullptr, "%1%: expected a constant value", expr->right);
-            cstring value = EBPFTablePSAInitializerUtils::genHexStr(lc->value, width, expr->left);
-            cstring mask = EBPFTablePSAInitializerUtils::genHexStr(rc->value, width, expr->right);
+            cstring value = EBPFInitializerUtils::genHexStr(lc->value, width, expr->left);
+            cstring mask = EBPFInitializerUtils::genHexStr(rc->value, width, expr->right);
             builder->append("{ ");
             for (size_t i = 0; i < value.size() / 2; ++i)
                 builder->appendFormat("(0x%s & 0x%s), ", value.substr(2 * i, 2),
@@ -241,7 +201,7 @@ class EBPFTablePSAInitializerCodeGen : public CodeGenInspector {
         cstring fieldName = ::get(table->keyFieldNames, key);
         cstring matchType = key->matchType->path->name.name;
         auto expr = currentEntry->keys->components[currentKeyEntryIndex];
-        unsigned width = EBPFTablePSAInitializerUtils::ebpfTypeWidth(typeMap, key->expression);
+        unsigned width = EBPFInitializerUtils::ebpfTypeWidth(typeMap, key->expression);
         bool isLPMMatch = matchType == P4::P4CoreLibrary::instance.lpmMatch.name;
         bool genPrefixLen = !tableHasTernaryMatch && isLPMMatch;
         bool doSwapBytes = genPrefixLen && EBPFScalarType::generatesScalar(width);
@@ -341,16 +301,16 @@ class EBPFTablePSATernaryTableMaskGenerator : public Inspector {
 
     bool preorder(const IR::Constant *expr) override {
         // exact match, set all bits as 'care'
-        unsigned bytes = ROUNDUP(EBPFTablePSAInitializerUtils::ebpfTypeWidth(typeMap, expr), 8);
+        unsigned bytes = ROUNDUP(EBPFInitializerUtils::ebpfTypeWidth(typeMap, expr), 8);
         for (unsigned i = 0; i < bytes; ++i) mask += "ff";
         return false;
     }
     bool preorder(const IR::Mask *expr) override {
         // Available value and mask, so use only this mask
         BUG_CHECK(expr->right->is<IR::Constant>(), "%1%: Expected a constant value", expr->right);
         auto &value = expr->right->to<IR::Constant>()->value;
-        unsigned width = EBPFTablePSAInitializerUtils::ebpfTypeWidth(typeMap, expr->right);
-        mask += EBPFTablePSAInitializerUtils::genHexStr(value, width, expr->right);
+        unsigned width = EBPFInitializerUtils::ebpfTypeWidth(typeMap, expr->right);
+        mask += EBPFInitializerUtils::genHexStr(value, width, expr->right);
         return false;
     }
 };
@@ -365,7 +325,7 @@ class EBPFTablePSATernaryKeyMaskGenerator : public EBPFTablePSAInitializerCodeGe
         // MidEnd transforms 0xffff... masks into exact match
         // So we receive there a Constant same as exact match
         // So we have to create 0xffff... mask on our own
-        unsigned width = EBPFTablePSAInitializerUtils::ebpfTypeWidth(typeMap, expr);
+        unsigned width = EBPFInitializerUtils::ebpfTypeWidth(typeMap, expr);
         unsigned bytes = ROUNDUP(width, 8);
 
         if (EBPFScalarType::generatesScalar(width)) {
@@ -383,7 +343,7 @@ class EBPFTablePSATernaryKeyMaskGenerator : public EBPFTablePSAInitializerCodeGe
     bool preorder(const IR::Mask *expr) override {
         // Mask value is our value which we want to generate
         BUG_CHECK(expr->right->is<IR::Constant>(), "%1%: expected constant value", expr->right);
-        EBPFTablePSAInitializerCodeGen::preorder(expr->right->to<IR::Constant>());
+        CodeGenInspector::preorder(expr->right->to<IR::Constant>());
         return false;
     }
 };