[ovs-dev] [PATCH ovn v4 0/2] Add a way to delete QoS directly

Han Zhou hzhou at ovn.org
Fri Jan 10 04:03:01 UTC 2020

Sorry for the late reply. Please see my response below.

On Wed, Jan 8, 2020 at 5:16 PM taoyunxiang at cmss.chinamobile.com <
taoyunxiang at cmss.chinamobile.com> wrote:
> Hi Han,
>             If you have time, Could you review this patch or give a
> Thanks ,
> Yun
> From: taoyunxiang at cmss.chinamobile.com
> Date: 2019-12-24 19:55
> To: Han Zhou
> CC: ovs-dev
> Subject: Re: Re: [ovs-dev] [PATCH ovn v4 0/2] Add a way to delete QoS
> Hi Han,
>      Thanks for your review!
>      1. As for point 1 and point 4, I think it is not bad  to use
external_ids to record association between Neutron and OVN, even we can
record LS info also.  But it is not directly and clearly.
>          Actually, I find  almost resource tables  have "name" column,
included ACL table, which summited by this patch[0]. The purpose of adding
"name" for ACL, is also to make ACL rule could be easily find and check in

Ok, the name column of ACL table was added for the logging purpose, but I
agree with you it can be used the way you intended to.

>      2. As for point 2, I am agreed, but do not know how to do now.

There are several ways.
Option1: Keep the old command and add a new command for deleting by name.
Option2: Add an option to the old command such as "--by-name" to delete by
Option3: Keep the SWITCH arg when deleting by name, but only for the given
switch. If the next argument can be found as a QoS name in the switch, it
is treated as delete by name, otherwise, it is handled as before.

>      3. As for point 3,  I think we can record  LS info, it is not bad.
>      As for why I want to change "qos-del" comand,  I want to explain
more  detailed.  Hope to get your advice.
>     1. QoS rule resource of Neutron is not mapping any resource of OVN,
and it just contains rate, burst, dircetion and so on.
>     2. In Neutron, when we bind QoS rule to port or network, it will
triger OVN to record in QoS table.
>     3. When we update QoS rule in Neutron , it will DIRECTLY update its
record of  Neutron DB(will not wait OVN to update, because no mapping
resource),  gradually, it will update port or network which binded this QoS
rule. But the QoS rule has already become the lasted one in Neutron DB, and
networking-ovn can not get the old  QoS rule of Neutron.  So we can not
update it exactly.
>     4. For the above situaiton, we can only delete old rules that have
the same direction as the new rules, or , we will expand the scope of
>       This patch [1]  is the logic process for networking-ovn to execute
QoS rule.  The update process is not exactly and suitable, I think the Core
OVN may need some changes.

I agree with you that it is more efficient to delete with an ID instead of
searching based on fields. I think we need to consider below two factors:
1. If the new "name" column is supposed to uniquely identify a row, it is
better to add the "index" constraint in the schema for this new column, to
avoid duplicated names.
2. Ben recently worked on the new feature of OVSDB to make it allows client
to specify row uuid when creating a row [0]. If this patch is merged, we
don't need a "name" column to uniquely identify a row any more. I'd prefer
to utilize this new feature and to avoid adding new "name" columns for
individual tables. Do you think this is helpful?

[0] https://patchwork.ozlabs.org/patch/1220644/

>       [0]
>       [1] https://review.opendev.org/#/c/692084/
>       Hope to have your advice.
> Thanks,
> Yun
> From: Han Zhou
> Date: 2019-12-24 01:33
> To: Yunxiang Tao
> CC: ovs-dev
> Subject: Re: [ovs-dev] [PATCH ovn v4 0/2] Add a way to delete QoS directly
> On Mon, Dec 23, 2019 at 2:37 AM Yunxiang Tao <
taoyunxiang at cmss.chinamobile.com> wrote:
> >
> > Currently, qos can only be deleted by indirect way which must designate
> > switch or more parameters. The first patch add "name" column to QoS
> > and make QoS table could be listed by "name" witch comand
> > "ovn-nbctl list qos "name"".
> >
> > The second patch change the original "qos-del" to "ls-qos-del",  add
> > a new "qos-del" command. By the new command, you can delete qos
> > by uuid or name of the qos. It is useful. For example, we can associate
> > the qos table in neutron and OVN by "name" of qos in OVN, so neutron
> > could find and easily delete the corresponding qos in OVN.
> >
> Thanks for the patch. I have below comments:
> 1. There are other ways to associate QoS between Neutron and OVN. For
example, the external-ids column can be used to add any customized
key-value pairs, such as external_ids:neutron:qos_name = <neutron qos
name>. Is this sufficient for your use case?
> 2. If we believe it is useful for qos-del command to delete qos by
name/uuid, it is better to extend the exist command "qos-del" to handle
both old and new formats, and we should NOT rename the existing command
because it will break existing customer tools.
> 3. It is more efficient to specify the lswitch in the qos-del command so
that it doesn't need to iterate every lswitch. Is there a use case that
neutron needs to delete a QoS record without knowing which lswitch should
it be deleted from?
> 4. QoS and ACL are implemented in similar way with similar principles. If
"name" is needed in QoS table, probably it is also needed in ACL table. In
other words, if we can handle ACL well without introducing the "name"
column, probably we could do the same for QoS table without problem. What's
your thought on this?

More information about the dev mailing list