[ovs-dev] [PATCH v3 01/13] ofproto: Don't use connmgr after destruction.

Ben Pfaff blp at ovn.org
Wed Sep 14 00:13:12 UTC 2016


On Tue, Sep 13, 2016 at 02:45:54PM -0700, Jarno Rajahalme wrote:
> 
> > On Sep 13, 2016, at 10:56 AM, Ben Pfaff <blp at ovn.org> wrote:
> > 
> > On Mon, Sep 12, 2016 at 01:52:31PM -0700, Jarno Rajahalme wrote:
> >> Set ofproto's connmgr pointer to NULL after the connmgr has been
> >> destructed, and check for NULL when sending a flow removed
> >> notification.
> >> 
> >> Verified by sending the flow removed message unconditionally and
> >> observing numerous core dumps in the test suite.
> >> 
> >> Found by inspection.
> >> 
> >> Fixes: f695ebfae5 ("ofproto: Postpone sending flow removed messages.")
> >> Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
> >> ---
> >> v3: New patch for v3.
> > 
> > I'm worried a bit that the rules needed to access these structures
> > correctly are getting complicated.  Maybe there should be a high-level
> > overview somewhere.
> > 
> 
> The main complication here is that if we tagged connmgr * with OVS_GUARDED_BY(ofproto_mutex), we would need to do widen the scope of ofproto_mutex quite a bit. Maybe introducing a rwlock for connmgr separately would do it, albeit with the assumption that the main thread remains the only writer, all that extra read locking from the main thread would be new overhead. It would be more maintainable, though.

Yes, I recognize that and I don't want to make the scope bigger if we
can avoid it.

Another option might be to just stick the flow_removed messages on a
list from the callback and then flush them later in the ofproto_run()
loop.  They're already asynchronous and unpredictable since RCU can
delay sending them an arbitrary amount of time.

> I made a note to look into this later.
> 
> Do you think we should apply this to branch-2.6 as well as the master?

It fixes a crash bug right?  So, yes, unless you know a simpler fix.



More information about the dev mailing list