From e3f8f887480856064fab42bc01ae364d64506d85 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Thu, 24 Oct 2013 13:19:31 -0700 Subject: [PATCH] ofp-actions: Simplify interface and internal structure. This makes later changes simpler. Signed-off-by: Jarno Rajahalme Signed-off-by: Ben Pfaff --- lib/ofp-actions.c | 274 ++++++++++++++++-------------------------- lib/ofp-actions.h | 32 +++-- lib/ofp-util.c | 127 ++++++++++++-------- lib/ofp-util.h | 1 + utilities/ovs-ofctl.c | 21 ++-- 5 files changed, 205 insertions(+), 250 deletions(-) diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index f166a73d9..556fd052e 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -228,7 +228,7 @@ dec_ttl_from_openflow(struct ofpbuf *out, enum ofputil_action_code compat) static enum ofperr dec_ttl_cnt_ids_from_openflow(const struct nx_action_cnt_ids *nac_ids, - struct ofpbuf *out) + struct ofpbuf *out) { struct ofpact_cnt_ids *ids; size_t ids_size; @@ -497,7 +497,9 @@ ofpact_from_nxast(const union ofp_action *a, enum ofputil_action_code code, } static enum ofperr -ofpact_from_openflow10(const union ofp_action *a, struct ofpbuf *out) +ofpact_from_openflow10(const union ofp_action *a, + enum ofp_version version OVS_UNUSED, + struct ofpbuf *out) { enum ofputil_action_code code; enum ofperr error; @@ -588,6 +590,10 @@ ofpact_from_openflow10(const union ofp_action *a, struct ofpbuf *out) return error; } +static enum ofperr ofpact_from_openflow11(const union ofp_action *, + enum ofp_version, + struct ofpbuf *out); + static inline union ofp_action * action_next(const union ofp_action *a) { @@ -629,15 +635,19 @@ log_bad_action(const union ofp_action *actions, size_t max_actions, static enum ofperr ofpacts_from_openflow(const union ofp_action *in, size_t n_in, - struct ofpbuf *out, - enum ofperr (*ofpact_from_openflow)( - const union ofp_action *a, struct ofpbuf *out)) + enum ofp_version version, struct ofpbuf *out) { const union ofp_action *a; size_t left; + enum ofperr (*ofpact_from_openflow)(const union ofp_action *a, + enum ofp_version, + struct ofpbuf *out) = + (version == OFP10_VERSION) ? + ofpact_from_openflow10 : ofpact_from_openflow11; + ACTION_FOR_EACH (a, left, in, n_in) { - enum ofperr error = ofpact_from_openflow(a, out); + enum ofperr error = ofpact_from_openflow(a, version, out); if (error) { log_bad_action(in, n_in, a, error); return error; @@ -653,20 +663,28 @@ ofpacts_from_openflow(const union ofp_action *in, size_t n_in, return 0; } -static enum ofperr -ofpacts_from_openflow10(const union ofp_action *in, size_t n_in, - struct ofpbuf *out) -{ - return ofpacts_from_openflow(in, n_in, out, ofpact_from_openflow10); -} - -static enum ofperr -ofpacts_pull_actions(struct ofpbuf *openflow, unsigned int actions_len, - struct ofpbuf *ofpacts, - enum ofperr (*translate)(const union ofp_action *actions, - size_t max_actions, - struct ofpbuf *ofpacts)) -{ +/* Attempts to convert 'actions_len' bytes of OpenFlow actions from the + * front of 'openflow' into ofpacts. On success, replaces any existing content + * in 'ofpacts' by the converted ofpacts; on failure, clears 'ofpacts'. + * Returns 0 if successful, otherwise an OpenFlow error. + * + * Actions are processed according to their OpenFlow version which + * is provided in the 'version' parameter. + * + * In most places in OpenFlow 1.1 and 1.2, actions appear encapsulated in + * instructions, so you should call ofpacts_pull_openflow_instructions() + * instead of this function. + * + * The parsed actions are valid generically, but they may not be valid in a + * specific context. For example, port numbers up to OFPP_MAX are valid + * generically, but specific datapaths may only support port numbers in a + * smaller range. Use ofpacts_check() to additional check whether actions are + * valid in a specific context. */ +enum ofperr +ofpacts_pull_openflow_actions(struct ofpbuf *openflow, + unsigned int actions_len, + enum ofp_version version, + struct ofpbuf *ofpacts) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); const union ofp_action *actions; enum ofperr error; @@ -687,7 +705,8 @@ ofpacts_pull_actions(struct ofpbuf *openflow, unsigned int actions_len, return OFPERR_OFPBRC_BAD_LEN; } - error = translate(actions, actions_len / OFP_ACTION_ALIGN, ofpacts); + error = ofpacts_from_openflow(actions, actions_len / OFP_ACTION_ALIGN, + version, ofpacts); if (error) { ofpbuf_clear(ofpacts); return error; @@ -700,23 +719,6 @@ ofpacts_pull_actions(struct ofpbuf *openflow, unsigned int actions_len, return error; } -/* Attempts to convert 'actions_len' bytes of OpenFlow 1.0 actions from the - * front of 'openflow' into ofpacts. On success, replaces any existing content - * in 'ofpacts' by the converted ofpacts; on failure, clears 'ofpacts'. - * Returns 0 if successful, otherwise an OpenFlow error. - * - * The parsed actions are valid generically, but they may not be valid in a - * specific context. For example, port numbers up to OFPP_MAX are valid - * generically, but specific datapaths may only support port numbers in a - * smaller range. Use ofpacts_check() to additional check whether actions are - * valid in a specific context. */ -enum ofperr -ofpacts_pull_openflow10(struct ofpbuf *openflow, unsigned int actions_len, - struct ofpbuf *ofpacts) -{ - return ofpacts_pull_actions(openflow, actions_len, ofpacts, - ofpacts_from_openflow10); -} /* OpenFlow 1.1 actions. */ @@ -1090,7 +1092,8 @@ output_from_openflow11(const struct ofp11_action_output *oao, } static enum ofperr -ofpact_from_openflow11(const union ofp_action *a, struct ofpbuf *out) +ofpact_from_openflow11(const union ofp_action *a, enum ofp_version version, + struct ofpbuf *out) { enum ofputil_action_code code; enum ofperr error; @@ -1208,7 +1211,10 @@ ofpact_from_openflow11(const union ofp_action *a, struct ofpbuf *out) break; case OFPUTIL_OFPAT11_PUSH_MPLS: + /* OpenFlow 1.3 has different semantics. */ error = push_mpls_from_openflow(a->push.ethertype, + version >= OFP13_VERSION ? + OFPACT_MPLS_BEFORE_VLAN : OFPACT_MPLS_AFTER_VLAN, out); break; @@ -1231,13 +1237,6 @@ ofpact_from_openflow11(const union ofp_action *a, struct ofpbuf *out) return error; } -static enum ofperr -ofpacts_from_openflow11(const union ofp_action *in, size_t n_in, - struct ofpbuf *out) -{ - return ofpacts_from_openflow(in, n_in, out, ofpact_from_openflow11); -} - /* True if an action sets the value of a field * in a way that is compatibile with the action set. * False otherwise. */ @@ -1449,13 +1448,15 @@ ofpacts_execute_action_set(struct ofpbuf *action_list, static enum ofperr ofpacts_from_openflow11_for_action_set(const union ofp_action *in, - size_t n_in, struct ofpbuf *out) + size_t n_in, enum ofp_version version, + struct ofpbuf *out) { enum ofperr error; struct ofpact *a; size_t start = out->size; - error = ofpacts_from_openflow11(in, n_in, out); + error = ofpacts_from_openflow(in, n_in, version, out); + if (error) { return error; } @@ -1470,35 +1471,6 @@ ofpacts_from_openflow11_for_action_set(const union ofp_action *in, return 0; } - -static enum ofperr -ofpact_from_openflow13(const union ofp_action *a, struct ofpbuf *out) -{ - enum ofputil_action_code code; - enum ofperr error; - - error = decode_openflow11_action(a, &code); - if (error) { - return error; - } - - if (code == OFPUTIL_OFPAT11_PUSH_MPLS) { - struct ofp11_action_push *oap = (struct ofp11_action_push *)a; - error = push_mpls_from_openflow(oap->ethertype, - OFPACT_MPLS_BEFORE_VLAN, out); - } else { - error = ofpact_from_openflow11(a, out); - } - - return error; -} - -static enum ofperr -ofpacts_from_openflow13(const union ofp_action *in, size_t n_in, - struct ofpbuf *out) -{ - return ofpacts_from_openflow(in, n_in, out, ofpact_from_openflow13); -} /* OpenFlow 1.1 instructions. */ @@ -1709,48 +1681,11 @@ get_actions_from_instruction(const struct ofp11_instruction *inst, *max_actions = (ntohs(inst->len) - sizeof *inst) / OFP11_INSTRUCTION_ALIGN; } -/* Attempts to convert 'actions_len' bytes of OpenFlow actions from the - * front of 'openflow' into ofpacts. On success, replaces any existing content - * in 'ofpacts' by the converted ofpacts; on failure, clears 'ofpacts'. - * Returns 0 if successful, otherwise an OpenFlow error. - * - * Actions are processed according to their OpenFlow version which - * is provided in the 'version' parameter. - * - * In most places in OpenFlow 1.1 and 1.2, actions appear encapsulated in - * instructions, so you should call ofpacts_pull_openflow11_instructions() - * instead of this function. - * - * The parsed actions are valid generically, but they may not be valid in a - * specific context. For example, port numbers up to OFPP_MAX are valid - * generically, but specific datapaths may only support port numbers in a - * smaller range. Use ofpacts_check() to additional check whether actions are - * valid in a specific context. */ -enum ofperr -ofpacts_pull_openflow11_actions(struct ofpbuf *openflow, - enum ofp_version version, - unsigned int actions_len, - struct ofpbuf *ofpacts) -{ - switch (version) { - case OFP10_VERSION: - case OFP11_VERSION: - case OFP12_VERSION: - return ofpacts_pull_actions(openflow, actions_len, ofpacts, - ofpacts_from_openflow11); - case OFP13_VERSION: - return ofpacts_pull_actions(openflow, actions_len, ofpacts, - ofpacts_from_openflow13); - default: - NOT_REACHED(); - } -} - enum ofperr -ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow, - enum ofp_version version, - unsigned int instructions_len, - struct ofpbuf *ofpacts) +ofpacts_pull_openflow_instructions(struct ofpbuf *openflow, + unsigned int instructions_len, + enum ofp_version version, + struct ofpbuf *ofpacts) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); const struct ofp11_instruction *instructions; @@ -1799,18 +1734,7 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow, get_actions_from_instruction(insts[OVSINST_OFPIT11_APPLY_ACTIONS], &actions, &max_actions); - switch (version) { - case OFP10_VERSION: - case OFP11_VERSION: - case OFP12_VERSION: - error = ofpacts_from_openflow11(actions, max_actions, ofpacts); - break; - case OFP13_VERSION: - error = ofpacts_from_openflow13(actions, max_actions, ofpacts); - break; - default: - NOT_REACHED(); - } + error = ofpacts_from_openflow(actions, max_actions, version, ofpacts); if (error) { goto exit; } @@ -1833,7 +1757,7 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow, get_actions_from_instruction(insts[OVSINST_OFPIT11_WRITE_ACTIONS], &actions, &max_actions); error = ofpacts_from_openflow11_for_action_set(actions, max_actions, - ofpacts); + version, ofpacts); if (error) { goto exit; } @@ -2314,10 +2238,6 @@ ofpact_to_nxast(const struct ofpact *a, struct ofpbuf *out) nxm_reg_load_to_nxast(ofpact_get_REG_LOAD(a), out); break; - case OFPACT_SET_FIELD: - set_field_to_openflow(ofpact_get_SET_FIELD(a), out); - break; - case OFPACT_STACK_PUSH: nxm_stack_push_to_nxast(ofpact_get_STACK_PUSH(a), out); break; @@ -2414,6 +2334,7 @@ ofpact_to_nxast(const struct ofpact *a, struct ofpbuf *out) case OFPACT_CLEAR_ACTIONS: case OFPACT_GOTO_TABLE: case OFPACT_METER: + case OFPACT_SET_FIELD: NOT_REACHED(); } } @@ -2518,12 +2439,15 @@ ofpact_to_openflow10(const struct ofpact *a, struct ofpbuf *out) case OFPACT_GROUP: break; + case OFPACT_SET_FIELD: + set_field_to_openflow(ofpact_get_SET_FIELD(a), out); + break; + case OFPACT_CONTROLLER: case OFPACT_OUTPUT_REG: case OFPACT_BUNDLE: case OFPACT_REG_MOVE: case OFPACT_REG_LOAD: - case OFPACT_SET_FIELD: case OFPACT_STACK_PUSH: case OFPACT_STACK_POP: case OFPACT_DEC_TTL: @@ -2548,20 +2472,6 @@ ofpact_to_openflow10(const struct ofpact *a, struct ofpbuf *out) break; } } - -/* Converts the 'ofpacts_len' bytes of ofpacts in 'ofpacts' into OpenFlow 1.0 - * actions in 'openflow', appending the actions to any existing data in - * 'openflow'. */ -void -ofpacts_put_openflow10(const struct ofpact ofpacts[], size_t ofpacts_len, - struct ofpbuf *openflow) -{ - const struct ofpact *a; - - OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) { - ofpact_to_openflow10(a, openflow); - } -} /* Converting ofpacts to OpenFlow 1.1. */ @@ -2721,12 +2631,15 @@ ofpact_to_openflow11(const struct ofpact *a, struct ofpbuf *out) htonl(ofpact_get_GROUP(a)->group_id); break; + case OFPACT_SET_FIELD: + set_field_to_openflow(ofpact_get_SET_FIELD(a), out); + break; + case OFPACT_CONTROLLER: case OFPACT_OUTPUT_REG: case OFPACT_BUNDLE: case OFPACT_REG_MOVE: case OFPACT_REG_LOAD: - case OFPACT_SET_FIELD: case OFPACT_STACK_PUSH: case OFPACT_STACK_POP: case OFPACT_SET_TUNNEL: @@ -2743,20 +2656,31 @@ ofpact_to_openflow11(const struct ofpact *a, struct ofpbuf *out) } } -/* Converts the ofpacts in 'ofpacts' (terminated by OFPACT_END) into OpenFlow - * 1.1 actions in 'openflow', appending the actions to any existing data in +static void +ofpact_to_openflow12(const struct ofpact *a, struct ofpbuf *out) +{ + ofpact_to_openflow11(a, out); +} + +/* Converts the 'ofpacts_len' bytes of ofpacts in 'ofpacts' into OpenFlow + * actions in 'openflow', appending the actions to any existing data in * 'openflow'. */ size_t -ofpacts_put_openflow11_actions(const struct ofpact ofpacts[], - size_t ofpacts_len, struct ofpbuf *openflow) +ofpacts_put_openflow_actions(const struct ofpact ofpacts[], size_t ofpacts_len, + struct ofpbuf *openflow, + enum ofp_version ofp_version) { const struct ofpact *a; size_t start_size = openflow->size; + void (*translate)(const struct ofpact *a, struct ofpbuf *out) = + (ofp_version == OFP10_VERSION) ? ofpact_to_openflow10 : + (ofp_version == OFP11_VERSION) ? ofpact_to_openflow11 : + ofpact_to_openflow12; + OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) { - ofpact_to_openflow11(a, openflow); + translate(a, openflow); } - return openflow->size - start_size; } @@ -2775,12 +2699,15 @@ ofpacts_update_instruction_actions(struct ofpbuf *openflow, size_t ofs) } void -ofpacts_put_openflow11_instructions(const struct ofpact ofpacts[], - size_t ofpacts_len, - struct ofpbuf *openflow) +ofpacts_put_openflow_instructions(const struct ofpact ofpacts[], + size_t ofpacts_len, + struct ofpbuf *openflow, + enum ofp_version ofp_version) { const struct ofpact *a; + ovs_assert(ofp_version >= OFP11_VERSION); + OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) { switch (ovs_instruction_type_from_ofpact_type(a->type)) { case OVSINST_OFPIT11_CLEAR_ACTIONS: @@ -2806,15 +2733,16 @@ ofpacts_put_openflow11_instructions(const struct ofpact ofpacts[], break; } - case OVSINST_OFPIT13_METER: { - const struct ofpact_meter *om; - struct ofp13_instruction_meter *oim; + case OVSINST_OFPIT13_METER: + if (ofp_version >= OFP13_VERSION) { + const struct ofpact_meter *om; + struct ofp13_instruction_meter *oim; - om = ofpact_get_METER(a); - oim = instruction_put_OFPIT13_METER(openflow); - oim->meter_id = htonl(om->meter_id); + om = ofpact_get_METER(a); + oim = instruction_put_OFPIT13_METER(openflow); + oim->meter_id = htonl(om->meter_id); + } break; - } case OVSINST_OFPIT11_APPLY_ACTIONS: { const size_t ofs = openflow->size; @@ -2829,7 +2757,11 @@ ofpacts_put_openflow11_instructions(const struct ofpact ofpacts[], != OVSINST_OFPIT11_APPLY_ACTIONS) { break; } - ofpact_to_openflow11(action, openflow); + if (ofp_version == OFP11_VERSION) { + ofpact_to_openflow11(action, openflow); + } else { + ofpact_to_openflow12(action, openflow); + } processed = action; } ofpacts_update_instruction_actions(openflow, ofs); @@ -2843,9 +2775,9 @@ ofpacts_put_openflow11_instructions(const struct ofpact ofpacts[], on = ofpact_get_WRITE_ACTIONS(a); instruction_put_OFPIT11_WRITE_ACTIONS(openflow); - ofpacts_put_openflow11_actions(on->actions, - ofpact_nest_get_action_len(on), - openflow); + ofpacts_put_openflow_actions(on->actions, + ofpact_nest_get_action_len(on), + openflow, ofp_version); ofpacts_update_instruction_actions(openflow, ofs); break; diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h index 2268a36a6..db2c02897 100644 --- a/lib/ofp-actions.h +++ b/lib/ofp-actions.h @@ -577,30 +577,26 @@ struct ofpact_group { }; /* Converting OpenFlow to ofpacts. */ -enum ofperr ofpacts_pull_openflow10(struct ofpbuf *openflow, - unsigned int actions_len, - struct ofpbuf *ofpacts); -enum ofperr ofpacts_pull_openflow11_actions(struct ofpbuf *openflow, - enum ofp_version version, - unsigned int actions_len, - struct ofpbuf *ofpacts); -enum ofperr ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow, - enum ofp_version version, - unsigned int instructions_len, - struct ofpbuf *ofpacts); +enum ofperr ofpacts_pull_openflow_actions(struct ofpbuf *openflow, + unsigned int actions_len, + enum ofp_version version, + struct ofpbuf *ofpacts); +enum ofperr ofpacts_pull_openflow_instructions(struct ofpbuf *openflow, + unsigned int instructions_len, + enum ofp_version version, + struct ofpbuf *ofpacts); enum ofperr ofpacts_check(struct ofpact[], size_t ofpacts_len, struct flow *, ofp_port_t max_ports, uint8_t table_id, bool enforce_consistency); enum ofperr ofpacts_verify(const struct ofpact ofpacts[], size_t ofpacts_len); /* Converting ofpacts to OpenFlow. */ -void ofpacts_put_openflow10(const struct ofpact[], size_t ofpacts_len, - struct ofpbuf *openflow); -size_t ofpacts_put_openflow11_actions(const struct ofpact[], size_t ofpacts_len, - struct ofpbuf *openflow); -void ofpacts_put_openflow11_instructions(const struct ofpact[], - size_t ofpacts_len, - struct ofpbuf *openflow); +size_t ofpacts_put_openflow_actions(const struct ofpact[], size_t ofpacts_len, + struct ofpbuf *openflow, enum ofp_version); +void ofpacts_put_openflow_instructions(const struct ofpact[], + size_t ofpacts_len, + struct ofpbuf *openflow, + enum ofp_version ofp_version); /* Working with ofpacts. */ bool ofpacts_output_to_port(const struct ofpact[], size_t ofpacts_len, diff --git a/lib/ofp-util.c b/lib/ofp-util.c index d435e9969..2223c97cb 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -1504,8 +1504,8 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm, return error; } - error = ofpacts_pull_openflow11_instructions(&b, oh->version, - b.size, ofpacts); + error = ofpacts_pull_openflow_instructions(&b, b.size, oh->version, + ofpacts); if (error) { return error; } @@ -1561,7 +1561,8 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm, ofputil_normalize_match(&fm->match); /* Now get the actions. */ - error = ofpacts_pull_openflow10(&b, b.size, ofpacts); + error = ofpacts_pull_openflow_actions(&b, b.size, oh->version, + ofpacts); if (error) { return error; } @@ -1594,7 +1595,8 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm, if (error) { return error; } - error = ofpacts_pull_openflow10(&b, b.size, ofpacts); + error = ofpacts_pull_openflow_actions(&b, b.size, oh->version, + ofpacts); if (error) { return error; } @@ -2066,7 +2068,8 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm, ofm->out_group = htonl(fm->out_group); ofm->flags = raw_flags; ofputil_put_ofp11_match(msg, &fm->match, protocol); - ofpacts_put_openflow11_instructions(fm->ofpacts, fm->ofpacts_len, msg); + ofpacts_put_openflow_instructions(fm->ofpacts, fm->ofpacts_len, msg, + version); break; } @@ -2086,7 +2089,8 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm, ofm->buffer_id = htonl(fm->buffer_id); ofm->out_port = htons(ofp_to_u16(fm->out_port)); ofm->flags = raw_flags; - ofpacts_put_openflow10(fm->ofpacts, fm->ofpacts_len, msg); + ofpacts_put_openflow_actions(fm->ofpacts, fm->ofpacts_len, msg, + version); break; } @@ -2109,7 +2113,8 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm, nfm->out_port = htons(ofp_to_u16(fm->out_port)); nfm->flags = raw_flags; nfm->match_len = htons(match_len); - ofpacts_put_openflow10(fm->ofpacts, fm->ofpacts_len, msg); + ofpacts_put_openflow_actions(fm->ofpacts, fm->ofpacts_len, msg, + version); break; } @@ -2638,9 +2643,9 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, return EINVAL; } - if (ofpacts_pull_openflow11_instructions(msg, oh->version, - length - sizeof *ofs - - padded_match_len, ofpacts)) { + if (ofpacts_pull_openflow_instructions(msg, length - sizeof *ofs - + padded_match_len, oh->version, + ofpacts)) { VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions"); return EINVAL; } @@ -2683,7 +2688,8 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, return EINVAL; } - if (ofpacts_pull_openflow10(msg, length - sizeof *ofs, ofpacts)) { + if (ofpacts_pull_openflow_actions(msg, length - sizeof *ofs, + oh->version, ofpacts)) { return EINVAL; } @@ -2723,7 +2729,8 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, } actions_len = length - sizeof *nfs - ROUND_UP(match_len, 8); - if (ofpacts_pull_openflow10(msg, actions_len, ofpacts)) { + if (ofpacts_pull_openflow_actions(msg, actions_len, oh->version, + ofpacts)) { return EINVAL; } @@ -2777,16 +2784,16 @@ ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *fs, struct ofpbuf *reply = ofpbuf_from_list(list_back(replies)); size_t start_ofs = reply->size; enum ofpraw raw; + enum ofp_version version = ((struct ofp_header *)reply->data)->version; ofpraw_decode_partial(&raw, reply->data, reply->size); if (raw == OFPRAW_OFPST11_FLOW_REPLY || raw == OFPRAW_OFPST13_FLOW_REPLY) { - const struct ofp_header *oh = reply->data; struct ofp11_flow_stats *ofs; ofpbuf_put_uninit(reply, sizeof *ofs); oxm_put_match(reply, &fs->match); - ofpacts_put_openflow11_instructions(fs->ofpacts, fs->ofpacts_len, - reply); + ofpacts_put_openflow_instructions(fs->ofpacts, fs->ofpacts_len, reply, + version); ofs = ofpbuf_at_assert(reply, start_ofs, sizeof *ofs); ofs->length = htons(reply->size - start_ofs); @@ -2798,7 +2805,7 @@ ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *fs, ofs->idle_timeout = htons(fs->idle_timeout); ofs->hard_timeout = htons(fs->hard_timeout); if (raw == OFPRAW_OFPST13_FLOW_REPLY) { - ofs->flags = ofputil_encode_flow_mod_flags(fs->flags, oh->version); + ofs->flags = ofputil_encode_flow_mod_flags(fs->flags, version); } else { ofs->flags = 0; } @@ -2810,8 +2817,8 @@ ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *fs, struct ofp10_flow_stats *ofs; ofpbuf_put_uninit(reply, sizeof *ofs); - ofpacts_put_openflow10(fs->ofpacts, fs->ofpacts_len, reply); - + ofpacts_put_openflow_actions(fs->ofpacts, fs->ofpacts_len, reply, + version); ofs = ofpbuf_at_assert(reply, start_ofs, sizeof *ofs); ofs->length = htons(reply->size - start_ofs); ofs->table_id = fs->table_id; @@ -2834,8 +2841,8 @@ ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *fs, ofpbuf_put_uninit(reply, sizeof *nfs); match_len = nx_put_match(reply, &fs->match, 0, 0); - ofpacts_put_openflow10(fs->ofpacts, fs->ofpacts_len, reply); - + ofpacts_put_openflow_actions(fs->ofpacts, fs->ofpacts_len, reply, + version); nfs = ofpbuf_at_assert(reply, start_ofs, sizeof *nfs); nfs->length = htons(reply->size - start_ofs); nfs->table_id = fs->table_id; @@ -3371,9 +3378,8 @@ ofputil_decode_packet_out(struct ofputil_packet_out *po, return error; } - error = ofpacts_pull_openflow11_actions(&b, oh->version, - ntohs(opo->actions_len), - ofpacts); + error = ofpacts_pull_openflow_actions(&b, ntohs(opo->actions_len), + oh->version, ofpacts); if (error) { return error; } @@ -3384,7 +3390,8 @@ ofputil_decode_packet_out(struct ofputil_packet_out *po, po->buffer_id = ntohl(opo->buffer_id); po->in_port = u16_to_ofp(ntohs(opo->in_port)); - error = ofpacts_pull_openflow10(&b, ntohs(opo->actions_len), ofpacts); + error = ofpacts_pull_openflow_actions(&b, ntohs(opo->actions_len), + oh->version, ofpacts); if (error) { return error; } @@ -4501,6 +4508,7 @@ ofputil_decode_flow_update(struct ofputil_flow_update *update, { struct nx_flow_update_header *nfuh; unsigned int length; + struct ofp_header *oh; if (!msg->l2) { msg->l2 = msg->data; @@ -4515,6 +4523,8 @@ ofputil_decode_flow_update(struct ofputil_flow_update *update, goto bad_len; } + oh = msg->l2; + nfuh = msg->data; update->event = ntohs(nfuh->event); length = ntohs(nfuh->length); @@ -4563,7 +4573,8 @@ ofputil_decode_flow_update(struct ofputil_flow_update *update, } actions_len = length - sizeof *nfuf - ROUND_UP(match_len, 8); - error = ofpacts_pull_openflow10(msg, actions_len, ofpacts); + error = ofpacts_pull_openflow_actions(msg, actions_len, oh->version, + ofpacts); if (error) { return error; } @@ -4623,9 +4634,11 @@ ofputil_append_flow_update(const struct ofputil_flow_update *update, struct nx_flow_update_header *nfuh; struct ofpbuf *msg; size_t start_ofs; + enum ofp_version version; msg = ofpbuf_from_list(list_back(replies)); start_ofs = msg->size; + version = ((struct ofp_header *)msg->l2)->version; if (update->event == NXFME_ABBREV) { struct nx_flow_update_abbrev *nfua; @@ -4638,8 +4651,8 @@ ofputil_append_flow_update(const struct ofputil_flow_update *update, ofpbuf_put_zeros(msg, sizeof *nfuf); match_len = nx_put_match(msg, update->match, htonll(0), htonll(0)); - ofpacts_put_openflow10(update->ofpacts, update->ofpacts_len, msg); - + ofpacts_put_openflow_actions(update->ofpacts, update->ofpacts_len, msg, + version); nfuf = ofpbuf_at_assert(msg, start_ofs, sizeof *nfuf); nfuf->reason = htons(update->reason); nfuf->priority = htons(update->priority); @@ -4678,7 +4691,8 @@ ofputil_encode_packet_out(const struct ofputil_packet_out *po, msg = ofpraw_alloc(OFPRAW_OFPT10_PACKET_OUT, OFP10_VERSION, size); ofpbuf_put_zeros(msg, sizeof *opo); actions_ofs = msg->size; - ofpacts_put_openflow10(po->ofpacts, po->ofpacts_len, msg); + ofpacts_put_openflow_actions(po->ofpacts, po->ofpacts_len, msg, + ofp_version); opo = msg->l3; opo->buffer_id = htonl(po->buffer_id); @@ -4695,8 +4709,8 @@ ofputil_encode_packet_out(const struct ofputil_packet_out *po, msg = ofpraw_alloc(OFPRAW_OFPT11_PACKET_OUT, ofp_version, size); ofpbuf_put_zeros(msg, sizeof *opo); - len = ofpacts_put_openflow11_actions(po->ofpacts, po->ofpacts_len, msg); - + len = ofpacts_put_openflow_actions(po->ofpacts, po->ofpacts_len, msg, + ofp_version); opo = msg->l3; opo->buffer_id = htonl(po->buffer_id); opo->in_port = ofputil_port_to_ofp11(po->in_port); @@ -5064,22 +5078,21 @@ size_t ofputil_count_phy_ports(uint8_t ofp_version, struct ofpbuf *b) return b->size / ofputil_get_phy_port_size(ofp_version); } -/* Returns the 'enum ofputil_action_code' corresponding to 'name' (e.g. if - * 'name' is "output" then the return value is OFPUTIL_OFPAT10_OUTPUT), or -1 if - * 'name' is not the name of any action. - * - * ofp-util.def lists the mapping from names to action. */ -int -ofputil_action_code_from_name(const char *name) -{ - static const char *const names[OFPUTIL_N_ACTIONS] = { - NULL, +/* ofp-util.def lists the mapping from names to action. */ +static const char *const names[OFPUTIL_N_ACTIONS] = { + NULL, #define OFPAT10_ACTION(ENUM, STRUCT, NAME) NAME, #define OFPAT11_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) NAME, #define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) NAME, #include "ofp-util.def" - }; +}; +/* Returns the 'enum ofputil_action_code' corresponding to 'name' (e.g. if + * 'name' is "output" then the return value is OFPUTIL_OFPAT10_OUTPUT), or -1 + * if 'name' is not the name of any action. */ +int +ofputil_action_code_from_name(const char *name) +{ const char *const *p; for (p = names; p < &names[ARRAY_SIZE(names)]; p++) { @@ -5090,6 +5103,15 @@ ofputil_action_code_from_name(const char *name) return -1; } +/* Returns name corresponding to the 'enum ofputil_action_code', + * or "Unkonwn action", if the name is not available. */ +const char * +ofputil_action_name_from_code(enum ofputil_action_code code) +{ + return code < (int)OFPUTIL_N_ACTIONS && names[code] ? names[code] + : "Unknown action"; +} + /* Appends an action of the type specified by 'code' to 'buf' and returns the * action. Initializes the parts of 'action' that identify it as having type * and length 'sizeof *action' and zeros the rest. For actions that @@ -5972,6 +5994,7 @@ ofputil_append_group_desc_reply(const struct ofputil_group_desc *gds, struct ofp11_group_desc_stats *ogds; struct ofputil_bucket *bucket; size_t start_ogds; + enum ofp_version version = ((struct ofp_header *)reply->data)->version; start_ogds = reply->size; ofpbuf_put_zeros(reply, sizeof *ogds); @@ -5981,9 +6004,8 @@ ofputil_append_group_desc_reply(const struct ofputil_group_desc *gds, start_ob = reply->size; ofpbuf_put_zeros(reply, sizeof *ob); - ofpacts_put_openflow11_actions(bucket->ofpacts, - bucket->ofpacts_len, reply); - + ofpacts_put_openflow_actions(bucket->ofpacts, bucket->ofpacts_len, + reply, version); ob = ofpbuf_at_assert(reply, start_ob, sizeof *ob); ob->len = htons(reply->size - start_ob); ob->weight = htons(bucket->weight); @@ -5999,8 +6021,8 @@ ofputil_append_group_desc_reply(const struct ofputil_group_desc *gds, } static enum ofperr -ofputil_pull_buckets(struct ofpbuf *msg, enum ofp_version version, - size_t buckets_length, struct list *buckets) +ofputil_pull_buckets(struct ofpbuf *msg, size_t buckets_length, + enum ofp_version version, struct list *buckets) { struct ofp11_bucket *ob; @@ -6033,8 +6055,8 @@ ofputil_pull_buckets(struct ofpbuf *msg, enum ofp_version version, buckets_length -= ob_len; ofpbuf_init(&ofpacts, 0); - error = ofpacts_pull_openflow11_actions(msg, version, - ob_len - sizeof *ob, &ofpacts); + error = ofpacts_pull_openflow_actions(msg, ob_len - sizeof *ob, + version, &ofpacts); if (error) { ofpbuf_uninit(&ofpacts); ofputil_bucket_list_destroy(buckets); @@ -6099,7 +6121,7 @@ ofputil_decode_group_desc_reply(struct ofputil_group_desc *gd, return OFPERR_OFPBRC_BAD_LEN; } - return ofputil_pull_buckets(msg, version, length - sizeof *ogds, + return ofputil_pull_buckets(msg, length - sizeof *ogds, version, &gd->buckets); } @@ -6141,8 +6163,9 @@ ofputil_encode_group_mod(enum ofp_version ofp_version, start_bucket = b->size; ofpbuf_put_uninit(b, sizeof *ob); if (bucket->ofpacts && bucket->ofpacts_len) { - ofpacts_put_openflow11_actions(bucket->ofpacts, - bucket->ofpacts_len, b); + ofpacts_put_openflow_actions(bucket->ofpacts, + bucket->ofpacts_len, b, + ofp_version); } ob = ofpbuf_at_assert(b, start_bucket, sizeof *ob); ob->len = htons(b->size - start_bucket);; @@ -6183,7 +6206,7 @@ ofputil_decode_group_mod(const struct ofp_header *oh, gm->type = ogm->type; gm->group_id = ntohl(ogm->group_id); - return ofputil_pull_buckets(&msg, oh->version, msg.size, &gm->buckets); + return ofputil_pull_buckets(&msg, msg.size, oh->version, &gm->buckets); } /* Parse a queue status request message into 'oqsr'. diff --git a/lib/ofp-util.h b/lib/ofp-util.h index eccce883b..f9b0be7ce 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -862,6 +862,7 @@ enum { }; int ofputil_action_code_from_name(const char *); +const char * ofputil_action_name_from_code(enum ofputil_action_code code); void *ofputil_put_action(enum ofputil_action_code, struct ofpbuf *buf); diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index da0a54b6c..a5c69c7bb 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -2813,7 +2813,8 @@ ofctl_parse_ofp10_actions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) /* Convert to ofpacts. */ ofpbuf_init(&ofpacts, 0); size = of10_in.size; - error = ofpacts_pull_openflow10(&of10_in, of10_in.size, &ofpacts); + error = ofpacts_pull_openflow_actions(&of10_in, of10_in.size, + OFP10_VERSION, &ofpacts); if (error) { printf("bad OF1.1 actions: %s\n\n", ofperr_get_name(error)); ofpbuf_uninit(&ofpacts); @@ -2831,7 +2832,8 @@ ofctl_parse_ofp10_actions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) /* Convert back to ofp10 actions and print differences from input. */ ofpbuf_init(&of10_out, 0); - ofpacts_put_openflow10(ofpacts.data, ofpacts.size, &of10_out); + ofpacts_put_openflow_actions(ofpacts.data, ofpacts.size, &of10_out, + OFP10_VERSION); print_differences("", of10_in.data, of10_in.size, of10_out.data, of10_out.size); @@ -2999,8 +3001,8 @@ ofctl_parse_ofp11_actions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) /* Convert to ofpacts. */ ofpbuf_init(&ofpacts, 0); size = of11_in.size; - error = ofpacts_pull_openflow11_actions(&of11_in, OFP11_VERSION, - of11_in.size, &ofpacts); + error = ofpacts_pull_openflow_actions(&of11_in, of11_in.size, + OFP11_VERSION, &ofpacts); if (error) { printf("bad OF1.1 actions: %s\n\n", ofperr_get_name(error)); ofpbuf_uninit(&ofpacts); @@ -3018,7 +3020,8 @@ ofctl_parse_ofp11_actions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) /* Convert back to ofp11 actions and print differences from input. */ ofpbuf_init(&of11_out, 0); - ofpacts_put_openflow11_actions(ofpacts.data, ofpacts.size, &of11_out); + ofpacts_put_openflow_actions(ofpacts.data, ofpacts.size, &of11_out, + OFP11_VERSION); print_differences("", of11_in.data, of11_in.size, of11_out.data, of11_out.size); @@ -3068,8 +3071,8 @@ ofctl_parse_ofp11_instructions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) /* Convert to ofpacts. */ ofpbuf_init(&ofpacts, 0); size = of11_in.size; - error = ofpacts_pull_openflow11_instructions(&of11_in, OFP11_VERSION, - of11_in.size, &ofpacts); + error = ofpacts_pull_openflow_instructions(&of11_in, of11_in.size, + OFP11_VERSION, &ofpacts); if (!error) { /* Verify actions, enforce consistency. */ struct flow flow; @@ -3095,8 +3098,8 @@ ofctl_parse_ofp11_instructions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) /* Convert back to ofp11 instructions and print differences from * input. */ ofpbuf_init(&of11_out, 0); - ofpacts_put_openflow11_instructions(ofpacts.data, ofpacts.size, - &of11_out); + ofpacts_put_openflow_instructions(ofpacts.data, ofpacts.size, + &of11_out, OFP13_VERSION); print_differences("", of11_in.data, of11_in.size, of11_out.data, of11_out.size); -- 2.43.0