From 558ec83dce80fe37f1dc10567db0e170131d1a44 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 15 Oct 2015 20:25:26 -0700 Subject: [PATCH] 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 Acked-by: Justin Pettit --- ovn/controller/lflow.c | 25 +++++----- ovn/lib/actions.c | 109 ++++++++++++++++++++++++++++++++--------- 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, 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, <able)) { + 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 @@
next;
+
next(table);
- 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 + table to jump to a specific table in the same pipeline.
field = constant;
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; -- 2.39.2