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 <ja...@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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev