[ovs-dev] [PATCH 06/10] flow.c: Only support inline values in miniflow_extract().

Jarno Rajahalme jrajahalme at nicira.com
Fri Nov 21 00:42:53 UTC 2014


All the users of miniflow extract supply a miniflow with inlined data.
Make the extraction a bit more efficient by only supporting this.

Also, clean up miniflow building utilities in preparation for moving
them to lib/flow.h in a later patch.

Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
 lib/dpif-netdev.c |   22 +++++---
 lib/flow.c        |  161 +++++++++++++++++++++++++++++++----------------------
 lib/flow.h        |   11 ----
 3 files changed, 106 insertions(+), 88 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 83dbd61..b0f3510 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -435,6 +435,8 @@ static void dp_netdev_reset_pmd_threads(struct dp_netdev *dp);
 
 static void emc_clear_entry(struct emc_entry *ce);
 
+static inline void netdev_flow_key_init(struct netdev_flow_key *key);
+
 static void
 emc_cache_init(struct emc_cache *flow_cache)
 {
@@ -444,11 +446,7 @@ emc_cache_init(struct emc_cache *flow_cache)
 
     for (i = 0; i < ARRAY_SIZE(flow_cache->entries); i++) {
         flow_cache->entries[i].flow = NULL;
-        flow_cache->entries[i].key.hash = 0;
-        flow_cache->entries[i].key.len
-            = offsetof(struct miniflow, inline_values);
-        miniflow_initialize(&flow_cache->entries[i].key.mf,
-                            flow_cache->entries[i].key.buf);
+        netdev_flow_key_init(&flow_cache->entries[i].key);
     }
 }
 
@@ -1289,6 +1287,15 @@ netdev_flow_key_size(uint32_t flow_u32s)
         MINIFLOW_VALUES_SIZE(flow_u32s);
 }
 
+static inline void
+netdev_flow_key_init(struct netdev_flow_key *key)
+{
+    key->hash = 0;
+    key->len = netdev_flow_key_size(0);
+    key->mf.map = 0;
+    key->mf.values_inline = true;
+}
+
 static inline bool
 netdev_flow_key_equal(const struct netdev_flow_key *a,
                       const struct netdev_flow_key *b)
@@ -1324,8 +1331,6 @@ netdev_flow_key_from_flow(struct netdev_flow_key *dst,
     uint64_t buf_stub[512 / 8];
     struct pkt_metadata md = pkt_metadata_from_flow(src);
 
-    miniflow_initialize(&dst->mf, dst->buf);
-
     ofpbuf_use_stub(&packet, buf_stub, sizeof buf_stub);
     flow_compose(&packet, src);
     miniflow_extract(&packet, &md, &dst->mf);
@@ -2767,7 +2772,6 @@ emc_processing(struct dp_netdev_pmd_thread *pmd, struct dpif_packet **packets,
     size_t notfound_cnt = 0;
 
     n_batches = 0;
-    miniflow_initialize(&key.mf, key.buf);
     for (i = 0; i < cnt; i++) {
         struct dp_netdev_flow *flow;
 
@@ -2777,7 +2781,7 @@ emc_processing(struct dp_netdev_pmd_thread *pmd, struct dpif_packet **packets,
         }
 
         miniflow_extract(&packets[i]->ofpbuf, &packets[i]->md, &key.mf);
-        key.len = 0; /* Not computed yet. */
+        key.len = netdev_flow_key_size(0); /* Not computed yet. */
         key.hash = dpif_netdev_packet_get_dp_hash(packets[i], &key.mf);
 
         flow = emc_lookup(flow_cache, &key);
diff --git a/lib/flow.c b/lib/flow.c
index 8a98ad6..37d6af8 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -108,97 +108,122 @@ data_try_pull(void **datap, size_t *sizep, size_t size)
     return OVS_LIKELY(*sizep >= size) ? data_pull(datap, sizep, size) : NULL;
 }
 
-/* Context for pushing data to a miniflow. */
-struct mf_ctx {
-    uint64_t map;
-    uint32_t *data;
-    uint32_t * const end;
-};
-
-/* miniflow_push_* macros allow filling in a miniflow data values in order.
- * Assertions are needed only when the layout of the struct flow is modified.
- * 'ofs' is a compile-time constant, which allows most of the code be optimized
- * away.  Some GCC versions gave warnings on ALWAYS_INLINE, so these are
- * defined as macros. */
+/* Macros for building miniflows.  Miniflow fields must be pushed in the order
+ * they appear in struct flow.  Using a out-of-sync FLOW_WC_SEQ value below
+ * will cause runtime assertions to make sure the code complies with the order
+ * requirement. */
 
 #if (FLOW_WC_SEQ != 28)
 #define MINIFLOW_ASSERT(X) ovs_assert(X)
 BUILD_MESSAGE("FLOW_WC_SEQ changed: miniflow_extract() will have runtime "
-               "assertions enabled. Consider updating FLOW_WC_SEQ after "
-               "testing")
+              "assertions enabled.  Consider updating FLOW_WC_SEQ value "
+              "above MINIFLOW_ASSERT() after testing.")
 #else
 #define MINIFLOW_ASSERT(X)
 #endif
 
-#define miniflow_push_uint32_(MF, OFS, VALUE)                   \
-{                                                               \
-    MINIFLOW_ASSERT(MF.data < MF.end && (OFS) % 4 == 0          \
-                    && !(MF.map & (UINT64_MAX << (OFS) / 4)));  \
-    *MF.data++ = VALUE;                                         \
-    MF.map |= UINT64_C(1) << (OFS) / 4;                         \
+/* This is useful for making sure the buffer is properly aligned. */
+static inline void miniflow_assert_inline(const struct miniflow *mf,
+                                          const uint32_t *buf)
+{
+    ovs_assert(buf == mf->inline_values || buf == (uint32_t *)(mf + 1));
 }
 
-#define miniflow_push_be32_(MF, OFS, VALUE) \
-    miniflow_push_uint32_(MF, OFS, (OVS_FORCE uint32_t)(VALUE))
+/* Context for pushing data to a miniflow. */
+struct mf_ctx {
+    uint64_t map;
+    uint32_t *data;
+    uint32_t * const end;
+};
+
+#define MF_CTX_INITIALIZER(BUF, COUNT) { 0, (BUF), (BUF) + COUNT }
+#define MF_CTX_FINISH(CTX, MINIFLOW)            \
+do {                                            \
+    (MINIFLOW)->map = (CTX).map;                \
+    (MINIFLOW)->values_inline = true;           \
+} while (0)
 
-#define miniflow_push_uint16_(MF, OFS, VALUE)                   \
-{                                                               \
-    MINIFLOW_ASSERT(MF.data < MF.end &&                                 \
-                    (((OFS) % 4 == 0 && !(MF.map & (UINT64_MAX << (OFS) / 4))) \
-                     || ((OFS) % 4 == 2 && MF.map & (UINT64_C(1) << (OFS) / 4) \
-                         && !(MF.map & (UINT64_MAX << ((OFS) / 4 + 1)))))); \
+/* miniflow_push_* macros allow filling in a miniflow data values in order.
+ * Assertions are needed only when the layout of the struct flow is modified.
+ * 'ofs' is a compile-time constant, which allows most of the code be optimized
+ * away. */
+
+#define miniflow_push_uint32_(CTX, OFS, VALUE)                  \
+do {                                                            \
+    MINIFLOW_ASSERT((CTX).data < (CTX).end);                    \
+    MINIFLOW_ASSERT((OFS) % 4 == 0);                            \
+    MINIFLOW_ASSERT(!((CTX).map & (UINT64_MAX << (OFS) / 4)));  \
+                                                                \
+    *(CTX).data++ = VALUE;                                      \
+    (CTX).map |= UINT64_C(1) << (OFS) / 4;                      \
+} while (0)
+
+#define miniflow_push_be32_(CTX, OFS, VALUE)                        \
+    miniflow_push_uint32_(CTX, OFS, (OVS_FORCE uint32_t)(VALUE))
+
+#define miniflow_push_uint16_(CTX, OFS, VALUE)                          \
+do {                                                                    \
+    MINIFLOW_ASSERT((CTX).data < (CTX).end);                            \
+    MINIFLOW_ASSERT(((OFS) % 4 == 0                                     \
+                     && !((CTX).map & (UINT64_MAX << (OFS) / 4)))       \
+                    || ((OFS) % 4 == 2                                  \
+                        && (CTX).map & (UINT64_C(1) << (OFS) / 4)       \
+                        && !((CTX).map & (UINT64_MAX << ((OFS) / 4 + 1))))); \
                                                                         \
     if ((OFS) % 4 == 0) {                                               \
-        *(uint16_t *)MF.data = VALUE;                                   \
-        MF.map |= UINT64_C(1) << (OFS) / 4;                             \
+        *(uint16_t *)(CTX).data = VALUE;                                \
+        (CTX).map |= UINT64_C(1) << (OFS) / 4;                          \
     } else if ((OFS) % 4 == 2) {                                        \
-        *((uint16_t *)MF.data + 1) = VALUE;                             \
-        MF.data++;                                                      \
+        *((uint16_t *)(CTX).data + 1) = VALUE;                          \
+        (CTX).data++;                                                   \
     }                                                                   \
-}
+} while (0)
 
-#define miniflow_push_be16_(MF, OFS, VALUE)             \
-    miniflow_push_uint16_(MF, OFS, (OVS_FORCE uint16_t)VALUE);
+#define miniflow_push_be16_(CTX, OFS, VALUE)                    \
+    miniflow_push_uint16_(CTX, OFS, (OVS_FORCE uint16_t)VALUE);
 
 /* Data at 'valuep' may be unaligned. */
-#define miniflow_push_words_(MF, OFS, VALUEP, N_WORDS)          \
-{                                                               \
-    int ofs32 = (OFS) / 4;                                      \
+#define miniflow_push_words_(CTX, OFS, VALUEP, N_WORDS)                 \
+do {                                                                    \
+    int ofs32 = (OFS) / 4;                                              \
                                                                         \
-    MINIFLOW_ASSERT(MF.data + (N_WORDS) <= MF.end && (OFS) % 4 == 0     \
-                    && !(MF.map & (UINT64_MAX << ofs32)));              \
+    MINIFLOW_ASSERT((CTX).data + (N_WORDS) <= (CTX).end);               \
+    MINIFLOW_ASSERT((OFS) % 4 == 0);                                    \
+    MINIFLOW_ASSERT(!((CTX).map & (UINT64_MAX << ofs32)));              \
                                                                         \
-    memcpy(MF.data, (VALUEP), (N_WORDS) * sizeof *MF.data);             \
-    MF.data += (N_WORDS);                                               \
-    MF.map |= ((UINT64_MAX >> (64 - (N_WORDS))) << ofs32);              \
-}
+    memcpy((CTX).data, (VALUEP), (N_WORDS) * sizeof *(CTX).data);       \
+    (CTX).data += (N_WORDS);                                            \
+    (CTX).map |= ((UINT64_MAX >> (64 - (N_WORDS))) << ofs32);           \
+} while (0)
 
-#define miniflow_push_uint32(MF, FIELD, VALUE)                          \
-    miniflow_push_uint32_(MF, offsetof(struct flow, FIELD), VALUE)
+#define miniflow_push_uint32(CTX, FIELD, VALUE)                     \
+    miniflow_push_uint32_(CTX, offsetof(struct flow, FIELD), VALUE)
 
-#define miniflow_push_be32(MF, FIELD, VALUE)                            \
-    miniflow_push_be32_(MF, offsetof(struct flow, FIELD), VALUE)
+#define miniflow_push_be32(CTX, FIELD, VALUE)                       \
+    miniflow_push_be32_(CTX, offsetof(struct flow, FIELD), VALUE)
 
-#define miniflow_push_uint32_check(MF, FIELD, VALUE)                    \
-    { if (OVS_LIKELY(VALUE)) {                                          \
-            miniflow_push_uint32_(MF, offsetof(struct flow, FIELD), VALUE); \
-        }                                                               \
-    }
+#define miniflow_push_uint32_check(CTX, FIELD, VALUE)                   \
+do {                                                                    \
+    if (OVS_LIKELY(VALUE)) {                                            \
+        miniflow_push_uint32_(CTX, offsetof(struct flow, FIELD), VALUE); \
+    }                                                                   \
+} while (0)
 
-#define miniflow_push_be32_check(MF, FIELD, VALUE)                      \
-    { if (OVS_LIKELY(VALUE)) {                                          \
-            miniflow_push_be32_(MF, offsetof(struct flow, FIELD), VALUE); \
-        }                                                               \
-    }
+#define miniflow_push_be32_check(CTX, FIELD, VALUE)                     \
+do {                                                                    \
+    if (OVS_LIKELY(VALUE)) {                                            \
+        miniflow_push_be32_(CTX, offsetof(struct flow, FIELD), VALUE);  \
+    }                                                                   \
+} while (0)
 
-#define miniflow_push_uint16(MF, FIELD, VALUE)                          \
-    miniflow_push_uint16_(MF, offsetof(struct flow, FIELD), VALUE)
+#define miniflow_push_uint16(CTX, FIELD, VALUE)                     \
+    miniflow_push_uint16_(CTX, offsetof(struct flow, FIELD), VALUE)
 
-#define miniflow_push_be16(MF, FIELD, VALUE)                            \
-    miniflow_push_be16_(MF, offsetof(struct flow, FIELD), VALUE)
+#define miniflow_push_be16(CTX, FIELD, VALUE)                       \
+    miniflow_push_be16_(CTX, offsetof(struct flow, FIELD), VALUE)
 
-#define miniflow_push_words(MF, FIELD, VALUEP, N_WORDS)                 \
-    miniflow_push_words_(MF, offsetof(struct flow, FIELD), VALUEP, N_WORDS)
+#define miniflow_push_words(CTX, FIELD, VALUEP, N_WORDS)                \
+    miniflow_push_words_(CTX, offsetof(struct flow, FIELD), VALUEP, N_WORDS)
 
 /* Pulls the MPLS headers at '*datap' and returns the count of them. */
 static inline int
@@ -354,7 +379,7 @@ flow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
 
     COVERAGE_INC(flow_extract);
 
-    miniflow_initialize(&m.mf, m.buf);
+    miniflow_assert_inline(&m.mf, m.buf);
     miniflow_extract(packet, md, &m.mf);
     miniflow_expand(&m.mf, flow);
 }
@@ -367,8 +392,8 @@ miniflow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
 {
     void *data = ofpbuf_data(packet);
     size_t size = ofpbuf_size(packet);
-    uint32_t *values = miniflow_values(dst);
-    struct mf_ctx mf = { 0, values, values + FLOW_U32S };
+    uint32_t *values = dst->inline_values;
+    struct mf_ctx mf = MF_CTX_INITIALIZER(values, FLOW_U32S);
     char *l2;
     ovs_be16 dl_type;
     uint8_t nw_frag, nw_tos, nw_ttl, nw_proto;
@@ -656,7 +681,7 @@ miniflow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
         miniflow_push_uint32_check(mf, dp_hash, md->dp_hash);
     }
  out:
-    dst->map = mf.map;
+    MF_CTX_FINISH(mf, dst);
 }
 
 /* For every bit of a field that is wildcarded in 'wildcards', sets the
diff --git a/lib/flow.h b/lib/flow.h
index 9e2e561..f19f330 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -443,17 +443,6 @@ static inline const ovs_be32 *miniflow_get_be32_values(const struct miniflow *mf
     return (OVS_FORCE const ovs_be32 *)miniflow_get_values(mf);
 }
 
-/* This is useful for initializing a miniflow for a miniflow_extract() call. */
-static inline void miniflow_initialize(struct miniflow *mf,
-                                       uint32_t buf[FLOW_U32S])
-{
-    mf->map = 0;
-    mf->values_inline = (buf == (uint32_t *)(mf + 1));
-    if (!mf->values_inline) {
-        mf->offline_values = buf;
-    }
-}
-
 struct pkt_metadata;
 
 /* The 'dst->values' must be initialized with a buffer with space for
-- 
1.7.10.4




More information about the dev mailing list