ofp-util: Centralize decoding of OpenFlow actions.

This significantly simplifies code in ofp-print and ofproto-dpif and is
likely to simplify any new ofproto implementations whose support for
actions differs from ofproto-dpif.

4 files changed, 398 insertions, 502 deletions

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 23ca528c..4f4e33c5 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -203,262 +203,146 @@ print_note(struct ds *string, const struct nx_action_note *nan)
     }
 }
 
-static int
-nx_action_len(enum nx_action_subtype subtype)
-{
-    switch (subtype) {
-    case NXAST_SNAT__OBSOLETE: return -1;
-    case NXAST_RESUBMIT: return sizeof(struct nx_action_resubmit);
-    case NXAST_SET_TUNNEL: return sizeof(struct nx_action_set_tunnel);
-    case NXAST_DROP_SPOOFED_ARP__OBSOLETE: return -1;
-    case NXAST_SET_QUEUE: return sizeof(struct nx_action_set_queue);
-    case NXAST_POP_QUEUE: return sizeof(struct nx_action_pop_queue);
-    case NXAST_REG_MOVE: return sizeof(struct nx_action_reg_move);
-    case NXAST_REG_LOAD: return sizeof(struct nx_action_reg_load);
-    case NXAST_NOTE: return -1;
-    case NXAST_SET_TUNNEL64: return sizeof(struct nx_action_set_tunnel64);
-    case NXAST_MULTIPATH: return sizeof(struct nx_action_multipath);
-    case NXAST_AUTOPATH: return sizeof (struct nx_action_autopath);
-    default: return -1;
-    }
-}
-
-static void
-ofp_print_nx_action(struct ds *string, const struct nx_action_header *nah)
-{
-    int subtype = ntohs(nah->subtype);
-    int required_len = nx_action_len(subtype);
-    int len = ntohs(nah->len);
-
-    if (required_len != -1 && required_len != len) {
-        ds_put_format(string, "***Nicira action %"PRIu16" wrong length: %d***",
-                      subtype, len);
-        return;
-    }
-
-    if (subtype <= TYPE_MAXIMUM(enum nx_action_subtype)) {
-        const struct nx_action_set_tunnel64 *nast64;
-        const struct nx_action_set_tunnel *nast;
-        const struct nx_action_set_queue *nasq;
-        const struct nx_action_resubmit *nar;
-        const struct nx_action_reg_move *move;
-        const struct nx_action_reg_load *load;
-        const struct nx_action_multipath *nam;
-        const struct nx_action_autopath *naa;
-
-        switch ((enum nx_action_subtype) subtype) {
-        case NXAST_RESUBMIT:
-            nar = (struct nx_action_resubmit *)nah;
-            ds_put_format(string, "resubmit:");
-            ofp_print_port_name(string, ntohs(nar->in_port));
-            return;
-
-        case NXAST_SET_TUNNEL:
-            nast = (struct nx_action_set_tunnel *)nah;
-            ds_put_format(string, "set_tunnel:%#"PRIx32, ntohl(nast->tun_id));
-            return;
-
-        case NXAST_SET_QUEUE:
-            nasq = (struct nx_action_set_queue *)nah;
-            ds_put_format(string, "set_queue:%u", ntohl(nasq->queue_id));
-            return;
-
-        case NXAST_POP_QUEUE:
-            ds_put_cstr(string, "pop_queue");
-            return;
-
-        case NXAST_NOTE:
-            print_note(string, (const struct nx_action_note *) nah);
-            return;
-
-        case NXAST_REG_MOVE:
-            move = (const struct nx_action_reg_move *) nah;
-            nxm_format_reg_move(move, string);
-            return;
-
-        case NXAST_REG_LOAD:
-            load = (const struct nx_action_reg_load *) nah;
-            nxm_format_reg_load(load, string);
-            return;
-
-        case NXAST_SET_TUNNEL64:
-            nast64 = (const struct nx_action_set_tunnel64 *) nah;
-            ds_put_format(string, "set_tunnel64:%#"PRIx64,
-                          ntohll(nast64->tun_id));
-            return;
-
-        case NXAST_MULTIPATH:
-            nam = (const struct nx_action_multipath *) nah;
-            multipath_format(nam, string);
-            return;
-
-        case NXAST_AUTOPATH:
-            naa = (const struct nx_action_autopath *)nah;
-            ds_put_format(string, "autopath(%u,", ntohl(naa->id));
-            nxm_format_field_bits(string, ntohl(naa->dst),
-                                  nxm_decode_ofs(naa->ofs_nbits),
-                                  nxm_decode_n_bits(naa->ofs_nbits));
-            ds_put_char(string, ')');
-            return;
-
-        case NXAST_SNAT__OBSOLETE:
-        case NXAST_DROP_SPOOFED_ARP__OBSOLETE:
-        default:
-            break;
-        }
-    }
-
-    ds_put_format(string, "***unknown Nicira action:%d***", subtype);
-}
-
-static int
-ofp_action_len(enum ofp_action_type type)
-{
-    switch (type) {
-    case OFPAT_OUTPUT: return sizeof(struct ofp_action_output);
-    case OFPAT_SET_VLAN_VID: return sizeof(struct ofp_action_vlan_vid);
-    case OFPAT_SET_VLAN_PCP: return sizeof(struct ofp_action_vlan_pcp);
-    case OFPAT_STRIP_VLAN: return sizeof(struct ofp_action_header);
-    case OFPAT_SET_DL_SRC: return sizeof(struct ofp_action_dl_addr);
-    case OFPAT_SET_DL_DST: return sizeof(struct ofp_action_dl_addr);
-    case OFPAT_SET_NW_SRC: return sizeof(struct ofp_action_nw_addr);
-    case OFPAT_SET_NW_DST: return sizeof(struct ofp_action_nw_addr);
-    case OFPAT_SET_NW_TOS: return sizeof(struct ofp_action_nw_tos);
-    case OFPAT_SET_TP_SRC: return sizeof(struct ofp_action_tp_port);
-    case OFPAT_SET_TP_DST: return sizeof(struct ofp_action_tp_port);
-    case OFPAT_ENQUEUE: return sizeof(struct ofp_action_enqueue);
-    case OFPAT_VENDOR: return -1;
-    default: return -1;
-    }
-}
-
 static void
-ofp_print_action(struct ds *string, const struct ofp_action_header *ah)
+ofp_print_action(struct ds *s, const union ofp_action *a,
+                 enum ofputil_action_code code)
 {
-    enum ofp_action_type type;
-    int required_len;
-    size_t len;
+    const struct ofp_action_enqueue *oae;
+    const struct ofp_action_dl_addr *oada;
+    const struct nx_action_set_tunnel64 *nast64;
+    const struct nx_action_set_tunnel *nast;
+    const struct nx_action_set_queue *nasq;
+    const struct nx_action_resubmit *nar;
+    const struct nx_action_reg_move *move;
+    const struct nx_action_reg_load *load;
+    const struct nx_action_multipath *nam;
+    const struct nx_action_autopath *naa;
+    uint16_t port;
 
-    type = ntohs(ah->type);
-    len = ntohs(ah->len);
-
-    required_len = ofp_action_len(type);
-    if (required_len >= 0 && len != required_len) {
-        ds_put_format(string,
-                      "***action %d wrong length: %zu***\n", (int) type, len);
-        return;
-    }
-
-    switch (type) {
-    case OFPAT_OUTPUT: {
-        struct ofp_action_output *oa = (struct ofp_action_output *)ah;
-        uint16_t port = ntohs(oa->port);
+    switch (code) {
+    case OFPUTIL_OFPAT_OUTPUT:
+        port = ntohs(a->output.port);
         if (port < OFPP_MAX) {
-            ds_put_format(string, "output:%"PRIu16, port);
+            ds_put_format(s, "output:%"PRIu16, port);
         } else {
-            ofp_print_port_name(string, port);
+            ofp_print_port_name(s, port);
             if (port == OFPP_CONTROLLER) {
-                if (oa->max_len) {
-                    ds_put_format(string, ":%"PRIu16, ntohs(oa->max_len));
+                if (a->output.max_len != htons(0)) {
+                    ds_put_format(s, ":%"PRIu16, ntohs(a->output.max_len));
                 } else {
-                    ds_put_cstr(string, ":all");
+                    ds_put_cstr(s, ":all");
                 }
             }
         }
         break;
-    }
 
-    case OFPAT_ENQUEUE: {
-        struct ofp_action_enqueue *ea = (struct ofp_action_enqueue *)ah;
-        unsigned int port = ntohs(ea->port);
-        unsigned int queue_id = ntohl(ea->queue_id);
-        ds_put_format(string, "enqueue:");
-        if (port != OFPP_IN_PORT) {
-            ds_put_format(string, "%u", port);
-        } else {
-            ds_put_cstr(string, "IN_PORT");
-        }
-        ds_put_format(string, "q%u", queue_id);
+    case OFPUTIL_OFPAT_ENQUEUE:
+        oae = (const struct ofp_action_enqueue *) a;
+        ds_put_format(s, "enqueue:");
+        ofp_print_port_name(s, ntohs(oae->port));
+        ds_put_format(s, "q%"PRIu32, ntohl(oae->queue_id));
         break;
-    }
 
-    case OFPAT_SET_VLAN_VID: {
-        struct ofp_action_vlan_vid *va = (struct ofp_action_vlan_vid *)ah;
-        ds_put_format(string, "mod_vlan_vid:%"PRIu16, ntohs(va->vlan_vid));
+    case OFPUTIL_OFPAT_SET_VLAN_VID:
+        ds_put_format(s, "mod_vlan_vid:%"PRIu16,
+                      ntohs(a->vlan_vid.vlan_vid));
         break;
-    }
 
-    case OFPAT_SET_VLAN_PCP: {
-        struct ofp_action_vlan_pcp *va = (struct ofp_action_vlan_pcp *)ah;
-        ds_put_format(string, "mod_vlan_pcp:%"PRIu8, va->vlan_pcp);
+    case OFPUTIL_OFPAT_SET_VLAN_PCP:
+        ds_put_format(s, "mod_vlan_pcp:%"PRIu8, a->vlan_pcp.vlan_pcp);
         break;
-    }
 
-    case OFPAT_STRIP_VLAN:
-        ds_put_cstr(string, "strip_vlan");
+    case OFPUTIL_OFPAT_STRIP_VLAN:
+        ds_put_cstr(s, "strip_vlan");
         break;
 
-    case OFPAT_SET_DL_SRC: {
-        struct ofp_action_dl_addr *da = (struct ofp_action_dl_addr *)ah;
-        ds_put_format(string, "mod_dl_src:"ETH_ADDR_FMT,
-                ETH_ADDR_ARGS(da->dl_addr));
+    case OFPUTIL_OFPAT_SET_DL_SRC:
+        oada = (const struct ofp_action_dl_addr *) a;
+        ds_put_format(s, "mod_dl_src:"ETH_ADDR_FMT,
+                      ETH_ADDR_ARGS(oada->dl_addr));
         break;
-    }
 
-    case OFPAT_SET_DL_DST: {
-        struct ofp_action_dl_addr *da = (struct ofp_action_dl_addr *)ah;
-        ds_put_format(string, "mod_dl_dst:"ETH_ADDR_FMT,
-                ETH_ADDR_ARGS(da->dl_addr));
+    case OFPUTIL_OFPAT_SET_DL_DST:
+        oada = (const struct ofp_action_dl_addr *) a;
+        ds_put_format(s, "mod_dl_dst:"ETH_ADDR_FMT,
+                      ETH_ADDR_ARGS(oada->dl_addr));
         break;
-    }
 
-    case OFPAT_SET_NW_SRC: {
-        struct ofp_action_nw_addr *na = (struct ofp_action_nw_addr *)ah;
-        ds_put_format(string, "mod_nw_src:"IP_FMT, IP_ARGS(&na->nw_addr));
+    case OFPUTIL_OFPAT_SET_NW_SRC:
+        ds_put_format(s, "mod_nw_src:"IP_FMT, IP_ARGS(&a->nw_addr.nw_addr));
         break;
-    }
 
-    case OFPAT_SET_NW_DST: {
-        struct ofp_action_nw_addr *na = (struct ofp_action_nw_addr *)ah;
-        ds_put_format(string, "mod_nw_dst:"IP_FMT, IP_ARGS(&na->nw_addr));
+    case OFPUTIL_OFPAT_SET_NW_DST:
+        ds_put_format(s, "mod_nw_dst:"IP_FMT, IP_ARGS(&a->nw_addr.nw_addr));
         break;
-    }
 
-    case OFPAT_SET_NW_TOS: {
-        struct ofp_action_nw_tos *nt = (struct ofp_action_nw_tos *)ah;
-        ds_put_format(string, "mod_nw_tos:%d", nt->nw_tos);
+    case OFPUTIL_OFPAT_SET_NW_TOS:
+        ds_put_format(s, "mod_nw_tos:%d", a->nw_tos.nw_tos);
         break;
-    }
 
-    case OFPAT_SET_TP_SRC: {
-        struct ofp_action_tp_port *ta = (struct ofp_action_tp_port *)ah;
-        ds_put_format(string, "mod_tp_src:%d", ntohs(ta->tp_port));
+    case OFPUTIL_OFPAT_SET_TP_SRC:
+        ds_put_format(s, "mod_tp_src:%d", ntohs(a->tp_port.tp_port));
         break;
-    }
 
-    case OFPAT_SET_TP_DST: {
-        struct ofp_action_tp_port *ta = (struct ofp_action_tp_port *)ah;
-        ds_put_format(string, "mod_tp_dst:%d", ntohs(ta->tp_port));
+    case OFPUTIL_OFPAT_SET_TP_DST:
+        ds_put_format(s, "mod_tp_dst:%d", ntohs(a->tp_port.tp_port));
         break;
-    }
 
-    case OFPAT_VENDOR: {
-        struct ofp_action_vendor_header *avh
-                = (struct ofp_action_vendor_header *)ah;
-        if (len < sizeof *avh) {
-            ds_put_format(string, "***ofpat_vendor truncated***\n");
-            return;
-        }
-        if (avh->vendor == htonl(NX_VENDOR_ID)) {
-            ofp_print_nx_action(string, (struct nx_action_header *)avh);
-        } else {
-            ds_put_format(string, "vendor action:0x%x", ntohl(avh->vendor));
-        }
+    case OFPUTIL_NXAST_RESUBMIT:
+        nar = (struct nx_action_resubmit *)a;
+        ds_put_format(s, "resubmit:");
+        ofp_print_port_name(s, ntohs(nar->in_port));
+        break;
+
+    case OFPUTIL_NXAST_SET_TUNNEL:
+        nast = (struct nx_action_set_tunnel *)a;
+        ds_put_format(s, "set_tunnel:%#"PRIx32, ntohl(nast->tun_id));
+        break;
+
+    case OFPUTIL_NXAST_SET_QUEUE:
+        nasq = (struct nx_action_set_queue *)a;
+        ds_put_format(s, "set_queue:%u", ntohl(nasq->queue_id));
+        break;
+
+    case OFPUTIL_NXAST_POP_QUEUE:
+        ds_put_cstr(s, "pop_queue");
+        break;
+
+    case OFPUTIL_NXAST_NOTE:
+        print_note(s, (const struct nx_action_note *) a);
+        break;
+
+    case OFPUTIL_NXAST_REG_MOVE:
+        move = (const struct nx_action_reg_move *) a;
+        nxm_format_reg_move(move, s);
+        break;
+
+    case OFPUTIL_NXAST_REG_LOAD:
+        load = (const struct nx_action_reg_load *) a;
+        nxm_format_reg_load(load, s);
+        break;
+
+    case OFPUTIL_NXAST_SET_TUNNEL64:
+        nast64 = (const struct nx_action_set_tunnel64 *) a;
+        ds_put_format(s, "set_tunnel64:%#"PRIx64,
+                      ntohll(nast64->tun_id));
+        break;
+
+    case OFPUTIL_NXAST_MULTIPATH:
+        nam = (const struct nx_action_multipath *) a;
+        multipath_format(nam, s);
+        break;
+
+    case OFPUTIL_NXAST_AUTOPATH:
+        naa = (const struct nx_action_autopath *)a;
+        ds_put_format(s, "autopath(%u,", ntohl(naa->id));
+        nxm_format_field_bits(s, ntohl(naa->dst),
+                              nxm_decode_ofs(naa->ofs_nbits),
+                              nxm_decode_n_bits(naa->ofs_nbits));
+        ds_put_char(s, ')');
         break;
-    }
 
     default:
-        ds_put_format(string, "(decoder %d not implemented)", (int) type);
         break;
     }
 }
@@ -474,11 +358,17 @@ ofp_print_actions(struct ds *string, const union ofp_action *actions,
     if (!n_actions) {
         ds_put_cstr(string, "drop");
     }
+
     OFPUTIL_ACTION_FOR_EACH (a, left, actions, n_actions) {
-        if (a != actions) {
-            ds_put_cstr(string, ",");
+        int code = ofputil_decode_action(a);
+        if (code >= 0) {
+            if (a != actions) {
+                ds_put_cstr(string, ",");
+            }
+            ofp_print_action(string, a, code);
+        } else {
+            ofp_print_error(string, -code);
         }
-        ofp_print_action(string, (struct ofp_action_header *)a);
     }
     if (left > 0) {
         ds_put_format(string, " ***%zu leftover bytes following actions",
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 56b1e412..00f9ce8b 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1941,33 +1941,6 @@ make_echo_reply(const struct ofp_header *rq)
     return out;
 }
 
-static int
-check_action_exact_len(const union ofp_action *a, unsigned int len,
-                       unsigned int required_len)
-{
-    if (len != required_len) {
-        VLOG_WARN_RL(&bad_ofmsg_rl, "action %"PRIu16" has invalid length "
-                     "%"PRIu16" (must be %u)\n",
-                     ntohs(a->type), ntohs(a->header.len), required_len);
-        return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN);
-    }
-    return 0;
-}
-
-static int
-check_nx_action_exact_len(const struct nx_action_header *a,
-                          unsigned int len, unsigned int required_len)
-{
-    if (len != required_len) {
-        VLOG_WARN_RL(&bad_ofmsg_rl,
-                     "Nicira action %"PRIu16" has invalid length %"PRIu16" "
-                     "(must be %u)\n",
-                     ntohs(a->subtype), ntohs(a->len), required_len);
-        return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN);
-    }
-    return 0;
-}
-
 /* Checks that 'port' is a valid output port for the OFPAT_OUTPUT action, given
  * that the switch will never have more than 'max_ports' ports.  Returns 0 if
  * 'port' is valid, otherwise an ofp_mkerr() return code. */
@@ -1988,208 +1961,232 @@ check_output_port(uint16_t port, int max_ports)
         if (port < max_ports) {
             return 0;
         }
-        VLOG_WARN_RL(&bad_ofmsg_rl, "unknown output port %x", port);
         return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_OUT_PORT);
     }
 }
 
-/* Checks that 'action' is a valid OFPAT_ENQUEUE action, given that the switch
- * will never have more than 'max_ports' ports.  Returns 0 if 'port' is valid,
- * otherwise an ofp_mkerr() return code. */
-static int
-check_enqueue_action(const union ofp_action *a, unsigned int len,
-                     int max_ports)
+int
+validate_actions(const union ofp_action *actions, size_t n_actions,
+                 const struct flow *flow, int max_ports)
 {
-    const struct ofp_action_enqueue *oae;
-    uint16_t port;
-    int error;
+    const union ofp_action *a;
+    size_t left;
 
-    error = check_action_exact_len(a, len, 16);
-    if (error) {
-        return error;
-    }
+    OFPUTIL_ACTION_FOR_EACH (a, left, actions, n_actions) {
+        uint16_t port;
+        int error;
+        int code;
 
-    oae = (const struct ofp_action_enqueue *) a;
-    port = ntohs(oae->port);
-    if (port < max_ports || port == OFPP_IN_PORT) {
-        return 0;
-    }
-    VLOG_WARN_RL(&bad_ofmsg_rl, "unknown enqueue port %x", port);
-    return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_OUT_PORT);
-}
+        code = ofputil_decode_action(a);
+        if (code < 0) {
+            char *msg;
 
-static int
-check_nicira_action(const union ofp_action *a, unsigned int len,
-                    const struct flow *flow)
-{
-    const struct nx_action_header *nah;
-    int subtype;
-    int error;
+            error = -code;
+            msg = ofputil_error_to_string(error);
+            VLOG_WARN_RL(&bad_ofmsg_rl,
+                         "action decoding error at offset %td (%s)",
+                         (a - actions) * sizeof *a, msg);
+            free(msg);
 
-    if (len < 16) {
-        VLOG_WARN_RL(&bad_ofmsg_rl,
-                     "Nicira vendor action only %u bytes", len);
-        return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN);
-    }
-    nah = (const struct nx_action_header *) a;
+            return error;
+        }
 
-    subtype = ntohs(nah->subtype);
-    if (subtype > TYPE_MAXIMUM(enum nx_action_subtype)) {
-        /* This is necessary because enum nx_action_subtype may be an
-         * 8-bit type, so the cast below throws away the top 8 bits. */
-        return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_VENDOR_TYPE);
-    }
+        error = 0;
+        switch ((enum ofputil_action_code) code) {
+        case OFPUTIL_OFPAT_OUTPUT:
+            error = check_output_port(ntohs(a->output.port), max_ports);
+            break;
 
-    switch ((enum nx_action_subtype) subtype) {
-    case NXAST_RESUBMIT:
-    case NXAST_SET_TUNNEL:
-    case NXAST_SET_QUEUE:
-    case NXAST_POP_QUEUE:
-        return check_nx_action_exact_len(nah, len, 16);
+        case OFPUTIL_OFPAT_SET_VLAN_VID:
+            if (a->vlan_vid.vlan_vid & ~htons(0xfff)) {
+                error = ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_ARGUMENT);
+            }
+            break;
 
-    case NXAST_REG_MOVE:
-        error = check_nx_action_exact_len(nah, len,
-                                          sizeof(struct nx_action_reg_move));
-        if (error) {
-            return error;
-        }
-        return nxm_check_reg_move((const struct nx_action_reg_move *) a, flow);
+        case OFPUTIL_OFPAT_SET_VLAN_PCP:
+            if (a->vlan_pcp.vlan_pcp & ~7) {
+                error = ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_ARGUMENT);
+            }
+            break;
 
-    case NXAST_REG_LOAD:
-        error = check_nx_action_exact_len(nah, len,
-                                          sizeof(struct nx_action_reg_load));
-        if (error) {
-            return error;
-        }
-        return nxm_check_reg_load((const struct nx_action_reg_load *) a, flow);
+        case OFPUTIL_OFPAT_ENQUEUE:
+            port = ntohs(((const struct ofp_action_enqueue *) a)->port);
+            if (port >= max_ports && port != OFPP_IN_PORT) {
+                error = ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_OUT_PORT);
+            }
+            break;
 
-    case NXAST_NOTE:
-        return 0;
+        case OFPUTIL_NXAST_REG_MOVE:
+            error = nxm_check_reg_move((const struct nx_action_reg_move *) a,
+                                       flow);
+            break;
 
-    case NXAST_SET_TUNNEL64:
-        return check_nx_action_exact_len(
-            nah, len, sizeof(struct nx_action_set_tunnel64));
+        case OFPUTIL_NXAST_REG_LOAD:
+            error = nxm_check_reg_load((const struct nx_action_reg_load *) a,
+                                       flow);
+            break;
 
-    case NXAST_MULTIPATH:
-        error = check_nx_action_exact_len(
-            nah, len, sizeof(struct nx_action_multipath));
-        if (error) {
-            return error;
+        case OFPUTIL_NXAST_MULTIPATH:
+            error = multipath_check((const struct nx_action_multipath *) a);
+            break;
+
+        case OFPUTIL_NXAST_AUTOPATH:
+            error = autopath_check((const struct nx_action_autopath *) a);
+            break;
+
+        case OFPUTIL_OFPAT_STRIP_VLAN:
+        case OFPUTIL_OFPAT_SET_NW_SRC:
+        case OFPUTIL_OFPAT_SET_NW_DST:
+        case OFPUTIL_OFPAT_SET_NW_TOS:
+        case OFPUTIL_OFPAT_SET_TP_SRC:
+        case OFPUTIL_OFPAT_SET_TP_DST:
+        case OFPUTIL_OFPAT_SET_DL_SRC:
+        case OFPUTIL_OFPAT_SET_DL_DST:
+        case OFPUTIL_NXAST_RESUBMIT:
+        case OFPUTIL_NXAST_SET_TUNNEL:
+        case OFPUTIL_NXAST_SET_QUEUE:
+        case OFPUTIL_NXAST_POP_QUEUE:
+        case OFPUTIL_NXAST_NOTE:
+        case OFPUTIL_NXAST_SET_TUNNEL64:
+            break;
         }
-        return multipath_check((const struct nx_action_multipath *) a);
 
-    case NXAST_AUTOPATH:
-        error = check_nx_action_exact_len(
-            nah, len, sizeof(struct nx_action_autopath));
         if (error) {
+            char *msg = ofputil_error_to_string(error);
+            VLOG_WARN_RL(&bad_ofmsg_rl, "bad action at offset %td (%s)",
+                         (a - actions) * sizeof *a, msg);
+            free(msg);
             return error;
         }
-        return autopath_check((const struct nx_action_autopath *) a);
-
-    case NXAST_SNAT__OBSOLETE:
-    case NXAST_DROP_SPOOFED_ARP__OBSOLETE:
-    default:
-        VLOG_WARN_RL(&bad_ofmsg_rl,
-                     "unknown Nicira vendor action subtype %d", subtype);
-        return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_VENDOR_TYPE);
     }
+    if (left) {
+        VLOG_WARN_RL(&bad_ofmsg_rl, "bad action format at offset %zu",
+                     (n_actions - left) * sizeof *a);
+        return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN);
+    }
+    return 0;
 }
 
-static int
-check_action(const union ofp_action *a, unsigned int len,
-             const struct flow *flow, int max_ports)
-{
-    enum ofp_action_type type = ntohs(a->type);
-    int error;
+struct ofputil_ofpat_action {
+    enum ofputil_action_code code;
+    unsigned int len;
+};
 
-    switch (type) {
-    case OFPAT_OUTPUT:
-        error = check_action_exact_len(a, len, 8);
-        if (error) {
-            return error;
-        }
-        return check_output_port(ntohs(a->output.port), max_ports);
+static const struct ofputil_ofpat_action ofpat_actions[] = {
+    { OFPUTIL_OFPAT_OUTPUT,        8 },
+    { OFPUTIL_OFPAT_SET_VLAN_VID,  8 },
+    { OFPUTIL_OFPAT_SET_VLAN_PCP,  8 },
+    { OFPUTIL_OFPAT_STRIP_VLAN,    8 },
+    { OFPUTIL_OFPAT_SET_DL_SRC,   16 },
+    { OFPUTIL_OFPAT_SET_DL_DST,   16 },
+    { OFPUTIL_OFPAT_SET_NW_SRC,    8 },
+    { OFPUTIL_OFPAT_SET_NW_DST,    8 },
+    { OFPUTIL_OFPAT_SET_NW_TOS,    8 },
+    { OFPUTIL_OFPAT_SET_TP_SRC,    8 },
+    { OFPUTIL_OFPAT_SET_TP_DST,    8 },
+    { OFPUTIL_OFPAT_ENQUEUE,      16 },
+};
 
-    case OFPAT_SET_VLAN_VID:
-        error = check_action_exact_len(a, len, 8);
-        if (error) {
-            return error;
-        }
-        if (a->vlan_vid.vlan_vid & ~htons(0xfff)) {
-            return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_ARGUMENT);
-        }
-        return 0;
+struct ofputil_nxast_action {
+    enum ofputil_action_code code;
+    unsigned int min_len;
+    unsigned int max_len;
+};
 
-    case OFPAT_SET_VLAN_PCP:
-        error = check_action_exact_len(a, len, 8);
-        if (error) {
-            return error;
-        }
-        if (a->vlan_pcp.vlan_pcp & ~7) {
-            return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_ARGUMENT);
-        }
-        return 0;
+static const struct ofputil_nxast_action nxast_actions[] = {
+    { 0, UINT_MAX, UINT_MAX }, /* NXAST_SNAT__OBSOLETE */
+    { OFPUTIL_NXAST_RESUBMIT,     16, 16 },
+    { OFPUTIL_NXAST_SET_TUNNEL,   16, 16 },
+    { 0, UINT_MAX, UINT_MAX }, /* NXAST_DROP_SPOOFED_ARP__OBSOLETE */
+    { OFPUTIL_NXAST_SET_QUEUE,    16, 16 },
+    { OFPUTIL_NXAST_POP_QUEUE,    16, 16 },
+    { OFPUTIL_NXAST_REG_MOVE,     24, 24 },
+    { OFPUTIL_NXAST_REG_LOAD,     24, 24 },
+    { OFPUTIL_NXAST_NOTE,         16, UINT_MAX },
+    { OFPUTIL_NXAST_SET_TUNNEL64, 24, 24 },
+    { OFPUTIL_NXAST_MULTIPATH,    32, 32 },
+    { OFPUTIL_NXAST_AUTOPATH,     24, 24 },
+};
 
-    case OFPAT_STRIP_VLAN:
-    case OFPAT_SET_NW_SRC:
-    case OFPAT_SET_NW_DST:
-    case OFPAT_SET_NW_TOS:
-    case OFPAT_SET_TP_SRC:
-    case OFPAT_SET_TP_DST:
-        return check_action_exact_len(a, len, 8);
+static int
+ofputil_decode_ofpat_action(const union ofp_action *a)
+{
+    int type = ntohs(a->type);
 
-    case OFPAT_SET_DL_SRC:
-    case OFPAT_SET_DL_DST:
-        return check_action_exact_len(a, len, 16);
+    if (type < ARRAY_SIZE(ofpat_actions)) {
+        const struct ofputil_ofpat_action *ooa = &ofpat_actions[type];
+        unsigned int len = ntohs(a->header.len);
 
-    case OFPAT_VENDOR:
-        return (a->vendor.vendor == htonl(NX_VENDOR_ID)
-                ? check_nicira_action(a, len, flow)
-                : ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_VENDOR));
+        return (len == ooa->len
+                ? ooa->code
+                : -ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN));
+    } else {
+        return -ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_TYPE);
+    }
+}
 
-    case OFPAT_ENQUEUE:
-        return check_enqueue_action(a, len, max_ports);
+static int
+ofputil_decode_nxast_action(const union ofp_action *a)
+{
+    unsigned int len = ntohs(a->header.len);
 
-    default:
-        VLOG_WARN_RL(&bad_ofmsg_rl, "unknown action type %d", (int) type);
-        return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_TYPE);
+    if (len < sizeof(struct nx_action_header)) {
+        return -ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN);
+    } else {
+        const struct nx_action_header *nah = (const void *) a;
+        int subtype = ntohs(nah->subtype);
+
+        if (subtype <= ARRAY_SIZE(nxast_actions)) {
+            const struct ofputil_nxast_action *ona = &nxast_actions[subtype];
+            if (len >= ona->min_len && len <= ona->max_len) {
+                return ona->code;
+            } else if (ona->min_len == UINT_MAX) {
+                return -ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_TYPE);
+            } else {
+                return -ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN);
+            }
+        } else {
+            return -ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_TYPE);
+        }
     }
 }
 
+/* Parses 'a' to determine its type.  Returns a nonnegative OFPUTIL_OFPAT_* or
+ * OFPUTIL_NXAST_* constant if successful, otherwise a negative OpenFlow error
+ * code (as returned by ofp_mkerr()).
+ *
+ * The caller must have already verified that 'a''s length is correct (that is,
+ * a->header.len is nonzero and a multiple of sizeof(union ofp_action) and no
+ * longer than the amount of space allocated to 'a').
+ *
+ * This function verifies that 'a''s length is correct for the type of action
+ * that it represents. */
 int
-validate_actions(const union ofp_action *actions, size_t n_actions,
-                 const struct flow *flow, int max_ports)
+ofputil_decode_action(const union ofp_action *a)
 {
-    size_t i;
-
-    for (i = 0; i < n_actions; ) {
-        const union ofp_action *a = &actions[i];
-        unsigned int len = ntohs(a->header.len);
-        unsigned int n_slots = len / OFP_ACTION_ALIGN;
-        unsigned int slots_left = &actions[n_actions] - a;
-        int error;
+    if (a->type != htons(OFPAT_VENDOR)) {
+        return ofputil_decode_ofpat_action(a);
+    } else if (a->vendor.vendor == htonl(NX_VENDOR_ID)) {
+        return ofputil_decode_nxast_action(a);
+    } else {
+        return -ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_VENDOR);
+    }
+}
 
-        if (n_slots > slots_left) {
-            VLOG_WARN_RL(&bad_ofmsg_rl,
-                         "action requires %u slots but only %u remain",
-                         n_slots, slots_left);
-            return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN);
-        } else if (!len) {
-            VLOG_WARN_RL(&bad_ofmsg_rl, "action has invalid length 0");
-            return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN);
-        } else if (len % OFP_ACTION_ALIGN) {
-            VLOG_WARN_RL(&bad_ofmsg_rl, "action length %u is not a multiple "
-                         "of %d", len, OFP_ACTION_ALIGN);
-            return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN);
-        }
+/* Parses 'a' and returns its type as an OFPUTIL_OFPAT_* or OFPUTIL_NXAST_*
+ * constant.  The caller must have already validated that 'a' is a valid action
+ * understood by Open vSwitch (e.g. by a previous successful call to
+ * ofputil_decode_action()). */
+enum ofputil_action_code
+ofputil_decode_action_unsafe(const union ofp_action *a)
+{
+    if (a->type != htons(OFPAT_VENDOR)) {
+        return ofpat_actions[ntohs(a->type)].code;
+    } else {
+        const struct nx_action_header *nah = (const void *) a;
 
-        error = check_action(a, len, flow, max_ports);
-        if (error) {
-            return error;
-        }
-        i += n_slots;
+        return nxast_actions[ntohs(nah->subtype)].code;
     }
-    return 0;
 }
 
 /* Returns true if 'action' outputs to 'port', false otherwise. */
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 8fb5b1bc..48b0a4cf 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -275,6 +275,38 @@ struct ofpbuf *make_echo_reply(const struct ofp_header *rq);
 
 /* Actions. */
 
+enum ofputil_action_code {
+    /* OFPAT_* actions. */
+    OFPUTIL_OFPAT_OUTPUT,
+    OFPUTIL_OFPAT_SET_VLAN_VID,
+    OFPUTIL_OFPAT_SET_VLAN_PCP,
+    OFPUTIL_OFPAT_STRIP_VLAN,
+    OFPUTIL_OFPAT_SET_DL_SRC,
+    OFPUTIL_OFPAT_SET_DL_DST,
+    OFPUTIL_OFPAT_SET_NW_SRC,
+    OFPUTIL_OFPAT_SET_NW_DST,
+    OFPUTIL_OFPAT_SET_NW_TOS,
+    OFPUTIL_OFPAT_SET_TP_SRC,
+    OFPUTIL_OFPAT_SET_TP_DST,
+    OFPUTIL_OFPAT_ENQUEUE,
+
+    /* NXAST_* actions. */
+    OFPUTIL_NXAST_RESUBMIT,
+    OFPUTIL_NXAST_SET_TUNNEL,
+    OFPUTIL_NXAST_SET_QUEUE,
+    OFPUTIL_NXAST_POP_QUEUE,
+    OFPUTIL_NXAST_REG_MOVE,
+    OFPUTIL_NXAST_REG_LOAD,
+    OFPUTIL_NXAST_NOTE,
+    OFPUTIL_NXAST_SET_TUNNEL64,
+    OFPUTIL_NXAST_MULTIPATH,
+    OFPUTIL_NXAST_AUTOPATH
+};
+
+int ofputil_decode_action(const union ofp_action *);
+enum ofputil_action_code ofputil_decode_action_unsafe(
+    const union ofp_action *);
+
 #define OFP_ACTION_ALIGN 8      /* Alignment of ofp_actions. */
 
 static inline union ofp_action *
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 5ce05d6e..1e77cda2 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3040,77 +3040,6 @@ xlate_autopath(struct action_xlate_ctx *ctx,
 }
 
 static void
-xlate_nicira_action(struct action_xlate_ctx *ctx,
-                    const struct nx_action_header *nah)
-{
-    const struct nx_action_resubmit *nar;
-    const struct nx_action_set_tunnel *nast;
-    const struct nx_action_set_queue *nasq;
-    const struct nx_action_multipath *nam;
-    const struct nx_action_autopath *naa;
-    enum nx_action_subtype subtype = ntohs(nah->subtype);
-    ovs_be64 tun_id;
-
-    assert(nah->vendor == htonl(NX_VENDOR_ID));
-    switch (subtype) {
-    case NXAST_RESUBMIT:
-        nar = (const struct nx_action_resubmit *) nah;
-        xlate_table_action(ctx, ntohs(nar->in_port));
-        break;
-
-    case NXAST_SET_TUNNEL:
-        nast = (const struct nx_action_set_tunnel *) nah;
-        tun_id = htonll(ntohl(nast->tun_id));
-        ctx->flow.tun_id = tun_id;
-        break;
-
-    case NXAST_SET_QUEUE:
-        nasq = (const struct nx_action_set_queue *) nah;
-        xlate_set_queue_action(ctx, nasq);
-        break;
-
-    case NXAST_POP_QUEUE:
-        ctx->priority = 0;
-        break;
-
-    case NXAST_REG_MOVE:
-        nxm_execute_reg_move((const struct nx_action_reg_move *) nah,
-                             &ctx->flow);
-        break;
-
-    case NXAST_REG_LOAD:
-        nxm_execute_reg_load((const struct nx_action_reg_load *) nah,
-                             &ctx->flow);
-        break;
-
-    case NXAST_NOTE:
-        /* Nothing to do. */
-        break;
-
-    case NXAST_SET_TUNNEL64:
-        tun_id = ((const struct nx_action_set_tunnel64 *) nah)->tun_id;
-        ctx->flow.tun_id = tun_id;
-        break;
-
-    case NXAST_MULTIPATH:
-        nam = (const struct nx_action_multipath *) nah;
-        multipath_execute(nam, &ctx->flow);
-        break;
-
-    case NXAST_AUTOPATH:
-        naa = (const struct nx_action_autopath *) nah;
-        xlate_autopath(ctx, naa);
-        break;
-
-    case NXAST_SNAT__OBSOLETE:
-    case NXAST_DROP_SPOOFED_ARP__OBSOLETE:
-    default:
-        VLOG_DBG_RL(&rl, "unknown Nicira action type %d", (int) subtype);
-        break;
-    }
-}
-
-static void
 do_xlate_actions(const union ofp_action *in, size_t n_in,
                  struct action_xlate_ctx *ctx)
 {
@@ -3130,68 +3059,116 @@ do_xlate_actions(const union ofp_action *in, size_t n_in,
 
     OFPUTIL_ACTION_FOR_EACH_UNSAFE (ia, left, in, n_in) {
         const struct ofp_action_dl_addr *oada;
-        enum ofp_action_type type = ntohs(ia->type);
-
-        switch (type) {
-        case OFPAT_OUTPUT:
+        const struct nx_action_resubmit *nar;
+        const struct nx_action_set_tunnel *nast;
+        const struct nx_action_set_queue *nasq;
+        const struct nx_action_multipath *nam;
+        const struct nx_action_autopath *naa;
+        enum ofputil_action_code code;
+        ovs_be64 tun_id;
+
+        code = ofputil_decode_action_unsafe(ia);
+        switch (code) {
+        case OFPUTIL_OFPAT_OUTPUT:
             xlate_output_action(ctx, &ia->output);
             break;
 
-        case OFPAT_SET_VLAN_VID:
+        case OFPUTIL_OFPAT_SET_VLAN_VID:
             ctx->flow.vlan_tci &= ~htons(VLAN_VID_MASK);
             ctx->flow.vlan_tci |= ia->vlan_vid.vlan_vid | htons(VLAN_CFI);
             break;
 
-        case OFPAT_SET_VLAN_PCP:
+        case OFPUTIL_OFPAT_SET_VLAN_PCP:
             ctx->flow.vlan_tci &= ~htons(VLAN_PCP_MASK);
             ctx->flow.vlan_tci |= htons(
                 (ia->vlan_pcp.vlan_pcp << VLAN_PCP_SHIFT) | VLAN_CFI);
             break;
 
-        case OFPAT_STRIP_VLAN:
+        case OFPUTIL_OFPAT_STRIP_VLAN:
             ctx->flow.vlan_tci = htons(0);
             break;
 
-        case OFPAT_SET_DL_SRC:
+        case OFPUTIL_OFPAT_SET_DL_SRC:
             oada = ((struct ofp_action_dl_addr *) ia);
             memcpy(ctx->flow.dl_src, oada->dl_addr, ETH_ADDR_LEN);
             break;
 
-        case OFPAT_SET_DL_DST:
+        case OFPUTIL_OFPAT_SET_DL_DST:
             oada = ((struct ofp_action_dl_addr *) ia);
             memcpy(ctx->flow.dl_dst, oada->dl_addr, ETH_ADDR_LEN);
             break;
 
-        case OFPAT_SET_NW_SRC:
+        case OFPUTIL_OFPAT_SET_NW_SRC:
             ctx->flow.nw_src = ia->nw_addr.nw_addr;
             break;
 
-        case OFPAT_SET_NW_DST:
+        case OFPUTIL_OFPAT_SET_NW_DST:
             ctx->flow.nw_dst = ia->nw_addr.nw_addr;
             break;
 
-        case OFPAT_SET_NW_TOS:
+        case OFPUTIL_OFPAT_SET_NW_TOS:
             ctx->flow.nw_tos = ia->nw_tos.nw_tos;
             break;
 
-        case OFPAT_SET_TP_SRC:
+        case OFPUTIL_OFPAT_SET_TP_SRC:
             ctx->flow.tp_src = ia->tp_port.tp_port;
             break;
 
-        case OFPAT_SET_TP_DST:
+        case OFPUTIL_OFPAT_SET_TP_DST:
             ctx->flow.tp_dst = ia->tp_port.tp_port;
             break;
 
-        case OFPAT_VENDOR:
-            xlate_nicira_action(ctx, (const struct nx_action_header *) ia);
+        case OFPUTIL_OFPAT_ENQUEUE:
+            xlate_enqueue_action(ctx, (const struct ofp_action_enqueue *) ia);
+            break;
+
+        case OFPUTIL_NXAST_RESUBMIT:
+            nar = (const struct nx_action_resubmit *) ia;
+            xlate_table_action(ctx, ntohs(nar->in_port));
             break;
 
-        case OFPAT_ENQUEUE:
-            xlate_enqueue_action(ctx, (const struct ofp_action_enqueue *) ia);
+        case OFPUTIL_NXAST_SET_TUNNEL:
+            nast = (const struct nx_action_set_tunnel *) ia;
+            tun_id = htonll(ntohl(nast->tun_id));
+            ctx->flow.tun_id = tun_id;
+            break;
+
+        case OFPUTIL_NXAST_SET_QUEUE:
+            nasq = (const struct nx_action_set_queue *) ia;
+            xlate_set_queue_action(ctx, nasq);
+            break;
+
+        case OFPUTIL_NXAST_POP_QUEUE:
+            ctx->priority = 0;
+            break;
+
+        case OFPUTIL_NXAST_REG_MOVE:
+            nxm_execute_reg_move((const struct nx_action_reg_move *) ia,
+                                 &ctx->flow);
+            break;
+
+        case OFPUTIL_NXAST_REG_LOAD:
+            nxm_execute_reg_load((const struct nx_action_reg_load *) ia,
+                                 &ctx->flow);
+            break;
+
+        case OFPUTIL_NXAST_NOTE:
+            /* Nothing to do. */
+            break;
+
+        case OFPUTIL_NXAST_SET_TUNNEL64:
+            tun_id = ((const struct nx_action_set_tunnel64 *) ia)->tun_id;
+            ctx->flow.tun_id = tun_id;
+            break;
+
+        case OFPUTIL_NXAST_MULTIPATH:
+            nam = (const struct nx_action_multipath *) ia;
+            multipath_execute(nam, &ctx->flow);
             break;
 
-        default:
-            VLOG_DBG_RL(&rl, "unknown action type %d", (int) type);
+        case OFPUTIL_NXAST_AUTOPATH:
+            naa = (const struct nx_action_autopath *) ia;
+            xlate_autopath(ctx, naa);
             break;
         }
     }