ovn-controller: Pass 'br_int' explicitly to functions that need it.

I found it hard otherwise to see what code depended on this.

Signed-off-by: Ben Pfaff <[email protected]>
Acked-by: Russell Bryant <[email protected]>

ovn/controller/binding.c

@@ -45,16 +45,16 @@ binding_init(struct controller_ctx *ctx)
 }
 
 static void
-get_local_iface_ids(struct controller_ctx *ctx, struct sset *lports)
+get_local_iface_ids(const struct ovsrec_bridge *br_int, struct sset *lports)
 {
     int i;
 
-    for (i = 0; i < ctx->br_int->n_ports; i++) {
-        const struct ovsrec_port *port_rec = ctx->br_int->ports[i];
+    for (i = 0; i < br_int->n_ports; i++) {
+        const struct ovsrec_port *port_rec = br_int->ports[i];
         const char *iface_id;
         int j;
 
-        if (!strcmp(port_rec->name, ctx->br_int_name)) {
+        if (!strcmp(port_rec->name, br_int->name)) {
             continue;
         }
 
@@ -72,7 +72,7 @@ get_local_iface_ids(struct controller_ctx *ctx, struct sset *lports)
 }
 
 void
-binding_run(struct controller_ctx *ctx)
+binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
 {
     const struct sbrec_chassis *chassis_rec;
     const struct sbrec_binding *binding_rec;
@@ -90,7 +90,7 @@ binding_run(struct controller_ctx *ctx)
 
     sset_init(&lports);
     sset_init(&all_lports);
-    get_local_iface_ids(ctx, &lports);
+    get_local_iface_ids(br_int, &lports);
     sset_clone(&all_lports, &lports);
 
     ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,

ovn/controller/binding.h

@@ -20,9 +20,10 @@
 #include <stdbool.h>
 
 struct controller_ctx;
+struct ovsrec_bridge;
 
 void binding_init(struct controller_ctx *);
-void binding_run(struct controller_ctx *);
+void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int);
 bool binding_cleanup(struct controller_ctx *);
 
 #endif /* ovn/binding.h */

ovn/controller/chassis.c

@@ -296,15 +296,15 @@ preferred_encap(const struct sbrec_chassis *chassis_rec)
 }
 
 static void
-update_encaps(struct controller_ctx *ctx)
+update_encaps(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
 {
     const struct sbrec_chassis *chassis_rec;
     const struct ovsrec_bridge *br;
 
     struct tunnel_ctx tc = {
         .tunnel_hmap = HMAP_INITIALIZER(&tc.tunnel_hmap),
         .port_names = SSET_INITIALIZER(&tc.port_names),
-        .br_int = ctx->br_int
+        .br_int = br_int
     };
 
     tc.ovs_txn = ctx->ovs_idl_txn;
@@ -357,21 +357,21 @@ update_encaps(struct controller_ctx *ctx)
 }
 
 void
-chassis_run(struct controller_ctx *ctx)
+chassis_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
 {
     if (ctx->ovnsb_idl_txn) {
         register_chassis(ctx);
     }
 
     if (ctx->ovs_idl_txn) {
-        update_encaps(ctx);
+        update_encaps(ctx, br_int);
     }
 }
 
 /* Returns true if the database is all cleaned up, false if more work is
  * required. */
 bool
-chassis_cleanup(struct controller_ctx *ctx)
+chassis_cleanup(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
 {
     if (!ctx->ovnsb_idl_txn || !ctx->ovs_idl_txn) {
         return false;
@@ -394,17 +394,17 @@ chassis_cleanup(struct controller_ctx *ctx)
     ovsdb_idl_txn_add_comment(ctx->ovs_idl_txn,
                               "ovn-controller: destroying tunnels");
     struct ovsrec_port **ports
-        = xmalloc(sizeof *ctx->br_int->ports * ctx->br_int->n_ports);
+        = xmalloc(sizeof *br_int->ports * br_int->n_ports);
     size_t n = 0;
-    for (size_t i = 0; i < ctx->br_int->n_ports; i++) {
-        if (!smap_get(&ctx->br_int->ports[i]->external_ids,
+    for (size_t i = 0; i < br_int->n_ports; i++) {
+        if (!smap_get(&br_int->ports[i]->external_ids,
                       "ovn-chassis-id")) {
-            ports[n++] = ctx->br_int->ports[i];
+            ports[n++] = br_int->ports[i];
             any_changes = true;
         }
     }
-    ovsrec_bridge_verify_ports(ctx->br_int);
-    ovsrec_bridge_set_ports(ctx->br_int, ports, n);
+    ovsrec_bridge_verify_ports(br_int);
+    ovsrec_bridge_set_ports(br_int, ports, n);
     free(ports);
 
     return !any_changes;

ovn/controller/chassis.h

@@ -19,9 +19,12 @@
 #include <stdbool.h>
 
 struct controller_ctx;
+struct ovsrec_bridge;
 
 void chassis_init(struct controller_ctx *);
-void chassis_run(struct controller_ctx *);
-bool chassis_cleanup(struct controller_ctx *);
+void chassis_run(struct controller_ctx *,
+                 const struct ovsrec_bridge *br_int);
+bool chassis_cleanup(struct controller_ctx *,
+                     const struct ovsrec_bridge *br_int);
 
 #endif /* ovn/chassis.h */

ovn/controller/ofctrl.c

@@ -27,6 +27,7 @@
 #include "openflow/openflow.h"
 #include "openvswitch/vlog.h"
 #include "ovn-controller.h"
+#include "vswitch-idl.h"
 #include "rconn.h"
 #include "socket-util.h"
 
@@ -82,17 +83,17 @@ ofctrl_init(void)
     hmap_init(&installed_flows);
 }
 
-/* Attempts to update the OpenFlow flows in bridge 'br_int_name' to those in
+/* Attempts to update the OpenFlow flows in bridge 'br_int' to those in
  * 'flow_table'.  Removes all of the flows from 'flow_table' and frees them.
  *
  * The flow table will only be updated if we've got an OpenFlow connection to
- * 'br_int_name' and it's not backlogged.  Otherwise, it'll have to wait until
- * the next iteration. */
+ * 'br_int' and it's not backlogged.  Otherwise, it'll have to wait until the
+ * next iteration. */
 void
-ofctrl_run(struct controller_ctx *ctx, struct hmap *flow_table)
+ofctrl_run(const struct ovsrec_bridge *br_int, struct hmap *flow_table)
 {
     char *target;
-    target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), ctx->br_int_name);
+    target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_int->name);
     if (strcmp(target, rconn_get_target(swconn))) {
         VLOG_INFO("%s: connecting to switch", target);
         rconn_connect(swconn, target, target);

ovn/controller/ofctrl.h

@@ -23,10 +23,11 @@ struct controller_ctx;
 struct hmap;
 struct match;
 struct ofpbuf;
+struct ovsrec_bridge;
 
 /* Interface for OVN main loop. */
 void ofctrl_init(void);
-void ofctrl_run(struct controller_ctx *, struct hmap *flow_table);
+void ofctrl_run(const struct ovsrec_bridge *br_int, struct hmap *flow_table);
 void ofctrl_wait(void);
 void ofctrl_destroy(void);
 

ovn/controller/ovn-controller.c

@@ -93,7 +93,7 @@ get_bridge(struct controller_ctx *ctx, const char *name)
  * xxx ovn-controller does not support changing any of these mid-run,
  * xxx but that should be addressed later. */
 static void
-get_core_config(struct controller_ctx *ctx)
+get_core_config(struct controller_ctx *ctx, char **br_int_namep)
 {
     while (1) {
         ovsdb_idl_run(ctx->ovs_idl);
@@ -113,12 +113,11 @@ get_core_config(struct controller_ctx *ctx)
         if (!br_int_name) {
             br_int_name = DEFAULT_BRIDGE_NAME;
         }
-        ctx->br_int_name = xstrdup(br_int_name);
 
-        br_int = get_bridge(ctx, ctx->br_int_name);
+        br_int = get_bridge(ctx, br_int_name);
         if (!br_int) {
             VLOG_INFO("Integration bridge '%s' does not exist.  Waiting...",
-                      ctx->br_int_name);
+                      br_int_name);
             goto try_again;
         }
 
@@ -136,6 +135,7 @@ get_core_config(struct controller_ctx *ctx)
 
         ovnsb_remote = xstrdup(remote);
         ctx->chassis_id = xstrdup(system_id);
+        *br_int_namep = xstrdup(br_int_name);
         return;
 
 try_again:
@@ -261,7 +261,8 @@ main(int argc, char *argv[])
 
     get_initial_snapshot(ctx.ovs_idl);
 
-    get_core_config(&ctx);
+    char *br_int_name;
+    get_core_config(&ctx, &br_int_name);
 
     ctx.ovnsb_idl = ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class,
                                      true, true);
@@ -278,21 +279,20 @@ main(int argc, char *argv[])
 
         /* xxx If run into any surprising changes, we exit.  We should
          * xxx handle this more gracefully. */
-        ctx.br_int = get_bridge(&ctx, ctx.br_int_name);
-        if (!ctx.br_int) {
-            VLOG_ERR("Integration bridge '%s' disappeared",
-                     ctx.br_int_name);
+        const struct ovsrec_bridge *br_int = get_bridge(&ctx, br_int_name);
+        if (!br_int) {
+            VLOG_ERR("Integration bridge '%s' disappeared", br_int_name);
             retval = EXIT_FAILURE;
             goto exit;
         }
 
-        chassis_run(&ctx);
-        binding_run(&ctx);
+        chassis_run(&ctx, br_int);
+        binding_run(&ctx, br_int);
 
         struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
         pipeline_run(&ctx, &flow_table);
-        physical_run(&ctx, &flow_table);
-        ofctrl_run(&ctx, &flow_table);
+        physical_run(&ctx, br_int, &flow_table);
+        ofctrl_run(br_int, &flow_table);
         hmap_destroy(&flow_table);
 
         unixctl_server_run(unixctl);
@@ -317,18 +317,17 @@ main(int argc, char *argv[])
 
         /* xxx If run into any surprising changes, we exit.  We should
          * xxx handle this more gracefully. */
-        ctx.br_int = get_bridge(&ctx, ctx.br_int_name);
-        if (!ctx.br_int) {
-            VLOG_ERR("Integration bridge '%s' disappeared",
-                     ctx.br_int_name);
+        const struct ovsrec_bridge *br_int = get_bridge(&ctx, br_int_name);
+        if (!br_int) {
+            VLOG_ERR("Integration bridge '%s' disappeared", br_int_name);
             retval = EXIT_FAILURE;
             goto exit;
         }
 
         /* Run both the binding and chassis cleanup, even if one of them
          * returns false.  We're done if both return true. */
         done = binding_cleanup(&ctx);
-        done = chassis_cleanup(&ctx) && done;
+        done = chassis_cleanup(&ctx, br_int) && done;
         if (done) {
             poll_immediate_wake();
         }
@@ -346,7 +345,7 @@ main(int argc, char *argv[])
     idl_loop_destroy(&ovs_idl_loop);
     idl_loop_destroy(&ovnsb_idl_loop);
 
-    free(ctx.br_int_name);
+    free(br_int_name);
     free(ctx.chassis_id);
     free(ovnsb_remote);
     free(ovs_remote);

ovn/controller/ovn-controller.h

@@ -21,19 +21,16 @@
 
 struct controller_ctx {
     char *chassis_id;               /* ID for this chassis. */
-    char *br_int_name;              /* Name of local integration bridge. */
 
     struct ovsdb_idl *ovnsb_idl;
     struct ovsdb_idl_txn *ovnsb_idl_txn;
 
     struct ovsdb_idl *ovs_idl;
     struct ovsdb_idl_txn *ovs_idl_txn;
-
-    const struct ovsrec_bridge *br_int;
 };
 
 static inline const struct sbrec_chassis *
-get_chassis_by_name(struct ovsdb_idl *ovnsb_idl, char *chassis_id)
+get_chassis_by_name(struct ovsdb_idl *ovnsb_idl, const char *chassis_id)
 {
     const struct sbrec_chassis *chassis_rec;
 

ovn/controller/physical.c

@@ -43,13 +43,14 @@ physical_init(struct controller_ctx *ctx)
 }
 
 void
-physical_run(struct controller_ctx *ctx, struct hmap *flow_table)
+physical_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
+             struct hmap *flow_table)
 {
     struct simap lport_to_ofport = SIMAP_INITIALIZER(&lport_to_ofport);
     struct simap chassis_to_ofport = SIMAP_INITIALIZER(&chassis_to_ofport);
-    for (int i = 0; i < ctx->br_int->n_ports; i++) {
-        const struct ovsrec_port *port_rec = ctx->br_int->ports[i];
-        if (!strcmp(port_rec->name, ctx->br_int_name)) {
+    for (int i = 0; i < br_int->n_ports; i++) {
+        const struct ovsrec_port *port_rec = br_int->ports[i];
+        if (!strcmp(port_rec->name, br_int->name)) {
             continue;
         }
 

ovn/controller/physical.h

@@ -26,8 +26,10 @@
 
 struct controller_ctx;
 struct hmap;
+struct ovsrec_bridge;
 
 void physical_init(struct controller_ctx *);
-void physical_run(struct controller_ctx *, struct hmap *flow_table);
+void physical_run(struct controller_ctx *, const struct ovsrec_bridge *br_int,
+                  struct hmap *flow_table);
 
 #endif /* ovn/physical.h */