Hi Ben, I have not tried it under OVS sandbox. I can test the patch on both OVS sandbox and my local testbed and update with the results.
Thanks & Regards, Anil Kumar -----Original Message----- From: Ben Pfaff <b...@ovn.org> Sent: Friday, 1 November, 2019 11:12 PM To: Ilya Maximets <ilya.maxim...@gmail.com> Cc: Ilya Maximets <i.maxim...@ovn.org>; Anil Kumar Koli <anilkuma...@altencalsoftlabs.com>; ovs-dev <ovs-dev@openvswitch.org> Subject: Re: [ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit Openflow rules with certain combinations of nested actions 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.
_______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
