dpif-netdev: Add packet count and core id paramters for study

This commit introduces additional command line paramter
for mfex study function. If user provides additional packet out
it is used in study to compare minimum packets which must be processed
else a default value is choosen.
Also introduces a third paramter for choosing a particular pmd core.

$ ovs-appctl dpif-netdev/miniflow-parser-set study 500 3

Signed-off-by: Kumar Amber <kumar.amber@intel.com>
Acked-by: Flavio Leitner <fbl@sysclose.org>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>

4 files changed, 221 insertions, 32 deletions

diff --git a/Documentation/topics/dpdk/bridge.rst b/Documentation/topics/dpdk/bridge.rst
index a47153495..8c500c504 100644
--- a/Documentation/topics/dpdk/bridge.rst
+++ b/Documentation/topics/dpdk/bridge.rst
@@ -284,12 +284,46 @@ command also shows whether the CPU supports each implementation ::
 
 An implementation can be selected manually by the following command ::
 
-    $ ovs-appctl dpif-netdev/miniflow-parser-set study
+    $ ovs-appctl dpif-netdev/miniflow-parser-set [-pmd core_id] [name]
+                                                 [study_cnt]
+
+The above command has two optional parameters: study_cnt and core_id.
+The core_id sets a particular miniflow extract function to a specific
+pmd thread on the core. The third parameter study_cnt, which is specific
+to study and ignored by other implementations, means how many packets
+are needed to choose the best implementation.
 
 Also user can select the study implementation which studies the traffic for
 a specific number of packets by applying all available implementations of
 miniflow extract and then chooses the one with the most optimal result for
-that traffic pattern.
+that traffic pattern. The user can optionally provide an packet count
+[study_cnt] parameter which is the minimum number of packets that OVS must
+study before choosing an optimal implementation. If no packet count is
+provided, then the default value, 128 is chosen. Also, as there is no
+synchronization point between threads, one PMD thread might still be running
+a previous round, and can now decide on earlier data.
+
+The per packet count is a global value, and parallel study executions with
+differing packet counts will use the most recent count value provided by user.
+
+Study can be selected with packet count by the following command ::
+
+    $ ovs-appctl dpif-netdev/miniflow-parser-set study 1024
+
+Study can be selected with packet count and explicit PMD selection
+by the following command ::
+
+    $ ovs-appctl dpif-netdev/miniflow-parser-set -pmd 3 study 1024
+
+In the above command the first parameter is the CORE ID of the PMD
+thread and this can also be used to explicitly set the miniflow
+extraction function pointer on different PMD threads.
+
+Scalar can be selected on core 3 by the following command where
+study count should not be provided for any implementation other
+than study ::
+
+    $ ovs-appctl dpif-netdev/miniflow-parser-set -pmd 3 scalar
 
 Miniflow Extract Validation
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/lib/dpif-netdev-extract-study.c b/lib/dpif-netdev-extract-study.c
index f28cd7fcf..5a9370669 100644
--- a/lib/dpif-netdev-extract-study.c
+++ b/lib/dpif-netdev-extract-study.c
@@ -25,7 +25,7 @@
 
 VLOG_DEFINE_THIS_MODULE(dpif_mfex_extract_study);
 
-static atomic_uint32_t mfex_study_pkts_count = 0;
+static atomic_uint32_t mfex_study_pkts_count = MFEX_MAX_PKT_COUNT;
 
 /* Struct to hold miniflow study stats. */
 struct study_stats {
@@ -48,6 +48,25 @@ mfex_study_get_study_stats_ptr(void)
     return stats;
 }
 
+int
+mfex_set_study_pkt_cnt(uint32_t pkt_cmp_count, const char *name)
+{
+    struct dpif_miniflow_extract_impl *miniflow_funcs;
+    miniflow_funcs = dpif_mfex_impl_info_get();
+
+    /* If the packet count is set and implementation called is study then
+     * set packet counter to requested number else return -EINVAL.
+     */
+    if ((strcmp(miniflow_funcs[MFEX_IMPL_STUDY].name, name) == 0) &&
+        (pkt_cmp_count != 0)) {
+
+        atomic_store_relaxed(&mfex_study_pkts_count, pkt_cmp_count);
+        return 0;
+    }
+
+    return -EINVAL;
+}
+
 uint32_t
 mfex_study_traffic(struct dp_packet_batch *packets,
                    struct netdev_flow_key *keys,
@@ -86,7 +105,10 @@ mfex_study_traffic(struct dp_packet_batch *packets,
     /* Choose the best implementation after a minimum packets have been
      * processed.
      */
-    if (stats->pkt_count >= MFEX_MAX_PKT_COUNT) {
+    uint32_t study_cnt_pkts;
+    atomic_read_relaxed(&mfex_study_pkts_count, &study_cnt_pkts);
+
+    if (stats->pkt_count >= study_cnt_pkts) {
         uint32_t best_func_index = MFEX_IMPL_START_IDX;
         uint32_t max_hits = 0;
         for (int i = MFEX_IMPL_START_IDX; i < MFEX_IMPL_MAX; i++) {
diff --git a/lib/dpif-netdev-private-extract.h b/lib/dpif-netdev-private-extract.h
index 9c03a1aa0..b93fecbbc 100644
--- a/lib/dpif-netdev-private-extract.h
+++ b/lib/dpif-netdev-private-extract.h
@@ -151,4 +151,13 @@ mfex_study_traffic(struct dp_packet_batch *packets,
                    uint32_t keys_size, odp_port_t in_port,
                    struct dp_netdev_pmd_thread *pmd_handle);
 
+/* Sets the packet count from user to the stats for use in
+ * study function to match against the classified packets to choose
+ * the optimal implementation.
+ * On error, returns -EINVAL.
+ * On success, returns 0.
+ */
+int
+mfex_set_study_pkt_cnt(uint32_t pkt_cmp_count, const char *name);
+
 #endif /* MFEX_AVX512_EXTRACT */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 707bf85c0..21d7714cc 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1076,36 +1076,132 @@ dpif_miniflow_extract_impl_get(struct unixctl_conn *conn, int argc OVS_UNUSED,
 }
 
 static void
-dpif_miniflow_extract_impl_set(struct unixctl_conn *conn, int argc OVS_UNUSED,
+dpif_miniflow_extract_impl_set(struct unixctl_conn *conn, int argc,
                                const char *argv[], void *aux OVS_UNUSED)
 {
-    /* This function requires just one parameter, the miniflow name.
+    /* This command takes some optional and mandatory arguments. The function
+     * here first parses all of the options, saving results in local variables.
+     * Then the parsed values are acted on.
      */
-    const char *mfex_name = argv[1];
+    unsigned int pmd_thread_to_change = NON_PMD_CORE_ID;
+    unsigned int study_count = MFEX_MAX_PKT_COUNT;
+    struct ds reply = DS_EMPTY_INITIALIZER;
+    bool pmd_thread_update_done = false;
+    bool mfex_name_is_study = false;
+    const char *mfex_name = NULL;
+    const char *reply_str = NULL;
     struct shash_node *node;
+    int err;
+
+    while (argc > 1) {
+        /* Optional argument "-pmd" limits the commands actions to just this
+         * PMD thread.
+         */
+        if ((!strcmp(argv[1], "-pmd") && !mfex_name)) {
+            if (argc < 3) {
+                ds_put_format(&reply,
+                              "Error: -pmd option requires a thread id"
+                              " argument.\n");
+                goto error;
+            }
+
+            /* Ensure argument can be parsed to an integer. */
+            if (!str_to_uint(argv[2], 10, &pmd_thread_to_change) ||
+                (pmd_thread_to_change == NON_PMD_CORE_ID)) {
+                ds_put_format(&reply,
+                              "Error: miniflow extract parser not changed,"
+                              " PMD thread passed is not valid: '%s'."
+                              " Pass a valid pmd thread ID.\n",
+                              argv[2]);
+                goto error;
+            }
+
+            argc -= 2;
+            argv += 2;
+
+        } else if (!mfex_name) {
+            /* Name of MFEX impl requested by user. */
+            mfex_name = argv[1];
+            mfex_name_is_study = strcmp("study", mfex_name) == 0;
+            argc -= 1;
+            argv += 1;
+
+        /* If name is study and more args exist, parse study_count value. */
+        } else if (mfex_name && mfex_name_is_study) {
+            if (!str_to_uint(argv[1], 10, &study_count) ||
+                (study_count == 0)) {
+                ds_put_format(&reply,
+                              "Error: invalid study_pkt_cnt value: %s.\n",
+                              argv[1]);
+                goto error;
+            }
+
+            argc -= 1;
+            argv += 1;
+        } else {
+            ds_put_format(&reply, "Error: unknown argument %s.\n", argv[1]);
+            goto error;
+            break;
+        }
+    }
+
+    /* Ensure user passed an MFEX name. */
+    if (!mfex_name) {
+        ds_put_format(&reply, "Error: no miniflow extract name provided."
+                      " Output of miniflow-parser-get shows implementation"
+                      " list.\n");
+        goto error;
+    }
 
-    int err = dp_mfex_impl_set_default_by_name(mfex_name);
+    /* If the MFEX name is "study", set the study packet count. */
+    if (mfex_name_is_study) {
+        err = mfex_set_study_pkt_cnt(study_count, mfex_name);
+        if (err) {
+            ds_put_format(&reply, "Error: failed to set study count %d for"
+                          " miniflow extract implementation %s.\n",
+                          study_count, mfex_name);
+            goto error;
+        }
+    }
 
+    /* Set the default MFEX impl only if the command was applied to all PMD
+     * threads. If a PMD thread was selected, do NOT update the default.
+     */
+    if (pmd_thread_to_change == NON_PMD_CORE_ID) {
+        err = dp_mfex_impl_set_default_by_name(mfex_name);
+        if (err == -ENODEV) {
+            ds_put_format(&reply,
+                          "Error: miniflow extract not available due to CPU"
+                          " ISA requirements: %s",
+                          mfex_name);
+            goto error;
+        } else if (err) {
+            ds_put_format(&reply,
+                          "Error: unknown miniflow extract implementation %s.",
+                          mfex_name);
+            goto error;
+        }
+    }
+
+    /* Get the desired MFEX function pointer and error check its usage. */
+    miniflow_extract_func mfex_func = NULL;
+    err = dp_mfex_impl_get_by_name(mfex_name, &mfex_func);
     if (err) {
-        struct ds reply = DS_EMPTY_INITIALIZER;
-        char *error_desc = NULL;
-        if (err == -EINVAL) {
-            error_desc = "Unknown miniflow extract implementation:";
-        } else if (err == -ENOENT) {
-            error_desc = "Miniflow extract implementation doesn't exist:";
-        } else if (err == -ENODEV) {
-            error_desc = "Miniflow extract implementation not available:";
+        if (err == -ENODEV) {
+            ds_put_format(&reply,
+                          "Error: miniflow extract not available due to CPU"
+                          " ISA requirements: %s", mfex_name);
         } else {
-            error_desc = "Miniflow extract implementation Error:";
+            ds_put_format(&reply,
+                          "Error: unknown miniflow extract implementation %s.",
+                          mfex_name);
         }
-        ds_put_format(&reply, "%s %s.\n", error_desc, mfex_name);
-        const char *reply_str = ds_cstr(&reply);
-        unixctl_command_reply_error(conn, reply_str);
-        VLOG_INFO("%s", reply_str);
-        ds_destroy(&reply);
-        return;
+        goto error;
     }
 
+    /* Apply the MFEX pointer to each pmd thread in each netdev, filtering
+     * by the users "-pmd" argument if required.
+     */
     ovs_mutex_lock(&dp_netdev_mutex);
 
     SHASH_FOR_EACH (node, &dp_netdevs) {
@@ -1114,7 +1210,6 @@ dpif_miniflow_extract_impl_set(struct unixctl_conn *conn, int argc OVS_UNUSED,
         size_t n;
 
         sorted_poll_thread_list(dp, &pmd_list, &n);
-        miniflow_extract_func default_func = dp_mfex_impl_get_default();
 
         for (size_t i = 0; i < n; i++) {
             struct dp_netdev_pmd_thread *pmd = pmd_list[i];
@@ -1122,23 +1217,51 @@ dpif_miniflow_extract_impl_set(struct unixctl_conn *conn, int argc OVS_UNUSED,
                 continue;
             }
 
-            /* Initialize MFEX function pointer to the newly configured
-             * default. */
+            /* If -pmd specified, skip all other pmd threads. */
+            if ((pmd_thread_to_change != NON_PMD_CORE_ID) &&
+                (pmd->core_id != pmd_thread_to_change)) {
+                continue;
+            }
+
+            pmd_thread_update_done = true;
             atomic_uintptr_t *pmd_func = (void *) &pmd->miniflow_extract_opt;
-            atomic_store_relaxed(pmd_func, (uintptr_t) default_func);
+            atomic_store_relaxed(pmd_func, (uintptr_t) mfex_func);
         };
     }
 
     ovs_mutex_unlock(&dp_netdev_mutex);
 
+    /* If PMD thread was specified, but it wasn't found, return error. */
+    if (pmd_thread_to_change != NON_PMD_CORE_ID && !pmd_thread_update_done) {
+        ds_put_format(&reply,
+                      "Error: miniflow extract parser not changed, "
+                      "PMD thread %d not in use, pass a valid pmd"
+                      " thread ID.\n", pmd_thread_to_change);
+        goto error;
+    }
+
     /* Reply with success to command. */
-    struct ds reply = DS_EMPTY_INITIALIZER;
-    ds_put_format(&reply, "Miniflow Extract implementation set to %s",
+    ds_put_format(&reply, "Miniflow extract implementation set to %s",
                   mfex_name);
-    const char *reply_str = ds_cstr(&reply);
+    if (pmd_thread_to_change != NON_PMD_CORE_ID) {
+        ds_put_format(&reply, ", on pmd thread %d", pmd_thread_to_change);
+    }
+    if (mfex_name_is_study) {
+        ds_put_format(&reply, ", studying %d packets", study_count);
+    }
+    ds_put_format(&reply, ".\n");
+
+    reply_str = ds_cstr(&reply);
     VLOG_INFO("%s", reply_str);
     unixctl_command_reply(conn, reply_str);
     ds_destroy(&reply);
+    return;
+
+error:
+    reply_str = ds_cstr(&reply);
+    VLOG_ERR("%s", reply_str);
+    unixctl_command_reply_error(conn, reply_str);
+    ds_destroy(&reply);
 }
 
 static void
@@ -1371,8 +1494,9 @@ dpif_netdev_init(void)
                              0, 0, dpif_netdev_impl_get,
                              NULL);
     unixctl_command_register("dpif-netdev/miniflow-parser-set",
-                             "miniflow_implementation_name",
-                             1, 1, dpif_miniflow_extract_impl_set,
+                             "[-pmd core] miniflow_implementation_name"
+                             " [study_pkt_cnt]",
+                             1, 5, dpif_miniflow_extract_impl_set,
                              NULL);
     unixctl_command_register("dpif-netdev/miniflow-parser-get", "",
                              0, 0, dpif_miniflow_extract_impl_get,