[ovs-dev] [PATCH 6/9] More compact ofpact_set_field.
Jarno Rajahalme
jrajahalme at nicira.com
Wed Oct 16 23:16:08 UTC 2013
Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
lib/meta-flow.h | 2 +-
lib/ofp-actions.c | 44 +++++++++++++++++++++++++++-----------
lib/ofp-actions.h | 48 +++++++++++++++++++++++++++++++++++++-----
lib/ofp-parse.c | 14 ++++++------
lib/ofp-util.c | 12 +++++------
ofproto/ofproto-dpif-xlate.c | 5 +++--
6 files changed, 91 insertions(+), 34 deletions(-)
diff --git a/lib/meta-flow.h b/lib/meta-flow.h
index 03d7794..29dbe48 100644
--- a/lib/meta-flow.h
+++ b/lib/meta-flow.h
@@ -31,7 +31,7 @@ struct match;
/* The comment on each of these indicates the member in "union mf_value" used
* to represent its value. */
-enum mf_field_id {
+enum OVS_PACKED_ENUM mf_field_id {
/* Metadata. */
MFF_TUN_ID, /* be64 */
MFF_TUN_SRC, /* be32 */
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index c666069..8c9dd8c 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -744,7 +744,6 @@ oxm_set_field_from_openflow12(
uint8_t oxm_length = NXM_LENGTH(oxm_header);
struct ofpact_set_field *sf;
const struct mf_field *mf;
- size_t prune;
/* ofp12_action_set_field is padded to 64 bits by zero */
if (oasf_len != ROUND_UP(sizeof(*oasf) + oxm_length, 8)) {
@@ -770,15 +769,9 @@ oxm_set_field_from_openflow12(
return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
}
- sf = ofpact_put_SET_FIELD(ofpacts);
- sf->field = mf->id;
+ sf = ofpact_set_field_from_be_value(mf, oasf + 1, ofpacts);
- memcpy(&sf->value.u8, oasf + 1, mf->n_bytes);
- prune = sizeof(union mf_value) - mf->n_bytes;
- sf->ofpact.len -= prune;
- ofpacts->size -= ROUND_DOWN(prune, OFPACT_ALIGNTO);
-
- if (!mf_is_value_valid(mf, &sf->value)) {
+ if (!mf_is_value_valid(mf, ofpact_set_field_get_const_value(sf))) {
return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
}
return 0;
@@ -1582,7 +1575,7 @@ ofpact_check__(const struct ofpact *a, struct flow *flow, ofp_port_t max_ports,
return nxm_reg_load_check(ofpact_get_REG_LOAD(a), flow);
case OFPACT_SET_FIELD:
- mf = mf_from_id(ofpact_get_SET_FIELD(a)->field);
+ mf = ofpact_set_field_get_field(ofpact_get_SET_FIELD(a));
if (!mf_are_prereqs_ok(mf, flow)) {
VLOG_WARN_RL(&rl, "set_field %s lacks correct prerequisites",
mf->name);
@@ -2717,9 +2710,9 @@ ofpact_format(const struct ofpact *a, struct ds *s)
case OFPACT_SET_FIELD:
set_field = ofpact_get_SET_FIELD(a);
- mf = mf_from_id(set_field->field);
+ mf = ofpact_set_field_get_field(set_field);
ds_put_format(s, "set_field:");
- mf_format(mf, &set_field->value, NULL, s);
+ mf_format(mf, ofpact_set_field_get_const_value(set_field), NULL, s);
ds_put_format(s, "->%s", mf->name);
break;
@@ -2940,3 +2933,30 @@ ofpact_pad(struct ofpbuf *ofpacts)
ofpbuf_put_zeros(ofpacts, OFPACT_ALIGNTO - rem);
}
}
+
+/* Depends on the network byte order 'value' having the size 'mf->n_bytes'. */
+struct ofpact_set_field *
+ofpact_set_field_from_be_value(const struct mf_field *mf, const void *value,
+ struct ofpbuf *ofpacts)
+{
+ struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
+ size_t offset;
+
+ /* Set the field type. */
+ sf->ofpact.field = mf->id;
+
+ /* Get the data offset depending on the data size. */
+ offset = (mf->n_bytes <= 4)
+ ? offsetof(struct ofpact_set_field, be32)
+ : offsetof(struct ofpact_set_field, be64);
+
+ /* Trim the ofpact of extra trailing space. */
+ sf->ofpact.len = offset + mf->n_bytes;
+ ofpacts->size -= ROUND_DOWN(sizeof *sf - sf->ofpact.len, OFPACT_ALIGNTO);
+
+ /* Copy the value. */
+ memcpy((char *)sf + offset, value, mf->n_bytes);
+
+ /* Any padding left was cleared by ofpacts_put_SET_FIELD(). */
+ return sf;
+}
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 128e2ab..b97d14b 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -154,7 +154,10 @@ enum {
*/
struct ofpact {
enum ofpact_type type; /* OFPACT_*. */
- enum ofputil_action_code compat; /* Original type when added, if any. */
+ union {
+ enum ofputil_action_code compat; /* Orig. type when added, if any. */
+ enum mf_field_id field; /* Target field, used by set field only. */
+ };
uint16_t len; /* Length of the action, in bytes, including
* struct ofpact, excluding padding. */
};
@@ -355,11 +358,46 @@ struct ofpact_reg_load {
* never referenced. 'ofpact.len' tells the correct size, and is never
* less than needed for the field in question. */
struct ofpact_set_field {
- struct ofpact ofpact;
- enum mf_field_id field;
- union mf_value value; /* Most-significant bits are used, the rest
- * might not be here. */
+ struct ofpact ofpact; /* has enum mf_field_id field */
+ union {
+ uint8_t u8;
+ ovs_be16 be16;
+ ovs_be32 be32;
+ };
+ union {
+ uint8_t mac[ETH_ADDR_LEN];
+ ovs_be64 be64;
+ struct in6_addr ipv6;
+ };
};
+#ifdef __GNUC__
+BUILD_ASSERT_DECL(sizeof(struct ofpact_set_field) == 24);
+#endif
+
+static inline const struct mf_field *
+ofpact_set_field_get_field(const struct ofpact_set_field *sf)
+{
+ return mf_from_id(sf->ofpact.field);
+}
+
+static inline enum mf_field_id
+ofpact_set_field_get_field_id(const struct ofpact_set_field *sf)
+{
+ return sf->ofpact.field;
+}
+
+/* Return the data pointer for a properly trimmed set field. */
+static inline const void *
+ofpact_set_field_get_const_value(const struct ofpact_set_field *sf)
+{
+ return (const char *)sf + sf->ofpact.len
+ - ofpact_set_field_get_field(sf)->n_bytes;
+}
+
+struct ofpact_set_field *
+ofpact_set_field_from_be_value(const struct mf_field *,
+ const void *value,
+ struct ofpbuf *ofpacts);
/* OFPACT_PUSH_VLAN/MPLS/PBB
*
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index bf7f56f..e06d1dd 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -464,13 +464,13 @@ static char * WARN_UNUSED_RESULT
set_field_parse__(char *arg, struct ofpbuf *ofpacts,
enum ofputil_protocol *usable_protocols)
{
- struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
+ struct ofpact_set_field *sf;
char *value;
char *delim;
char *key;
const struct mf_field *mf;
char *error;
- size_t prune;
+ union mf_value mf_value;
value = arg;
delim = strstr(arg, "->");
@@ -489,17 +489,15 @@ set_field_parse__(char *arg, struct ofpbuf *ofpacts,
if (!mf->writable) {
return xasprintf("%s is read-only", key);
}
- sf->field = mf->id;
delim[0] = '\0';
- error = mf_parse_value(mf, value, &sf->value);
+ error = mf_parse_value(mf, value, &mf_value);
if (error) {
return error;
}
- prune = sizeof(union mf_value) - mf->n_bytes;
- sf->ofpact.len -= prune;
- ofpacts->size -= ROUND_DOWN(prune, OFPACT_ALIGNTO);
- if (!mf_is_value_valid(mf, &sf->value)) {
+ sf = ofpact_set_field_from_be_value(mf, &mf_value, ofpacts);
+
+ if (!mf_is_value_valid(mf, ofpact_set_field_get_const_value(sf))) {
return xasprintf("%s is not a valid value for field %s", value, key);
}
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index f689d1d..c9d42a9 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -6137,7 +6137,7 @@ static void
set_field_to_ofast(const struct ofpact_set_field *sf,
struct ofpbuf *openflow)
{
- const struct mf_field *mf = mf_from_id(sf->field);
+ const struct mf_field *mf = ofpact_set_field_get_field(sf);
uint16_t padded_value_len = ROUND_UP(mf->n_bytes, 8);
struct ofp12_action_set_field *oasf;
char *value;
@@ -6149,7 +6149,7 @@ set_field_to_ofast(const struct ofpact_set_field *sf,
oasf->len = htons(ntohs(oasf->len) + padded_value_len);
value = ofpbuf_put_zeros(openflow, padded_value_len);
- memcpy(value, &sf->value, mf->n_bytes);
+ memcpy(value, ofpact_set_field_get_const_value(sf), mf->n_bytes);
}
void
@@ -6165,7 +6165,7 @@ ofputil_set_field_to_openflow(const struct ofpact_set_field *sf,
return;
}
- mf = mf_from_id(sf->field);
+ mf = ofpact_set_field_get_field(sf);
/* Convert to one or two REG_LOADs */
@@ -6176,18 +6176,18 @@ ofputil_set_field_to_openflow(const struct ofpact_set_field *sf,
narl = ofputil_put_NXAST_REG_LOAD(openflow);
narl->ofs_nbits = nxm_encode_ofs_nbits(0, 64);
narl->dst = htonl(mf->nxm_header);
- narl->value = *(&sf->value.be64 + 1);
+ narl->value = *(&sf->be64 + 1);
/* Higher bits next. */
narl = ofputil_put_NXAST_REG_LOAD(openflow);
narl->ofs_nbits = nxm_encode_ofs_nbits(64, mf->n_bits - 64);
narl->dst = htonl(mf->nxm_header);
- narl->value = sf->value.be64;
+ narl->value = sf->be64;
} else {
narl = ofputil_put_NXAST_REG_LOAD(openflow);
narl->ofs_nbits = nxm_encode_ofs_nbits(0, mf->n_bits);
narl->dst = htonl(mf->nxm_header);
memset(&narl->value, 0, 8 - mf->n_bytes);
memcpy((char*)&narl->value + (8 - mf->n_bytes),
- &sf->value, mf->n_bytes);
+ ofpact_set_field_get_const_value(sf), mf->n_bytes);
}
}
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 65ee8e2..d10c601 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2425,9 +2425,10 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
case OFPACT_SET_FIELD:
set_field = ofpact_get_SET_FIELD(a);
- mf = mf_from_id(set_field->field);
+ mf = ofpact_set_field_get_field(set_field);
mf_mask_field_and_prereqs(mf, &wc->masks);
- mf_set_flow_value(mf, &set_field->value, flow);
+ mf_set_flow_value(mf, ofpact_set_field_get_const_value(set_field),
+ flow);
break;
case OFPACT_STACK_PUSH:
--
1.7.10.4
More information about the dev
mailing list