Implement hash fields select group

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.horman@netronome.com>
[blp@nicira.com added a NEWS entry]
Signed-off-by: Ben Pfaff <blp@nicira.com>

NEWS

@@ -38,6 +38,9 @@ Post-v2.3.0
        is executed last, and only if the action set has no "output" or "group"
        action.
      * OpenFlow 1.4+ flow "importance" is now maintained in the flow table.
+     * A new Netronome extension to OpenFlow 1.5+ allows control over the
+       fields hashed for OpenFlow select groups.  See "selection_method" and
+       related options in ovs-ofctl(8) for details.
    - ovs-pki: Changed message digest algorithm from MD5 to SHA-1 because
      MD5 is no longer secure and some operating systems have started to disable
      it in OpenSSL.

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.

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)

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
+    if (strcmp("hash", prop->selection_method)) {
-     * matches a suported method. As no methods are currently supported
+        log_property(false, "ntr selection method '%s' is not supported",
-     * this check is a no-op that always fails. As selection methods are
+                     prop->selection_method);
-     * added they should be checked against the selection_method field
+        return OFPERR_OFPBPC_BAD_VALUE;
-     * here. */
+    }
-    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);

ofproto/ofproto-dpif-xlate.c

@@ -3116,6 +3116,71 @@ 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)
 {
@@ -3123,6 +3188,8 @@ xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *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();

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'.

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 *,

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
 

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])