[ovs-dev] [PATCH v2 17/26] ofproto-dpif-xlate: Hash only fields specified for 'hash' selection method.
Jarno Rajahalme
jarno at ovn.org
Fri Jul 29 00:56:09 UTC 2016
The mask for non-present fields in struct field_array is always zero,
so hashing a prerequisite field that was not also specified for the
"hash" selection method boiled down to hashing a all-zeroes value and
unwildcarding the prerequisite field. Now that mf_are_prereqs_ok()
already takes care of unwildcarding, we can simplify the code by
hashing only the specified fields.
Also change the test case to include fields that have prerequisities.
Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
---
include/openvswitch/meta-flow.h | 2 --
lib/meta-flow.c | 36 ------------------------------------
ofproto/ofproto-dpif-xlate.c | 36 +++++++-----------------------------
tests/ofproto-dpif.at | 2 +-
4 files changed, 8 insertions(+), 68 deletions(-)
diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index 10e1f77..d8c5dd8 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -2062,8 +2062,6 @@ void mf_get_mask(const struct mf_field *, const struct flow_wildcards *,
/* Prerequisites. */
bool mf_are_prereqs_ok(const struct mf_field *mf, const struct flow *flow,
struct flow_wildcards *wc);
-void mf_bitmap_set_field_and_prereqs(const struct mf_field *mf, struct
- mf_bitmap *bm);
static inline bool
mf_is_l3_or_higher(const struct mf_field *mf)
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 1701520..c44dd2a 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -408,42 +408,6 @@ mf_are_prereqs_ok(const struct mf_field *mf, const struct flow *flow,
OVS_NOT_REACHED();
}
-/* Set bits of 'bm' corresponding to the field 'mf' and it's prerequisities. */
-void
-mf_bitmap_set_field_and_prereqs(const struct mf_field *mf, struct mf_bitmap *bm)
-{
- bitmap_set1(bm->bm, mf->id);
-
- switch (mf->prereqs) {
- case MFP_ND:
- case MFP_ND_SOLICIT:
- case MFP_ND_ADVERT:
- bitmap_set1(bm->bm, MFF_TCP_SRC);
- bitmap_set1(bm->bm, MFF_TCP_DST);
- /* Fall through. */
- case MFP_TCP:
- case MFP_UDP:
- case MFP_SCTP:
- case MFP_ICMPV4:
- case MFP_ICMPV6:
- /* nw_frag always unwildcarded. */
- bitmap_set1(bm->bm, MFF_IP_PROTO);
- /* Fall through. */
- case MFP_ARP:
- case MFP_IPV4:
- case MFP_IPV6:
- case MFP_MPLS:
- case MFP_IP_ANY:
- bitmap_set1(bm->bm, MFF_ETH_TYPE);
- break;
- case MFP_VLAN_VID:
- bitmap_set1(bm->bm, MFF_VLAN_TCI);
- break;
- case MFP_NONE:
- break;
- }
-}
-
/* Returns true if 'value' may be a valid value *as part of a masked match*,
* false otherwise.
*
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 1fc738d..3efba3a 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3490,7 +3490,6 @@ xlate_default_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
static void
xlate_hash_fields_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
{
- struct mf_bitmap hash_fields = MF_BITMAP_INITIALIZER;
const struct field_array *fields;
struct ofputil_bucket *bucket;
uint32_t basis;
@@ -3499,44 +3498,23 @@ xlate_hash_fields_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
fields = group_dpif_get_fields(group);
basis = hash_uint64(group_dpif_get_selection_method_param(group));
- /* Determine which fields to hash */
for (i = 0; i < MFF_N_IDS; i++) {
if (bitmap_is_set(fields->used.bm, i)) {
- const struct mf_field *mf;
-
- /* If the field is already present in 'hash_fields' then
- * this loop has already checked that it and its pre-requisites
- * are present in the flow and its pre-requisites have
- * already been added to 'hash_fields'. There is nothing more
- * to do here and as an optimisation the loop can continue. */
- if (bitmap_is_set(hash_fields.bm, i)) {
- continue;
- }
-
- mf = mf_from_id(i);
+ const struct mf_field *mf = mf_from_id(i);
- /* Only hash a field if it and its pre-requisites are present
- * in the flow. */
+ /* Skip fields for which prerequisities are not met. */
if (!mf_are_prereqs_ok(mf, &ctx->xin->flow, ctx->wc)) {
continue;
}
- /* Hash both the field and its pre-requisites */
- mf_bitmap_set_field_and_prereqs(mf, &hash_fields);
- }
- }
-
- /* Hash the fields */
- for (i = 0; i < MFF_N_IDS; i++) {
- if (bitmap_is_set(hash_fields.bm, i)) {
- const struct mf_field *mf = mf_from_id(i);
union mf_value value;
- int j;
+ union mf_value mask;
mf_get_value(mf, &ctx->xin->flow, &value);
/* This seems inefficient but so does apply_mask() */
- for (j = 0; j < mf->n_bytes; j++) {
- ((uint8_t *) &value)[j] &= ((uint8_t *) &fields->value[i])[j];
+ 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);
@@ -3545,7 +3523,7 @@ xlate_hash_fields_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
basis = hash_boolean(mf_is_set(mf, &ctx->xin->flow), basis);
}
- mf_mask_field(mf, ctx->wc);
+ mf_mask_field_masked(mf, &mask, ctx->wc);
}
}
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 19ff4ce..11e0748 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -418,7 +418,7 @@ AT_CLEANUP
AT_SETUP([ofproto-dpif - select group with hash selection method])
OVS_VSWITCHD_START
add_of_ports br0 1 10 11
-AT_CHECK([ovs-ofctl -O OpenFlow15 add-group br0 'group_id=1234,type=select,selection_method=hash,fields=eth_dst,bucket=output:10,bucket=output:11'])
+AT_CHECK([ovs-ofctl -O OpenFlow15 add-group br0 'group_id=1234,type=select,selection_method=hash,fields(eth_dst,ip_dst,tcp_dst),bucket=output:10,bucket=output:11'])
AT_CHECK([ovs-ofctl -O OpenFlow15 add-flow br0 'ip actions=write_actions(group:1234)'])
# Try a bunch of different flows and make sure that they get distributed
--
2.1.4
More information about the dev
mailing list