| Message ID | 0326dd3f348095913d12014988640f28bd41bbc3.1587735081.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,ovn] Rely on unique name for ovn qos meters | expand |
On Fri, Apr 24, 2020 at 6:33 AM Lorenzo Bianconi < lorenzo.bianconi@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@redhat.com> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > controller/ofctrl.c | 2 +- > lib/actions.c | 11 ++++++----- > 2 files changed, 7 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 deca18842..20a7a5caf 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, > -- > 2.25.3 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev Thanks for the fix. Looks good to me. It would be great if you could add/update QoS test case in ovn.at to cover this scenario. Acked-by: Han Zhou <hzhou@ovn.org>
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 deca18842..20a7a5caf 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,