On Wed, Apr 29, 2020 at 3:34 PM Numan Siddique <num...@ovn.org> wrote:
> > > On Mon, Apr 27, 2020 at 9:15 PM Lorenzo Bianconi < > lorenzo.bianc...@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 <mjoze...@redhat.com> >> Acked-by: Han Zhou <hz...@ovn.org> >> Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com> >> > > Thanks Lorenzo and Han. > I applied this patch to master. > And also to branch-20.03 Thanks Numan > > 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 >> 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