struct field_array is included in each ofgroup, but the current
implementation is very sparse, using more than 20kb of data.
Also loop over 1-bits instead of each and every MF type to make
processing faster.
Signed-off-by: Jarno Rajahalme <ja...@ovn.org>
---
include/openvswitch/meta-flow.h | 6 ++++--
include/openvswitch/ofp-util.h | 3 +++
lib/meta-flow.c | 23 +++++++++++++++++++-
lib/nx-match.c | 32 ++++++++++++++++-----------
lib/ofp-util.c | 16 ++++++++++++++
ofproto/ofproto-dpif-xlate.c | 48 +++++++++++++++++++++--------------------
ofproto/ofproto.c | 9 ++++++--
7 files changed, 96 insertions(+), 41 deletions(-)
diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index d8c5dd8..aaf7da1 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -2035,10 +2035,12 @@ 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 *);
-/* An array of fields with values */
+/* 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 {
struct mf_bitmap used;
- union mf_value value[MFF_N_IDS];
+ size_t values_size; /* Number of bytes currently in 'values'. */
+ uint8_t *values; /* Dynamically allocated to the correct size. */
};
/* Finding mf_fields. */
diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
index 7a5c905..fecdb43 100644
--- a/include/openvswitch/ofp-util.h
+++ b/include/openvswitch/ofp-util.h
@@ -1262,6 +1262,9 @@ struct ofputil_group_desc {
struct ofputil_group_props props; /* Group properties. */
};
+void ofputil_group_properties_destroy(struct ofputil_group_props *);
+void ofputil_group_properties_copy(struct ofputil_group_props *to,
+ const struct ofputil_group_props *from);
void ofputil_bucket_list_destroy(struct ovs_list *buckets);
void ofputil_bucket_clone_list(struct ovs_list *dest,
const struct ovs_list *src,
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index c44dd2a..2c89613 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -2515,7 +2515,28 @@ void
field_array_set(enum mf_field_id id, const union mf_value *value,
struct field_array *fa)
{
+ size_t i, offset = 0;
+
ovs_assert(id < MFF_N_IDS);
+
+ /* Find the spot for 'id'. */
+ BITMAP_FOR_EACH_1 (i, id, fa->used.bm) {
+ offset += mf_from_id(i)->n_bytes;
+ }
+
+ size_t value_size = mf_from_id(id)->n_bytes;
+
+ /* make room if necessary. */
+ if (!bitmap_is_set(fa->used.bm, id)) {
+ fa->values = xrealloc(fa->values, fa->values_size + value_size);
+ /* Move remainder forward, if any. */
+ if (offset < fa->values_size) {
+ memmove(fa->values + offset + value_size, fa->values + offset,
+ fa->values_size - offset);
+ }
+ fa->values_size += value_size;
+ }
bitmap_set1(fa->used.bm, id);
- fa->value[id] = *value;
+
+ memcpy(fa->values + offset, value, value_size);
}
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 297e361..7e324e4 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1178,12 +1178,15 @@ void
oxm_format_field_array(struct ds *ds, const struct field_array *fa)
{
size_t start_len = ds->length;
- int i;
+ size_t i, offset = 0;
- for (i = 0; i < MFF_N_IDS; i++) {
- if (bitmap_is_set(fa->used.bm, i)) {
- nx_format_mask_tlv(ds, i, &fa->value[i]);
- }
+ BITMAP_FOR_EACH_1 (i, MFF_N_IDS, fa->used.bm) {
+ const struct mf_field *mf = mf_from_id(i);
+ union mf_value value;
+
+ memcpy(&value, fa->values + offset, mf->n_bytes);
+ nx_format_mask_tlv(ds, i, &value);
+ offset += mf->n_bytes;
}
if (ds->length > start_len) {
@@ -1205,7 +1208,6 @@ oxm_put_field_array(struct ofpbuf *b, const struct
field_array *fa,
enum ofp_version version)
{
size_t start_len = b->size;
- int i;
/* Field arrays are only used with the group selection method
* property and group properties are only available in OpenFlow 1.5+.
@@ -1220,13 +1222,17 @@ oxm_put_field_array(struct ofpbuf *b, const struct
field_array *fa,
*/
ovs_assert(version >= OFP15_VERSION);
- for (i = 0; i < MFF_N_IDS; i++) {
- if (bitmap_is_set(fa->used.bm, i)) {
- int len = mf_field_len(mf_from_id(i), &fa->value[i], NULL, NULL);
- nxm_put__(b, i, version,
- &fa->value[i].u8 + mf_from_id(i)->n_bytes - len, NULL,
- len);
- }
+ size_t i, offset = 0;
+
+ BITMAP_FOR_EACH_1 (i, MFF_N_IDS, fa->used.bm) {
+ const struct mf_field *mf = mf_from_id(i);
+ union mf_value value;
+
+ memcpy(&value, fa->values + offset, mf->n_bytes);
+
+ int len = mf_field_len(mf, &value, NULL, NULL);
+ nxm_put__(b, i, version, &value + mf->n_bytes - len, NULL, len);
+ offset += mf->n_bytes;
}
return b->size - start_len;
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 175ae2d..c677ba4 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -8205,6 +8205,7 @@ void
ofputil_uninit_group_desc(struct ofputil_group_desc *gd)
{
ofputil_bucket_list_destroy(&gd->buckets);
+ ofputil_group_properties_destroy(&gd->props);
}
/* Decodes the OpenFlow group description request in 'oh', returning the group
@@ -8870,6 +8871,20 @@ ofputil_init_group_properties(struct ofputil_group_props
*gp)
memset(gp, 0, sizeof *gp);
}
+void
+ofputil_group_properties_copy(struct ofputil_group_props *to,
+ const struct ofputil_group_props *from)
+{
+ *to = *from;
+ to->fields.values = xmemdup(from->fields.values, from->fields.values_size);
+}
+
+void
+ofputil_group_properties_destroy(struct ofputil_group_props *gp)
+{
+ free(gp->fields.values);
+}
+
static enum ofperr
parse_group_prop_ntr_selection_method(struct ofpbuf *payload,
enum ofp11_group_type group_type,
@@ -9128,6 +9143,7 @@ void
ofputil_uninit_group_mod(struct ofputil_group_mod *gm)
{
ofputil_bucket_list_destroy(&gm->buckets);
+ ofputil_group_properties_destroy(&gm->props);
}
static struct ofpbuf *
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 3efba3a..0a1e2c4 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3492,39 +3492,41 @@ xlate_hash_fields_select_group(struct xlate_ctx *ctx,
struct group_dpif *group)
{
const struct field_array *fields;
struct ofputil_bucket *bucket;
+ const uint8_t *mask_values;
uint32_t basis;
- int i;
+ size_t i;
fields = group_dpif_get_fields(group);
+ mask_values = fields->values;
basis = hash_uint64(group_dpif_get_selection_method_param(group));
- for (i = 0; i < MFF_N_IDS; i++) {
- if (bitmap_is_set(fields->used.bm, i)) {
- const struct mf_field *mf = mf_from_id(i);
+ BITMAP_FOR_EACH_1 (i, MFF_N_IDS, fields->used.bm) {
+ const struct mf_field *mf = mf_from_id(i);
- /* Skip fields for which prerequisities are not met. */
- if (!mf_are_prereqs_ok(mf, &ctx->xin->flow, ctx->wc)) {
- continue;
- }
+ /* Skip fields for which prerequisities are not met. */
+ if (!mf_are_prereqs_ok(mf, &ctx->xin->flow, ctx->wc)) {
+ /* Skip the mask bytes for this field. */
+ mask_values += mf->n_bytes;
+ continue;
+ }
- union mf_value value;
- union mf_value mask;
+ union mf_value value;
+ union mf_value mask;
- mf_get_value(mf, &ctx->xin->flow, &value);
- /* This seems inefficient but so does apply_mask() */
- for (int j = 0; j < mf->n_bytes; j++) {
- mask.b[j] = fields->value[i].b[j];
- value.b[j] &= mask.b[j];
- }
- basis = hash_bytes(&value, mf->n_bytes, basis);
-
- /* For tunnels, hash in whether the field is present. */
- if (mf_is_tun_metadata(mf)) {
- basis = hash_boolean(mf_is_set(mf, &ctx->xin->flow), basis);
- }
+ mf_get_value(mf, &ctx->xin->flow, &value);
+ /* Mask the value. */
+ for (int j = 0; j < mf->n_bytes; j++) {
+ mask.b[j] = *mask_values++;
+ value.b[j] &= mask.b[j];
+ }
+ basis = hash_bytes(&value, mf->n_bytes, basis);
- mf_mask_field_masked(mf, &mask, ctx->wc);
+ /* For tunnels, hash in whether the field is present. */
+ if (mf_is_tun_metadata(mf)) {
+ basis = hash_boolean(mf_is_set(mf, &ctx->xin->flow), basis);
}
+
+ mf_mask_field_masked(mf, &mask, ctx->wc);
}
bucket = group_best_live_bucket(ctx, group, basis);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index cd0c90f..408e026 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2979,6 +2979,8 @@ static void
group_destroy_cb(struct ofgroup *group)
{
group->ofproto->ofproto_class->group_destruct(group);
+ ofputil_group_properties_destroy(CONST_CAST(struct ofputil_group_props *,
+ &group->props));
ofputil_bucket_list_destroy(&group->buckets);
group->ofproto->ofproto_class->group_dealloc(group);
}
@@ -6570,8 +6572,9 @@ init_group(struct ofproto *ofproto, const struct
ofputil_group_mod *gm,
*CONST_CAST(uint32_t *, &(*ofgroup)->n_buckets) =
ovs_list_size(&(*ofgroup)->buckets);
- memcpy(CONST_CAST(struct ofputil_group_props *, &(*ofgroup)->props),
- &gm->props, sizeof (struct ofputil_group_props));
+ ofputil_group_properties_copy(CONST_CAST(struct ofputil_group_props *,
+ &(*ofgroup)->props),
+ &gm->props);
rule_collection_init(&(*ofgroup)->rules);
/* Make group visible from 'version'. */
@@ -6581,6 +6584,8 @@ init_group(struct ofproto *ofproto, const struct
ofputil_group_mod *gm,
/* Construct called BEFORE any locks are held. */
error = ofproto->ofproto_class->group_construct(*ofgroup);
if (error) {
+ ofputil_group_properties_destroy(CONST_CAST(struct ofputil_group_props
*,
+ &(*ofgroup)->props));
ofputil_bucket_list_destroy(&(*ofgroup)->buckets);
ofproto->ofproto_class->group_dealloc(*ofgroup);
}
--
2.1.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
