[ovs-dev] [PATCH v3] dpctl: add add/mod/del-flows command to dpctl

Eelco Chaudron echaudro at redhat.com
Tue Oct 13 08:36:48 UTC 2020


When you would like to add, modify, or delete a lot of flows in the
datapath, for example when you want to measure performance, adding
one flow at the time won't scale. This as it takes a decent amount
of time to set up the datapath connection.

This new command is in-line with the same command available in
ovs-ofctl which allows the same thing, with the only difference that
we do not verify all lines before we start execution. This allows for
a continuous add/delete stream. For example with a command like this:

python3 -c 'while True:
  for i in range(0, 1000):
    print("add in_port(0),eth(),eth_type(0x800),ipv4(src=100.1.{}.{}) 1".format(int(i / 256), i % 256))
  for i in range(0, 1000):
    print("delete in_port(0),eth(),eth_type(0x800),ipv4(src=100.1.{}.{})".format(int(i / 256), i % 256))' \
|  sudo utilities/ovs-dpctl add-flows -


Signed-off-by: Eelco Chaudron <echaudro at redhat.com>
---
v3: - Fixed NEWS section
    - Added/update mod-flows/del-flows command to support FILE input
    - Added some selftests
v2: - Added change to NEWS
    - Updated man page to be more clear

 NEWS                  |    3 +
 lib/dpctl.c           |  195 ++++++++++++++++++++++++++++++++++++++++++++-----
 lib/dpctl.man         |   16 ++++
 tests/dpctl.at        |   50 +++++++++++++
 utilities/ovs-dpctl.c |   10 ++-
 5 files changed, 251 insertions(+), 23 deletions(-)

diff --git a/NEWS b/NEWS
index 4619e73bf..fed222765 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,9 @@ Post-v2.14.0
      * Removed support for vhost-user dequeue zero-copy.
    - The environment variable OVS_UNBOUND_CONF, if set, is now used
      as the DNS resolver's (unbound) configuration file.
+  - ovs-dpctl and 'ovs-appctl dpctl/':
+    * New commands where added, which allow adding, deleting, or modifying
+      flows based on information read from a file.
 
 
 v2.14.0 - 17 Aug 2020
diff --git a/lib/dpctl.c b/lib/dpctl.c
index 2f859a753..6fdcfd26f 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -52,6 +52,12 @@
 #include "openvswitch/ofp-flow.h"
 #include "openvswitch/ofp-port.h"
 
+enum {
+    DPCTL_FLOWS_ADD = 0,
+    DPCTL_FLOWS_DEL,
+    DPCTL_FLOWS_MOD
+};
+
 typedef int dpctl_command_handler(int argc, const char *argv[],
                                   struct dpctl_params *);
 struct dpctl_command {
@@ -1102,28 +1108,22 @@ out_free:
 }
 
 static int
-dpctl_put_flow(int argc, const char *argv[], enum dpif_flow_put_flags flags,
-               struct dpctl_params *dpctl_p)
+dpctl_put_flow_dpif(struct dpif *dpif, const char *key_s,
+                    const char *actions_s,
+                    enum dpif_flow_put_flags flags,
+                    struct dpctl_params *dpctl_p)
 {
-    const char *key_s = argv[argc - 2];
-    const char *actions_s = argv[argc - 1];
     struct dpif_flow_stats stats;
     struct dpif_port dpif_port;
     struct dpif_port_dump port_dump;
     struct ofpbuf actions;
     struct ofpbuf key;
     struct ofpbuf mask;
-    struct dpif *dpif;
     ovs_u128 ufid;
     bool ufid_present;
     struct simap port_names;
     int n, error;
 
-    error = opt_dpif_open(argc, argv, dpctl_p, 4, &dpif);
-    if (error) {
-        return error;
-    }
-
     ufid_present = false;
     n = odp_ufid_from_string(key_s, &ufid);
     if (n < 0) {
@@ -1195,6 +1195,24 @@ out_freeactions:
 out_freekeymask:
     ofpbuf_uninit(&mask);
     ofpbuf_uninit(&key);
+    return error;
+}
+
+static int
+dpctl_put_flow(int argc, const char *argv[], enum dpif_flow_put_flags flags,
+               struct dpctl_params *dpctl_p)
+{
+    struct dpif *dpif;
+    int error;
+
+    error = opt_dpif_open(argc, argv, dpctl_p, 4, &dpif);
+    if (error) {
+        return error;
+    }
+
+    error = dpctl_put_flow_dpif(dpif, argv[argc - 2], argv[argc - 1], flags,
+                                dpctl_p);
+
     dpif_close(dpif);
     return error;
 }
@@ -1268,26 +1286,21 @@ out:
 }
 
 static int
-dpctl_del_flow(int argc, const char *argv[], struct dpctl_params *dpctl_p)
+dpctl_del_flow_dpif(struct dpif *dpif, const char *key_s,
+                    struct dpctl_params *dpctl_p)
 {
-    const char *key_s = argv[argc - 1];
     struct dpif_flow_stats stats;
     struct dpif_port dpif_port;
     struct dpif_port_dump port_dump;
     struct ofpbuf key;
     struct ofpbuf mask; /* To be ignored. */
-    struct dpif *dpif;
+
     ovs_u128 ufid;
     bool ufid_generated;
     bool ufid_present;
     struct simap port_names;
     int n, error;
 
-    error = opt_dpif_open(argc, argv, dpctl_p, 3, &dpif);
-    if (error) {
-        return error;
-    }
-
     ufid_present = false;
     n = odp_ufid_from_string(key_s, &ufid);
     if (n < 0) {
@@ -1353,16 +1366,156 @@ out:
     ofpbuf_uninit(&mask);
     ofpbuf_uninit(&key);
     simap_destroy(&port_names);
+    return error;
+}
+
+static int
+dpctl_del_flow(int argc, const char *argv[], struct dpctl_params *dpctl_p)
+{
+    const char *key_s = argv[argc - 1];
+    struct dpif *dpif;
+    int error;
+
+    error = opt_dpif_open(argc, argv, dpctl_p, 3, &dpif);
+    if (error) {
+        return error;
+    }
+
+    error = dpctl_del_flow_dpif(dpif, key_s, dpctl_p);
+
     dpif_close(dpif);
     return error;
 }
 
+static int
+dpctl_parse_flow_line(int command, struct ds *s, char **flow, char **action)
+{
+    const char *line = ds_cstr(s);
+    size_t len;
+
+    /* First figure out the command, or fallback to FLOWS_ADD. */
+    line += strspn(line, " \t\r\n");
+    len = strcspn(line, ", \t\r\n");
+
+    if (!strncmp(line, "add", len)) {
+         command = DPCTL_FLOWS_ADD;
+    } else if (!strncmp(line, "delete", len)) {
+        command = DPCTL_FLOWS_DEL;
+    } else if (!strncmp(line, "modify", len)) {
+        command = DPCTL_FLOWS_MOD;
+    } else {
+        len = 0;
+    }
+    line += len;
+
+    /* Isolate flow and action (for add/modify). */
+    line += strspn(line, " \t\r\n");
+    len = strcspn(line, " \t\r\n");
+
+    if (len == 0) {
+        *flow = NULL;
+        *action = NULL;
+        return command;
+    }
+
+    *flow = strndup(line, len);
+
+    line += len;
+    line += strspn(line, " \t\r\n");
+    if (strlen(line)) {
+        *action = xstrdup(line);
+    } else {
+        *action = NULL;
+    }
+
+    return command;
+}
+
+static int
+dpctl_process_flows(int argc, const char *argv[], struct dpctl_params *dpctl_p)
+{
+    const char *file_name = argv[argc - 1];
+    int line_number = 0;
+    struct dpif *dpif;
+    struct ds line;
+    FILE *stream;
+    int error;
+    int def_cmd = DPCTL_FLOWS_ADD;
+
+    if (strstr(argv[0], "mod-flows")) {
+        def_cmd = DPCTL_FLOWS_MOD;
+    } else if (strstr(argv[0], "del-flows")) {
+        def_cmd = DPCTL_FLOWS_DEL;
+    }
+
+    error = opt_dpif_open(argc, argv, dpctl_p, 4, &dpif);
+    if (error) {
+        return error;
+    }
+
+    stream = !strcmp(file_name, "-") ? stdin : fopen(file_name, "r");
+    if (!stream) {
+        error = errno;
+        dpctl_error(dpctl_p, error, "Opening file \"%s\" failed", file_name);
+        goto out_close_dpif;
+    }
+
+    ds_init(&line);
+    while (!ds_get_preprocessed_line(&line, stream, &line_number)) {
+        /* We do not process all the lines first and then execute the actions
+         * as we would like to take commands as a continuous stream of
+         * commands from stdin.
+         */
+        char *flow = NULL;
+        char *action = NULL;
+        int cmd = dpctl_parse_flow_line(def_cmd, &line, &flow, &action);
+
+        if ((!flow && !action)
+            || ((cmd == DPCTL_FLOWS_ADD || cmd == DPCTL_FLOWS_MOD) && !action)
+            || (cmd == DPCTL_FLOWS_DEL && action)) {
+            dpctl_error(dpctl_p, 0,
+                        "Failed parsing line number %u, skipped!",
+                        line_number);
+        } else {
+            switch (cmd) {
+            case DPCTL_FLOWS_ADD:
+                dpctl_put_flow_dpif(dpif, flow, action,
+                                    DPIF_FP_CREATE, dpctl_p);
+                break;
+            case DPCTL_FLOWS_MOD:
+                dpctl_put_flow_dpif(dpif, flow, action,
+                                    DPIF_FP_MODIFY, dpctl_p);
+                break;
+            case DPCTL_FLOWS_DEL:
+                dpctl_del_flow_dpif(dpif, flow, dpctl_p);
+                break;
+            }
+        }
+
+        free(flow);
+        free(action);
+    }
+
+    ds_destroy(&line);
+    if (stream != stdin) {
+        fclose(stream);
+    }
+out_close_dpif:
+    dpif_close(dpif);
+    return 0;
+}
+
 static int
 dpctl_del_flows(int argc, const char *argv[], struct dpctl_params *dpctl_p)
 {
     struct dpif *dpif;
+    int error;
 
-    int error = opt_dpif_open(argc, argv, dpctl_p, 2, &dpif);
+    if ((!dp_arg_exists(argc, argv) && argc == 2) || argc > 2) {
+        return dpctl_process_flows(argc, argv, dpctl_p);
+    }
+
+    error = opt_dpif_open(argc, argv, dpctl_p, 2, &dpif);
     if (error) {
         return error;
     }
@@ -2528,7 +2681,9 @@ static const struct dpctl_command all_commands[] = {
     { "mod-flow", "[dp] flow actions", 2, 3, dpctl_mod_flow, DP_RW },
     { "get-flow", "[dp] ufid", 1, 2, dpctl_get_flow, DP_RO },
     { "del-flow", "[dp] flow", 1, 2, dpctl_del_flow, DP_RW },
-    { "del-flows", "[dp]", 0, 1, dpctl_del_flows, DP_RW },
+    { "add-flows", "[dp] file", 1, 2, dpctl_process_flows, DP_RW },
+    { "mod-flows", "[dp] file", 1, 2, dpctl_process_flows, DP_RW },
+    { "del-flows", "[dp] [file]", 0, 2, dpctl_del_flows, DP_RW },
     { "dump-conntrack", "[dp] [zone=N]", 0, 2, dpctl_dump_conntrack, DP_RO },
     { "flush-conntrack", "[dp] [zone=N] [ct-tuple]", 0, 3,
       dpctl_flush_conntrack, DP_RW },
diff --git a/lib/dpctl.man b/lib/dpctl.man
index 727d1f7be..50f7379a8 100644
--- a/lib/dpctl.man
+++ b/lib/dpctl.man
@@ -194,6 +194,22 @@ ovs-dpctl add-flow myDP \\
 .
 .RE
 .TP
+\*(DX\fBadd\-flows\fR [\fIdp\fR] \fIfile\fR
+.TQ
+\*(DX\fBmod\-flows\fR [\fIdp\fR] \fIfile\fR
+.TQ
+\*(DX\fBdel\-flows\fR [\fIdp\fR] \fIfile\fR
+Reads flow entries from \fIfile\fR (or \fBstdin\fR if \fIfile\fR is
+\fB\-\fR) and adds, modifies, or deletes each entry to the datapath.
+.
+Each flow specification (e.g., each line in \fIfile\fR) may start with
+\fBadd\fR, \fBmodify\fR, or \fBdelete\fR keyword to specify whether a
+flow is to be added, modified, or deleted. A flow specification without
+one of these keywords is treated based on the used command.  All flow
+modifications are executed as individual transactions in the order
+specified.
+.
+.TP
 .DO "[\fB\-s\fR | \fB\-\-statistics\fR]" "\*(DX\fBdel\-flow\fR" "[\fIdp\fR] \fIflow\fR"
 Deletes the flow from \fIdp\fR's flow table that matches \fIflow\fR.
 If \fB\-s\fR or \fB\-\-statistics\fR is specified, then
diff --git a/tests/dpctl.at b/tests/dpctl.at
index deec54959..7454a51ec 100644
--- a/tests/dpctl.at
+++ b/tests/dpctl.at
@@ -85,3 +85,53 @@ OVS_VSWITCHD_STOP(["/dummy at br0: port_del failed/d
 /dummy at br0: failed to add vif1.0 as port/d
 /Dropped 1 log messages in last/d"])
 AT_CLEANUP
+
+AT_SETUP([dpctl - add/mod/del-flows])
+OVS_VSWITCHD_START
+AT_CHECK([ovs-appctl dpctl/add-dp dummy at br0])
+AT_DATA([flows.txt], [dnl
+in_port(1),eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:02),eth_type(0x1234) 2
+])
+AT_CHECK([ovs-appctl dpctl/add-flows dummy at br0 flows.txt], [0], [dnl
+])
+AT_CHECK([ovs-appctl dpctl/dump-flows dummy at br0 | sort], [0], [dnl
+flow-dump from the main thread:
+recirc_id(0),in_port(1),eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:02),eth_type(0x1234), packets:0, bytes:0, used:never, actions:2
+])
+AT_DATA([flows.txt], [dnl
+in_port(1),eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:02),eth_type(0x1234) 3
+])
+AT_CHECK([ovs-appctl dpctl/mod-flows dummy at br0 flows.txt], [0], [dnl
+])
+AT_CHECK([ovs-appctl dpctl/dump-flows dummy at br0 | sort], [0], [dnl
+flow-dump from the main thread:
+recirc_id(0),in_port(1),eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:02),eth_type(0x1234), packets:0, bytes:0, used:never, actions:3
+])
+AT_DATA([flows.txt], [dnl
+in_port(1),eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:02),eth_type(0x1234)
+])
+AT_CHECK([ovs-appctl dpctl/del-flows dummy at br0 flows.txt], [0], [dnl
+])
+AT_CHECK([ovs-appctl dpctl/dump-flows dummy at br0 | sort], [0], [dnl
+])
+AT_DATA([flows.txt], [dnl
+add in_port(1),eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:02),eth_type(0x1234) 2
+add in_port(1),eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:03),eth_type(0x1234) 2
+add in_port(1),eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:04),eth_type(0x1234) 2
+modify in_port(1),eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:02),eth_type(0x1234) 1
+delete in_port(1),eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:03),eth_type(0x1234)
+])
+AT_CHECK([ovs-appctl dpctl/add-flows dummy at br0 flows.txt], [0], [dnl
+])
+AT_CHECK([ovs-appctl dpctl/dump-flows dummy at br0 | sort], [0], [dnl
+flow-dump from the main thread:
+recirc_id(0),in_port(1),eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:02),eth_type(0x1234), packets:0, bytes:0, used:never, actions:1
+recirc_id(0),in_port(1),eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:04),eth_type(0x1234), packets:0, bytes:0, used:never, actions:2
+])
+AT_CHECK([ovs-appctl dpctl/del-flows dummy at br0], [0], [dnl
+])
+AT_CHECK([ovs-appctl dpctl/dump-flows dummy at br0 | sort], [0], [dnl
+])
+AT_CHECK([ovs-appctl dpctl/del-dp dummy at br0])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c
index 7d99607f4..f616995c3 100644
--- a/utilities/ovs-dpctl.c
+++ b/utilities/ovs-dpctl.c
@@ -191,10 +191,13 @@ usage(void *userdata OVS_UNUSED)
            "  show DP...               show basic info on each DP\n"
            "  dump-flows [DP]          display flows in DP\n"
            "  add-flow [DP] FLOW ACTIONS add FLOW with ACTIONS to DP\n"
+           "  add-flows [DP] FILE        add flows from FILE\n"
            "  mod-flow [DP] FLOW ACTIONS change FLOW actions to ACTIONS in DP\n"
+           "  mod-flows [DP] FILE        change flows from FILE\n"
            "  get-flow [DP] ufid:UFID    fetch flow corresponding to UFID\n"
            "  del-flow [DP] FLOW         delete FLOW from DP\n"
-           "  del-flows [DP]             delete all flows from DP\n"
+           "  del-flows [DP] [FILE]      " \
+               "delete all or specified flows from DP\n"
            "  dump-conntrack [DP] [zone=ZONE]  " \
                "display conntrack entries for ZONE\n"
            "  flush-conntrack [DP] [zone=ZONE] [ct-tuple]" \
@@ -205,8 +208,9 @@ usage(void *userdata OVS_UNUSED)
            "Each IFACE on add-dp, add-if, and set-if may be followed by\n"
            "comma-separated options.  See ovs-dpctl(8) for syntax, or the\n"
            "Interface table in ovs-vswitchd.conf.db(5) for an options list.\n"
-           "For COMMAND dump-flows, add-flow, mod-flow, del-flow and\n"
-           "del-flows, DP is optional if there is only one datapath.\n",
+           "For COMMAND dump-flows, add-flow, add-flows, mod-flow,\n"
+           "mod-flows, del-flow and del-flows, DP is optional if there is\n"
+           "only one datapath.\n",
            program_name, program_name);
     vlog_usage();
     printf("\nOptions for show and mod-flow:\n"



More information about the dev mailing list