Documentation for architecture model + code cleanup

p4lang · mihaibudiu · Mar 4, 2020 · Apr 8, 2019 · Apr 8, 2019 · Apr 13, 2019
commit 1fd859e2cf09cbb9c6eec326bee21cc4a6dcc14e
diff --git a/backends/ubpf/docs/EXAMPLES.md b/backends/ubpf/docs/EXAMPLES.md
@@ -93,7 +93,8 @@ The parameters are as follows:
 - `key_type` - is bit array type (i.e. bit<32>) or struct like type  
 - `number_of_elements` - the maximum number of key-value pairs
 
-Currently registers have a limitation - they are not being initialized with default values. Initialization has to be done by a control plane. 
+Currently, the `ubpf` architecture model does not allow to initialize registers with default values. 
+Initialization has to be done by a control plane. 
 
 ### Rate limiter (rate-limiter.p4)
 
@@ -152,7 +153,8 @@ $ watch sudo ovs-ofctl dump-bpf-map br0 1 0
 
 This is very simple example of stateful firewall. Every TCP packet is analyzed to track the state of the TCP connection. 
 If the traffic belongs to known connection it is passed. Otherwise, it is dropped.  
-Notice that the example program uses hash function which is constrained to hash only 64 bit values - that's why TCP connection is identified via IP source and destination address.  
+Notice that the example program uses hash function which is constrained to hash only 64 bit values - that's why TCP connection is identified via IP source and destination address. 
+This is the known limitation of the `uBPF` backend used in P4rt-OVS (to be fixed in the future).
 
 Due to registers limitation before starting your own tests initialize simple firewall registers with zeros:
 

diff --git a/backends/ubpf/examples/gtp.p4 b/backends/ubpf/examples/gtp.p4
@@ -138,8 +138,6 @@ control pipe(inout Headers_t hdr, inout metadata meta) {
             gtp_decap;
             NoAction;
         }
-
-        //const default_action = NoAction();
     }
 
     action gtp_encap(bit<32> tunnelId) {
@@ -189,8 +187,6 @@ control pipe(inout Headers_t hdr, inout metadata meta) {
             gtp_encap;
             NoAction;
         }
-
-        //const default_action = NoAction();
     }
 
 

diff --git a/backends/ubpf/p4c-ubpf.cpp b/backends/ubpf/p4c-ubpf.cpp
@@ -39,7 +39,7 @@ void compile(EbpfOptions& options) {
     auto hook = options.getDebugHook();
     bool isv1 = options.langVersion == CompilerOptions::FrontendVersion::P4_14;
     if (isv1) {
-        ::error("This compiler only handles P4-16");
+        ::error(ErrorType::ERR_UNSUPPORTED, "- this compiler only handles P4-16");
         return;
     }
     auto program = P4::parseP4File(options);

diff --git a/backends/ubpf/p4include/ubpf_model.p4 b/backends/ubpf/p4include/ubpf_model.p4
@@ -14,37 +14,108 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-#ifndef _FILTER_MODEL_P4_
-#define _FILTER_MODEL_P4_
+#ifndef _UBPF_MODEL_P4_
+#define _UBPF_MODEL_P4_
 
 #include <core.p4>
 
-parser parse<H, M>(packet_in packet, out H headers, inout M meta);
-control filter<H, M>(inout H headers, inout M meta);
-@deparser
-control deparser<H>(packet_out b, in H headers);
-
-package ubpf<H, M>(parse<H, M> prs,
-                filter<H, M> filt,
-                deparser<H> dprs);
-
+/*
+ * The uBPF target can currently pass the packet or drop it.
+ * By default, all packets are passed.
+ * The mark_to_drop() extern can be used to mark a packet to be dropped.
+ * The mark_to_drop() modifies only the state hidden from the user's P4 program.
+ * mark_to_drop() should be called only in the 'pipe' control.
+ */
 extern void mark_to_drop();
+
+/*
+ * The uBPF target can currently pass the packet or drop it.
+ * By default, all packets are passed.
+ * The mark_to_pass() extern can be used to mark a packet to be passed (it cancels previous mark_to_drop() action).
+ * The mark_to_pass() modifies only the state hidden from the user's P4 program.
+ * mark_to_pass() should be called only in the 'pipe' control.
+ */
 extern void mark_to_pass();
 
+
 extern Register<T, S> {
+  /***
+   * A Register object is created by calling its constructor.
+   * You must provide a size of Register. The size specifies
+   * the maximum number of entries stored by Register.
+   * After constructing the Register object, you can use it in
+   * both actions or apply blocks.
+   * The Register is not intialized when created.
+   */
   Register(bit<32> size);
 
+  /***
+   * read() reads the state (T) of the register array stored at the
+   * specified index S, and returns it as the value written to the
+   * result parameter.
+   *
+   * @param index The index of the register array element to be
+   *              read, normally a value in the range [0, size-1].
+   * @return Returns result  of type T. Only types T that are bit<W>
+   *         are currently supported.  When index is in range, the value of
+   *         result becomes the value read from the register
+   *         array element.  When index >= size, the final
+   *         value of result is not specified, and should be
+   *         ignored by the caller.
+   */
   T read  (in S index);
+
+
   void write (in S index, in T value);
 }
 
+/*
+ * The extern used to get the current timestamp in nanoseconds.
+ */
 extern bit<48> ubpf_time_get_ns();
 
+
 enum HashAlgorithm {
     lookup3
 }
 
+/***
+ * Calculate a hash function of the value specified by the data
+ * parameter. Due to the limitation of uBPF back-end the maximum width of data is bit<64>.
+ *
+ * Note that the types of all of the parameters may be the same as, or
+ * different from, each other, and thus their bit widths are allowed
+ * to be different.
+ *
+ * Note that the result will have always the bit<32> width.
+ *
+ * @param D    Must be a tuple type where all the fields are bit-fields (type bit<W> or int<W>) or varbits.
+ *             Maximum width of D is 64 bit (limitation of uBPF back-end).
+ */
 extern void hash<D>(out bit<32> result, in HashAlgorithm algo, in D data);
 
-#endif
+/*
+ * Architecture.
+ *
+ * M must be a struct.
+ *
+ * H must be a struct where every one of its members is of type
+ * header, header stack, or header_union.
+ */
+
+parser parse<H, M>(packet_in packet, out H headers, inout M meta);
+control pipeline<H, M>(inout H headers, inout M meta);
+
+/*
+ * The only legal statements in the body of the deparser control are:
+ * calls to the packet_out.emit() method.
+ */
+@deparser
+control deparser<H>(packet_out b, in H headers);
+
+package ubpf<H, M>(parse<H, M> prs,
+                pipeline<H, M> p,
+                deparser<H> dprs);
+
+#endif /* _UBPF_MODEL_P4_ */
 
diff --git a/backends/ubpf/run-ubpf-test.py b/backends/ubpf/run-ubpf-test.py
@@ -79,8 +79,8 @@ def compare_output(self):
         c_differences = self.compare(options.cfilepath, reference_c_filepath)
 
         reference_h_filepath = os.path.join(options.referencedir, options.cfilename).replace(".c", ".h")
-        compiled_c_filepath = options.cfilepath.replace(".c", ".h")
-        h_differences = self.compare(compiled_c_filepath, reference_h_filepath)
+        compiled_h_filepath = options.cfilepath.replace(".c", ".h")
+        h_differences = self.compare(compiled_h_filepath, reference_h_filepath)
 
         return not c_differences and not h_differences
 

diff --git a/backends/ubpf/target.cpp b/backends/ubpf/target.cpp
@@ -37,7 +37,7 @@ namespace UBPF {
     void UbpfTarget::emitTableLookup(Util::SourceCodeBuilder *builder,
                                      cstring tblName,
                                      cstring key,
-                                     cstring value) const {
+                                     UNUSED cstring value) const {
         builder->appendFormat("ubpf_map_lookup(&%s, &%s)",
                               tblName.c_str(), key.c_str());
     }
@@ -68,4 +68,41 @@ namespace UBPF {
         builder->appendFormat("ubpf_packet_data(%s)", ctxVar.c_str());
     }
 
+    void UbpfTarget::emitUbpfHelpers(EBPF::CodeBuilder *builder) const {
+        builder->append(
+                "static void *(*ubpf_map_lookup)(const void *, const void *) = (void *)1;\n"
+                "static int (*ubpf_map_update)(void *, const void *, void *) = (void *)2;\n"
+                "static int (*ubpf_map_delete)(void *, const void *) = (void *)3;\n"
+                "static int (*ubpf_map_add)(void *, const void *) = (void *)4;\n"
+                "static uint64_t (*ubpf_time_get_ns)() = (void *)5;\n"
+                "static uint32_t (*ubpf_hash)(const void *, uint64_t) = (void *)6;\n"
+                "static void (*ubpf_printf)(const char *fmt, ...) = (void *)7;\n"
+                "static void *(*ubpf_packet_data)(const void *) = (void *)9;\n"
+                "static void *(*ubpf_adjust_head)(const void *, uint64_t) = (void *)8;\n"
+                "\n");
+        builder->newline();
+        builder->appendLine(
+                "#define write_partial(a, w, s, v) do { *((uint8_t*)a) = ((*((uint8_t*)a)) "
+                "& ~(BPF_MASK(uint8_t, w) << s)) | (v << s) ; } while (0)");
+        builder->appendLine("#define write_byte(base, offset, v) do { "
+                            "*(uint8_t*)((base) + (offset)) = (v); "
+                            "} while (0)");
+        builder->newline();
+        builder->append("static uint32_t\n"
+                        "bpf_htonl(uint32_t val) {\n"
+                        "    return htonl(val);\n"
+                        "}");
+        builder->newline();
+        builder->append("static uint16_t\n"
+                        "bpf_htons(uint16_t val) {\n"
+                        "    return htons(val);\n"
+                        "}");
+        builder->newline();
+        builder->append("static uint64_t\n"
+                        "bpf_htonll(uint64_t val) {\n"
+                        "    return htonll(val);\n"
+                        "}\n");
+        builder->newline();
+    }
+
 }
diff --git a/backends/ubpf/tests/testdata/test-simple-firewall.p4 b/backends/ubpf/tests/testdata/test-simple-firewall.p4
@@ -121,47 +121,45 @@ control pipe(inout Headers_t headers, inout metadata meta) {
 
     apply {
         if (headers.tcp.isValid()) {
-            @atomic {
-                if (headers.ipv4.srcAddr < headers.ipv4.dstAddr) {
-                    hash(meta.conn_id, HashAlgorithm.lookup3, { headers.ipv4.srcAddr, headers.ipv4.dstAddr });
-                } else {
-                    hash(meta.conn_id, HashAlgorithm.lookup3, { headers.ipv4.dstAddr, headers.ipv4.srcAddr });
-                }
+            if (headers.ipv4.srcAddr < headers.ipv4.dstAddr) {
+                hash(meta.conn_id, HashAlgorithm.lookup3, { headers.ipv4.srcAddr, headers.ipv4.dstAddr });
+            } else {
+                hash(meta.conn_id, HashAlgorithm.lookup3, { headers.ipv4.dstAddr, headers.ipv4.srcAddr });
+            }
 
-                meta.connInfo.s = conn_state.read(meta.conn_id);
-                meta.connInfo.srv_addr = conn_srv_addr.read(meta.conn_id);
-                if (meta.connInfo.s == 0 || meta.connInfo.srv_addr == 0) {
-                    if (headers.tcp.syn == 1 && headers.tcp.ack == 0) {
-                        // It's a SYN
-                        update_conn_info(SYNSENT, headers.ipv4.dstAddr);
+            meta.connInfo.s = conn_state.read(meta.conn_id);
+            meta.connInfo.srv_addr = conn_srv_addr.read(meta.conn_id);
+            if (meta.connInfo.s == 0 || meta.connInfo.srv_addr == 0) {
+                if (headers.tcp.syn == 1 && headers.tcp.ack == 0) {
+                    // It's a SYN
+                    update_conn_info(SYNSENT, headers.ipv4.dstAddr);
+                }
+            } else if (meta.connInfo.srv_addr == headers.ipv4.srcAddr) {
+                if (meta.connInfo.s == SYNSENT) {
+                    if (headers.tcp.syn == 1 && headers.tcp.ack == 1) {
+                        // It's a SYN-ACK
+                        update_conn_state(SYNACKED);
                     }
-                } else if (meta.connInfo.srv_addr == headers.ipv4.srcAddr) {
-                    if (meta.connInfo.s == SYNSENT) {
-                        if (headers.tcp.syn == 1 && headers.tcp.ack == 1) {
-                            // It's a SYN-ACK
-                            update_conn_state(SYNACKED);
-                        }
-                    } else if (meta.connInfo.s == SYNACKED) {
-                        _drop();
-                        return;
-                    } else if (meta.connInfo.s == ESTABLISHED) {
-                        if (headers.tcp.fin == 1 && headers.tcp.ack == 1) {
-                            update_conn_info(0, 0);  // clear register entry
-                        }
+                } else if (meta.connInfo.s == SYNACKED) {
+                    _drop();
+                    return;
+                } else if (meta.connInfo.s == ESTABLISHED) {
+                    if (headers.tcp.fin == 1 && headers.tcp.ack == 1) {
+                        update_conn_info(0, 0);  // clear register entry
+                    }
+                }
+            } else {
+                if (meta.connInfo.s == SYNSENT) {
+                    _drop();
+                    return;
+                } else if (meta.connInfo.s == SYNACKED) {
+                    if (headers.tcp.syn == 0 && headers.tcp.ack == 1) {
+                        // It's a ACK
+                        update_conn_state(ESTABLISHED);
                     }
-                } else {
-                    if (meta.connInfo.s == SYNSENT) {
-                        _drop();
-                        return;
-                    } else if (meta.connInfo.s == SYNACKED) {
-                        if (headers.tcp.syn == 0 && headers.tcp.ack == 1) {
-                            // It's a ACK
-                            update_conn_state(ESTABLISHED);
-                        }
-                    } else if (meta.connInfo.s == ESTABLISHED) {
-                        if (headers.tcp.fin == 1 && headers.tcp.ack == 1) {
-                            update_conn_info(0, 0); // clear register entry
-                        }
+                } else if (meta.connInfo.s == ESTABLISHED) {
+                    if (headers.tcp.fin == 1 && headers.tcp.ack == 1) {
+                        update_conn_info(0, 0); // clear register entry
                     }
                 }
             }

diff --git a/backends/ubpf/ubpfControl.h b/backends/ubpf/ubpfControl.h
@@ -35,47 +35,26 @@ namespace UBPF {
         std::vector<cstring> pointerVariables;
 
         explicit UBPFControlBodyTranslator(const UBPFControl *control);
-
         virtual void adjustPacketHead(const IR::Expression *expression, bool add);
-
         virtual void processMethod(const P4::ExternMethod *method);
-
         virtual void processApply(const P4::ApplyMethod *method);
-
         virtual void processFunction(const P4::ExternFunction *function);
-
         bool preorder(const IR::PathExpression *expression) override;
-
         bool preorder(const IR::MethodCallStatement *s) override;
-
         bool preorder(const IR::MethodCallExpression *expression) override;
-
         bool preorder(const IR::AssignmentStatement *a) override;
-
         bool preorder(const IR::BlockStatement *s) override;
-
         bool preorder(const IR::ExitStatement *) override;
-
         bool preorder(const IR::ReturnStatement *) override;
-
         bool preorder(const IR::IfStatement *statement) override;
-
         bool preorder(const IR::SwitchStatement *statement) override;
-
         bool preorder(const IR::Operation_Binary *b) override;
-
         bool comparison(const IR::Operation_Relation *b);
-
         bool preorder(const IR::Member *expression) override;
-
         void addPadding(std::vector<cstring> &paddingInitializers, unsigned int remainingBits, int paddingIndex) const;
-
         cstring createHashKeyInstance(const P4::ExternFunction *function);
-
         void appendValueAtOperator(const IR::AssignmentStatement *a) const;
-
         void emitAssignmentStatement(const IR::AssignmentStatement *a);
-
         bool emitRegisterRead(const IR::AssignmentStatement *a, const IR::MethodCallExpression *method);
     };
 

diff --git a/backends/ubpf/ubpfDeparser.cpp b/backends/ubpf/ubpfDeparser.cpp
@@ -34,7 +34,8 @@ namespace UBPF {
                                                           EBPF::EBPFType *type) {
 
         unsigned widthToEmit = dynamic_cast<EBPF::IHasWidth *>(type)->widthInBits();
-        unsigned loadSize;
+
+        unsigned loadSize = 0;
         cstring swap = "";
         if (widthToEmit <= 8) {
             loadSize = 8;
@@ -153,7 +154,6 @@ namespace UBPF {
             return;
         }
 
-        auto program = deparser->program;
         builder->emitIndent();
         builder->append("if (");
         visit(expr);
@@ -202,7 +202,7 @@ namespace UBPF {
     bool UBPFDeparser::build() {
         auto pl = controlBlock->container->type->applyParams;
         if (pl->size() != 2) {
-            ::error("Expected deparser to have exactly 2 parameters");
+            ::error("%1%: Expected deparser to have exactly 2 parameters", controlBlock->getNode());
             return false;
         }