[ovs-dev] [PATCH 01/23] ovn: Extend logical "next" action to jump to arbitrary flow tables.
Ben Pfaff
blp at nicira.com
Sat Oct 10 04:15:22 UTC 2015
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 at nicira.com>
---
ovn/controller/lflow.c | 15 +++++----
ovn/lib/actions.c | 88 +++++++++++++++++++++++++++++++++++++++-----------
ovn/lib/actions.h | 10 +++---
ovn/lib/expr.c | 11 ++-----
ovn/lib/lex.c | 21 ++++++++++++
ovn/lib/lex.h | 2 ++
ovn/ovn-sb.xml | 5 ++-
tests/ovn.at | 10 ++++--
tests/test-ovn.c | 4 +--
9 files changed, 123 insertions(+), 43 deletions(-)
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 9246e61..2b1984a 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 first_table = (ingress
+ ? OFTABLE_LOG_INGRESS_PIPELINE
+ : OFTABLE_LOG_EGRESS_PIPELINE);
+ uint8_t phys_table = first_table + lflow->table_id;
uint8_t output_phys_table = (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_table, LOG_PIPELINE_LEN,
+ lflow->table_id, output_phys_table,
&ofpacts, &prereqs);
if (error) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 4170fc5..2bc495c 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -30,8 +30,10 @@ struct action_context {
/* 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. */
+ uint8_t first_table; /* First OpenFlow table. */
+ uint8_t n_tables; /* Number of OpenFlow tables. */
+ uint8_t cur_table; /* 0 <= cur_table < n_tables. */
+ uint8_t output_table; /* OpenFlow table for 'output' to resubmit. */
const struct simap *ports; /* Map from port name to number. */
/* State. */
@@ -131,6 +133,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 table;
+
+ if (!action_get_int(ctx, &table)) {
+ return;
+ }
+ if (!lexer_match(ctx->lexer, LEX_T_RPAREN)) {
+ action_syntax_error(ctx, "expecting `)'");
+ return;
+ }
+
+ if (table >= 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_table + table);
+ } else {
+ if (ctx->cur_table < ctx->n_tables) {
+ emit_resubmit(ctx, ctx->first_table + ctx->cur_table + 1);
+ } else {
+ action_error(ctx, "\"next\" action not allowed in last table.");
+ }
+ }
+}
+
static void
parse_actions(struct action_context *ctx)
{
@@ -158,13 +202,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_table);
} else {
action_syntax_error(ctx, "expecting action");
}
@@ -188,10 +228,16 @@ 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".
+ * 'first_table' and 'n_tables' define the range of OpenFlow tables that the
+ * logical "next" action should be able to jump to. Logical table 0 maps to
+ * OpenFlow table 'first_table', logical table 1 to 'first_table + 1', and so
+ * on. If 'n_tables' is 0 then "next" is disallowed entirely.
+ *
+ * 'cur_table' is an offset from 'first_table' (e.g. 0 <= cur_table < n_tables)
+ * of the logical flow that contains the actions. If cur_table + 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
+ * 'output_table' should be the OpenFlow table to which the "output" action
* will resubmit
*
* Some actions add extra requirements (prerequisites) to the flow's match. If
@@ -206,8 +252,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_table, uint8_t n_tables, uint8_t cur_table,
+ uint8_t output_table, struct ofpbuf *ofpacts,
struct expr **prereqsp)
{
size_t ofpacts_start = ofpacts->size;
@@ -216,8 +263,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_table = first_table;
+ ctx.n_tables = n_tables;
+ ctx.cur_table = cur_table;
+ ctx.output_table = output_table;
ctx.error = NULL;
ctx.ofpacts = ofpacts;
ctx.prereqs = NULL;
@@ -238,8 +287,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 +297,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 74cd185..ae897d6 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_table,
+ uint8_t n_tables, uint8_t cur_table,
+ uint8_t output_table, 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_table,
+ uint8_t n_tables, uint8_t cur_table,
+ uint8_t output_table, struct ofpbuf *ofpacts,
struct expr **prereqsp)
OVS_WARN_UNUSED_RESULT;
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 8270b82..6fbe3e2 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 2ffcfb9..308d216 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 b5828a2..7ad6f55 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 4609c2d..8c457d4 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 1eb6d0b..330f723 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 774ebdf..0e9d2d2 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;
--
2.1.3
More information about the dev
mailing list