[ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action

Anju Thomas anju.thomas at ericsson.com
Wed Feb 27 09:40:03 UTC 2019


Sure Ben. I will send a patch for 2.9 branch.

Regards
Anju

-----Original Message-----
From: Ben Pfaff [mailto:blp at ovn.org] 
Sent: Tuesday, February 26, 2019 8:55 PM
To: Ilya Maximets <i.maximets at samsung.com>
Cc: ovs-dev at openvswitch.org; Anju Thomas <anju.thomas at ericsson.com>; Stokes, Ian <ian.stokes at intel.com>
Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action

Thanks for the report.

For now, I reverted this from branch-2.9.  I'd be pleased to have a fixed patch for branch-2.9, though.

On Tue, Feb 26, 2019 at 03:22:08PM +0300, Ilya Maximets wrote:
> Hi Ben, Anju.
> 
> This patch breaks tunnel_push_pop unit tests on branch-2.9:
> 
> tunnel_push_pop
> 
> 791: tunnel_push_pop - action                        FAILED (tunnel-push-pop.at:115)
> 792: tunnel_push_pop - packet_out                    ok
> 793: tunnel_push_pop - underlay bridge match         FAILED (tunnel-push-pop.at:327)
> 
> 
> ./tunnel-push-pop.at:114: ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'
> stdout:
> Flow: 
> ip,in_port=LOCAL,vlan_tci=0x0000,dl_src=f8:bc:12:44:34:b6,dl_dst=aa:55
> :aa:55:00:00,nw_src=1.1.3.88,nw_dst=1.1.3.112,nw_proto=47,nw_tos=0,nw_
> ecn=0,nw_ttl=64
> 
> bridge("int-br")
> ----------------
>  0. priority 32768
>     output:2
>      -> output to native tunnel
>      -> tunneling to 1.1.2.92 via br0
>      -> tunneling from aa:55:aa:55:00:00 1.1.2.88 to f8:bc:12:44:34:b6 
> 1.1.2.92
> 
> bridge("br0")
> -------------
>  0. priority 32768
>     NORMAL
>      -> learned port is input port, dropping
> 
> Final flow: unchanged
> Megaflow: recirc_id=0,eth,ip,in_port=LOCAL,nw_ecn=0,nw_frag=no
> Datapath actions: drop
> ./tunnel-push-pop.at:115: tail -1 stdout
> --- -   2019-02-26 15:14:47.663428963 +0300
> +++ /tests/testsuite.dir/at-groups/791/stdout        2019-02-26 15:14:47.659754269 +0300
> @@ -1,2 +1,2 @@
> -Datapath actions: 
> tnl_push(tnl_port(4789),header(size=50,type=4,eth(dst=f8:bc:12:44:34:b
> 6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92
> ,proto=17,tos=0,ttl=64,frag=0x4000),udp(src=0,dst=4789,csum=0x0),vxlan
> (flags=0x8000000,vni=0x7b)),out_port(100))
> +Datapath actions: drop
> 
> 
> Could you please take a look?
> 
> Best regards, Ilya Maximets.
> 
> > OK.  I applied the first part to all relevant branches.
> > 
> > On Mon, Jan 21, 2019 at 04:06:26AM +0000, Anju Thomas wrote:
> >> Hi Ben/Ian,
> >> I understand that the second part of the patch needs more review. 
> >> But do you think we can atleast merge the first part of this viz https://mail.openvswitch.org/pipermail/ovs-dev/2018-May/346649.html  so that to crash and more importantly silent leaks . We can widely review the other part of the code as it might require more indepth testing and review.
> >> 
> >> What are your suggestions ?
> >> 
> >> Regards
> >> Anju
> >> 
> >> -----Original Message-----
> >> From: Ben Pfaff [mailto:blp at ovn.org]
> >> Sent: Thursday, January 17, 2019 11:39 PM
> >> To: Stokes, Ian <ian.stokes at intel.com>
> >> Cc: Lam, Tiago <tiago.lam at intel.com>; Anju Thomas <anju.thomas 
> >> at ericsson.com>; dev at openvswitch.org
> >> Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl 
> >> push action
> >> 
> >> On Wed, Jan 16, 2019 at 11:38:38AM +0000, Stokes, Ian wrote:
> >> > > On 16/01/2019 09:30, Anju Thomas wrote:
> >> > > >
> >> > > > Hi Folks,
> >> > > >
> >> > > > Are these changes planned to be merged as well?
> >> > > >
> >> > > > Regards
> >> > > > Anju
> >> > > 
> >> > > Hi Anju,
> >> > > 
> >> > > Unfortunately, no. An RFC based on the below was proposed to 
> >> > > the mailing list here [1], but no discussion / comments 
> >> > > happened after that. Further discussion and testing would be needed to move this forward...
> >> > > 
> >> > 
> >> > I wasn't aware of this tbh. It seems like a bug fix so I believe it would be eligible to be merged for the 2.11 release a(and possibly backported also).
> >> > 
> >> > As it manipulates the dp packets functionality it would need wider testing and discussion among the community.
> >> > 
> >> > It won't make it upstream this week before we cut for 2.11 I would think but there is 4 weeks allowed until release where bug fixes are allowed. If further work continues it could be merged within that window or afterwards and backported?
> >> 
> >> Bug fixes are welcomed at any time, pre- or post-release.
> 


More information about the dev mailing list