[ovs-dev] [v11 02/11] dpif-netdev: Add auto validation function for miniflow extract

kumar Amber kumar.amber at intel.com
Wed Jul 14 02:02:13 UTC 2021


From: Kumar Amber <kumar.amber at intel.com>

This patch introduced the auto-validation function which
allows users to compare the batch of packets obtained from
different miniflow implementations against the linear
miniflow extract and return a hitmask.

The autovaidator function can be triggered at runtime using the
following command:

$ ovs-appctl dpif-netdev/miniflow-parser-set autovalidator

Signed-off-by: Kumar Amber <kumar.amber at intel.com>
Co-authored-by: Harry van Haaren <harry.van.haaren at intel.com>
Signed-off-by: Harry van Haaren <harry.van.haaren at intel.com>
Acked-by: Eelco Chaudron <echaudro at redhat.com>

---
v9:
- fix review comments Flavio
v6:
-fix review comments(Eelco)
v5:
- fix review comments(Ian, Flavio, Eelco)
- remove ovs assert and switch to default after a batch of packets
  is processed
- Atomic set and get introduced
- fix raw_ctz for windows build
---
---
 NEWS                              |   2 +
 lib/dpif-netdev-private-extract.c | 150 ++++++++++++++++++++++++++++++
 lib/dpif-netdev-private-extract.h |  22 +++++
 lib/dpif-netdev.c                 |   2 +-
 4 files changed, 175 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index b0f08e96d..cf254bcfe 100644
--- a/NEWS
+++ b/NEWS
@@ -33,6 +33,8 @@ Post-v2.15.0
        CPU supports it. This enhances performance by using the native vpopcount
        instructions, instead of the emulated version of vpopcount.
      * Add command line option to switch between MFEX function pointers.
+     * Add miniflow extract auto-validator function to compare different
+       miniflow extract implementations against default implementation.
    - ovs-ctl:
      * New option '--no-record-hostname' to disable hostname configuration
        in ovsdb on startup.
diff --git a/lib/dpif-netdev-private-extract.c b/lib/dpif-netdev-private-extract.c
index 688190e70..a9ca56bd0 100644
--- a/lib/dpif-netdev-private-extract.c
+++ b/lib/dpif-netdev-private-extract.c
@@ -38,6 +38,11 @@ static ATOMIC(miniflow_extract_func) default_mfex_func;
  */
 static struct dpif_miniflow_extract_impl mfex_impls[] = {
 
+    [MFEX_IMPL_AUTOVALIDATOR] = {
+        .probe = NULL,
+        .extract_func = dpif_miniflow_extract_autovalidator,
+        .name = "autovalidator", },
+
     [MFEX_IMPL_SCALAR] = {
         .probe = NULL,
         .extract_func = NULL,
@@ -160,3 +165,148 @@ dp_mfex_impl_get_by_name(const char *name, miniflow_extract_func *out_func)
 
     return -ENOENT;
 }
+
+uint32_t
+dpif_miniflow_extract_autovalidator(struct dp_packet_batch *packets,
+                                    struct netdev_flow_key *keys,
+                                    uint32_t keys_size, odp_port_t in_port,
+                                    struct dp_netdev_pmd_thread *pmd_handle)
+{
+    const size_t cnt = dp_packet_batch_size(packets);
+    uint16_t good_l2_5_ofs[NETDEV_MAX_BURST];
+    uint16_t good_l3_ofs[NETDEV_MAX_BURST];
+    uint16_t good_l4_ofs[NETDEV_MAX_BURST];
+    uint16_t good_l2_pad_size[NETDEV_MAX_BURST];
+    struct dp_packet *packet;
+    struct dp_netdev_pmd_thread *pmd = pmd_handle;
+    struct netdev_flow_key test_keys[NETDEV_MAX_BURST];
+
+    if (keys_size < cnt) {
+        miniflow_extract_func default_func = NULL;
+        atomic_uintptr_t *pmd_func = (void *)&pmd->miniflow_extract_opt;
+        atomic_store_relaxed(pmd_func, (uintptr_t) default_func);
+        VLOG_ERR("Invalid key size supplied, Key_size: %d less than"
+                 "batch_size:  %" PRIuSIZE"\n", keys_size, cnt);
+        VLOG_ERR("Autovalidatior is disabled.\n");
+        return 0;
+    }
+
+    /* Run scalar miniflow_extract to get default result. */
+    DP_PACKET_BATCH_FOR_EACH (i, packet, packets) {
+        pkt_metadata_init(&packet->md, in_port);
+        miniflow_extract(packet, &keys[i].mf);
+
+        /* Store known good metadata to compare with optimized metadata. */
+        good_l2_5_ofs[i] = packet->l2_5_ofs;
+        good_l3_ofs[i] = packet->l3_ofs;
+        good_l4_ofs[i] = packet->l4_ofs;
+        good_l2_pad_size[i] = packet->l2_pad_size;
+    }
+
+    uint32_t batch_failed = 0;
+    /* Iterate through each version of miniflow implementations. */
+    for (int j = MFEX_IMPL_START_IDX; j < MFEX_IMPL_MAX; j++) {
+        if (!mfex_impls[j].available) {
+            continue;
+        }
+        /* Reset keys and offsets before each implementation. */
+        memset(test_keys, 0, keys_size * sizeof(struct netdev_flow_key));
+        DP_PACKET_BATCH_FOR_EACH (i, packet, packets) {
+            dp_packet_reset_offsets(packet);
+        }
+        /* Call optimized miniflow for each batch of packet. */
+        uint32_t hit_mask = mfex_impls[j].extract_func(packets, test_keys,
+                                                       keys_size, in_port,
+                                                       pmd_handle);
+
+        /* Do a miniflow compare for bits, blocks and offsets for all the
+         * classified packets in the hitmask marked by set bits. */
+        while (hit_mask) {
+            /* Index for the set bit. */
+            uint32_t i = raw_ctz(hit_mask);
+            /* Set the index in hitmask to Zero. */
+            hit_mask &= (hit_mask - 1);
+
+            uint32_t failed = 0;
+
+            struct ds log_msg = DS_EMPTY_INITIALIZER;
+            ds_put_format(&log_msg, "MFEX autovalidator pkt %d\n", i);
+
+            /* Check miniflow bits are equal. */
+            if ((keys[i].mf.map.bits[0] != test_keys[i].mf.map.bits[0]) ||
+                (keys[i].mf.map.bits[1] != test_keys[i].mf.map.bits[1])) {
+                ds_put_format(&log_msg, "Autovalidation map failed\n"
+                              "Good 0x%llx 0x%llx\tTest 0x%llx"
+                              " 0x%llx\n", keys[i].mf.map.bits[0],
+                              keys[i].mf.map.bits[1],
+                              test_keys[i].mf.map.bits[0],
+                              test_keys[i].mf.map.bits[1]);
+                failed = 1;
+            }
+
+            if (!miniflow_equal(&keys[i].mf, &test_keys[i].mf)) {
+                uint32_t block_cnt = miniflow_n_values(&keys[i].mf);
+                ds_put_format(&log_msg, "Autovalidation blocks failed\n"
+                              "nGood hex:\n");
+                ds_put_hex_dump(&log_msg, &keys[i].buf, block_cnt * 8, 0,
+                                false);
+                ds_put_format(&log_msg, "Test hex:\n");
+                ds_put_hex_dump(&log_msg, &test_keys[i].buf, block_cnt * 8, 0,
+                                false);
+                failed = 1;
+            }
+
+            packet = packets->packets[i];
+            if ((packet->l2_pad_size != good_l2_pad_size[i]) ||
+                    (packet->l2_5_ofs != good_l2_5_ofs[i]) ||
+                    (packet->l3_ofs != good_l3_ofs[i]) ||
+                    (packet->l4_ofs != good_l4_ofs[i])) {
+                ds_put_format(&log_msg, "Autovalidation packet offsets failed"
+                              "\n");
+                ds_put_format(&log_msg, "Good offsets: l2_pad_size %u,"
+                              " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
+                              good_l2_pad_size[i], good_l2_5_ofs[i],
+                              good_l3_ofs[i], good_l4_ofs[i]);
+                ds_put_format(&log_msg, "  Test offsets: l2_pad_size %u,"
+                              " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
+                              packet->l2_pad_size, packet->l2_5_ofs,
+                              packet->l3_ofs, packet->l4_ofs);
+                failed = 1;
+            }
+
+            if (failed) {
+                VLOG_ERR("Autovalidation for %s failed in pkt %d,"
+                         " disabling.", mfex_impls[j].name, i);
+                VLOG_ERR("Autovalidation failure details:\n%s",
+                         ds_cstr(&log_msg));
+                batch_failed = 1;
+            }
+            ds_destroy(&log_msg);
+        }
+    }
+
+    /* Having dumped the debug info for the batch, disable autovalidator. */
+    if (batch_failed) {
+        miniflow_extract_func default_func = NULL;
+        atomic_uintptr_t *pmd_func = (void *)&pmd->miniflow_extract_opt;
+        atomic_store_relaxed(pmd_func, (uintptr_t) default_func);
+    }
+
+    /* Preserve packet correctness by storing back the good offsets in
+     * packets back. */
+    DP_PACKET_BATCH_FOR_EACH (i, packet, packets) {
+        packet->l2_5_ofs = good_l2_5_ofs[i];
+        packet->l3_ofs = good_l3_ofs[i];
+        packet->l4_ofs = good_l4_ofs[i];
+        packet->l2_pad_size = good_l2_pad_size[i];
+    }
+
+    /* Returning zero implies no packets were hit by autovalidation. This
+     * simplifies unit-tests as changing --enable-mfex-default-autovalidator
+     * would pass/fail. By always returning zero, autovalidator is a little
+     * slower, but we gain consistency in testing. The auto-validator is only
+     * meant to test different implementaions against a batch of packets
+     * without incrementing hit counters.
+     */
+    return 0;
+}
diff --git a/lib/dpif-netdev-private-extract.h b/lib/dpif-netdev-private-extract.h
index 614677dd0..88c517506 100644
--- a/lib/dpif-netdev-private-extract.h
+++ b/lib/dpif-netdev-private-extract.h
@@ -42,6 +42,9 @@ typedef uint32_t (*miniflow_extract_func)(struct dp_packet_batch *batch,
 /* The function pointer miniflow_extract_func depends on batch size. */
 BUILD_ASSERT_DECL(NETDEV_MAX_BURST == 32);
 
+/* Assert if there is flow map units change. */
+BUILD_ASSERT_DECL(FLOWMAP_UNITS == 2);
+
 /* Probe function is used to detect if this CPU has the ISA required
  * to run the optimized miniflow implementation.
  * returns one on successful probe.
@@ -75,12 +78,19 @@ struct dpif_miniflow_extract_impl {
  * should always come before the generic AVX512-F version.
  */
 enum dpif_miniflow_extract_impl_idx {
+    MFEX_IMPL_AUTOVALIDATOR,
     MFEX_IMPL_SCALAR,
     MFEX_IMPL_MAX
 };
 
 extern struct ovs_mutex dp_netdev_mutex;
 
+/* Define a index which points to the first traffic optimized MFEX
+ * option from the enum list else holds max value.
+ */
+
+#define MFEX_IMPL_START_IDX MFEX_IMPL_MAX
+
 /* This function returns all available implementations to the caller. The
  * quantity of implementations is returned by the int return value.
  */
@@ -110,4 +120,16 @@ int dp_mfex_impl_set_default_by_name(const char *name);
 void
 dpif_miniflow_extract_init(void);
 
+/* Retrieve the hitmask of the batch of pakcets which is obtained by comparing
+ * different miniflow implementations with linear miniflow extract.
+ * Key_size need to be at least the size of the batch.
+ * On error, returns a zero.
+ * On success, returns the number of packets in the batch compared.
+ */
+uint32_t
+dpif_miniflow_extract_autovalidator(struct dp_packet_batch *batch,
+                                    struct netdev_flow_key *keys,
+                                    uint32_t keys_size, odp_port_t in_port,
+                                    struct dp_netdev_pmd_thread *pmd_handle);
+
 #endif /* MFEX_AVX512_EXTRACT */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 75c3a3214..707bf85c0 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1136,8 +1136,8 @@ dpif_miniflow_extract_impl_set(struct unixctl_conn *conn, int argc OVS_UNUSED,
     ds_put_format(&reply, "Miniflow Extract implementation set to %s",
                   mfex_name);
     const char *reply_str = ds_cstr(&reply);
-    unixctl_command_reply(conn, reply_str);
     VLOG_INFO("%s", reply_str);
+    unixctl_command_reply(conn, reply_str);
     ds_destroy(&reply);
 }
 
-- 
2.25.1



More information about the dev mailing list