[ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit Openflow rules with certain combinations of nested actions

[Ben Pfaff]

On Fri, Nov 01, 2019 at 02:35:55PM +0100, Ilya Maximets wrote:
> CC: ovs-dev
> Sorry.
> 
> On 01.11.2019 14:34, Ilya Maximets wrote:
> > On 31.10.2019 18:12, Ben Pfaff wrote:
> > > On Thu, Sep 05, 2019 at 11:12:26PM +0530, Anil Kumar Koli wrote:
> > > > > * We can't get rid of ofproto_mutex in do_bundle_commit(), or drop it
> > > > >    temporarily around flow translation (i.e. the call to
> > > > >    ofproto_packet_out_start()), because it might need to revert all the
> > > > >    flow table changes and dropping the mutex would allow other threads to
> > > > >    race in and make conflicting changes
> > > > 
> > > > But it seems that the issue happens on ofproto_packet_out_finish() and not on
> > > > ofproto_packet_out_start(). So, the flow translation should be OK under the
> > > > ofproto_mutex and revert is still possible under the mutex.
> > > > The only thing we need to take out of the mutex is real action execution by
> > > > the datapath triggered by ofproto_packet_out_finish(). Callers never check the
> > > > status of this function so it should be not so hard.
> > > > 
> > > > So, possible solution:
> > > > * Move ofproto_packet_out_finish() out of ofproto_mutex in all the callers:
> > > >    * It's easy for handle_packet_out()
> > > >    * For do_bundle_commit() we could temporary store all the ofproto_packet_out
> > > >      entities and finish them later.
> > > > 
> > > > Am I missing something?  Is there reason for ofproto_packet_out_finish() to
> > > > require ofproto_mutex?
> > > > 
> > > > Ben, Anil, what do you think?
> > > > 
> > > > Best regards, Ilya Maximets.
> > > 
> > > This does seem like a reasonable solution.  Do you want to take a shot
> > > at implementing it?  I promise to review a patch much more quickly than
> > > I responded to the thread :-(
> > > 
> > 
> > I could try. The hard part here is that I can not reproduce the issue.

I can't either.

Anil, do you get the crash when you run the five commands cited in your
commit message under the OVS sandbox (that you can obtain running "make
sandbox")?  It would be a lot easier for Ilya and I to work on and talk
about this issue if we could reproduce the problem.