From 38f2e36072c9065cae3d4fbab4a70e4f502706cd Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 30 Jun 2011 10:05:52 -0700 Subject: [PATCH] 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. --- lib/ofp-print.c | 334 ++++++++++++------------------------ lib/ofp-util.c | 375 ++++++++++++++++++++--------------------- lib/ofp-util.h | 32 ++++ ofproto/ofproto-dpif.c | 159 ++++++++--------- 4 files changed, 398 insertions(+), 502 deletions(-) diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 23ca528ce..4f4e33c51 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 56b1e4125..00f9ce8bf 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 8fb5b1bc1..48b0a4cfb 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 5ce05d6e4..1e77cda23 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -3039,77 +3039,6 @@ xlate_autopath(struct action_xlate_ctx *ctx, autopath_execute(naa, &ctx->flow, ofp_port); } -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; } } -- 2.43.0