[ovs-dev] [PATCH monitor_cond V11] ovn: implementation of conditional monitoring usage

Mickey Spiegel emspiege at us.ibm.com
Sun Jul 31 17:59:29 UTC 2016


Comments inline with <Mickey>

-----"dev" <dev-bounces at openvswitch.org> wrote: -----
To: Ben Pfaff <blp at ovn.org>
From: Liran Schour 
Sent by: "dev" 
Date: 07/28/2016 04:49AM
Cc: dev at openvswitch.org
Subject: [ovs-dev] [PATCH monitor_cond V11] ovn: implementation of	conditional monitoring usage

Conditional monitor of: Port_Binding, Logical_Flow, Multicast_Group
MAC_Binding tables. As a result ovn-controller will be notified only about
records belongs to a datapath that is being served by this hypervisor.

Performance evaluation:
OVN is the main candidate for conditional monitoring usage. It is clear that
conditional monitoring reduces computation on the ovn-controller (client) side
due to the reduced size of flow tables and update messages. Performance
evaluation shows up to 75% computation reduction on the ovn-controller side.
However, performance evaluation shows also a reduction in computation on the SB
ovsdb-server side proportional to the degree that each logical network is
spread over physical hosts in the DC. Evaluation shows that in a realistic
scenarios there is a computation reduction also in the server side.

Evaluation on simulated environment of 50 hosts and 1000 logical ports shows
the following results (cycles #):

LN spread over # hosts|    master    | patch        | change
-------------------------------------------------------------
            1         | 24597200127  | 24339235374  |  1.0%
            6         | 23788521572  | 19145229352  | 19.5%
           12         | 23886405758  | 17913143176  | 25.0%
           18         | 25812686279  | 23675094540  |  8.2%
           24         | 28414671499  | 24770202308  | 12.8%
           30         | 31487218890  | 28397543436  |  9.8%
           36         | 36116993930  | 34105388739  |  5.5%
           42         | 37898342465  | 38647139083  | -1.9%
           48         | 41637996229  | 41846616306  | -0.5%
           50         | 41679995357  | 43455565977  | -4.2%

Signed-off-by: Liran Schour <lirans at il.ibm.com>
---
 ovn/controller/automake.mk      |   4 +-
 ovn/controller/binding.c        |  45 ++++++---
 ovn/controller/binding.h        |   4 +-
 ovn/controller/filter.c         | 207 ++++++++++++++++++++++++++++++++++++++++
 ovn/controller/filter.h         |  36 +++++++
 ovn/controller/ovn-controller.c |  18 +++-
 ovn/controller/ovn-controller.h |   2 +
 ovn/controller/patch.c          |  30 ++++--
 ovn/controller/patch.h          |   4 +-
 tests/ovn-controller.at         |   3 +
 tests/ovn.at                    |   1 +
 11 files changed, 327 insertions(+), 27 deletions(-)
 create mode 100644 ovn/controller/filter.c
 create mode 100644 ovn/controller/filter.h

<Mickey> Thanks to all_lports and possibly more, this no longer applies cleanly. Rebase required.

<Mickey> A few nits inline.


diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
index cf57bbd..0318654 100644
--- a/ovn/controller/automake.mk
+++ b/ovn/controller/automake.mk
@@ -19,7 +19,9 @@ ovn_controller_ovn_controller_SOURCES = \
 	ovn/controller/ovn-controller.c \
 	ovn/controller/ovn-controller.h \
 	ovn/controller/physical.c \
-	ovn/controller/physical.h
+	ovn/controller/physical.h \
+	ovn/controller/filter.c \
+	ovn/controller/filter.h
 ovn_controller_ovn_controller_LDADD = ovn/lib/libovn.la lib/libopenvswitch.la
 man_MANS += ovn/controller/ovn-controller.8
 EXTRA_DIST += ovn/controller/ovn-controller.8.xml
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index e83c1d5..be77619 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -27,6 +27,8 @@
 #include "openvswitch/vlog.h"
 #include "ovn/lib/ovn-sb-idl.h"
 #include "ovn-controller.h"
+#include "lport.h"
+#include "filter.h"
 
 VLOG_DEFINE_THIS_MODULE(binding);
 
@@ -66,7 +68,9 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 
 static bool
 get_local_iface_ids(const struct ovsrec_bridge *br_int,
-                    struct shash *lport_to_iface)
+                    struct shash *lport_to_iface,
+                    struct lport_index *lports_index,
+                    struct controller_ctx *ctx)
 {
     int i;
     bool changed = false;
@@ -92,6 +96,9 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
                 continue;
             }
             shash_add(lport_to_iface, iface_id, iface_rec);
+            if (!lport_lookup_by_name(lports_index, iface_id)) {
+                filter_lport(ctx, iface_id);
+            }
             if (!sset_find_and_delete(&old_local_ids, iface_id)) {
                 sset_add(&local_ids, iface_id);
                 changed = true;
@@ -123,8 +130,14 @@ local_datapath_lookup_by_uuid(struct hmap *hmap_p, const struct uuid *uuid)
 }
 
 static void
-remove_local_datapath(struct hmap *local_datapaths, struct local_datapath *ld)
+remove_local_datapath(struct controller_ctx *ctx, struct hmap *local_datapaths,
+                      struct hmap *patched_datapaths,
+                      struct local_datapath *ld)
 {
+    if (!get_patched_datapath(patched_datapaths, ld->tunnel_key)) {
+        unfilter_datapath(ctx, ld->tunnel_key);
+    }
+
     if (ld->logical_port) {
         free(ld->logical_port);
         ld->logical_port = NULL;
@@ -136,7 +149,8 @@ remove_local_datapath(struct hmap *local_datapaths, struct local_datapath *ld)
 
 static void
 add_local_datapath(struct hmap *local_datapaths,
-        const struct sbrec_port_binding *binding_rec)
+                   const struct sbrec_port_binding *binding_rec,
+                   struct controller_ctx *ctx)
 {
     if (get_local_datapath(local_datapaths,
                            binding_rec->datapath->tunnel_key)) {
@@ -146,11 +160,13 @@ add_local_datapath(struct hmap *local_datapaths,
     struct local_datapath *ld = xzalloc(sizeof *ld);
     ld->logical_port = xstrdup(binding_rec->logical_port);
     memcpy(&ld->uuid, &binding_rec->header_.uuid, sizeof ld->uuid);
+    ld->tunnel_key = binding_rec->datapath->tunnel_key;
     hmap_insert(local_datapaths, &ld->hmap_node,
                 binding_rec->datapath->tunnel_key);
     lport_index_reset();
     mcgroup_index_reset();
     lflow_reset_processing();
+    filter_datapath(ctx, binding_rec->datapath);
 }
 
 static void
@@ -177,7 +193,7 @@ consider_local_datapath(struct controller_ctx *ctx,
     if (iface_rec
         || (binding_rec->parent_port && binding_rec->parent_port[0] &&
             sset_contains(&local_ids, binding_rec->parent_port))) {
-        add_local_datapath(local_datapaths, binding_rec);
+        add_local_datapath(local_datapaths, binding_rec, ctx);
         if (iface_rec && ctx->ovs_idl_txn) {
             update_qos(iface_rec, binding_rec);
         }
@@ -216,7 +232,7 @@ consider_local_datapath(struct controller_ctx *ctx,
             VLOG_INFO("Claiming l2gateway port %s for this chassis.",
                       binding_rec->logical_port);
             sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
-            add_local_datapath(local_datapaths, binding_rec);
+            add_local_datapath(local_datapaths, binding_rec, ctx);
         }
     } else if (chassis_rec && binding_rec->chassis == chassis_rec
                && strcmp(binding_rec->type, "gateway")) {
@@ -230,7 +246,8 @@ consider_local_datapath(struct controller_ctx *ctx,
 
 void
 binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
-            const char *chassis_id, struct hmap *local_datapaths)
+            const char *chassis_id, struct lport_index *lports_index,
+            struct hmap *local_datapaths, struct hmap *patched_datapaths)
 {
     const struct sbrec_chassis *chassis_rec;
     const struct sbrec_port_binding *binding_rec;
@@ -242,7 +259,8 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
     }
 
     if (br_int) {
-        if (ctx->ovnsb_idl_txn && get_local_iface_ids(br_int, &lport_to_iface)) {
+        if (ctx->ovnsb_idl_txn && get_local_iface_ids(br_int, &lport_to_iface,
+                                                      lports_index, ctx)) {
             process_full_binding = true;
         }
     } else {
@@ -258,8 +276,9 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
         struct hmap keep_local_datapath_by_uuid =
             HMAP_INITIALIZER(&keep_local_datapath_by_uuid);
         SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
-            consider_local_datapath(ctx, chassis_rec, binding_rec,
-                                    local_datapaths, &lport_to_iface);
+            consider_local_datapath(ctx, chassis_rec,
+                                    binding_rec, local_datapaths,
+                                    &lport_to_iface);

<Mickey> There is no longer any change here, just reformatting that is not required.

             struct local_datapath *ld = xzalloc(sizeof *ld);
             memcpy(&ld->uuid, &binding_rec->header_.uuid, sizeof ld->uuid);
             hmap_insert(&keep_local_datapath_by_uuid, &ld->uuid_hmap_node,
@@ -269,7 +288,8 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
         HMAP_FOR_EACH_SAFE (old_ld, next, hmap_node, local_datapaths) {
             if (!local_datapath_lookup_by_uuid(&keep_local_datapath_by_uuid,
                                                &old_ld->uuid)) {
-                remove_local_datapath(local_datapaths, old_ld);
+                remove_local_datapath(ctx, local_datapaths, patched_datapaths,
+                                      old_ld);
             }
         }
         hmap_destroy(&keep_local_datapath_by_uuid);
@@ -286,8 +306,9 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
                     poll_immediate_wake();
                 }
             } else {
-                consider_local_datapath(ctx, chassis_rec, binding_rec,
-                                        local_datapaths, &lport_to_iface);
+                consider_local_datapath(ctx, chassis_rec,
+                                        binding_rec, local_datapaths,
+                                        &lport_to_iface);

<Mickey> There is no longer any change here, just reformatting that is not required.

             }
         }
     }
diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
index 8753d44..60d99db 100644
--- a/ovn/controller/binding.h
+++ b/ovn/controller/binding.h
@@ -25,11 +25,13 @@ struct ovsdb_idl;
 struct ovsrec_bridge;
 struct simap;
 struct sset;
+struct lport_index;
 
 void binding_register_ovs_idl(struct ovsdb_idl *);
 void binding_reset_processing(void);
 void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int,
-                 const char *chassis_id, struct hmap *local_datapaths);
+                 const char *chassis_id, struct lport_index *lports_index,
+                 struct hmap *local_datapaths, struct hmap *patched_datapaths);
 bool binding_cleanup(struct controller_ctx *, const char *chassis_id);
 
 #endif /* ovn/binding.h */
diff --git a/ovn/controller/filter.c b/ovn/controller/filter.c
new file mode 100644
index 0000000..d90fb79
--- /dev/null
+++ b/ovn/controller/filter.c
@@ -0,0 +1,207 @@
+/* Copyright (c) 2015, 2016 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <config.h>
+#include "filter.h"
+
+#include "openvswitch/vlog.h"
+#include "ovn/lib/ovn-sb-idl.h"
+#include "ovn-controller.h"
+#include "lport.h"
+
+VLOG_DEFINE_THIS_MODULE(filter);
+
+static struct hmap filtered_dps = HMAP_INITIALIZER(&filtered_dps);
+static struct hmap filtered_lps = HMAP_INITIALIZER(&filtered_lps);
+
+struct filtered_dp {
+    struct hmap_node hmap_node;
+    int64_t tunnel_key;
+    const struct sbrec_datapath_binding *datapath;
+};
+
+struct filtered_lp {
+    struct hmap_node hmap_node;
+    const char *lport_name;
+    bool used;
+};
+
+void
+filter_init(struct ovsdb_idl *idl)
+{
+    sbrec_port_binding_add_clause_false(idl);
+    sbrec_mac_binding_add_clause_false(idl);
+    sbrec_logical_flow_add_clause_false(idl);
+    sbrec_multicast_group_add_clause_false(idl);
+}
+
+void
+filter_mark_unused(void)
+{
+    struct filtered_lp *lp;
+
+    HMAP_FOR_EACH(lp, hmap_node, &filtered_lps) {
+        lp->used = false;
+    }
+}
+
+void
+filter_clear(struct ovsdb_idl *idl)
+{
+    struct filtered_lp *lp, *next;
+    struct filtered_lp *dp, *next_dp;
+
+    HMAP_FOR_EACH_SAFE(lp, next, hmap_node, &filtered_lps) {
+        hmap_remove(&filtered_lps, &lp->hmap_node);
+        free(lp);
+    }
+    HMAP_FOR_EACH_SAFE(dp, next_dp, hmap_node, &filtered_dps) {
+        hmap_remove(&filtered_dps, &dp->hmap_node);
+        free(dp);
+    }
+
+    ovsdb_idl_condition_reset(idl,
+                              &sbrec_table_port_binding);
+    ovsdb_idl_condition_reset(idl,
+                              &sbrec_table_logical_flow);
+    ovsdb_idl_condition_reset(idl,
+                              &sbrec_table_mac_binding);
+    ovsdb_idl_condition_reset(idl,
+                              &sbrec_table_multicast_group);
+}
+
+static struct filtered_dp *
+lookup_dp_by_key(int64_t tunnel_key)
+{
+    struct filtered_dp *dp;
+
+    HMAP_FOR_EACH_WITH_HASH(dp, hmap_node, tunnel_key,
+                            &filtered_dps) {
+        if (dp->tunnel_key == tunnel_key) {
+            return dp;
+        }
+    }
+    return NULL;
+}
+
+void
+filter_remove_unused_lports(struct controller_ctx *ctx,
+                            const struct lport_index *lports_index)
+{
+    struct filtered_lp *lp, *next;
+
+    HMAP_FOR_EACH_SAFE(lp, next, hmap_node, &filtered_lps) {
+        if (!lp->used) {
+            const struct sbrec_port_binding *pb =
+                lport_lookup_by_name(lports_index, lp->lport_name);
+            if (!pb) {
+                continue;
+            }
+            if (lookup_dp_by_key(pb->datapath->tunnel_key)) {
+                VLOG_INFO("Unfilter Port %s", lp->lport_name);

<Mickey> Wondering if the VLOGs should have a rate limit on them.

+                sbrec_port_binding_remove_clause_logical_port(ctx->ovnsb_idl,
+                                                              OVSDB_F_EQ,
+                                                              lp->lport_name);
+                hmap_remove(&filtered_lps, &lp->hmap_node);
+                free(lp);
+            }
+        }
+    }
+}
+
+void
+filter_lport(struct controller_ctx *ctx, const char *lport_name)
+{
+    struct filtered_lp *lp;
+    size_t hash = hash_bytes(lport_name, strlen(lport_name), 0);
+
+    HMAP_FOR_EACH_WITH_HASH(lp, hmap_node, hash, &filtered_lps) {
+        if (!strcmp(lp->lport_name, lport_name)) {
+            lp->used = true;
+            return;
+        }
+    }
+
+    VLOG_INFO("Filter Port %s", lport_name);

<Mickey> Another VLOG candidate for a rate limit.

+
+    sbrec_port_binding_add_clause_logical_port(ctx->ovnsb_idl,
+                                               OVSDB_F_EQ,
+                                               lport_name);
+
+    char *name = xmalloc(strlen(lport_name));
+    lp = xmalloc(sizeof *lp);
+
+    strcpy(name, lport_name);
+    lp->lport_name = name;
+    lp->used = true;
+    hmap_insert(&filtered_lps, &lp->hmap_node,
+                hash);
+}
+
+void
+filter_datapath(struct controller_ctx *ctx,
+                const struct sbrec_datapath_binding *datapath)
+{
+    struct filtered_dp *dp;
+
+    dp = lookup_dp_by_key(datapath->tunnel_key);
+    if (dp) {
+        return;
+    }
+
+    VLOG_INFO("Filter DP "UUID_FMT, UUID_ARGS(&datapath->header_.uuid));

<Mickey> Another VLOG candidate for a rate limit.

+    sbrec_port_binding_add_clause_datapath(ctx->ovnsb_idl,
+                                           OVSDB_F_EQ,
+                                           datapath);
+    sbrec_mac_binding_add_clause_datapath(ctx->ovnsb_idl,
+                                          OVSDB_F_EQ,
+                                          datapath);
+    sbrec_logical_flow_add_clause_logical_datapath(ctx->ovnsb_idl,
+                                                   OVSDB_F_EQ,
+                                                   datapath);
+    sbrec_multicast_group_add_clause_datapath(ctx->ovnsb_idl,
+                                              OVSDB_F_EQ,
+                                              datapath);
+
+    dp = xmalloc(sizeof *dp);
+    dp->tunnel_key = datapath->tunnel_key;
+    dp->datapath = datapath;
+    hmap_insert(&filtered_dps, &dp->hmap_node, datapath->tunnel_key);
+}
+
+void
+unfilter_datapath(struct controller_ctx *ctx, int64_t tunnel_key)
+{
+    struct filtered_dp *dp = lookup_dp_by_key(tunnel_key);
+
+    if (dp) {
+        VLOG_INFO("Unfilter DP "UUID_FMT,

<Mickey> Another VLOG candidate for a rate limit.

+                  UUID_ARGS(&dp->datapath->header_.uuid));
+        sbrec_port_binding_remove_clause_datapath(ctx->ovnsb_idl,
+                                                  OVSDB_F_EQ,
+                                                  dp->datapath);
+        sbrec_mac_binding_remove_clause_datapath(ctx->ovnsb_idl,
+                                                 OVSDB_F_EQ,
+                                                 dp->datapath);
+        sbrec_logical_flow_remove_clause_logical_datapath(ctx->ovnsb_idl,
+                                                          OVSDB_F_EQ,
+                                                          dp->datapath);
+        sbrec_multicast_group_remove_clause_datapath(ctx->ovnsb_idl,
+                                                     OVSDB_F_EQ,
+                                                     dp->datapath);
+        hmap_remove(&filtered_dps, &dp->hmap_node);
+        free(dp);
+    }
+}
diff --git a/ovn/controller/filter.h b/ovn/controller/filter.h
new file mode 100644
index 0000000..e09c1c8
--- /dev/null
+++ b/ovn/controller/filter.h
@@ -0,0 +1,36 @@
+/* Copyright (c) 2015 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef OVN_FILTER_H
+#define OVN_FILTER_H 1
+
+#include <stdint.h>
+
+struct controller_ctx;
+struct ovsdb_idl;
+struct sbrec_datapath_binding;
+struct lport_index;
+
+void filter_init(struct ovsdb_idl *idl);
+void filter_clear(struct ovsdb_idl *idl);
+void filter_mark_unused(void);
+void filter_remove_unused_lports(struct controller_ctx *ctx,
+                                 const struct lport_index *lports);
+void filter_lport(struct controller_ctx *ctx, const char *lport_name);
+void filter_datapath(struct controller_ctx *ctx,
+                     const struct sbrec_datapath_binding *datapath);
+void unfilter_datapath(struct controller_ctx *ctx, int64_t tunnel_key);
+
+#endif /* ovn/controller/filter.h */
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 5c74186..1782561 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -54,6 +54,7 @@
 #include "stream.h"
 #include "unixctl.h"
 #include "util.h"
+#include "filter.h"
 
 VLOG_DEFINE_THIS_MODULE(main);
 
@@ -387,6 +388,8 @@ main(int argc, char *argv[])
         ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class, true, true));
     ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, &sbrec_chassis_col_nb_cfg);
 
+    filter_init(ovnsb_idl_loop.idl);
+
     /* Track the southbound idl. */
     ovsdb_idl_track_add_all(ovnsb_idl_loop.idl);
 
@@ -412,6 +415,7 @@ main(int argc, char *argv[])
             binding_reset_processing();
             lport_index_clear(&lports);
             mcgroup_index_clear(&mcgroups);
+            filter_clear(ovnsb_idl_loop.idl);
         } else {
             free(new_ovnsb_remote);
         }
@@ -431,18 +435,21 @@ main(int argc, char *argv[])
         const char *chassis_id = get_chassis_id(ctx.ovs_idl);
 
         const struct sbrec_chassis *chassis = NULL;
+
+        lport_index_fill(&lports, ctx.ovnsb_idl);
+        mcgroup_index_fill(&mcgroups, ctx.ovnsb_idl);
+        filter_mark_unused();
+
         if (chassis_id) {
             chassis = chassis_run(&ctx, chassis_id);
             encaps_run(&ctx, br_int, chassis_id);
-            binding_run(&ctx, br_int, chassis_id, &local_datapaths);
+            binding_run(&ctx, br_int, chassis_id, &lports, &local_datapaths,
+                        &patched_datapaths);
         }
 
         if (br_int && chassis_id) {
             patch_run(&ctx, br_int, chassis_id, &local_datapaths,
-                      &patched_datapaths);
-
-            lport_index_fill(&lports, ctx.ovnsb_idl);
-            mcgroup_index_fill(&mcgroups, ctx.ovnsb_idl);
+                      &patched_datapaths, &lports);
 
             enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int);
 
@@ -466,6 +473,7 @@ main(int argc, char *argv[])
             }
         }
 
+        filter_remove_unused_lports(&ctx, &lports);
         sset_destroy(&all_lports);
 
         unixctl_server_run(unixctl);
diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h
index 470b1f5..8eb0b23 100644
--- a/ovn/controller/ovn-controller.h
+++ b/ovn/controller/ovn-controller.h
@@ -40,6 +40,7 @@ struct local_datapath {
     struct hmap_node hmap_node;
     struct hmap_node uuid_hmap_node;
     struct uuid uuid;
+    int64_t tunnel_key;
     char *logical_port;
     const struct sbrec_port_binding *localnet_port;
 };
@@ -55,6 +56,7 @@ struct patched_datapath {
     bool local; /* 'True' if the datapath is for gateway router. */
     bool stale; /* 'True' if the datapath is not referenced by any patch
                  * port. */
+    int64_t tunnel_key;
 };
 
 struct patched_datapath *get_patched_datapath(const struct hmap *,
diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
index 707d08b..72e39c8 100644
--- a/ovn/controller/patch.c
+++ b/ovn/controller/patch.c
@@ -24,6 +24,7 @@
 #include "openvswitch/hmap.h"
 #include "openvswitch/vlog.h"
 #include "ovn-controller.h"
+#include "filter.h"
 
 VLOG_DEFINE_THIS_MODULE(patch);
 
@@ -258,7 +259,8 @@ add_bridge_mappings(struct controller_ctx *ctx,
 
 static void
 add_patched_datapath(struct hmap *patched_datapaths,
-                     const struct sbrec_port_binding *binding_rec, bool local)
+                     const struct sbrec_port_binding *binding_rec, bool local,
+                     struct controller_ctx *ctx)
 {
     struct patched_datapath *pd = get_patched_datapath(patched_datapaths,
                                        binding_rec->datapath->tunnel_key);
@@ -273,9 +275,11 @@ add_patched_datapath(struct hmap *patched_datapaths,
     pd->local = local;
     pd->key = xasprintf(UUID_FMT,
                         UUID_ARGS(&binding_rec->datapath->header_.uuid));
+    pd->tunnel_key = binding_rec->datapath->tunnel_key;
     /* stale is set to false. */
     hmap_insert(patched_datapaths, &pd->hmap_node,
                 binding_rec->datapath->tunnel_key);
+    filter_datapath(ctx, binding_rec->datapath);
 }
 
 static void
@@ -292,7 +296,9 @@ add_logical_patch_ports_preprocess(struct hmap *patched_datapaths)
 /* This function should cleanup stale patched datapaths and any memory
  * allocated for fields within a stale patched datapath. */
 static void
-add_logical_patch_ports_postprocess(struct hmap *patched_datapaths)
+add_logical_patch_ports_postprocess(struct hmap *patched_datapaths,
+                                    struct hmap *local_datapaths,
+                                    struct controller_ctx *ctx)
 {
     /* Clean up stale patched datapaths. */
     struct patched_datapath *pd_cur_node, *pd_next_node;
@@ -300,6 +306,9 @@ add_logical_patch_ports_postprocess(struct hmap *patched_datapaths)
                         patched_datapaths) {
         if (pd_cur_node->stale == true) {
             hmap_remove(patched_datapaths, &pd_cur_node->hmap_node);
+            if (!get_local_datapath(local_datapaths, pd_cur_node->tunnel_key)) {
+                unfilter_datapath(ctx, pd_cur_node->tunnel_key);
+            }
             free(pd_cur_node->key);
             free(pd_cur_node);
         }
@@ -333,7 +342,9 @@ add_logical_patch_ports(struct controller_ctx *ctx,
                         const struct ovsrec_bridge *br_int,
                         const char *local_chassis_id,
                         struct shash *existing_ports,
-                        struct hmap *patched_datapaths)
+                        struct hmap *patched_datapaths,
+                        struct hmap *local_datapaths,
+                        struct lport_index *lports_index)
 {
     const struct sbrec_chassis *chassis_rec;
     chassis_rec = get_chassis(ctx->ovnsb_idl, local_chassis_id);
@@ -368,21 +379,26 @@ add_logical_patch_ports(struct controller_ctx *ctx,
                               existing_ports);
             free(dst_name);
             free(src_name);
-            add_patched_datapath(patched_datapaths, binding, local_port);
+            add_patched_datapath(patched_datapaths, binding, local_port, ctx);
             if (local_port) {
                 if (binding->chassis != chassis_rec && ctx->ovnsb_idl_txn) {
                     sbrec_port_binding_set_chassis(binding, chassis_rec);
                 }
             }
+            if (peer) {

<Mickey> There is "if (!peer) { continue; }" a few lines up, so this "if (peer)" is redundant.

+                if (!lport_lookup_by_name(lports_index, peer)) {
+                    filter_lport(ctx, peer);
+                }
+            }
         }
     }
-    add_logical_patch_ports_postprocess(patched_datapaths);
+    add_logical_patch_ports_postprocess(patched_datapaths, local_datapaths, ctx);
 }
 
 void
 patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
           const char *chassis_id, struct hmap *local_datapaths,
-          struct hmap *patched_datapaths)
+          struct hmap *patched_datapaths, struct lport_index *lports_index)
 {
     if (!ctx->ovs_idl_txn) {
         return;
@@ -404,7 +420,7 @@ patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
      * should be there. */
     add_bridge_mappings(ctx, br_int, &existing_ports, local_datapaths, chassis_id);
     add_logical_patch_ports(ctx, br_int, chassis_id, &existing_ports,
-                            patched_datapaths);
+                            patched_datapaths, local_datapaths, lports_index);
 
     /* Now 'existing_ports' only still contains patch ports that exist in the
      * database but shouldn't.  Delete them from the database. */
diff --git a/ovn/controller/patch.h b/ovn/controller/patch.h
index 7920a48..6177559 100644
--- a/ovn/controller/patch.h
+++ b/ovn/controller/patch.h
@@ -25,9 +25,11 @@
 struct controller_ctx;
 struct hmap;
 struct ovsrec_bridge;
+struct lport_index;
 
 void patch_run(struct controller_ctx *, const struct ovsrec_bridge *br_int,
                const char *chassis_id, struct hmap *local_datapaths,
-               struct hmap *patched_datapaths);
+               struct hmap *patched_datapaths,
+               struct lport_index *lports_index);
 
 #endif /* ovn/patch.h */
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index a2349a4..f2962eb 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -92,11 +92,14 @@ AT_CHECK([ovn-sbctl \
     -- --id=@dp2 create Datapath_Binding tunnel_key=2 \
     -- create Port_Binding datapath=@dp1 logical_port=foo tunnel_key=1 type=patch options:peer=bar \
     -- create Port_Binding datapath=@dp2 logical_port=bar tunnel_key=2 type=patch options:peer=foo \
+    -- create Port_Binding datapath=@dp1 logical_port=localvif3 tunnel_key=3 \
 | ${PERL} $srcdir/uuidfilt.pl], [0], [<0>
 <1>
 <2>
 <3>
+<4>
 ])
+ovs-vsctl add-port br-int localvif3 -- set Interface localvif3 external_ids:iface-id=localvif3
 check_patches \
     'br-int  patch-br-int-to-localnet2 patch-localnet2-to-br-int' \
     'br-eth0 patch-localnet2-to-br-int patch-br-int-to-localnet2' \
diff --git a/tests/ovn.at b/tests/ovn.at
index 86efcf5..0754e7c 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1356,6 +1356,7 @@ sim_add hv_gw
 as hv_gw
 ovs-vsctl add-br br-phys
 ovn_attach n1 br-phys 192.168.0.3
+ovs-vsctl add-port br-int viflp-gw -- set Interface viflp-gw external-ids:iface-id=lp-gw
 ovs-vsctl add-br br-phys2
 net_attach n2 br-phys2
 ovs-vsctl set open . external_ids:ovn-bridge-mappings="physnet1:br-phys2"
-- 
2.1.4

_______________________________________________
dev mailing list
dev at openvswitch.org
http://openvswitch.org/mailman/listinfo/dev





More information about the dev mailing list