[ovs-dev] [PATCH v2 2/2] packets: Reorganize the pkt_metdata structure.

[Ben Pfaff]

From: Bhanuprakash Bodireddy <bhanuprakash.bodireddy at intel.com>

pkt_metadata_init() is called for every packet in userspace datapath and
initializes few members in pkt_metadata. Before this the members that
needs to be initialized are prefetched using pkt_metadata_prefetch_init().

The above functions are critical to the userspace datapath performance and
should be in sync. Any changes to the pkt_metadata should also include
changes to metadata_init() and prefetch_init() if necessary.

This commit slightly refactors the pkt_metadata structure and introduces
cache line markers to catch any violations to the structure. Also only
prefetch the cachelines having the members that needs to be zeroed out.

Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy at intel.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
---
v1->v2: Use PADDED_MEMBERS.

 lib/packets.h | 25 +++++++++++++++++++++++--
 lib/util.h    |  1 +
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/lib/packets.h b/lib/packets.h
index a9d5e844eab8..a0e01368118e 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -92,6 +92,8 @@ flow_tnl_equal(const struct flow_tnl *a, const struct flow_tnl *b)
 
 /* Datapath packet metadata */
 struct pkt_metadata {
+    OVS_CACHE_LINE_MARKER cacheline0;
+PADDED_MEMBERS(CACHE_LINE_SIZE,
     uint32_t recirc_id;         /* Recirculation id carried with the
                                    recirculating packets. 0 for packets
                                    received from the wire. */
@@ -104,15 +106,29 @@ struct pkt_metadata {
     uint16_t ct_zone;           /* Connection zone. */
     uint32_t ct_mark;           /* Connection mark. */
     ovs_u128 ct_label;          /* Connection label. */
+    union flow_in_port in_port; /* Input port. */
+);
+
+    OVS_CACHE_LINE_MARKER cacheline1;
+PADDED_MEMBERS(CACHE_LINE_SIZE,
     union {                     /* Populated only for non-zero 'ct_state'. */
         struct ovs_key_ct_tuple_ipv4 ipv4;
         struct ovs_key_ct_tuple_ipv6 ipv6;   /* Used only if                */
     } ct_orig_tuple;                         /* 'ct_orig_tuple_ipv6' is set */
-    union flow_in_port in_port; /* Input port. */
+);
+
+    OVS_CACHE_LINE_MARKER cacheline2;
+PADDED_MEMBERS(CACHE_LINE_SIZE,
     struct flow_tnl tunnel;     /* Encapsulating tunnel parameters. Note that
                                  * if 'ip_dst' == 0, the rest of the fields may
                                  * be uninitialized. */
+);
 };
+BUILD_ASSERT_DECL(offsetof(struct pkt_metadata, cacheline0) == 0);
+BUILD_ASSERT_DECL(offsetof(struct pkt_metadata, cacheline1) == \
+                           CACHE_LINE_SIZE);
+BUILD_ASSERT_DECL(offsetof(struct pkt_metadata, cacheline2) == \
+                           2 * CACHE_LINE_SIZE);
 
 static inline void
 pkt_metadata_init_tnl(struct pkt_metadata *md)
@@ -140,7 +156,12 @@ pkt_metadata_init(struct pkt_metadata *md, odp_port_t port)
 static inline void
 pkt_metadata_prefetch_init(struct pkt_metadata *md)
 {
-    ovs_prefetch_range(md, offsetof(struct pkt_metadata, tunnel.ip_src));
+    /* Prefetch cacheline0 as first 17 bytes and odp_port will be
+     * initialized later.  */
+    OVS_PREFETCH(md->cacheline0);
+
+    /* Prefetch cachline2 as ip_dst & ipv6_dst fields will be initialized. */
+    OVS_PREFETCH(md->cacheline2);
 }
 
 bool dpid_from_string(const char *s, uint64_t *dpidp);
diff --git a/lib/util.h b/lib/util.h
index c2d1c3fc6467..b5697cb5e4c8 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -50,6 +50,7 @@ extern char *program_name;
  * Being wrong hurts performance but not correctness. */
 #define CACHE_LINE_SIZE 64
 BUILD_ASSERT_DECL(IS_POW2(CACHE_LINE_SIZE));
+typedef void *OVS_CACHE_LINE_MARKER[0];
 
 static inline void
 ovs_prefetch_range(const void *start, size_t size)
-- 
2.10.2