Hi Han,
Thank you for your patience in answering my questions. I agree with
your comments.
I am interested in "specifying uuid when creating a row is not yet
supported by the ctl tools", I will try to summit patch to support it . Do you
have some sugesstion?
Bye the way, how to reply email with automaticly incresing ">" or
">>.....". :):)
Thanks,
YUN
From: Han Zhou
Date: 2020-01-11 01:54
To: [email protected]
CC: Han Zhou; ovs-dev
Subject: Re: Re: [ovs-dev] [PATCH ovn v4 0/2] Add a way to delete QoS directly
Hi Yun,
On Fri, Jan 10, 2020 at 3:10 AM [email protected]
<[email protected]> wrote:
>
> Hi Han,
> Thanks for your reply. I agree with your following comment 1. For
> the comment 2, I think Ben' s patch will make "UUID" and "name" be similar, I
> don't know the reason for summit this patch, I have comment for that patch in
> another email.
>
Ben's patch "Allow OVSDB clients to specify the UUID for inserted rows" was
initially intended for DDlog's use case to reduce memory footprint in DDlog.
However, I think it is very useful for many other OVSDB clients, such as
networking-ovn.
With this feature, clients doesn't need to store a mapping between its own
objects and the rows in OVSDB, either by storing the its own ID in
external-ids, or by saving the mapping in its own DB. Instead, clients can
generate the object uuid and tell OVSDB to use the same uuid as the row uuid. I
feel it is also the feature you are looking for to solve the efficiency of QoS
operations.
I agree that you will need some work in networking-ovn to benefit from this
feature.
> Enventually, If we use Ben's patch to record mappings between OVN
> and Neutron,we may also need to finish two jobs:
> 1. A patch to make QoS could be search by UUID, which not
> supported now, has finished in my patch 2.
For read/update/delete operations, OVSDB already supports directly using uuid
to identify a row, and all the ovs/ovn-xxctl tools supports this (see man page
of any ctl tool, e.g. man ovn-nbctl, in section "Database Commands"). So even
without any change, users are able to operating any records using uuid.
However, I agree that it is more user-friendly if we add the support of
operations on uuid to the QoS related commands. And also, specifying uuid when
creating a row is not yet supported by the ctl tools (I have mentioned it in
the code review, and I think it should be an easy task to add it).
> 2. Alter the methods which has used "name" to record the
> mappings .
Do you mean in OVN or in Neutron networking-ovn? In general I don't think we
need to alter any existing methods, because they will just continue to work. I
agree that we will need some work in networking-ovn to benefit from this
feature, as an optimization, such as for QoS operations. And eventually we may
convert other operations in networking-ovn to benefit from this feature, if
needed.
Before this feature can be used by networking-ovn, probably the first thing to
do is to update the python ovsdbapp to support specifying uuid when creating a
record.
>
> What do you think? :)
>
> Thanks,
> Yun
>
> > 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/
>
>
> From: Han Zhou
> Date: 2020-01-10 12:03
> To: [email protected]
> CC: ovs-dev
> Subject: Re: Re: [ovs-dev] [PATCH ovn v4 0/2] Add a way to delete QoS directly
> Sorry for the late reply. Please see my response below.
>
> On Wed, Jan 8, 2020 at 5:16 PM [email protected]
> <[email protected]> wrote:
> >
> > Hi Han,
> > If you have time, Could you review this patch or give a
> > response.
> >
> >
> > Thanks ,
> > Yun
> >
> >
> >
> > From: [email protected]
> > 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
> > directly
> > 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
> > OVN.
>
> 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
> name.
> 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 deletion.
> > 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]
> > https://github.com/ovn-org/ovn/commit/17df18dac9d2fa875c2f86a54bbc84931689d42c
> > [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
> > <[email protected]> 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 table,
> > > 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?
> >
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
