On Wed, Aug 31, 2016 at 12:59:05PM -0500, Ryan Moats wrote:
> 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
Ah, OK, didn't understand that, I'll try to help.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
