Simplify some of the ofp-actions interface.
Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
---
lib/ofp-actions.c | 137 +++++++++++++++++++++++--------------------------
lib/ofp-actions.h | 17 +++---
lib/ofp-util.c | 77 +++++++++++++++++----------
lib/ofp-util.h | 1 +
utilities/ovs-ofctl.c | 11 ++--
5 files changed, 128 insertions(+), 115 deletions(-)
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 0963874..d5e3acd 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -481,7 +481,8 @@ 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, struct ofpbuf *out,
+ enum ofp_version ofp_version OVS_UNUSED)
{
enum ofputil_action_code code;
enum ofperr error;
@@ -565,6 +566,10 @@ ofpact_from_openflow10(const union ofp_action *a, struct
ofpbuf *out)
return error;
}
+static enum ofperr ofpact_from_openflow11(const union ofp_action *,
+ struct ofpbuf *out,
+ enum ofp_version);
+
static inline union ofp_action *
action_next(const union ofp_action *a)
{
@@ -605,15 +610,18 @@ log_bad_action(const union ofp_action *actions, size_t
n_actions, size_t ofs,
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))
+ struct ofpbuf *out, enum ofp_version ofp_version)
{
const union ofp_action *a;
size_t left;
+ enum ofperr (*ofpact_from_openflow)
+ (const union ofp_action *a, struct ofpbuf *out,
+ enum ofp_version) = (ofp_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, out, ofp_version);
if (error) {
log_bad_action(in, n_in, a - in, error);
return error;
@@ -629,19 +637,25 @@ 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 n_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.
+ *
+ * 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,
+ struct ofpbuf *ofpacts,
+ enum ofp_version ofp_version)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
const union ofp_action *actions;
@@ -663,7 +677,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,
+ ofpacts, ofp_version);
if (error) {
ofpbuf_clear(ofpacts);
return error;
@@ -676,23 +691,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. */
@@ -796,7 +794,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, struct ofpbuf *out,
+ enum ofp_version ofp_version)
{
enum ofputil_action_code code;
enum ofperr error;
@@ -806,6 +805,23 @@ ofpact_from_openflow11(const union ofp_action *a, struct
ofpbuf *out)
return error;
}
+ if (ofp_version >= OFP12_VERSION) {
+ switch ((int)code) {
+ case OFPUTIL_OFPAT11_SET_VLAN_VID:
+ case OFPUTIL_OFPAT11_SET_VLAN_PCP:
+ case OFPUTIL_OFPAT11_SET_DL_SRC:
+ case OFPUTIL_OFPAT11_SET_DL_DST:
+ case OFPUTIL_OFPAT11_SET_NW_SRC:
+ case OFPUTIL_OFPAT11_SET_NW_DST:
+ case OFPUTIL_OFPAT11_SET_NW_TOS:
+ case OFPUTIL_OFPAT11_SET_TP_SRC:
+ case OFPUTIL_OFPAT11_SET_TP_DST:
+ VLOG_WARN_RL(&rl, "Deprecated action %s received over %s",
+ ofputil_action_name_from_code(code),
+ ofputil_version_to_string(ofp_version));
+ }
+ }
+
switch (code) {
case OFPUTIL_ACTION_INVALID:
#define OFPAT10_ACTION(ENUM, STRUCT, NAME) case OFPUTIL_##ENUM:
@@ -942,13 +958,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. */
@@ -1160,13 +1169,14 @@ 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, struct ofpbuf *out,
+ enum ofp_version ofp_version)
{
enum ofperr error;
struct ofpact *a;
size_t start = out->size;
- error = ofpacts_from_openflow(in, n_in, out, ofpact_from_openflow11);
+ error = ofpacts_from_openflow(in, n_in, out, ofp_version);
if (error) {
return error;
}
@@ -1391,33 +1401,11 @@ get_actions_from_instruction(const struct
ofp11_instruction *inst,
*n_actions = (ntohs(inst->len) - sizeof *inst) / OFP11_INSTRUCTION_ALIGN;
}
-/* Attempts to convert 'actions_len' bytes of OpenFlow 1.1 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.
- *
- * 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,
- unsigned int actions_len,
- struct ofpbuf *ofpacts)
-{
- return ofpacts_pull_actions(openflow, actions_len, ofpacts,
- ofpacts_from_openflow11);
-}
-
enum ofperr
-ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
- unsigned int instructions_len,
- struct ofpbuf *ofpacts)
+ofpacts_pull_openflow_instructions(struct ofpbuf *openflow,
+ unsigned int instructions_len,
+ struct ofpbuf *ofpacts,
+ enum ofp_version ofp_version)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
const struct ofp11_instruction *instructions;
@@ -1466,7 +1454,8 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf
*openflow,
get_actions_from_instruction(insts[OVSINST_OFPIT11_APPLY_ACTIONS],
&actions, &n_actions);
- error = ofpacts_from_openflow11(actions, n_actions, ofpacts);
+ error = ofpacts_from_openflow(actions, n_actions, ofpacts,
+ ofp_version);
if (error) {
goto exit;
}
@@ -1487,7 +1476,7 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf
*openflow,
get_actions_from_instruction(insts[OVSINST_OFPIT11_WRITE_ACTIONS],
&actions, &n_actions);
error = ofpacts_from_openflow11_for_action_set(actions, n_actions,
- ofpacts);
+ ofpacts, ofp_version);
if (error) {
goto exit;
}
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 4e13472..b6868db 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -593,15 +593,14 @@ 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,
- unsigned int actions_len,
- struct ofpbuf *ofpacts);
-enum ofperr ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
- unsigned int instructions_len,
- struct ofpbuf *ofpacts);
+enum ofperr ofpacts_pull_openflow_actions(struct ofpbuf *openflow,
+ unsigned int actions_len,
+ struct ofpbuf *ofpacts,
+ enum ofp_version ofp_version);
+enum ofperr ofpacts_pull_openflow_instructions(struct ofpbuf *openflow,
+ unsigned int instructions_len,
+ struct ofpbuf *ofpacts,
+ enum ofp_version ofp_version);
enum ofperr ofpacts_check(struct ofpact[], size_t ofpacts_len,
struct flow *, ofp_port_t max_ports,
uint8_t table_id);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index ee701e9..879b384 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1504,7 +1504,8 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
return error;
}
- error = ofpacts_pull_openflow11_instructions(&b, b.size, ofpacts);
+ error = ofpacts_pull_openflow_instructions(&b, b.size, ofpacts,
+ oh->version);
if (error) {
return error;
}
@@ -1560,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, ofpacts,
+ oh->version);
if (error) {
return error;
}
@@ -1593,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, ofpacts,
+ oh->version);
if (error) {
return error;
}
@@ -2363,8 +2366,9 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats
*fs,
return EINVAL;
}
- if (ofpacts_pull_openflow11_instructions(msg, length - sizeof *ofs -
- padded_match_len, ofpacts)) {
+ if (ofpacts_pull_openflow_instructions(msg, length - sizeof *ofs -
+ padded_match_len, ofpacts,
+ oh->version)) {
VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions");
return EINVAL;
}
@@ -2407,7 +2411,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, ofpacts,
+ oh->version)) {
return EINVAL;
}
@@ -2447,7 +2452,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, ofpacts,
+ oh->version)) {
return EINVAL;
}
@@ -3095,8 +3101,8 @@ ofputil_decode_packet_out(struct ofputil_packet_out *po,
return error;
}
- error = ofpacts_pull_openflow11_actions(&b, ntohs(opo->actions_len),
- ofpacts);
+ error = ofpacts_pull_openflow_actions(&b, ntohs(opo->actions_len),
+ ofpacts, oh->version);
if (error) {
return error;
}
@@ -3107,7 +3113,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),
+ ofpacts, oh->version);
if (error) {
return error;
}
@@ -4179,6 +4186,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;
@@ -4193,6 +4201,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);
@@ -4241,7 +4251,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, ofpacts,
+ oh->version);
if (error) {
return error;
}
@@ -4745,22 +4756,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++) {
@@ -4771,6 +4781,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 < 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
* <ENUM> and length 'sizeof *action' and zeros the rest. For actions that
@@ -5681,7 +5700,7 @@ ofputil_append_group_desc_reply(const struct
ofputil_group_desc *gds,
static enum ofperr
ofputil_pull_buckets(struct ofpbuf *msg, size_t buckets_length,
- struct list *buckets)
+ struct list *buckets, enum ofp_version ofp_version)
{
struct ofp11_bucket *ob;
@@ -5714,8 +5733,8 @@ ofputil_pull_buckets(struct ofpbuf *msg, size_t
buckets_length,
buckets_length -= ob_len;
ofpbuf_init(&ofpacts, 0);
- error = ofpacts_pull_openflow11_actions(msg, ob_len - sizeof *ob,
- &ofpacts);
+ error = ofpacts_pull_openflow_actions(msg, ob_len - sizeof *ob,
+ &ofpacts, ofp_version);
if (error) {
ofpbuf_uninit(&ofpacts);
ofputil_bucket_list_destroy(buckets);
@@ -5755,6 +5774,7 @@ ofputil_decode_group_desc_reply(struct ofputil_group_desc
*gd,
{
struct ofp11_group_desc_stats *ogds;
size_t length;
+ struct ofp_header *oh;
if (!msg->l2) {
ofpraw_pull_assert(msg);
@@ -5763,6 +5783,7 @@ ofputil_decode_group_desc_reply(struct ofputil_group_desc
*gd,
if (!msg->size) {
return EOF;
}
+ oh = msg->l2;
ogds = ofpbuf_try_pull(msg, sizeof *ogds);
if (!ogds) {
@@ -5780,7 +5801,8 @@ ofputil_decode_group_desc_reply(struct ofputil_group_desc
*gd,
return OFPERR_OFPBRC_BAD_LEN;
}
- return ofputil_pull_buckets(msg, length - sizeof *ogds, &gd->buckets);
+ return ofputil_pull_buckets(msg, length - sizeof *ogds, &gd->buckets,
+ oh->version);
}
/* Converts abstract group mod 'gm' into a message for OpenFlow version
@@ -5864,7 +5886,8 @@ 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, msg.size, &gm->buckets);
+ return ofputil_pull_buckets(&msg, msg.size, &gm->buckets,
+ oh->version);
}
/* Parse a queue status request message into 'oqsr'.
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 17d14c0..83cd031 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -809,6 +809,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 5ec8f11..586fc04 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2784,7 +2784,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, &ofpacts,
+ OFP10_VERSION);
if (error) {
printf("bad OF1.1 actions: %s\n\n", ofperr_get_name(error));
ofpbuf_uninit(&ofpacts);
@@ -2971,8 +2972,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, of11_in.size,
- &ofpacts);
+ error = ofpacts_pull_openflow_actions(&of11_in, of11_in.size,
+ &ofpacts, OFP11_VERSION);
if (error) {
printf("bad OF1.1 actions: %s\n\n", ofperr_get_name(error));
ofpbuf_uninit(&ofpacts);
@@ -3041,8 +3042,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, of11_in.size,
- &ofpacts);
+ error = ofpacts_pull_openflow_instructions(&of11_in, of11_in.size,
+ &ofpacts, OFP11_VERSION);
if (!error) {
/* Verify actions. */
struct flow flow;
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
