diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h index 10e1f771ca0..d8c5dd868c5 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 17015206042..c44dd2a2db5 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 1fc738d8807..3efba3a9f02 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 2abf346a2b3..5ce64398982 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