From 07e82c86d4f6540b701ce43b0c39859a957ab820 Mon Sep 17 00:00:00 2001

From: Dumitru Ceara <dceara@redhat.com>

Date: Mon, 2 Dec 2019 14:20:32 +0100

Subject: [PATCH ovn] ovn-controller: Add missing port group lflow

 references.

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@redhat.com>

Reported-at: https://bugzilla.redhat.com/1778164

CC: Han Zhou <hzhou@ovn.org>

Fixes: 978f5e90af0a ("ovn-controller: Incremental processing for port-group changes.")

Tested-By: Daniel Alvarez <dalvarez@redhat.com>

Signed-off-by: Dumitru Ceara <dceara@redhat.com>

Signed-off-by: Han Zhou <hzhou@ovn.org>

(cherry picked from upstream commit bbcac48d443e98cbe47d3941f7e192c9c3443cb5)

---

 include/ovn/expr.h        |  4 +++-

 ovn/controller/lflow.c    |  9 ++++++++-

 ovn/lib/actions.c         |  4 ++--

 ovn/lib/expr.c            | 24 +++++++++++++++++-------

 ovn/utilities/ovn-trace.c |  2 +-

 tests/test-ovn.c          |  8 ++++----

 6 files changed, 35 insertions(+), 16 deletions(-)

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/ovn/controller/lflow.c b/ovn/controller/lflow.c

index 9dda76f..9dbce91 100644

--- a/ovn/controller/lflow.c

+++ b/ovn/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/ovn/lib/actions.c b/ovn/lib/actions.c

index 7857f65..82a9334 100644

--- a/ovn/lib/actions.c

+++ b/ovn/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/ovn/lib/expr.c b/ovn/lib/expr.c

index ca72075..4af1cb2 100644

--- a/ovn/lib/expr.c

+++ b/ovn/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/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c

index 7ed4a38..05d7113 100644

--- a/ovn/utilities/ovn-trace.c

+++ b/ovn/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);

diff --git a/tests/test-ovn.c b/tests/test-ovn.c

index a7adc1c..b6eea4d 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);

-- 

1.8.3.1