[ovs-dev] [PATCHv4 7/7] ofproto-dpif: Remove the flow_dumper thread.

Joe Stringer joestringer at nicira.com
Fri Feb 28 00:55:49 UTC 2014


On 27 February 2014 15:23, Ben Pfaff <blp at nicira.com> wrote:
>
> I would appreciate xpthread_*() wrappers for the barrier functions.  I
> know that they shouldn't normally fail, but error checking has found
> real bugs for other types of pthread objects and it could find them
> for barriers too.
>

Sure, I can look at this.


> Datapath flow dumps can sometimes repeat or omit flows, but
> revalidate() appears to create a new ukey if it doesn't find one
> without making sure that a duplicate wasn't added in the meantime.
>
> That said, I guess that the common case is that a given ukey is being
> accessed by only a single thread at a time.  Is that true?  If so, did
> you consider locking it once in revalidate() as soon as we find it,
> and then releasing it when we go on to the next flow?  (I guess that
> we only need to deal with a single ukey once per dump, so we could
> possibly even use trylock at the beginning and skip on to the next
> flow if some other thread is already working on that one?)
>

I would agree that this race is unusual, but quite possible.

The simplest case to consider here is if two threads attempt to perform the
same ukey lookup at the same time:
* Thread A locks the revalidator, searches for (and fails to find) the
ukey. Unlocks and continues revalidate().
* Thread B does the same
* Thread A begins creating the ukey, locks revalidator, inserts ukey,
unlocks
* Thread B will then attempt to do the same.

I've got two possible fixes in mind:
1) Lock revalidator->mutex before the lookup. Only release after insertion
(or the equivalent codepath if we are not creating a ukey). Thread B is
unable to process the duplicate flow until Thread A has set up the ukey and
inserted it. However, Thread B has no idea that it is handling a duplicate.
2) Perform an additional lookup immediately before the hmap insertion
(while holding revalidator->mutex) to make sure we don't double-insert. If
it exists, then we're in this race condition and can skip processing the
current flow.

I'm leaning towards #2, although it is still possible to have the same flow
processed twice (for example, if the ukey exists, or if the ukey exists but
the flow is ready to expire). Thoughts?

Here are a few minor style bits I suggest folding in.



 (...)


Thanks, I'll roll those in.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140227/ab8f8c05/attachment-0005.html>


More information about the dev mailing list