diff mbox series

[ovs-dev,v4] ofproto-dpif-xlate: Optimize the clone for patch ports

Message ID 20220905104454.988101-1-amusil@redhat.com
State Superseded
Headers show
Series [ovs-dev,v4] ofproto-dpif-xlate: Optimize the clone for patch ports | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Ales Musil Sept. 5, 2022, 10:44 a.m. UTC 
When the packet was traveling through patch port boundary
OvS would check if any of the actions is reversible,
if not it would clone the packet. However, the check
was only at the first level of the second bridge.
That caused some issues when the packet had gone
through more actions, some of them might have been
irreversible.

In order to keep the semantics the same we might
need to run the actions twice in the worst case
scenario. During the clone there are 4 scenarios
that can happen.

1) The action is last one for that flow,
in that case we just continue without clone.

2) There is irreversible action in the action
set (first level). In this case we know that
there is at leas one irreversible action which
is enough to do clone.

3) All actions in first level are reversible,
we can try to run all actions as if we don't
need any clone and inspect the ofpbuf at the
end. In positive case there are no irreversible
actions so we will just submit the buffer and continue.

4) This is same as 3) with the difference that
there is irreversible action in the ofpbuf.
To keep the semantics we need to re-run the actions
and treat it as clone. This requires resotration
of the xlate_ctx.

Add test cases for all irreversible actions
to see if they are properly cloned.

Signed-off-by: Ales Musil <amusil@redhat.com>
---
v4: Rebase on master.
    Address comments from Eelco.
---
 lib/odp-util.c               | 148 +++++++++++++++++++++++++++++++++++
 lib/odp-util.h               |   1 +
 ofproto/ofproto-dpif-trace.c |   2 +-
 ofproto/ofproto-dpif-trace.h |   1 +
 ofproto/ofproto-dpif-xlate.c | 115 ++++++++++++++++++++-------
 tests/ofproto-dpif.at        | 105 +++++++++++++++++++++++++
 6 files changed, 344 insertions(+), 28 deletions(-)

Comments

0-day Robot Sept. 5, 2022, 10:58 a.m. UTC | #1 
Bleep bloop.  Greetings Ales Musil, 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.


checkpatch:
ERROR: Improper whitespace around control block
#154 FILE: lib/odp-util.c:8865:
    NL_ATTR_FOR_EACH(attr, left, attrs, attrs_len) {

Lines checked: 525, Warnings: 0, Errors: 1


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

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/lib/odp-util.c b/lib/odp-util.c
index ba5be4bb3..2503a4af7 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -8768,3 +8768,151 @@  commit_odp_actions(const struct flow *flow, struct flow *base,
 
     return slow1 ? slow1 : slow2;
 }
+
+static inline bool
+nlattr_action_is_reversible(const uint16_t type)
+{
+    switch ((enum ovs_action_attr) type) {
+        case OVS_ACTION_ATTR_CT:
+        case OVS_ACTION_ATTR_CT_CLEAR:
+        case OVS_ACTION_ATTR_TRUNC:
+        case OVS_ACTION_ATTR_PUSH_ETH:
+        case OVS_ACTION_ATTR_POP_ETH:
+        case OVS_ACTION_ATTR_PUSH_NSH:
+        case OVS_ACTION_ATTR_POP_NSH:
+        case OVS_ACTION_ATTR_METER:
+        case OVS_ACTION_ATTR_TUNNEL_PUSH:
+        case OVS_ACTION_ATTR_TUNNEL_POP:
+            return false;
+
+        case OVS_ACTION_ATTR_UNSPEC:
+        case OVS_ACTION_ATTR_OUTPUT:
+        case OVS_ACTION_ATTR_USERSPACE:
+        case OVS_ACTION_ATTR_SET:
+        case OVS_ACTION_ATTR_PUSH_VLAN:
+        case OVS_ACTION_ATTR_POP_VLAN:
+        case OVS_ACTION_ATTR_SAMPLE:
+        case OVS_ACTION_ATTR_RECIRC:
+        case OVS_ACTION_ATTR_HASH:
+        case OVS_ACTION_ATTR_SET_MASKED:
+        case OVS_ACTION_ATTR_CLONE:
+        case OVS_ACTION_ATTR_CHECK_PKT_LEN:
+        case OVS_ACTION_ATTR_LB_OUTPUT:
+        case OVS_ACTION_ATTR_ADD_MPLS:
+        case OVS_ACTION_ATTR_PUSH_MPLS:
+        case OVS_ACTION_ATTR_POP_MPLS:
+        case OVS_ACTION_ATTR_DROP:
+        case __OVS_ACTION_ATTR_MAX:
+            return true;
+    }
+    return false;
+}
+
+static bool
+odp_cpl_contains_irreversible_actions(const struct nlattr *attr)
+{
+    static const struct nl_policy ovs_cpl_policy[] = {
+        [OVS_CHECK_PKT_LEN_ATTR_PKT_LEN] = {.type = NL_A_U16},
+        [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER] = {.type = NL_A_NESTED},
+        [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL] = {.type = NL_A_NESTED},
+    };
+    struct nlattr *a[ARRAY_SIZE(ovs_cpl_policy)];
+
+    if (!nl_parse_nested(attr, ovs_cpl_policy, a, ARRAY_SIZE(a))) {
+        return false;
+    }
+
+    const struct nlattr *greater =
+        a[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER];
+    const struct nlattr *less =
+        a[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL];
+    const void *greater_data = nl_attr_get(greater);
+    const void *less_data = nl_attr_get(less);
+    size_t greater_len = nl_attr_get_size(greater);
+    size_t less_len = nl_attr_get_size(less);
+
+    return odp_contains_irreversible_action(greater_data, greater_len) ||
+           odp_contains_irreversible_action(less_data, less_len);
+}
+
+static bool
+odp_sample_contains_irreversible_actions(const struct nlattr *attr)
+{
+    static const struct nl_policy ovs_sample_policy[] = {
+        [OVS_SAMPLE_ATTR_PROBABILITY] = {.type = NL_A_U32},
+        [OVS_SAMPLE_ATTR_ACTIONS] = {.type = NL_A_NESTED}
+    };
+    struct nlattr *a[ARRAY_SIZE(ovs_sample_policy)];
+
+    if (!nl_parse_nested(attr, ovs_sample_policy, a, ARRAY_SIZE(a))) {
+        return false;
+    }
+
+    const struct nlattr *actions = a[OVS_SAMPLE_ATTR_ACTIONS];
+    const void *actions_data = nl_attr_get(actions);
+    size_t actions_len = nl_attr_get_size(actions);
+
+    return odp_contains_irreversible_action(actions_data, actions_len);
+}
+
+/* Check if any of the actions in the ofpbuf is irreversible. */
+bool
+odp_contains_irreversible_action(const void *attrs, size_t attrs_len)
+{
+    const struct nlattr *attr;
+    int left;
+
+    NL_ATTR_FOR_EACH(attr, left, attrs, attrs_len) {
+        uint16_t type = attr->nla_type;
+
+        switch ((enum ovs_action_attr) type) {
+            /* Skip "clone" because it already encapsulates irreversible *
+             * actions. */
+            case OVS_ACTION_ATTR_CLONE:
+                continue;
+            /* Check nested actions of "check_packet_len". */
+            case OVS_ACTION_ATTR_CHECK_PKT_LEN:
+                if (odp_cpl_contains_irreversible_actions(attr)) {
+                    return true;
+                }
+                break;
+            /* Check nested actions of "sample". */
+            case OVS_ACTION_ATTR_SAMPLE:
+                if (odp_sample_contains_irreversible_actions(attr)) {
+                    return true;
+                }
+                break;
+            /* Check any other action. */
+            case OVS_ACTION_ATTR_CT:
+            case OVS_ACTION_ATTR_CT_CLEAR:
+            case OVS_ACTION_ATTR_TRUNC:
+            case OVS_ACTION_ATTR_PUSH_ETH:
+            case OVS_ACTION_ATTR_POP_ETH:
+            case OVS_ACTION_ATTR_PUSH_NSH:
+            case OVS_ACTION_ATTR_POP_NSH:
+            case OVS_ACTION_ATTR_METER:
+            case OVS_ACTION_ATTR_TUNNEL_PUSH:
+            case OVS_ACTION_ATTR_TUNNEL_POP:
+            case OVS_ACTION_ATTR_UNSPEC:
+            case OVS_ACTION_ATTR_OUTPUT:
+            case OVS_ACTION_ATTR_USERSPACE:
+            case OVS_ACTION_ATTR_SET:
+            case OVS_ACTION_ATTR_PUSH_VLAN:
+            case OVS_ACTION_ATTR_POP_VLAN:
+            case OVS_ACTION_ATTR_RECIRC:
+            case OVS_ACTION_ATTR_HASH:
+            case OVS_ACTION_ATTR_SET_MASKED:
+            case OVS_ACTION_ATTR_LB_OUTPUT:
+            case OVS_ACTION_ATTR_ADD_MPLS:
+            case OVS_ACTION_ATTR_PUSH_MPLS:
+            case OVS_ACTION_ATTR_POP_MPLS:
+            case OVS_ACTION_ATTR_DROP:
+                if (!nlattr_action_is_reversible(type)) {
+                    return true;
+                }
+            case __OVS_ACTION_ATTR_MAX:
+                continue;
+        }
+    }
+    return false;
+}
diff --git a/lib/odp-util.h b/lib/odp-util.h
index a1d0d0fba..d842974ba 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -373,6 +373,7 @@  void odp_put_pop_eth_action(struct ofpbuf *odp_actions);
 void odp_put_push_eth_action(struct ofpbuf *odp_actions,
                              const struct eth_addr *eth_src,
                              const struct eth_addr *eth_dst);
+bool odp_contains_irreversible_action(const void *attrs, size_t attrs_len);
 
 struct attr_len_tbl {
     int len;
diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index 109940ad2..c5102e3d9 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -108,7 +108,7 @@  oftrace_add_recirc_node(struct ovs_list *recirc_queue,
     return true;
 }
 
-static void
+void
 oftrace_recirc_node_destroy(struct oftrace_recirc_node *node)
 {
     if (node) {
diff --git a/ofproto/ofproto-dpif-trace.h b/ofproto/ofproto-dpif-trace.h
index 4b04f1756..463ff5b16 100644
--- a/ofproto/ofproto-dpif-trace.h
+++ b/ofproto/ofproto-dpif-trace.h
@@ -95,5 +95,6 @@  bool oftrace_add_recirc_node(struct ovs_list *recirc_queue,
                              const struct ofpact_nat *,
                              const struct dp_packet *, uint32_t recirc_id,
                              const uint16_t zone);
+void oftrace_recirc_node_destroy(struct oftrace_recirc_node *node);
 
 #endif /* ofproto-dpif-trace.h */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index fda802e83..8f97294a0 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -5824,12 +5824,11 @@  reversible_actions(const struct ofpact *ofpacts, size_t ofpacts_len)
 }
 
 static void
-clone_xlate_actions(const struct ofpact *actions, size_t actions_len,
-                    struct xlate_ctx *ctx, bool is_last_action,
-                    bool group_bucket_action OVS_UNUSED)
+do_clone_xlate_actions(const struct ofpact *actions, size_t actions_len,
+                       struct xlate_ctx *ctx)
 {
     struct ofpbuf old_stack = ctx->stack;
-    union mf_subvalue new_stack[1024 / sizeof(union mf_subvalue)];
+    union mf_subvalue new_stack[1024 / sizeof (union mf_subvalue)];
     ofpbuf_use_stub(&ctx->stack, new_stack, sizeof new_stack);
     ofpbuf_put(&ctx->stack, old_stack.data, old_stack.size);
 
@@ -5841,18 +5840,6 @@  clone_xlate_actions(const struct ofpact *actions, size_t actions_len,
     size_t offset, ac_offset;
     struct flow old_flow = ctx->xin->flow;
 
-    if (reversible_actions(actions, actions_len) || is_last_action) {
-        old_flow = ctx->xin->flow;
-        do_xlate_actions(actions, actions_len, ctx, is_last_action, false);
-        if (!ctx->freezing) {
-            xlate_action_set(ctx);
-        }
-        if (ctx->freezing) {
-            finish_freezing(ctx);
-        }
-        goto xlate_done;
-    }
-
     /* Commit datapath actions before emitting the clone action to
      * avoid emitting those actions twice. Once inside
      * the clone, another time for the action after clone.  */
@@ -5875,10 +5862,7 @@  clone_xlate_actions(const struct ofpact *actions, size_t actions_len,
             finish_freezing(ctx);
         }
         nl_msg_end_non_empty_nested(ctx->odp_actions, offset);
-        goto dp_clone_done;
-    }
-
-    if (ctx->xbridge->support.sample_nesting > 3) {
+    } else if (ctx->xbridge->support.sample_nesting > 3) {
         /* Use sample action as datapath clone. */
         offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_SAMPLE);
         ac_offset = nl_msg_start_nested(ctx->odp_actions,
@@ -5897,14 +5881,12 @@  clone_xlate_actions(const struct ofpact *actions, size_t actions_len,
                            UINT32_MAX); /* 100% probability. */
             nl_msg_end_nested(ctx->odp_actions, offset);
         }
-        goto dp_clone_done;
+    } else {
+        /* Datapath does not support clone, skip xlate 'oc' and
+         * report an error */
+        xlate_report_error(ctx, "Failed to compose clone action");
     }
 
-    /* Datapath does not support clone, skip xlate 'oc' and
-     * report an error */
-    xlate_report_error(ctx, "Failed to compose clone action");
-
-dp_clone_done:
     /* The clone's conntrack execution should have no effect on the original
      * packet. */
     ctx->conntracked = old_conntracked;
@@ -5916,7 +5898,6 @@  dp_clone_done:
     /* Restore the 'base_flow' for the next action.  */
     ctx->base_flow = old_base;
 
-xlate_done:
     ofpbuf_uninit(&ctx->action_set);
     ctx->action_set = old_action_set;
     ofpbuf_uninit(&ctx->stack);
@@ -5924,6 +5905,86 @@  xlate_done:
     ctx->xin->flow = old_flow;
 }
 
+static void
+clone_xlate_actions(const struct ofpact *actions, size_t actions_len,
+                    struct xlate_ctx *ctx, bool is_last_action,
+                    bool group_bucket_action OVS_UNUSED)
+{
+    /* We will never clone on last action, just proceed. */
+    if (is_last_action) {
+        do_xlate_actions(actions, actions_len, ctx, true, false);
+        if (!ctx->freezing) {
+            xlate_action_set(ctx);
+        }
+        if (ctx->freezing) {
+            finish_freezing(ctx);
+        }
+        return;
+    }
+
+    /* We have to clone if any of the actions is irreversible. */
+    if (!reversible_actions(actions, actions_len)) {
+        do_clone_xlate_actions(actions, actions_len, ctx);
+        return;
+    }
+
+    /* In any other case we cannot be certain if there is any irreversible
+     * action down the action stack. Let's run the actions as if they are
+     * reversible, if we find out that there was any irreversible action we
+     * can restore the xlate_ctx and run the do_clone instead. */
+    uint32_t old_stack_size = ctx->stack.size;
+    uint32_t old_action_set_size = ctx->action_set.size;
+    uint32_t old_odp_actions_size = ctx->odp_actions->size;
+
+    struct flow old_flow = ctx->xin->flow;
+    bool old_was_mpls = ctx->was_mpls;
+    bool old_conntracked = ctx->conntracked;
+    struct flow old_base = ctx->base_flow;
+    size_t old_recirc_size = ctx->xin->recirc_queue
+                             ? ovs_list_size(ctx->xin->recirc_queue)
+                             : 0;
+
+    do_xlate_actions(actions, actions_len, ctx, false, false);
+
+    /* Restore ctx parts that are common even if don't need to clone. */
+    nl_msg_reset_size(&ctx->action_set, old_action_set_size);
+    nl_msg_reset_size(&ctx->stack, old_stack_size);
+    ctx->xin->flow = old_flow;
+
+    void *added_actions =
+        ofpbuf_at_assert(ctx->odp_actions, old_odp_actions_size, 0);
+    uint32_t added_size = ctx->odp_actions->size - old_odp_actions_size;
+
+    if (!odp_contains_irreversible_action(added_actions, added_size)) {
+        if (!ctx->freezing) {
+            xlate_action_set(ctx);
+        }
+        if (ctx->freezing) {
+            finish_freezing(ctx);
+        }
+    } else {
+        nl_msg_reset_size(ctx->odp_actions, old_odp_actions_size);
+        ctx->was_mpls = old_was_mpls;
+        ctx->conntracked = old_conntracked;
+        ctx->base_flow = old_base;
+        /* Clear recirculations that were added by non-clone path so that
+         * packets are not recirculated twice. */
+        size_t recirc_size = ctx->xin->recirc_queue
+                             ? ovs_list_size(ctx->xin->recirc_queue)
+                             : 0;
+
+        for (; old_recirc_size < recirc_size; old_recirc_size++) {
+            struct oftrace_recirc_node *node =
+                CONTAINER_OF(ovs_list_pop_back(ctx->xin->recirc_queue),
+                             struct oftrace_recirc_node, node);
+
+            oftrace_recirc_node_destroy(node);
+        }
+
+        do_clone_xlate_actions(actions, actions_len, ctx);
+    }
+}
+
 static void
 compose_clone(struct xlate_ctx *ctx, const struct ofpact_nest *oc,
               bool is_last_action)
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 2e91ae1a1..93245b341 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -11817,3 +11817,108 @@  AT_CHECK([test 1 = `ovs-ofctl parse-pcap p2-tx.pcap | wc -l`])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([ofproto-dpif - nested patch ports clone])
+
+OVS_VSWITCHD_START(
+  [add-port br0 br0-p0 -- set Interface br0-p0 type=dummy ofport_request=1 -- \
+   add-port br0 br0-p1 -- set Interface br0-p1 type=patch options:peer=br1-p0 -- \
+   add-br br1 -- \
+   set bridge br1 other-config:hwaddr=aa:66:aa:66:00:01 -- \
+   set bridge br1 datapath-type=dummy other-config:datapath-id=1111 fail-mode=secure -- \
+   add-port br1 br1-p0 -- set Interface br1-p0 type=patch options:peer=br0-p1 -- \
+   add-port br1 br1-p1 -- set Interface br1-p1 type=patch options:peer=br2-p0 -- \
+   add-port br1 br1-p2 -- set Interface br1-p2 type=dummy ofport_request=2 --\
+   add-br br2 -- \
+   set bridge br2 other-config:hwaddr=aa:66:aa:66:00:02 -- \
+   set bridge br2 datapath-type=dummy other-config:datapath-id=2222 fail-mode=secure -- \
+   add-port br2 br2-p0 -- set Interface br2-p0 type=patch options:peer=br1-p1 -- \
+   add-port br2 br2-p1 -- set Interface br2-p1 type=dummy ofport_request=3])
+
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br1 'meter=1 pktps stats bands=type=drop rate=2'])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br2 'meter=1 pktps stats bands=type=drop rate=2'])
+AT_CHECK([ovs-ofctl del-flows br0])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 in_port=local,ip,actions=br0-p1,br0-p0])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br1 in_port=br1-p0,ip,actions=meter:1,br1-p1,br1-p2])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br2 in_port=br2-p0,ip,actions=meter:1,br2-p1])
+
+AT_CHECK([ovs-appctl ofproto/trace br0 "in_port(local),ip" | grep "Datapath actions:"], [0], [dnl
+Datapath actions: clone(meter(0),clone(meter(1),102),101),100
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto-dpif - patch ports - ct (clone)])
+
+OVS_VSWITCHD_START(
+  [add-port br0 p0 -- set Interface p0 type=dummy ofport_request=1 -- \
+   add-port br0 p1 -- set Interface p1 type=patch options:peer=p2 ofport_request=2 -- \
+   add-br br1 -- \
+   set bridge br1 other-config:hwaddr=aa:66:aa:66:00:00 -- \
+   set bridge br1 datapath-type=dummy other-config:datapath-id=1234 fail-mode=secure -- \
+   add-port br1 p2 -- set Interface p2 type=patch options:peer=p1 -- \
+   add-port br1 p3 -- set Interface p3 type=dummy ofport_request=3])
+
+AT_CHECK([ovs-ofctl del-flows br0])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 in_port=local,ip,actions=p1,p0])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br1 "in_port=1,ip,actions=resubmit(,3)"])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br1 "table=3,in_port=1,ip,ct_state=-trk,actions=ct(table=0)"])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br1 "table=3,in_port=1,ip,ct_state=+trk,actions=p3"])
+
+AT_CHECK([ovs-appctl ofproto/trace br0 "in_port(local),ip" | \
+          sed 's/recirc(0x[[0-9]]\+)/recirc(X)/' | grep "Datapath actions:"], [0], [dnl
+Datapath actions: clone(ct,recirc(X)),1
+Datapath actions: 3
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+
+AT_SETUP([ofproto-dpif - patch ports - trunc (clone)])
+
+OVS_VSWITCHD_START(
+  [add-port br0 p0 -- set Interface p0 type=dummy ofport_request=1 -- \
+   add-port br0 p1 -- set Interface p1 type=patch options:peer=p2 ofport_request=2 -- \
+   add-br br1 -- \
+   set bridge br1 other-config:hwaddr=aa:66:aa:66:00:00 -- \
+   set bridge br1 datapath-type=dummy other-config:datapath-id=1234 fail-mode=secure -- \
+   add-port br1 p2 -- set Interface p2 type=patch options:peer=p1 -- \
+   add-port br1 p3 -- set Interface p3 type=dummy ofport_request=3])
+
+AT_CHECK([ovs-ofctl del-flows br0])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 in_port=local,ip,actions=p1,p0])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br1 "in_port=1,ip,actions=resubmit(,3)"])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br1 "table=3,in_port=1,ip,actions=output(port=p3, max_len=20)"])
+
+AT_CHECK([ovs-appctl ofproto/trace br0 "in_port(local),ip" | grep "Datapath actions:"], [0], [dnl
+Datapath actions: clone(trunc(20),3),1
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto-dpif - patch ports - encap (clone)])
+
+OVS_VSWITCHD_START(
+  [add-port br0 p0 -- set Interface p0 type=dummy ofport_request=1 -- \
+   add-port br0 p1 -- set Interface p1 type=patch options:peer=p2 ofport_request=2 -- \
+   add-br br1 -- \
+   set bridge br1 other-config:hwaddr=aa:66:aa:66:00:00 -- \
+   set bridge br1 datapath-type=dummy other-config:datapath-id=1234 fail-mode=secure -- \
+   add-port br1 p2 -- set Interface p2 type=patch options:peer=p1 -- \
+   add-port br1 p3 -- set Interface p3 type=dummy ofport_request=3])
+
+AT_CHECK([ovs-ofctl del-flows br0])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 "in_port=local,ip,actions=encap(nsh()),encap(ethernet),p1,p0"])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br1 "in_port=1,dl_type=0x894f,actions=resubmit(,3)"])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br1 "in_port=1,dl_type=0x894f,actions=decap(),decap(),p3"])
+
+AT_CHECK([ovs-appctl ofproto/trace br0 "in_port(local),ip" | \
+          sed 's/recirc(0x[[0-9]]\+)/recirc(X)/' | grep "Datapath actions:"], [0], [dnl
+Datapath actions: push_nsh(flags=0,ttl=63,mdtype=1,np=3,spi=0x0,si=255,c1=0x0,c2=0x0,c3=0x0,c4=0x0),push_eth(src=00:00:00:00:00:00,dst=00:00:00:00:00:00),clone(pop_eth,pop_nsh(),recirc(X)),1
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP