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.
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
