[ovs-dev] [PATCH v3] dpctl: add add/mod/del-flows command to dpctl
Paolo Valerio
pvalerio at redhat.com
Tue Dec 22 15:16:10 UTC 2020
Eelco Chaudron <echaudro at redhat.com> writes:
> 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.
>
Tested it successfully.
NEWS has conflicts, besides that, it would be nice to add the commands
({add,mod,del}-flows) explicitly in the description for consistency with
the other dpctl entries.
Otherwise,
Acked-by: Paolo Valerio <pvalerio at redhat.com>
>
> 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"
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list