Ben Pfaff <b...@ovn.org> wrote on 08/31/2016 12:46:17 PM:
> From: Ben Pfaff <b...@ovn.org>
> To: Jesse Gross <je...@kernel.org>
> Cc: Ryan Moats/Omaha/IBM@IBMUS, ovs dev <dev@openvswitch.org>
> Date: 08/31/2016 12:46 PM
> Subject: Re: [ovs-dev] [PATCH] ovn-controller: Convert encaps module
> back to full processing
>
> On Tue, Aug 30, 2016 at 09:04:10AM -0700, Jesse Gross wrote:
> > On Sun, Aug 28, 2016 at 3:51 PM, Ryan Moats <rmo...@us.ibm.com> wrote:
> > > diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
> > > index d99ba05..844447f 100644
> > > --- a/ovn/controller/encaps.c
> > > +++ b/ovn/controller/encaps.c
> > > + SBREC_CHASSIS_FOR_EACH (chassis_rec, ctx->ovnsb_idl) {
> > > + for (int i = 0; i < chassis_rec->n_encaps; i++) {
> > > + encap_rec = chassis_rec->encaps[i];
> > > +
> > > + struct encap_hash_node *encap_hash_node;
> > > + encap_hash_node =
> lookup_encap_uuid(&encap_rec->header_.uuid);
> > > + if (encap_hash_node) {
> > > + /* A change might have invalidated our mapping.
> Process the
> > > + * new version and then iterate over everything
> to see if it
> > > + * is OK. */
> > > + delete_encap_uuid(encap_hash_node);
> > > + poll_immediate_wake();
> > > }
> >
> > Doesn't this result in essentially infinite wakeups? It used to be
> > that this loop would run only when a chassis was add/removed/changed
> > and so the if (encap_hash_node) condition would only trigger when an
> > existing chassis is modified. However, after this change every wakeup
> > will loop through all chassises and any existing ones will trigger
> > another wakeup and loop, etc.
> >
> > In general, I don't think it makes sense to remove incremental
> > processing without removing persistent state. The result ends up being
> > not very logical and actually more complicated.
>
> I want to second Jesse's feedback here. I think that this should really
> be stateless, not half-incremental.
Yes that poll_immediate_wake does result in infinite wakeups and yes
it has been removed from v2 of the patch set.
I explained the situation in [1] and that "cry for help" still holds...
[1] http://openvswitch.org/pipermail/dev/2016-August/078692.html
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
