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 <wanjun...@bytedance.com>

Hi.  Thanks for the patch!  See some comemnts inline.

Best regards, Ilya Maximets.

> 
> ---
> 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_.

> +
>  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.


> +
>  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?

>             "  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' \

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to