Hi Ben/Ilya,

I was not able to reproduce the issue under OVS sandbox. 
I have tested the below patch in my local testbed and didn't observe the
crash.
https://patchwork.ozlabs.org/patch/1188193/

Please find attached the commands to reproduce the issue on a physical
host/VM.
1. Start the ovs-vswitchd in daemon mode.
2. Create a bridge of type netdev datapath
    # sudo ovs-vsctl add-br br-int -- set bridge br-int datapath_type=netdev
3. Add a vhostuser port to br-int bridge of type dpdkvhostuser
    # sudo ovs-vsctl add-port br-int mqvm1p1 -- set Interface mqvm1p1
type=dpdkvhostuser ofport_request=10
4. Add a physical port to br-int bridge of type dpdk0
   # sudo ovs-vsctl add-port br-int dpdk0 -- set Interface dpdk0 type=dpdk
options:dpdk-devargs=0000:00:07.0
5. Add the following conntrack rules
  # sudo ovs-ofctl -OOpenflow13 add-flow br-int "table=0, priority=50,
ct_state=-trk,ip, in_port=10 actions=ct(table=0)"
  # sudo ovs-ofctl -OOpenflow13 add-flow br-int "table=0, priority=50, \
   ct_state=+trk,ip, in_port=10 actions=ct(commit),resubmit(,1)"
  # sudo ovs-ofctl -OOpenflow13 add-flow br-int "table=1 \
 
actions=learn(table=2,NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[],load:NXM_OF_IN_PORT[
]->NXM_NX_REG0[0..15], \
   output:NXM_NX_REG0[0..15]),resubmit(,2)"
  #sudo ovs-ofctl -OOpenflow13 packet-out br-int 'in_port=10
packet=505400000007101111111111080045000028000000004006f97cc0a80001c0a800020
008000a0000000000000000500200002e7d0000, actions=TABLE'


Thanks & Regards,
Anil Kumar.

-----Original Message-----
From: Anil Kumar Koli <[email protected]> 
Sent: Monday, 4 November, 2019 09:46 AM
To: 'Ben Pfaff' <[email protected]>; 'Ilya Maximets' <[email protected]>
Cc: 'Ilya Maximets' <[email protected]>; 'ovs-dev'
<[email protected]>
Subject: RE: [ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit
Openflow rules with certain combinations of nested actions

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 <[email protected]> 
Sent: Friday, 1 November, 2019 11:12 PM
To: Ilya Maximets <[email protected]>
Cc: Ilya Maximets <[email protected]>; Anil Kumar Koli
<[email protected]>; ovs-dev <[email protected]>
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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to