[ovs-dev] [PATCH v6 ovn 1/1] northd: Enhance the implementation of ACL log meters (pre-ddlog merge).
Numan Siddique
numans at ovn.org
Mon Nov 23 09:55:05 UTC 2020
On Fri, Nov 20, 2020 at 11:53 PM Flavio Fernandes <flavio at flaviof.com> wrote:
>
> When multiple ACL rows use the same Meter for log rate-limiting, the
> 'noisiest' ACL matches may completely consume the Meter and shadow other
> ACL logs. This patch introduces a column in northbound's Meter table
> called fair that allows for a Meter to rate-limit each ACL log separately.
>
> The change is backward compatible. Based on the fair column of a Meter row,
> northd will turn a single Meter in the NB into multiple Meters in the SB
> by appending the ACL uuid to its name. It will also make action in logical
> flow use the unique Meter name for each affected ACL log. In order to
> make the Meter behave as described, add Meter with this option:
>
> ovn-nbctl --fair meter-add ${meter_name} drop ${rate} ${unit}
>
> Example:
>
> ovn-nbctl --fair meter-add meter_me drop 1 pktps
>
> Reported-by: Flavio Fernandes <flavio at flaviof.com>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2020-October/050769.html
> Suggested-by: Ben Pfaff <blp at ovn.org>
> Suggested-by: Dumitru Ceara <dceara at redhat.com>
> Suggested-by: Numan Siddique <numans at ovn.org>
> Signed-off-by: Flavio Fernandes <flavio at flaviof.com>
Thanks for addressing the comments. I applied this patch to master.
Since I only provide review comments
and didn't suggest any solution, I removed the suggested-by tag with my name :).
Numan
> ---
> This change can be tracked in the following github clone/branch:
>
> https://github.com/flavio-fernandes/ovn/commits/acl-meters.v6
>
> NEWS | 2 +
> northd/ovn-northd.c | 168 ++++++++++++++++++++++++++++++------------
> ovn-nb.ovsschema | 5 +-
> ovn-nb.xml | 16 +++-
> tests/ovn-nbctl.at | 6 +-
> tests/ovn-northd.at | 97 ++++++++++++++++++++++++
> utilities/ovn-nbctl.c | 16 +++-
> 7 files changed, 256 insertions(+), 54 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index a681573aa..18da14b18 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,8 @@ Post-v20.09.0
> traffic.
> - Propagate currently processed SB_Global.nb_cfg in ovn-controller to the
> local OVS DB integration bridge external_ids:ovn-nb-cfg.
> + - Add "fair" column in Meter table to allow multiple ACL logs to use the
> + same Meter while being rate-limited independently.
>
> OVN v20.09.0 - 28 Sep 2020
> --------------------------
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index d08a8a643..c2176f105 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -5265,8 +5265,46 @@ build_acl_hints(struct ovn_datapath *od, struct hmap *lflows)
> }
> }
>
> +static const struct nbrec_meter*
> +fair_meter_lookup_by_name(const struct shash *meter_groups,
> + const char *meter_name)
> +{
> + const struct nbrec_meter *nb_meter =
> + meter_name ? shash_find_data(meter_groups, meter_name) : NULL;
> + if (nb_meter) {
> + return (nb_meter->fair && *nb_meter->fair) ? nb_meter : NULL;
> + }
> + return NULL;
> +}
> +
> +static char*
> +alloc_acl_log_unique_meter_name(const struct nbrec_acl *acl)
> +{
> + return xasprintf("%s__" UUID_FMT,
> + acl->meter, UUID_ARGS(&acl->header_.uuid));
> +}
> +
> static void
> -build_acl_log(struct ds *actions, const struct nbrec_acl *acl)
> +build_acl_log_meter(struct ds *actions, const struct nbrec_acl *acl,
> + const struct shash *meter_groups)
> +{
> + if (!acl->meter) {
> + return;
> + }
> +
> + /* If ACL log meter uses a fair meter, use unique Meter name. */
> + if (fair_meter_lookup_by_name(meter_groups, acl->meter)) {
> + char *meter_name = alloc_acl_log_unique_meter_name(acl);
> + ds_put_format(actions, "meter=\"%s\", ", meter_name);
> + free(meter_name);
> + } else {
> + ds_put_format(actions, "meter=\"%s\", ", acl->meter);
> + }
> +}
> +
> +static void
> +build_acl_log(struct ds *actions, const struct nbrec_acl *acl,
> + const struct shash *meter_groups)
> {
> if (!acl->log) {
> return;
> @@ -5294,9 +5332,7 @@ build_acl_log(struct ds *actions, const struct nbrec_acl *acl)
> ds_put_cstr(actions, "verdict=allow, ");
> }
>
> - if (acl->meter) {
> - ds_put_format(actions, "meter=\"%s\", ", acl->meter);
> - }
> + build_acl_log_meter(actions, acl, meter_groups);
>
> ds_chomp(actions, ' ');
> ds_chomp(actions, ',');
> @@ -5307,7 +5343,8 @@ static void
> build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows,
> enum ovn_stage stage, struct nbrec_acl *acl,
> struct ds *extra_match, struct ds *extra_actions,
> - const struct ovsdb_idl_row *stage_hint)
> + const struct ovsdb_idl_row *stage_hint,
> + const struct shash *meter_groups)
> {
> struct ds match = DS_EMPTY_INITIALIZER;
> struct ds actions = DS_EMPTY_INITIALIZER;
> @@ -5319,7 +5356,7 @@ build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows,
> ingress ? ovn_stage_get_table(S_SWITCH_OUT_QOS_MARK)
> : ovn_stage_get_table(S_SWITCH_IN_L2_LKUP));
>
> - build_acl_log(&actions, acl);
> + build_acl_log(&actions, acl, meter_groups);
> if (extra_match->length > 0) {
> ds_put_format(&match, "(%s) && ", extra_match->string);
> }
> @@ -5344,7 +5381,8 @@ build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows,
>
> static void
> consider_acl(struct hmap *lflows, struct ovn_datapath *od,
> - struct nbrec_acl *acl, bool has_stateful)
> + struct nbrec_acl *acl, bool has_stateful,
> + const struct shash *meter_groups)
> {
> bool ingress = !strcmp(acl->direction, "from-lport") ? true :false;
> enum ovn_stage stage = ingress ? S_SWITCH_IN_ACL : S_SWITCH_OUT_ACL;
> @@ -5358,7 +5396,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
> * associated conntrack entry and would return "+invalid". */
> if (!has_stateful) {
> struct ds actions = DS_EMPTY_INITIALIZER;
> - build_acl_log(&actions, acl);
> + build_acl_log(&actions, acl, meter_groups);
> ds_put_cstr(&actions, "next;");
> ovn_lflow_add_with_hint(lflows, od, stage,
> acl->priority + OVN_ACL_PRI_OFFSET,
> @@ -5384,7 +5422,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
> ds_put_format(&match, REGBIT_ACL_HINT_ALLOW_NEW " == 1 && (%s)",
> acl->match);
> ds_put_cstr(&actions, REGBIT_CONNTRACK_COMMIT" = 1; ");
> - build_acl_log(&actions, acl);
> + build_acl_log(&actions, acl, meter_groups);
> ds_put_cstr(&actions, "next;");
> ovn_lflow_add_with_hint(lflows, od, stage,
> acl->priority + OVN_ACL_PRI_OFFSET,
> @@ -5403,7 +5441,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
> ds_put_format(&match, REGBIT_ACL_HINT_ALLOW " == 1 && (%s)",
> acl->match);
>
> - build_acl_log(&actions, acl);
> + build_acl_log(&actions, acl, meter_groups);
> ds_put_cstr(&actions, "next;");
> ovn_lflow_add_with_hint(lflows, od, stage,
> acl->priority + OVN_ACL_PRI_OFFSET,
> @@ -5428,10 +5466,10 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
> ds_put_cstr(&match, REGBIT_ACL_HINT_DROP " == 1");
> if (!strcmp(acl->action, "reject")) {
> build_reject_acl_rules(od, lflows, stage, acl, &match,
> - &actions, &acl->header_);
> + &actions, &acl->header_, meter_groups);
> } else {
> ds_put_format(&match, " && (%s)", acl->match);
> - build_acl_log(&actions, acl);
> + build_acl_log(&actions, acl, meter_groups);
> ds_put_cstr(&actions, "/* drop */");
> ovn_lflow_add_with_hint(lflows, od, stage,
> acl->priority + OVN_ACL_PRI_OFFSET,
> @@ -5455,10 +5493,10 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
> ds_put_cstr(&actions, "ct_commit { ct_label.blocked = 1; }; ");
> if (!strcmp(acl->action, "reject")) {
> build_reject_acl_rules(od, lflows, stage, acl, &match,
> - &actions, &acl->header_);
> + &actions, &acl->header_, meter_groups);
> } else {
> ds_put_format(&match, " && (%s)", acl->match);
> - build_acl_log(&actions, acl);
> + build_acl_log(&actions, acl, meter_groups);
> ds_put_cstr(&actions, "/* drop */");
> ovn_lflow_add_with_hint(lflows, od, stage,
> acl->priority + OVN_ACL_PRI_OFFSET,
> @@ -5471,9 +5509,9 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
> * logical flow action in all cases. */
> if (!strcmp(acl->action, "reject")) {
> build_reject_acl_rules(od, lflows, stage, acl, &match,
> - &actions, &acl->header_);
> + &actions, &acl->header_, meter_groups);
> } else {
> - build_acl_log(&actions, acl);
> + build_acl_log(&actions, acl, meter_groups);
> ds_put_cstr(&actions, "/* drop */");
> ovn_lflow_add_with_hint(lflows, od, stage,
> acl->priority + OVN_ACL_PRI_OFFSET,
> @@ -5553,7 +5591,7 @@ build_port_group_lswitches(struct northd_context *ctx, struct hmap *pgs,
>
> static void
> build_acls(struct ovn_datapath *od, struct hmap *lflows,
> - struct hmap *port_groups)
> + struct hmap *port_groups, const struct shash *meter_groups)
> {
> bool has_stateful = (has_stateful_acl(od) || has_lb_vip(od));
>
> @@ -5656,13 +5694,14 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
> /* Ingress or Egress ACL Table (Various priorities). */
> for (size_t i = 0; i < od->nbs->n_acls; i++) {
> struct nbrec_acl *acl = od->nbs->acls[i];
> - consider_acl(lflows, od, acl, has_stateful);
> + consider_acl(lflows, od, acl, has_stateful, meter_groups);
> }
> struct ovn_port_group *pg;
> HMAP_FOR_EACH (pg, key_node, port_groups) {
> if (ovn_port_group_ls_find(pg, &od->nbs->header_.uuid)) {
> for (size_t i = 0; i < pg->nb_pg->n_acls; i++) {
> - consider_acl(lflows, od, pg->nb_pg->acls[i], has_stateful);
> + consider_acl(lflows, od, pg->nb_pg->acls[i], has_stateful,
> + meter_groups);
> }
> }
> }
> @@ -7311,7 +7350,7 @@ build_lswitch_lflows_pre_acl_and_acl(struct ovn_datapath *od,
> build_pre_lb(od, lflows, meter_groups, lbs);
> build_pre_stateful(od, lflows);
> build_acl_hints(od, lflows);
> - build_acls(od, lflows, port_groups);
> + build_acls(od, lflows, port_groups, meter_groups);
> build_qos(od, lflows);
> build_lb(od, lflows);
> build_stateful(od, lflows, lbs);
> @@ -11502,12 +11541,50 @@ done:
> return need_update;
> }
>
> +static void
> +sync_meters_iterate_nb_meter(struct northd_context *ctx,
> + const char *meter_name,
> + const struct nbrec_meter *nb_meter,
> + struct shash *sb_meters)
> +{
> + bool new_sb_meter = false;
> +
> + const struct sbrec_meter *sb_meter = shash_find_and_delete(sb_meters,
> + meter_name);
> + if (!sb_meter) {
> + sb_meter = sbrec_meter_insert(ctx->ovnsb_txn);
> + sbrec_meter_set_name(sb_meter, 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);
> +}
> +
> /* 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.
> + * OVN_Southbound. Additionally, ACL logs that use fair meters have
> + * a private copy of its meter in the SB table.
> */
> static void
> -sync_meters(struct northd_context *ctx)
> +sync_meters(struct northd_context *ctx, struct hmap *datapaths,
> + struct shash *meter_groups)
> {
> struct shash sb_meters = SHASH_INITIALIZER(&sb_meters);
>
> @@ -11518,33 +11595,32 @@ sync_meters(struct northd_context *ctx)
>
> const struct nbrec_meter *nb_meter;
> NBREC_METER_FOR_EACH (nb_meter, ctx->ovnnb_idl) {
> - bool new_sb_meter = false;
> + sync_meters_iterate_nb_meter(ctx, nb_meter->name, nb_meter,
> + &sb_meters);
> + }
>
> - 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;
> + /*
> + * In addition to creating Meters in the SB from the block above, check
> + * and see if additional rows are needed to get ACLs logs individually
> + * rate-limited.
> + */
> + struct ovn_datapath *od;
> + HMAP_FOR_EACH (od, key_node, datapaths) {
> + if (!od->nbs) {
> + continue;
> }
> -
> - 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);
> + for (size_t i = 0; i < od->nbs->n_acls; i++) {
> + struct nbrec_acl *acl = od->nbs->acls[i];
> + nb_meter = fair_meter_lookup_by_name(meter_groups, acl->meter);
> + if (!nb_meter) {
> + continue;
> }
> - sbrec_meter_set_bands(sb_meter, sb_bands, nb_meter->n_bands);
> - free(sb_bands);
> - }
>
> - sbrec_meter_set_unit(sb_meter, nb_meter->unit);
> + char *meter_name = alloc_acl_log_unique_meter_name(acl);
> + sync_meters_iterate_nb_meter(ctx, meter_name, nb_meter,
> + &sb_meters);
> + free(meter_name);
> + }
> }
>
> struct shash_node *node, *next;
> @@ -12037,7 +12113,7 @@ ovnnb_db_run(struct northd_context *ctx,
>
> sync_address_sets(ctx);
> sync_port_groups(ctx, &port_groups);
> - sync_meters(ctx);
> + sync_meters(ctx, datapaths, &meter_groups);
> sync_dns_entries(ctx, datapaths);
>
> struct ovn_northd_lb *lb;
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index 092322ab2..269e3a888 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Northbound",
> - "version": "5.27.0",
> - "cksum": "3507518247 26773",
> + "version": "5.28.0",
> + "cksum": "610359755 26847",
> "tables": {
> "NB_Global": {
> "columns": {
> @@ -264,6 +264,7 @@
> "refType": "strong"},
> "min": 1,
> "max": "unlimited"}},
> + "fair": {"type": {"key": "boolean", "min": 0, "max": 1}},
> "external_ids": {
> "type": {"key": "string", "value": "string",
> "min": 0, "max": "unlimited"}}},
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 619c0e299..a457b9cee 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -1785,7 +1785,9 @@
> The name of a meter to rate-limit log messages for the ACL.
> The string must match the <ref column="name" table="meter"/>
> column of a row in the <ref table="Meter"/> table. By
> - default, log messages are not rate-limited.
> + default, log messages are not rate-limited. In order to ensure
> + that the same <ref table="Meter"/> rate limits multiple ACL logs
> + separately, set the <ref column="fair" table="meter"/> column.
> </p>
> </column>
> </group>
> @@ -2089,6 +2091,18 @@
> </p>
> </column>
>
> + <column name="fair">
> + <p>
> + This column is used to further describe the desired behavior
> + of the meter when there are multiple references to it. If this
> + column is empty or is set to <code>false</code>, the rate will
> + be shared across all rows that refer to the same Meter
> + <ref column="name" table="meter"/>. Conversely, when this column
> + is set to <code>true</code>, each user of the same Meter will be
> + rate-limited on its own.
> + </p>
> + </column>
> +
> <column name="external_ids">
> See <em>External IDs</em> at the beginning of this document.
> </column>
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index 79d580d3f..11091d8d0 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -346,7 +346,7 @@ OVN_NBCTL_TEST([ovn_nbctl_meters], [meters], [
> AT_CHECK([ovn-nbctl meter-add meter1 drop 10 kbps])
> AT_CHECK([ovn-nbctl meter-add meter2 drop 3 kbps 2])
> AT_CHECK([ovn-nbctl meter-add meter3 drop 100 kbps 200])
> -AT_CHECK([ovn-nbctl meter-add meter4 drop 10 pktps 30])
> +AT_CHECK([ovn-nbctl --fair meter-add meter4 drop 10 pktps 30])
>
> dnl Add duplicate meter name
> AT_CHECK([ovn-nbctl meter-add meter1 drop 10 kbps], [1], [], [stderr])
> @@ -382,7 +382,7 @@ meter2: bands:
> drop: 3 kbps, 2 kb burst
> meter3: bands:
> drop: 100 kbps, 200 kb burst
> -meter4: bands:
> +meter4: (fair) bands:
> drop: 10 pktps, 30 packet burst
> ])
>
> @@ -393,7 +393,7 @@ meter1: bands:
> drop: 10 kbps
> meter3: bands:
> drop: 100 kbps, 200 kb burst
> -meter4: bands:
> +meter4: (fair) bands:
> drop: 10 pktps, 30 packet burst
> ])
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 85f61f357..ef77c5318 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -1813,6 +1813,103 @@ action=(reg0 = 0; reject { /* eth.dst <-> eth.src; ip.dst <-> ip.src; is implici
>
> AT_CLEANUP
>
> +AT_SETUP([ovn -- ACL fair Meters])
> +AT_KEYWORDS([acl log meter fair])
> +ovn_start
> +
> +check ovn-nbctl ls-add sw0
> +check ovn-nbctl lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1 "50:54:00:00:00:01 10.0.0.11"
> +check ovn-nbctl lsp-add sw0 sw0-p2 -- lsp-set-addresses sw0-p2 "50:54:00:00:00:02 10.0.0.12"
> +check ovn-nbctl lsp-add sw0 sw0-p3 -- lsp-set-addresses sw0-p3 "50:54:00:00:00:03 10.0.0.13"
> +
> +check ovn-nbctl meter-add meter_me drop 1 pktps
> +nb_meter_uuid=$(fetch_column nb:Meter _uuid name=meter_me)
> +
> +check ovn-nbctl acl-add sw0 to-lport 1002 'outport == "sw0-p1" && ip4.src == 10.0.0.12' allow
> +check ovn-nbctl acl-add sw0 to-lport 1002 'outport == "sw0-p1" && ip4.src == 10.0.0.13' allow
> +
> +acl1=$(ovn-nbctl --bare --column _uuid,match find acl | grep -B1 '10.0.0.12' | head -1)
> +acl2=$(ovn-nbctl --bare --column _uuid,match find acl | grep -B1 '10.0.0.13' | head -1)
> +check ovn-nbctl set acl $acl1 log=true severity=alert meter=meter_me name=acl_one
> +check ovn-nbctl set acl $acl2 log=true severity=info meter=meter_me name=acl_two
> +check ovn-nbctl --wait=sb sync
> +
> +check_row_count nb:meter 1
> +check_column meter_me nb:meter name
> +
> +check_acl_lflow() {
> + acl_log_name=$1
> + meter_name=$2
> + # echo checking that logical flow for acl log $acl_log_name has $meter_name
> + AT_CHECK([ovn-sbctl lflow-list | grep ls_out_acl | \
> + grep "\"${acl_log_name}\"" | \
> + grep -c "meter=\"${meter_name}\""], [0], [1
> +])
> +}
> +
> +check_meter_by_name() {
> + [test "$1" = "NOT"] && { expected_count=0; shift; } || expected_count=1
> + for meter_name in $* ; do
> + # echo checking for $expected_count $meter_name in sb meter table
> + check_row_count meter $expected_count name=$meter_name
> + done
> +}
> +
> +# Make sure 'fair' value properly affects the Meters in SB
> +check_meter_by_name meter_me
> +check_meter_by_name NOT meter_me__${acl1} meter_me__${acl2}
> +
> +check ovn-nbctl --wait=sb set Meter $nb_meter_uuid fair=true
> +check_meter_by_name meter_me meter_me__${acl1} meter_me__${acl2}
> +
> +check ovn-nbctl --wait=sb set Meter $nb_meter_uuid fair=false
> +check_meter_by_name meter_me
> +check_meter_by_name NOT meter_me__${acl1} meter_me__${acl2}
> +
> +check ovn-nbctl --wait=sb set Meter $nb_meter_uuid fair=true
> +check_meter_by_name meter_me meter_me__${acl1} meter_me__${acl2}
> +
> +# Change template meter and make sure that is reflected on acl meters as well
> +template_band=$(fetch_column nb:meter bands name=meter_me)
> +check ovn-nbctl --wait=sb set meter_band $template_band rate=123
> +# Make sure that every Meter_Band has the right rate. (ovn-northd
> +# creates 3 identical Meter_Band rows, all identical; ovn-northd-ddlog
> +# creates just 1. It doesn't matter, they work just as well.)
> +n_meter_bands=$(count_rows meter_band)
> +AT_FAIL_IF([test "$n_meter_bands" != 1 && test "$n_meter_bands" != 3])
> +check_row_count meter_band $n_meter_bands rate=123
> +
> +# Check meter in logical flows for acl logs
> +check_acl_lflow acl_one meter_me__${acl1}
> +check_acl_lflow acl_two meter_me__${acl2}
> +
> +# Stop using meter for acl1
> +check ovn-nbctl --wait=sb clear acl $acl1 meter
> +check_meter_by_name meter_me meter_me__${acl2}
> +check_meter_by_name NOT meter_me__${acl1}
> +check_acl_lflow acl_two meter_me__${acl2}
> +
> +# Remove template Meter should remove all others as well
> +check ovn-nbctl --wait=sb meter-del meter_me
> +check_row_count meter 0
> +# Check that logical flow remains but uses non-unique meter since fair
> +# attribute is lost by the removal of the Meter row.
> +check_acl_lflow acl_two meter_me
> +
> +# Re-add template meter and make sure acl2's meter is back in sb
> +check ovn-nbctl --wait=sb --fair meter-add meter_me drop 1 pktps
> +check_meter_by_name meter_me meter_me__${acl2}
> +check_meter_by_name NOT meter_me__${acl1}
> +check_acl_lflow acl_two meter_me__${acl2}
> +
> +# Remove acl2
> +sw0=$(fetch_column nb:logical_switch _uuid name=sw0)
> +check ovn-nbctl --wait=sb remove logical_switch $sw0 acls $acl2
> +check_meter_by_name meter_me
> +check_meter_by_name NOT meter_me__${acl1} meter_me__${acl2}
> +
> +AT_CLEANUP
> +
> AT_SETUP([datapath requested-tnl-key])
> AT_KEYWORDS([requested tnl tunnel key keys])
> ovn_start
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index b5633c725..11c6b5576 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -617,6 +617,7 @@ QoS commands:\n\
> qos-list SWITCH print QoS rules for SWITCH\n\
> \n\
> Meter commands:\n\
> + [--fair]\n\
> meter-add NAME ACTION RATE UNIT [BURST]\n\
> add a meter\n\
> meter-del [NAME] remove meters\n\
> @@ -2693,7 +2694,12 @@ nbctl_meter_list(struct ctl_context *ctx)
>
> for (size_t i = 0; i < n_meters; i++) {
> meter = meters[i];
> - ds_put_format(&ctx->output, "%s: bands:\n", meter->name);
> + ds_put_format(&ctx->output, "%s:", meter->name);
> + if (meter->fair) {
> + ds_put_format(&ctx->output, " (%s)",
> + *meter->fair ? "fair" : "shared");
> + }
> + ds_put_format(&ctx->output, " bands:\n");
>
> for (size_t j = 0; j < meter->n_bands; j++) {
> const struct nbrec_meter_band *band = meter->bands[j];
> @@ -2770,6 +2776,12 @@ nbctl_meter_add(struct ctl_context *ctx)
> nbrec_meter_set_name(meter, name);
> nbrec_meter_set_unit(meter, unit);
> nbrec_meter_set_bands(meter, &band, 1);
> +
> + /* Fair option */
> + bool fair = shash_find(&ctx->options, "--fair") != NULL;
> + if (fair) {
> + nbrec_meter_set_fair(meter, &fair, 1);
> + }
> }
>
> static void
> @@ -6464,7 +6476,7 @@ static const struct ctl_command_syntax nbctl_commands[] = {
>
> /* meter commands. */
> { "meter-add", 4, 5, "NAME ACTION RATE UNIT [BURST]", NULL,
> - nbctl_meter_add, NULL, "", RW },
> + nbctl_meter_add, NULL, "--fair", RW },
> { "meter-del", 0, 1, "[NAME]", NULL, nbctl_meter_del, NULL, "", RW },
> { "meter-list", 0, 0, "", NULL, nbctl_meter_list, NULL, "", RO },
>
> --
> 2.18.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list