[ovs-dev] [race-fix 5/6] ofproto: Make OFPFC_ADD internally modify a rule instead of swapping.

Ben Pfaff blp at nicira.com
Tue Aug 27 19:52:43 UTC 2013


On Tue, Aug 27, 2013 at 11:29:26AM -0700, Ethan Jackson wrote:
> In add_flow, should we take the evict lock on the rule while we're
> still holding the classifier readlock?  It doesn't matter much if
> add_flow will only be called by the main thread, however if that isn't
> the case, we don't want the rule we're working on to be evicted from
> under us.

I'm not ready yet (and the code isn't ready yet) to call add_flow()
with any concurrency.  I wasn't really thinking about the possibility
yet while writing this (although it's the next thing to do) because I
wanted to simplify the code first.  I bet there are plenty of other
problems too.  So I would rather leave it as-is in this patch, if you
that is OK with you.

> There's a serious problem with this code which I think pre-dated this
> patch.  Specifically, modify_flows__() can change a rule's actions
> without holding a lock.  If some child thread is using those actions
> to do xlation, I think we'll run into trouble.

Oops.

> I'll put some thought into this.

Thanks!



More information about the dev mailing list