ofproto-dpif-rid: Make lookups cheaper.

This patch removes a large-ish copy from the recirculation context
lookup, which is performed for each recirculated upcall and
revalidation of a recirculating flow.

Tunnel metadata has grown large since the addition of Geneve options,
and copying that metadata for performing a lookup is not necessary.
Change recirc_metadata to use a pointer to struct flow_tnl, and only
copy the tunnel metadata when needed, and only copy as little of it as
possible.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

2 files changed, 60 insertions, 34 deletions

diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c
index b8207c496..5a02bb094 100644
--- a/ofproto/ofproto-dpif-rid.c
+++ b/ofproto/ofproto-dpif-rid.c
@@ -130,8 +130,16 @@ recirc_metadata_hash(const struct recirc_state *state)
 
     hash = hash_pointer(state->ofproto, 0);
     hash = hash_int(state->table_id, hash);
-    hash = hash_words64((const uint64_t *) &state->metadata,
-                        sizeof state->metadata / sizeof(uint64_t),
+    if (state->metadata.tunnel->ip_dst) {
+        /* We may leave remainder bytes unhashed, but that is unlikely as
+         * the tunnel is not in the datapath format. */
+        hash = hash_words64((const uint64_t *) state->metadata.tunnel,
+                            flow_tnl_size(state->metadata.tunnel)
+                            / sizeof(uint64_t), hash);
+    }
+    hash = hash_words64((const uint64_t *) &state->metadata.metadata,
+                        (sizeof state->metadata - sizeof state->metadata.tunnel)
+                        / sizeof(uint64_t),
                         hash);
     if (state->stack && state->stack->size != 0) {
         hash = hash_words64((const uint64_t *) state->stack->data,
@@ -153,7 +161,9 @@ recirc_metadata_equal(const struct recirc_state *a,
 {
     return (a->table_id == b->table_id
             && a->ofproto == b->ofproto
-            && !memcmp(&a->metadata, &b->metadata, sizeof a->metadata)
+            && flow_tnl_equal(a->metadata.tunnel, b->metadata.tunnel)
+            && !memcmp(&a->metadata.metadata, &b->metadata.metadata,
+                       sizeof a->metadata - sizeof a->metadata.tunnel)
             && (((!a->stack || !a->stack->size) &&
                  (!b->stack || !b->stack->size))
                 || (a->stack && b->stack && ofpbuf_equal(a->stack, b->stack)))
@@ -193,9 +203,13 @@ recirc_ref_equal(const struct recirc_state *target, uint32_t hash)
 }
 
 static void
-recirc_state_clone(struct recirc_state *new, const struct recirc_state *old)
+recirc_state_clone(struct recirc_state *new, const struct recirc_state *old,
+                   struct flow_tnl *tunnel)
 {
     *new = *old;
+    flow_tnl_copy__(tunnel, old->metadata.tunnel);
+    new->metadata.tunnel = tunnel;
+
     if (new->stack) {
         new->stack = new->stack->size ? ofpbuf_clone(new->stack) : NULL;
     }
@@ -216,9 +230,11 @@ recirc_alloc_id__(const struct recirc_state *state, uint32_t hash)
     ovs_assert(state->action_set_len <= state->ofpacts_len);
 
     struct recirc_id_node *node = xzalloc(sizeof *node);
+
     node->hash = hash;
     ovs_refcount_init(&node->refcount);
-    recirc_state_clone(CONST_CAST(struct recirc_state *, &node->state), state);
+    recirc_state_clone(CONST_CAST(struct recirc_state *, &node->state), state,
+                       &node->state_metadata_tunnel);
 
     ovs_mutex_lock(&mutex);
     for (;;) {
@@ -269,10 +285,12 @@ recirc_alloc_id_ctx(const struct recirc_state *state)
 uint32_t
 recirc_alloc_id(struct ofproto_dpif *ofproto)
 {
+    struct flow_tnl tunnel;
+    tunnel.ip_dst = htonl(0);
     struct recirc_state state = {
         .table_id = TBL_INTERNAL,
         .ofproto = ofproto,
-        .metadata = { .in_port = OFPP_NONE },
+        .metadata = { .tunnel = &tunnel, .in_port = OFPP_NONE },
     };
     return recirc_alloc_id__(&state, recirc_metadata_hash(&state))->id;
 }
diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h
index 11ae4864e..e7d95bf14 100644
--- a/ofproto/ofproto-dpif-rid.h
+++ b/ofproto/ofproto-dpif-rid.h
@@ -34,40 +34,40 @@ struct rule;
  * =============
  *
  * Recirculation is a technique to allow a frame to re-enter the datapath
- * packet processing path for one or multiple times to achieve more flexible
- * packet processing, such modifying header fields after MPLS POP action and
- * selecting bond a slave port for bond ports.
+ * packet processing path to achieve more flexible packet processing, such as
+ * modifying header fields after MPLS POP action and selecting a slave port for
+ * bond ports.
  *
  * Data path and user space interface
  * -----------------------------------
  *
  * Recirculation uses two uint32_t fields, recirc_id and dp_hash, and a RECIRC
- * action.  The value recirc_id is used to select the next packet processing
- * steps among multiple instances of recirculation.  When a packet initially
- * enters the data path it is assigned with recirc_id 0, which indicates no
- * recirculation.  Recirc_ids are managed by the user space, opaque to the
- * data path.
+ * action.  recirc_id is used to select the next packet processing steps among
+ * multiple instances of recirculation.  When a packet initially enters the
+ * datapath it is assigned with recirc_id 0, which indicates no recirculation.
+ * Recirc_ids are managed by the user space, opaque to the datapath.
  *
- * On the other hand, dp_hash can only be computed by the data path, opaque to
- * the user space.  In fact, user space may not able to recompute the hash
- * value.  The dp_hash value should be wildcarded for a newly received
- * packet.  HASH action specifies whether the hash is computed, and if
- * computed, how many fields are to be included in the hash computation.  The
- * computed hash value is stored into the dp_hash field prior to recirculation.
+ * On the other hand, dp_hash can only be computed by the datapath, opaque to
+ * the user space, as the datapath is free to choose the hashing algorithm
+ * without informing user space about it.  The dp_hash value should be
+ * wildcarded for newly received packets.  HASH action specifies whether the
+ * hash is computed, and if computed, how many fields are to be included in the
+ * hash computation.  The computed hash value is stored into the dp_hash field
+ * prior to recirculation.
  *
  * The RECIRC action sets the recirc_id field and then reprocesses the packet
- * as if it was received on the same input port.  RECIRC action works like a
- * function call; actions listed behind the RECIRC action will be executed
- * after its execution.  RECIRC action can be nested, data path implementation
- * limits the number of recirculation executed to prevent unreasonable nesting
- * depth or infinite loop.
+ * as if it was received again on the same input port.  RECIRC action works
+ * like a function call; actions listed after the RECIRC action will be
+ * executed after recirculation.  RECIRC action can be nested, but datapath
+ * implementation limits the number of nested recirculations to prevent
+ * unreasonable nesting depth or infinite loop.
  *
  * User space recirculation context
  * ---------------------------------
  *
- * Recirculation is hidden from the OpenFlow controllers.  Action translation
- * code deduces when recirculation is necessary and issues a data path
- * recirculation action.  All OpenFlow actions to be performed after
+ * Recirculation is usually hidden from the OpenFlow controllers.  Action
+ * translation code deduces when recirculation is necessary and issues a
+ * datapath recirculation action.  All OpenFlow actions to be performed after
  * recirculation are derived from the OpenFlow pipeline and are stored with the
  * recirculation ID.  When the OpenFlow tables are changed in a way affecting
  * the recirculation flows, new recirculation ID with new metadata and actions
@@ -76,9 +76,10 @@ struct rule;
  * Recirculation ID pool
  * ----------------------
  *
- * Recirculation ID needs to be unique for all data paths.  Recirculation ID
- * pool keeps track recirculation ids and stores OpenFlow pipeline translation
- * context so that flow processing may continue after recirculation.
+ * Recirculation ID needs to be unique for all datapaths.  Recirculation ID
+ * pool keeps track of recirculation ids and stores OpenFlow pipeline
+ * translation context so that flow processing may continue after
+ * recirculation.
  *
  * A Recirculation ID can be any uint32_t value, except for that the value 0 is
  * reserved for 'no recirculation' case.
@@ -96,7 +97,7 @@ BUILD_ASSERT_DECL(FLOW_WC_SEQ == 33);
 
 struct recirc_metadata {
     /* Metadata in struct flow. */
-    struct flow_tnl tunnel;       /* Encapsulating tunnel parameters. */
+    const struct flow_tnl *tunnel; /* Encapsulating tunnel parameters. */
     ovs_be64 metadata;            /* OpenFlow Metadata. */
     uint64_t regs[FLOW_N_XREGS];  /* Registers. */
     ofp_port_t in_port;           /* Incoming port. */
@@ -108,7 +109,7 @@ recirc_metadata_from_flow(struct recirc_metadata *md,
                           const struct flow *flow)
 {
     memset(md, 0, sizeof *md);
-    md->tunnel = flow->tunnel;
+    md->tunnel = &flow->tunnel;
     md->metadata = flow->metadata;
     memcpy(md->regs, flow->regs, sizeof md->regs);
     md->in_port = flow->in_port.ofp_port;
@@ -119,7 +120,11 @@ static inline void
 recirc_metadata_to_flow(const struct recirc_metadata *md,
                         struct flow *flow)
 {
-    flow->tunnel = md->tunnel;
+    if (md->tunnel && md->tunnel->ip_dst) {
+        flow->tunnel = *md->tunnel;
+    } else {
+        memset(&flow->tunnel, 0, sizeof flow->tunnel);
+    }
     flow->metadata = md->metadata;
     memcpy(flow->regs, md->regs, sizeof flow->regs);
     flow->in_port.ofp_port = md->in_port;
@@ -161,6 +166,9 @@ struct recirc_id_node {
      * This state should not be modified after inserting a node in the pool,
      * hence the 'const' to emphasize that. */
     const struct recirc_state state;
+
+    /* Storage for tunnel metadata. */
+    struct flow_tnl state_metadata_tunnel;
 };
 
 void recirc_init(void);