[ovs-dev] [PATCH 1/2] ofp-actions: Waste less memory in learn actions.
Jarno Rajahalme
jarno at ovn.org
Wed Aug 24 00:51:36 UTC 2016
Make the immediate data member 'src_imm' of a learn spec allocated at
the end of the action for just the right size. This, together with
some structure packing saves on average of ~128 bytes for each learn
spec in each learn action. Typical learn actions have about 4 specs
each, so this amounts to saving about 0.5kb for each learn action.
Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
---
include/openvswitch/meta-flow.h | 14 ++++++++
include/openvswitch/ofp-actions.h | 42 +++++++++++++++--------
lib/learn.c | 72 ++++++++++++++++++++++-----------------
lib/meta-flow.c | 14 ++++++++
lib/ofp-actions.c | 26 +++++++++-----
5 files changed, 114 insertions(+), 54 deletions(-)
diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index 23f9916..966ff7f 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -19,6 +19,7 @@
#include <limits.h>
#include <stdarg.h>
+#include <string.h>
#include <sys/types.h>
#include <netinet/in.h>
#include <netinet/ip6.h>
@@ -2044,6 +2045,16 @@ int mf_subvalue_width(const union mf_subvalue *);
void mf_subvalue_shift(union mf_subvalue *, int n);
void mf_subvalue_format(const union mf_subvalue *, struct ds *);
+static inline void mf_subvalue_from_value(const struct mf_subfield *sf,
+ union mf_subvalue *sv,
+ const void *value)
+{
+ unsigned int n_bytes = DIV_ROUND_UP(sf->n_bits, 8);
+ memset(sv, 0, sizeof *sv - n_bytes);
+ memcpy(&sv->u8[sizeof sv->u8 - n_bytes], value, n_bytes);
+}
+
+
/* Set of field values. 'values' only includes the actual data bytes for each
* field for which is used, as marked by 1-bits in 'used'. */
struct field_array {
@@ -2115,6 +2126,9 @@ void mf_write_subfield_flow(const struct mf_subfield *,
const union mf_subvalue *, struct flow *);
void mf_write_subfield(const struct mf_subfield *, const union mf_subvalue *,
struct match *);
+void mf_write_subfield_value(const struct mf_subfield *, const void *src,
+ struct match *);
+
void mf_mask_subfield(const struct mf_field *,
const union mf_subvalue *value,
const union mf_subvalue *mask,
diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h
index 310ec33..b30b270 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -651,19 +651,6 @@ struct ofpact_resubmit {
uint8_t table_id;
};
-/* Part of struct ofpact_learn, below. */
-struct ofpact_learn_spec {
- int n_bits; /* Number of bits in source and dest. */
-
- int src_type; /* One of NX_LEARN_SRC_*. */
- struct mf_subfield src; /* NX_LEARN_SRC_FIELD only. */
- union mf_subvalue src_imm; /* NX_LEARN_SRC_IMMEDIATE only. */
-
- int dst_type; /* One of NX_LEARN_DST_*. */
- struct mf_subfield dst; /* NX_LEARN_DST_MATCH, NX_LEARN_DST_LOAD only. */
-};
-
-
/* Bits for 'flags' in struct nx_action_learn.
*
* If NX_LEARN_F_SEND_FLOW_REM is set, then the learned flows will have their
@@ -708,6 +695,33 @@ enum nx_learn_flags {
#define NX_LEARN_DST_RESERVED (3 << 11) /* Not yet defined. */
#define NX_LEARN_DST_MASK (3 << 11)
+/* Part of struct ofpact_learn, below. */
+struct ofpact_learn_spec {
+ struct mf_subfield src; /* NX_LEARN_SRC_FIELD only. */
+ struct mf_subfield dst; /* NX_LEARN_DST_MATCH, NX_LEARN_DST_LOAD only. */
+ uint16_t src_type; /* One of NX_LEARN_SRC_*. */
+ uint16_t dst_type; /* One of NX_LEARN_DST_*. */
+ uint8_t n_bits; /* Number of bits in source and dest. */
+ uint64_t src_imm[]; /* OFPACT_ALIGNTO (uint64_t) aligned. */
+};
+BUILD_ASSERT_DECL(offsetof(struct ofpact_learn_spec, src_imm)
+ % OFPACT_ALIGNTO == 0);
+BUILD_ASSERT_DECL(offsetof(struct ofpact_learn_spec, src_imm)
+ == sizeof(struct ofpact_learn_spec));
+
+static inline const struct ofpact_learn_spec *
+ofpact_learn_spec_next(const struct ofpact_learn_spec *spec)
+{
+ if (spec->src_type == NX_LEARN_SRC_IMMEDIATE) {
+ unsigned int n_uint64s
+ = OFPACT_ALIGN(DIV_ROUND_UP(spec->n_bits, 8)) / sizeof (uint64_t);
+ return (const struct ofpact_learn_spec *)
+ ((const uint64_t *)(spec + 1) + n_uint64s);
+ } else {
+ return spec + 1;
+ }
+}
+
/* OFPACT_LEARN.
*
* Used for NXAST_LEARN. */
@@ -718,8 +732,8 @@ struct ofpact_learn {
uint16_t hard_timeout; /* Max time before discarding (seconds). */
uint16_t priority; /* Priority level of flow entry. */
uint8_t table_id; /* Table to insert flow entry. */
- ovs_be64 cookie; /* Cookie for new flow. */
enum nx_learn_flags flags; /* NX_LEARN_F_*. */
+ ovs_be64 cookie; /* Cookie for new flow. */
uint16_t fin_idle_timeout; /* Idle timeout after FIN, if nonzero. */
uint16_t fin_hard_timeout; /* Hard timeout after FIN, if nonzero. */
diff --git a/lib/learn.c b/lib/learn.c
index a8469fa..563b034 100644
--- a/lib/learn.c
+++ b/lib/learn.c
@@ -41,7 +41,8 @@ learn_check(const struct ofpact_learn *learn, const struct flow *flow)
struct match match;
match_init_catchall(&match);
- for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; spec++) {
+ for (spec = learn->specs; spec < &learn->specs[learn->n_specs];
+ spec = ofpact_learn_spec_next(spec)) {
enum ofperr error;
/* Check the source. */
@@ -59,8 +60,7 @@ learn_check(const struct ofpact_learn *learn, const struct flow *flow)
if (error) {
return error;
}
-
- mf_write_subfield(&spec->dst, &spec->src_imm, &match);
+ mf_write_subfield_value(&spec->dst, spec->src_imm, &match);
break;
case NX_LEARN_DST_LOAD:
@@ -120,14 +120,15 @@ learn_execute(const struct ofpact_learn *learn, const struct flow *flow,
oft->fin_hard_timeout = learn->fin_hard_timeout;
}
- for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; spec++) {
+ for (spec = learn->specs; spec < &learn->specs[learn->n_specs];
+ spec = ofpact_learn_spec_next(spec)) {
struct ofpact_set_field *sf;
union mf_subvalue value;
if (spec->src_type == NX_LEARN_SRC_FIELD) {
mf_read_subfield(&spec->src, flow, &value);
} else {
- value = spec->src_imm;
+ mf_subvalue_from_value(&spec->dst, &value, spec->src_imm);
}
switch (spec->dst_type) {
@@ -175,7 +176,8 @@ learn_mask(const struct ofpact_learn *learn, struct flow_wildcards *wc)
union mf_subvalue value;
memset(&value, 0xff, sizeof value);
- for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; spec++) {
+ for (spec = learn->specs; spec < &learn->specs[learn->n_specs];
+ spec = ofpact_learn_spec_next(spec)) {
if (spec->src_type == NX_LEARN_SRC_FIELD) {
mf_write_subfield_flow(&spec->src, &value, &wc->masks);
}
@@ -185,7 +187,8 @@ learn_mask(const struct ofpact_learn *learn, struct flow_wildcards *wc)
/* Returns NULL if successful, otherwise a malloc()'d string describing the
* error. The caller is responsible for freeing the returned string. */
static char * OVS_WARN_UNUSED_RESULT
-learn_parse_load_immediate(const char *s, struct ofpact_learn_spec *spec)
+learn_parse_load_immediate(const char *s, struct ofpact_learn_spec *spec,
+ struct ofpbuf *ofpacts)
{
const char *full_s = s;
struct mf_subfield dst;
@@ -220,9 +223,14 @@ learn_parse_load_immediate(const char *s, struct ofpact_learn_spec *spec)
spec->n_bits = dst.n_bits;
spec->src_type = NX_LEARN_SRC_IMMEDIATE;
- spec->src_imm = imm;
spec->dst_type = NX_LEARN_DST_LOAD;
spec->dst = dst;
+
+ /* Push value last, as this may reallocate 'spec'! */
+ unsigned int n_bytes = DIV_ROUND_UP(dst.n_bits, 8);
+ uint8_t *src_imm = ofpbuf_put_zeros(ofpacts, OFPACT_ALIGN(n_bytes));
+ memcpy(src_imm, &imm.u8[sizeof imm.u8 - n_bytes], n_bytes);
+
return NULL;
}
@@ -230,7 +238,8 @@ learn_parse_load_immediate(const char *s, struct ofpact_learn_spec *spec)
* error. The caller is responsible for freeing the returned string. */
static char * OVS_WARN_UNUSED_RESULT
learn_parse_spec(const char *orig, char *name, char *value,
- struct ofpact_learn_spec *spec)
+ struct ofpact_learn_spec *spec,
+ struct ofpbuf *ofpacts, struct match *match)
{
if (mf_from_name(name)) {
const struct mf_field *dst = mf_from_name(name);
@@ -244,13 +253,19 @@ learn_parse_spec(const char *orig, char *name, char *value,
spec->n_bits = dst->n_bits;
spec->src_type = NX_LEARN_SRC_IMMEDIATE;
- memset(&spec->src_imm, 0, sizeof spec->src_imm);
- memcpy(&spec->src_imm.u8[sizeof spec->src_imm - dst->n_bytes],
- &imm, dst->n_bytes);
spec->dst_type = NX_LEARN_DST_MATCH;
spec->dst.field = dst;
spec->dst.ofs = 0;
spec->dst.n_bits = dst->n_bits;
+
+ /* Update 'match' to allow for satisfying destination
+ * prerequisites. */
+ mf_set_value(dst, &imm, match, NULL);
+
+ /* Push value last, as this may reallocate 'spec'! */
+ uint8_t *src_imm = ofpbuf_put_zeros(ofpacts,
+ OFPACT_ALIGN(dst->n_bytes));
+ memcpy(src_imm, &imm, dst->n_bytes);
} else if (strchr(name, '[')) {
/* Parse destination and check prerequisites. */
char *error;
@@ -284,7 +299,7 @@ learn_parse_spec(const char *orig, char *name, char *value,
spec->dst_type = NX_LEARN_DST_MATCH;
} else if (!strcmp(name, "load")) {
if (value[strcspn(value, "[-")] == '-') {
- char *error = learn_parse_load_immediate(value, spec);
+ char *error = learn_parse_load_immediate(value, spec, ofpacts);
if (error) {
return error;
}
@@ -363,20 +378,12 @@ learn_parse__(char *orig, char *arg, struct ofpbuf *ofpacts)
char *error;
spec = ofpbuf_put_zeros(ofpacts, sizeof *spec);
- learn = ofpacts->header;
- learn->n_specs++;
-
- error = learn_parse_spec(orig, name, value, spec);
+ error = learn_parse_spec(orig, name, value, spec, ofpacts, &match);
if (error) {
return error;
}
-
- /* Update 'match' to allow for satisfying destination
- * prerequisites. */
- if (spec->src_type == NX_LEARN_SRC_IMMEDIATE
- && spec->dst_type == NX_LEARN_DST_MATCH) {
- mf_write_subfield(&spec->dst, &spec->src_imm, &match);
- }
+ learn = ofpacts->header;
+ learn->n_specs++;
}
}
ofpact_finish_LEARN(ofpacts, &learn);
@@ -449,19 +456,20 @@ learn_format(const struct ofpact_learn *learn, struct ds *s)
colors.param, colors.end, ntohll(learn->cookie));
}
- for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; spec++) {
+ for (spec = learn->specs; spec < &learn->specs[learn->n_specs];
+ spec = ofpact_learn_spec_next(spec)) {
+ unsigned int n_bytes = DIV_ROUND_UP(spec->dst.n_bits, 8);
ds_put_char(s, ',');
switch (spec->src_type | spec->dst_type) {
- case NX_LEARN_SRC_IMMEDIATE | NX_LEARN_DST_MATCH:
+ case NX_LEARN_SRC_IMMEDIATE | NX_LEARN_DST_MATCH: {
if (spec->dst.ofs == 0
&& spec->dst.n_bits == spec->dst.field->n_bits) {
union mf_value value;
memset(&value, 0, sizeof value);
- bitwise_copy(&spec->src_imm, sizeof spec->src_imm, 0,
- &value, spec->dst.field->n_bytes, 0,
- spec->dst.field->n_bits);
+ memcpy(&value.b[spec->dst.field->n_bytes - n_bytes],
+ spec->src_imm, n_bytes);
ds_put_format(s, "%s%s=%s", colors.param,
spec->dst.field->name, colors.end);
mf_format(spec->dst.field, &value, NULL, s);
@@ -469,10 +477,10 @@ learn_format(const struct ofpact_learn *learn, struct ds *s)
ds_put_format(s, "%s", colors.param);
mf_format_subfield(&spec->dst, s);
ds_put_format(s, "=%s", colors.end);
- mf_format_subvalue(&spec->src_imm, s);
+ ds_put_hex(s, spec->src_imm, n_bytes);
}
break;
-
+ }
case NX_LEARN_SRC_FIELD | NX_LEARN_DST_MATCH:
ds_put_format(s, "%s", colors.param);
mf_format_subfield(&spec->dst, s);
@@ -486,7 +494,7 @@ learn_format(const struct ofpact_learn *learn, struct ds *s)
case NX_LEARN_SRC_IMMEDIATE | NX_LEARN_DST_LOAD:
ds_put_format(s, "%sload:%s", colors.special, colors.end);
- mf_format_subvalue(&spec->src_imm, s);
+ ds_put_hex(s, spec->src_imm, n_bytes);
ds_put_format(s, "%s->%s", colors.special, colors.end);
mf_format_subfield(&spec->dst, s);
break;
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 3dc2770..d07f927 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -2535,6 +2535,20 @@ mf_write_subfield(const struct mf_subfield *sf, const union mf_subvalue *x,
mf_set(field, &value, &mask, match, NULL);
}
+void
+mf_write_subfield_value(const struct mf_subfield *sf, const void *src,
+ struct match *match)
+{
+ const struct mf_field *field = sf->field;
+ union mf_value value, mask;
+ unsigned int size = DIV_ROUND_UP(sf->n_bits, 8);
+
+ mf_get(field, match, &value, &mask);
+ bitwise_copy(src, size, 0, &value, field->n_bytes, sf->ofs, sf->n_bits);
+ bitwise_one ( &mask, field->n_bytes, sf->ofs, sf->n_bits);
+ mf_set(field, &value, &mask, match, NULL);
+}
+
/* 'v' and 'm' correspond to values of 'field'. This function copies them into
* 'match' in the correspond positions. */
void
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 456b9cb..5b35edd 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -4293,15 +4293,16 @@ decode_NXAST_RAW_LEARN(const struct nx_action_learn *nal,
}
/* Get the source. */
+ const uint8_t *imm = NULL;
+ unsigned int imm_bytes = 0;
if (spec->src_type == NX_LEARN_SRC_FIELD) {
get_subfield(spec->n_bits, &p, &spec->src);
} else {
int p_bytes = 2 * DIV_ROUND_UP(spec->n_bits, 16);
-
- bitwise_copy(p, p_bytes, 0,
- &spec->src_imm, sizeof spec->src_imm, 0,
- spec->n_bits);
p = (const uint8_t *) p + p_bytes;
+
+ imm_bytes = DIV_ROUND_UP(spec->n_bits, 8);
+ imm = (const uint8_t *) p - imm_bytes;
}
/* Get the destination. */
@@ -4309,6 +4310,14 @@ decode_NXAST_RAW_LEARN(const struct nx_action_learn *nal,
spec->dst_type == NX_LEARN_DST_LOAD) {
get_subfield(spec->n_bits, &p, &spec->dst);
}
+
+ if (imm) {
+ uint8_t *src_imm = ofpbuf_put_zeros(ofpacts,
+ OFPACT_ALIGN(imm_bytes));
+ memcpy(src_imm, imm, imm_bytes);
+
+ learn = ofpacts->header;
+ }
}
ofpact_finish_LEARN(ofpacts, &learn);
@@ -4362,7 +4371,8 @@ encode_LEARN(const struct ofpact_learn *learn,
nal->flags = htons(learn->flags);
nal->table_id = learn->table_id;
- for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; spec++) {
+ for (spec = learn->specs; spec < &learn->specs[learn->n_specs];
+ spec = ofpact_learn_spec_next(spec)) {
put_u16(out, spec->n_bits | spec->dst_type | spec->src_type);
if (spec->src_type == NX_LEARN_SRC_FIELD) {
@@ -4371,9 +4381,9 @@ encode_LEARN(const struct ofpact_learn *learn,
} else {
size_t n_dst_bytes = 2 * DIV_ROUND_UP(spec->n_bits, 16);
uint8_t *bits = ofpbuf_put_zeros(out, n_dst_bytes);
- bitwise_copy(&spec->src_imm, sizeof spec->src_imm, 0,
- bits, n_dst_bytes, 0,
- spec->n_bits);
+ unsigned int n_bytes = DIV_ROUND_UP(spec->dst.n_bits, 8);
+
+ memcpy(bits + n_dst_bytes - n_bytes, spec->src_imm, n_bytes);
}
if (spec->dst_type == NX_LEARN_DST_MATCH ||
--
2.1.4
More information about the dev
mailing list