| Message ID | 160257809545.175360.13166705201555359545.stgit@ebuild |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v3] dpctl: add add/mod/del-flows command to dpctl | expand |
Eelco Chaudron <echaudro@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@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@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@br0: port_del failed/d > /dummy@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@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@br0 flows.txt], [0], [dnl > +]) > +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@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@br0 flows.txt], [0], [dnl > +]) > +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@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@br0 flows.txt], [0], [dnl > +]) > +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@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@br0 flows.txt], [0], [dnl > +]) > +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@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@br0], [0], [dnl > +]) > +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@br0 | sort], [0], [dnl > +]) > +AT_CHECK([ovs-appctl dpctl/del-dp dummy@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@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 22 Dec 2020, at 16:16, Paolo Valerio wrote: > Eelco Chaudron <echaudro@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@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. Thanks Paolo! > 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. Guess you mean in the NEWS file itself, like: + - ‘ovs-dpctl {add,mod,del}-flows’ and 'ovs-appctl dpctl/{add,mod,del}-flows': + * New commands were added, which allow adding, deleting, or modifying + flows based on information read from a file. Ilya let me know if you need a new revision if this is all that needs changing? > Otherwise, > > Acked-by: Paolo Valerio <pvalerio@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@br0: port_del failed/d >> /dummy@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@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@br0 flows.txt], [0], [dnl >> +]) >> +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@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@br0 flows.txt], [0], [dnl >> +]) >> +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@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@br0 flows.txt], [0], [dnl >> +]) >> +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@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@br0 flows.txt], [0], [dnl >> +]) >> +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@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@br0], [0], [dnl >> +]) >> +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@br0 | sort], [0], [dnl >> +]) >> +AT_CHECK([ovs-appctl dpctl/del-dp dummy@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@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 12/22/20 4:48 PM, Eelco Chaudron wrote: > > > On 22 Dec 2020, at 16:16, Paolo Valerio wrote: > >> Eelco Chaudron <echaudro@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@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. > > Thanks Paolo! > >> 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. > > Guess you mean in the NEWS file itself, like: > > + - ‘ovs-dpctl {add,mod,del}-flows’ and 'ovs-appctl dpctl/{add,mod,del}-flows': > + * New commands were added, which allow adding, deleting, or modifying > + flows based on information read from a file. > > Ilya let me know if you need a new revision if this is all that needs changing? No need for a new version. I slightly modified this text, though. > >> Otherwise, >> >> Acked-by: Paolo Valerio <pvalerio@redhat.com> Thanks! Applied. Best regards, Ilya Maximets.
On 1/5/21 9:20 PM, Ilya Maximets wrote: > On 12/22/20 4:48 PM, Eelco Chaudron wrote: >> >> >> On 22 Dec 2020, at 16:16, Paolo Valerio wrote: >> >>> Eelco Chaudron <echaudro@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@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. >> >> Thanks Paolo! >> >>> 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. >> >> Guess you mean in the NEWS file itself, like: >> >> + - ‘ovs-dpctl {add,mod,del}-flows’ and 'ovs-appctl dpctl/{add,mod,del}-flows': >> + * New commands were added, which allow adding, deleting, or modifying >> + flows based on information read from a file. >> >> Ilya let me know if you need a new revision if this is all that needs changing? > > No need for a new version. I slightly modified this text, though. > >> >>> Otherwise, >>> >>> Acked-by: Paolo Valerio <pvalerio@redhat.com> > > Thanks! Applied. > > Best regards, Ilya Maximets. > Ugh. This patch broke windows build: lib/dpctl.c(1433): error C4013: 'strndup' undefined; assuming extern returning int Eelco, could you, please, take a look. I need to configure appveyor for my account to avoid this kind of issues. Sorry about that. Best regards, Ilya Maximets.
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@br0: port_del failed/d /dummy@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@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@br0 flows.txt], [0], [dnl +]) +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@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@br0 flows.txt], [0], [dnl +]) +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@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@br0 flows.txt], [0], [dnl +]) +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@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@br0 flows.txt], [0], [dnl +]) +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@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@br0], [0], [dnl +]) +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@br0 | sort], [0], [dnl +]) +AT_CHECK([ovs-appctl dpctl/del-dp dummy@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"
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@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(-)