[ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit Openflow rules with certain combinations of nested actions
Ben Pfaff
blp at ovn.org
Thu Oct 31 17:24:01 UTC 2019
For Ilya and others: the email to Anil bounced, so he probably isn't at
Calsoft Labs any longer.
On Thu, Oct 31, 2019 at 10:12:04AM -0700, 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 :-(
More information about the dev
mailing list