Hi Wan,

Are you planning in sending a new version of this patch addressing
Ilya's comments? After all the hard work you put on it by the overall
good state of the current version, it would be a pitty not to get it
merged.

Thanks.
Adrián

On Sat, May 06, 2023 at 07:43:18PM +0800, 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>
>
> ---
> v11:
> add OFP_ prefix to macro
> add testcases for meters
>
> 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 ++++++++--
>  tests/ovs-ofctl.at              |  70 ++++++++++++++++
>  utilities/ovs-ofctl.8.in        |  25 +++++-
>  utilities/ovs-ofctl.c           | 140 ++++++++++++++++++++++++--------
>  utilities/ovs-save              |   8 ++
>  9 files changed, 374 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..fbaac2bea 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 */
> +#define OFP_ONELINE_BIT 7
> +#define OFP_ONELINE_MASK (1 << OFP_ONELINE_BIT)
> +#define OFP_VERBOSITY_MASK (~OFP_ONELINE_MASK)
> +
> +#define OFP_VERBOSITY(verbosity) (verbosity & OFP_VERBOSITY_MASK)
> +
> +#define OFP_ONELINE_SET(verbosity) (verbosity | OFP_ONELINE_MASK)
> +#define OFP_ONELINE_GET(verbosity) (verbosity & OFP_ONELINE_MASK)
> +
>  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..4d72748c2 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 = !!OFP_ONELINE_GET(verbosity);
> +    verbosity = OFP_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 (OFP_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
> +
>  ovs-appctl time/warp 5000
>  for i in `seq 1 7`; do
>    AT_CHECK(
> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> index 8531b2e2e..0f6a9e22c 100644
> --- a/tests/ovs-ofctl.at
> +++ b/tests/ovs-ofctl.at
> @@ -3309,3 +3309,73 @@ AT_CHECK([grep -q "ct_dpif|DBG|.*ct_flush: <all>" 
> ovs-vswitchd.log])
>
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
> +
> +
> +dnl This checks basic add_meter --oneline operations.
> +AT_SETUP([ofproto - add meters and dump meters with oneline option (OpenFlow 
> 1.3)])
> +OVS_VSWITCHD_START
> +AT_DATA([meters.txt], [dnl
> +meter=12 pktps burst stats bands=type=drop rate=1 burst_size=1
> +meter=34 pktps burst stats bands=type=drop rate=1 burst_size=1 type=drop 
> rate=10 burst_size=10
> +meter=56 kbps burst stats bands=type=drop rate=1 burst_size=1
> +meter=78 kbps burst stats bands=type=drop rate=1 burst_size=1 type=drop 
> rate=10 burst_size=10
> +])
> +AT_CHECK([ovs-ofctl -O openflow13 -vwarn add-meters br0 meters.txt])
> +AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn dump-meters br0 --oneline], [0], 
> [stdout])
> +AT_CHECK([strip_xids < stdout | sort -f], [0], [dnl
> +meter=12 pktps burst stats bands= type=drop rate=1 burst_size=1
> +meter=34 pktps burst stats bands= type=drop rate=1 burst_size=1 type=drop 
> rate=10 burst_size=10
> +meter=56 kbps burst stats bands= type=drop rate=1 burst_size=1
> +meter=78 kbps burst stats bands= type=drop rate=1 burst_size=1 type=drop 
> rate=10 burst_size=10
> +OFPST_METER_CONFIG reply (OF1.3):
> +])
> +AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn del-meters br0 meter=12])
> +AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn dump-meters br0 --oneline], [0], 
> [stdout])
> +AT_CHECK([strip_xids < stdout | sort -f], [0], [dnl
> +meter=34 pktps burst stats bands= type=drop rate=1 burst_size=1 type=drop 
> rate=10 burst_size=10
> +meter=56 kbps burst stats bands= type=drop rate=1 burst_size=1
> +meter=78 kbps burst stats bands= type=drop rate=1 burst_size=1 type=drop 
> rate=10 burst_size=10
> +OFPST_METER_CONFIG reply (OF1.3):
> +])
> +AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn del-meters br0 meter=34])
> +AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn dump-meters br0 --oneline], [0], 
> [stdout])
> +AT_CHECK([strip_xids < stdout | sort -f], [0], [dnl
> +meter=56 kbps burst stats bands= type=drop rate=1 burst_size=1
> +meter=78 kbps burst stats bands= type=drop rate=1 burst_size=1 type=drop 
> rate=10 burst_size=10
> +OFPST_METER_CONFIG reply (OF1.3):
> +])
> +AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn del-meters br0], [0])
> +AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn dump-meters br0 --oneline], [0], 
> [stdout])
> +AT_CHECK([strip_xids < stdout], [0], [dnl
> +OFPST_METER_CONFIG reply (OF1.3):
> +])
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
> +dnl This checks basic meter operations in file.
> +AT_SETUP([ofproto - add modify and delete meter in files (OpenFlow 1.3)])
> +OVS_VSWITCHD_START
> +AT_DATA([meters.txt], [dnl
> +add meter=12 pktps burst stats bands=type=drop rate=1 burst_size=1
> +add meter=34 pktps burst stats bands=type=drop rate=1 burst_size=1 type=drop 
> rate=10 burst_size=10
> +add meter=56 pktps burst stats bands=type=drop rate=2 burst_size=2 type=drop 
> rate=10 burst_size=10
> +modify meter=12 kbps burst stats bands=type=drop rate=1 burst_size=1
> +modify meter=34 kbps burst stats bands=type=drop rate=1 burst_size=1 
> type=drop rate=10 burst_size=10
> +delete meter=56
> +])
> +AT_CHECK([ovs-ofctl -O openflow13 -vwarn add-meters br0 meters.txt])
> +AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn dump-meters br0 --oneline], [0], 
> [stdout])
> +AT_CHECK([strip_xids < stdout | sort -f], [0], [dnl
> +meter=12 kbps burst stats bands= type=drop rate=1 burst_size=1
> +meter=34 kbps burst stats bands= type=drop rate=1 burst_size=1 type=drop 
> rate=10 burst_size=10
> +OFPST_METER_CONFIG reply (OF1.3):
> +])
> +AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn mod-meter br0 "meter=12 pktps burst 
> stats bands=type=drop rate=2 burst_size=2"])
> +AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn dump-meters br0 --oneline], [0], 
> [stdout])
> +AT_CHECK([strip_xids < stdout | sort -f], [0], [dnl
> +meter=12 pktps burst stats bands= type=drop rate=2 burst_size=2
> +meter=34 kbps burst stats bands= type=drop rate=1 burst_size=1 type=drop 
> rate=10 burst_size=10
> +OFPST_METER_CONFIG reply (OF1.3):
> +])
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> \ No newline at end of file
> 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..a928af869 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 = OFP_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"
>             "  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' \
> --
> 2.39.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

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

Reply via email to