[ovs-dev] [PATCH v2 ovn] Rely on unique name for ovn qos meters

[Numan Siddique]

On Mon, Apr 27, 2020 at 9:15 PM Lorenzo Bianconi <
lorenzo.bianconi at redhat.com> wrote:

> ovn currently identifies qos meters according to the rate and burst values
> configured. Doing so 2 meters on the same hv assigned to 2 different
> logical
> switch ports and configured with the same values for rate and burst will be
> mapped to the same ovs kernel mater and will share the bandwidth.
> Fix this behavior making qos meter name unique
>
> Tested-By: Maciej Jozefczyk <mjozefcz at redhat.com>
> Acked-by: Han Zhou <hzhou at ovn.org>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
>

Thanks Lorenzo and Han.
I applied this patch to master.

Numan


> ---
> Changes since v1:
> - added unit test
> ---
>  controller/ofctrl.c |  2 +-
>  lib/actions.c       | 11 ++++++-----
>  tests/ovn.at        | 10 ++++++++++
>  3 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/controller/ofctrl.c b/controller/ofctrl.c
> index 36e39ba06..485a857d1 100644
> --- a/controller/ofctrl.c
> +++ b/controller/ofctrl.c
> @@ -970,7 +970,7 @@ add_meter_string(struct ovn_extend_table_info
> *m_desired,
>      enum ofputil_protocol usable_protocols;
>      char *meter_string = xasprintf("meter=%"PRIu32",%s",
>                                     m_desired->table_id,
> -                                   &m_desired->name[9]);
> +                                   &m_desired->name[52]);
>      char *error = parse_ofp_meter_mod_str(&mm, meter_string, OFPMC13_ADD,
>                                            &usable_protocols);
>      if (!error) {
> diff --git a/lib/actions.c b/lib/actions.c
> index c19813de0..6ba392ec1 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -2851,12 +2851,13 @@ encode_SET_METER(const struct ovnact_set_meter *cl,
>       * describes the meter itself. */
>      char *name;
>      if (cl->burst) {
> -        name = xasprintf("__string: kbps burst stats bands=type=drop "
> -                         "rate=%"PRId64" burst_size=%"PRId64"", cl->rate,
> -                         cl->burst);
> +        name = xasprintf("__string: uuid "UUID_FMT" kbps burst stats "
> +                         "bands=type=drop rate=%"PRId64"
> burst_size=%"PRId64,
> +                         UUID_ARGS(&ep->lflow_uuid), cl->rate, cl->burst);
>      } else {
> -        name = xasprintf("__string: kbps stats bands=type=drop "
> -                         "rate=%"PRId64"", cl->rate);
> +        name = xasprintf("__string: uuid "UUID_FMT" kbps stats "
> +                         "bands=type=drop rate=%"PRId64,
> +                         UUID_ARGS(&ep->lflow_uuid), cl->rate);
>      }
>
>      table_id = ovn_extend_table_assign_id(ep->meter_table, name,
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 2dd2f9211..3d9868824 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -7678,6 +7678,16 @@ AT_CHECK([as hv ovs-ofctl dump-flows br-int -O
> OpenFlow13 | grep meter | wc -l],
>  AT_CHECK([as hv ovs-ofctl dump-meters br-int -O OpenFlow13 | grep
> rate=11123 | wc -l], [0], [0
>  ])
>
> +# Check multiple qos meters
> +ovn-nbctl qos-del lsw0
> +ovn-nbctl qos-add lsw0 to-lport 1001 'inport=="lp1" &&
> is_chassis_resident("lp1")' rate=100000 burst=100000
> +ovn-nbctl qos-add lsw0 to-lport 1001 'inport=="lp2" &&
> is_chassis_resident("lp2")' rate=100000 burst=100000
> +ovn-nbctl qos-add lsw0 to-lport 1002 'inport=="lp1" &&
> is_chassis_resident("lp1")' rate=100001 burst=100001
> +ovn-nbctl qos-add lsw0 to-lport 1002 'inport=="lp2" &&
> is_chassis_resident("lp2")' rate=100001 burst=100001
> +
> +AT_CHECK([as hv ovs-ofctl dump-meters br-int -O OpenFlow13 | grep meter |
> wc -l], [0], [4
> +])
> +
>  OVN_CLEANUP([hv])
>  AT_CLEANUP
>
> --
> 2.25.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>