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

During slow path packet processing, if the action is to output to a
tunnel port, the slow path processing of the encapsulated packet
continues on the underlay bridge and additional actions (e.g. optional
VLAN encapsulation, bond link selection and finally output to port) are
collected there.

To prepare for a continuation of the processing of the original packet
(e.g. output to other tunnel ports in a flooding scenario), the
“tunnel_push” action and the actions of the underlay bridge are
encapsulated in a clone() action to preserve the original packet.

If the underlay bridge decides to drop the tunnel packet (for example if
both bonded ports are down simultaneously), the clone(tunnel_push))
actions previously generated as part of translation of the output to
tunnel port are discarded and a stand-alone tunnel_push action is added
instead. Thus the tunnel header is pushed on to the original packet.
This is the bug.

Consequences: If packet processing continues with sending to further
tunnel ports, multiple tunnel header pushes will happen on the original
packet as typically the tunnels all traverse the same underlay bond
which is down. The packet may not have enough headroom to accommodate
all the tunnel headers. OVS crashes if it runs out of space while trying
to push the tunnel headers.

Even in case there is enough headroom, the packet will not be freed
since the accumulated action list contains only the tunnel header push
action without any output port action. Thus, we either have a crash or a
packet buffer leak.

Signed-off-by: Anju Thomas <anju.thomas@ericsson.com>
---
 ofproto/ofproto-dpif-xlate.c  |  7 -------
 tests/tunnel-push-pop-ipv6.at | 10 +++++-----
 tests/tunnel-push-pop.at      | 18 +++++++++---------
 3 files changed, 14 insertions(+), 21 deletions(-)

Bleep bloop.  Greetings Anju Thomas, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
Failed to merge in the changes.
Patch failed at 0001 Fix crash due to multiple tnl push action
The copy of the patch that failed is found in:
   /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot

Hi.
Thanks for the patch.

Few comments:

1. You may use --subject-prefix="PATCH branch-2.9". In this case ovsrobot
   will be able to apply patch to the right branch for checking.

2. This patch fixes tests (I didn't check), but it effectively makes them
   useless by replacing all the useful data by simple 'drop' action.
   It's the same as just removing the broken test.

Best regards, Ilya Maximets.

On 27.02.2019 14:41, Anju Thomas wrote:
> During slow path packet processing, if the action is to output to a
> tunnel port, the slow path processing of the encapsulated packet
> continues on the underlay bridge and additional actions (e.g. optional
> VLAN encapsulation, bond link selection and finally output to port) are
> collected there.
> 
> To prepare for a continuation of the processing of the original packet
> (e.g. output to other tunnel ports in a flooding scenario), the
> “tunnel_push” action and the actions of the underlay bridge are
> encapsulated in a clone() action to preserve the original packet.
> 
> If the underlay bridge decides to drop the tunnel packet (for example if
> both bonded ports are down simultaneously), the clone(tunnel_push))
> actions previously generated as part of translation of the output to
> tunnel port are discarded and a stand-alone tunnel_push action is added
> instead. Thus the tunnel header is pushed on to the original packet.
> This is the bug.
> 
> Consequences: If packet processing continues with sending to further
> tunnel ports, multiple tunnel header pushes will happen on the original
> packet as typically the tunnels all traverse the same underlay bond
> which is down. The packet may not have enough headroom to accommodate
> all the tunnel headers. OVS crashes if it runs out of space while trying
> to push the tunnel headers.
> 
> Even in case there is enough headroom, the packet will not be freed
> since the accumulated action list contains only the tunnel header push
> action without any output port action. Thus, we either have a crash or a
> packet buffer leak.
> 
> Signed-off-by: Anju Thomas <anju.thomas@ericsson.com>
> ---
>  ofproto/ofproto-dpif-xlate.c  |  7 -------
>  tests/tunnel-push-pop-ipv6.at | 10 +++++-----
>  tests/tunnel-push-pop.at      | 18 +++++++++---------
>  3 files changed, 14 insertions(+), 21 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 7777ed8..7e69469 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3510,13 +3510,6 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct xport *xport,
>              nl_msg_end_non_empty_nested(ctx->odp_actions, clone_ofs);
>          } else {
>              nl_msg_cancel_nested(ctx->odp_actions, clone_ofs);
> -            /* XXX : There is no real use-case for a tunnel push without
> -             * any post actions. However keeping it now
> -             * as is to make the 'make check' happy. Should remove when all the
> -             * make check tunnel test case does something meaningful on a
> -             * tunnel encap packets.
> -             */
> -            odp_put_tnl_push_action(ctx->odp_actions, &tnl_push_data);
>          }
>  
>          /* Restore context status. */
> diff --git a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at
> index 7ca522a..c9e4a2b 100644
> --- a/tests/tunnel-push-pop-ipv6.at
> +++ b/tests/tunnel-push-pop-ipv6.at
> @@ -93,28 +93,28 @@ dnl Check VXLAN tunnel push
>  AT_CHECK([ovs-ofctl add-flow int-br action=2])
>  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'], [0], [stdout])
>  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,csum=0xffff),vxlan(flags=0x8000000,vni=0x7b)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check VXLAN tunnel push set tunnel id by flow and checksum
>  AT_CHECK([ovs-ofctl add-flow int-br "actions=set_tunnel:124,4"])
>  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'], [0], [stdout])
>  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b7,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::93,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,csum=0xffff),vxlan(flags=0x8000000,vni=0x7c)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check GRE tunnel push
>  AT_CHECK([ovs-ofctl add-flow int-br action=3])
>  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'], [0], [stdout])
>  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: tnl_push(tnl_port(3),header(size=62,type=3,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::92,label=0,proto=47,tclass=0x0,hlimit=64),gre((flags=0x2000,proto=0x6558),key=0x1c8)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check Geneve tunnel push
>  AT_CHECK([ovs-ofctl add-flow int-br "actions=set_field:2001:cafe::92->tun_ipv6_dst,5"])
>  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'], [0], [stdout])
>  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: tnl_push(tnl_port(6081),header(size=70,type=5,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=6081,csum=0xffff),geneve(vni=0x7b)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check Geneve tunnel push with options
> @@ -122,7 +122,7 @@ AT_CHECK([ovs-ofctl add-tlv-map int-br "{class=0xffff,type=0x80,len=4}->tun_meta
>  AT_CHECK([ovs-ofctl add-flow int-br "actions=set_field:2001:cafe::92->tun_ipv6_dst,set_field:0xa->tun_metadata0,5"])
>  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'], [0], [stdout])
>  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: tnl_push(tnl_port(6081),header(size=78,type=5,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=6081,csum=0xffff),geneve(crit,vni=0x7b,options({class=0xffff,type=0x80,len=4,0xa}))),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check decapsulation of GRE packet
> diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
> index cc8b1b5..07832fc 100644
> --- a/tests/tunnel-push-pop.at
> +++ b/tests/tunnel-push-pop.at
> @@ -113,49 +113,49 @@ dnl Check VXLAN tunnel push
>  AT_CHECK([ovs-ofctl add-flow int-br action=2])
>  AT_CHECK([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)'], [0], [stdout])
>  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: tnl_push(tnl_port(4789),header(size=50,type=4,eth(dst=f8:bc:12:44:34:b6,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
>  ])
>  
>  dnl Check VXLAN GPE tunnel push
>  AT_CHECK([ovs-ofctl add-flow int-br action=8])
>  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'], [0], [stdout])
>  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: tnl_push(tnl_port(4789),header(size=50,type=4,eth(dst=f8:bc:12:44:34:b6,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=0xc000003,vni=0x159)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check VXLAN tunnel push set tunnel id by flow and checksum
>  AT_CHECK([ovs-ofctl add-flow int-br "actions=set_tunnel:124,4"])
>  AT_CHECK([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)'], [0], [stdout])
>  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: tnl_push(tnl_port(4789),header(size=50,type=4,eth(dst=f8:bc:12:44:34:b7,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.93,proto=17,tos=0,ttl=64,frag=0x4000),udp(src=0,dst=4789,csum=0xffff),vxlan(flags=0x8000000,vni=0x7c)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check GRE tunnel push
>  AT_CHECK([ovs-ofctl add-flow int-br action=3])
>  AT_CHECK([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)'], [0], [stdout])
>  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x6558),key=0x1c8)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check L3GRE tunnel push
>  AT_CHECK([ovs-ofctl add-flow int-br action=7])
>  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'], [0], [stdout])
>  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: pop_eth,tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x800),key=0x1c8)),out_port(100))
> +  [Datapath actions: pop_eth
>  ])
>  
>  dnl Check Geneve tunnel push
>  AT_CHECK([ovs-ofctl add-flow int-br "actions=set_field:1.1.2.92->tun_dst,5"])
>  AT_CHECK([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)'], [0], [stdout])
>  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: tnl_push(tnl_port(6081),header(size=50,type=5,eth(dst=f8:bc:12:44:34:b6,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=6081,csum=0x0),geneve(vni=0x7b)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check Geneve tunnel push with pkt-mark
>  AT_CHECK([ovs-ofctl add-flow int-br "actions=set_tunnel:234,6"])
>  AT_CHECK([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)'], [0], [stdout])
>  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: set(skb_mark(0x4d2)),tnl_push(tnl_port(6081),header(size=50,type=5,eth(dst=f8:bc:12:44:34:b7,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.93,proto=17,tos=0,ttl=64,frag=0x4000),udp(src=0,dst=6081,csum=0x0),geneve(vni=0xea)),out_port(100))
> +  [Datapath actions: set(skb_mark(0x4d2))
>  ])
>  
>  dnl Check Geneve tunnel push with options
> @@ -163,7 +163,7 @@ AT_CHECK([ovs-ofctl add-tlv-map int-br "{class=0xffff,type=0x80,len=4}->tun_meta
>  AT_CHECK([ovs-ofctl add-flow int-br "actions=set_field:1.1.2.92->tun_dst,set_field:0xa->tun_metadata0,5"])
>  AT_CHECK([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)'], [0], [stdout])
>  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: tnl_push(tnl_port(6081),header(size=58,type=5,eth(dst=f8:bc:12:44:34:b6,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=6081,csum=0x0),geneve(crit,vni=0x7b,options({class=0xffff,type=0x80,len=4,0xa}))),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check decapsulation of GRE packet
> @@ -325,7 +325,7 @@ AT_CHECK([ovs-ofctl add-flow int-br action=3])
>  
>  AT_CHECK([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=17,tos=0,ttl=64,frag=no),udp(src=51283,dst=4789)'], [0], [stdout])
>  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x6558),key=0x1c8)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Verify outer L2 and L3 header flow fields can be matched in the underlay bridge
>

So then should we refrain from adding this fix to 2.9 if it does not make sense or should I remove the cases altogether?

Regards
Anju

-----Original Message-----
From: Ilya Maximets [mailto:i.maximets@samsung.com] 
Sent: Wednesday, February 27, 2019 6:55 PM
To: Anju Thomas <anju.thomas@ericsson.com>; dev@openvswitch.org
Cc: Ben Pfaff <blp@ovn.org>
Subject: Re: [ovs-dev] Fix crash due to multiple tnl push action

Hi.
Thanks for the patch.

Few comments:

1. You may use --subject-prefix="PATCH branch-2.9". In this case ovsrobot
   will be able to apply patch to the right branch for checking.

2. This patch fixes tests (I didn't check), but it effectively makes them
   useless by replacing all the useful data by simple 'drop' action.
   It's the same as just removing the broken test.

Best regards, Ilya Maximets.

On 27.02.2019 14:41, Anju Thomas wrote:
> During slow path packet processing, if the action is to output to a 
> tunnel port, the slow path processing of the encapsulated packet 
> continues on the underlay bridge and additional actions (e.g. optional 
> VLAN encapsulation, bond link selection and finally output to port) 
> are collected there.
> 
> To prepare for a continuation of the processing of the original packet 
> (e.g. output to other tunnel ports in a flooding scenario), the 
> “tunnel_push” action and the actions of the underlay bridge are 
> encapsulated in a clone() action to preserve the original packet.
> 
> If the underlay bridge decides to drop the tunnel packet (for example 
> if both bonded ports are down simultaneously), the clone(tunnel_push)) 
> actions previously generated as part of translation of the output to 
> tunnel port are discarded and a stand-alone tunnel_push action is 
> added instead. Thus the tunnel header is pushed on to the original packet.
> This is the bug.
> 
> Consequences: If packet processing continues with sending to further 
> tunnel ports, multiple tunnel header pushes will happen on the 
> original packet as typically the tunnels all traverse the same 
> underlay bond which is down. The packet may not have enough headroom 
> to accommodate all the tunnel headers. OVS crashes if it runs out of 
> space while trying to push the tunnel headers.
> 
> Even in case there is enough headroom, the packet will not be freed 
> since the accumulated action list contains only the tunnel header push 
> action without any output port action. Thus, we either have a crash or 
> a packet buffer leak.
> 
> Signed-off-by: Anju Thomas <anju.thomas@ericsson.com>
> ---
>  ofproto/ofproto-dpif-xlate.c  |  7 -------  
> tests/tunnel-push-pop-ipv6.at | 10 +++++-----
>  tests/tunnel-push-pop.at      | 18 +++++++++---------
>  3 files changed, 14 insertions(+), 21 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-xlate.c 
> b/ofproto/ofproto-dpif-xlate.c index 7777ed8..7e69469 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3510,13 +3510,6 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct xport *xport,
>              nl_msg_end_non_empty_nested(ctx->odp_actions, clone_ofs);
>          } else {
>              nl_msg_cancel_nested(ctx->odp_actions, clone_ofs);
> -            /* XXX : There is no real use-case for a tunnel push without
> -             * any post actions. However keeping it now
> -             * as is to make the 'make check' happy. Should remove when all the
> -             * make check tunnel test case does something meaningful on a
> -             * tunnel encap packets.
> -             */
> -            odp_put_tnl_push_action(ctx->odp_actions, &tnl_push_data);
>          }
>  
>          /* Restore context status. */ diff --git 
> a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at index 
> 7ca522a..c9e4a2b 100644
> --- a/tests/tunnel-push-pop-ipv6.at
> +++ b/tests/tunnel-push-pop-ipv6.at
> @@ -93,28 +93,28 @@ dnl Check VXLAN tunnel push  AT_CHECK([ovs-ofctl 
> add-flow int-br action=2])  AT_CHECK([ovs-appctl ofproto/trace 
> ovs-dummy 
> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: 
> tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b
> 6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=200
> 1:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,c
> sum=0xffff),vxlan(flags=0x8000000,vni=0x7b)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check VXLAN tunnel push set tunnel id by flow and checksum  
> AT_CHECK([ovs-ofctl add-flow int-br "actions=set_tunnel:124,4"])  
> AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: 
> tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b
> 7,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=200
> 1:cafe::93,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,c
> sum=0xffff),vxlan(flags=0x8000000,vni=0x7c)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check GRE tunnel push
>  AT_CHECK([ovs-ofctl add-flow int-br action=3])  AT_CHECK([ovs-appctl 
> ofproto/trace ovs-dummy 
> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: 
> tnl_push(tnl_port(3),header(size=62,type=3,eth(dst=f8:bc:12:44:34:b6,s
> rc=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:c
> afe::92,label=0,proto=47,tclass=0x0,hlimit=64),gre((flags=0x2000,proto
> =0x6558),key=0x1c8)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check Geneve tunnel push
>  AT_CHECK([ovs-ofctl add-flow int-br 
> "actions=set_field:2001:cafe::92->tun_ipv6_dst,5"])
>  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: 
> tnl_push(tnl_port(6081),header(size=70,type=5,eth(dst=f8:bc:12:44:34:b
> 6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=200
> 1:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=6081,c
> sum=0xffff),geneve(vni=0x7b)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check Geneve tunnel push with options @@ -122,7 +122,7 @@ 
> AT_CHECK([ovs-ofctl add-tlv-map int-br 
> "{class=0xffff,type=0x80,len=4}->tun_meta
>  AT_CHECK([ovs-ofctl add-flow int-br 
> "actions=set_field:2001:cafe::92->tun_ipv6_dst,set_field:0xa->tun_meta
> data0,5"])  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: 
> tnl_push(tnl_port(6081),header(size=78,type=5,eth(dst=f8:bc:12:44:34:b
> 6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=200
> 1:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=6081,c
> sum=0xffff),geneve(crit,vni=0x7b,options({class=0xffff,type=0x80,len=4
> ,0xa}))),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check decapsulation of GRE packet diff --git 
> a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at index 
> cc8b1b5..07832fc 100644
> --- a/tests/tunnel-push-pop.at
> +++ b/tests/tunnel-push-pop.at
> @@ -113,49 +113,49 @@ dnl Check VXLAN tunnel push  AT_CHECK([ovs-ofctl 
> add-flow int-br action=2])  AT_CHECK([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)
> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> -  [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
>  ])
>  
>  dnl Check VXLAN GPE tunnel push
>  AT_CHECK([ovs-ofctl add-flow int-br action=8])  AT_CHECK([ovs-appctl 
> ofproto/trace ovs-dummy 
> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> -  [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=0xc000003,vni=0x159)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check VXLAN tunnel push set tunnel id by flow and checksum  
> AT_CHECK([ovs-ofctl add-flow int-br "actions=set_tunnel:124,4"])  
> AT_CHECK([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)
> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: 
> tnl_push(tnl_port(4789),header(size=50,type=4,eth(dst=f8:bc:12:44:34:b
> 7,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.93
> ,proto=17,tos=0,ttl=64,frag=0x4000),udp(src=0,dst=4789,csum=0xffff),vx
> lan(flags=0x8000000,vni=0x7c)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check GRE tunnel push
>  AT_CHECK([ovs-ofctl add-flow int-br action=3])  AT_CHECK([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)
> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: 
> tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,s
> rc=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,pr
> oto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x6558),key=0
> x1c8)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check L3GRE tunnel push
>  AT_CHECK([ovs-ofctl add-flow int-br action=7])  AT_CHECK([ovs-appctl 
> ofproto/trace ovs-dummy 
> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: 
> pop_eth,tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44
> :34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1
> .2.92,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x800
> ),key=0x1c8)),out_port(100))
> +  [Datapath actions: pop_eth
>  ])
>  
>  dnl Check Geneve tunnel push
>  AT_CHECK([ovs-ofctl add-flow int-br 
> "actions=set_field:1.1.2.92->tun_dst,5"])
>  AT_CHECK([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)
> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: 
> tnl_push(tnl_port(6081),header(size=50,type=5,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=6081,csum=0x0),genev
> e(vni=0x7b)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check Geneve tunnel push with pkt-mark  AT_CHECK([ovs-ofctl 
> add-flow int-br "actions=set_tunnel:234,6"])  AT_CHECK([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)
> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: 
> set(skb_mark(0x4d2)),tnl_push(tnl_port(6081),header(size=50,type=5,eth
> (dst=f8:bc:12:44:34:b7,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=
> 1.1.2.88,dst=1.1.2.93,proto=17,tos=0,ttl=64,frag=0x4000),udp(src=0,dst
> =6081,csum=0x0),geneve(vni=0xea)),out_port(100))
> +  [Datapath actions: set(skb_mark(0x4d2))
>  ])
>  
>  dnl Check Geneve tunnel push with options @@ -163,7 +163,7 @@ 
> AT_CHECK([ovs-ofctl add-tlv-map int-br 
> "{class=0xffff,type=0x80,len=4}->tun_meta
>  AT_CHECK([ovs-ofctl add-flow int-br 
> "actions=set_field:1.1.2.92->tun_dst,set_field:0xa->tun_metadata0,5"])
>  AT_CHECK([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)
> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> -  [Datapath actions: 
> tnl_push(tnl_port(6081),header(size=58,type=5,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=6081,csum=0x0),genev
> e(crit,vni=0x7b,options({class=0xffff,type=0x80,len=4,0xa}))),out_port
> (100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Check decapsulation of GRE packet @@ -325,7 +325,7 @@ 
> AT_CHECK([ovs-ofctl add-flow int-br action=3])
>  
>  AT_CHECK([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=17,tos=0,ttl=64,frag=no)
> ,udp(src=51283,dst=4789)'], [0], [stdout])  AT_CHECK([tail -1 stdout], 
> [0],
> -  [Datapath actions: 
> tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,s
> rc=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,pr
> oto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x6558),key=0
> x1c8)),out_port(100))
> +  [Datapath actions: drop
>  ])
>  
>  dnl Verify outer L2 and L3 header flow fields can be matched in the 
> underlay bridge
>

Looks like I found the solution.
Following patch on branch-2.10 makes tests immune to the change:

  2ce9e71bb960 ("tests: Inject ARP replies for snoop tests on different port")

It's the test change only that makes tunnel_push to be not the last action.
If we'll backport it to 2.9, we could safely un-revert the fix.
However, commit message of the 2ce9e71bb960 will not be fully correct because
we're not going to backport the feature it was made for.

Ben, what do you think?

Best regards, Ilya Maximets.

On 27.02.2019 16:28, Anju Thomas wrote:
> So then should we refrain from adding this fix to 2.9 if it does not make sense or should I remove the cases altogether?
> 
> Regards
> Anju
> 
> -----Original Message-----
> From: Ilya Maximets [mailto:i.maximets@samsung.com] 
> Sent: Wednesday, February 27, 2019 6:55 PM
> To: Anju Thomas <anju.thomas@ericsson.com>; dev@openvswitch.org
> Cc: Ben Pfaff <blp@ovn.org>
> Subject: Re: [ovs-dev] Fix crash due to multiple tnl push action
> 
> Hi.
> Thanks for the patch.
> 
> Few comments:
> 
> 1. You may use --subject-prefix="PATCH branch-2.9". In this case ovsrobot
>    will be able to apply patch to the right branch for checking.
> 
> 2. This patch fixes tests (I didn't check), but it effectively makes them
>    useless by replacing all the useful data by simple 'drop' action.
>    It's the same as just removing the broken test.
> 
> Best regards, Ilya Maximets.
> 
> On 27.02.2019 14:41, Anju Thomas wrote:
>> During slow path packet processing, if the action is to output to a 
>> tunnel port, the slow path processing of the encapsulated packet 
>> continues on the underlay bridge and additional actions (e.g. optional 
>> VLAN encapsulation, bond link selection and finally output to port) 
>> are collected there.
>>
>> To prepare for a continuation of the processing of the original packet 
>> (e.g. output to other tunnel ports in a flooding scenario), the 
>> “tunnel_push” action and the actions of the underlay bridge are 
>> encapsulated in a clone() action to preserve the original packet.
>>
>> If the underlay bridge decides to drop the tunnel packet (for example 
>> if both bonded ports are down simultaneously), the clone(tunnel_push)) 
>> actions previously generated as part of translation of the output to 
>> tunnel port are discarded and a stand-alone tunnel_push action is 
>> added instead. Thus the tunnel header is pushed on to the original packet.
>> This is the bug.
>>
>> Consequences: If packet processing continues with sending to further 
>> tunnel ports, multiple tunnel header pushes will happen on the 
>> original packet as typically the tunnels all traverse the same 
>> underlay bond which is down. The packet may not have enough headroom 
>> to accommodate all the tunnel headers. OVS crashes if it runs out of 
>> space while trying to push the tunnel headers.
>>
>> Even in case there is enough headroom, the packet will not be freed 
>> since the accumulated action list contains only the tunnel header push 
>> action without any output port action. Thus, we either have a crash or 
>> a packet buffer leak.
>>
>> Signed-off-by: Anju Thomas <anju.thomas@ericsson.com>
>> ---
>>  ofproto/ofproto-dpif-xlate.c  |  7 -------  
>> tests/tunnel-push-pop-ipv6.at | 10 +++++-----
>>  tests/tunnel-push-pop.at      | 18 +++++++++---------
>>  3 files changed, 14 insertions(+), 21 deletions(-)
>>
>> diff --git a/ofproto/ofproto-dpif-xlate.c 
>> b/ofproto/ofproto-dpif-xlate.c index 7777ed8..7e69469 100644
>> --- a/ofproto/ofproto-dpif-xlate.c
>> +++ b/ofproto/ofproto-dpif-xlate.c
>> @@ -3510,13 +3510,6 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct xport *xport,
>>              nl_msg_end_non_empty_nested(ctx->odp_actions, clone_ofs);
>>          } else {
>>              nl_msg_cancel_nested(ctx->odp_actions, clone_ofs);
>> -            /* XXX : There is no real use-case for a tunnel push without
>> -             * any post actions. However keeping it now
>> -             * as is to make the 'make check' happy. Should remove when all the
>> -             * make check tunnel test case does something meaningful on a
>> -             * tunnel encap packets.
>> -             */
>> -            odp_put_tnl_push_action(ctx->odp_actions, &tnl_push_data);
>>          }
>>  
>>          /* Restore context status. */ diff --git 
>> a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at index 
>> 7ca522a..c9e4a2b 100644
>> --- a/tests/tunnel-push-pop-ipv6.at
>> +++ b/tests/tunnel-push-pop-ipv6.at
>> @@ -93,28 +93,28 @@ dnl Check VXLAN tunnel push  AT_CHECK([ovs-ofctl 
>> add-flow int-br action=2])  AT_CHECK([ovs-appctl ofproto/trace 
>> ovs-dummy 
>> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
>> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
>> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
>> -  [Datapath actions: 
>> tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b
>> 6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=200
>> 1:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,c
>> sum=0xffff),vxlan(flags=0x8000000,vni=0x7b)),out_port(100))
>> +  [Datapath actions: drop
>>  ])
>>  
>>  dnl Check VXLAN tunnel push set tunnel id by flow and checksum  
>> AT_CHECK([ovs-ofctl add-flow int-br "actions=set_tunnel:124,4"])  
>> AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
>> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
>> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
>> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
>> -  [Datapath actions: 
>> tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b
>> 7,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=200
>> 1:cafe::93,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,c
>> sum=0xffff),vxlan(flags=0x8000000,vni=0x7c)),out_port(100))
>> +  [Datapath actions: drop
>>  ])
>>  
>>  dnl Check GRE tunnel push
>>  AT_CHECK([ovs-ofctl add-flow int-br action=3])  AT_CHECK([ovs-appctl 
>> ofproto/trace ovs-dummy 
>> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
>> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
>> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
>> -  [Datapath actions: 
>> tnl_push(tnl_port(3),header(size=62,type=3,eth(dst=f8:bc:12:44:34:b6,s
>> rc=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:c
>> afe::92,label=0,proto=47,tclass=0x0,hlimit=64),gre((flags=0x2000,proto
>> =0x6558),key=0x1c8)),out_port(100))
>> +  [Datapath actions: drop
>>  ])
>>  
>>  dnl Check Geneve tunnel push
>>  AT_CHECK([ovs-ofctl add-flow int-br 
>> "actions=set_field:2001:cafe::92->tun_ipv6_dst,5"])
>>  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
>> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
>> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
>> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
>> -  [Datapath actions: 
>> tnl_push(tnl_port(6081),header(size=70,type=5,eth(dst=f8:bc:12:44:34:b
>> 6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=200
>> 1:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=6081,c
>> sum=0xffff),geneve(vni=0x7b)),out_port(100))
>> +  [Datapath actions: drop
>>  ])
>>  
>>  dnl Check Geneve tunnel push with options @@ -122,7 +122,7 @@ 
>> AT_CHECK([ovs-ofctl add-tlv-map int-br 
>> "{class=0xffff,type=0x80,len=4}->tun_meta
>>  AT_CHECK([ovs-ofctl add-flow int-br 
>> "actions=set_field:2001:cafe::92->tun_ipv6_dst,set_field:0xa->tun_meta
>> data0,5"])  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
>> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
>> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
>> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
>> -  [Datapath actions: 
>> tnl_push(tnl_port(6081),header(size=78,type=5,eth(dst=f8:bc:12:44:34:b
>> 6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=200
>> 1:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=6081,c
>> sum=0xffff),geneve(crit,vni=0x7b,options({class=0xffff,type=0x80,len=4
>> ,0xa}))),out_port(100))
>> +  [Datapath actions: drop
>>  ])
>>  
>>  dnl Check decapsulation of GRE packet diff --git 
>> a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at index 
>> cc8b1b5..07832fc 100644
>> --- a/tests/tunnel-push-pop.at
>> +++ b/tests/tunnel-push-pop.at
>> @@ -113,49 +113,49 @@ dnl Check VXLAN tunnel push  AT_CHECK([ovs-ofctl 
>> add-flow int-br action=2])  AT_CHECK([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)
>> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
>> -  [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
>>  ])
>>  
>>  dnl Check VXLAN GPE tunnel push
>>  AT_CHECK([ovs-ofctl add-flow int-br action=8])  AT_CHECK([ovs-appctl 
>> ofproto/trace ovs-dummy 
>> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
>> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
>> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
>> -  [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=0xc000003,vni=0x159)),out_port(100))
>> +  [Datapath actions: drop
>>  ])
>>  
>>  dnl Check VXLAN tunnel push set tunnel id by flow and checksum  
>> AT_CHECK([ovs-ofctl add-flow int-br "actions=set_tunnel:124,4"])  
>> AT_CHECK([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)
>> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
>> -  [Datapath actions: 
>> tnl_push(tnl_port(4789),header(size=50,type=4,eth(dst=f8:bc:12:44:34:b
>> 7,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.93
>> ,proto=17,tos=0,ttl=64,frag=0x4000),udp(src=0,dst=4789,csum=0xffff),vx
>> lan(flags=0x8000000,vni=0x7c)),out_port(100))
>> +  [Datapath actions: drop
>>  ])
>>  
>>  dnl Check GRE tunnel push
>>  AT_CHECK([ovs-ofctl add-flow int-br action=3])  AT_CHECK([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)
>> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
>> -  [Datapath actions: 
>> tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,s
>> rc=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,pr
>> oto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x6558),key=0
>> x1c8)),out_port(100))
>> +  [Datapath actions: drop
>>  ])
>>  
>>  dnl Check L3GRE tunnel push
>>  AT_CHECK([ovs-ofctl add-flow int-br action=7])  AT_CHECK([ovs-appctl 
>> ofproto/trace ovs-dummy 
>> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
>> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
>> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
>> -  [Datapath actions: 
>> pop_eth,tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44
>> :34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1
>> .2.92,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x800
>> ),key=0x1c8)),out_port(100))
>> +  [Datapath actions: pop_eth
>>  ])
>>  
>>  dnl Check Geneve tunnel push
>>  AT_CHECK([ovs-ofctl add-flow int-br 
>> "actions=set_field:1.1.2.92->tun_dst,5"])
>>  AT_CHECK([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)
>> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
>> -  [Datapath actions: 
>> tnl_push(tnl_port(6081),header(size=50,type=5,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=6081,csum=0x0),genev
>> e(vni=0x7b)),out_port(100))
>> +  [Datapath actions: drop
>>  ])
>>  
>>  dnl Check Geneve tunnel push with pkt-mark  AT_CHECK([ovs-ofctl 
>> add-flow int-br "actions=set_tunnel:234,6"])  AT_CHECK([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)
>> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
>> -  [Datapath actions: 
>> set(skb_mark(0x4d2)),tnl_push(tnl_port(6081),header(size=50,type=5,eth
>> (dst=f8:bc:12:44:34:b7,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=
>> 1.1.2.88,dst=1.1.2.93,proto=17,tos=0,ttl=64,frag=0x4000),udp(src=0,dst
>> =6081,csum=0x0),geneve(vni=0xea)),out_port(100))
>> +  [Datapath actions: set(skb_mark(0x4d2))
>>  ])
>>  
>>  dnl Check Geneve tunnel push with options @@ -163,7 +163,7 @@ 
>> AT_CHECK([ovs-ofctl add-tlv-map int-br 
>> "{class=0xffff,type=0x80,len=4}->tun_meta
>>  AT_CHECK([ovs-ofctl add-flow int-br 
>> "actions=set_field:1.1.2.92->tun_dst,set_field:0xa->tun_metadata0,5"])
>>  AT_CHECK([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)
>> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
>> -  [Datapath actions: 
>> tnl_push(tnl_port(6081),header(size=58,type=5,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=6081,csum=0x0),genev
>> e(crit,vni=0x7b,options({class=0xffff,type=0x80,len=4,0xa}))),out_port
>> (100))
>> +  [Datapath actions: drop
>>  ])
>>  
>>  dnl Check decapsulation of GRE packet @@ -325,7 +325,7 @@ 
>> AT_CHECK([ovs-ofctl add-flow int-br action=3])
>>  
>>  AT_CHECK([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=17,tos=0,ttl=64,frag=no)
>> ,udp(src=51283,dst=4789)'], [0], [stdout])  AT_CHECK([tail -1 stdout], 
>> [0],
>> -  [Datapath actions: 
>> tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,s
>> rc=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,pr
>> oto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x6558),key=0
>> x1c8)),out_port(100))
>> +  [Datapath actions: drop
>>  ])
>>  
>>  dnl Verify outer L2 and L3 header flow fields can be matched in the 
>> underlay bridge
>>

Well, I guess ideally one would backport 2ce9e71bb960 with a
clarification to the commit message and then un-revert the fix.  That
could be made easy for me by posting a 2-patch series, I guess.

On Wed, Feb 27, 2019 at 05:26:30PM +0300, Ilya Maximets wrote:
> Looks like I found the solution.
> Following patch on branch-2.10 makes tests immune to the change:
> 
>   2ce9e71bb960 ("tests: Inject ARP replies for snoop tests on different port")
> 
> It's the test change only that makes tunnel_push to be not the last action.
> If we'll backport it to 2.9, we could safely un-revert the fix.
> However, commit message of the 2ce9e71bb960 will not be fully correct because
> we're not going to backport the feature it was made for.
> 
> Ben, what do you think?
> 
> Best regards, Ilya Maximets.
> 
> On 27.02.2019 16:28, Anju Thomas wrote:
> > So then should we refrain from adding this fix to 2.9 if it does not make sense or should I remove the cases altogether?
> > 
> > Regards
> > Anju
> > 
> > -----Original Message-----
> > From: Ilya Maximets [mailto:i.maximets@samsung.com] 
> > Sent: Wednesday, February 27, 2019 6:55 PM
> > To: Anju Thomas <anju.thomas@ericsson.com>; dev@openvswitch.org
> > Cc: Ben Pfaff <blp@ovn.org>
> > Subject: Re: [ovs-dev] Fix crash due to multiple tnl push action
> > 
> > Hi.
> > Thanks for the patch.
> > 
> > Few comments:
> > 
> > 1. You may use --subject-prefix="PATCH branch-2.9". In this case ovsrobot
> >    will be able to apply patch to the right branch for checking.
> > 
> > 2. This patch fixes tests (I didn't check), but it effectively makes them
> >    useless by replacing all the useful data by simple 'drop' action.
> >    It's the same as just removing the broken test.
> > 
> > Best regards, Ilya Maximets.
> > 
> > On 27.02.2019 14:41, Anju Thomas wrote:
> >> During slow path packet processing, if the action is to output to a 
> >> tunnel port, the slow path processing of the encapsulated packet 
> >> continues on the underlay bridge and additional actions (e.g. optional 
> >> VLAN encapsulation, bond link selection and finally output to port) 
> >> are collected there.
> >>
> >> To prepare for a continuation of the processing of the original packet 
> >> (e.g. output to other tunnel ports in a flooding scenario), the 
> >> “tunnel_push” action and the actions of the underlay bridge are 
> >> encapsulated in a clone() action to preserve the original packet.
> >>
> >> If the underlay bridge decides to drop the tunnel packet (for example 
> >> if both bonded ports are down simultaneously), the clone(tunnel_push)) 
> >> actions previously generated as part of translation of the output to 
> >> tunnel port are discarded and a stand-alone tunnel_push action is 
> >> added instead. Thus the tunnel header is pushed on to the original packet.
> >> This is the bug.
> >>
> >> Consequences: If packet processing continues with sending to further 
> >> tunnel ports, multiple tunnel header pushes will happen on the 
> >> original packet as typically the tunnels all traverse the same 
> >> underlay bond which is down. The packet may not have enough headroom 
> >> to accommodate all the tunnel headers. OVS crashes if it runs out of 
> >> space while trying to push the tunnel headers.
> >>
> >> Even in case there is enough headroom, the packet will not be freed 
> >> since the accumulated action list contains only the tunnel header push 
> >> action without any output port action. Thus, we either have a crash or 
> >> a packet buffer leak.
> >>
> >> Signed-off-by: Anju Thomas <anju.thomas@ericsson.com>
> >> ---
> >>  ofproto/ofproto-dpif-xlate.c  |  7 -------  
> >> tests/tunnel-push-pop-ipv6.at | 10 +++++-----
> >>  tests/tunnel-push-pop.at      | 18 +++++++++---------
> >>  3 files changed, 14 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/ofproto/ofproto-dpif-xlate.c 
> >> b/ofproto/ofproto-dpif-xlate.c index 7777ed8..7e69469 100644
> >> --- a/ofproto/ofproto-dpif-xlate.c
> >> +++ b/ofproto/ofproto-dpif-xlate.c
> >> @@ -3510,13 +3510,6 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct xport *xport,
> >>              nl_msg_end_non_empty_nested(ctx->odp_actions, clone_ofs);
> >>          } else {
> >>              nl_msg_cancel_nested(ctx->odp_actions, clone_ofs);
> >> -            /* XXX : There is no real use-case for a tunnel push without
> >> -             * any post actions. However keeping it now
> >> -             * as is to make the 'make check' happy. Should remove when all the
> >> -             * make check tunnel test case does something meaningful on a
> >> -             * tunnel encap packets.
> >> -             */
> >> -            odp_put_tnl_push_action(ctx->odp_actions, &tnl_push_data);
> >>          }
> >>  
> >>          /* Restore context status. */ diff --git 
> >> a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at index 
> >> 7ca522a..c9e4a2b 100644
> >> --- a/tests/tunnel-push-pop-ipv6.at
> >> +++ b/tests/tunnel-push-pop-ipv6.at
> >> @@ -93,28 +93,28 @@ dnl Check VXLAN tunnel push  AT_CHECK([ovs-ofctl 
> >> add-flow int-br action=2])  AT_CHECK([ovs-appctl ofproto/trace 
> >> ovs-dummy 
> >> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> >> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> >> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> >> -  [Datapath actions: 
> >> tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b
> >> 6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=200
> >> 1:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,c
> >> sum=0xffff),vxlan(flags=0x8000000,vni=0x7b)),out_port(100))
> >> +  [Datapath actions: drop
> >>  ])
> >>  
> >>  dnl Check VXLAN tunnel push set tunnel id by flow and checksum  
> >> AT_CHECK([ovs-ofctl add-flow int-br "actions=set_tunnel:124,4"])  
> >> AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
> >> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> >> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> >> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> >> -  [Datapath actions: 
> >> tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b
> >> 7,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=200
> >> 1:cafe::93,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,c
> >> sum=0xffff),vxlan(flags=0x8000000,vni=0x7c)),out_port(100))
> >> +  [Datapath actions: drop
> >>  ])
> >>  
> >>  dnl Check GRE tunnel push
> >>  AT_CHECK([ovs-ofctl add-flow int-br action=3])  AT_CHECK([ovs-appctl 
> >> ofproto/trace ovs-dummy 
> >> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> >> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> >> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> >> -  [Datapath actions: 
> >> tnl_push(tnl_port(3),header(size=62,type=3,eth(dst=f8:bc:12:44:34:b6,s
> >> rc=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:c
> >> afe::92,label=0,proto=47,tclass=0x0,hlimit=64),gre((flags=0x2000,proto
> >> =0x6558),key=0x1c8)),out_port(100))
> >> +  [Datapath actions: drop
> >>  ])
> >>  
> >>  dnl Check Geneve tunnel push
> >>  AT_CHECK([ovs-ofctl add-flow int-br 
> >> "actions=set_field:2001:cafe::92->tun_ipv6_dst,5"])
> >>  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
> >> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> >> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> >> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> >> -  [Datapath actions: 
> >> tnl_push(tnl_port(6081),header(size=70,type=5,eth(dst=f8:bc:12:44:34:b
> >> 6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=200
> >> 1:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=6081,c
> >> sum=0xffff),geneve(vni=0x7b)),out_port(100))
> >> +  [Datapath actions: drop
> >>  ])
> >>  
> >>  dnl Check Geneve tunnel push with options @@ -122,7 +122,7 @@ 
> >> AT_CHECK([ovs-ofctl add-tlv-map int-br 
> >> "{class=0xffff,type=0x80,len=4}->tun_meta
> >>  AT_CHECK([ovs-ofctl add-flow int-br 
> >> "actions=set_field:2001:cafe::92->tun_ipv6_dst,set_field:0xa->tun_meta
> >> data0,5"])  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
> >> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> >> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> >> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> >> -  [Datapath actions: 
> >> tnl_push(tnl_port(6081),header(size=78,type=5,eth(dst=f8:bc:12:44:34:b
> >> 6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=200
> >> 1:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=6081,c
> >> sum=0xffff),geneve(crit,vni=0x7b,options({class=0xffff,type=0x80,len=4
> >> ,0xa}))),out_port(100))
> >> +  [Datapath actions: drop
> >>  ])
> >>  
> >>  dnl Check decapsulation of GRE packet diff --git 
> >> a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at index 
> >> cc8b1b5..07832fc 100644
> >> --- a/tests/tunnel-push-pop.at
> >> +++ b/tests/tunnel-push-pop.at
> >> @@ -113,49 +113,49 @@ dnl Check VXLAN tunnel push  AT_CHECK([ovs-ofctl 
> >> add-flow int-br action=2])  AT_CHECK([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)
> >> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> >> -  [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
> >>  ])
> >>  
> >>  dnl Check VXLAN GPE tunnel push
> >>  AT_CHECK([ovs-ofctl add-flow int-br action=8])  AT_CHECK([ovs-appctl 
> >> ofproto/trace ovs-dummy 
> >> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> >> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> >> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> >> -  [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=0xc000003,vni=0x159)),out_port(100))
> >> +  [Datapath actions: drop
> >>  ])
> >>  
> >>  dnl Check VXLAN tunnel push set tunnel id by flow and checksum  
> >> AT_CHECK([ovs-ofctl add-flow int-br "actions=set_tunnel:124,4"])  
> >> AT_CHECK([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)
> >> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> >> -  [Datapath actions: 
> >> tnl_push(tnl_port(4789),header(size=50,type=4,eth(dst=f8:bc:12:44:34:b
> >> 7,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.93
> >> ,proto=17,tos=0,ttl=64,frag=0x4000),udp(src=0,dst=4789,csum=0xffff),vx
> >> lan(flags=0x8000000,vni=0x7c)),out_port(100))
> >> +  [Datapath actions: drop
> >>  ])
> >>  
> >>  dnl Check GRE tunnel push
> >>  AT_CHECK([ovs-ofctl add-flow int-br action=3])  AT_CHECK([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)
> >> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> >> -  [Datapath actions: 
> >> tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,s
> >> rc=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,pr
> >> oto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x6558),key=0
> >> x1c8)),out_port(100))
> >> +  [Datapath actions: drop
> >>  ])
> >>  
> >>  dnl Check L3GRE tunnel push
> >>  AT_CHECK([ovs-ofctl add-flow int-br action=7])  AT_CHECK([ovs-appctl 
> >> ofproto/trace ovs-dummy 
> >> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> >> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> >> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> >> -  [Datapath actions: 
> >> pop_eth,tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44
> >> :34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1
> >> .2.92,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x800
> >> ),key=0x1c8)),out_port(100))
> >> +  [Datapath actions: pop_eth
> >>  ])
> >>  
> >>  dnl Check Geneve tunnel push
> >>  AT_CHECK([ovs-ofctl add-flow int-br 
> >> "actions=set_field:1.1.2.92->tun_dst,5"])
> >>  AT_CHECK([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)
> >> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> >> -  [Datapath actions: 
> >> tnl_push(tnl_port(6081),header(size=50,type=5,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=6081,csum=0x0),genev
> >> e(vni=0x7b)),out_port(100))
> >> +  [Datapath actions: drop
> >>  ])
> >>  
> >>  dnl Check Geneve tunnel push with pkt-mark  AT_CHECK([ovs-ofctl 
> >> add-flow int-br "actions=set_tunnel:234,6"])  AT_CHECK([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)
> >> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> >> -  [Datapath actions: 
> >> set(skb_mark(0x4d2)),tnl_push(tnl_port(6081),header(size=50,type=5,eth
> >> (dst=f8:bc:12:44:34:b7,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=
> >> 1.1.2.88,dst=1.1.2.93,proto=17,tos=0,ttl=64,frag=0x4000),udp(src=0,dst
> >> =6081,csum=0x0),geneve(vni=0xea)),out_port(100))
> >> +  [Datapath actions: set(skb_mark(0x4d2))
> >>  ])
> >>  
> >>  dnl Check Geneve tunnel push with options @@ -163,7 +163,7 @@ 
> >> AT_CHECK([ovs-ofctl add-tlv-map int-br 
> >> "{class=0xffff,type=0x80,len=4}->tun_meta
> >>  AT_CHECK([ovs-ofctl add-flow int-br 
> >> "actions=set_field:1.1.2.92->tun_dst,set_field:0xa->tun_metadata0,5"])
> >>  AT_CHECK([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)
> >> '], [0], [stdout])  AT_CHECK([tail -1 stdout], [0],
> >> -  [Datapath actions: 
> >> tnl_push(tnl_port(6081),header(size=58,type=5,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=6081,csum=0x0),genev
> >> e(crit,vni=0x7b,options({class=0xffff,type=0x80,len=4,0xa}))),out_port
> >> (100))
> >> +  [Datapath actions: drop
> >>  ])
> >>  
> >>  dnl Check decapsulation of GRE packet @@ -325,7 +325,7 @@ 
> >> AT_CHECK([ovs-ofctl add-flow int-br action=3])
> >>  
> >>  AT_CHECK([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=17,tos=0,ttl=64,frag=no)
> >> ,udp(src=51283,dst=4789)'], [0], [stdout])  AT_CHECK([tail -1 stdout], 
> >> [0],
> >> -  [Datapath actions: 
> >> tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,s
> >> rc=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,pr
> >> oto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x6558),key=0
> >> x1c8)),out_port(100))
> >> +  [Datapath actions: drop
> >>  ])
> >>  
> >>  dnl Verify outer L2 and L3 header flow fields can be matched in the 
> >> underlay bridge
> >>