lib/flow: Simplify miniflow accessors, add ipv6 support.

Add new macro MINIFLOW_MAP(FIELD) that returns the map covering the
given struct flow field.

Change the miniflow accessors to macros so that they can take the
field name directly.

Use these to add ipv6 support to miniflow_hash_5tuple().

Add ipv6 support to flow_hash_5tuple() as well so that these two
functions continue to return the same hash value for the corresponding
flows.

Also, simplify miniflow_get_metadata().

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>

diff --git a/lib/flow.c b/lib/flow.c
index 093dec8..9700d29 100644 (file)
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -355,7 +355,7 @@ flow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
 }
 
 /* Caller is responsible for initializing 'dst->values' with enough storage
- * (FLOW_U64S * 8 bytes is enough). */
+ * for FLOW_U32S * 4 bytes. */
 void
 miniflow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
                  struct miniflow *dst)
@@ -943,50 +943,76 @@ flow_wildcards_set_reg_mask(struct flow_wildcards *wc, int idx, uint32_t mask)
     wc->masks.regs[idx] = mask;
 }
 
-/* Calculates the 5-tuple hash from the given flow. */
+/* Calculates the 5-tuple hash from the given miniflow.
+ * This returns the same value as flow_hash_5tuple for the corresponding
+ * flow. */
 uint32_t
 miniflow_hash_5tuple(const struct miniflow *flow, uint32_t basis)
 {
-    uint32_t hash = 0;
+    uint32_t hash = basis;
 
-    if (!flow) {
-        return 0;
-    }
+    if (flow) {
+        ovs_be16 dl_type = MINIFLOW_GET_BE16(flow, dl_type);
+
+        hash = mhash_add(hash, MINIFLOW_GET_U8(flow, nw_proto));
 
-    hash = mhash_add(basis,
-                     miniflow_get_u32(flow, offsetof(struct flow, nw_src)));
-    hash = mhash_add(hash,
-                     miniflow_get_u32(flow, offsetof(struct flow, nw_dst)));
-    hash = mhash_add(hash,
-                     miniflow_get_u32(flow, offsetof(struct flow, tp_src)));
-    hash = mhash_add(hash,
-                     miniflow_get_u8(flow, offsetof(struct flow, nw_proto)));
+        /* Separate loops for better optimization. */
+        if (dl_type == htons(ETH_TYPE_IPV6)) {
+            uint64_t map = MINIFLOW_MAP(ipv6_src) | MINIFLOW_MAP(ipv6_dst)
+                | MINIFLOW_MAP(tp_src); /* Covers both ports */
+            uint32_t value;
 
-    return mhash_finish(hash, 13);
+            MINIFLOW_FOR_EACH_IN_MAP(value, flow, map) {
+                hash = mhash_add(hash, value);
+            }
+        } else {
+            uint64_t map = MINIFLOW_MAP(nw_src) | MINIFLOW_MAP(nw_dst)
+                | MINIFLOW_MAP(tp_src); /* Covers both ports */
+            uint32_t value;
+
+            MINIFLOW_FOR_EACH_IN_MAP(value, flow, map) {
+                hash = mhash_add(hash, value);
+            }
+        }
+        hash = mhash_finish(hash, 42); /* Arbitrary number. */
+    }
+    return hash;
 }
 
 BUILD_ASSERT_DECL(offsetof(struct flow, tp_src) + 2
                   == offsetof(struct flow, tp_dst) &&
                   offsetof(struct flow, tp_src) / 4
                   == offsetof(struct flow, tp_dst) / 4);
+BUILD_ASSERT_DECL(offsetof(struct flow, ipv6_src) + 16
+                  == offsetof(struct flow, ipv6_dst));
 
 /* Calculates the 5-tuple hash from the given flow. */
 uint32_t
 flow_hash_5tuple(const struct flow *flow, uint32_t basis)
 {
-    const uint32_t *flow_u32 = (const uint32_t *)flow;
-    uint32_t hash = 0;
+    uint32_t hash = basis;
 
-    if (!flow) {
-        return 0;
-    }
+    if (flow) {
+        const uint32_t *flow_u32 = (const uint32_t *)flow;
+
+        hash = mhash_add(hash, flow->nw_proto);
 
-    hash = mhash_add(basis, (OVS_FORCE uint32_t) flow->nw_src);
-    hash = mhash_add(hash, (OVS_FORCE uint32_t) flow->nw_dst);
-    hash = mhash_add(hash, flow_u32[offsetof(struct flow, tp_src) / 4]);
-    hash = mhash_add(hash, flow->nw_proto);
+        if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
+            int ofs = offsetof(struct flow, ipv6_src) / 4;
+            int end = ofs + 2 * sizeof flow->ipv6_src / 4;
 
-    return mhash_finish(hash, 13);
+            while (ofs < end) {
+                hash = mhash_add(hash, flow_u32[ofs++]);
+            }
+        } else {
+            hash = mhash_add(hash, (OVS_FORCE uint32_t) flow->nw_src);
+            hash = mhash_add(hash, (OVS_FORCE uint32_t) flow->nw_dst);
+        }
+        hash = mhash_add(hash, flow_u32[offsetof(struct flow, tp_src) / 4]);
+
+        hash = mhash_finish(hash, 42); /* Arbitrary number. */
+    }
+    return hash;
 }
 
 /* Hashes 'flow' based on its L2 through L4 protocol information. */

diff --git a/lib/flow.h b/lib/flow.h
index e63e4e2..02a5e18 100644 (file)
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -402,6 +402,13 @@ void miniflow_destroy(struct miniflow *);
 
 void miniflow_expand(const struct miniflow *, struct flow *);
 
+#define FLOW_U32_SIZE(FIELD)                                            \
+    DIV_ROUND_UP(sizeof(((struct flow *)0)->FIELD), sizeof(uint32_t))
+
+#define MINIFLOW_MAP(FIELD)                       \
+    (((UINT64_C(1) << FLOW_U32_SIZE(FIELD)) - 1)  \
+     << (offsetof(struct flow, FIELD) / 4))
+
 static inline uint32_t
 mf_get_next_in_map(uint64_t *fmap, uint64_t rm1bit, const uint32_t **fp,
                    uint32_t *value)
@@ -428,23 +435,30 @@ mf_get_next_in_map(uint64_t *fmap, uint64_t rm1bit, const uint32_t **fp,
          mf_get_next_in_map(&fmap_, rm1bit_, &fp_, &(VALUE));           \
          map_ -= rm1bit_, rm1bit_ = rightmost_1bit(map_))
 
-/* These accessors use byte offsets, which are assumed to be compile-time
- * constants. */
-static inline uint8_t miniflow_get_u8(const struct miniflow *,
-                                       unsigned int ofs);
-static inline uint16_t miniflow_get_u16(const struct miniflow *,
-                                        unsigned int ofs);
-static inline ovs_be16 miniflow_get_be16(const struct miniflow *,
-                                         unsigned int ofs);
-static inline uint32_t miniflow_get_u32(const struct miniflow *,
-                                        unsigned int ofs);
-static inline ovs_be32 miniflow_get_be32(const struct miniflow *,
-                                         unsigned int ofs);
+/* Get the value of 'FIELD' of an up to 4 byte wide integer type 'TYPE' of
+ * a miniflow. */
+#define MINIFLOW_GET_TYPE(MF, TYPE, OFS)                                \
+    (((MF)->map & (UINT64_C(1) << (OFS) / 4))                           \
+     ? ((OVS_FORCE const TYPE *)                                        \
+        ((MF)->values                                                   \
+         + count_1bits((MF)->map & ((UINT64_C(1) << (OFS) / 4) - 1))))  \
+       [(OFS) % 4 / sizeof(TYPE)]                                       \
+     : 0)                                                               \
+
+#define MINIFLOW_GET_U8(FLOW, FIELD)                                    \
+    MINIFLOW_GET_TYPE(FLOW, uint8_t, offsetof(struct flow, FIELD))
+#define MINIFLOW_GET_U16(FLOW, FIELD)                                    \
+    MINIFLOW_GET_TYPE(FLOW, uint16_t, offsetof(struct flow, FIELD))
+#define MINIFLOW_GET_BE16(FLOW, FIELD)                                    \
+    MINIFLOW_GET_TYPE(FLOW, ovs_be16, offsetof(struct flow, FIELD))
+#define MINIFLOW_GET_U32(FLOW, FIELD)                                    \
+    MINIFLOW_GET_TYPE(FLOW, uint32_t, offsetof(struct flow, FIELD))
+#define MINIFLOW_GET_BE32(FLOW, FIELD)                                    \
+    MINIFLOW_GET_TYPE(FLOW, ovs_be32, offsetof(struct flow, FIELD))
 
 static inline uint16_t miniflow_get_vid(const struct miniflow *);
 static inline uint16_t miniflow_get_tcp_flags(const struct miniflow *);
 static inline ovs_be64 miniflow_get_metadata(const struct miniflow *);
-static inline uint8_t miniflow_get_u8(const struct miniflow *, unsigned int ofs);
 
 bool miniflow_equal(const struct miniflow *a, const struct miniflow *b);
 bool miniflow_equal_in_minimask(const struct miniflow *a,
@@ -495,54 +509,12 @@ bool minimask_has_extra(const struct minimask *, const struct minimask *);
 bool minimask_is_catchall(const struct minimask *);
 \f
 
-/* 'OFS' is a compile-time constant. */
-#define MINIFLOW_GET_TYPE(MF, TYPE, OFS)                                \
-    (MF->map & UINT64_C(1) << OFS / 4)                                  \
-    ? ((OVS_FORCE const TYPE *)                                         \
-       (MF->values + count_1bits(MF->map & ((UINT64_C(1) << OFS / 4) - 1)))) \
-      [OFS % 4 / sizeof(TYPE)]                                          \
-    : 0
-
-static inline uint8_t
-miniflow_get_u8(const struct miniflow *flow, unsigned int ofs)
-{
-    return MINIFLOW_GET_TYPE(flow, uint8_t, ofs);
-}
-
-static inline uint16_t
-miniflow_get_u16(const struct miniflow *flow, unsigned int ofs)
-{
-    return MINIFLOW_GET_TYPE(flow, uint16_t, ofs);
-}
-
-/* Returns the ovs_be16 that would be at byte offset 'u8_ofs' if 'flow' were
- * expanded into a "struct flow". */
-static inline ovs_be16
-miniflow_get_be16(const struct miniflow *flow, unsigned int ofs)
-{
-    return MINIFLOW_GET_TYPE(flow, ovs_be16, ofs);
-}
-
-static inline uint32_t
-miniflow_get_u32(const struct miniflow *flow, unsigned int ofs)
-{
-    return MINIFLOW_GET_TYPE(flow, uint32_t, ofs);
-}
-
-static inline ovs_be32
-miniflow_get_be32(const struct miniflow *flow, unsigned int ofs)
-{
-    return MINIFLOW_GET_TYPE(flow, ovs_be32, ofs);
-}
-
-#undef MINIFLOW_GET_TYPE
-
 /* Returns the VID within the vlan_tci member of the "struct flow" represented
  * by 'flow'. */
 static inline uint16_t
 miniflow_get_vid(const struct miniflow *flow)
 {
-    ovs_be16 tci = miniflow_get_be16(flow, offsetof(struct flow, vlan_tci));
+    ovs_be16 tci = MINIFLOW_GET_BE16(flow, vlan_tci);
     return vlan_tci_to_vid(tci);
 }
 
@@ -558,19 +530,27 @@ minimask_get_vid_mask(const struct minimask *mask)
 static inline uint16_t
 miniflow_get_tcp_flags(const struct miniflow *flow)
 {
-    return ntohs(miniflow_get_be16(flow, offsetof(struct flow, tcp_flags)));
+    return ntohs(MINIFLOW_GET_BE16(flow, tcp_flags));
 }
 
 /* Returns the value of the OpenFlow 1.1+ "metadata" field in 'flow'. */
 static inline ovs_be64
 miniflow_get_metadata(const struct miniflow *flow)
 {
+    union {
+        ovs_be64 be64;
+        struct {
+            ovs_be32 hi;
+            ovs_be32 lo;
+        };
+    } value;
+
     enum { MD_OFS = offsetof(struct flow, metadata) };
     BUILD_ASSERT_DECL(MD_OFS % sizeof(uint32_t) == 0);
-    ovs_be32 hi = miniflow_get_be32(flow, MD_OFS);
-    ovs_be32 lo = miniflow_get_be32(flow, MD_OFS + 4);
+    value.hi = MINIFLOW_GET_TYPE(flow, ovs_be32, MD_OFS);
+    value.lo = MINIFLOW_GET_TYPE(flow, ovs_be32, MD_OFS + 4);
 
-    return htonll(((uint64_t) ntohl(hi) << 32) | ntohl(lo));
+    return value.be64;
 }
 
 /* Returns the mask for the OpenFlow 1.1+ "metadata" field in 'mask'.

diff --git a/tests/test-classifier.c b/tests/test-classifier.c
index 3518db0..e9e253a 100644 (file)
--- a/tests/test-classifier.c
+++ b/tests/test-classifier.c
@@ -1203,7 +1203,8 @@ test_miniflow(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
         /* Check that the flow equals its miniflow. */
         assert(miniflow_get_vid(&miniflow) == vlan_tci_to_vid(flow.vlan_tci));
         for (i = 0; i < FLOW_U32S; i++) {
-            assert(miniflow_get_u32(&miniflow, i * 4) == flow_u32[i]);
+            assert(MINIFLOW_GET_TYPE(&miniflow, uint32_t, i * 4)
+                   == flow_u32[i]);
         }
 
         /* Check that the miniflow equals itself. */