[ovs-dev] [PATCH v2 4/7] actions: Omit table number when possible for formatting "next" action.

[Ben Pfaff]

Until now, formatting the "next" action has always required including
the table number, because the action struct didn't include enough context
so that the formatter could decide whether the table number was the next
table or some other table.  This is more or less OK, but an upcoming commit
will add a "pipeline" field to the "next" action, which means that the same
policy there would require that the pipeline always be printed.  That's a
little obnoxious because 99+% of the time, the pipeline to be printed is
the same pipeline that the flow is in and printing it would be distracting.
So it's better to store some context to help with formatting.  This commit
begins adopting that policy for the existing table number field.

Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 include/ovn/actions.h |  8 ++++++++
 ovn/lib/actions.c     | 43 +++++++++++++++++++++----------------------
 tests/ovn.at          |  8 ++++----
 3 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 92c8888..38764fe 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -143,7 +143,15 @@ struct ovnact_null {
 /* OVNACT_NEXT. */
 struct ovnact_next {
     struct ovnact ovnact;
+
+    /* Arguments. */
     uint8_t ltable;             /* Logical table ID of next table. */
+
+    /* Information about the flow that the action is in.  This does not affect
+     * behavior, since the implementation of "next" doesn't depend on the
+     * source table or pipeline.  It does affect how ovnacts_format() prints
+     * the action. */
+    uint8_t src_ltable;            /* Logical table ID of source table. */
 };
 
 /* OVNACT_LOAD. */
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 2162dad..bcc690f 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -259,36 +259,35 @@ parse_NEXT(struct action_context *ctx)
 {
     if (!ctx->pp->n_tables) {
         lexer_error(ctx->lexer, "\"next\" action not allowed here.");
-    } else if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
-        int ltable;
-
-        if (!lexer_force_int(ctx->lexer, &ltable) ||
-            !lexer_force_match(ctx->lexer, LEX_T_RPAREN)) {
-            return;
-        }
+        return;
+    }
 
-        if (ltable >= ctx->pp->n_tables) {
-            lexer_error(ctx->lexer,
-                        "\"next\" argument must be in range 0 to %d.",
-                         ctx->pp->n_tables - 1);
-            return;
-        }
+    int table = ctx->pp->cur_ltable + 1;
+    if (lexer_match(ctx->lexer, LEX_T_LPAREN)
+        && (!lexer_force_int(ctx->lexer, &table) ||
+            !lexer_force_match(ctx->lexer, LEX_T_RPAREN))) {
+        return;
+    }
 
-        ovnact_put_NEXT(ctx->ovnacts)->ltable = ltable;
-    } else {
-        if (ctx->pp->cur_ltable < ctx->pp->n_tables) {
-            ovnact_put_NEXT(ctx->ovnacts)->ltable = ctx->pp->cur_ltable + 1;
-        } else {
-            lexer_error(ctx->lexer,
-                        "\"next\" action not allowed in last table.");
-        }
+    if (table >= ctx->pp->n_tables) {
+        lexer_error(ctx->lexer,
+                    "\"next\" action cannot advance beyond table %d.",
+                    ctx->pp->n_tables - 1);
     }
+
+    struct ovnact_next *next = ovnact_put_NEXT(ctx->ovnacts);
+    next->ltable = table;
+    next->src_ltable = ctx->pp->cur_ltable;
 }
 
 static void
 format_NEXT(const struct ovnact_next *next, struct ds *s)
 {
-    ds_put_format(s, "next(%d);", next->ltable);
+    if (next->ltable != next->src_ltable + 1) {
+        ds_put_format(s, "next(%d);", next->ltable);
+    } else {
+        ds_put_cstr(s, "next;");
+    }
 }
 
 static void
diff --git a/tests/ovn.at b/tests/ovn.at
index 67d73c5..f71a4af 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -643,9 +643,9 @@ output;
 
 # next
 next;
-    formats as next(11);
     encodes as resubmit(,27)
 next(11);
+    formats as next;
     encodes as resubmit(,27)
 next(0);
     encodes as resubmit(,16)
@@ -657,7 +657,7 @@ next();
 next(10;
     Syntax error at `;' expecting `)'.
 next(16);
-    "next" argument must be in range 0 to 15.
+    "next" action cannot advance beyond table 15.
 
 # Loading a constant value.
 tcp.dst=80;
@@ -678,7 +678,7 @@ ip.ttl=4;
     encodes as set_field:4->nw_ttl
     has prereqs eth.type == 0x800 || eth.type == 0x86dd
 outport="eth0"; next; outport="LOCAL"; next;
-    formats as outport = "eth0"; next(11); outport = "LOCAL"; next(11);
+    formats as outport = "eth0"; next; outport = "LOCAL"; next;
     encodes as set_field:0x5->reg15,resubmit(,27),set_field:0xfffe->reg15,resubmit(,27)
 
 inport[1] = 1;
@@ -868,7 +868,7 @@ ct_snat();
     Syntax error at `)' expecting IPv4 address.
 
 # clone
-clone { ip4.dst = 255.255.255.255; output; }; next(11);
+clone { ip4.dst = 255.255.255.255; output; }; next;
     encodes as clone(set_field:255.255.255.255->ip_dst,resubmit(,64)),resubmit(,27)
     has prereqs eth.type == 0x800
 
-- 
2.10.2