[ovs-dev] [PATCH] dpif-netdev: apply subtable-lookup-prio-set on any datapath

Timothy Redaelli tredaelli at redhat.com
Wed Jun 23 18:54:43 UTC 2021


Currently, if you try to set subtable-lookup-prio-set when you don't have
any datapath (for example if an user wants to set AVX512 before creating
any bridge) it sets it globally (dpcls_subtable_set_prio),
but it returns an error:

  please specify an existing datapath
  ovs-appctl: ovs-vswitchd: server returned an error

and, in this case, the exit code of ovs-appctl is 2.

This commit changes the behaviour by removing the [dp] optional
parameter of subtable-lookup-prio-set and by changing the priority
level on any datapath and globally. This means if you don't have any
datapath or if you have only one datapath, the behaviour is the same as
now, but without the confusing error when you don't have any datapath.

Signed-off-by: Timothy Redaelli <tredaelli at redhat.com>
Fixes: 3d018c3ea79d ("dpif-netdev: add subtable lookup prio set command.")
Cc: harry.van.haaren at intel.com
---
 lib/dpif-netdev.c | 78 +++++++++++++++++++----------------------------
 1 file changed, 32 insertions(+), 46 deletions(-)
---
There is no need to change the documentation, since the manpage is not
merged and it's currently part of another series [1], but the optional
datapath parameter is not present in the patchset and so there is not need
to change it. Currently the only document that contains a reference to
subtable-lookup-prio-set (Documentation/topics/dpdk/bridge.rst),
doesn't contain any reference to the optional datapath parameter too.

[1] https://patchwork.ozlabs.org/project/openvswitch/patch/20210617161825.94741-9-cian.ferriter@intel.com/

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 8fa7eb6d4..478eb7cf3 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1345,13 +1345,15 @@ dpif_netdev_subtable_lookup_set(struct unixctl_conn *conn, int argc,
     /* This function requires 2 parameters (argv[1] and argv[2]) to execute.
      *   argv[1] is subtable name
      *   argv[2] is priority
-     *   argv[3] is the datapath name (optional if only 1 datapath exists)
      */
     const char *func_name = argv[1];
 
     errno = 0;
     char *err_char;
     uint32_t new_prio = strtoul(argv[2], &err_char, 10);
+    uint32_t lookup_dpcls_changed = 0;
+    uint32_t lookup_subtable_changed = 0;
+    struct shash_node *node;
     if (errno != 0 || new_prio > UINT8_MAX) {
         unixctl_command_reply_error(conn,
             "error converting priority, use integer in range 0-255\n");
@@ -1365,58 +1367,42 @@ dpif_netdev_subtable_lookup_set(struct unixctl_conn *conn, int argc,
         return;
     }
 
-    /* argv[3] is optional datapath instance. If no datapath name is provided
-     * and only one datapath exists, the one existing datapath is reprobed.
-     */
     ovs_mutex_lock(&dp_netdev_mutex);
-    struct dp_netdev *dp = NULL;
-
-    if (argc == 4) {
-        dp = shash_find_data(&dp_netdevs, argv[3]);
-    } else if (shash_count(&dp_netdevs) == 1) {
-        dp = shash_first(&dp_netdevs)->data;
-    }
-
-    if (!dp) {
-        ovs_mutex_unlock(&dp_netdev_mutex);
-        unixctl_command_reply_error(conn,
-                                    "please specify an existing datapath");
-        return;
-    }
-
-    /* Get PMD threads list, required to get DPCLS instances. */
-    size_t n;
-    uint32_t lookup_dpcls_changed = 0;
-    uint32_t lookup_subtable_changed = 0;
-    struct dp_netdev_pmd_thread **pmd_list;
-    sorted_poll_thread_list(dp, &pmd_list, &n);
+    SHASH_FOR_EACH(node, &dp_netdevs) {
+        struct dp_netdev *dp = node->data;
 
-    /* take port mutex as HMAP iters over them. */
-    ovs_mutex_lock(&dp->port_mutex);
+        /* Get PMD threads list, required to get DPCLS instances. */
+        size_t n;
+        struct dp_netdev_pmd_thread **pmd_list;
+        sorted_poll_thread_list(dp, &pmd_list, &n);
 
-    for (size_t i = 0; i < n; i++) {
-        struct dp_netdev_pmd_thread *pmd = pmd_list[i];
-        if (pmd->core_id == NON_PMD_CORE_ID) {
-            continue;
-        }
+        /* take port mutex as HMAP iters over them. */
+        ovs_mutex_lock(&dp->port_mutex);
 
-        struct dp_netdev_port *port = NULL;
-        HMAP_FOR_EACH (port, node, &dp->ports) {
-            odp_port_t in_port = port->port_no;
-            struct dpcls *cls = dp_netdev_pmd_lookup_dpcls(pmd, in_port);
-            if (!cls) {
+        for (size_t i = 0; i < n; i++) {
+            struct dp_netdev_pmd_thread *pmd = pmd_list[i];
+            if (pmd->core_id == NON_PMD_CORE_ID) {
                 continue;
             }
-            uint32_t subtbl_changes = dpcls_subtable_lookup_reprobe(cls);
-            if (subtbl_changes) {
-                lookup_dpcls_changed++;
-                lookup_subtable_changed += subtbl_changes;
+
+            struct dp_netdev_port *port = NULL;
+            HMAP_FOR_EACH (port, node, &dp->ports) {
+                odp_port_t in_port = port->port_no;
+                struct dpcls *cls = dp_netdev_pmd_lookup_dpcls(pmd, in_port);
+                if (!cls) {
+                    continue;
+                }
+                uint32_t subtbl_changes = dpcls_subtable_lookup_reprobe(cls);
+                if (subtbl_changes) {
+                    lookup_dpcls_changed++;
+                    lookup_subtable_changed += subtbl_changes;
+                }
             }
         }
-    }
 
-    /* release port mutex before netdev mutex. */
-    ovs_mutex_unlock(&dp->port_mutex);
+        /* release port mutex before netdev mutex. */
+        ovs_mutex_unlock(&dp->port_mutex);
+    }
     ovs_mutex_unlock(&dp_netdev_mutex);
 
     struct ds reply = DS_EMPTY_INITIALIZER;
@@ -1645,8 +1631,8 @@ dpif_netdev_init(void)
                              0, 1, dpif_netdev_bond_show,
                              NULL);
     unixctl_command_register("dpif-netdev/subtable-lookup-prio-set",
-                             "[lookup_func] [prio] [dp]",
-                             2, 3, dpif_netdev_subtable_lookup_set,
+                             "[lookup_func] [prio]",
+                             2, 2, dpif_netdev_subtable_lookup_set,
                              NULL);
     unixctl_command_register("dpif-netdev/subtable-lookup-prio-get", "",
                              0, 0, dpif_netdev_subtable_lookup_get,
-- 
2.31.1



More information about the dev mailing list