"Eelco Chaudron" <[email protected]> writes: > On 22 Sep 2020, at 15:21, Aaron Conole wrote: > >> Eelco Chaudron <[email protected]> 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 <[email protected]> >>> --- >> >> It may be worth mentioning this in NEWS. > > Good suggestion, will send out a v2 later. > >> >> Also, 'ovs-ofctl add-flows' is subtly different - there is the >> possibility to do a single transaction with 'ovs-ofctl add-flows', >> but this corresponding command cannot do that. I guess that might be >> what is intended by: >> >> All flow mods are executed in the order specified. >> >> in the manpage. > > Not sure what you mean with single transaction? The following does work: > > echo "in_port(0),eth(),eth_type(0x800),ipv4(src=100.1.0.2) 0" | > ovs-dpctl add-flows test -
IIRC, all the flow rules can be added as part of a single openflow transaction with ovs-ofctl. So multiple flow rules will be committed at exactly the same time (--bundle, etc). This command will only ever do 1 at a time. That's okay, but maybe it's worth pointing out this difference. >> Acked-by: Aaron Conole <[email protected]> >> >>> lib/dpctl.c | 179 >>> ++++++++++++++++++++++++++++++++++++++++++++----- >>> lib/dpctl.man | 11 +++ >>> utilities/ovs-dpctl.c | 5 + >>> 3 files changed, 175 insertions(+), 20 deletions(-) >>> >>> diff --git a/lib/dpctl.c b/lib/dpctl.c >>> index db2b1f896..eba2bdfa2 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) { >>> @@ -1185,6 +1185,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; >>> } >>> @@ -1258,25 +1276,20 @@ 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_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) { >>> @@ -1334,6 +1347,23 @@ 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; >>> } >>> @@ -1356,6 +1386,118 @@ dpctl_del_flows(int argc, const char >>> *argv[], struct dpctl_params *dpctl_p) >>> return error; >>> } >>> >>> +static int >>> +dpctl_parse_add_flows_line(struct ds *s, char **flow, char **action) >>> +{ >>> + int command = DPCTL_FLOWS_ADD; >>> + 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_add_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; >>> + >>> + 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_add_flows_line(&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_help(int argc OVS_UNUSED, const char *argv[] OVS_UNUSED, >>> struct dpctl_params *dpctl_p) >>> @@ -2506,6 +2648,7 @@ static const struct dpctl_command >>> all_commands[] = { >>> { "dump-flows", "[dp] [filter=..] [type=..]", >>> 0, 3, dpctl_dump_flows, DP_RO }, >>> { "add-flow", "[dp] flow actions", 2, 3, dpctl_add_flow, DP_RW >>> }, >>> + { "add-flows", "[dp] file", 1, 2, dpctl_add_flows, DP_RW }, >>> { "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 }, >>> diff --git a/lib/dpctl.man b/lib/dpctl.man >>> index 727d1f7be..e384e00da 100644 >>> --- a/lib/dpctl.man >>> +++ b/lib/dpctl.man >>> @@ -194,6 +194,17 @@ ovs-dpctl add-flow myDP \\ >>> . >>> .RE >>> .TP >>> +.DO "\*(DX\fBadd\-flows\fR [\fIdp\fR] \fIfile\fR" >>> +Reads flow entries from \fIfile\fR (or \fBstdin\fR if \fIfile\fR is >>> +\fB\-\fR) and adds 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 as a flow add. All flow mods are >>> +executed 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/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c >>> index 7d99607f4..3ff3f714c 100644 >>> --- a/utilities/ovs-dpctl.c >>> +++ b/utilities/ovs-dpctl.c >>> @@ -191,6 +191,7 @@ 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" >>> " get-flow [DP] ufid:UFID fetch flow corresponding >>> to UFID\n" >>> " del-flow [DP] FLOW delete FLOW from DP\n" >>> @@ -205,8 +206,8 @@ 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, >>> del-flow\n" >>> + "and del-flows, DP is optional if there is only one >>> datapath.\n", >>> program_name, program_name); >>> vlog_usage(); >>> printf("\nOptions for show and mod-flow:\n" >>> >>> _______________________________________________ >>> dev mailing list >>> [email protected] >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
