[ovs-dev] [PATCH v2 2/3] ovn-controller: Move local_datapaths calculation.

Russell Bryant russell at ovn.org
Wed Jan 20 16:31:07 UTC 2016


Before this patch, physical.c build up the set of local datapaths for
its own use.  I'd like to use it in another module in a later patch, so
pull it out of physical.  It's now populated by the bindings module,
since that seems like a more appropriate place to do it, and it's also
done much earlier in the main loop, making it easier to re-use.

Signed-off-by: Russell Bryant <russell at ovn.org>
Acked-by: Han Zhou <zhouhan at gmail.com>
---

v1->v2:
 - added ack from Han Zhou

 ovn/controller/binding.c        | 25 ++++++++++++++++++++++++-
 ovn/controller/binding.h        |  3 ++-
 ovn/controller/ovn-controller.c | 18 ++++++++++++++++--
 ovn/controller/physical.c       | 25 +++----------------------
 ovn/controller/physical.h       |  3 ++-
 5 files changed, 47 insertions(+), 27 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 1854ff4..82cd10b 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -17,6 +17,7 @@
 #include "binding.h"
 
 #include "lib/bitmap.h"
+#include "lib/hmap.h"
 #include "lib/sset.h"
 #include "lib/util.h"
 #include "lib/vswitch-idl.h"
@@ -117,10 +118,24 @@ update_ct_zones(struct sset *lports, struct simap *ct_zones,
     }
 }
 
+static void
+add_local_datapath(struct hmap *local_datapaths,
+        const struct sbrec_port_binding *binding_rec)
+{
+    struct hmap_node *ld;
+    ld = hmap_first_with_hash(local_datapaths,
+                              binding_rec->datapath->tunnel_key);
+    if (!ld) {
+        ld = xmalloc(sizeof *ld);
+        hmap_insert(local_datapaths, ld,
+                    binding_rec->datapath->tunnel_key);
+    }
+}
+
 void
 binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
             const char *chassis_id, struct simap *ct_zones,
-            unsigned long *ct_zone_bitmap)
+            unsigned long *ct_zone_bitmap, struct hmap *local_datapaths)
 {
     const struct sbrec_chassis *chassis_rec;
     const struct sbrec_port_binding *binding_rec;
@@ -150,6 +165,13 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
         ctx->ovnsb_idl_txn,"ovn-controller: updating port bindings for '%s'",
         chassis_id);
 
+    /* Clear local_datapaths, as we're going to rebuild it next. */
+    struct hmap_node *node;
+    while ((node = hmap_first(local_datapaths))) {
+        hmap_remove(local_datapaths, node);
+        free(node);
+    }
+
     /* Run through each binding record to see if it is resident on this
      * chassis and update the binding accordingly.  This includes both
      * directly connected logical ports and children of those ports. */
@@ -161,6 +183,7 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
                 /* Add child logical port to the set of all local ports. */
                 sset_add(&all_lports, binding_rec->logical_port);
             }
+            add_local_datapath(local_datapaths, binding_rec);
             if (binding_rec->chassis == chassis_rec) {
                 continue;
             }
diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
index 3d4a91e..6e19c10 100644
--- a/ovn/controller/binding.h
+++ b/ovn/controller/binding.h
@@ -20,6 +20,7 @@
 #include <stdbool.h>
 
 struct controller_ctx;
+struct hmap;
 struct ovsdb_idl;
 struct ovsrec_bridge;
 struct simap;
@@ -27,7 +28,7 @@ struct simap;
 void binding_register_ovs_idl(struct ovsdb_idl *);
 void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int,
                  const char *chassis_id, struct simap *ct_zones,
-                 unsigned long *ct_zone_bitmap);
+                 unsigned long *ct_zone_bitmap, struct hmap *local_datapaths);
 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 02ecb3e..aefe1a5 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -33,6 +33,7 @@
 #include "ovn/lib/ovn-sb-idl.h"
 #include "poll-loop.h"
 #include "fatal-signal.h"
+#include "lib/hmap.h"
 #include "lib/vswitch-idl.h"
 #include "smap.h"
 #include "stream.h"
@@ -267,6 +268,10 @@ main(int argc, char *argv[])
     unixctl_command_register("ct-zone-list", "", 0, 0,
                              ct_zone_list, &ct_zones);
 
+    /* Contains bare "struct hmap_node"s whose hash values are the tunnel_key
+     * of datapaths with at least one local port binding. */
+    struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths);
+
     /* Main loop. */
     exiting = false;
     while (!exiting) {
@@ -288,7 +293,8 @@ main(int argc, char *argv[])
         if (chassis_id) {
             chassis_run(&ctx, chassis_id);
             encaps_run(&ctx, br_int, chassis_id);
-            binding_run(&ctx, br_int, chassis_id, &ct_zones, ct_zone_bitmap);
+            binding_run(&ctx, br_int, chassis_id, &ct_zones, ct_zone_bitmap,
+                    &local_datapaths);
         }
 
         if (br_int) {
@@ -300,7 +306,8 @@ main(int argc, char *argv[])
             lflow_run(&ctx, &flow_table, &ct_zones);
             if (chassis_id) {
                 physical_run(&ctx, mff_ovn_geneve,
-                             br_int, chassis_id, &ct_zones, &flow_table);
+                             br_int, chassis_id, &ct_zones, &flow_table,
+                             &local_datapaths);
             }
             ofctrl_put(&flow_table);
             hmap_destroy(&flow_table);
@@ -360,6 +367,13 @@ main(int argc, char *argv[])
 
     simap_destroy(&ct_zones);
 
+    struct hmap_node *node;
+    while ((node = hmap_first(&local_datapaths))) {
+        hmap_remove(&local_datapaths, node);
+        free(node);
+    }
+    hmap_destroy(&local_datapaths);
+
     ovsdb_idl_loop_destroy(&ovs_idl_loop);
     ovsdb_idl_loop_destroy(&ovnsb_idl_loop);
 
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 356c493..b2772f0 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -138,7 +138,8 @@ put_stack(enum mf_field_id field, struct ofpact_stack *stack)
 void
 physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
              const struct ovsrec_bridge *br_int, const char *this_chassis_id,
-             const struct simap *ct_zones, struct hmap *flow_table)
+             const struct simap *ct_zones, struct hmap *flow_table,
+             struct hmap *local_datapaths)
 {
     struct simap localvif_to_ofport = SIMAP_INITIALIZER(&localvif_to_ofport);
     struct hmap tunnels = HMAP_INITIALIZER(&tunnels);
@@ -236,10 +237,6 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
     /* Maps from network name to "struct localnet_bindings". */
     struct shash localnet_inputs = SHASH_INITIALIZER(&localnet_inputs);
 
-    /* Contains bare "struct hmap_node"s whose hash values are the tunnel_key
-     * of datapaths with at least one local port binding. */
-    struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths);
-
     /* Set up flows in table 0 for physical-to-logical translation and in table
      * 64 for logical-to-physical translation. */
     const struct sbrec_port_binding *binding;
@@ -359,15 +356,6 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
                 b->binding = binding;
                 list_insert(&ln_vlan->bindings, &b->list_elem);
             } else {
-                struct hmap_node *ld;
-                ld = hmap_first_with_hash(&local_datapaths,
-                                          binding->datapath->tunnel_key);
-                if (!ld) {
-                    ld = xmalloc(sizeof *ld);
-                    hmap_insert(&local_datapaths, ld,
-                                binding->datapath->tunnel_key);
-                }
-
                 ofpbuf_clear(&ofpacts);
                 match_init_catchall(&match);
                 match_set_in_port(&match, ofport);
@@ -762,7 +750,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
             struct binding_elem *b;
             LIST_FOR_EACH_POP (b, list_elem, &ln_vlan->bindings) {
                 struct hmap_node *ld;
-                ld = hmap_first_with_hash(&local_datapaths,
+                ld = hmap_first_with_hash(local_datapaths,
                                           b->binding->datapath->tunnel_key);
                 if (ld) {
                     /* Set MFF_LOG_DATAPATH and MFF_LOG_INPORT. */
@@ -800,12 +788,5 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
     }
     shash_destroy(&localnet_inputs);
 
-    struct hmap_node *node;
-    while ((node = hmap_first(&local_datapaths))) {
-        hmap_remove(&local_datapaths, node);
-        free(node);
-    }
-    hmap_destroy(&local_datapaths);
-
     simap_destroy(&localnet_to_ofport);
 }
diff --git a/ovn/controller/physical.h b/ovn/controller/physical.h
index 2906937..826b99b 100644
--- a/ovn/controller/physical.h
+++ b/ovn/controller/physical.h
@@ -43,6 +43,7 @@ struct simap;
 void physical_register_ovs_idl(struct ovsdb_idl *);
 void physical_run(struct controller_ctx *, enum mf_field_id mff_ovn_geneve,
                   const struct ovsrec_bridge *br_int, const char *chassis_id,
-                  const struct simap *ct_zones, struct hmap *flow_table);
+                  const struct simap *ct_zones, struct hmap *flow_table,
+                  struct hmap *local_datapaths);
 
 #endif /* ovn/physical.h */
-- 
2.5.0




More information about the dev mailing list