On Wed, Oct 27, 2021 at 10:54 AM Lorenzo Bianconi
<lorenzo.bianc...@redhat.com> wrote:
>
> Add --may-exist support to meter-add command in order to update
> meter parameters if it has been already created
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
> ---
> tests/ovn-nbctl.at | 6 +++--
> tests/ovn-northd.at | 11 +++++++++-
> utilities/ovn-nbctl.c | 51 ++++++++++++++++++++++++-------------------
> 3 files changed, 43 insertions(+), 25 deletions(-)
>
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index 9b80ae410..a8946fef8 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -371,6 +371,8 @@ dnl Add duplicate meter name
> AT_CHECK([ovn-nbctl meter-add meter1 drop 10 kbps], [1], [], [stderr])
> AT_CHECK([grep 'already exists' stderr], [0], [ignore])
>
> +AT_CHECK([ovn-nbctl --may-exist meter-add meter1 drop 11 kbps])
> +
> dnl Add reserved meter name
> AT_CHECK([ovn-nbctl meter-add __meter1 drop 10 kbps], [1], [], [stderr])
> AT_CHECK([grep 'reserved' stderr], [0], [ignore])
> @@ -396,7 +398,7 @@ AT_CHECK([ovn-nbctl meter-add meter5 drop 10 100010111111
> kbps], [1], [],
>
> AT_CHECK([ovn-nbctl meter-list], [0], [dnl
> meter1: bands:
> - drop: 10 kbps
> + drop: 11 kbps
> meter2: bands:
> drop: 3 kbps, 2 kb burst
> meter3: bands:
> @@ -409,7 +411,7 @@ dnl Delete a single meter.
> AT_CHECK([ovn-nbctl meter-del meter2])
> AT_CHECK([ovn-nbctl meter-list], [0], [dnl
> meter1: bands:
> - drop: 10 kbps
> + drop: 11 kbps
> meter3: bands:
> drop: 100 kbps, 200 kb burst
> meter4: (fair) bands:
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index e2b9924b6..ca1b8a117 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -3235,7 +3235,16 @@ event-elb: meter0
>
> AT_CHECK([ovn-sbctl list logical_flow | grep trigger_event -A 2 | grep -q
> meter0])
>
> -check ovn-nbctl --wait=hv meter-add meter1 drop 200 pktps 10
> +check ovn-nbctl --wait=hv meter-add meter1 drop 300 pktps 10
> +AT_CHECK([ovn-nbctl meter-list |grep meter1 -A 1], [0], [dnl
> +meter1: bands:
> + drop: 300 pktps, 10 packet burst
> +])
> +check ovn-nbctl --wait=hv --may-exist meter-add meter1 drop 200 pktps 10
> +AT_CHECK([ovn-nbctl meter-list |grep meter1 -A 1], [0], [dnl
> +meter1: bands:
> + drop: 200 pktps, 10 packet burst
> +])
> check ovn-nbctl --wait=hv lr-copp-add r0 arp meter1
> AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
> arp: meter1
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index b04b24d4b..1f71cae46 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -2585,19 +2585,6 @@ meter_cmp(const void *meter1_, const void *meter2_)
> return strcmp(meter1->name, meter2->name);
> }
>
> -static void
> -nbctl_pre_meter_list(struct ctl_context *ctx)
> -{
> - ovsdb_idl_add_column(ctx->idl, &nbrec_meter_col_name);
> - ovsdb_idl_add_column(ctx->idl, &nbrec_meter_col_fair);
> - ovsdb_idl_add_column(ctx->idl, &nbrec_meter_col_bands);
> - ovsdb_idl_add_column(ctx->idl, &nbrec_meter_col_unit);
> -
> - ovsdb_idl_add_column(ctx->idl, &nbrec_meter_band_col_action);
> - ovsdb_idl_add_column(ctx->idl, &nbrec_meter_band_col_rate);
> - ovsdb_idl_add_column(ctx->idl, &nbrec_meter_band_col_burst_size);
> -}
> -
Hi Lorenzo,
Thanks for the patch. I applied this patch to the main branch with one change.
I kept the function nbctl_pre_meter_list() AS IS even though
nbctl_pre_meter_add()
are exactly the same. I felt it's a bit odd to call
nbctl_pre_meter_add() in the meter-list pre check command.
Thanks
Numan
> static void
> nbctl_meter_list(struct ctl_context *ctx)
> {
> @@ -2650,18 +2637,34 @@ static void
> nbctl_pre_meter_add(struct ctl_context *ctx)
> {
> ovsdb_idl_add_column(ctx->idl, &nbrec_meter_col_name);
> + ovsdb_idl_add_column(ctx->idl, &nbrec_meter_col_fair);
> + ovsdb_idl_add_column(ctx->idl, &nbrec_meter_col_bands);
> + ovsdb_idl_add_column(ctx->idl, &nbrec_meter_col_unit);
> +
> + ovsdb_idl_add_column(ctx->idl, &nbrec_meter_band_col_action);
> + ovsdb_idl_add_column(ctx->idl, &nbrec_meter_band_col_rate);
> + ovsdb_idl_add_column(ctx->idl, &nbrec_meter_band_col_burst_size);
> }
>
> static void
> nbctl_meter_add(struct ctl_context *ctx)
> {
> - const struct nbrec_meter *meter;
> + const struct nbrec_meter *meter = NULL, *iter;
> + struct nbrec_meter_band *band = NULL;
>
> const char *name = ctx->argv[1];
> - NBREC_METER_FOR_EACH (meter, ctx->idl) {
> - if (!strcmp(meter->name, name)) {
> - ctl_error(ctx, "meter with name \"%s\" already exists", name);
> - return;
> + NBREC_METER_FOR_EACH (iter, ctx->idl) {
> + if (!strcmp(iter->name, name)) {
> + if (!shash_find(&ctx->options, "--may-exist")) {
> + ctl_error(ctx, "meter with name \"%s\" already exists",
> name);
> + return;
> + } else {
> + meter = iter;
> + if (meter->n_bands) {
> + band = meter->bands[0];
> + }
> + break;
> + }
> }
> }
>
> @@ -2699,13 +2702,17 @@ nbctl_meter_add(struct ctl_context *ctx)
> }
>
> /* Create the band. We only support adding a single band. */
> - struct nbrec_meter_band *band = nbrec_meter_band_insert(ctx->txn);
> + if (!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);
>
> /* Create the meter. */
> - meter = nbrec_meter_insert(ctx->txn);
> + if (!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);
> @@ -6853,10 +6860,10 @@ static const struct ctl_command_syntax
> nbctl_commands[] = {
>
> /* meter commands. */
> { "meter-add", 4, 5, "NAME ACTION RATE UNIT [BURST]",
> nbctl_pre_meter_add,
> - nbctl_meter_add, NULL, "--fair", RW },
> + nbctl_meter_add, NULL, "--fair,--may-exist", RW },
> { "meter-del", 0, 1, "[NAME]", nbctl_pre_meter_del, nbctl_meter_del,
> NULL, "", RW },
> - { "meter-list", 0, 0, "", nbctl_pre_meter_list, nbctl_meter_list,
> + { "meter-list", 0, 0, "", nbctl_pre_meter_add, nbctl_meter_list,
> NULL, "", RO },
>
> /* logical switch port commands. */
> --
> 2.31.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
