[ovs-dev] [PATCH v2 ovn] Rely on unique name for ovn qos meters
Numan Siddique
numans at ovn.org
Wed Apr 29 10:33:26 UTC 2020
On Wed, Apr 29, 2020 at 3:34 PM Numan Siddique <numans at ovn.org> wrote:
>
>
> 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.
>
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
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
More information about the dev
mailing list