[ovs-dev] [ACL Meters 4/7] ovn: Add Meter and Meter_Band tables to the NB and SB databases.

Mark Michelson mmichels at redhat.com
Mon Jul 30 18:40:37 UTC 2018


Hi Justin,

I took a look through the patch series, and this is the only one that I 
had some immediate feedback on.

First, it would be nice if we could refer to Meters by name when issuing 
DB commands. For instance `ovn-nbctl add Meter <meter_name> bands <band 
UUID>`.

Second, I noticed that the algorithm for computing southbound meter 
bands can result in a larger number of bands than is in the northbound 
table. For instance, you could issue the following:

ovn-nbctl --id=@id create Meter_Band action=drop rate=1000 -- \
create Meter name=foo unit=kbps band=@id -- \
create Meter name=bar unit=kbps band=@id

In the northbound database, you'll have one entry in the meter_band 
table. In the southbound database, you'll have two entries in the 
meter_band table. The algorithm does not take into account that multiple 
northbound meters may refer to the same meter_band. I'm not sure how big 
an issue this is (or really if it is an issue), but it surprised me a 
bit when I was playing around with it.


On 07/30/2018 02:46 AM, Justin Pettit wrote:
> Add support for configuring meters through the Meter and Meter_Band
> tables in the Northbound database.  This commit also has ovn-northd
> sync those tables between the Northbound and Southbound databases.
> 
> Add support for configuring meters with ovn-nbctl.
> 
> Signed-off-by: Justin Pettit <jpettit at ovn.org>
> ---
>   ovn/northd/ovn-northd.c       | 145 ++++++++++++++++++++++++++++++++++
>   ovn/ovn-nb.ovsschema          |  33 +++++++-
>   ovn/ovn-nb.xml                |  80 +++++++++++++++++++
>   ovn/ovn-sb.ovsschema          |  27 ++++++-
>   ovn/ovn-sb.xml                |  72 +++++++++++++++++
>   ovn/utilities/ovn-nbctl.8.xml |  50 ++++++++++++
>   ovn/utilities/ovn-nbctl.c     | 143 +++++++++++++++++++++++++++++++++
>   tests/ovn-nbctl.at            |  58 ++++++++++++++
>   8 files changed, 604 insertions(+), 4 deletions(-)
> 
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 04a072ba8de7..45557170edc8 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -6606,6 +6606,140 @@ sync_port_groups(struct northd_context *ctx)
>       shash_destroy(&sb_port_groups);
>   }
>   
> +struct band_entry {
> +    int64_t rate;
> +    int64_t burst_size;
> +    const char *action;
> +};
> +
> +static int
> +band_cmp(const void *band1_, const void *band2_)
> +{
> +    const struct band_entry *band1p = band1_;
> +    const struct band_entry *band2p = band2_;
> +
> +    if (band1p->rate != band2p->rate) {
> +        return band1p->rate > band2p->rate ? -1 : 1;
> +    } else if (band1p->burst_size != band2p->burst_size) {
> +        return band1p->burst_size > band2p->burst_size ? -1 : 1;
> +    } else {
> +        return strcmp(band1p->action, band2p->action);
> +    }
> +}
> +
> +static bool
> +bands_need_update(const struct nbrec_meter *nb_meter,
> +                  const struct sbrec_meter *sb_meter)
> +{
> +    if (nb_meter->n_bands != sb_meter->n_bands) {
> +        return true;
> +    }
> +
> +    /* A single band is the most common scenario, so speed up that
> +     * check. */
> +    if (nb_meter->n_bands == 1) {
> +        struct nbrec_meter_band *nb_band = nb_meter->bands[0];
> +        struct sbrec_meter_band *sb_band = sb_meter->bands[0];
> +
> +        return !(nb_band->rate == sb_band->rate
> +                 && nb_band->burst_size == sb_band->burst_size
> +                 && !strcmp(sb_band->action, nb_band->action));
> +    }
> +
> +    /* Place the Northbound entries in sorted order. */
> +    struct band_entry *nb_bands;
> +    nb_bands = xmalloc(sizeof *nb_bands * nb_meter->n_bands);
> +    for (size_t i = 0; i < nb_meter->n_bands; i++) {
> +        struct nbrec_meter_band *nb_band = nb_meter->bands[i];
> +
> +        nb_bands[i].rate = nb_band->rate;
> +        nb_bands[i].burst_size = nb_band->burst_size;
> +        nb_bands[i].action = nb_band->action;
> +    }
> +    qsort(nb_bands, nb_meter->n_bands, sizeof *nb_bands, band_cmp);
> +
> +    /* Place the Southbound entries in sorted order. */
> +    struct band_entry *sb_bands;
> +    sb_bands = xmalloc(sizeof *sb_bands * sb_meter->n_bands);
> +    for (size_t i = 0; i < sb_meter->n_bands; i++) {
> +        struct sbrec_meter_band *sb_band = sb_meter->bands[i];
> +
> +        sb_bands[i].rate = sb_band->rate;
> +        sb_bands[i].burst_size = sb_band->burst_size;
> +        sb_bands[i].action = sb_band->action;
> +    }
> +    qsort(sb_bands, sb_meter->n_bands, sizeof *sb_bands, band_cmp);
> +
> +    bool need_update = false;
> +    for (size_t i = 0; i < nb_meter->n_bands; i++) {
> +        if (nb_bands[i].rate != sb_bands[i].rate
> +            || nb_bands[i].burst_size != sb_bands[i].burst_size
> +            || strcmp(nb_bands[i].action, nb_bands[i].action)) {
> +            need_update = true;
> +            goto done;
> +        }
> +    }
> +
> +done:
> +    free(nb_bands);
> +    free(sb_bands);
> +
> +    return need_update;
> +}
> +
> +/* Each entry in the Meter and Meter_Band tables in OVN_Northbound have
> + * a corresponding entries in the Meter and Meter_Band tables in
> + * OVN_Southbound.
> + */
> +static void
> +sync_meters(struct northd_context *ctx)
> +{
> +    struct shash sb_meters = SHASH_INITIALIZER(&sb_meters);
> +
> +    const struct sbrec_meter *sb_meter;
> +    SBREC_METER_FOR_EACH (sb_meter, ctx->ovnsb_idl) {
> +        shash_add(&sb_meters, sb_meter->name, sb_meter);
> +    }
> +
> +    const struct nbrec_meter *nb_meter;
> +    NBREC_METER_FOR_EACH (nb_meter, ctx->ovnnb_idl) {
> +        bool new_sb_meter = false;
> +
> +        sb_meter = shash_find_and_delete(&sb_meters, nb_meter->name);
> +        if (!sb_meter) {
> +            sb_meter = sbrec_meter_insert(ctx->ovnsb_txn);
> +            sbrec_meter_set_name(sb_meter, nb_meter->name);
> +            new_sb_meter = true;
> +        }
> +
> +        if (new_sb_meter || bands_need_update(nb_meter, sb_meter)) {
> +            struct sbrec_meter_band **sb_bands;
> +            sb_bands = xcalloc(nb_meter->n_bands, sizeof *sb_bands);
> +            for (size_t i = 0; i < nb_meter->n_bands; i++) {
> +                const struct nbrec_meter_band *nb_band = nb_meter->bands[i];
> +
> +                sb_bands[i] = sbrec_meter_band_insert(ctx->ovnsb_txn);
> +
> +                sbrec_meter_band_set_action(sb_bands[i], nb_band->action);
> +                sbrec_meter_band_set_rate(sb_bands[i], nb_band->rate);
> +                sbrec_meter_band_set_burst_size(sb_bands[i],
> +                                                nb_band->burst_size);
> +            }
> +            sbrec_meter_set_bands(sb_meter, sb_bands, nb_meter->n_bands);
> +            free(sb_bands);
> +        }
> +
> +        sbrec_meter_set_unit(sb_meter, nb_meter->unit);
> +    }
> +
> +    struct shash_node *node, *next;
> +    SHASH_FOR_EACH_SAFE (node, next, &sb_meters) {
> +        sbrec_meter_delete(node->data);
> +        shash_delete(&sb_meters, node);
> +    }
> +    shash_destroy(&sb_meters);
> +}
> +
>   /*
>    * struct 'dns_info' is used to sync the DNS records between OVN Northbound db
>    * and Southbound db.
> @@ -6726,6 +6860,7 @@ ovnnb_db_run(struct northd_context *ctx,
>   
>       sync_address_sets(ctx);
>       sync_port_groups(ctx);
> +    sync_meters(ctx);
>       sync_dns_entries(ctx, &datapaths);
>   
>       struct ovn_port_group *pg, *next_pg;
> @@ -7351,6 +7486,16 @@ main(int argc, char *argv[])
>                          &sbrec_rbac_permission_col_insert_delete);
>       add_column_noalert(ovnsb_idl_loop.idl, &sbrec_rbac_permission_col_update);
>   
> +    ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_meter);
> +    ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_meter_col_name);
> +    ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_meter_col_unit);
> +    ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_meter_col_bands);
> +
> +    ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_meter_band);
> +    ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_meter_band_col_action);
> +    ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_meter_band_col_rate);
> +    ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_meter_band_col_burst_size);
> +
>       ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_chassis);
>       ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_nb_cfg);
>       ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_name);
> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> index 8e6ddec4662f..9a0d8ec70514 100644
> --- a/ovn/ovn-nb.ovsschema
> +++ b/ovn/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>   {
>       "name": "OVN_Northbound",
> -    "version": "5.11.0",
> -    "cksum": "1149260021 18713",
> +    "version": "5.12.0",
> +    "cksum": "2812995200 20238",
>       "tables": {
>           "NB_Global": {
>               "columns": {
> @@ -195,6 +195,35 @@
>                       "type": {"key": "string", "value": "string",
>                                "min": 0, "max": "unlimited"}}},
>               "isRoot": false},
> +        "Meter": {
> +            "columns": {
> +                "name": {"type": "string"},
> +                "unit": {"type": {"key": {"type": "string",
> +                                          "enum": ["set", ["kbps", "pktps"]]}}},
> +                "bands": {"type": {"key": {"type": "uuid",
> +                                           "refTable": "Meter_Band",
> +                                           "refType": "strong"},
> +                                   "min": 1,
> +                                   "max": "unlimited"}},
> +                "external_ids": {
> +                    "type": {"key": "string", "value": "string",
> +                             "min": 0, "max": "unlimited"}}},
> +            "indexes": [["name"]],
> +            "isRoot": true},
> +        "Meter_Band": {
> +            "columns": {
> +                "action": {"type": {"key": {"type": "string",
> +                                            "enum": ["set", ["drop"]]}}},
> +                "rate": {"type": {"key": {"type": "integer",
> +                                          "minInteger": 1,
> +                                          "maxInteger": 4294967295}}},
> +                "burst_size": {"type": {"key": {"type": "integer",
> +                                                "minInteger": 0,
> +                                                "maxInteger": 4294967295}}},
> +                "external_ids": {
> +                    "type": {"key": "string", "value": "string",
> +                             "min": 0, "max": "unlimited"}}},
> +            "isRoot": false},
>           "Logical_Router": {
>               "columns": {
>                   "name": {"type": "string"},
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index e4e72b27cf36..1feb2af52027 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -1356,6 +1356,86 @@
>       </column>
>     </table>
>   
> +  <table name="Meter" title="Meter table">
> +    <p>
> +      Each row in this table represents a meter that can be used for QoS or
> +      rate-limiting.
> +    </p>
> +
> +    <column name="name">
> +      <p>
> +        A name for this meter.
> +      </p>
> +
> +      <p>
> +        Names that begin with "__" are reserved for OVN internal use and should
> +        not be added manually.
> +      </p>
> +    </column>
> +
> +    <column name="unit">
> +      <p>
> +        The unit for <ref column="rate" table="Meter_Band"/> and
> +        <ref column="burst_rate" table="Meter_Band"/> parameters in
> +        the <ref column="bands"/> entry.  <code>kbps</code> specifies
> +        kilobits per second, and <code>pktps</code> specifies packets
> +        per second.
> +      </p>
> +    </column>
> +
> +    <column name="bands">
> +      <p>
> +        The bands associated with this meter.  Each band specifies a
> +        rate above which the band is to take the action
> +        <code>action</code>.  If multiple bands' rates are exceeded,
> +        then the band with the highest rate among the exceeded bands is
> +        selected.
> +      </p>
> +    </column>
> +
> +    <column name="external_ids">
> +      See <em>External IDs</em> at the beginning of this document.
> +    </column>
> +  </table>
> +
> +  <table name="Meter_Band" title="Meter_Band table">
> +    <p>
> +      Each row in this table represents a meter band which specifies the
> +      rate above which the configured action should be applied.  These bands
> +      are referenced by the <ref column="bands" table="Meter"/> column in
> +      the <ref table="Meter"/> table.
> +    </p>
> +
> +    <column name="action">
> +      <p>
> +        The action to execute when this band matches.  The only supported
> +        action is <code>drop</code>.
> +      </p>
> +    </column>
> +
> +    <column name="rate">
> +      <p>
> +        The relative rate limit for this band, in kilobits per second or
> +        bits per second, depending on whether the parent <ref table="Meter"/>
> +        entry's <ref column="unit" table="Meter"/> column specified
> +        <code>kbps</code> or <code>pktps</code>.
> +      </p>
> +    </column>
> +
> +    <column name="burst_size">
> +      <p>
> +        The maximum burst allowed for the band in kilobits or packets,
> +        depending on whether <code>kbps</code> or <code>pktps</code> was
> +        selected in the parent <ref table="Meter"/> entry's
> +        <ref column="unit" table="Meter"/> column.
> +      </p>
> +    </column>
> +
> +    <column name="external_ids">
> +      See <em>External IDs</em> at the beginning of this document.
> +    </column>
> +  </table>
> +
>     <table name="Logical_Router_Port" title="L3 logical router port">
>       <p>
>         A port within an L3 logical router.
> diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema
> index 9e271d433246..ad6ad3b71da0 100644
> --- a/ovn/ovn-sb.ovsschema
> +++ b/ovn/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
>   {
>       "name": "OVN_Southbound",
> -    "version": "1.15.0",
> -    "cksum": "1839738004 13639",
> +    "version": "1.16.0",
> +    "cksum": "3046632234 14844",
>       "tables": {
>           "SB_Global": {
>               "columns": {
> @@ -98,6 +98,29 @@
>               "indexes": [["datapath", "tunnel_key"],
>                           ["datapath", "name"]],
>               "isRoot": true},
> +        "Meter": {
> +            "columns": {
> +                "name": {"type": "string"},
> +                "unit": {"type": {"key": {"type": "string",
> +                                          "enum": ["set", ["kbps", "pktps"]]}}},
> +                "bands": {"type": {"key": {"type": "uuid",
> +                                           "refTable": "Meter_Band",
> +                                           "refType": "strong"},
> +                                   "min": 1,
> +                                   "max": "unlimited"}}},
> +            "indexes": [["name"]],
> +            "isRoot": true},
> +        "Meter_Band": {
> +            "columns": {
> +                "action": {"type": {"key": {"type": "string",
> +                                            "enum": ["set", ["drop"]]}}},
> +                "rate": {"type": {"key": {"type": "integer",
> +                                          "minInteger": 1,
> +                                          "maxInteger": 4294967295}}},
> +                "burst_size": {"type": {"key": {"type": "integer",
> +                                                "minInteger": 0,
> +                                                "maxInteger": 4294967295}}}},
> +            "isRoot": false},
>           "Datapath_Binding": {
>               "columns": {
>                   "tunnel_key": {
> diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
> index f9724d398ce6..57d8a9e042a5 100644
> --- a/ovn/ovn-sb.xml
> +++ b/ovn/ovn-sb.xml
> @@ -1923,6 +1923,78 @@ tcp.flags = RST;
>       </column>
>     </table>
>   
> +  <table name="Meter" title="Meter table">
> +    <p>
> +      Each row in this table represents a meter that can be used for QoS or
> +      rate-limiting.
> +    </p>
> +
> +    <column name="name">
> +      <p>
> +        A name for this meter.
> +      </p>
> +
> +      <p>
> +        Names that begin with "__" are reserved for OVN internal use and should
> +        not be added manually.
> +      </p>
> +    </column>
> +
> +    <column name="unit">
> +      <p>
> +        The unit for <ref column="rate" table="Meter_Band"/> and
> +        <ref column="burst_rate" table="Meter_Band"/> parameters in
> +        the <ref column="bands"/> entry.  <code>kbps</code> specifies
> +        kilobits per second, and <code>pktps</code> specifies packets
> +        per second.
> +      </p>
> +    </column>
> +
> +    <column name="bands">
> +      <p>
> +        The bands associated with this meter.  Each band specifies a
> +        rate above which the band is to take the action
> +        <code>action</code>.  If multiple bands' rates are exceeded,
> +        then the band with the highest rate among the exceeded bands is
> +        selected.
> +      </p>
> +    </column>
> +  </table>
> +
> +  <table name="Meter_Band" title="Meter_Band table">
> +    <p>
> +      Each row in this table represents a meter band which specifies the
> +      rate above which the configured action should be applied.  These bands
> +      are referenced by the <ref column="bands" table="Meter"/> column in
> +      the <ref table="Meter"/> table.
> +    </p>
> +
> +    <column name="action">
> +      <p>
> +        The action to execute when this band matches.  The only supported
> +        action is <code>drop</code>.
> +      </p>
> +    </column>
> +
> +    <column name="rate">
> +      <p>
> +        The relative rate limit for this band, in kilobits per second or
> +        bits per second, depending on whether the parent <ref table="Meter"/>
> +        entry's <ref column="unit" table="Meter"/> column specified
> +        <code>kbps</code> or <code>pktps</code>.
> +      </p>
> +    </column>
> +
> +    <column name="burst_size">
> +      <p>
> +        The maximum burst allowed for the band in kilobits or packets,
> +        depending on whether <code>kbps</code> or <code>pktps</code> was
> +        selected in the parent <ref table="Meter"/> entry's
> +        <ref column="unit" table="Meter"/> column.
> +      </p>
> +    </column>
> +  </table>
> +
>     <table name="Datapath_Binding" title="Physical-Logical Datapath Bindings">
>       <p>
>         Each row in this table represents a logical datapath, which implements a
> diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
> index 2cd2fab304cd..a8ea7d8cb1e1 100644
> --- a/ovn/utilities/ovn-nbctl.8.xml
> +++ b/ovn/utilities/ovn-nbctl.8.xml
> @@ -172,6 +172,56 @@
>         </dd>
>       </dl>
>   
> +    <h1>Meter Commands</h1>
> +    <dl>
> +        <dt><code>meter-add</code> <var>name</var> <var>unit</var> <var>action</var> <var>rate</var> [<var>burst_size</var>]</dt>
> +      <dd>
> +        <p>
> +          Adds the specified meter.  <var>name</var> must be a unique
> +          name to identify this meter.  The <var>action</var> argument
> +          specifies what should happen when this meter is exceeded.
> +          The only supported action is <code>drop</code>.
> +        </p>
> +
> +        <p>
> +          The <var>unit</var> specifies the unit for the <var>rate</var>
> +          argument; valid values are <code>kbps</code> and
> +          <code>pktps</code> for kilobits per second and packets per
> +          second, respectively.  The <var>burst_rate</var> option
> +          configures the maximum burst allowed for the band in kilobits
> +          or packets depending on whether the <var>unit</var> chosen was
> +          <code>kbps</code> or <code>pktps</code>, respectively.
> +        </p>
> +
> +        <p>
> +          <code>ovn-nbctl</code> only supports adding a meter with a
> +          single band, but the other commands support meters with
> +          multiple bands.
> +        </p>
> +
> +        <p>
> +          Names that start with "__" are reserved for internal use by OVN,
> +          so <code>ovn-nbctl</code> does not allow adding them.
> +        </p>
> +      </dd>
> +
> +      <dt><code>meter-del</code> [<var>name</var>]</dt>
> +      <dd>
> +        <p>
> +          Deletes meters.  By default, all meters are deleted.  If
> +          <var>name</var> is supplied, only the meter with that name
> +          will be deleted.
> +      </p>
> +      </dd>
> +
> +      <dt><code>meter-list</code></dt>
> +      <dd>
> +        <p>
> +          Lists all meters.
> +        </p>
> +      </dd>
> +    </dl>
> +
>       <h1>Logical Switch Port Commands</h1>
>       <dl>
>         <dt>[<code>--may-exist</code>] <code>lsp-add</code> <var>switch</var> <var>port</var></dt>
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index 3c3e582cb906..9f0e6347c104 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -496,6 +496,12 @@ QoS commands:\n\
>                               remove QoS rules from SWITCH\n\
>     qos-list SWITCH           print QoS rules for SWITCH\n\
>   \n\
> +Meter commands:\n\
> +  meter-add NAME UNIT ACTION RATE [BURST_SIZE]\n\
> +                            add a meter\n\
> +  meter-del [NAME]          remove meters\n\
> +  meter-list                print meters\n\
> +\n\
>   Logical switch port commands:\n\
>     lsp-add SWITCH PORT       add logical port PORT on SWITCH\n\
>     lsp-add SWITCH PORT PARENT TAG\n\
> @@ -2290,6 +2296,137 @@ nbctl_qos_del(struct ctl_context *ctx)
>       }
>   }
>   
> +static int
> +meter_cmp(const void *meter1_, const void *meter2_)
> +{
> +    struct nbrec_meter *const *meter1p = meter1_;
> +    struct nbrec_meter *const *meter2p = meter2_;
> +    const struct nbrec_meter *meter1 = *meter1p;
> +    const struct nbrec_meter *meter2 = *meter2p;
> +
> +    return strcmp(meter1->name, meter2->name);
> +}
> +
> +static void
> +nbctl_meter_list(struct ctl_context *ctx)
> +{
> +    const struct nbrec_meter **meters = NULL;
> +    const struct nbrec_meter *meter;
> +    size_t n_capacity = 0;
> +    size_t n_meters = 0;
> +
> +    NBREC_METER_FOR_EACH (meter, ctx->idl) {
> +        if (n_meters == n_capacity) {
> +            meters = x2nrealloc(meters, &n_capacity, sizeof *meters);
> +        }
> +
> +        meters[n_meters] = meter;
> +        n_meters++;
> +    }
> +
> +    if (n_meters) {
> +        qsort(meters, n_meters, sizeof *meters, meter_cmp);
> +    }
> +
> +    for (size_t i = 0; i < n_meters; i++) {
> +        meter = meters[i];
> +        ds_put_format(&ctx->output, "%s: unit=%s bands:\n", meter->name,
> +                      meter->unit);
> +
> +        for (size_t j = 0; j < meter->n_bands; j++) {
> +            const struct nbrec_meter_band *band = meter->bands[j];
> +
> +            ds_put_format(&ctx->output, "  %s: rate=%"PRId64"",
> +                          band->action, band->rate);
> +            if (band->burst_size) {
> +                ds_put_format(&ctx->output, ", burst_size=%"PRId64"",
> +                              band->burst_size);
> +            }
> +        }
> +
> +        ds_put_cstr(&ctx->output, "\n");
> +    }
> +
> +    free(meters);
> +}
> +
> +static void
> +nbctl_meter_add(struct ctl_context *ctx)
> +{
> +    const struct nbrec_meter *meter;
> +
> +    const char *name = ctx->argv[1];
> +    NBREC_METER_FOR_EACH (meter, ctx->idl) {
> +        if (!strcmp(meter->name, name)) {
> +            ctl_fatal("meter with name \"%s\" already exists", name);
> +        }
> +    }
> +
> +    if (!strncmp(name, "__", 2)) {
> +        ctl_fatal("meter names that begin with \"__\" are reserved");
> +    }
> +
> +    const char *unit = ctx->argv[2];
> +    if (strcmp(unit, "kbps") && strcmp(unit, "pktps")) {
> +        ctl_fatal("unit must be \"kbps\" or \"pktps\"");
> +    }
> +
> +    const char *action = ctx->argv[3];
> +    if (strcmp(action, "drop")) {
> +        ctl_fatal("action must be \"drop\"");
> +    }
> +
> +    int64_t rate;
> +    if (!ovs_scan(ctx->argv[4], "%"SCNd64, &rate)
> +        || rate < 1 || rate > UINT32_MAX) {
> +        ctl_fatal("rate must be in the range 1...4294967295");
> +    }
> +
> +    int64_t burst_size = 0;
> +    if (ctx->argc > 5) {
> +        if (!ovs_scan(ctx->argv[5], "%"SCNd64, &burst_size)
> +            || burst_size < 0 || burst_size > UINT32_MAX) {
> +            ctl_fatal("burst_size must be in the range 0...4294967295");
> +        }
> +    }
> +
> +    /* Create the band.  We only support adding a single band. */
> +    struct nbrec_meter_band *band = nbrec_meter_band_insert(ctx->txn);
> +    nbrec_meter_band_set_action(band, action);
> +    nbrec_meter_band_set_rate(band, rate);
> +    nbrec_meter_band_set_burst_size(band, burst_size);
> +
> +    /* Create the meter. */
> +    meter = nbrec_meter_insert(ctx->txn);
> +    nbrec_meter_set_name(meter, name);
> +    nbrec_meter_set_unit(meter, unit);
> +    nbrec_meter_set_bands(meter, &band, 1);
> +}
> +
> +static void
> +nbctl_meter_del(struct ctl_context *ctx)
> +{
> +    const struct nbrec_meter *meter, *next;
> +
> +    /* If a name is not specified, delete all meters. */
> +    if (ctx->argc == 1) {
> +        NBREC_METER_FOR_EACH_SAFE (meter, next, ctx->idl) {
> +            nbrec_meter_delete(meter);
> +        }
> +        return;
> +    }
> +
> +    /* Remove the matching meter. */
> +    NBREC_METER_FOR_EACH (meter, ctx->idl) {
> +        if (strcmp(ctx->argv[1], meter->name)) {
> +            continue;
> +        }
> +
> +        nbrec_meter_delete(meter);
> +        return;
> +    }
> +}
> +
>   static void
>   nbctl_lb_add(struct ctl_context *ctx)
>   {
> @@ -4678,6 +4815,12 @@ static const struct ctl_command_syntax nbctl_commands[] = {
>         nbctl_qos_del, NULL, "", RW },
>       { "qos-list", 1, 1, "SWITCH", NULL, nbctl_qos_list, NULL, "", RO },
>   
> +    /* meter commands. */
> +    { "meter-add", 4, 5, "NAME UNIT ACTION RATE [BURST_SIZE]", NULL,
> +      nbctl_meter_add, NULL, "", RW },
> +    { "meter-del", 0, 1, "[NAME]", NULL, nbctl_meter_del, NULL, "", RW },
> +    { "meter-list", 0, 0, "", NULL, nbctl_meter_list, NULL, "", RO },
> +
>       /* logical switch port commands. */
>       { "lsp-add", 2, 4, "SWITCH PORT [PARENT] [TAG]", NULL, nbctl_lsp_add,
>         NULL, "--may-exist", RW },
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index 64e217654c2f..7a1445e312ff 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -323,6 +323,64 @@ OVN_NBCTL_TEST_STOP
>   AT_CLEANUP
>   
>   dnl ---------------------------------------------------------------------
> +
> +AT_SETUP([ovn-nbctl - Meters])
> +OVN_NBCTL_TEST_START
> +
> +AT_CHECK([ovn-nbctl meter-add meter1 kbps drop 10])
> +AT_CHECK([ovn-nbctl meter-add meter2 kbps drop 3 2])
> +AT_CHECK([ovn-nbctl meter-add meter3 kbps drop 100 200])
> +
> +dnl Add duplicate meter name
> +AT_CHECK([ovn-nbctl meter-add meter1 kbps drop 10], [1], [], [stderr])
> +AT_CHECK([grep 'already exists' stderr], [0], [ignore])
> +
> +dnl Add reserved meter name
> +AT_CHECK([ovn-nbctl meter-add __meter1 kbps drop 10], [1], [], [stderr])
> +AT_CHECK([grep 'reserved' stderr], [0], [ignore])
> +
> +dnl Add meter with invalid rates
> +AT_CHECK([ovn-nbctl meter-add meter4 kbps drop 100010111111], [1], [],
> +[ovn-nbctl: rate must be in the range 1...4294967295
> +])
> +
> +AT_CHECK([ovn-nbctl meter-add meter4 kbps drop 0], [1], [],
> +[ovn-nbctl: rate must be in the range 1...4294967295
> +])
> +
> +dnl Add meter with invalid burst_size
> +AT_CHECK([ovn-nbctl meter-add meter4 kbps drop 10 100010111111], [1], [],
> +[ovn-nbctl: burst_size must be in the range 0...4294967295
> +])
> +
> +AT_CHECK([ovn-nbctl meter-list], [0], [dnl
> +meter1: unit=kbps bands:
> +  drop: rate=10
> +meter2: unit=kbps bands:
> +  drop: rate=3, burst_size=2
> +meter3: unit=kbps bands:
> +  drop: rate=100, burst_size=200
> +])
> +
> +dnl Delete a single meter.
> +AT_CHECK([ovn-nbctl meter-del meter2])
> +AT_CHECK([ovn-nbctl meter-list], [0], [dnl
> +meter1: unit=kbps bands:
> +  drop: rate=10
> +meter3: unit=kbps bands:
> +  drop: rate=100, burst_size=200
> +])
> +
> +dnl Delete all meters.
> +AT_CHECK([ovn-nbctl meter-del])
> +AT_CHECK([ovn-nbctl meter-list], [0], [dnl
> +])
> +
> +OVN_NBCTL_TEST_STOP
> +AT_CLEANUP
> +
> +dnl ---------------------------------------------------------------------
> +
>   AT_SETUP([ovn-nbctl - NATs])
>   OVN_NBCTL_TEST_START
>   AT_CHECK([ovn-nbctl lr-add lr0])
> 



More information about the dev mailing list