[ovs-dev] [PATCH ovn v2] ovn-controller: Add missing port group lflow references.

Dumitru Ceara dceara at redhat.com
Mon Dec 2 13:20:32 UTC 2019


The commit that adds incremental processing for port-group changes
doesn't store logical flow references for port groups. If a port group
is updated (e.g., a port is added) no logical flow recalculation will be
performed.

To fix this, when parsing the flow expression also store the referenced
port groups and bind them to the logical flows that depend on them. If
the port group is updated then the logical flows referring them will
also be reinstalled.

Reported-by: Daniel Alvarez <dalvarez at redhat.com>
Reported-at: https://bugzilla.redhat.com/1778164
CC: Han Zhou <hzhou at ovn.org>
Fixes: 978f5e90af0a ("ovn-controller: Incremental processing for port-group changes.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>

---
v2: remove Change-Id from commit message.
---
 controller/lflow.c    |  9 ++++++++-
 include/ovn/expr.h    |  4 +++-
 lib/actions.c         |  4 ++--
 lib/expr.c            | 24 +++++++++++++++++-------
 tests/test-ovn.c      |  8 ++++----
 utilities/ovn-trace.c |  2 +-
 6 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index 36150bd..a689320 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -616,14 +616,21 @@ consider_logical_flow(
     struct expr *expr;
 
     struct sset addr_sets_ref = SSET_INITIALIZER(&addr_sets_ref);
+    struct sset port_groups_ref = SSET_INITIALIZER(&port_groups_ref);
     expr = expr_parse_string(lflow->match, &symtab, addr_sets, port_groups,
-                             &addr_sets_ref, &error);
+                             &addr_sets_ref, &port_groups_ref, &error);
     const char *addr_set_name;
     SSET_FOR_EACH (addr_set_name, &addr_sets_ref) {
         lflow_resource_add(lfrr, REF_TYPE_ADDRSET, addr_set_name,
                            &lflow->header_.uuid);
     }
+    const char *port_group_name;
+    SSET_FOR_EACH (port_group_name, &port_groups_ref) {
+        lflow_resource_add(lfrr, REF_TYPE_PORTGROUP, port_group_name,
+                           &lflow->header_.uuid);
+    }
     sset_destroy(&addr_sets_ref);
+    sset_destroy(&port_groups_ref);
 
     if (!error) {
         if (prereqs) {
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index 22f633e..21bf51c 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -390,11 +390,13 @@ void expr_print(const struct expr *);
 struct expr *expr_parse(struct lexer *, const struct shash *symtab,
                         const struct shash *addr_sets,
                         const struct shash *port_groups,
-                        struct sset *addr_sets_ref);
+                        struct sset *addr_sets_ref,
+                        struct sset *port_groups_ref);
 struct expr *expr_parse_string(const char *, const struct shash *symtab,
                                const struct shash *addr_sets,
                                const struct shash *port_groups,
                                struct sset *addr_sets_ref,
+                               struct sset *port_groups_ref,
                                char **errorp);
 
 struct expr *expr_clone(struct expr *);
diff --git a/lib/actions.c b/lib/actions.c
index 586d7b7..051e6c8 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -240,8 +240,8 @@ add_prerequisite(struct action_context *ctx, const char *prerequisite)
     struct expr *expr;
     char *error;
 
-    expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL, NULL,
-                             &error);
+    expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL,
+                             NULL, NULL, &error);
     ovs_assert(!error);
     ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
 }
diff --git a/lib/expr.c b/lib/expr.c
index 71de615..e5e4d21 100644
--- a/lib/expr.c
+++ b/lib/expr.c
@@ -480,7 +480,8 @@ struct expr_context {
     const struct shash *symtab;    /* Symbol table. */
     const struct shash *addr_sets; /* Address set table. */
     const struct shash *port_groups; /* Port group table. */
-    struct sset *addr_sets_ref;       /* The set of address set referenced. */
+    struct sset *addr_sets_ref;      /* The set of address set referenced. */
+    struct sset *port_groups_ref;    /* The set of port groups referenced. */
     bool not;                    /* True inside odd number of NOT operators. */
     unsigned int paren_depth;    /* Depth of nested parentheses. */
 };
@@ -782,6 +783,10 @@ static bool
 parse_port_group(struct expr_context *ctx, struct expr_constant_set *cs,
                  size_t *allocated_values)
 {
+    if (ctx->port_groups_ref) {
+        sset_add(ctx->port_groups_ref, ctx->lexer->token.s);
+    }
+
     struct expr_constant_set *port_group
         = (ctx->port_groups
            ? shash_find_data(ctx->port_groups, ctx->lexer->token.s)
@@ -1296,13 +1301,15 @@ struct expr *
 expr_parse(struct lexer *lexer, const struct shash *symtab,
            const struct shash *addr_sets,
            const struct shash *port_groups,
-           struct sset *addr_sets_ref)
+           struct sset *addr_sets_ref,
+           struct sset *port_groups_ref)
 {
     struct expr_context ctx = { .lexer = lexer,
                                 .symtab = symtab,
                                 .addr_sets = addr_sets,
                                 .port_groups = port_groups,
-                                .addr_sets_ref = addr_sets_ref };
+                                .addr_sets_ref = addr_sets_ref,
+                                .port_groups_ref = port_groups_ref };
     return lexer->error ? NULL : expr_parse__(&ctx);
 }
 
@@ -1317,6 +1324,7 @@ expr_parse_string(const char *s, const struct shash *symtab,
                   const struct shash *addr_sets,
                   const struct shash *port_groups,
                   struct sset *addr_sets_ref,
+                  struct sset *port_groups_ref,
                   char **errorp)
 {
     struct lexer lexer;
@@ -1324,7 +1332,7 @@ expr_parse_string(const char *s, const struct shash *symtab,
     lexer_init(&lexer, s);
     lexer_get(&lexer);
     struct expr *expr = expr_parse(&lexer, symtab, addr_sets, port_groups,
-                                   addr_sets_ref);
+                                   addr_sets_ref, port_groups_ref);
     lexer_force_end(&lexer);
     *errorp = lexer_steal_error(&lexer);
     if (*errorp) {
@@ -1550,7 +1558,8 @@ expr_get_level(const struct expr *expr)
 static enum expr_level
 expr_parse_level(const char *s, const struct shash *symtab, char **errorp)
 {
-    struct expr *expr = expr_parse_string(s, symtab, NULL, NULL, NULL, errorp);
+    struct expr *expr = expr_parse_string(s, symtab, NULL, NULL, NULL, NULL,
+                                          errorp);
     enum expr_level level = expr ? expr_get_level(expr) : EXPR_L_NOMINAL;
     expr_destroy(expr);
     return level;
@@ -1721,7 +1730,7 @@ parse_and_annotate(const char *s, const struct shash *symtab,
     char *error;
     struct expr *expr;
 
-    expr = expr_parse_string(s, symtab, NULL, NULL, NULL, &error);
+    expr = expr_parse_string(s, symtab, NULL, NULL, NULL, NULL, &error);
     if (expr) {
         expr = expr_annotate_(expr, symtab, nesting, &error);
     }
@@ -3445,7 +3454,8 @@ expr_parse_microflow(const char *s, const struct shash *symtab,
     lexer_init(&lexer, s);
     lexer_get(&lexer);
 
-    struct expr *e = expr_parse(&lexer, symtab, addr_sets, port_groups, NULL);
+    struct expr *e = expr_parse(&lexer, symtab, addr_sets, port_groups,
+                                NULL, NULL);
     lexer_force_end(&lexer);
 
     if (e) {
diff --git a/tests/test-ovn.c b/tests/test-ovn.c
index c16f9c5..5366905 100644
--- a/tests/test-ovn.c
+++ b/tests/test-ovn.c
@@ -289,7 +289,7 @@ test_parse_expr__(int steps)
         char *error;
 
         expr = expr_parse_string(ds_cstr(&input), &symtab, &addr_sets,
-                                 &port_groups, NULL, &error);
+                                 &port_groups, NULL, NULL, &error);
         if (!error && steps > 0) {
             expr = expr_annotate(expr, &symtab, &error);
         }
@@ -413,8 +413,8 @@ test_evaluate_expr(struct ovs_cmdl_context *ctx)
     while (!ds_get_test_line(&input, stdin)) {
         struct expr *expr;
 
-        expr = expr_parse_string(ds_cstr(&input), &symtab, NULL, NULL, NULL,
-                                 &error);
+        expr = expr_parse_string(ds_cstr(&input), &symtab, NULL, NULL,
+                                 NULL, NULL, &error);
         if (!error) {
             expr = expr_annotate(expr, &symtab, &error);
         }
@@ -889,7 +889,7 @@ test_tree_shape_exhaustively(struct expr *expr, struct shash *symtab,
 
             char *error;
             modified = expr_parse_string(ds_cstr(&s), symtab, NULL,
-                                         NULL, NULL, &error);
+                                         NULL, NULL, NULL, &error);
             if (error) {
                 fprintf(stderr, "%s fails to parse (%s)\n",
                         ds_cstr(&s), error);
diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
index 19b82e6..2645438 100644
--- a/utilities/ovn-trace.c
+++ b/utilities/ovn-trace.c
@@ -866,7 +866,7 @@ read_flows(void)
         char *error;
         struct expr *match;
         match = expr_parse_string(sblf->match, &symtab, &address_sets,
-                                  &port_groups, NULL, &error);
+                                  &port_groups, NULL, NULL, &error);
         if (error) {
             VLOG_WARN("%s: parsing expression failed (%s)",
                       sblf->match, error);
-- 
1.8.3.1



More information about the dev mailing list