[ovs-dev] [PATCH v2] ovn-controller: Restore ct zone assignment.

Russell Bryant russell at ovn.org
Fri Jul 29 15:18:49 UTC 2016


From: Babu Shanmugam <bschanmu at redhat.com>

Recent commits reorganizing bindings handling and also moving ct zone
assignment to ovn-controller.c caused ct zone assignment to no longer
work.  The code relies on an "all_lports" sset that should contain all
logical ports that we should be assigning ct zones for.  Prior to this
change, all_lports was always empty.

Signed-off-by: Babu Shanmugam <bschanmu at redhat.com>
Co-authored-by: Russell Bryant <russell at ovn.org>
Signed-off-by: Russell Bryant <russell at ovn.org>
---
 ovn/controller/binding.c        | 46 +++++++++++++++++++++++++++++++++++------
 ovn/controller/binding.h        |  3 ++-
 ovn/controller/ovn-controller.c | 13 +++++++-----
 3 files changed, 50 insertions(+), 12 deletions(-)

v1->v2:
 - Ensure all_lports also includes sub-ports, gateway ports, and localnet ports,
   as well as ports for local interfaces.

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 41165bc..3073727 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -66,7 +66,8 @@ 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 sset *all_lports)
 {
     int i;
     bool changed = false;
@@ -94,6 +95,7 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
             shash_add(lport_to_iface, iface_id, iface_rec);
             if (!sset_find_and_delete(&old_local_ids, iface_id)) {
                 sset_add(&local_ids, iface_id);
+                sset_add(all_lports, iface_id);
                 changed = true;
             }
         }
@@ -107,6 +109,7 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
         const char *cur_id;
         SSET_FOR_EACH(cur_id, &old_local_ids) {
             sset_find_and_delete(&local_ids, cur_id);
+            sset_find_and_delete(all_lports, cur_id);
         }
     }
 
@@ -174,7 +177,8 @@ consider_local_datapath(struct controller_ctx *ctx,
                         const struct sbrec_chassis *chassis_rec,
                         const struct sbrec_port_binding *binding_rec,
                         struct hmap *local_datapaths,
-                        struct shash *lport_to_iface)
+                        struct shash *lport_to_iface,
+                        struct sset *all_lports)
 {
     const struct ovsrec_interface *iface_rec
         = shash_find_data(lport_to_iface, binding_rec->logical_port);
@@ -200,6 +204,9 @@ consider_local_datapath(struct controller_ctx *ctx,
                           binding_rec->logical_port);
             }
             sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
+            if (binding_rec->parent_port && binding_rec->parent_port[0]) {
+                sset_add(all_lports, binding_rec->logical_port);
+            }
         }
     } else if (!strcmp(binding_rec->type, "l2gateway")) {
         const char *chassis_id = smap_get(&binding_rec->options,
@@ -209,6 +216,7 @@ consider_local_datapath(struct controller_ctx *ctx,
                 VLOG_INFO("Releasing l2gateway port %s from this chassis.",
                           binding_rec->logical_port);
                 sbrec_port_binding_set_chassis(binding_rec, NULL);
+                sset_find_and_delete(all_lports, binding_rec->logical_port);
             }
             return;
         }
@@ -221,6 +229,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);
+            sset_add(all_lports, binding_rec->logical_port);
             add_local_datapath(local_datapaths, binding_rec);
         }
     } else if (chassis_rec && binding_rec->chassis == chassis_rec
@@ -229,13 +238,20 @@ consider_local_datapath(struct controller_ctx *ctx,
             VLOG_INFO("Releasing lport %s from this chassis.",
                       binding_rec->logical_port);
             sbrec_port_binding_set_chassis(binding_rec, NULL);
+            sset_find_and_delete(all_lports, binding_rec->logical_port);
         }
+    } else if (!binding_rec->chassis
+               && !strcmp(binding_rec->type, "localnet")) {
+        /* Add all localnet ports to all_lports so that we allocate ct zones
+         * for them. */
+        sset_add(all_lports, binding_rec->logical_port);
     }
 }
 
 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 hmap *local_datapaths,
+            struct sset *all_lports)
 {
     const struct sbrec_chassis *chassis_rec;
     const struct sbrec_port_binding *binding_rec;
@@ -247,7 +263,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,
+                                                      all_lports)) {
             process_full_binding = true;
         }
     } else {
@@ -260,11 +277,19 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
      * chassis and update the binding accordingly.  This includes both
      * directly connected logical ports and children of those ports. */
     if (process_full_binding) {
+        /* Detect any entries in all_lports that have been deleted.
+         * In particular, this will catch localnet ports that we
+         * put in all_lports. */
+        struct sset removed_lports = SSET_INITIALIZER(&removed_lports);
+        sset_clone(&removed_lports, all_lports);
+
         struct hmap keep_local_datapath_by_uuid =
             HMAP_INITIALIZER(&keep_local_datapath_by_uuid);
         SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
+            sset_find_and_delete(&removed_lports, binding_rec->logical_port);
             consider_local_datapath(ctx, chassis_rec, binding_rec,
-                                    local_datapaths, &lport_to_iface);
+                                    local_datapaths, &lport_to_iface,
+                                    all_lports);
             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,
@@ -278,6 +303,14 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
             }
         }
         hmap_destroy(&keep_local_datapath_by_uuid);
+
+        /* Any remaining entries in removed_lports are logical ports that
+         * have been deleted and should also be removed from all_ports. */
+        const char *cur_id;
+        SSET_FOR_EACH(cur_id, &removed_lports) {
+            sset_find_and_delete(all_lports, cur_id);
+        }
+
         process_full_binding = false;
     } else {
         SBREC_PORT_BINDING_FOR_EACH_TRACKED(binding_rec, ctx->ovnsb_idl) {
@@ -292,7 +325,8 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
                 }
             } else {
                 consider_local_datapath(ctx, chassis_rec, binding_rec,
-                                        local_datapaths, &lport_to_iface);
+                                        local_datapaths, &lport_to_iface,
+                                        all_lports);
             }
         }
     }
diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
index 8753d44..fbd16c8 100644
--- a/ovn/controller/binding.h
+++ b/ovn/controller/binding.h
@@ -29,7 +29,8 @@ struct sset;
 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 hmap *local_datapaths,
+                 struct sset *all_lports);
 bool binding_cleanup(struct controller_ctx *, const char *chassis_id);
 
 #endif /* ovn/binding.h */
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 5c74186..5a2daa8 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -314,6 +314,11 @@ static struct hmap patched_datapaths = HMAP_INITIALIZER(&patched_datapaths);
 static struct lport_index lports;
 static struct mcgroup_index mcgroups;
 
+/* Contains the names of all logical ports currently bound to the chassis
+ * managed by this instance of ovn-controller. The contents are managed
+ * in binding.c, but consumed elsewhere. */
+static struct sset all_lports = SSET_INITIALIZER(&all_lports);
+
 int
 main(int argc, char *argv[])
 {
@@ -425,8 +430,6 @@ main(int argc, char *argv[])
 
         update_probe_interval(&ctx);
 
-        struct sset all_lports = SSET_INITIALIZER(&all_lports);
-
         const struct ovsrec_bridge *br_int = get_br_int(&ctx);
         const char *chassis_id = get_chassis_id(ctx.ovs_idl);
 
@@ -434,7 +437,9 @@ main(int argc, char *argv[])
         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, &local_datapaths,
+                        &all_lports);
+            VLOG_INFO("all_lports size: %lu", sset_count(&all_lports));
         }
 
         if (br_int && chassis_id) {
@@ -466,8 +471,6 @@ main(int argc, char *argv[])
             }
         }
 
-        sset_destroy(&all_lports);
-
         unixctl_server_run(unixctl);
 
         unixctl_server_wait(unixctl);
-- 
2.7.4




More information about the dev mailing list