[ovs-dev] [PATCH 2/2] xlate: normalize the actions after translation

Zoltan Balogh zoltan.balogh at ericsson.com
Mon Oct 30 11:22:50 UTC 2017


When all OF actions have been translated, there could be actions at
the end of list of odp actions which are not needed to be executed.
So, the list can be normalized at the end of xlate_actions().

Signed-off-by: Zoltan Balogh <zoltan.balogh at ericsson.com>
Signed-off-by: Sugesh Chandran <sugesh.chandran at intel.com>
Co-authored-by: Sugesh Chandran <sugesh.chandran at intel.com>
Tested-by: Sugesh Chandran <sugesh.chandran at intel.com>
---
 ofproto/ofproto-dpif-xlate.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
 tests/ofproto-dpif.at        | 48 +++++++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 0cc59e7..5f3e5f8 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -6938,6 +6938,62 @@ xlate_wc_finish(struct xlate_ctx *ctx)
     }
 }
 
+/* Returns true if the action stored in 'nla' can be a valid last action of a
+ * datapath flow. */
+static bool
+is_valid_last_action(struct nlattr *nla)
+{
+    switch (nl_attr_type(nla)) {
+    case OVS_ACTION_ATTR_USERSPACE:
+    case OVS_ACTION_ATTR_SAMPLE:
+    case OVS_ACTION_ATTR_TRUNC:
+    case OVS_ACTION_ATTR_RECIRC:
+    case OVS_ACTION_ATTR_TUNNEL_PUSH:
+    case OVS_ACTION_ATTR_TUNNEL_POP:
+    case OVS_ACTION_ATTR_OUTPUT:
+    case OVS_ACTION_ATTR_CLONE:
+    case OVS_ACTION_ATTR_CT:
+        return true;
+    default:
+        return false;
+    }
+}
+
+/* Returns offset of last netlink attribute storing valid action in array
+ * 'data'. Execution of actions beyond this last attribute does not make sense.
+ */
+static size_t
+last_action_offset(struct nlattr *data, const size_t data_len)
+{
+    uint16_t left;
+    struct nlattr *a, *b = NULL;
+
+    NL_ATTR_FOR_EACH (a, left, data, data_len) {
+        if (is_valid_last_action(a)) {
+            b = a;
+        }
+    }
+
+    if (b) {
+        return NLA_ALIGN(((char *)b - (char *)data) + b->nla_len);
+    } else {
+        return 0;
+    }
+}
+
+static void
+normalize_odp_actions(struct xlate_ctx *ctx)
+{
+    struct nlattr *data = ctx->odp_actions->data;
+    size_t size = ctx->odp_actions->size;
+    size_t new_size = last_action_offset(data, size);
+
+    /* Get rid of any unneeded actions at the tail end. */
+    if (OVS_UNLIKELY(new_size != size)) {
+        ctx->odp_actions->size = new_size;
+    }
+}
+
 /* Translates the flow, actions, or rule in 'xin' into datapath actions in
  * 'xout'.
  * The caller must take responsibility for eventually freeing 'xout', with
@@ -7343,7 +7399,10 @@ exit:
         if (xin->odp_actions) {
             ofpbuf_clear(xin->odp_actions);
         }
+    } else {
+        normalize_odp_actions(&ctx);
     }
+
     return ctx.error;
 }
 
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 1124f0e..d9f0432 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -10133,3 +10133,51 @@ AT_CHECK([grep "Datapath actions" stdout], [0],
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([ofproto-dpif - xlate error - normalize actions])
+
+#      ->-+
+#         | p1
+#       +-o-------+
+#       |   br0   |
+#       +-o-----o-+
+#   patch0|     |patch1
+#         +-->--+
+
+OVS_VSWITCHD_START([dnl
+    -- add-port br0 patch0 \
+    -- set interface patch0 type=patch options:peer=patch1 ofport_request=10 \
+    -- add-port br0 patch1 \
+    -- set interface patch1 type=patch options:peer=patch0 ofport_request=20
+])
+
+AT_CHECK([
+    ovs-vsctl add-port br0 p1 -- set interface p1 type=dummy ofport_request=1
+])
+
+AT_CHECK([
+    ovs-appctl dpif/show | grep -v hit | sed "s/$(printf \\t)/    /g" | sed 's./[[0-9]]\{1,\}..'
+], [0], [dnl
+    br0:
+        br0 65534: (dummy-internal)
+        p1 1: (dummy)
+        patch0 10/none: (patch: peer=patch1)
+        patch1 20/none: (patch: peer=patch0)
+])
+
+# Error due to pushing too many MPLS labels.
+AT_CHECK([
+    ovs-ofctl del-flows br0
+    ovs-ofctl add-flow br0 "table=0,in_port=1,actions=mod_dl_src:aa:aa:aa:bb:bb:00,goto_table:1" -OOpenFlow13
+    ovs-ofctl add-flow br0 "table=0,in_port=patch1,actions=goto_table:2" -OOpenFlow13
+    ovs-ofctl add-flow br0 "table=1,actions=push_vlan:0x8100,set_field:4196->vlan_vid,output:patch0" -OOpenFlow13
+    ovs-ofctl add-flow br0 "table=2,actions=push_mpls:0x8847,output:patch0"
+], [0])
+
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,ip'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: drop
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
-- 
1.9.1



More information about the dev mailing list