ovn: Extend logical "next" action to jump to arbitrary flow tables.

This makes it easier to route a "destination unreachable" message
generated because of an IP routing failure, because the destination
unreachable message must itself be routed the same way.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Justin Pettit <jpettit@nicira.com>

9 files changed, 144 insertions, 53 deletions

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 9246e61de..70687eab6 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -264,16 +264,16 @@ lflow_run(struct controller_ctx *ctx, struct hmap *flow_table)
             continue;
         }
 
-        /* Translate logical table ID to physical table ID. */
+        /* Determine translation of logical table IDs to physical table IDs. */
         bool ingress = !strcmp(lflow->pipeline, "ingress");
-        uint8_t phys_table = lflow->table_id + (ingress
-                                                ? OFTABLE_LOG_INGRESS_PIPELINE
-                                                : OFTABLE_LOG_EGRESS_PIPELINE);
-        uint8_t next_phys_table
-            = lflow->table_id + 1 < LOG_PIPELINE_LEN ? phys_table + 1 : 0;
-        uint8_t output_phys_table = (ingress
-                                     ? OFTABLE_REMOTE_OUTPUT
-                                     : OFTABLE_LOG_TO_PHY);
+        uint8_t first_ptable = (ingress
+                                ? OFTABLE_LOG_INGRESS_PIPELINE
+                                : OFTABLE_LOG_EGRESS_PIPELINE);
+        uint8_t ptable = first_ptable + lflow->table_id;
+        uint8_t output_ptable = (ingress
+                                 ? OFTABLE_REMOTE_OUTPUT
+                                 : OFTABLE_LOG_TO_PHY);
+
         /* Translate OVN actions into OpenFlow actions.
          *
          * XXX Deny changes to 'outport' in egress pipeline. */
@@ -284,7 +284,8 @@ lflow_run(struct controller_ctx *ctx, struct hmap *flow_table)
 
         ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
         error = actions_parse_string(lflow->actions, &symtab, &ldp->ports,
-                                     next_phys_table, output_phys_table,
+                                     first_ptable, LOG_PIPELINE_LEN,
+                                     lflow->table_id, output_ptable,
                                      &ofpacts, &prereqs);
         if (error) {
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
@@ -329,7 +330,7 @@ lflow_run(struct controller_ctx *ctx, struct hmap *flow_table)
                 m->match.flow.conj_id += conj_id_ofs;
             }
             if (!m->n) {
-                ofctrl_add_flow(flow_table, phys_table, lflow->priority,
+                ofctrl_add_flow(flow_table, ptable, lflow->priority,
                                 &m->match, &ofpacts);
             } else {
                 uint64_t conj_stubs[64 / 8];
@@ -345,7 +346,7 @@ lflow_run(struct controller_ctx *ctx, struct hmap *flow_table)
                     dst->clause = src->clause;
                     dst->n_clauses = src->n_clauses;
                 }
-                ofctrl_add_flow(flow_table, phys_table, lflow->priority,
+                ofctrl_add_flow(flow_table, ptable, lflow->priority,
                                 &m->match, &conj);
                 ofpbuf_uninit(&conj);
             }
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 4170fc508..8afddee7e 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -27,17 +27,24 @@
 
 /* Context maintained during actions_parse(). */
 struct action_context {
-    /* Input. */
+/* Input. */
+
     struct lexer *lexer;        /* Lexer for pulling more tokens. */
-    const struct shash *symtab; /* Symbol table. */
-    uint8_t next_table_id;      /* OpenFlow table for 'next' to resubmit. */
-    uint8_t output_table_id;    /* OpenFlow table for 'output' to resubmit. */
     const struct simap *ports;  /* Map from port name to number. */
+    const struct shash *symtab; /* Symbol table. */
+
+    /* OVN maps each logical flow table (ltable), one-to-one, onto a physical
+     * OpenFlow flow table (ptable).  These members describe the mapping and
+     * data related to flow tables. */
+    uint8_t n_tables;           /* Number of flow tables. */
+    uint8_t first_ptable;       /* First OpenFlow table. */
+    uint8_t cur_ltable;         /* 0 <= cur_ltable < n_tables. */
+    uint8_t output_ptable;      /* OpenFlow table for 'output' to resubmit. */
 
-    /* State. */
+/* State. */
     char *error;                /* Error, if any, otherwise NULL. */
 
-    /* Output. */
+/* Output. */
     struct ofpbuf *ofpacts;     /* Actions. */
     struct expr *prereqs;       /* Prerequisites to apply to match. */
 };
@@ -131,6 +138,48 @@ emit_resubmit(struct action_context *ctx, uint8_t table_id)
     resubmit->table_id = table_id;
 }
 
+static bool
+action_get_int(struct action_context *ctx, int *value)
+{
+    bool ok = lexer_get_int(ctx->lexer, value);
+    if (!ok) {
+        action_syntax_error(ctx, "expecting small integer");
+    }
+    return ok;
+}
+
+static void
+parse_next_action(struct action_context *ctx)
+{
+    if (!ctx->n_tables) {
+        action_error(ctx, "\"next\" action not allowed here.");
+    } else if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
+        int ltable;
+
+        if (!action_get_int(ctx, &ltable)) {
+            return;
+        }
+        if (!lexer_match(ctx->lexer, LEX_T_RPAREN)) {
+            action_syntax_error(ctx, "expecting `)'");
+            return;
+        }
+
+        if (ltable >= ctx->n_tables) {
+            action_error(ctx, "\"next\" argument must be in range 0 to %d.",
+                         ctx->n_tables - 1);
+            return;
+        }
+
+        emit_resubmit(ctx, ctx->first_ptable + ltable);
+    } else {
+        if (ctx->cur_ltable < ctx->n_tables) {
+            emit_resubmit(ctx, ctx->first_ptable + ctx->cur_ltable + 1);
+        } else {
+            action_error(ctx, "\"next\" action not allowed in last table.");
+        }
+    }
+}
+
 static void
 parse_actions(struct action_context *ctx)
 {
@@ -158,13 +207,9 @@ parse_actions(struct action_context *ctx)
             || lookahead == LEX_T_LSQUARE) {
             parse_set_action(ctx);
         } else if (lexer_match_id(ctx->lexer, "next")) {
-            if (ctx->next_table_id) {
-                emit_resubmit(ctx, ctx->next_table_id);
-            } else {
-                action_error(ctx, "\"next\" action not allowed here.");
-            }
+            parse_next_action(ctx);
         } else if (lexer_match_id(ctx->lexer, "output")) {
-            emit_resubmit(ctx, ctx->output_table_id);
+            emit_resubmit(ctx, ctx->output_ptable);
         } else {
             action_syntax_error(ctx, "expecting action");
         }
@@ -188,11 +233,23 @@ parse_actions(struct action_context *ctx)
  * (as one would provide to expr_to_matches()).  Strings used in the actions
  * that are not in 'ports' are translated to zero.
  *
- * 'next_table_id' should be the OpenFlow table to which the "next" action will
- * resubmit, or 0 to disable "next".
+ * OVN maps each logical flow table (ltable), one-to-one, onto a physical
+ * OpenFlow flow table (ptable).  A number of parameters describe this mapping
+ * and data related to flow tables:
+ *
+ *     - 'first_ptable' and 'n_tables' define the range of OpenFlow tables to
+ *       which the logical "next" action should be able to jump.  Logical table
+ *       0 maps to OpenFlow table 'first_ptable', logical table 1 to
+ *       'first_ptable + 1', and so on.  If 'n_tables' is 0 then "next" is
+ *       disallowed entirely.
+ *
+ *     - 'cur_ltable' is an offset from 'first_ptable' (e.g. 0 <= cur_ltable <
+ *       n_ptables) of the logical flow that contains the actions.  If
+ *       cur_ltable + 1 < n_tables, then this defines the default table that
+ *       "next" will jump to.
  *
- * 'output_table_id' should be the OpenFlow table to which the "output" action
- * will resubmit
+ *     - 'output_ptable' should be the OpenFlow table to which the logical
+ *       "output" action will resubmit
  *
  * Some actions add extra requirements (prerequisites) to the flow's match.  If
  * so, this function sets '*prereqsp' to the actions' prerequisites; otherwise,
@@ -206,8 +263,9 @@ parse_actions(struct action_context *ctx)
   */
 char * OVS_WARN_UNUSED_RESULT
 actions_parse(struct lexer *lexer, const struct shash *symtab,
-              const struct simap *ports, uint8_t next_table_id,
-              uint8_t output_table_id, struct ofpbuf *ofpacts,
+              const struct simap *ports,
+              uint8_t first_ptable, uint8_t n_tables, uint8_t cur_ltable,
+              uint8_t output_ptable, struct ofpbuf *ofpacts,
               struct expr **prereqsp)
 {
     size_t ofpacts_start = ofpacts->size;
@@ -216,8 +274,10 @@ actions_parse(struct lexer *lexer, const struct shash *symtab,
     ctx.lexer = lexer;
     ctx.symtab = symtab;
     ctx.ports = ports;
-    ctx.next_table_id = next_table_id;
-    ctx.output_table_id = output_table_id;
+    ctx.first_ptable = first_ptable;
+    ctx.n_tables = n_tables;
+    ctx.cur_ltable = cur_ltable;
+    ctx.output_ptable = output_ptable;
     ctx.error = NULL;
     ctx.ofpacts = ofpacts;
     ctx.prereqs = NULL;
@@ -238,8 +298,9 @@ actions_parse(struct lexer *lexer, const struct shash *symtab,
 /* Like actions_parse(), but the actions are taken from 's'. */
 char * OVS_WARN_UNUSED_RESULT
 actions_parse_string(const char *s, const struct shash *symtab,
-                     const struct simap *ports, uint8_t next_table_id,
-                     uint8_t output_table_id, struct ofpbuf *ofpacts,
+                     const struct simap *ports, uint8_t first_table,
+                     uint8_t n_tables, uint8_t cur_table,
+                     uint8_t output_table, struct ofpbuf *ofpacts,
                      struct expr **prereqsp)
 {
     struct lexer lexer;
@@ -247,8 +308,8 @@ actions_parse_string(const char *s, const struct shash *symtab,
 
     lexer_init(&lexer, s);
     lexer_get(&lexer);
-    error = actions_parse(&lexer, symtab, ports, next_table_id,
-                          output_table_id, ofpacts, prereqsp);
+    error = actions_parse(&lexer, symtab, ports, first_table, n_tables,
+                          cur_table, output_table, ofpacts, prereqsp);
     lexer_destroy(&lexer);
 
     return error;
diff --git a/ovn/lib/actions.h b/ovn/lib/actions.h
index 74cd18548..a84c05b06 100644
--- a/ovn/lib/actions.h
+++ b/ovn/lib/actions.h
@@ -27,13 +27,15 @@ struct shash;
 struct simap;
 
 char *actions_parse(struct lexer *, const struct shash *symtab,
-                    const struct simap *ports, uint8_t next_table_id,
-                    uint8_t output_table_id, struct ofpbuf *ofpacts,
+                    const struct simap *ports, uint8_t first_ptable,
+                    uint8_t n_tables, uint8_t cur_ltable,
+                    uint8_t output_ptable, struct ofpbuf *ofpacts,
                     struct expr **prereqsp)
     OVS_WARN_UNUSED_RESULT;
 char *actions_parse_string(const char *s, const struct shash *symtab,
-                           const struct simap *ports, uint8_t next_table_id,
-                           uint8_t output_table_id, struct ofpbuf *ofpacts,
+                           const struct simap *ports, uint8_t first_ptable,
+                           uint8_t n_tables, uint8_t cur_ltable,
+                           uint8_t output_ptable, struct ofpbuf *ofpacts,
                            struct expr **prereqsp)
     OVS_WARN_UNUSED_RESULT;
 
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 8ec62b531..8a69e3e6a 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -668,16 +668,11 @@ exit:
 static bool
 expr_get_int(struct expr_context *ctx, int *value)
 {
-    if (ctx->lexer->token.type == LEX_T_INTEGER
-        && ctx->lexer->token.format == LEX_F_DECIMAL
-        && ntohll(ctx->lexer->token.value.integer) <= INT_MAX) {
-        *value = ntohll(ctx->lexer->token.value.integer);
-        lexer_get(ctx->lexer);
-        return true;
-    } else {
+    bool ok = lexer_get_int(ctx->lexer, value);
+    if (!ok) {
         expr_syntax_error(ctx, "expecting small integer.");
-        return false;
     }
+    return ok;
 }
 
 static bool
diff --git a/ovn/lib/lex.c b/ovn/lib/lex.c
index 2ffcfb94d..308d216e8 100644
--- a/ovn/lib/lex.c
+++ b/ovn/lib/lex.c
@@ -750,3 +750,24 @@ lexer_match_id(struct lexer *lexer, const char *id)
         return false;
     }
 }
+
+bool
+lexer_is_int(const struct lexer *lexer)
+{
+    return (lexer->token.type == LEX_T_INTEGER
+            && lexer->token.format == LEX_F_DECIMAL
+            && ntohll(lexer->token.value.integer) <= INT_MAX);
+}
+
+bool
+lexer_get_int(struct lexer *lexer, int *value)
+{
+    if (lexer_is_int(lexer)) {
+        *value = ntohll(lexer->token.value.integer);
+        lexer_get(lexer);
+        return true;
+    } else {
+        *value = 0;
+        return false;
+    }
+}
diff --git a/ovn/lib/lex.h b/ovn/lib/lex.h
index b5828a2c7..7ad6f55a2 100644
--- a/ovn/lib/lex.h
+++ b/ovn/lib/lex.h
@@ -109,5 +109,7 @@ enum lex_type lexer_get(struct lexer *);
 enum lex_type lexer_lookahead(const struct lexer *);
 bool lexer_match(struct lexer *, enum lex_type);
 bool lexer_match_id(struct lexer *, const char *id);
+bool lexer_is_int(const struct lexer *);
+bool lexer_get_int(struct lexer *, int *value);
 
 #endif /* ovn/lex.h */
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index 088c4b76b..9892ff0aa 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -767,8 +767,11 @@
         </dd>
 
         <dt><code>next;</code></dt>
+        <dt><code>next(<var>table</var>);</code></dt>
         <dd>
-          Executes the next logical datapath table as a subroutine.
+          Executes another logical datapath table as a subroutine.  By default,
+          the table after the current one is executed.  Specify
+          <var>table</var> to jump to a specific table in the same pipeline.
         </dd>
 
         <dt><code><var>field</var> = <var>constant</var>;</code></dt>
diff --git a/tests/ovn.at b/tests/ovn.at
index b8b9e36d4..9195ec432 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -438,9 +438,11 @@ dnl Text before => is input, text after => is expected output.
 AT_DATA([test-cases.txt], [[
 # Positive tests.
 drop; => actions=drop, prereqs=1
-next; => actions=resubmit(,11), prereqs=1
+next; => actions=resubmit(,27), prereqs=1
+next(0); => actions=resubmit(,16), prereqs=1
+next(15); => actions=resubmit(,31), prereqs=1
 output; => actions=resubmit(,64), prereqs=1
-outport="eth0"; next; outport="LOCAL"; next; => actions=set_field:0x5->reg7,resubmit(,11),set_field:0xfffe->reg7,resubmit(,11), prereqs=1
+outport="eth0"; next; outport="LOCAL"; next; => actions=set_field:0x5->reg7,resubmit(,27),set_field:0xfffe->reg7,resubmit(,27), prereqs=1
 tcp.dst=80; => actions=set_field:80->tcp_dst, prereqs=ip.proto == 0x6 && (eth.type == 0x800 || eth.type == 0x86dd)
 eth.dst[40] = 1; => actions=set_field:01:00:00:00:00:00/01:00:00:00:00:00->eth_dst, prereqs=1
 vlan.pcp = 2; => actions=set_field:0x4000/0xe000->vlan_tci, prereqs=vlan.tci[12]
@@ -471,6 +473,10 @@ next; drop; => Syntax error at `drop' expecting action.
 # Missing ";":
 next => Syntax error at end of input expecting ';'.
 
+next(); => Syntax error at `)' expecting small integer.
+next(10; => Syntax error at `;' expecting `)'.
+next(16); => "next" argument must be in range 0 to 15.
+
 inport[1] = 1; => Cannot select subfield of string field inport.
 ip.proto[1] = 1; => Cannot select subfield of nominal field ip.proto.
 eth.dst[40] == 1; => Syntax error at `==' expecting `='.
diff --git a/tests/test-ovn.c b/tests/test-ovn.c
index 774ebdffa..0e9d2d2fc 100644
--- a/tests/test-ovn.c
+++ b/tests/test-ovn.c
@@ -1225,8 +1225,8 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED)
         char *error;
 
         ofpbuf_init(&ofpacts, 0);
-        error = actions_parse_string(ds_cstr(&input), &symtab, &ports, 11, 64,
-                                     &ofpacts, &prereqs);
+        error = actions_parse_string(ds_cstr(&input), &symtab, &ports,
+                                     16, 16, 10, 64, &ofpacts, &prereqs);
         if (!error) {
             struct ds output;