Thanks for the review. I applied this to master.
On Fri, Mar 15, 2013 at 12:56:03PM -0700, Andy Zhou wrote: > This looks good. Thanks for resending it. > > > On Fri, Mar 15, 2013 at 12:32 PM, Ben Pfaff <[email protected]> wrote: > > > Signed-off-by: Ben Pfaff <[email protected]> > > --- > > utilities/ovs-dpctl.8.in | 57 +++++++++++++++----- > > utilities/ovs-dpctl.c | 128 > > ++++++++++++++++++++++++++++++++++++++++++++-- > > 2 files changed, 166 insertions(+), 19 deletions(-) > > > > diff --git a/utilities/ovs-dpctl.8.in b/utilities/ovs-dpctl.8.in > > index b1b2570..2b0036c 100644 > > --- a/utilities/ovs-dpctl.8.in > > +++ b/utilities/ovs-dpctl.8.in > > @@ -104,27 +104,54 @@ port. > > If one or more datapaths are specified, information on only those > > datapaths are displayed. Otherwise, \fBovs\-dpctl\fR displays information > > about all configured datapaths. > > +.SS "DEBUGGING COMMANDS" > > +The following commands are primarily useful for debugging Open > > +vSwitch. The flow table entries (both matches and actions) that they > > +work with are not OpenFlow flow entries. Instead, they are different > > +and considerably simpler flows maintained by the Open vSwitch kernel > > +module. Use \fBovs\-ofctl\fR(8), instead, to work with OpenFlow flow > > +entries. > > +. > > +.PP > > +The \fIdp\fR argument to each of these commands is optional when > > +exactly one datapath exists, in which case that datapath is the > > +default. When multiple datapaths exist, then a datapath name is > > +required. > > . > > .IP "\fBdump\-flows\fR [\fIdp\fR]" > > Prints to the console all flow entries in datapath \fIdp\fR's > > -flow table. If \fIdp\fR is not specified and exactly one datapath > > -exists, the flows for that datapath will be printed. > > +flow table. > > +. > > +.IP "\fBadd\-flow\fR [\fIdp\fR] \fIflow actions\fR" > > +.IQ "[\fB\-\-clear\fR] [\fB\-\-may-create\fR] [\fB\-s\fR | > > \fB\-\-statistics\fR] \fBmod\-flow\fR [\fIdp\fR] \fIflow actions\fR" > > +Adds or modifies a flow in \fIdp\fR's flow table that, when a packet > > +matching \fIflow\fR arrives, causes \fIactions\fR to be executed. > > +.IP > > +The \fBadd\-flow\fR command succeeds only if \fIflow\fR does not > > +already exist in \fIdp\fR. Contrariwise, \fBmod\-flow\fR without > > +\fB\-\-may\-create\fR only modifies the actions for an existing flow. > > +With \fB\-\-may\-create\fR, \fBmod\-flow\fR will add a new flow or > > +modify an existing one. > > .IP > > -This command is primarily useful for debugging Open vSwitch. The flow > > -table entries that it displays are not > > -OpenFlow flow entries. Instead, they are different and considerably > > -simpler flows maintained by the Open vSwitch kernel module. If you wish > > -to see the OpenFlow flow entries, use \fBovs\-ofctl dump\-flows\fR. > > +If \fB\-s\fR or \fB\-\-statistics\fR is specified, then > > +\fBmod\-flows\fR prints the modified flow's statistics. A flow's > > +statistics are the number of packets and bytes that have passed > > +through the flow, the elapsed time since the flow last processed a > > +packet (if ever), and (for TCP flows) the union of the TCP flags > > +processed through the flow. > > +.IP > > +With \fB\-\-clear\fR, \fBmod\-flows\fR zeros out the flow's > > +statistics. The statistics printed if \fB\-s\fR or > > +\fB\-\-statistics\fR is also specified are those from just before > > +clearing the statistics. > > +. > > +.IP "[\fB\-s\fR | \fB\-\-statistics\fR] \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 > > +\fBmod\-flows\fR prints the deleted flow's statistics. > > . > > .IP "\fBdel\-flows\fR [\fIdp\fR]" > > -Deletes all flow entries from datapath \fIdp\fR's flow table. If > > -\fIdp\fR is not specified and exactly one datapath exists, the flows for > > -that datapath will be deleted. > > -.IP > > -This command is primarily useful for debugging Open vSwitch. As > > -discussed in \fBdump\-flows\fR, these entries are > > -not OpenFlow flow entries. By deleting them, the process that set them > > -up may be confused about their disappearance. > > +Deletes all flow entries from datapath \fIdp\fR's flow table. > > . > > .SH OPTIONS > > .IP "\fB\-s\fR" > > diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c > > index 363485b..3b6e6a5 100644 > > --- a/utilities/ovs-dpctl.c > > +++ b/utilities/ovs-dpctl.c > > @@ -51,9 +51,15 @@ > > > > VLOG_DEFINE_THIS_MODULE(dpctl); > > > > -/* -s, --statistics: Print port statistics? */ > > +/* -s, --statistics: Print port/flow statistics? */ > > static bool print_statistics; > > > > +/* --clear: Reset existing statistics to zero when modifying a flow? */ > > +static bool zero_statistics; > > + > > +/* --may-create: Allow mod-flows command to create a new flow? */ > > +static bool may_create; > > + > > /* -m, --more: Output verbosity. > > * > > * So far only undocumented commands honor this option, so we don't > > document > > @@ -79,11 +85,14 @@ static void > > parse_options(int argc, char *argv[]) > > { > > enum { > > - OPT_DUMMY = UCHAR_MAX + 1, > > + OPT_CLEAR = UCHAR_MAX + 1, > > + OPT_MAY_CREATE, > > VLOG_OPTION_ENUMS > > }; > > static struct option long_options[] = { > > {"statistics", no_argument, NULL, 's'}, > > + {"clear", no_argument, NULL, OPT_CLEAR}, > > + {"may-create", no_argument, NULL, OPT_MAY_CREATE}, > > {"more", no_argument, NULL, 'm'}, > > {"timeout", required_argument, NULL, 't'}, > > {"help", no_argument, NULL, 'h'}, > > @@ -107,6 +116,14 @@ parse_options(int argc, char *argv[]) > > print_statistics = true; > > break; > > > > + case OPT_CLEAR: > > + zero_statistics = true; > > + break; > > + > > + case OPT_MAY_CREATE: > > + may_create = true; > > + break; > > + > > case 'm': > > verbosity++; > > break; > > @@ -154,14 +171,20 @@ usage(void) > > " show show basic info on all datapaths\n" > > " 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" > > + " mod-flow DP FLOW ACTIONS change FLOW actions to ACTIONS in > > DP\n" > > + " del-flow DP FLOW delete FLOW from DP\n" > > " del-flows DP delete all flows from DP\n" > > "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", > > program_name, program_name); > > vlog_usage(); > > - printf("\nOptions for show:\n" > > - " -s, --statistics print port statistics\n" > > + printf("\nOptions for show and mod-flow:\n" > > + " -s, --statistics print statistics for port or > > flow\n" > > + "\nOptions for mod-flow:\n" > > + " --may-create create flow if it doesn't > > exist\n" > > + " --clear reset existing stats to zero\n" > > "\nOther options:\n" > > " -t, --timeout=SECS give up after SECS seconds\n" > > " -h, --help display this help message\n" > > @@ -748,6 +771,100 @@ dpctl_dump_flows(int argc, char *argv[]) > > } > > > > static void > > +dpctl_put_flow(int argc, char *argv[], enum dpif_flow_put_flags flags) > > +{ > > + const char *key_s = argv[argc - 2]; > > + const char *actions_s = argv[argc - 1]; > > + struct dpif_flow_stats stats; > > + struct ofpbuf actions; > > + struct ofpbuf key; > > + struct dpif *dpif; > > + char *dp_name; > > + > > + ofpbuf_init(&key, 0); > > + run(odp_flow_key_from_string(key_s, NULL, &key), "parsing flow key"); > > + > > + ofpbuf_init(&actions, 0); > > + run(odp_actions_from_string(actions_s, NULL, &actions), "parsing > > actions"); > > + > > + dp_name = argc == 3 ? xstrdup(argv[1]) : get_one_dp(); > > + run(parsed_dpif_open(dp_name, false, &dpif), "opening datapath"); > > + free(dp_name); > > + > > + run(dpif_flow_put(dpif, flags, > > + key.data, key.size, > > + actions.data, actions.size, > > + print_statistics ? &stats : NULL), > > + "updating flow table"); > > + > > + ofpbuf_uninit(&key); > > + ofpbuf_uninit(&actions); > > + > > + if (print_statistics) { > > + struct ds s; > > + > > + ds_init(&s); > > + dpif_flow_stats_format(&stats, &s); > > + puts(ds_cstr(&s)); > > + ds_destroy(&s); > > + } > > +} > > + > > +static void > > +dpctl_add_flow(int argc, char *argv[]) > > +{ > > + dpctl_put_flow(argc, argv, DPIF_FP_CREATE); > > +} > > + > > +static void > > +dpctl_mod_flow(int argc OVS_UNUSED, char *argv[]) > > +{ > > + enum dpif_flow_put_flags flags; > > + > > + flags = DPIF_FP_MODIFY; > > + if (may_create) { > > + flags |= DPIF_FP_CREATE; > > + } > > + if (zero_statistics) { > > + flags |= DPIF_FP_ZERO_STATS; > > + } > > + > > + dpctl_put_flow(argc, argv, flags); > > +} > > + > > +static void > > +dpctl_del_flow(int argc, char *argv[]) > > +{ > > + const char *key_s = argv[argc - 1]; > > + struct dpif_flow_stats stats; > > + struct ofpbuf key; > > + struct dpif *dpif; > > + char *dp_name; > > + > > + ofpbuf_init(&key, 0); > > + run(odp_flow_key_from_string(key_s, NULL, &key), "parsing flow key"); > > + > > + dp_name = argc == 2 ? xstrdup(argv[1]) : get_one_dp(); > > + run(parsed_dpif_open(dp_name, false, &dpif), "opening datapath"); > > + free(dp_name); > > + > > + run(dpif_flow_del(dpif, > > + key.data, key.size, > > + print_statistics ? &stats : NULL), "deleting flow"); > > + > > + ofpbuf_uninit(&key); > > + > > + if (print_statistics) { > > + struct ds s; > > + > > + ds_init(&s); > > + dpif_flow_stats_format(&stats, &s); > > + puts(ds_cstr(&s)); > > + ds_destroy(&s); > > + } > > +} > > + > > +static void > > dpctl_del_flows(int argc, char *argv[]) > > { > > struct dpif *dpif; > > @@ -1004,6 +1121,9 @@ static const struct command all_commands[] = { > > { "dump-dps", 0, 0, dpctl_dump_dps }, > > { "show", 0, INT_MAX, dpctl_show }, > > { "dump-flows", 0, 1, dpctl_dump_flows }, > > + { "add-flow", 2, 3, dpctl_add_flow }, > > + { "mod-flow", 2, 3, dpctl_mod_flow }, > > + { "del-flow", 1, 2, dpctl_del_flow }, > > { "del-flows", 0, 1, dpctl_del_flows }, > > { "help", 0, INT_MAX, dpctl_help }, > > > > -- > > 1.7.2.5 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > http://openvswitch.org/mailman/listinfo/dev > > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
