From: Ben Pfaff Date: Tue, 8 Dec 2015 00:56:34 +0000 (-0800) Subject: actions: Bundle action parsing parameters into a structure. X-Git-Url: http://git.cascardo.eti.br/?p=cascardo%2Fovs.git;a=commitdiff_plain;h=1d7b2eceaeb059e42c1e1cd3d32c192e2ab22271 actions: Bundle action parsing parameters into a structure. This will make it easier to add and change parameters, as done in an upcoming commit. Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c index 91ad5dbd8..32491cf04 100644 --- a/ovn/controller/lflow.c +++ b/ovn/controller/lflow.c @@ -305,10 +305,17 @@ lflow_run(struct controller_ctx *ctx, struct hmap *flow_table, char *error; ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub); - error = actions_parse_string(lflow->actions, &symtab, &ldp->ports, - ct_zones, first_ptable, LOG_PIPELINE_LEN, - lflow->table_id, output_ptable, - &ofpacts, &prereqs); + struct action_params ap = { + .symtab = &symtab, + .ports = &ldp->ports, + .ct_zones = ct_zones, + + .n_tables = LOG_PIPELINE_LEN, + .first_ptable = first_ptable, + .cur_ltable = lflow->table_id, + .output_ptable = output_ptable, + }; + error = actions_parse_string(lflow->actions, &ap, &ofpacts, &prereqs); if (error) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s", diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index 6bc452a92..42e7f3b22 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -29,25 +29,9 @@ /* Context maintained during actions_parse(). */ struct action_context { -/* Input. */ - + const struct action_params *ap; /* Parameters. */ struct lexer *lexer; /* Lexer for pulling more tokens. */ - 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. */ - const struct simap *ct_zones; /* Map from port name to conntrack zone. */ - -/* State. */ char *error; /* Error, if any, otherwise NULL. */ - -/* Output. */ struct ofpbuf *ofpacts; /* Actions. */ struct expr *prereqs; /* Prerequisites to apply to match. */ }; @@ -124,7 +108,7 @@ parse_set_action(struct action_context *ctx) struct expr *prereqs; char *error; - error = expr_parse_assignment(ctx->lexer, ctx->symtab, ctx->ports, + error = expr_parse_assignment(ctx->lexer, ctx->ap->symtab, ctx->ap->ports, ctx->ofpacts, &prereqs); if (error) { action_error(ctx, "%s", error); @@ -156,7 +140,7 @@ action_get_int(struct action_context *ctx, int *value) static void parse_next_action(struct action_context *ctx) { - if (!ctx->n_tables) { + if (!ctx->ap->n_tables) { action_error(ctx, "\"next\" action not allowed here."); } else if (lexer_match(ctx->lexer, LEX_T_LPAREN)) { int ltable; @@ -169,16 +153,17 @@ parse_next_action(struct action_context *ctx) return; } - if (ltable >= ctx->n_tables) { + if (ltable >= ctx->ap->n_tables) { action_error(ctx, "\"next\" argument must be in range 0 to %d.", - ctx->n_tables - 1); + ctx->ap->n_tables - 1); return; } - emit_resubmit(ctx, ctx->first_ptable + ltable); + emit_resubmit(ctx, ctx->ap->first_ptable + ltable); } else { - if (ctx->cur_ltable < ctx->n_tables) { - emit_resubmit(ctx, ctx->first_ptable + ctx->cur_ltable + 1); + if (ctx->ap->cur_ltable < ctx->ap->n_tables) { + emit_resubmit(ctx, + ctx->ap->first_ptable + ctx->ap->cur_ltable + 1); } else { action_error(ctx, "\"next\" action not allowed in last table."); } @@ -193,7 +178,7 @@ add_prerequisite(struct action_context *ctx, const char *prerequisite) struct expr *expr; char *error; - expr = expr_parse_string(prerequisite, ctx->symtab, &error); + expr = expr_parse_string(prerequisite, ctx->ap->symtab, &error); ovs_assert(!error); ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr); } @@ -206,8 +191,8 @@ emit_ct(struct action_context *ctx, bool recirc_next, bool commit) /* If "recirc" is set, we automatically go to the next table. */ if (recirc_next) { - if (ctx->cur_ltable < ctx->n_tables) { - ct->recirc_table = ctx->first_ptable + ctx->cur_ltable + 1; + if (ctx->ap->cur_ltable < ctx->ap->n_tables) { + ct->recirc_table = ctx->ap->first_ptable + ctx->ap->cur_ltable + 1; } else { action_error(ctx, "\"ct_next\" action not allowed in last table."); return; @@ -242,7 +227,7 @@ parse_action(struct action_context *ctx) } else if (lexer_match_id(ctx->lexer, "next")) { parse_next_action(ctx); } else if (lexer_match_id(ctx->lexer, "output")) { - emit_resubmit(ctx, ctx->output_ptable); + emit_resubmit(ctx, ctx->ap->output_ptable); } else if (lexer_match_id(ctx->lexer, "ip.ttl")) { if (lexer_match(ctx->lexer, LEX_T_DECREMENT)) { add_prerequisite(ctx, "ip"); @@ -290,35 +275,7 @@ parse_actions(struct action_context *ctx) * Logical_Flow table in ovn-sb(5), and appends the parsed versions of the * actions to 'ofpacts' as "struct ofpact"s. * - * 'symtab' provides a table of "struct expr_symbol"s to support (as one would - * provide to expr_parse()). - * - * 'ports' must be a map from strings (presumably names of ports) to integers - * (as one would provide to expr_to_matches()). Strings used in the actions - * that are not in 'ports' are translated to zero. - * - * 'ct_zones' provides a map from a port name to its connection tracking zone. - * - * 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. - * - * 'next_table_id' should be the OpenFlow table to which the "next" action will - * resubmit, or 0 to disable "next". - * - * - 'output_ptable' should be the OpenFlow table to which the logical - * "output" action will resubmit + * 'ap' provides most of the parameters for translation. * * Some actions add extra requirements (prerequisites) to the flow's match. If * so, this function sets '*prereqsp' to the actions' prerequisites; otherwise, @@ -331,27 +288,18 @@ parse_actions(struct action_context *ctx) * 'lexer'. */ char * OVS_WARN_UNUSED_RESULT -actions_parse(struct lexer *lexer, const struct shash *symtab, - const struct simap *ports, const struct simap *ct_zones, - uint8_t first_ptable, uint8_t n_tables, uint8_t cur_ltable, - uint8_t output_ptable, struct ofpbuf *ofpacts, - struct expr **prereqsp) +actions_parse(struct lexer *lexer, const struct action_params *ap, + struct ofpbuf *ofpacts, struct expr **prereqsp) { size_t ofpacts_start = ofpacts->size; - struct action_context ctx; - ctx.lexer = lexer; - ctx.symtab = symtab; - ctx.ports = ports; - ctx.ct_zones = ct_zones; - 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; - + struct action_context ctx = { + .ap = ap, + .lexer = lexer, + .error = NULL, + .ofpacts = ofpacts, + .prereqs = NULL, + }; parse_actions(&ctx); if (!ctx.error) { @@ -367,20 +315,15 @@ 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, const struct simap *ct_zones, - uint8_t first_table, uint8_t n_tables, uint8_t cur_table, - uint8_t output_table, struct ofpbuf *ofpacts, - struct expr **prereqsp) +actions_parse_string(const char *s, const struct action_params *ap, + struct ofpbuf *ofpacts, struct expr **prereqsp) { struct lexer lexer; char *error; lexer_init(&lexer, s); lexer_get(&lexer); - error = actions_parse(&lexer, symtab, ports, ct_zones, first_table, - n_tables, cur_table, output_table, ofpacts, - prereqsp); + error = actions_parse(&lexer, ap, ofpacts, prereqsp); lexer_destroy(&lexer); return error; diff --git a/ovn/lib/actions.h b/ovn/lib/actions.h index 92f71ded8..2c3644a3f 100644 --- a/ovn/lib/actions.h +++ b/ovn/lib/actions.h @@ -26,18 +26,47 @@ struct ofpbuf; struct shash; struct simap; -char *actions_parse(struct lexer *, const struct shash *symtab, - const struct simap *ports, const struct simap *ct_zones, - uint8_t first_ptable, uint8_t n_tables, uint8_t cur_ltable, - uint8_t output_ptable, struct ofpbuf *ofpacts, - struct expr **prereqsp) +struct action_params { + /* A table of "struct expr_symbol"s to support (as one would provide to + * expr_parse()). */ + const struct shash *symtab; + + /* 'ports' must be a map from strings (presumably names of ports) to + * integers (as one would provide to expr_to_matches()). Strings used in + * the actions that are not in 'ports' are translated to zero. */ + const struct simap *ports; + + /* A map from a port name to its connection tracking zone. */ + const struct simap *ct_zones; + + /* 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_ptable' should be the OpenFlow table to which the logical + * "output" action will resubmit. */ + 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. */ +}; + +char *actions_parse(struct lexer *, const struct action_params *, + 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, - const struct simap *ct_zones, uint8_t first_ptable, - uint8_t n_tables, uint8_t cur_ltable, - uint8_t output_ptable, struct ofpbuf *ofpacts, - struct expr **prereqsp) +char *actions_parse_string(const char *s, const struct action_params *, + struct ofpbuf *ofpacts, struct expr **prereqsp) OVS_WARN_UNUSED_RESULT; #endif /* ovn/actions.h */ diff --git a/tests/test-ovn.c b/tests/test-ovn.c index 7c1fc135e..0a5136968 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -1226,9 +1226,18 @@ 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, - &ct_zones, 16, 16, 10, 64, - &ofpacts, &prereqs); + + struct action_params ap = { + .symtab = &symtab, + .ports = &ports, + .ct_zones = &ct_zones, + + .n_tables = 16, + .first_ptable = 16, + .cur_ltable = 10, + .output_ptable = 64, + }; + error = actions_parse_string(ds_cstr(&input), &ap, &ofpacts, &prereqs); if (!error) { struct ds output;