On Thu, May 19, 2016 at 11:52:05AM -0500, Ryan Moats wrote: > Ben Pfaff <[email protected]> wrote on 05/19/2016 10:32:53 AM: > > > From: Ben Pfaff <[email protected]> > > To: Ryan Moats/Omaha/IBM@IBMUS > > Cc: [email protected] > > Date: 05/19/2016 10:33 AM > > Subject: Re: [ovs-dev,v16,1/5] Change encaps_run to work incrementally > > > > On Thu, May 19, 2016 at 10:02:08AM -0500, Ryan Moats wrote: > > > Ben Pfaff <[email protected]> wrote on 05/17/2016 10:13:19 PM: > > > > > > > From: Ben Pfaff <[email protected]> > > > > To: Ryan Moats/Omaha/IBM@IBMUS > > > > Cc: [email protected] > > > > Date: 05/17/2016 10:14 PM > > > > Subject: Re: [ovs-dev,v16,1/5] Change encaps_run to work > incrementally > > > > > > > > On Mon, May 02, 2016 at 10:26:54AM -0500, Ryan Moats wrote: > > > > > As a side effect, tunnel context is persisted. > > > > > > > > > > Signed-off-by: Ryan Moats <[email protected]> > > > > > > > > Thanks for updating this. > > > > > > and thanks for looking - sorry for the delayed reply (I've been > > > doing OVN training the past couple of days) > > > > > > > In a couple of places, this uses hmap_first_with_hash() to find an > > > > element in a hash table. ovn-controller uses this method in some > > > > special cases where the hash value is known to be unique; for > example, I > > > > think that it's used for a hash table where the "hash" is the > assigned > > > > logical datapath ID, which is a unique 32-bit (maybe shorter? I don't > > > > recall at the moment) number. But that trick doesn't work when the > hash > > > > value is really a hash. For example, it can't be used in this code > > > > where the hash is taken from a UUID, because there might be multiple > > > > UUIDs with the same hash value. It's necessary, instead, to iterate > > > > through the items that have the desired hash value, with > > > > HMAP_FOR_EACH_WITH_HASH, and then compare the item's full key instead > of > > > > just the hash. > > > > > > Ugh - I thought I had changed that, but when I couple this with your > > > comments below, I'm thinking I've confused myself as to what patches > > > I have and haven't pushed > > > > > > > In the process_full_encaps case, I don't see what removes tunnels > that > > > > are no longer needed. > > > > > > > > This has some TODOs and commented-out code in it, so I suspect that > it's > > > > not really ready for full review? > > > > > > As I implied above, those shouldn't be there, so now I'm suspicious if > I've > > > lost track of a ball that I've been juggling... Since I've got to > rebase > > > the rest of the patches anyway, adding this one to the list won't add > > > that much additional effort... > > > > Yeah, it did seem weird, since going into it I expected that this patch > > was ready. I'll look forward to the next version. > > > > I've found the right code for encaps.c and re-looking at the tunnel > update path, I'm really dubious about it being correct - I'm out this > afternoon (CDT) for a HS graduation, but if I post just the first patch > as an RFC how soon would it capture eyeballs? I ask because if it will > be a while, then I'll just work through the rest of the patches first.
I think it'll be a while, because I'm trying to get from code review back to coding. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
