ofproto-dpif-rid: Use array instead of ofpbuf for recirc_state stack.

In my opinion, this makes better sense for the stack, because it's not
a packet or a collection of bytes, it's an array of struct mf_subvalue.
(I left it as an ofpbuf for accumulating stack entries during
translation, because the automatic reallocation and especially the stub
support there is helpful.)

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Jarno Rajahalme <jarno@ovn.org>

diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c
index cb00301..c61b007 100644 (file)
--- a/ofproto/ofproto-dpif-rid.c
+++ b/ofproto/ofproto-dpif-rid.c
@@ -142,9 +142,9 @@ recirc_metadata_hash(const struct recirc_state *state)
     hash = hash_bytes64((const uint64_t *) &state->metadata.metadata,
                         sizeof state->metadata - sizeof state->metadata.tunnel,
                         hash);
-    if (state->stack && state->stack->size != 0) {
-        hash = hash_bytes64((const uint64_t *) state->stack->data,
-                            state->stack->size, hash);
+    if (state->stack && state->n_stack) {
+        hash = hash_bytes64((const uint64_t *) state->stack,
+                            state->n_stack * sizeof *state->stack, hash);
     }
     hash = hash_int(state->mirrors, hash);
     hash = hash_int(state->action_set_len, hash);
@@ -164,9 +164,8 @@ recirc_metadata_equal(const struct recirc_state *a,
             && 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)))
+            && a->n_stack == b->n_stack
+            && !memcmp(a->stack, b->stack, a->n_stack * sizeof *a->stack)
             && a->mirrors == b->mirrors
             && a->conntracked == b->conntracked
             && a->action_set_len == b->action_set_len
@@ -211,20 +210,18 @@ recirc_state_clone(struct recirc_state *new, const struct recirc_state *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;
-    }
-    if (new->ofpacts) {
-        new->ofpacts = (new->ofpacts_len
-                        ? xmemdup(new->ofpacts, new->ofpacts_len)
-                        : NULL);
-    }
+    new->stack = (new->n_stack
+                  ? xmemdup(new->stack, new->n_stack * sizeof *new->stack)
+                  : NULL);
+    new->ofpacts = (new->ofpacts_len
+                    ? xmemdup(new->ofpacts, new->ofpacts_len)
+                    : NULL);
 }
 
 static void
 recirc_state_free(struct recirc_state *state)
 {
-    ofpbuf_delete(state->stack);
+    free(state->stack);
     free(state->ofpacts);
 }
 

diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h
index 4bbc7bf..101bd6e 100644 (file)
--- a/ofproto/ofproto-dpif-rid.h
+++ b/ofproto/ofproto-dpif-rid.h
@@ -140,7 +140,8 @@ struct recirc_state {
     /* Pipeline context for post-recirculation processing. */
     struct ofproto_dpif *ofproto; /* Post-recirculation bridge. */
     struct recirc_metadata metadata; /* Flow metadata. */
-    struct ofpbuf *stack;         /* Stack if any. */
+    union mf_subvalue *stack;     /* Stack if any. */
+    size_t n_stack;
     mirror_mask_t mirrors;        /* Mirrors already output. */
     bool conntracked;             /* Conntrack occurred prior to recirc. */
 

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 2ee647b..6d67e91 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3626,7 +3626,8 @@ compose_recirculate_action__(struct xlate_ctx *ctx, uint8_t table)
         .table_id = table,
         .ofproto = ctx->xbridge->ofproto,
         .metadata = md,
-        .stack = &ctx->stack,
+        .stack = ctx->stack.data,
+        .n_stack = ctx->stack.size / sizeof(union mf_subvalue),
         .mirrors = ctx->mirrors,
         .conntracked = ctx->conntracked,
         .action_set_len = ctx->recirc_action_offset,
@@ -5163,7 +5164,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
 
         /* Restore stack, if any. */
         if (state->stack) {
-            ofpbuf_put(&ctx.stack, state->stack->data, state->stack->size);
+            ofpbuf_put(&ctx.stack, state->stack,
+                       state->n_stack * sizeof *state->stack);
         }
 
         /* Restore mirror state. */