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

[Lorenzo Bianconi]

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>
---
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