From 3cdc2f9cdd9b4911a236c731dfa76535e3af38e6 Mon Sep 17 00:00:00 2001

From: Dumitru Ceara <dceara@redhat.com>

Date: Mon, 29 Jun 2020 17:24:41 +0200

Subject: [PATCH] Split SB Port_Group per datapath.

In order to avoid ovn-controller reinstalling all logical flows that

refer a port_group when some ports are added/removed from the port group

we now change the way ovn-northd populates the Southbound DB Port_Group

table.

Instead of copying NB.Port_Group.name to SB.Port_Group.name we now

create one SB.Port_Group record for every datapath that has ports

referenced by the NB.Port_Group.ports field. In order to maintain the

SB.Port_Group.name uniqueness constraint, ovn-northd populates the field

with the value: <SB.Logical_Datapath.tunnel_key>_<NB.Port_Group.name>.

In specific scenarios we see significant improvements in time to

install/remove all logical flows to/from OVS. One such scenario, in the

BZ referenced below has:

$ ovn-nbctl acl-list pg

  from-lport  1001 (inport == @pg && ip) drop

    to-lport  1001 (outport == @pg && ip) drop

Then, incrementally, creates new logical ports on different logical

switches, binds them to OVS interfaces and adds them to the port_group.

Measuring the total time to perform the above steps 500 times (for 500

new ports attached to 100 switches, 5 per switch) on a test setup

we observe an improvement of 50% in time it takes to install all

openflow rules when port_groups are split in the SB database.

Suggested-by: Numan Siddique <numans@ovn.org>

Reported-by: Venkata Anil <anilvenkata@redhat.com>

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

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

Acked-by: Numan Siddique <numans@ovn.org>

Signed-off-by: Mark Michelson <mmichels@redhat.com>

Conflicts:

	TODO.rst

	lib/ovn-util.h

Change-Id: Ibb8ffc5dbf4deb33a46e94b3f7b57c248d669073

---

 TODO.rst              |  8 ++++++

 controller/lflow.c    |  4 ++-

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

 lib/actions.c         |  2 +-

 lib/expr.c            | 48 ++++++++++++++++++++++++-------

 lib/ovn-util.h        |  7 +++++

 northd/ovn-northd.c   | 79 ++++++++++++++++++++++++++++++++++-----------------

 tests/ovn-northd.at   | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++

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

 utilities/ovn-trace.c |  3 +-

 10 files changed, 198 insertions(+), 46 deletions(-)

diff --git a/TODO.rst b/TODO.rst

index 809d1c9..cfd33be 100644

--- a/TODO.rst

+++ b/TODO.rst

@@ -149,3 +149,11 @@ OVN To-do List

 * OVN Interconnection

   * Packaging for RHEL, Debian, etc.

+

+* ovn-controller: Remove backwards compatibility for Southbound DB Port_Group

+  names in expr.c a few releases after the 20.09 version. Right now

+  ovn-controller maintains backwards compatibility when connecting to a

+  SB database that doesn't store Port_Group.name as

+  <Logical_Datapath.tunnel_key_NB-Port_Group.name>. This causes an additional

+  hashtable lookup in parse_port_group() which can be avoided when we are sure

+  that the Southbound DB uses the new format.

diff --git a/controller/lflow.c b/controller/lflow.c

index 01214a3..0e57327 100644

--- a/controller/lflow.c

+++ b/controller/lflow.c

@@ -552,7 +552,9 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow,

     struct sset port_groups_ref = SSET_INITIALIZER(&port_groups_ref);

     expr = expr_parse_string(lflow->match, &symtab, l_ctx_in->addr_sets,

                              l_ctx_in->port_groups,

-                             &addr_sets_ref, &port_groups_ref, &error);

+                             &addr_sets_ref, &port_groups_ref,

+                             lflow->logical_datapath->tunnel_key,

+                             &error);

     const char *addr_set_name;

     SSET_FOR_EACH (addr_set_name, &addr_sets_ref) {

         lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_ADDRSET, addr_set_name,

diff --git a/include/ovn/expr.h b/include/ovn/expr.h

index 21bf51c..9838251 100644

--- a/include/ovn/expr.h

+++ b/include/ovn/expr.h

@@ -391,12 +391,14 @@ 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 *port_groups_ref);

+                        struct sset *port_groups_ref,

+                        int64_t dp_id);

 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,

+                               int64_t dp_id,

                                char **errorp);

 struct expr *expr_clone(struct expr *);

diff --git a/lib/actions.c b/lib/actions.c

index 3181126..d107871 100644

--- a/lib/actions.c

+++ b/lib/actions.c

@@ -242,7 +242,7 @@ add_prerequisite(struct action_context *ctx, const char *prerequisite)

     char *error;

     expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL,

-                             NULL, NULL, &error);

+                             NULL, NULL, 0, &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 078d178..497b2ac 100644

--- a/lib/expr.c

+++ b/lib/expr.c

@@ -29,6 +29,7 @@

 #include "simap.h"

 #include "sset.h"

 #include "util.h"

+#include "ovn-util.h"

 VLOG_DEFINE_THIS_MODULE(expr);

@@ -482,6 +483,10 @@ struct expr_context {

     const struct shash *port_groups; /* Port group table. */

     struct sset *addr_sets_ref;      /* The set of address set referenced. */

     struct sset *port_groups_ref;    /* The set of port groups referenced. */

+    int64_t dp_id;                   /* The tunnel_key of the datapath for

+                                        which we're parsing the current

+                                        expression. */

+

     bool not;                    /* True inside odd number of NOT operators. */

     unsigned int paren_depth;    /* Depth of nested parentheses. */

 };

@@ -783,14 +788,32 @@ static bool

 parse_port_group(struct expr_context *ctx, struct expr_constant_set *cs,

                  size_t *allocated_values)

 {

+    struct ds sb_name = DS_EMPTY_INITIALIZER;

+

+    get_sb_port_group_name(ctx->lexer->token.s, ctx->dp_id, &sb_name);

     if (ctx->port_groups_ref) {

-        sset_add(ctx->port_groups_ref, ctx->lexer->token.s);

+        sset_add(ctx->port_groups_ref, ds_cstr(&sb_name));

+    }

+

+    struct expr_constant_set *port_group = NULL;

+

+    if (ctx->port_groups) {

+        port_group = shash_find_data(ctx->port_groups, ds_cstr(&sb_name));

+        if (!port_group) {

+            /* For backwards compatibility (e.g., ovn-controller was

+             * upgraded but ovn-northd not yet), perform an additional

+             * lookup because the NB Port_Group.name might have been

+             * stored as is in the SB Port_Group.name field.

+             */

+            port_group = shash_find_data(ctx->port_groups,

+                                         ctx->lexer->token.s);

+            if (ctx->port_groups_ref) {

+                sset_add(ctx->port_groups_ref, ctx->lexer->token.s);

+            }

+        }

     }

+    ds_destroy(&sb_name);

-    struct expr_constant_set *port_group

-        = (ctx->port_groups

-           ? shash_find_data(ctx->port_groups, ctx->lexer->token.s)

-           : NULL);

     if (!port_group) {

         lexer_syntax_error(ctx->lexer, "expecting port group name");

         return false;

@@ -1302,14 +1325,16 @@ 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 *port_groups_ref)

+           struct sset *port_groups_ref,

+           int64_t dp_id)

 {

     struct expr_context ctx = { .lexer = lexer,

                                 .symtab = symtab,

                                 .addr_sets = addr_sets,

                                 .port_groups = port_groups,

                                 .addr_sets_ref = addr_sets_ref,

-                                .port_groups_ref = port_groups_ref };

+                                .port_groups_ref = port_groups_ref,

+                                .dp_id = dp_id };

     return lexer->error ? NULL : expr_parse__(&ctx;;

 }

@@ -1325,6 +1350,7 @@ expr_parse_string(const char *s, const struct shash *symtab,

                   const struct shash *port_groups,

                   struct sset *addr_sets_ref,

                   struct sset *port_groups_ref,

+                  int64_t dp_id,

                   char **errorp)

 {

     struct lexer lexer;

@@ -1332,7 +1358,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, port_groups_ref);

+                                   addr_sets_ref, port_groups_ref, dp_id);

     lexer_force_end(&lexer);

     *errorp = lexer_steal_error(&lexer);

     if (*errorp) {

@@ -1558,7 +1584,7 @@ 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, NULL,

+    struct expr *expr = expr_parse_string(s, symtab, NULL, NULL, NULL, NULL, 0,

                                           errorp);

     enum expr_level level = expr ? expr_get_level(expr) : EXPR_L_NOMINAL;

     expr_destroy(expr);

@@ -1730,7 +1756,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, NULL, &error);

+    expr = expr_parse_string(s, symtab, NULL, NULL, NULL, NULL, 0, &error);

     if (expr) {

         expr = expr_annotate_(expr, symtab, nesting, &error);

     }

@@ -3456,7 +3482,7 @@ expr_parse_microflow(const char *s, const struct shash *symtab,

     lexer_get(&lexer);

     struct expr *e = expr_parse(&lexer, symtab, addr_sets, port_groups,

-                                NULL, NULL);

+                                NULL, NULL, 0);

     lexer_force_end(&lexer);

     if (e) {

diff --git a/lib/ovn-util.h b/lib/ovn-util.h

index ec5f2cf..9c6d357 100644

--- a/lib/ovn-util.h

+++ b/lib/ovn-util.h

@@ -111,6 +111,13 @@ bool ovn_tnlid_in_use(const struct hmap *set, uint32_t tnlid);

 uint32_t ovn_allocate_tnlid(struct hmap *set, const char *name, uint32_t min,

                             uint32_t max, uint32_t *hint);

+static inline void

+get_sb_port_group_name(const char *nb_pg_name, int64_t dp_tunnel_key,

+                       struct ds *sb_pg_name)

+{

+    ds_put_format(sb_pg_name, "%"PRId64"_%s", dp_tunnel_key, nb_pg_name);

+}

+

 char *ovn_chassis_redirect_name(const char *port_name);

 void ovn_set_pidfile(const char *name);

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c

index fc25031..8a809d0 100644

--- a/northd/ovn-northd.c

+++ b/northd/ovn-northd.c

@@ -4457,7 +4457,11 @@ build_dhcpv6_action(struct ovn_port *op, struct in6_addr *offer_ip,

 struct ovn_port_group_ls {

     struct hmap_node key_node;  /* Index on 'key'. */

     struct uuid key;            /* nb_ls->header_.uuid. */

-    const struct nbrec_logical_switch *nb_ls;

+    struct ovn_datapath *od;

+

+    struct ovn_port **ports; /* Ports in 'od' referrenced by the PG. */

+    size_t n_ports;

+    size_t n_allocated_ports;

 };

 struct ovn_port_group {

@@ -4467,14 +4471,14 @@ struct ovn_port_group {

     struct hmap nb_lswitches;   /* NB lswitches related to the port group */

 };

-static void

-ovn_port_group_ls_add(struct ovn_port_group *pg,

-                      const struct nbrec_logical_switch *nb_ls)

+static struct ovn_port_group_ls *

+ovn_port_group_ls_add(struct ovn_port_group *pg, struct ovn_datapath *od)

 {

     struct ovn_port_group_ls *pg_ls = xzalloc(sizeof *pg_ls);

-    pg_ls->key = nb_ls->header_.uuid;

-    pg_ls->nb_ls = nb_ls;

+    pg_ls->key = od->nbs->header_.uuid;

+    pg_ls->od = od;

     hmap_insert(&pg->nb_lswitches, &pg_ls->key_node, uuid_hash(&pg_ls->key));

+    return pg_ls;

 }

 static struct ovn_port_group_ls *

@@ -4491,6 +4495,18 @@ ovn_port_group_ls_find(struct ovn_port_group *pg, const struct uuid *ls_uuid)

     return NULL;

 }

+static void

+ovn_port_group_ls_add_port(struct ovn_port_group_ls *pg_ls,

+                           struct ovn_port *op)

+{

+    if (pg_ls->n_ports == pg_ls->n_allocated_ports) {

+        pg_ls->ports = x2nrealloc(pg_ls->ports,

+                                  &pg_ls->n_allocated_ports,

+                                  sizeof *pg_ls->ports);

+    }

+    pg_ls->ports[pg_ls->n_ports++] = op;

+}

+

 struct ovn_ls_port_group {

     struct hmap_node key_node;  /* Index on 'key'. */

     struct uuid key;            /* nb_pg->header_.uuid. */

@@ -5250,6 +5266,7 @@ ovn_port_group_destroy(struct hmap *pgs, struct ovn_port_group *pg)

         hmap_remove(pgs, &pg->key_node);

         struct ovn_port_group_ls *ls;

         HMAP_FOR_EACH_POP (ls, key_node, &pg->nb_lswitches) {

+            free(ls->ports);

             free(ls);

         }

         hmap_destroy(&pg->nb_lswitches);

@@ -5287,9 +5304,10 @@ build_port_group_lswitches(struct northd_context *ctx, struct hmap *pgs,

             struct ovn_port_group_ls *pg_ls =

                 ovn_port_group_ls_find(pg, &op->od->nbs->header_.uuid);

             if (!pg_ls) {

-                ovn_port_group_ls_add(pg, op->od->nbs);

+                pg_ls = ovn_port_group_ls_add(pg, op->od);

                 ovn_ls_port_group_add(&op->od->nb_pgs, nb_pg);

             }

+            ovn_port_group_ls_add_port(pg_ls, op);

         }

     }

 }

@@ -10454,7 +10472,7 @@ sync_address_sets(struct northd_context *ctx)

  * contains lport uuids, while in OVN_Southbound we store the lport names.

  */

 static void

-sync_port_groups(struct northd_context *ctx)

+sync_port_groups(struct northd_context *ctx, struct hmap *pgs)

 {

     struct shash sb_port_groups = SHASH_INITIALIZER(&sb_port_groups);

@@ -10463,26 +10481,35 @@ sync_port_groups(struct northd_context *ctx)

         shash_add(&sb_port_groups, sb_port_group->name, sb_port_group);

     }

-    const struct nbrec_port_group *nb_port_group;

-    NBREC_PORT_GROUP_FOR_EACH (nb_port_group, ctx->ovnnb_idl) {

-        sb_port_group = shash_find_and_delete(&sb_port_groups,

-                                               nb_port_group->name);

-        if (!sb_port_group) {

-            sb_port_group = sbrec_port_group_insert(ctx->ovnsb_txn);

-            sbrec_port_group_set_name(sb_port_group, nb_port_group->name);

-        }

+    struct ds sb_name = DS_EMPTY_INITIALIZER;

-        const char **nb_port_names = xcalloc(nb_port_group->n_ports,

-                                             sizeof *nb_port_names);

-        int i;

-        for (i = 0; i < nb_port_group->n_ports; i++) {

-            nb_port_names[i] = nb_port_group->ports[i]->name;

+    struct ovn_port_group *pg;

+    HMAP_FOR_EACH (pg, key_node, pgs) {

+

+        struct ovn_port_group_ls *pg_ls;

+        HMAP_FOR_EACH (pg_ls, key_node, &pg->nb_lswitches) {

+            ds_clear(&sb_name);

+            get_sb_port_group_name(pg->nb_pg->name, pg_ls->od->sb->tunnel_key,

+                                   &sb_name);

+            sb_port_group = shash_find_and_delete(&sb_port_groups,

+                                                  ds_cstr(&sb_name));

+            if (!sb_port_group) {

+                sb_port_group = sbrec_port_group_insert(ctx->ovnsb_txn);

+                sbrec_port_group_set_name(sb_port_group, ds_cstr(&sb_name));

+            }

+

+            const char **nb_port_names = xcalloc(pg_ls->n_ports,

+                                                 sizeof *nb_port_names);

+            for (size_t i = 0; i < pg_ls->n_ports; i++) {

+                nb_port_names[i] = pg_ls->ports[i]->nbsp->name;

+            }

+            sbrec_port_group_set_ports(sb_port_group,

+                                       nb_port_names,

+                                       pg_ls->n_ports);

+            free(nb_port_names);

         }

-        sbrec_port_group_set_ports(sb_port_group,

-                                   nb_port_names,

-                                   nb_port_group->n_ports);

-        free(nb_port_names);

     }

+    ds_destroy(&sb_name);

     struct shash_node *node, *next;

     SHASH_FOR_EACH_SAFE (node, next, &sb_port_groups) {

@@ -11081,7 +11108,7 @@ ovnnb_db_run(struct northd_context *ctx,

     ovn_update_ipv6_prefix(ports);

     sync_address_sets(ctx);

-    sync_port_groups(ctx);

+    sync_port_groups(ctx, &port_groups);

     sync_meters(ctx);

     sync_dns_entries(ctx, datapaths);

     destroy_ovn_lbs(&lbs;;

diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at

index e6a8c04..37805d3 100644

--- a/tests/ovn-northd.at

+++ b/tests/ovn-northd.at

@@ -1406,3 +1406,82 @@ AT_CHECK([ovn-nbctl --wait=sb sync], [0])

 AT_CHECK([test 0 = $(ovn-sbctl list Ha_Chassis_Group | wc -l)])

 AT_CLEANUP

+

+AT_SETUP([ovn -- check NB/SB Port_Group translation (lsp add/del)])

+ovn_start

+

+ovn-nbctl ls-add ls1

+ovn-nbctl ls-add ls2

+ovn-nbctl lsp-add ls1 lsp1

+ovn-nbctl lsp-add ls2 lsp2

+ovn-nbct --wait=sb sync

+ls1_key=$(ovn-sbctl --columns tunnel_key --bare list Datapath_Binding ls1)

+ls2_key=$(ovn-sbctl --columns tunnel_key --bare list Datapath_Binding ls2)

+

+# Add an empty port group. This should generate no entry in the SB.

+ovn-nbctl --wait=sb pg-add pg_test

+AT_CHECK([test 0 = $(ovn-sbctl --columns _uuid list Port_Group | grep uuid -c)])

+

+# Add lsp1 to the port group. This should generate an entry in the SB only

+# for ls1.

+ovn-nbctl --wait=sb pg-set-ports pg_test lsp1

+AT_CHECK([test 1 = $(ovn-sbctl --columns _uuid list Port_Group | grep uuid -c)])

+AT_CHECK([ovn-sbctl --columns ports --bare find Port_Group name=${ls1_key}_pg_test], [0], [dnl

+lsp1

+])

+

+# Add lsp2 to the port group. This should generate a new entry in the SB, for

+# ls2.

+ovn-nbctl --wait=sb pg-set-ports pg_test lsp1 lsp2

+AT_CHECK([test 2 = $(ovn-sbctl --columns _uuid list Port_Group | grep uuid -c)])

+AT_CHECK([ovn-sbctl --columns ports --bare find Port_Group name=${ls1_key}_pg_test], [0], [dnl

+lsp1

+])

+AT_CHECK([ovn-sbctl --columns ports --bare find Port_Group name=${ls2_key}_pg_test], [0], [dnl

+lsp2

+])

+

+# Remove lsp1 from the port group. The SB Port_Group for ls1 should be

+# removed.

+ovn-nbctl --wait=sb pg-set-ports pg_test lsp2

+AT_CHECK([test 1 = $(ovn-sbctl --columns _uuid list Port_Group | grep uuid -c)])

+AT_CHECK([ovn-sbctl --columns ports --bare find Port_Group name=${ls2_key}_pg_test], [0], [dnl

+lsp2

+])

+

+# Remove lsp2 from the port group. All SB Port_Groups should be purged.

+ovn-nbctl --wait=sb clear Port_Group pg_test ports

+AT_CHECK([test 0 = $(ovn-sbctl --columns _uuid list Port_Group | grep uuid -c)])

+

+AT_CLEANUP

+

+AT_SETUP([ovn -- check NB/SB Port_Group translation (ls del)])

+ovn_start

+

+ovn-nbctl ls-add ls1

+ovn-nbctl ls-add ls2

+ovn-nbctl lsp-add ls1 lsp1

+ovn-nbctl lsp-add ls2 lsp2

+ovn-nbct --wait=sb sync

+ls1_key=$(ovn-sbctl --columns tunnel_key --bare list Datapath_Binding ls1)

+ls2_key=$(ovn-sbctl --columns tunnel_key --bare list Datapath_Binding ls2)

+

+# Add lsp1 & lsp2 to a port group. This should generate two entries in the

+# SB (one per logical switch).

+ovn-nbctl --wait=sb pg-add pg_test lsp1 lsp2

+AT_CHECK([test 2 = $(ovn-sbctl --columns _uuid list Port_Group | grep uuid -c)])

+AT_CHECK([ovn-sbctl --columns ports --bare find Port_Group name=${ls1_key}_pg_test], [0], [dnl

+lsp1

+])

+AT_CHECK([ovn-sbctl --columns ports --bare find Port_Group name=${ls2_key}_pg_test], [0], [dnl

+lsp2

+])

+

+# Delete logical switch ls1. This should remove the associated SB Port_Group.

+ovn-nbctl --wait=sb ls-del ls1

+AT_CHECK([test 1 = $(ovn-sbctl --columns _uuid list Port_Group | grep uuid -c)])

+AT_CHECK([ovn-sbctl --columns ports --bare find Port_Group name=${ls2_key}_pg_test], [0], [dnl

+lsp2

+])

+

+AT_CLEANUP

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

index 11697eb..9f74c5c 100644

--- a/tests/test-ovn.c

+++ b/tests/test-ovn.c

@@ -235,8 +235,8 @@ create_port_groups(struct shash *port_groups)

     };

     static const char *const pg2[] = { NULL };

-    expr_const_sets_add(port_groups, "pg1", pg1, 3, false);

-    expr_const_sets_add(port_groups, "pg_empty", pg2, 0, false);

+    expr_const_sets_add(port_groups, "0_pg1", pg1, 3, false);

+    expr_const_sets_add(port_groups, "0_pg_empty", pg2, 0, false);

 }

 static bool

@@ -302,7 +302,7 @@ test_parse_expr__(int steps)

         char *error;

         expr = expr_parse_string(ds_cstr(&input), &symtab, &addr_sets,

-                                 &port_groups, NULL, NULL, &error);

+                                 &port_groups, NULL, NULL, 0, &error);

         if (!error && steps > 0) {

             expr = expr_annotate(expr, &symtab, &error);

         }

@@ -428,7 +428,7 @@ test_evaluate_expr(struct ovs_cmdl_context *ctx)

         struct expr *expr;

         expr = expr_parse_string(ds_cstr(&input), &symtab, NULL, NULL,

-                                 NULL, NULL, &error);

+                                 NULL, NULL, 0, &error);

         if (!error) {

             expr = expr_annotate(expr, &symtab, &error);

         }

@@ -903,7 +903,7 @@ test_tree_shape_exhaustively(struct expr *expr, struct shash *symtab,

             char *error;

             modified = expr_parse_string(ds_cstr(&s), symtab, NULL,

-                                         NULL, NULL, NULL, &error);

+                                         NULL, NULL, NULL, 0, &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 d7251e7..2666c10 100644

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

+++ b/utilities/ovn-trace.c

@@ -889,7 +889,8 @@ read_flows(void)

         char *error;

         struct expr *match;

         match = expr_parse_string(sblf->match, &symtab, &address_sets,

-                                  &port_groups, NULL, NULL, &error);

+                                  &port_groups, NULL, NULL, dp->tunnel_key,

+                                  &error);

         if (error) {

             VLOG_WARN("%s: parsing expression failed (%s)",

                       sblf->match, error);

-- 

1.8.3.1