[ovs-dev,v2] bundle: add symmetric_l3 hash method for multipath

Message ID 1536735576-45741-1-git-send-email-martinxu9.ovs@gmail.com
State Changes Requested
Headers show
Series
  • [ovs-dev,v2] bundle: add symmetric_l3 hash method for multipath
Related show

Commit Message

Martin Xu Sept. 12, 2018, 6:59 a.m.
Add a symmetric_l3 hash method that uses both network destination
address and network source address.

VMware-BZ: #2112940

Signed-off-by: Martin Xu <martinxu9.ovs@gmail.com>
CC: Ben Pfaff <blp@ovn.org>
---
 include/openflow/nicira-ext.h |  4 +++-
 lib/bundle.c                  |  2 ++
 lib/flow.c                    | 44 ++++++++++++++++++++++++++++++++++++++++++-
 lib/flow.h                    |  1 +
 lib/multipath.c               |  2 ++
 utilities/ovs-ofctl.8.in      |  2 ++
 6 files changed, 53 insertions(+), 2 deletions(-)

Comments

Ben Pfaff Sept. 18, 2018, 8:33 a.m. | #1
On Tue, Sep 11, 2018 at 11:59:36PM -0700, Martin Xu wrote:
> Add a symmetric_l3 hash method that uses both network destination
> address and network source address.
> 
> VMware-BZ: #2112940
> 
> Signed-off-by: Martin Xu <martinxu9.ovs@gmail.com>
> CC: Ben Pfaff <blp@ovn.org>

Thanks for the patch.  It looks OK to me.  I have a few comments.

I think that this should be mentioned in NEWS.

In ovs-ofctl.8.in, I don't think "Network" needs to be capitalized, in
two places.

In the tests directory, grepping for "symmetric", I see a number of
places where we test related features.  It would be wise to add similar
tests for the new hashing method.

Thanks,

Ben.

Patch

diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index 1f5cbbf..dc12101 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -109,8 +109,10 @@  enum nx_hash_fields {
     NX_HASH_FIELDS_NW_SRC,
 
     /* Network destination address (NXM_OF_IP_DST) only. */
-    NX_HASH_FIELDS_NW_DST
+    NX_HASH_FIELDS_NW_DST,
 
+    /* Both network destination and source destination addresses. */
+    NX_HASH_FIELDS_SYMMETRIC_L3
 };
 
 /* NXT_PACKET_IN (analogous to OFPT_PACKET_IN).
diff --git a/lib/bundle.c b/lib/bundle.c
index edf6676..558e890 100644
--- a/lib/bundle.c
+++ b/lib/bundle.c
@@ -198,6 +198,8 @@  bundle_parse__(const char *s, const struct ofputil_port_map *port_map,
         bundle->fields = NX_HASH_FIELDS_NW_SRC;
     } else if (!strcasecmp(fields, "nw_dst")) {
         bundle->fields = NX_HASH_FIELDS_NW_DST;
+    } else if (!strcasecmp(fields, "symmetric_l3")) {
+        bundle->fields = NX_HASH_FIELDS_SYMMETRIC_L3;
     } else {
         return xasprintf("%s: unknown fields `%s'", s, fields);
     }
diff --git a/lib/flow.c b/lib/flow.c
index 77ed3d9..92eea51 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -2301,6 +2301,34 @@  flow_hash_symmetric_l3l4(const struct flow *flow, uint32_t basis,
     return hash_finish(hash, basis);
 }
 
+/* Hashes 'flow' based on its nw_dst and nw_src for multipath. */
+uint32_t
+flow_hash_symmetric_l3(const struct flow *flow, uint32_t basis)
+{
+    struct {
+        union {
+            ovs_be32 ipv4_addr;
+            struct in6_addr ipv6_addr;
+        };
+    } fields;
+
+    int i;
+
+    memset(&fields, 0, sizeof fields);
+    if (flow->dl_type == htons(ETH_TYPE_IP)) {
+        fields.ipv4_addr = flow->nw_src ^ flow->nw_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];
+        }
+    }
+    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)
@@ -2416,6 +2444,16 @@  flow_mask_hash_fields(const struct flow *flow, struct flow_wildcards *wc,
         }
         break;
 
+    case NX_HASH_FIELDS_SYMMETRIC_L3:
+        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);
+        }
+        break;
+
     default:
         OVS_NOT_REACHED();
     }
@@ -2458,6 +2496,8 @@  flow_hash_fields(const struct flow *flow, enum nx_hash_fields fields,
             return basis;
         }
 
+    case NX_HASH_FIELDS_SYMMETRIC_L3:
+        return flow_hash_symmetric_l3(flow, basis);
     }
 
     OVS_NOT_REACHED();
@@ -2474,6 +2514,7 @@  flow_hash_fields_to_str(enum nx_hash_fields fields)
     case NX_HASH_FIELDS_SYMMETRIC_L3L4_UDP: return "symmetric_l3l4+udp";
     case NX_HASH_FIELDS_NW_SRC: return "nw_src";
     case NX_HASH_FIELDS_NW_DST: return "nw_dst";
+    case NX_HASH_FIELDS_SYMMETRIC_L3: return "symmetric_l3";
     default: return "<unknown>";
     }
 }
@@ -2487,7 +2528,8 @@  flow_hash_fields_valid(enum nx_hash_fields fields)
         || fields == NX_HASH_FIELDS_SYMMETRIC_L3L4
         || fields == NX_HASH_FIELDS_SYMMETRIC_L3L4_UDP
         || fields == NX_HASH_FIELDS_NW_SRC
-        || fields == NX_HASH_FIELDS_NW_DST;
+        || fields == NX_HASH_FIELDS_NW_DST
+        || fields == NX_HASH_FIELDS_SYMMETRIC_L3;
 }
 
 /* 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 d03f1ba..1f6c1d6 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -241,6 +241,7 @@  uint32_t flow_hash_symmetric_l4(const struct flow *flow, uint32_t basis);
 uint32_t flow_hash_symmetric_l2(const struct flow *flow, uint32_t basis);
 uint32_t flow_hash_symmetric_l3l4(const struct flow *flow, uint32_t basis,
                          bool inc_udp_ports );
+uint32_t flow_hash_symmetric_l3(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 659c0bd..e6ba4c6 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -171,6 +171,8 @@  multipath_parse__(struct ofpact_multipath *mp, const char *s_, char *s)
         mp->fields = NX_HASH_FIELDS_NW_SRC;
     } else if (!strcasecmp(fields, "nw_dst")) {
         mp->fields = NX_HASH_FIELDS_NW_DST;
+    } else if (!strcasecmp(fields, "symmetric_l3")) {
+        mp->fields = NX_HASH_FIELDS_SYMMETRIC_L3;
     } else {
         return xasprintf("%s: unknown fields `%s'", s_, fields);
     }
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 4850f4a..e93ce59 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1421,6 +1421,8 @@  and IPv6, which hash to a constant zero.
 Like \fBsymmetric_l3l4+udp\fR, but UDP ports are included in the hash.
 This is a more effective hash when asymmetric UDP protocols such as
 VXLAN are not a consideration.
+.IP \fBsymmetric_l3\fR
+Hashes Network source address and Network destination address.
 .IP \fBnw_src\fR
 Hashes Network source address only.
 .IP \fBnw_dst\fR