[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openvswitch-dev
Subject:    [ovs-dev] Hashing: Add truly symmetric L3+L4 fields option for multipath and bundle hashing
From:       jvb127 () gmail ! com (Jeroen van Bemmel)
Date:       2015-06-28 18:43:20
Message-ID: CAOpSVYrGCC4ud1oxKY+8TPeL3y9jph-jgUkMJ5W9UN7BrBpzEw () mail ! gmail ! com
[Download RAW message or body]

The symmetric_l4 function implements a hash over various fields
including L2 fields such as ethernet source and destination MAC.
Inspite of its name, there are situations in which this hash does not
yield symmetric results ( e.g. when using VRRP, where the router
receives packets on a virtual MAC but responds from its physical MAC )

This patch adds a new 'symmetric_l3l4' function which is mostly an
intelligent copy&paste of existing code. It includes only L3 and L4
fields, including UDP and SCTP ports

I updated the documentation references that I could find, and tried
copying some of the symmetric_l3 test cases. However, 'make check'
wasn't working for me even without these patches, so I left out those
changes

Signed-off-by:Jeroen van Bemmel <jvb127 at gmail.com>
Suggested-by: Benn Pfaff <blp at nicira.com>

diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index 22325aa..225708b 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -109,7 +109,17 @@ enum nx_hash_fields {
      *  - NXM_OF_IP_SRC / NXM_OF_IP_DST
      *  - NXM_OF_TCP_SRC / NXM_OF_TCP_DST
      */
-    NX_HASH_FIELDS_SYMMETRIC_L4
+    NX_HASH_FIELDS_SYMMETRIC_L4,
+
+ /* L3+L4 only, including UDP ports:
+ *
+     *  - NXM_OF_IP_PROTO
+     *  - NXM_OF_IP_SRC / NXM_OF_IP_DST
+     *  - NXM_OF_TCP_SRC / NXM_OF_TCP_DST
+     *  - NXM_OF_UDP_SRC / NXM_OF_UDP_DST
+     *  - NXM_OF_SCTP_SRC / NXM_OF_SCTP_DST
+ */
+ NX_HASH_FIELDS_SYMMETRIC_L3L4
 };

 /* This command enables or disables an Open vSwitch extension that allows a
diff --git a/lib/bundle.c b/lib/bundle.c
index ee8079c..2d820b8 100644
--- a/lib/bundle.c
+++ b/lib/bundle.c
@@ -186,6 +186,8 @@ bundle_parse__(const char *s, char **save_ptr,
         bundle->fields = NX_HASH_FIELDS_ETH_SRC;
     } else if (!strcasecmp(fields, "symmetric_l4")) {
         bundle->fields = NX_HASH_FIELDS_SYMMETRIC_L4;
+    } else if (!strcasecmp(fields, "symmetric_l3l4")) {
+        bundle->fields = NX_HASH_FIELDS_SYMMETRIC_L3L4;
     } else {
         return xasprintf("%s: unknown fields `%s'", s, fields);
     }
diff --git a/lib/flow.c b/lib/flow.c
index 6bfe738..1edcb53 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1347,6 +1347,47 @@ flow_hash_symmetric_l4(const struct flow *flow,
uint32_t basis)
     return jhash_bytes(&fields, sizeof fields, basis);
 }

+/* Hashes 'flow' based on its L3 through L4 protocol information. */
+uint32_t
+flow_hash_symmetric_l3l4(const struct flow *flow, uint32_t basis)
+{
+    struct {
+        union {
+            ovs_be32 ipv4_addr;
+            struct in6_addr ipv6_addr;
+        };
+        ovs_be16 tp_port;
+        uint8_t ip_proto;
+    } fields;
+
+    int i;
+
+    memset(&fields, 0, sizeof fields);
+
+    /* UDP source and destination port are also taken into account */
+    if ( flow->dl_type == htons(ETH_TYPE_IP)) {
+        fields.ipv4_addr = flow->nw_src ^ flow->nw_dst;
+        fields.ip_proto = flow->nw_proto;
+        if (fields.ip_proto == IPPROTO_TCP || fields.ip_proto ==
IPPROTO_UDP || fields.ip_proto == IPPROTO_SCTP) {
+            fields.tp_port = flow->tp_src ^ flow->tp_dst;
+        }
+    } else if ( flow->dl_type == htons(ETH_TYPE_IPV6)) {
+        const uint8_t *a = &flow->ipv6_src.s6_addr[0];
+        const uint8_t *b = &flow->ipv6_dst.s6_addr[0];
+        uint8_t *ipv6_addr = &fields.ipv6_addr.s6_addr[0];
+
+        for (i=0; i<16; i++) {
+            ipv6_addr[i] = a[i] ^ b[i];
+        }
+        fields.ip_proto = flow->nw_proto;
+        if (fields.ip_proto == IPPROTO_TCP || fields.ip_proto ==
IPPROTO_UDP || fields.ip_proto == IPPROTO_SCTP) {
+            fields.tp_port = flow->tp_src ^ flow->tp_dst;
+        }
+    }
+    // Would be nice to use murmur hashing here...
+    return jhash_bytes(&fields, sizeof fields, basis);
+}
+
 /* Initialize a flow with random fields that matter for nx_hash_fields. */
 void
 flow_random_hash_fields(struct flow *flow)
@@ -1414,6 +1455,23 @@ flow_mask_hash_fields(const struct flow *flow,
struct flow_wildcards *wc,
         wc->masks.vlan_tci |= htons(VLAN_VID_MASK | VLAN_CFI);
         break;

+    case NX_HASH_FIELDS_SYMMETRIC_L3L4:
+            if (flow->dl_type == htons(ETH_TYPE_IP)) {
+                memset(&wc->masks.nw_src, 0xff, sizeof wc->masks.nw_src);
+                memset(&wc->masks.nw_dst, 0xff, sizeof wc->masks.nw_dst);
+            } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
+                memset(&wc->masks.ipv6_src, 0xff, sizeof wc->masks.ipv6_src);
+                memset(&wc->masks.ipv6_dst, 0xff, sizeof wc->masks.ipv6_dst);
+            }
+            if (is_ip_any(flow)) {
+                memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
+                if (( flow->nw_proto == IPPROTO_TCP )||(
flow->nw_proto == IPPROTO_UDP )||( flow->nw_proto == IPPROTO_SCTP )) {
+                    memset(&wc->masks.tp_src, 0xff, sizeof wc->masks.tp_src);
+                    memset(&wc->masks.tp_dst, 0xff, sizeof wc->masks.tp_dst);
+                }
+            }
+            break;
+
     default:
         OVS_NOT_REACHED();
     }
@@ -1431,6 +1489,9 @@ flow_hash_fields(const struct flow *flow, enum
nx_hash_fields fields,

     case NX_HASH_FIELDS_SYMMETRIC_L4:
         return flow_hash_symmetric_l4(flow, basis);
+
+    case NX_HASH_FIELDS_SYMMETRIC_L3L4:
+        return flow_hash_symmetric_l3l4(flow, basis);
     }

     OVS_NOT_REACHED();
@@ -1443,6 +1504,7 @@ flow_hash_fields_to_str(enum nx_hash_fields fields)
     switch (fields) {
     case NX_HASH_FIELDS_ETH_SRC: return "eth_src";
     case NX_HASH_FIELDS_SYMMETRIC_L4: return "symmetric_l4";
+    case NX_HASH_FIELDS_SYMMETRIC_L3L4: return "symmetric_l3l4";
     default: return "<unknown>";
     }
 }
@@ -1452,7 +1514,8 @@ bool
 flow_hash_fields_valid(enum nx_hash_fields fields)
 {
     return fields == NX_HASH_FIELDS_ETH_SRC
-        || fields == NX_HASH_FIELDS_SYMMETRIC_L4;
+        || fields == NX_HASH_FIELDS_SYMMETRIC_L4
+ || fields == NX_HASH_FIELDS_SYMMETRIC_L3L4;
 }

 /* Returns a hash value for the bits of 'flow' that are active based on
diff --git a/lib/flow.h b/lib/flow.h
index 384a031..856dfd0 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -341,6 +341,7 @@ bool flow_wildcards_equal(const struct flow_wildcards *,
                           const struct flow_wildcards *);
 uint32_t flow_hash_5tuple(const struct flow *flow, uint32_t basis);
 uint32_t flow_hash_symmetric_l4(const struct flow *flow, uint32_t basis);
+uint32_t flow_hash_symmetric_l3l4(const struct flow *flow, uint32_t basis);

 /* Initialize a flow with random fields that matter for nx_hash_fields. */
 void flow_random_hash_fields(struct flow *);
diff --git a/lib/multipath.c b/lib/multipath.c
index 355ef4b..8eefb41 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -162,6 +162,8 @@ multipath_parse__(struct ofpact_multipath *mp,
const char *s_, char *s)
         mp->fields = NX_HASH_FIELDS_ETH_SRC;
     } else if (!strcasecmp(fields, "symmetric_l4")) {
         mp->fields = NX_HASH_FIELDS_SYMMETRIC_L4;
+    } else if (!strcasecmp(fields, "symmetric_l3l4")) {
+        mp->fields = NX_HASH_FIELDS_SYMMETRIC_L3L4;
     } else {
         return xasprintf("%s: unknown fields `%s'", s_, fields);
     }
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 395d851..d484f26 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1607,8 +1607,8 @@ numbered 0 through \fIn_links\fR minus 1, and
stores the link into
 \fIdst\fB[\fIstart\fB..\fIend\fB]\fR, which must be an NXM field as
 described above.
 .IP
-Currently, \fIfields\fR must be either \fBeth_src\fR or
-\fBsymmetric_l4\fR and \fIalgorithm\fR must be one of \fBmodulo_n\fR,
+Currently, \fIfields\fR must be either \fBeth_src\fR, \fBsymmetric_l4\fR or
+\fBsymmetric_l3l4\fR and \fIalgorithm\fR must be one of \fBmodulo_n\fR,
 \fBhash_threshold\fR, \fBhrw\fR, and \fBiter_hash\fR.  Only
 the \fBiter_hash\fR algorithm uses \fIarg\fR.
 .IP
@@ -1621,7 +1621,7 @@ slaves represented as \fIslave_type\fR.
Currently the only supported
 \fIslave_type\fR is \fBofport\fR.  Thus, each \fIs1\fR through \fIsN\fR should
 be an OpenFlow port number. Outputs to the selected slave.
 .IP
-Currently, \fIfields\fR must be either \fBeth_src\fR or \fBsymmetric_l4\fR and
+Currently, \fIfields\fR must be either \fBeth_src\fR,
\fBsymmetric_l4\fR or \fBsymmetric_l3l4\fR and
 \fIalgorithm\fR must be one of \fBhrw\fR and \fBactive_backup\fR.
 .IP
 Example: \fBbundle(eth_src,0,hrw,ofport,slaves:4,8)\fR uses an Ethernet source

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic