[ovs-dev,2/9] ovn-controller: Expose address sets to the main loop.
diff mbox

Message ID 1483582391-114359-2-git-send-email-jpettit@ovn.org
State Accepted
Headers show

Commit Message

Justin Pettit Jan. 5, 2017, 2:13 a.m. UTC
Other functions in the main loop will need access to address sets in a
future commit.

Signed-off-by: Justin Pettit <jpettit@ovn.org>
---
 ovn/controller/lflow.c          | 41 ++++++++++-------------------------------
 ovn/controller/lflow.h          |  1 +
 ovn/controller/ovn-controller.c | 22 +++++++++++++++++++++-
 3 files changed, 32 insertions(+), 32 deletions(-)

Comments

Justin Pettit Jan. 5, 2017, 9:49 p.m. UTC | #1
> On Jan 5, 2017, at 7:34 AM, Ben Pfaff <blp@ovn.org> wrote:
> 
> On Wed, Jan 04, 2017 at 06:13:04PM -0800, Justin Pettit wrote:
>> Other functions in the main loop will need access to address sets in a
>> future commit.
>> 
>> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> 
> I'd prefer to limit the actual scope of the data structure, because that
> makes it clear where the data is actually up-to-date.  Like this, for
> example.

Thanks for the review and suggestion.  I've applied your suggestion.

--Justin

Patch
diff mbox

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 724ab99..2aad682 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -43,21 +43,6 @@  lflow_init(void)
 {
     ovn_init_symtab(&symtab);
 }
-
-/* Iterate address sets in the southbound database.  Create and update the
- * corresponding symtab entries as necessary. */
-static void
-update_address_sets(struct controller_ctx *ctx,
-                    struct shash *expr_address_sets_p)
-
-{
-    const struct sbrec_address_set *as;
-    SBREC_ADDRESS_SET_FOR_EACH (as, ctx->ovnsb_idl) {
-        expr_addr_sets_add(expr_address_sets_p, as->name,
-                           (const char *const *) as->addresses,
-                           as->n_addresses);
-    }
-}
 
 struct lookup_port_aux {
     const struct lport_index *lports;
@@ -74,8 +59,8 @@  static void consider_logical_flow(const struct lport_index *lports,
                                   struct hmap *dhcp_opts_p,
                                   struct hmap *dhcpv6_opts_p,
                                   uint32_t *conj_id_ofs_p,
-                                  struct hmap *flow_table,
-                                  struct shash *expr_address_sets_p);
+                                  const struct shash *addr_sets,
+                                  struct hmap *flow_table);
 
 static bool
 lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp)
@@ -113,8 +98,8 @@  add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports,
                   const struct hmap *local_datapaths,
                   struct group_table *group_table,
                   const struct simap *ct_zones,
-                  struct hmap *flow_table,
-                  struct shash *expr_address_sets_p)
+                  const struct shash *addr_sets,
+                  struct hmap *flow_table)
 {
     uint32_t conj_id_ofs = 1;
     const struct sbrec_logical_flow *lflow;
@@ -138,7 +123,7 @@  add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports,
         consider_logical_flow(lports, mcgroups, lflow, local_datapaths,
                               group_table, ct_zones,
                               &dhcp_opts, &dhcpv6_opts, &conj_id_ofs,
-                              flow_table, expr_address_sets_p);
+                              addr_sets, flow_table);
     }
 
     dhcp_opts_destroy(&dhcp_opts);
@@ -155,8 +140,8 @@  consider_logical_flow(const struct lport_index *lports,
                       struct hmap *dhcp_opts_p,
                       struct hmap *dhcpv6_opts_p,
                       uint32_t *conj_id_ofs_p,
-                      struct hmap *flow_table,
-                      struct shash *expr_address_sets_p)
+                      const struct shash *addr_sets,
+                      struct hmap *flow_table)
 {
     /* Determine translation of logical table IDs to physical table IDs. */
     bool ingress = !strcmp(lflow->pipeline, "ingress");
@@ -232,8 +217,7 @@  consider_logical_flow(const struct lport_index *lports,
     struct hmap matches;
     struct expr *expr;
 
-    expr = expr_parse_string(lflow->match, &symtab,
-                             expr_address_sets_p, &error);
+    expr = expr_parse_string(lflow->match, &symtab, addr_sets, &error);
     if (!error) {
         if (prereqs) {
             expr = expr_combine(EXPR_T_AND, expr, prereqs);
@@ -377,17 +361,12 @@  lflow_run(struct controller_ctx *ctx, const struct lport_index *lports,
           const struct hmap *local_datapaths,
           struct group_table *group_table,
           const struct simap *ct_zones,
+          const struct shash *addr_sets,
           struct hmap *flow_table)
 {
-    struct shash expr_address_sets = SHASH_INITIALIZER(&expr_address_sets);
-
-    update_address_sets(ctx, &expr_address_sets);
     add_logical_flows(ctx, lports, mcgroups, local_datapaths,
-                      group_table, ct_zones, flow_table, &expr_address_sets);
+                      group_table, ct_zones, addr_sets, flow_table);
     add_neighbor_flows(ctx, lports, flow_table);
-
-    expr_addr_sets_destroy(&expr_address_sets);
-    shash_destroy(&expr_address_sets);
 }
 
 void
diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
index 6305ce0..0bb2b18 100644
--- a/ovn/controller/lflow.h
+++ b/ovn/controller/lflow.h
@@ -66,6 +66,7 @@  void lflow_run(struct controller_ctx *, const struct lport_index *,
                const struct hmap *local_datapaths,
                struct group_table *group_table,
                const struct simap *ct_zones,
+               const struct shash *addr_sets,
                struct hmap *flow_table);
 void lflow_destroy(void);
 
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 368ccc9..2533899 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -258,6 +258,22 @@  get_chassis_id(const struct ovsdb_idl *ovs_idl)
     return chassis_id;
 }
 
+/* Iterate address sets in the southbound database.  Create and update the
+ * corresponding symtab entries as necessary. */
+static void
+update_addr_sets(struct controller_ctx *ctx, struct shash *addr_sets)
+
+{
+    expr_addr_sets_destroy(addr_sets);
+
+    const struct sbrec_address_set *as;
+    SBREC_ADDRESS_SET_FOR_EACH (as, ctx->ovnsb_idl) {
+        expr_addr_sets_add(addr_sets, as->name,
+                           (const char *const *) as->addresses,
+                           as->n_addresses);
+    }
+}
+
 /* Retrieves the OVN Southbound remote location from the
  * "external-ids:ovn-remote" key in 'ovs_idl' and returns a copy of it. */
 static char *
@@ -529,6 +545,8 @@  main(int argc, char *argv[])
     unixctl_command_register("ct-zone-list", "", 0, 0,
                              ct_zone_list, &ct_zones);
 
+    struct shash addr_sets = SHASH_INITIALIZER(&addr_sets);
+
     /* Main loop. */
     exiting = false;
     while (!exiting) {
@@ -589,11 +607,12 @@  main(int argc, char *argv[])
             update_ct_zones(&local_lports, &local_datapaths, &ct_zones,
                             ct_zone_bitmap, &pending_ct_zones);
             if (ctx.ovs_idl_txn) {
+                update_addr_sets(&ctx, &addr_sets);
                 commit_ct_zones(br_int, &pending_ct_zones);
 
                 struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
                 lflow_run(&ctx, &lports, &mcgroups, &local_datapaths,
-                          &group_table, &ct_zones, &flow_table);
+                          &group_table, &ct_zones, &addr_sets, &flow_table);
 
                 physical_run(&ctx, mff_ovn_geneve,
                              br_int, chassis, &ct_zones, &lports,
@@ -691,6 +710,7 @@  main(int argc, char *argv[])
     pinctrl_destroy();
 
     simap_destroy(&ct_zones);
+    shash_destroy(&addr_sets);
 
     bitmap_free(group_table.group_ids);
     hmap_destroy(&group_table.desired_groups);