On Fri, Aug 12, 2016 at 12:57 PM, Ben Pfaff <b...@ovn.org> wrote:
> On Thu, Aug 11, 2016 at 05:20:33PM -0700, Jesse Gross wrote:
>> Originally, processing of encapsulations simply iterated over all tables on
>> every wakeup and would replace anything that changed. This is somewhat
>> inefficient but it captured all changes.
>>
>> Incremental processing avoided the need to do so much work but it could
>> miss several types of changes. In particular, it only monitored the chassis
>> table in the southbound database, so other changes (particularly in the
>> encap table) were not reflected. In addition, while it corrected some
>> changes to its data in OVS, others could go unnoticed.
>>
>> This attempts to fix those issues by reflecting the most recent updates
>> to the southbound database in OVS at all times. It also increases safety
>> by avoiding the possibility of dangling pointers to old database rows and
>> eliminates the need to traverse the OVS database at all during most wakeups.
>>
>> Fixes: 1d45d5a9 ("ovn-controller: Change encaps_run to work incrementally.")
>> Signed-off-by: Jesse Gross <je...@kernel.org>
>
> Thanks for working on this.
>
> This could use a few more high-level code comments. It's a little hard
> to follow encaps_run(). I had to study it. I'm still not entirely sure
> I understand all of it.
Thanks for the starter comments - I took your incremental and then
added several more comments to give more of an overview.
> Usually, your code is super-robust, so some of this is on faith:
> Acked-by: Ben Pfaff <b...@ovn.org>
I think the code is reasonably robust at this point and certainly it
fixes a number of existing bugs. However, the truth is that I wasn't
really all that happy about it. It took a fairly long time to work
through the various corner cases and all of this was to support what
is really quite simple "business logic" in the mapping between the
Southbound and OVS databases. I tried to separate out the
synchronization and logic as much as possible so future changes should
be easier but this definitely doesn't feel like the best long term
solution to me. I suspect that there might be similar issues in other
places where incremental processing tries to avoid doing a full walk
of database tables.
I can definitely see the benefits of nlog or another rule engine to
managing the synchronization. I don't yet have a good concept of how
the generic engine will map onto specific pieces of translation or how
easy it will be to adapt to different components (i.e. ovn-northd vs.
ovn-controller and all of their individual pieces). However, I'm
certainly more enthusiastic about the concept than I have been in the
past.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
