This is intended as a usable demonstration of how
the NTR selection method extension might may be used.
NTR selection method
Signed-off-by: Simon Horman <simon.hor...@netronome.com>
---
v4
* Update group_dpif_get_fields and group_dpif_get_selection_method_param to
reflect that struct ofgroup now has a props field.
* Use all of selection_method_param rather than only lower 32 bits.
* Use hash_bytes() instead of jhash_bytes().
* Hash flow fields as needed rather than entire flow
* Add ofp-print tests now that a selection method is defined:
they were previously part of an earlier patch
* Only hash fields which are present with their pre-requisites in the flow
* Hash pre-requisites of fields
* Expand ofproto-dpif test to check that flows are not distributed
if they only vary over a field that is not hashed
v3
* Update new check in parse_group_prop_nmx_selection_method() to
allow decoding hash selection method
* Use fixed array for fields_array rather than constructing a list
* Use NTR instead of NMX as Netronome extension prefix
v2
* Use list of struct field_array of TLVs rather than OF1.1 match
for fields field of NTR selection method property
---
lib/meta-flow.c | 35 +++++++++++++++++++++++
lib/meta-flow.h | 2 ++
lib/ofp-util.c | 13 ++++-----
ofproto/ofproto-dpif-xlate.c | 67 ++++++++++++++++++++++++++++++++++++++++++++
ofproto/ofproto-dpif.c | 12 ++++++++
ofproto/ofproto-dpif.h | 2 ++
tests/ofp-print.at | 16 ++++++++---
tests/ofproto-dpif.at | 33 ++++++++++++++++++++++
8 files changed, 168 insertions(+), 12 deletions(-)
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 54e7f58..124b525 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -352,6 +352,41 @@ mf_mask_field_and_prereqs(const struct mf_field *mf,
struct flow *mask)
}
}
+/* 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/lib/meta-flow.h b/lib/meta-flow.h
index ba87aff..265f066 100644
--- a/lib/meta-flow.h
+++ b/lib/meta-flow.h
@@ -1601,6 +1601,8 @@ void mf_get_mask(const struct mf_field *, const struct
flow_wildcards *,
/* Prerequisites. */
bool mf_are_prereqs_ok(const struct mf_field *, const struct flow *);
void mf_mask_field_and_prereqs(const struct mf_field *, struct flow *mask);
+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/ofp-util.c b/lib/ofp-util.c
index 7b68564..a0da289 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -7819,14 +7819,11 @@ parse_group_prop_ntr_selection_method(struct ofpbuf
*payload,
return OFPERR_OFPBPC_BAD_VALUE;
}
- /* Only allow selection method property if the selection_method field
- * matches a suported method. As no methods are currently supported
- * this check is a no-op that always fails. As selection methods are
- * added they should be checked against the selection_method field
- * here. */
- log_property(false, "ntr selection method '%s' is not supported",
- prop->selection_method);
- return OFPERR_OFPBPC_BAD_VALUE;
+ if (strcmp("hash", prop->selection_method)) {
+ log_property(false, "ntr selection method '%s' is not supported",
+ prop->selection_method);
+ return OFPERR_OFPBPC_BAD_VALUE;
+ }
strcpy(gp->selection_method, prop->selection_method);
gp->selection_method_param = ntohll(prop->selection_method_param);
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 75d3eed..a7cfe06 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3117,12 +3117,79 @@ 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;
+ struct flow_wildcards *wc = &ctx->xout->wc;
+ const struct field_array *fields;
+ struct ofputil_bucket *bucket;
+ uint32_t basis;
+ int i;
+
+ 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);
+
+ /* Only hash a field if it and its pre-requisites are present
+ * in the flow. */
+ if (!mf_are_prereqs_ok(mf, &ctx->xin->flow)) {
+ 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;
+
+ 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];
+ }
+ basis = hash_bytes(&value, mf->n_bytes, basis);
+
+ mf_mask_field(mf, &wc->masks);
+ }
+ }
+
+ bucket = group_best_live_bucket(ctx, group, basis);
+ if (bucket) {
+ xlate_group_bucket(ctx, bucket);
+ xlate_group_stats(ctx, group, bucket);
+ }
+}
+
+static void
xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
{
const char *selection_method = group_dpif_get_selection_method(group);
if (selection_method[0] == '\0') {
xlate_default_select_group(ctx, group);
+ } else if (!strcasecmp("hash", selection_method)) {
+ xlate_hash_fields_select_group(ctx, group);
} else {
/* Parsing of groups should ensure this never happens */
OVS_NOT_REACHED();
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 43a21c2..d6a4d8c 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4265,6 +4265,18 @@ ofproto_dpif_send_packet(const struct ofport_dpif
*ofport, struct dp_packet *pac
ovs_mutex_unlock(&ofproto->stats_mutex);
return error;
}
+
+uint64_t
+group_dpif_get_selection_method_param(const struct group_dpif *group)
+{
+ return group->up.props.selection_method_param;
+}
+
+const struct field_array *
+group_dpif_get_fields(const struct group_dpif *group)
+{
+ return &group->up.props.fields;
+}
/* Return the version string of the datapath that backs up
* this 'ofproto'.
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index fd099a2..19c91f0 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -138,6 +138,8 @@ void group_dpif_get_buckets(const struct group_dpif *group,
const struct ovs_list **buckets);
enum ofp11_group_type group_dpif_get_type(const struct group_dpif *group);
const char *group_dpif_get_selection_method(const struct group_dpif *group);
+uint64_t group_dpif_get_selection_method_param(const struct group_dpif *group);
+const struct field_array *group_dpif_get_fields(const struct group_dpif
*group);
bool ofproto_has_vlan_splinters(const struct ofproto_dpif *);
ofp_port_t vsp_realdev_to_vlandev(const struct ofproto_dpif *,
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index dc809e2..2bef3fa 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -2026,7 +2026,7 @@ AT_CLEANUP
AT_SETUP([OFPST_GROUP_DESC reply - OF1.5])
AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
AT_CHECK([ovs-ofctl ofp-print "\
-06 13 00 98 00 00 00 02 00 07 00 00 00 00 00 00 \
+06 13 00 d8 00 00 00 02 00 07 00 00 00 00 00 00 \
00 88 01 00 00 00 20 00 00 78 00 00 00 00 00 00 \
00 28 00 10 00 00 00 00 00 00 00 10 00 00 00 01 \
00 00 00 00 00 00 00 00 00 00 00 08 00 64 00 00 \
@@ -2037,9 +2037,14 @@ AT_CHECK([ovs-ofctl ofp-print "\
00 28 00 10 00 00 00 02 00 00 00 10 00 00 00 03 \
00 00 00 00 00 00 00 00 00 00 00 08 00 c8 00 00 \
00 01 00 08 00 00 00 03 \
+ff ff 00 3b 00 00 15 40 00 00 00 01 00 00 00 00 \
+68 61 73 68 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 \
+80 00 18 04 ff ff ff 00 80 00 1a 02 ff ff 80 00 \
+14 01 ff 00 00 00 00 00 \
"], [0], [dnl
OFPST_GROUP_DESC reply (OF1.5) (xid=0x2):
-
group_id=8192,type=select,bucket=bucket_id:0,weight:100,watch_port:1,actions=output:1,bucket=bucket_id:1,weight:200,watch_port:2,actions=output:2,bucket=bucket_id:2,weight:200,watch_port:3,actions=output:3
+
group_id=8192,type=select,selection_method=hash,fields=ip_dst=255.255.255.0,nw_proto,tcp_src,bucket=bucket_id:0,weight:100,watch_port:1,actions=output:1,bucket=bucket_id:1,weight:200,watch_port:2,actions=output:2,bucket=bucket_id:2,weight:200,watch_port:3,actions=output:3
])
AT_CLEANUP
@@ -2845,7 +2850,7 @@ AT_CLEANUP
AT_SETUP([OFPT_GROUP_MOD add - OF1.5])
AT_KEYWORDS([ofp-print])
AT_CHECK([ovs-ofctl ofp-print "\
-06 0f 00 90 11 22 33 44 00 00 01 00 87 65 43 21 \
+06 0f 00 b8 11 22 33 44 00 00 01 00 87 65 43 21 \
00 78 00 00 ff ff ff ff 00 28 00 10 00 00 00 00 \
00 00 00 10 00 00 00 01 00 00 00 00 00 00 00 00 \
00 00 00 08 00 64 00 00 00 01 00 08 00 00 00 01 \
@@ -2854,9 +2859,12 @@ AT_CHECK([ovs-ofctl ofp-print "\
00 01 00 08 00 00 00 02 00 28 00 10 00 00 00 02 \
00 00 00 10 00 00 00 03 00 00 00 00 00 00 00 00 \
00 00 00 08 00 c8 00 00 00 01 00 08 00 00 00 03 \
+ff ff 00 28 00 00 15 40 00 00 00 01 00 00 00 00 \
+68 61 73 68 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 07 \
"], [0], [dnl
OFPT_GROUP_MOD (OF1.5) (xid=0x11223344):
- ADD
group_id=2271560481,type=select,bucket=bucket_id:0,weight:100,watch_port:1,actions=output:1,bucket=bucket_id:1,weight:200,watch_port:2,actions=output:2,bucket=bucket_id:2,weight:200,watch_port:3,actions=output:3
+ ADD
group_id=2271560481,type=select,selection_method=hash,selection_method_param=7,bucket=bucket_id:0,weight:100,watch_port:1,actions=output:1,bucket=bucket_id:1,weight:200,watch_port:2,actions=output:2,bucket=bucket_id:2,weight:200,watch_port:3,actions=output:3
])
AT_CLEANUP
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 897df4e..5752175 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -403,6 +403,39 @@ AT_CHECK([tail -1 stdout], [0],
OVS_VSWITCHD_STOP
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-flow br0 'ip
actions=write_actions(group:1234)'])
+
+# Try a bunch of different flows and make sure that they get distributed
+# at least somewhat.
+for d in 0 1 2 3 4 5 6 7 8 9 a b c d e f; do
+ AT_CHECK([ovs-appctl ofproto/trace br0
"in_port=1,dl_src=50:54:00:00:00:07,dl_dst=50:54:00:00:00:0$d,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=1,nw_tos=0,nw_ttl=128,icmp_type=8,icmp_code=0"],
[0], [stdout])
+ tail -1 stdout >> results
+done
+sort results | uniq -c
+AT_CHECK([sort results | uniq], [0],
+ [Datapath actions: 10
+Datapath actions: 11
+])
+
+> results
+# Try a bunch of different flows and make sure that they are not distributed
+# as they only vary a field that is not hashed
+for d in 0 1 2 3 4 5 6 7 8 9 a b c d e f; do
+ AT_CHECK([ovs-appctl ofproto/trace br0
"in_port=1,dl_src=50:54:00:00:00:$d,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=1,nw_tos=0,nw_ttl=128,icmp_type=8,icmp_code=0"],
[0], [stdout])
+ tail -1 stdout >> results
+done
+sort results | uniq -c
+AT_CHECK([sort results | uniq], [0],
+ [Datapath actions: 10
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([ofproto-dpif - fast failover group])
OVS_VSWITCHD_START
ADD_OF_PORTS([br0], [1], [10], [11])
--
2.1.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
