[ovs-dev] [PATCH v3] ovn-controller: Omit alert for Port_Binding.external_ids changes

Numan Siddique nusiddiq at redhat.com
Thu Jun 27 14:53:36 UTC 2019


On Thu, Jun 27, 2019 at 2:33 AM Ben Pfaff <blp at ovn.org> wrote:

> On Thu, Jun 27, 2019 at 12:43:18AM +0530, Numan Siddique wrote:
> > On Thu, Jun 27, 2019 at 12:19 AM Ben Pfaff <blp at ovn.org> wrote:
> >
> > > On Wed, Jun 26, 2019 at 07:46:25PM +0530, nusiddiq at redhat.com wrote:
> > > > From: Numan Siddique <nusiddiq at redhat.com>
> > > >
> > > > Running the command "ovn-nbctl set logical_switch_port foo
> > > external_ids:foo=bar"
> > > > results in the incremetal processing engine to recompute the flows
> on the
> > > > chassis where the logical port 'foo' is claimed.
> > > >
> > > > This patch avoids this unnecessary recomputation by omitting the
> alert
> > > for the
> > > > Port_Binding.external_ids column.
> > > >
> > > > Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
> > > > Acked-by: Han Zhou <hzhou8 at ebay.com>
> > > > ---
> > > >
> > > > v2 -> v3
> > > > ------
> > > >   * Based on the review comments from Han, dropped p2 and p3 from the
> > > series.
> > >
> > > This only omits the alert for external-ids from one table.  I think
> that
> > > ovn-controller should not look at the external_ids in most tables.  The
> > > only ones I see it dealing with are: the one on Chassis, which seems
> > > legitimate (it is only setting keys there, and the behavior is
> > > documented) and external_ids:dns_id in the DNS table, which makes me
> > > suspicious because it appears that it's an undocumented communication
> > > channel from ovn-northd to ovn-controller.  I think that all the others
> > > can omit alerts.
> > >
> >
> > To sync the dns entries from nb db to sb db, we store the uuid of nb db
> dns
> > row in sb db.
> > I will submit a doc patch for this. Thanks for pointing this out.
>
> OK.
>
> external-ids fields are intended for use by higher-level systems
> (e.. ovn-northd in the case of sbdb) to keep notes on how to associate
> entities in a database with corresponding higher-level entities
> (e.g. nbdb, in this case).  This means that the lower-level software
> (e.g. ovn-controller, in this case) don't normally read the external-ids
> fields since they don't have any way to interpret it.  Usually, if an
> external-ids key is being used to communicate between higher and lower
> level systems, it is a mistake: there should be some other interface for
> the purpose, such as a dedicated column.
>
>
Agree. And looking into the ovn-controller code which uses the external_ids,
I think we can remove it quite easily. I will work on it to remove the
reference for dns.

Thanks
Numan


> > > However, why just omit alerts?  If ovn-controller does not read these
> > > (it should not), then they may be omitted entirely.
> > >
> > Right?
> > >
> >
> > That's right. I don't understand the alert APIs properly. I will check it
> > and update the patch to omit
> > tracking the external_ids of all tables.
>
> I guess they weren't explained very well.  I sent out what I hope
> improves it:
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-June/360140.html
>


More information about the dev mailing list