On Thu, May 4, 2023 at 7:08 PM Ilya Maximets <[email protected]> wrote:
>
> On 4/11/23 15:25, Wan Junjie via dev wrote:
> > put dump-meters' result in one line so add-meters can handle.
> > save and restore meters when restart ovs.
> > bundle functions are not implemented in this patch.
> >
> > Signed-off-by: Wan Junjie <[email protected]>
>
> Hi. Thanks for the patch! See some comemnts inline.
>
> Best regards, Ilya Maximets.
>
Hi Ilya,
> >
> > ---
> > v10:
> > merge oneline to verbosity in parse_options
> > ofp_to_string__ handle verbosity bits right
> >
> > v9:
> > fix verbosity mask bits for testcase
> > apologies for mess
> >
> > v8:
> > fix missing code for testcase
> >
> > v7:
> > typo in code
> >
> > v6:
> > code style
> >
> > v5:
> > merge oneline to verbosity higher bits
> > remove duplicate dump_meters code
> >
> > v4:
> > code refactor according to comments
> >
> > v3:
> > add '--oneline' option for dump-meter(s) command
> >
> > v2:
> > fix failed testcases, as dump-meters format changes
> > ---
> > include/openvswitch/ofp-meter.h | 9 +-
> > include/openvswitch/ofp-print.h | 10 +++
> > lib/ofp-meter.c | 100 ++++++++++++++++++++++-
> > lib/ofp-print.c | 20 +++--
> > tests/dpif-netdev.at | 44 ++++++++--
> > utilities/ovs-ofctl.8.in | 25 +++++-
> > utilities/ovs-ofctl.c | 140 ++++++++++++++++++++++++--------
> > utilities/ovs-save | 8 ++
> > 8 files changed, 304 insertions(+), 52 deletions(-)
> >
> > diff --git a/include/openvswitch/ofp-meter.h
> > b/include/openvswitch/ofp-meter.h
> > index 6776eae87..a8ee2d61d 100644
> > --- a/include/openvswitch/ofp-meter.h
> > +++ b/include/openvswitch/ofp-meter.h
> > @@ -17,6 +17,7 @@
> > #ifndef OPENVSWITCH_OFP_METER_H
> > #define OPENVSWITCH_OFP_METER_H 1
> >
> > +#include <stdbool.h>
> > #include "openflow/openflow.h"
> > #include "openvswitch/ofp-protocol.h"
> >
> > @@ -61,7 +62,8 @@ int ofputil_decode_meter_config(struct ofpbuf *,
> > struct ofputil_meter_config *,
> > struct ofpbuf *bands);
> > void ofputil_format_meter_config(struct ds *,
> > - const struct ofputil_meter_config *);
> > + const struct ofputil_meter_config *,
> > + bool oneline);
> >
> > struct ofputil_meter_mod {
> > uint16_t command;
> > @@ -79,6 +81,11 @@ char *parse_ofp_meter_mod_str(struct ofputil_meter_mod
> > *, const char *string,
> > OVS_WARN_UNUSED_RESULT;
> > void ofputil_format_meter_mod(struct ds *, const struct ofputil_meter_mod
> > *);
> >
> > +char *parse_ofp_meter_mod_file(const char *file_name, int command,
> > + struct ofputil_meter_mod **mms, size_t
> > *n_mms,
> > + enum ofputil_protocol *usable_protocols)
> > + OVS_WARN_UNUSED_RESULT;
> > +
> > struct ofputil_meter_stats {
> > uint32_t meter_id;
> > uint32_t flow_count;
> > diff --git a/include/openvswitch/ofp-print.h
> > b/include/openvswitch/ofp-print.h
> > index d76f06872..cd7261c4b 100644
> > --- a/include/openvswitch/ofp-print.h
> > +++ b/include/openvswitch/ofp-print.h
> > @@ -38,6 +38,16 @@ struct dp_packet;
> > extern "C" {
> > #endif
> >
> > +/* manipulate higher bits in verbosity for other usage */
>
> Comments should start with a capital letter and end with a period.
>
> > +#define ONELINE_BIT 7
> > +#define ONELINE_MASK (1 << ONELINE_BIT)
> > +#define VERBOSITY_MASK (~ONELINE_MASK)
> > +
> > +#define VERBOSITY(verbosity) (verbosity & VERBOSITY_MASK)
> > +
> > +#define ONELINE_SET(verbosity) (verbosity | ONELINE_MASK)
> > +#define ONELINE_GET(verbosity) (verbosity & ONELINE_MASK)
>
> This is a public header and the names above are too generic to be safely
> exported. You should add some sort of prefix. e.g. OFP_PRINT_.
>
OK
> > +
> > void ofp_print(FILE *, const void *, size_t, const struct ofputil_port_map
> > *,
> > const struct ofputil_table_map *, int verbosity);
> > void ofp_print_packet(FILE *stream, const void *data,
> > diff --git a/lib/ofp-meter.c b/lib/ofp-meter.c
> > index 9ea40a0bf..6d760620d 100644
> > --- a/lib/ofp-meter.c
> > +++ b/lib/ofp-meter.c
> > @@ -15,6 +15,7 @@
> > */
> >
> > #include <config.h>
> > +#include <errno.h>
> > #include "openvswitch/ofp-meter.h"
> > #include "byte-order.h"
> > #include "nx-match.h"
> > @@ -57,7 +58,7 @@ void
> > ofputil_format_meter_band(struct ds *s, enum ofp13_meter_flags flags,
> > const struct ofputil_meter_band *mb)
> > {
> > - ds_put_cstr(s, "\ntype=");
> > + ds_put_cstr(s, "type=");
> > switch (mb->type) {
> > case OFPMBT13_DROP:
> > ds_put_cstr(s, "drop");
> > @@ -343,7 +344,8 @@ ofp_print_meter_flags(struct ds *s, enum
> > ofp13_meter_flags flags)
> >
> > void
> > ofputil_format_meter_config(struct ds *s,
> > - const struct ofputil_meter_config *mc)
> > + const struct ofputil_meter_config *mc,
> > + bool oneline)
> > {
> > uint16_t i;
> >
> > @@ -354,6 +356,7 @@ ofputil_format_meter_config(struct ds *s,
> >
> > ds_put_cstr(s, "bands=");
> > for (i = 0; i < mc->n_bands; ++i) {
> > + ds_put_cstr(s, oneline ? " ": "\n");
> > ofputil_format_meter_band(s, mc->flags, &mc->bands[i]);
> > }
> > ds_put_char(s, '\n');
> > @@ -578,6 +581,24 @@ parse_ofp_meter_mod_str__(struct ofputil_meter_mod
> > *mm, char *string,
> >
> > /* Meters require at least OF 1.3. */
> > *usable_protocols = OFPUTIL_P_OF13_UP;
> > + if (command == -2) {
> > + size_t len;
> > +
> > + string += strspn(string, " \t\r\n"); /* Skip white space. */
> > + len = strcspn(string, ", \t\r\n"); /* Get length of the first
> > token. */
> > +
> > + if (!strncmp(string, "add", len)) {
> > + command = OFPMC13_ADD;
> > + } else if (!strncmp(string, "delete", len)) {
> > + command = OFPMC13_DELETE;
> > + } else if (!strncmp(string, "modify", len)) {
> > + command = OFPMC13_MODIFY;
> > + } else {
> > + len = 0;
> > + command = OFPMC13_ADD;
> > + }
> > + string += len;
> > + }
> >
> > switch (command) {
> > case -1:
> > @@ -606,6 +627,11 @@ parse_ofp_meter_mod_str__(struct ofputil_meter_mod
> > *mm, char *string,
> > mm->meter.n_bands = 0;
> > mm->meter.bands = NULL;
> >
> > + if (command == OFPMC13_DELETE && string[0] == '\0') {
> > + mm->meter.meter_id = OFPM13_ALL;
> > + return NULL;
> > + }
> > +
> > if (fields & F_BANDS) {
> > band_str = strstr(string, "band");
> > if (!band_str) {
> > @@ -805,5 +831,73 @@ ofputil_format_meter_mod(struct ds *s, const struct
> > ofputil_meter_mod *mm)
> > ds_put_format(s, " cmd:%d ", mm->command);
> > }
> >
> > - ofputil_format_meter_config(s, &mm->meter);
> > + ofputil_format_meter_config(s, &mm->meter, false);
> > +}
> > +
> > +/* If 'command' is given as -2, each line may start with a command name
> > ("add",
> > + * "modify", "delete"). A missing command name is treated as "add".
> > + */
> > +char * OVS_WARN_UNUSED_RESULT
> > +parse_ofp_meter_mod_file(const char *file_name,
> > + int command,
> > + struct ofputil_meter_mod **mms, size_t *n_mms,
> > + enum ofputil_protocol *usable_protocols)
> > +{
> > + size_t allocated_mms;
> > + int line_number;
> > + FILE *stream;
> > + struct ds s;
> > +
> > + *mms = NULL;
> > + *n_mms = 0;
> > +
> > + stream = !strcmp(file_name, "-") ? stdin : fopen(file_name, "r");
> > + if (stream == NULL) {
> > + return xasprintf("%s: open failed (%s)",
> > + file_name, ovs_strerror(errno));
> > + }
> > +
> > + allocated_mms = *n_mms;
> > + ds_init(&s);
> > + line_number = 0;
> > + *usable_protocols = OFPUTIL_P_ANY;
> > + while (!ds_get_preprocessed_line(&s, stream, &line_number)) {
> > + enum ofputil_protocol usable;
> > + char *error;
> > +
> > + if (*n_mms >= allocated_mms) {
> > + *mms = x2nrealloc(*mms, &allocated_mms, sizeof **mms);
> > + }
> > + error = parse_ofp_meter_mod_str(&(*mms)[*n_mms], ds_cstr(&s),
> > command,
> > + &usable);
> > + if (error) {
> > + size_t i;
> > +
> > + for (i = 0; i < *n_mms; i++) {
> > + if (mms[i]->meter.bands) {
> > + free(mms[i]->meter.bands);
> > + }
> > + }
> > + free(*mms);
> > + *mms = NULL;
> > + *n_mms = 0;
> > +
> > + ds_destroy(&s);
> > + if (stream != stdin) {
> > + fclose(stream);
> > + }
> > +
> > + char *ret = xasprintf("%s:%d: %s", file_name, line_number,
> > error);
> > + free(error);
> > + return ret;
> > + }
> > + *usable_protocols &= usable;
> > + *n_mms += 1;
> > + }
> > +
> > + ds_destroy(&s);
> > + if (stream != stdin) {
> > + fclose(stream);
> > + }
> > + return NULL;
> > }
> > diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> > index 874079b84..0879e705f 100644
> > --- a/lib/ofp-print.c
> > +++ b/lib/ofp-print.c
> > @@ -54,6 +54,7 @@
> > #include "openvswitch/ofp-monitor.h"
> > #include "openvswitch/ofp-msgs.h"
> > #include "openvswitch/ofp-port.h"
> > +#include "openvswitch/ofp-print.h"
> > #include "openvswitch/ofp-queue.h"
> > #include "openvswitch/ofp-switch.h"
> > #include "openvswitch/ofp-table.h"
> > @@ -365,12 +366,17 @@ ofp_print_meter_features_reply(struct ds *s, const
> > struct ofp_header *oh)
> > }
> >
> > static enum ofperr
> > -ofp_print_meter_config_reply(struct ds *s, const struct ofp_header *oh)
> > +ofp_print_meter_config_reply(struct ds *s, const struct ofp_header *oh,
> > + bool oneline)
> > {
> > struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length));
> > struct ofpbuf bands;
> > int retval;
> >
> > + if (oneline) {
> > + ds_put_char(s, '\n');
> > + }
> > +
> > ofpbuf_init(&bands, 64);
> > for (;;) {
> > struct ofputil_meter_config mc;
> > @@ -379,8 +385,10 @@ ofp_print_meter_config_reply(struct ds *s, const
> > struct ofp_header *oh)
> > if (retval) {
> > break;
> > }
> > - ds_put_char(s, '\n');
> > - ofputil_format_meter_config(s, &mc);
> > + if (!oneline) {
> > + ds_put_char(s, '\n');
> > + }
> > + ofputil_format_meter_config(s, &mc, oneline ? true : false);
> > }
> > ofpbuf_uninit(&bands);
> >
> > @@ -977,6 +985,8 @@ ofp_to_string__(const struct ofp_header *oh,
> > ofp_print_stats(string, oh);
> > }
> >
> > + bool oneline = !!ONELINE_GET(verbosity);
> > + verbosity = VERBOSITY(verbosity);
> > const void *msg = oh;
> > enum ofptype type = ofptype_from_ofpraw(raw);
> > switch (type) {
> > @@ -1090,7 +1100,7 @@ ofp_to_string__(const struct ofp_header *oh,
> > return ofp_print_meter_stats_reply(string, oh);
> >
> > case OFPTYPE_METER_CONFIG_STATS_REPLY:
> > - return ofp_print_meter_config_reply(string, oh);
> > + return ofp_print_meter_config_reply(string, oh, oneline);
> >
> > case OFPTYPE_METER_FEATURES_STATS_REPLY:
> > return ofp_print_meter_features_reply(string, oh);
> > @@ -1278,7 +1288,7 @@ ofp_to_string(const void *oh_, size_t len,
> > ofp_print_error(&string, error);
> > }
> >
> > - if (verbosity >= 5 || error) {
> > + if (VERBOSITY(verbosity) >= 5 || error) {
> > add_newline(&string);
> > ds_put_hex_dump(&string, oh, len, 0, true);
> > }
> > diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
> > index baab60a22..cbfd02ea6 100644
> > --- a/tests/dpif-netdev.at
> > +++ b/tests/dpif-netdev.at
> > @@ -283,11 +283,6 @@ AT_CHECK([ovs-appctl vlog/set dpif:dbg
> > dpif_netdev:dbg])
> >
> > AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=1 pktps burst stats
> > bands=type=drop rate=1 burst_size=1'])
> > AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=2 kbps burst stats
> > bands=type=drop rate=1 burst_size=2'])
> > -AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 'in_port=1
> > action=meter:1,7'])
> > -AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 'in_port=7
> > action=meter:2,1'])
> > -AT_CHECK([ovs-ofctl add-flow br1 'in_port=2 action=8'])
> > -AT_CHECK([ovs-ofctl add-flow br1 'in_port=8 action=2'])
> > -ovs-appctl time/stop
> >
> > AT_CHECK([ovs-ofctl -O OpenFlow13 dump-meters br0], [0], [dnl
> > OFPST_METER_CONFIG reply (OF1.3) (xid=0x2):
> > @@ -298,6 +293,45 @@ meter=2 kbps burst stats bands=
> > type=drop rate=1 burst_size=2
> > ])
> >
> > +AT_CHECK([ovs-ofctl -O OpenFlow13 dump-meters br0 --oneline], [0], [dnl
> > +OFPST_METER_CONFIG reply (OF1.3) (xid=0x2):
> > +meter=1 pktps burst stats bands= type=drop rate=1 burst_size=1
> > +meter=2 kbps burst stats bands= type=drop rate=1 burst_size=2
> > +])
> > +
> > +AT_CHECK([ovs-ofctl -O OpenFlow13 dump-meters br0 meter=1 --oneline], [0],
> > [dnl
> > +OFPST_METER_CONFIG reply (OF1.3) (xid=0x2):
> > +meter=1 pktps burst stats bands= type=drop rate=1 burst_size=1
> > +])
> > +
> > +AT_CHECK([ovs-ofctl del-meters -O openflow13 br0])
> > +AT_CHECK([ovs-ofctl -O OpenFlow13 dump-meters br0], [0], [dnl
> > +OFPST_METER_CONFIG reply (OF1.3) (xid=0x2):
> > +])
> > +
> > +AT_DATA([meters.txt], [dnl
> > +meter=1 pktps burst stats bands= type=drop rate=1 burst_size=1
> > +
> > +meter=2 kbps burst stats bands= type=drop rate=1 burst_size=2
> > +meter=3 kbps burst stats bands= type=drop rate=2 burst_size=3
> > +])
> > +
> > +AT_CHECK([ovs-ofctl add-meters -O openflow13 br0 meters.txt])
> > +AT_CHECK([ovs-ofctl -O OpenFlow13 dump-meters br0 --oneline], [0], [dnl
> > +OFPST_METER_CONFIG reply (OF1.3) (xid=0x2):
> > +meter=1 pktps burst stats bands= type=drop rate=1 burst_size=1
> > +meter=2 kbps burst stats bands= type=drop rate=1 burst_size=2
> > +meter=3 kbps burst stats bands= type=drop rate=2 burst_size=3
> > +])
> > +
> > +AT_CHECK([ovs-ofctl del-meters -O openflow13 br0 meter=3])
> > +
> > +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 'in_port=1
> > action=meter:1,7'])
> > +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 'in_port=7
> > action=meter:2,1'])
> > +AT_CHECK([ovs-ofctl add-flow br1 'in_port=2 action=8'])
> > +AT_CHECK([ovs-ofctl add-flow br1 'in_port=8 action=2'])
> > +ovs-appctl time/stop
>
> The patch adds a lot of new ways of parsing and dumping meters, but this
> one is the only test that is covering new functionality.
>
> For exmaple, we can have 'add', 'delete', 'modify' commands in the file,
> but we don't have a test for this.
> All cases above have no bands specified. I'm curious how the oneline
> prints will look with bands, multiple bands per meter and if the code
> is actually capable of parsing multi-band dumps in a oneline form.
> That's, I think, the main reason why meter dumps are multi-line.
>
> Please, add tests to cover at least these cases to ovs-ofctl.at.
>
>
OK.
The old code will print the meter like "..bands=\ntype=..." even we
have only one band here.
This is the main reason we need a oneline form to process the meters saved.
And your concern about multiple bands is right, they should be appended to the
same line and they are.
> > +
> > ovs-appctl time/warp 5000
> > for i in `seq 1 7`; do
> > AT_CHECK(
> > diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> > index 0a611b2ee..c44091906 100644
> > --- a/utilities/ovs-ofctl.8.in
> > +++ b/utilities/ovs-ofctl.8.in
> > @@ -492,23 +492,35 @@ the \fBBridge\fR table). For more information, see
> > ``Q: What versions
> > of OpenFlow does Open vSwitch support?'' in the Open vSwitch FAQ.
> > .
> > .IP "\fBadd\-meter \fIswitch meter\fR"
> > +.IQ "\fBadd\-meter \fIswitch - < file\fR"
> > Add a meter entry to \fIswitch\fR's tables. The \fImeter\fR syntax is
> > described in section \fBMeter Syntax\fR, below.
> > .
> > +.IP "\fBadd\-meters \fIswitch file\fR"
> > +Add each meter entry to \fIswitch\fR's tables. Each meter specification
> > +(each line in file) may start with \fBadd\fI, \fBmodify\fI or \fBdelete\fI
> > +keyword to specify whether a meter is to be added, modified, or deleted.
> > +For backwards compatibility a meter specification without one of these
> > keywords
> > +is treated as a meter add. The \fImeter\fR syntax is described in section
> > +\fBMeter Syntax\fR, below.
> > +.
> > .IP "\fBmod\-meter \fIswitch meter\fR"
> > +.IQ "\fBmod\-meter \fIswitch - < file\fR"
> > Modify an existing meter.
> > .
> > .IP "\fBdel\-meters \fIswitch\fR [\fImeter\fR]"
> > +.IQ "\fBdel\-meters \fIswitch\fR - < file"
> > Delete entries from \fIswitch\fR's meter table. To delete only a
> > -specific meter, specify its number as \fImeter\fR. Otherwise, if
> > +specific meter, specify a meter syntax \fBmeter=\fIid\fR. Otherwise, if
> > \fImeter\fR is omitted, or if it is specified as \fBall\fR, then all
> > meters are deleted.
> > .
> > -.IP "\fBdump\-meters \fIswitch\fR [\fImeter\fR]"
> > +.IP "[\fB\-\-oneline\fR] \fBdump\-meters \fIswitch\fR [\fImeter\fR]"
> > Print entries from \fIswitch\fR's meter table. To print only a
> > -specific meter, specify its number as \fImeter\fR. Otherwise, if
> > +specific meter, specify a meter syntax \fBmeter=\fIid\fR. Otherwise, if
> > \fImeter\fR is omitted, or if it is specified as \fBall\fR, then all
> > -meters are printed.
> > +meters are printed. With \fB\-\-oneline\fR, band information will be
> > +combined into one line.
> > .
> > .IP "\fBmeter\-stats \fIswitch\fR [\fImeter\fR]"
> > Print meter statistics. \fImeter\fR can specify a single meter with
> > @@ -1342,6 +1354,11 @@ includes flow duration, packet and byte counts, and
> > idle and hard age
> > in its output. With \fB\-\-no\-stats\fR, it omits all of these, as
> > well as cookie values and table IDs if they are zero.
> > .
> > +.IP "\fB\-\-oneline\fR"
> > +The \fBdump\-meters\fR command prints each band in a separate line by
> > +default. With \fB\-\-oneline\fR, all bands will be printed in a single
> > +line. This is useful for dumping meters to a file and restoring them.
> > +.
> > .IP "\fB\-\-read-only\fR"
> > Do not execute read/write commands.
> > .
> > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> > index 24d0941cf..a5ff4b3c4 100644
> > --- a/utilities/ovs-ofctl.c
> > +++ b/utilities/ovs-ofctl.c
> > @@ -157,6 +157,9 @@ static int print_pcap = 0;
> > /* --raw: Makes "ofp-print" read binary data from stdin. */
> > static int raw = 0;
> >
> > +/* --oneline: show meter config in a single line. */
> > +static bool oneline = false;
> > +
> > static const struct ovs_cmdl_command *get_all_commands(void);
> >
> > OVS_NO_RETURN static void usage(void);
> > @@ -217,6 +220,7 @@ parse_options(int argc, char *argv[])
> > OPT_COLOR,
> > OPT_MAY_CREATE,
> > OPT_READ_ONLY,
> > + OPT_ONELINE,
> > DAEMON_OPTION_ENUMS,
> > OFP_VERSION_OPTION_ENUMS,
> > VLOG_OPTION_ENUMS,
> > @@ -245,6 +249,7 @@ parse_options(int argc, char *argv[])
> > {"pcap", no_argument, &print_pcap, 1},
> > {"raw", no_argument, &raw, 1},
> > {"read-only", no_argument, NULL, OPT_READ_ONLY},
> > + {"oneline", no_argument, NULL, OPT_ONELINE},
> > DAEMON_LONG_OPTIONS,
> > OFP_VERSION_LONG_OPTIONS,
> > VLOG_LONG_OPTIONS,
> > @@ -314,6 +319,10 @@ parse_options(int argc, char *argv[])
> > ovs_cmdl_print_options(long_options);
> > exit(EXIT_SUCCESS);
> >
> > + case OPT_ONELINE:
> > + oneline = true;
> > + break;
> > +
> > case OPT_BUNDLE:
> > bundle = true;
> > break;
> > @@ -390,6 +399,10 @@ parse_options(int argc, char *argv[])
> > }
> > }
> >
> > + if (oneline) {
> > + verbosity = ONELINE_SET(verbosity);
> > + }
> > +
> > ctl_timeout_setup(timeout);
> >
> > if (n_criteria) {
> > @@ -475,9 +488,10 @@ usage(void)
> > " dump-group-stats SWITCH [GROUP] print group statistics\n"
> > " queue-get-config SWITCH [PORT] print queue config for
> > PORT\n"
> > " add-meter SWITCH METER add meter described by METER\n"
> > + " add-meters SWITCH FILE add meters from FILE\n"
> > " mod-meter SWITCH METER modify specific METER\n"
> > " del-meters SWITCH [METER] delete meters matching METER\n"
> > - " dump-meters SWITCH [METER] print METER configuration\n"
> > + " dump-meters SWITCH [METER] print METER entries\n"
>
> Why this change?
Be more specific. dump-meters with [METER] or not will print different results.
>
> > " meter-stats SWITCH [METER] print meter statistics\n"
> > " meter-features SWITCH print meter features\n"
> > " add-tlv-map SWITCH MAP add TLV option MAPpings\n"
> > @@ -515,6 +529,7 @@ usage(void)
> > " --rsort[=field] sort in descending order\n"
> > " --names show port names instead of
> > numbers\n"
> > " --no-names show port numbers, but not
> > names\n"
> > + " --oneline show meter bands in a single
> > line\n"
> > " --unixctl=SOCKET set control socket name\n"
> > " --color[=always|never|auto] control use of color in output\n"
> > " -h, --help display this help message\n"
> > @@ -1443,7 +1458,7 @@ set_protocol_for_flow_dump(struct vconn *vconn,
> > if (usable_protocols & allowed_protocols) {
> > ovs_fatal(0, "switch does not support any of the usable flow "
> > "formats (%s)", usable_s);
> > -} else {
> > + } else {
> > char *allowed_s = ofputil_protocols_to_string(allowed_protocols);
> > ovs_fatal(0, "none of the usable flow formats (%s) is among the "
> > "allowed flow formats (%s)", usable_s, allowed_s);
> > @@ -4084,57 +4099,107 @@ ofctl_diff_flows(struct ovs_cmdl_context *ctx)
> > }
> >
> > static void
> > -ofctl_meter_mod__(const char *bridge, const char *str, int command)
> > +ofctl_meter_mod__(const char *remote, struct ofputil_meter_mod *mms,
> > + size_t n_mms, enum ofputil_protocol usable_protocols)
> > {
> > - struct ofputil_meter_mod mm;
> > - struct vconn *vconn;
> > enum ofputil_protocol protocol;
> > - enum ofputil_protocol usable_protocols;
> > + struct ofputil_meter_mod *mm;
> > enum ofp_version version;
> > + struct ofpbuf *request;
> > + struct vconn *vconn;
> > + size_t i;
> >
> > - memset(&mm, 0, sizeof mm);
> > - if (str) {
> > + protocol = open_vconn_for_flow_mod(remote, &vconn, usable_protocols);
> > + version = ofputil_protocol_to_ofp_version(protocol);
> > +
> > + for (i = 0; i < n_mms; i++) {
> > + mm = &mms[i];
> > + request = ofputil_encode_meter_mod(version, mm);
> > + transact_noreply(vconn, request);
> > + free(mm->meter.bands);
> > + }
> > +
> > + vconn_close(vconn);
> > +}
> > +
> > +static void
> > +ofctl_meter_mod_file(int argc OVS_UNUSED, char *argv[], int command)
> > +{
> > + enum ofputil_protocol usable_protocols;
> > + struct ofputil_meter_mod *mms = NULL;
> > + size_t n_mms = 0;
> > + char *error;
> > +
> > + if (command == OFPMC13_ADD) {
> > + /* Allow the file to specify a mix of commands. If none specified
> > at
> > + * the beginning of any given line, then the default is
> > OFPMC13_ADD, so
> > + * this is backwards compatible. */
> > + command = -2;
> > + }
> > + error = parse_ofp_meter_mod_file(argv[2], command,
> > + &mms, &n_mms, &usable_protocols);
> > + if (error) {
> > + ovs_fatal(0, "%s", error);
> > + }
> > + ofctl_meter_mod__(argv[1], mms, n_mms, usable_protocols);
> > + free(mms);
> > +}
> > +
> > +static void
> > +ofctl_meter_mod(int argc, char *argv[], uint16_t command)
> > +{
> > + if (argc > 2 && !strcmp(argv[2], "-")) {
> > + ofctl_meter_mod_file(argc, argv, command);
> > + } else {
> > + enum ofputil_protocol usable_protocols;
> > + struct ofputil_meter_mod mm;
> > char *error;
> > - error = parse_ofp_meter_mod_str(&mm, str, command,
> > &usable_protocols);
> > + memset(&mm, 0, sizeof mm);
> > + error = parse_ofp_meter_mod_str(&mm, argc > 2 ? argv[2] : "",
> > command,
> > + &usable_protocols);
> > if (error) {
> > ovs_fatal(0, "%s", error);
> > }
> > - } else {
> > - usable_protocols = OFPUTIL_P_OF13_UP;
> > - mm.command = command;
> > - mm.meter.meter_id = OFPM13_ALL;
> > + ofctl_meter_mod__(argv[1], &mm, 1, usable_protocols);
> > }
> > -
> > - protocol = open_vconn_for_flow_mod(bridge, &vconn, usable_protocols);
> > - version = ofputil_protocol_to_ofp_version(protocol);
> > - transact_noreply(vconn, ofputil_encode_meter_mod(version, &mm));
> > - free(mm.meter.bands);
> > - vconn_close(vconn);
> > }
> >
> > -static void
> > -ofctl_meter_request__(const char *bridge, const char *str,
> > - enum ofputil_meter_request_type type)
> > +static struct vconn *
> > +prepare_dump_meters(const char *bridge, const char *str,
> > + struct ofputil_meter_mod *mm,
> > + enum ofputil_protocol *protocolp)
> > {
> > - struct ofputil_meter_mod mm;
> > struct vconn *vconn;
> > enum ofputil_protocol usable_protocols;
> > enum ofputil_protocol protocol;
> > - enum ofp_version version;
> >
> > - memset(&mm, 0, sizeof mm);
> > if (str) {
> > char *error;
> > - error = parse_ofp_meter_mod_str(&mm, str, -1, &usable_protocols);
> > + error = parse_ofp_meter_mod_str(mm, str, -1, &usable_protocols);
> > if (error) {
> > ovs_fatal(0, "%s", error);
> > }
> > } else {
> > usable_protocols = OFPUTIL_P_OF13_UP;
> > - mm.meter.meter_id = OFPM13_ALL;
> > + mm->meter.meter_id = OFPM13_ALL;
> > }
> >
> > - protocol = open_vconn_for_flow_mod(bridge, &vconn, usable_protocols);
> > + protocol = open_vconn(bridge, &vconn);
> > + *protocolp = set_protocol_for_flow_dump(vconn, protocol,
> > usable_protocols);
> > + return vconn;
> > +}
> > +
> > +static void
> > +ofctl_meter_request__(const char *bridge, const char *str,
> > + enum ofputil_meter_request_type type)
> > +{
> > + enum ofputil_protocol protocol;
> > + struct ofputil_meter_mod mm;
> > + enum ofp_version version;
> > + struct vconn *vconn;
> > +
> > + memset(&mm, 0, sizeof mm);
> > + vconn = prepare_dump_meters(bridge, str, &mm, &protocol);
> > version = ofputil_protocol_to_ofp_version(protocol);
> > dump_transaction(vconn, ofputil_encode_meter_request(version, type,
> >
> > mm.meter.meter_id));
> > @@ -4142,23 +4207,28 @@ ofctl_meter_request__(const char *bridge, const
> > char *str,
> > vconn_close(vconn);
> > }
> >
> > -
> > static void
> > ofctl_add_meter(struct ovs_cmdl_context *ctx)
> > {
> > - ofctl_meter_mod__(ctx->argv[1], ctx->argv[2], OFPMC13_ADD);
> > + ofctl_meter_mod(ctx->argc, ctx->argv, OFPMC13_ADD);
> > +}
> > +
> > +static void
> > +ofctl_add_meters(struct ovs_cmdl_context *ctx)
> > +{
> > + ofctl_meter_mod_file(ctx->argc, ctx->argv, OFPMC13_ADD);
> > }
> >
> > static void
> > ofctl_mod_meter(struct ovs_cmdl_context *ctx)
> > {
> > - ofctl_meter_mod__(ctx->argv[1], ctx->argv[2], OFPMC13_MODIFY);
> > + ofctl_meter_mod(ctx->argc, ctx->argv, OFPMC13_MODIFY);
> > }
> >
> > static void
> > ofctl_del_meters(struct ovs_cmdl_context *ctx)
> > {
> > - ofctl_meter_mod__(ctx->argv[1], ctx->argc > 2 ? ctx->argv[2] : NULL,
> > OFPMC13_DELETE);
> > + ofctl_meter_mod(ctx->argc, ctx->argv, OFPMC13_DELETE);
> > }
> >
> > static void
> > @@ -5072,15 +5142,17 @@ static const struct ovs_cmdl_command all_commands[]
> > = {
> > 2, 2, ofctl_diff_flows, OVS_RW },
> > { "add-meter", "switch meter",
> > 2, 2, ofctl_add_meter, OVS_RW },
> > + { "add-meters", "switch file",
> > + 2, 2, ofctl_add_meters, OVS_RW },
> > { "mod-meter", "switch meter",
> > 2, 2, ofctl_mod_meter, OVS_RW },
> > - { "del-meter", "switch meter",
> > + { "del-meter", "switch [meter]",
> > 1, 2, ofctl_del_meters, OVS_RW },
> > { "del-meters", "switch",
> > 1, 2, ofctl_del_meters, OVS_RW },
> > { "dump-meter", "switch meter",
> > 1, 2, ofctl_dump_meters, OVS_RO },
> > - { "dump-meters", "switch",
> > + { "dump-meters", "switch [meter]",
> > 1, 2, ofctl_dump_meters, OVS_RO },
> > { "meter-stats", "switch [meter]",
> > 1, 2, ofctl_meter_stats, OVS_RO },
> > diff --git a/utilities/ovs-save b/utilities/ovs-save
> > index 67092ecf7..d1baa3415 100755
> > --- a/utilities/ovs-save
> > +++ b/utilities/ovs-save
> > @@ -139,6 +139,9 @@ save_flows () {
> > echo "ovs-ofctl -O $ofp_version add-groups ${bridge} \
> > \"$workdir/$bridge.groups.dump\" ${bundle}"
> >
> > + echo "ovs-ofctl -O $ofp_version add-meters ${bridge} \
> > + \"$workdir/$bridge.meters.dump\""
> > +
> > echo "ovs-ofctl -O $ofp_version replace-flows ${bridge} \
> > \"$workdir/$bridge.flows.dump\" ${bundle}"
> >
> > @@ -147,6 +150,11 @@ save_flows () {
> > -e '/^NXST_GROUP_DESC/d' > \
> > "$workdir/$bridge.groups.dump"
> >
> > + ovs-ofctl -O $ofp_version dump-meters "$bridge" --oneline | \
> > + sed -e '/^OFPST_METER_CONFIG/d' \
> > + -e '/^NXST_METER_CONFIG/d' > \
> > + "$workdir/$bridge.meters.dump"
> > +
> > ovs-ofctl -O $ofp_version dump-flows --no-names --no-stats
> > "$bridge" | \
> > sed -e '/NXST_FLOW/d' \
> > -e '/OFPST_FLOW/d' \
>
Regards,
Wan Junjie
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
