On Tue, Mar 05, 2013 at 04:27:55PM -0800, Andy Zhou wrote:
> The Push action takes a single parameter. Any source allowed by NXAST_REG_MOVE
> is allowed to be pushed onto the stack. When the source is a bit field,
> its value will be right shifted to bit zero before being pushed onto the
> stack. The remaining bits will be set to zero.
> 
> The Pop action also takes a single parameter. Any destination allowed by
> NXAST_REG_MOVE can be used as the destination of the action. The value, in
> case of a bit field, will be taken from top of the stack, starting from
> bit zero.
> 
> The stack size is not limited. The initial 8KB is statically allocated to
> efficiently handle most common use cases. When more stack space is
> required, the stack can grow using malloc().
> 
> Signed-off-by: Andy Zhou <az...@nicira.com>

"git am" pointed out whitespace errors:

    Applying: nicira-ext: Add Nicira actions NXAST_STACK_PUSH and 
NXAST_STACK_POP.
    /home/blp/ovs/.git/rebase-apply/patch:282: trailing whitespace.
            error = nxm_reg_move_from_openflow( 
    /home/blp/ovs/.git/rebase-apply/patch:289: space before tab in indent.
                    (const struct nx_action_reg_load *) a, out);
    /home/blp/ovs/.git/rebase-apply/patch:449: trailing whitespace.
        union mf_subvalue init_stack[1024/sizeof(union mf_subvalue)];   
    warning: 3 lines add whitespace errors.

I fixed up a few minor stylistic things by folding in the following.
I shortened or deleted a few of the comments because I felt like they
did not add much valuable information.  (For example, it should
normally be clear that an ofpbuf can expand as needed.)

I applied this to master.  Thank you!

diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index c6b5035..c46185a 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -564,7 +564,7 @@ OFP_ASSERT(sizeof(struct nx_action_reg_load) == 24);
 
 /* Action structure for NXAST_STACK_PUSH and NXAST_STACK_POP.
  *
- * Pushes (or pop) field[offset: offset + n_bits] to (or from)
+ * Pushes (or pops) field[offset: offset + n_bits] to (or from)
  * top of the stack.
  */
 struct nx_action_stack {
@@ -575,7 +575,7 @@ struct nx_action_stack {
     ovs_be16 offset;                /* Bit offset into the field. */
     ovs_be32 field;                 /* The field used for push or pop. */
     ovs_be16 n_bits;                /* (n_bits + 1) bits of the field. */
-    uint8_t zero[6];                /* Reserved for future use, must be zero.*/
+    uint8_t zero[6];                /* Reserved, must be zero. */
 };
 OFP_ASSERT(sizeof(struct nx_action_stack) == 24);
 
diff --git a/lib/nx-match.c b/lib/nx-match.c
index f4a93a0..093f885 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1412,21 +1412,17 @@ nxm_stack_pop_to_nxast(const struct ofpact_stack *stack,
 static void
 nx_stack_push(struct ofpbuf *stack, union mf_subvalue *v)
 {
-    /*
-     *  Assume we have infinitely deep stack. Let ofpbuf grow
-     *  the stack if necessary.
-     */
     ofpbuf_put(stack, v, sizeof *v);
 }
 
 static union mf_subvalue *
 nx_stack_pop(struct ofpbuf *stack)
 {
-    union mf_subvalue* v = NULL;
+    union mf_subvalue *v = NULL;
 
     if (stack->size) {
         stack->size -= sizeof *v;
-        v = (union mf_subvalue *)(ofpbuf_tail(stack));
+        v = (union mf_subvalue *) ofpbuf_tail(stack);
     }
 
     return v;
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 1b92225..f1ff392 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -330,13 +330,13 @@ ofpact_from_nxast(const union ofp_action *a, enum 
ofputil_action_code code,
         break;
 
     case OFPUTIL_NXAST_REG_MOVE:
-        error = nxm_reg_move_from_openflow( 
-               (const struct nx_action_reg_move *) a, out);
+        error = nxm_reg_move_from_openflow(
+            (const struct nx_action_reg_move *) a, out);
         break;
 
     case OFPUTIL_NXAST_REG_LOAD:
         error = nxm_reg_load_from_openflow(
-               (const struct nx_action_reg_load *) a, out);
+            (const struct nx_action_reg_load *) a, out);
         break;
 
     case OFPUTIL_NXAST_STACK_PUSH:
@@ -1146,10 +1146,10 @@ ofpact_check__(const struct ofpact *a, const struct 
flow *flow, int max_ports,
         }
 
     case OFPACT_STACK_PUSH:
-            return nxm_stack_push_check(ofpact_get_STACK_PUSH(a), flow);
+        return nxm_stack_push_check(ofpact_get_STACK_PUSH(a), flow);
 
     case OFPACT_STACK_POP:
-            return nxm_stack_pop_check(ofpact_get_STACK_POP(a), flow);
+        return nxm_stack_pop_check(ofpact_get_STACK_POP(a), flow);
 
     case OFPACT_DEC_TTL:
     case OFPACT_SET_TUNNEL:
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 0440bc4..4006624 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -306,7 +306,7 @@ struct ofpact_reg_move {
 
 /* OFPACT_STACK_PUSH.
  *
- * Used for NXAST_STACK_PUSH and NXAST_STACK_POP */
+ * Used for NXAST_STACK_PUSH and NXAST_STACK_POP. */
 struct ofpact_stack {
     struct ofpact ofpact;
     struct mf_subfield subfield;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index aaf87eb..c538282 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -216,18 +216,10 @@ struct action_xlate_ctx {
      * this flow when actions change header fields. */
     struct flow flow;
 
-    /*
-       The stack for the push and pop actions.
-       Statically allocating 1KB byte stack within this structure.
-       This size should be sufficient for normal use. However, if necessary,
-       the stack can grow into larger size using malloc automatically.
-
-       Each stack element is of the type "union mf_subvalue".
-
-    */
+    /* stack for the push and pop actions.
+     * Each stack element is of the type "union mf_subvalue". */
     struct ofpbuf stack;
-    union mf_subvalue init_stack[1024/sizeof(union mf_subvalue)];   
-                                /*Statically allocated stack. 1KB */
+    union mf_subvalue init_stack[1024 / sizeof(union mf_subvalue)];
 
     /* The packet corresponding to 'flow', or a null pointer if we are
      * revalidating without a packet to refer to. */
@@ -6573,7 +6565,6 @@ xlate_actions(struct action_xlate_ctx *ctx,
     ctx->table_id = 0;
     ctx->exit = false;
 
-    /* Set up the stack, provide initial stack from init_stack */
     ofpbuf_use_stub(&ctx->stack, ctx->init_stack, sizeof ctx->init_stack);
 
     if (ctx->ofproto->has_mirrors || hit_resubmit_limit) {
@@ -6661,7 +6652,6 @@ xlate_actions(struct action_xlate_ctx *ctx,
         fix_sflow_action(ctx);
     }
 
-    /* Clean up the stack */
     ofpbuf_uninit(&ctx->stack);
 }
 
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to