[ovs-dev] [PATCH] ovn-controller: Drop remove_local_datapath_by_binding().

Russell Bryant russell at ovn.org
Mon Jul 18 21:03:09 UTC 2016


On Mon, Jul 18, 2016 at 4:56 PM, Ryan Moats <rmoats at us.ibm.com> wrote:

> Russell Bryant <russell at ovn.org> wrote on 07/18/2016 03:47:34 PM:
>
> > From: Russell Bryant <russell at ovn.org>
> > To: dev at openvswitch.org
> > Cc: Ryan Moats/Omaha/IBM at IBMUS, Russell Bryant <russell at ovn.org>
> > Date: 07/18/2016 03:47 PM
> > Subject: [PATCH] ovn-controller: Drop remove_local_datapath_by_binding().
> >
> > ovn-controller has an hmap called 'local_datapaths' which tracks
> > all OVN datapaths that have at least one port binding on the local
> > chassis.  This patch corrects the case where a port binding row is
> > deleted from the southbound DB while it's still bound to the chassis,
> > meaning it was deleted before the ovs interface was deleted.
> >
> > The previous code tried to handle this case by calling
> > remove_local_datapath_by_binding().  The function appears to try
> > to look up local_datapath by the binding UUID.  If it finds it,
> > it will delete the local datapath entry.  On the surface, this
> > looks like a bug where it deletes a local datapath entry even
> > when there could be other ports still bound to the chassis.
> > The reality is that this function was always a no-op.  It was
> > doing a lookup using a different hash value than how local_datapath
> > entries are actually hashed. In practice, this wasn't a big problem
> > because local_datapaths are correctly cleaned in in the
> > process_full_binding case after an ovs interface is added or removed.
> >
> > The new change ensures that we run the process_full_binding code
> > in this case right away, even if the interface is not deleted.
> >
> > Fixes: 263064aeaa31 ("Convert binding_run to incremental processing.")
> > Signed-off-by: Russell Bryant <russell at ovn.org>
> > ---
>
> I think it *might* have been easier to just change to using the by
> uuid hashtable, but it's all good :)
>

Not quite, as that would have introduced the bug I alluded to in the second
paragraph.  It would have deleted local_datapath entries too soon.

> Acked-by: Ryan Moats <rmoats at us.ibm.com>
>
Thanks, Ryan!   I applied this to master.  I'm not expecting to post any
more changes in this area right now.

> P.S. Ben, I'll be sending a rebased pair of incremental processing
> patches later this evening, after doubles league.
>

-- 
Russell Bryant



More information about the dev mailing list