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 <blp@ovn.org>
Acked-by: Justin Pettit <jpettit@ovn.org>

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 91ad5db..32491cf 100644 (file)
--- 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 6bc452a..42e7f3b 100644 (file)
--- 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 92f71de..2c3644a 100644 (file)
--- 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 7c1fc13..0a51369 100644 (file)
--- 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;