From ab7a370f24ec88a019f1aa4da76f1a050bf398c6 Mon Sep 17 00:00:00 2001
Message-Id: <ab7a370f24ec88a019f1aa4da76f1a050bf398c6.1588167268.git.lorenzo.bianconi@redhat.com>
From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date: Mon, 27 Apr 2020 17:45:20 +0200
Subject: [PATCH] Rely on unique name for ovn qos meters

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@redhat.com>
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
---
 controller/ofctrl.c |  2 +-
 lib/actions.c       | 11 ++++++-----
 tests/ovn.at        | 10 ++++++++++
 3 files changed, 17 insertions(+), 6 deletions(-)

--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -970,7 +970,7 @@ add_meter_string(struct ovn_extend_table
     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) {
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -2796,12 +2796,13 @@ encode_SET_METER(const struct ovnact_set
      * 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,
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -7653,6 +7653,16 @@ AT_CHECK([as hv ovs-ofctl dump-flows br-
 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