Re: [ovs-dev] [PATCH 1/2] Revert "tunneling: Avoid recirculation on datapath."

2017-05-09 Thread Darrell Ball 
It might be good to add a test that checks that “all the fields” associated 
with tunnel header can be
matched in the underlay bridge context
I did a quick test with your patch:
ip_proto/nw_proto was fine and matching GRE
I could also successfully match on ip_dst/nw_dst but not on ip_src/nw_src, for 
example.



On 5/9/17, 4:55 AM, "ovs-dev-boun...@openvswitch.org on behalf of Zoltán 
Balogh" <ovs-dev-boun...@openvswitch.org on behalf of 
zoltan.bal...@ericsson.com> wrote:

Hi,

I created a patch that updates flow and base_flow L2/L3 members when 
pushing packet to tunnel, and fixes statistics on underlay bridge. 
I posted it to the original thread:

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DMay_332265.html=DwICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=lNfYVOoPGZRl8beVCPBv23EX5aIqrWrIRcpodjAIkwE=6UWX4CTnpSwFgj4kj0bhBaDTfh5fldH_61-lbc9AG8U=
 

As I see, you have not reverted the "Avoid recirculation" patch yet. If my 
new patch is accepted then it's not needed to revert the original commit. Could 
you have a look at the new patch, please?

If you are going to revert the original patch, then I'm going to merge the 
new patch with the original one and resend it to the list.

Best regards,
Zoltan


> -Original Message-
> From: ovs-dev-boun...@openvswitch.org 
[mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Joe Stringer
> Sent: Tuesday, May 09, 2017 5:41 AM
> To: William Tu <u9012...@gmail.com>
> Cc: ovs-dev <ovs-dev@openvswitch.org>
> Subject: Re: [ovs-dev] [PATCH 1/2] Revert "tunneling: Avoid recirculation 
on datapath."
> 
> On 8 May 2017 at 17:35, William Tu <u9012...@gmail.com> wrote:
> > Hi Joe and Greg,
> >
> > Maybe it's better I put this revert tunneling patch (1/2) and its
> > tunnel-tests (2/2) in one patch, so the "make check" can pass?
> 
> They can be separate. It's currently broken, the revert will fix it.
> The test can be an independent submission. I'd rather not fold yet
> more changes into the revert.
> ___
> dev mailing list
> d...@openvswitch.org
> 
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=lNfYVOoPGZRl8beVCPBv23EX5aIqrWrIRcpodjAIkwE=x9d45zty5nZTMZsW3A2BSENnB72aAhk6Ed8hMQ3h_xA=
 
___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=lNfYVOoPGZRl8beVCPBv23EX5aIqrWrIRcpodjAIkwE=x9d45zty5nZTMZsW3A2BSENnB72aAhk6Ed8hMQ3h_xA=
 






___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] Revert "tunneling: Avoid recirculation on datapath."

2017-05-09 Thread Zoltán Balogh 
Hi,

I created a patch that updates flow and base_flow L2/L3 members when pushing 
packet to tunnel, and fixes statistics on underlay bridge. 
I posted it to the original thread:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/332265.html

As I see, you have not reverted the "Avoid recirculation" patch yet. If my new 
patch is accepted then it's not needed to revert the original commit. Could you 
have a look at the new patch, please?

If you are going to revert the original patch, then I'm going to merge the new 
patch with the original one and resend it to the list.

Best regards,
Zoltan


> -Original Message-
> From: ovs-dev-boun...@openvswitch.org 
> [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Joe Stringer
> Sent: Tuesday, May 09, 2017 5:41 AM
> To: William Tu <u9012...@gmail.com>
> Cc: ovs-dev <ovs-dev@openvswitch.org>
> Subject: Re: [ovs-dev] [PATCH 1/2] Revert "tunneling: Avoid recirculation on 
> datapath."
> 
> On 8 May 2017 at 17:35, William Tu <u9012...@gmail.com> wrote:
> > Hi Joe and Greg,
> >
> > Maybe it's better I put this revert tunneling patch (1/2) and its
> > tunnel-tests (2/2) in one patch, so the "make check" can pass?
> 
> They can be separate. It's currently broken, the revert will fix it.
> The test can be an independent submission. I'd rather not fold yet
> more changes into the revert.
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] Revert "tunneling: Avoid recirculation on datapath."

2017-05-08 Thread Joe Stringer 
On 8 May 2017 at 17:35, William Tu  wrote:
> Hi Joe and Greg,
>
> Maybe it's better I put this revert tunneling patch (1/2) and its
> tunnel-tests (2/2) in one patch, so the "make check" can pass?

They can be separate. It's currently broken, the revert will fix it.
The test can be an independent submission. I'd rather not fold yet
more changes into the revert.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] Revert "tunneling: Avoid recirculation on datapath."

2017-05-08 Thread Greg Rose 
On Mon, 2017-05-08 at 17:35 -0700, William Tu wrote:
> Hi Joe and Greg,
> 
> Maybe it's better I put this revert tunneling patch (1/2) and its
> tunnel-tests (2/2) in one patch, so the "make check" can pass?
> 
> Regards,
> William

I don't understand... the revert has to be it's own separate patch so
that it can address the commit specified.

I'm not sure about the other implied policy here - are we not able to
revert a patch and then commit another one because in between those two
commits a 'make check' will fail?

- Greg

> 
> On Mon, May 8, 2017 at 11:34 AM, Greg Rose  wrote:
> > On Mon, 2017-05-08 at 11:15 -0700, Joe Stringer wrote:
> >> This reverts commit f1dac5128ce6db2e493f0d1c7a8b53fb9f34476f. When this
> >> commit was introduced, it broke the 'make check-system-userspace'
> >> testsuite. It appears that the new translation fails to modify the flow
> >> in a way that would represent the flow as an encapsulated flow when the
> >> traffic is patched through to the second bridge. As such, rather than
> >> matching on, for example, "ip,proto=47" for gre, it would use the inner
> >> packet's flow headers. It also results in problems reporting statistics,
> >> as the tunnel's header is not reflected in subsequent statistics and
> >> truncation is not properly applied during translation.
> >>
> >> While a refreshed approach to solving the above problem is formed,
> >> revert this patch.
> >>
> >> Reported-at: 
> >> https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/331972.html
> >> Signed-off-by: Joe Stringer 
> >
> > Acked-by: Greg Rose 
> >
> >> ---
> >>  lib/dpif-netdev.c |  18 ++-
> >>  ofproto/ofproto-dpif-xlate.c  | 280 
> >> --
> >>  tests/ofproto-dpif.at |  11 +-
> >>  tests/ovn.at  |   6 +-
> >>  tests/tunnel-push-pop-ipv6.at |  10 +-
> >>  tests/tunnel-push-pop.at  |  12 +-
> >>  6 files changed, 173 insertions(+), 164 deletions(-)
> >>
> >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> >> index 4ee5d058aff8..d21515657634 100644
> >> --- a/lib/dpif-netdev.c
> >> +++ b/lib/dpif-netdev.c
> >> @@ -4970,8 +4970,24 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
> >> *packets_,
> >>
> >>  case OVS_ACTION_ATTR_TUNNEL_PUSH:
> >>  if (*depth < MAX_RECIRC_DEPTH) {
> >> +struct dp_packet_batch tnl_pkt;
> >> +struct dp_packet_batch *orig_packets_ = packets_;
> >> +int err;
> >> +
> >> +if (!may_steal) {
> >> +dp_packet_batch_clone(_pkt, packets_);
> >> +packets_ = _pkt;
> >> +dp_packet_batch_reset_cutlen(orig_packets_);
> >> +}
> >> +
> >>  dp_packet_batch_apply_cutlen(packets_);
> >> -push_tnl_action(pmd, a, packets_);
> >> +
> >> +err = push_tnl_action(pmd, a, packets_);
> >> +if (!err) {
> >> +(*depth)++;
> >> +dp_netdev_recirculate(pmd, packets_);
> >> +(*depth)--;
> >> +}
> >>  return;
> >>  }
> >>  break;
> >> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> >> index b308f21de100..bc3a310227da 100644
> >> --- a/ofproto/ofproto-dpif-xlate.c
> >> +++ b/ofproto/ofproto-dpif-xlate.c
> >> @@ -425,10 +425,6 @@ static void xlate_action_set(struct xlate_ctx *ctx);
> >>  static void xlate_commit_actions(struct xlate_ctx *ctx);
> >>
> >>  static void
> >> -apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport 
> >> *in_dev,
> >> -  struct xport *out_dev);
> >> -
> >> -static void
> >>  ctx_trigger_freeze(struct xlate_ctx *ctx)
> >>  {
> >>  ctx->exit = true;
> >> @@ -3215,17 +3211,7 @@ build_tunnel_send(struct xlate_ctx *ctx, const 
> >> struct xport *xport,
> >>  }
> >>  tnl_push_data.tnl_port = odp_to_u32(tunnel_odp_port);
> >>  tnl_push_data.out_port = odp_to_u32(out_dev->odp_port);
> >> -
> >> -size_t push_action_size = 0;
> >> -size_t clone_ofs = nl_msg_start_nested(ctx->odp_actions,
> >> -   OVS_ACTION_ATTR_CLONE);
> >>  odp_put_tnl_push_action(ctx->odp_actions, _push_data);
> >> -push_action_size = ctx->odp_actions->size;
> >> -apply_nested_clone_actions(ctx, xport, out_dev);
> >> -if (ctx->odp_actions->size > push_action_size) {
> >> -/* Update the CLONE action only when combined */
> >> -nl_msg_end_nested(ctx->odp_actions, clone_ofs);
> >> -}
> >>  return 0;
> >>  }
> >>
> >> @@ -3261,136 +3247,6 @@ xlate_flow_is_protected(const struct xlate_ctx 
> >> *ctx, const struct flow *flow, co
> >>  xport_in->xbundle->protected && 
> >> xport_out->xbundle->protected);
> >>  }
> >>
> >> -/* Populate and apply nested actions on 'out_dev'.
> >> - * The nested actions are applied on cloned packets in dp while 
> >> outputting to
> >> - *

Re: [ovs-dev] [PATCH 1/2] Revert "tunneling: Avoid recirculation on datapath."

2017-05-08 Thread William Tu 
Hi Joe and Greg,

Maybe it's better I put this revert tunneling patch (1/2) and its
tunnel-tests (2/2) in one patch, so the "make check" can pass?

Regards,
William

On Mon, May 8, 2017 at 11:34 AM, Greg Rose  wrote:
> On Mon, 2017-05-08 at 11:15 -0700, Joe Stringer wrote:
>> This reverts commit f1dac5128ce6db2e493f0d1c7a8b53fb9f34476f. When this
>> commit was introduced, it broke the 'make check-system-userspace'
>> testsuite. It appears that the new translation fails to modify the flow
>> in a way that would represent the flow as an encapsulated flow when the
>> traffic is patched through to the second bridge. As such, rather than
>> matching on, for example, "ip,proto=47" for gre, it would use the inner
>> packet's flow headers. It also results in problems reporting statistics,
>> as the tunnel's header is not reflected in subsequent statistics and
>> truncation is not properly applied during translation.
>>
>> While a refreshed approach to solving the above problem is formed,
>> revert this patch.
>>
>> Reported-at: 
>> https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/331972.html
>> Signed-off-by: Joe Stringer 
>
> Acked-by: Greg Rose 
>
>> ---
>>  lib/dpif-netdev.c |  18 ++-
>>  ofproto/ofproto-dpif-xlate.c  | 280 
>> --
>>  tests/ofproto-dpif.at |  11 +-
>>  tests/ovn.at  |   6 +-
>>  tests/tunnel-push-pop-ipv6.at |  10 +-
>>  tests/tunnel-push-pop.at  |  12 +-
>>  6 files changed, 173 insertions(+), 164 deletions(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 4ee5d058aff8..d21515657634 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -4970,8 +4970,24 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
>> *packets_,
>>
>>  case OVS_ACTION_ATTR_TUNNEL_PUSH:
>>  if (*depth < MAX_RECIRC_DEPTH) {
>> +struct dp_packet_batch tnl_pkt;
>> +struct dp_packet_batch *orig_packets_ = packets_;
>> +int err;
>> +
>> +if (!may_steal) {
>> +dp_packet_batch_clone(_pkt, packets_);
>> +packets_ = _pkt;
>> +dp_packet_batch_reset_cutlen(orig_packets_);
>> +}
>> +
>>  dp_packet_batch_apply_cutlen(packets_);
>> -push_tnl_action(pmd, a, packets_);
>> +
>> +err = push_tnl_action(pmd, a, packets_);
>> +if (!err) {
>> +(*depth)++;
>> +dp_netdev_recirculate(pmd, packets_);
>> +(*depth)--;
>> +}
>>  return;
>>  }
>>  break;
>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>> index b308f21de100..bc3a310227da 100644
>> --- a/ofproto/ofproto-dpif-xlate.c
>> +++ b/ofproto/ofproto-dpif-xlate.c
>> @@ -425,10 +425,6 @@ static void xlate_action_set(struct xlate_ctx *ctx);
>>  static void xlate_commit_actions(struct xlate_ctx *ctx);
>>
>>  static void
>> -apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport 
>> *in_dev,
>> -  struct xport *out_dev);
>> -
>> -static void
>>  ctx_trigger_freeze(struct xlate_ctx *ctx)
>>  {
>>  ctx->exit = true;
>> @@ -3215,17 +3211,7 @@ build_tunnel_send(struct xlate_ctx *ctx, const struct 
>> xport *xport,
>>  }
>>  tnl_push_data.tnl_port = odp_to_u32(tunnel_odp_port);
>>  tnl_push_data.out_port = odp_to_u32(out_dev->odp_port);
>> -
>> -size_t push_action_size = 0;
>> -size_t clone_ofs = nl_msg_start_nested(ctx->odp_actions,
>> -   OVS_ACTION_ATTR_CLONE);
>>  odp_put_tnl_push_action(ctx->odp_actions, _push_data);
>> -push_action_size = ctx->odp_actions->size;
>> -apply_nested_clone_actions(ctx, xport, out_dev);
>> -if (ctx->odp_actions->size > push_action_size) {
>> -/* Update the CLONE action only when combined */
>> -nl_msg_end_nested(ctx->odp_actions, clone_ofs);
>> -}
>>  return 0;
>>  }
>>
>> @@ -3261,136 +3247,6 @@ xlate_flow_is_protected(const struct xlate_ctx *ctx, 
>> const struct flow *flow, co
>>  xport_in->xbundle->protected && xport_out->xbundle->protected);
>>  }
>>
>> -/* Populate and apply nested actions on 'out_dev'.
>> - * The nested actions are applied on cloned packets in dp while outputting 
>> to
>> - * either patch or tunnel ports.
>> - * On output to a patch port, the output action will be replaced with set of
>> - * nested actions on the peer patch port.
>> - * Similarly on output to a tunnel port, the post nested actions on
>> - * tunnel are chained up with the tunnel-push action.
>> - */
>> -static void
>> -apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport 
>> *in_dev,
>> -  struct xport *out_dev)
>> -{
>> -struct flow *flow = >xin->flow;
>> -struct flow old_flow = ctx->xin->flow;
>> -struct flow_tnl old_flow_tnl_wc =

Re: [ovs-dev] [PATCH 1/2] Revert "tunneling: Avoid recirculation on datapath."

2017-05-08 Thread Greg Rose 
On Mon, 2017-05-08 at 11:15 -0700, Joe Stringer wrote:
> This reverts commit f1dac5128ce6db2e493f0d1c7a8b53fb9f34476f. When this
> commit was introduced, it broke the 'make check-system-userspace'
> testsuite. It appears that the new translation fails to modify the flow
> in a way that would represent the flow as an encapsulated flow when the
> traffic is patched through to the second bridge. As such, rather than
> matching on, for example, "ip,proto=47" for gre, it would use the inner
> packet's flow headers. It also results in problems reporting statistics,
> as the tunnel's header is not reflected in subsequent statistics and
> truncation is not properly applied during translation.
> 
> While a refreshed approach to solving the above problem is formed,
> revert this patch.
> 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/331972.html
> Signed-off-by: Joe Stringer 

Acked-by: Greg Rose 

> ---
>  lib/dpif-netdev.c |  18 ++-
>  ofproto/ofproto-dpif-xlate.c  | 280 
> --
>  tests/ofproto-dpif.at |  11 +-
>  tests/ovn.at  |   6 +-
>  tests/tunnel-push-pop-ipv6.at |  10 +-
>  tests/tunnel-push-pop.at  |  12 +-
>  6 files changed, 173 insertions(+), 164 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 4ee5d058aff8..d21515657634 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4970,8 +4970,24 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
> *packets_,
>  
>  case OVS_ACTION_ATTR_TUNNEL_PUSH:
>  if (*depth < MAX_RECIRC_DEPTH) {
> +struct dp_packet_batch tnl_pkt;
> +struct dp_packet_batch *orig_packets_ = packets_;
> +int err;
> +
> +if (!may_steal) {
> +dp_packet_batch_clone(_pkt, packets_);
> +packets_ = _pkt;
> +dp_packet_batch_reset_cutlen(orig_packets_);
> +}
> +
>  dp_packet_batch_apply_cutlen(packets_);
> -push_tnl_action(pmd, a, packets_);
> +
> +err = push_tnl_action(pmd, a, packets_);
> +if (!err) {
> +(*depth)++;
> +dp_netdev_recirculate(pmd, packets_);
> +(*depth)--;
> +}
>  return;
>  }
>  break;
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index b308f21de100..bc3a310227da 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -425,10 +425,6 @@ static void xlate_action_set(struct xlate_ctx *ctx);
>  static void xlate_commit_actions(struct xlate_ctx *ctx);
>  
>  static void
> -apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport *in_dev,
> -  struct xport *out_dev);
> -
> -static void
>  ctx_trigger_freeze(struct xlate_ctx *ctx)
>  {
>  ctx->exit = true;
> @@ -3215,17 +3211,7 @@ build_tunnel_send(struct xlate_ctx *ctx, const struct 
> xport *xport,
>  }
>  tnl_push_data.tnl_port = odp_to_u32(tunnel_odp_port);
>  tnl_push_data.out_port = odp_to_u32(out_dev->odp_port);
> -
> -size_t push_action_size = 0;
> -size_t clone_ofs = nl_msg_start_nested(ctx->odp_actions,
> -   OVS_ACTION_ATTR_CLONE);
>  odp_put_tnl_push_action(ctx->odp_actions, _push_data);
> -push_action_size = ctx->odp_actions->size;
> -apply_nested_clone_actions(ctx, xport, out_dev);
> -if (ctx->odp_actions->size > push_action_size) {
> -/* Update the CLONE action only when combined */
> -nl_msg_end_nested(ctx->odp_actions, clone_ofs);
> -}
>  return 0;
>  }
>  
> @@ -3261,136 +3247,6 @@ xlate_flow_is_protected(const struct xlate_ctx *ctx, 
> const struct flow *flow, co
>  xport_in->xbundle->protected && xport_out->xbundle->protected);
>  }
>  
> -/* Populate and apply nested actions on 'out_dev'.
> - * The nested actions are applied on cloned packets in dp while outputting to
> - * either patch or tunnel ports.
> - * On output to a patch port, the output action will be replaced with set of
> - * nested actions on the peer patch port.
> - * Similarly on output to a tunnel port, the post nested actions on
> - * tunnel are chained up with the tunnel-push action.
> - */
> -static void
> -apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport *in_dev,
> -  struct xport *out_dev)
> -{
> -struct flow *flow = >xin->flow;
> -struct flow old_flow = ctx->xin->flow;
> -struct flow_tnl old_flow_tnl_wc = ctx->wc->masks.tunnel;
> -bool old_conntrack = ctx->conntracked;
> -bool old_was_mpls = ctx->was_mpls;
> -ovs_version_t old_version = ctx->xin->tables_version;
> -struct ofpbuf old_stack = ctx->stack;
> -union mf_subvalue new_stack[1024 / sizeof(union mf_subvalue)];
> -struct ofpbuf old_action_set = ctx->action_set;
> -struct ovs_list

[ovs-dev] [PATCH 1/2] Revert "tunneling: Avoid recirculation on datapath."

2017-05-08 Thread Joe Stringer 
This reverts commit f1dac5128ce6db2e493f0d1c7a8b53fb9f34476f. When this
commit was introduced, it broke the 'make check-system-userspace'
testsuite. It appears that the new translation fails to modify the flow
in a way that would represent the flow as an encapsulated flow when the
traffic is patched through to the second bridge. As such, rather than
matching on, for example, "ip,proto=47" for gre, it would use the inner
packet's flow headers. It also results in problems reporting statistics,
as the tunnel's header is not reflected in subsequent statistics and
truncation is not properly applied during translation.

While a refreshed approach to solving the above problem is formed,
revert this patch.

Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/331972.html
Signed-off-by: Joe Stringer 
---
 lib/dpif-netdev.c |  18 ++-
 ofproto/ofproto-dpif-xlate.c  | 280 --
 tests/ofproto-dpif.at |  11 +-
 tests/ovn.at  |   6 +-
 tests/tunnel-push-pop-ipv6.at |  10 +-
 tests/tunnel-push-pop.at  |  12 +-
 6 files changed, 173 insertions(+), 164 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4ee5d058aff8..d21515657634 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4970,8 +4970,24 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
*packets_,
 
 case OVS_ACTION_ATTR_TUNNEL_PUSH:
 if (*depth < MAX_RECIRC_DEPTH) {
+struct dp_packet_batch tnl_pkt;
+struct dp_packet_batch *orig_packets_ = packets_;
+int err;
+
+if (!may_steal) {
+dp_packet_batch_clone(_pkt, packets_);
+packets_ = _pkt;
+dp_packet_batch_reset_cutlen(orig_packets_);
+}
+
 dp_packet_batch_apply_cutlen(packets_);
-push_tnl_action(pmd, a, packets_);
+
+err = push_tnl_action(pmd, a, packets_);
+if (!err) {
+(*depth)++;
+dp_netdev_recirculate(pmd, packets_);
+(*depth)--;
+}
 return;
 }
 break;
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index b308f21de100..bc3a310227da 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -425,10 +425,6 @@ static void xlate_action_set(struct xlate_ctx *ctx);
 static void xlate_commit_actions(struct xlate_ctx *ctx);
 
 static void
-apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport *in_dev,
-  struct xport *out_dev);
-
-static void
 ctx_trigger_freeze(struct xlate_ctx *ctx)
 {
 ctx->exit = true;
@@ -3215,17 +3211,7 @@ build_tunnel_send(struct xlate_ctx *ctx, const struct 
xport *xport,
 }
 tnl_push_data.tnl_port = odp_to_u32(tunnel_odp_port);
 tnl_push_data.out_port = odp_to_u32(out_dev->odp_port);
-
-size_t push_action_size = 0;
-size_t clone_ofs = nl_msg_start_nested(ctx->odp_actions,
-   OVS_ACTION_ATTR_CLONE);
 odp_put_tnl_push_action(ctx->odp_actions, _push_data);
-push_action_size = ctx->odp_actions->size;
-apply_nested_clone_actions(ctx, xport, out_dev);
-if (ctx->odp_actions->size > push_action_size) {
-/* Update the CLONE action only when combined */
-nl_msg_end_nested(ctx->odp_actions, clone_ofs);
-}
 return 0;
 }
 
@@ -3261,136 +3247,6 @@ xlate_flow_is_protected(const struct xlate_ctx *ctx, 
const struct flow *flow, co
 xport_in->xbundle->protected && xport_out->xbundle->protected);
 }
 
-/* Populate and apply nested actions on 'out_dev'.
- * The nested actions are applied on cloned packets in dp while outputting to
- * either patch or tunnel ports.
- * On output to a patch port, the output action will be replaced with set of
- * nested actions on the peer patch port.
- * Similarly on output to a tunnel port, the post nested actions on
- * tunnel are chained up with the tunnel-push action.
- */
-static void
-apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport *in_dev,
-  struct xport *out_dev)
-{
-struct flow *flow = >xin->flow;
-struct flow old_flow = ctx->xin->flow;
-struct flow_tnl old_flow_tnl_wc = ctx->wc->masks.tunnel;
-bool old_conntrack = ctx->conntracked;
-bool old_was_mpls = ctx->was_mpls;
-ovs_version_t old_version = ctx->xin->tables_version;
-struct ofpbuf old_stack = ctx->stack;
-union mf_subvalue new_stack[1024 / sizeof(union mf_subvalue)];
-struct ofpbuf old_action_set = ctx->action_set;
-struct ovs_list *old_trace = ctx->xin->trace;
-uint64_t actset_stub[1024 / 8];
-
-ofpbuf_use_stub(>stack, new_stack, sizeof new_stack);
-ofpbuf_use_stub(>action_set, actset_stub, sizeof actset_stub);
-flow->in_port.ofp_port = out_dev->ofp_port;
-flow->metadata = htonll(0);
-memset(>tunnel, 0, sizeof flow->tunnel);
-