On Tue, Sep 13, 2016 at 02:45:54PM -0700, Jarno Rajahalme wrote:
>
> > On Sep 13, 2016, at 10:56 AM, Ben Pfaff <b...@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 <ja...@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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
