| Message ID | 20161205071746.16989-6-blp@ovn.org |
|---|---|
| State | Changes Requested |
| Headers | show |
On Sun, Dec 4, 2016 at 11:17 PM, Ben Pfaff <blp@ovn.org> wrote: > This will have its first real user in an upcoming commit. > I am not sure this is really necessary. See my comments on patch 6. At the end there is deleted code for destroying patched_datapaths. That deletion should not be in this patch, it should be in patch 6. Acked-by: Mickey Spiegel <mickeys.dev@gmail.com> > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- > ovn/controller/lport.c | 62 ++++++++++++++++++++++++++++++ > +++++++++++ > ovn/controller/lport.h | 33 ++++++++++++++++++++-- > ovn/controller/ovn-controller.c | 12 +++----- > 3 files changed, 97 insertions(+), 10 deletions(-) > > diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c > index 38aca22..906fda2 100644 > --- a/ovn/controller/lport.c > +++ b/ovn/controller/lport.c > @@ -22,6 +22,68 @@ > > VLOG_DEFINE_THIS_MODULE(lport); > > +static struct ldatapath *ldatapath_lookup_by_key__( > + const struct ldatapath_index *, uint32_t dp_key); > + > +void > +ldatapath_index_init(struct ldatapath_index *ldatapaths, > + struct ovsdb_idl *ovnsb_idl) > +{ > + hmap_init(&ldatapaths->by_key); > + > + const struct sbrec_port_binding *pb; > + SBREC_PORT_BINDING_FOR_EACH (pb, ovnsb_idl) { > + if (!pb->datapath) { > + continue; > + } > + uint32_t dp_key = pb->datapath->tunnel_key; > + struct ldatapath *ld = ldatapath_lookup_by_key__(ldatapaths, > dp_key); > + if (!ld) { > + ld = xzalloc(sizeof *ld); > + hmap_insert(&ldatapaths->by_key, &ld->by_key_node, dp_key); > + ld->db = pb->datapath; > + } > + > + if (ld->n_lports >= ld->allocated_lports) { > + ld->lports = x2nrealloc(ld->lports, &ld->allocated_lports, > + sizeof *ld->lports); > + } > + ld->lports[ld->n_lports++] = pb; > + } > +} > + > +void > +ldatapath_index_destroy(struct ldatapath_index *ldatapaths) > +{ > + if (!ldatapaths) { > + return; > + } > + > + struct ldatapath *ld, *ld_next; > + HMAP_FOR_EACH_SAFE (ld, ld_next, by_key_node, &ldatapaths->by_key) { > + hmap_remove(&ldatapaths->by_key, &ld->by_key_node); > + free(ld->lports); > + free(ld); > + } > + hmap_destroy(&ldatapaths->by_key); > +} > + > +static struct ldatapath *ldatapath_lookup_by_key__( > + const struct ldatapath_index *ldatapaths, uint32_t dp_key) > +{ > + struct ldatapath *ld; > + HMAP_FOR_EACH_WITH_HASH (ld, by_key_node, dp_key, > &ldatapaths->by_key) { > + return ld; > + } > + return NULL; > +} > + > +const struct ldatapath *ldatapath_lookup_by_key( > + const struct ldatapath_index *ldatapaths, uint32_t dp_key) > +{ > + return ldatapath_lookup_by_key__(ldatapaths, dp_key); > +} > + > /* A logical port. */ > struct lport { > struct hmap_node name_node; /* Index by name. */ > diff --git a/ovn/controller/lport.h b/ovn/controller/lport.h > index 0cad74a..fe0e430 100644 > --- a/ovn/controller/lport.h > +++ b/ovn/controller/lport.h > @@ -22,8 +22,37 @@ > struct ovsdb_idl; > struct sbrec_datapath_binding; > > -/* Logical port and multicast group indexes > - * ======================================== > +/* Database indexes. > + * ================= > + * > + * If the database IDL were a little smarter, it would allow us to > directly > + * look up data based on values of its fields. It's not that smart > (yet), so > + * instead we define our own indexes. > + */ > + > +/* Logical datapath index > + * ====================== > + */ > + > +struct ldatapath { > + struct hmap_node by_key_node; /* Index by tunnel key. */ > + const struct sbrec_datapath_binding *db; > + const struct sbrec_port_binding **lports; > + size_t n_lports, allocated_lports; > +}; > + > +struct ldatapath_index { > + struct hmap by_key; > +}; > + > +void ldatapath_index_init(struct ldatapath_index *, struct ovsdb_idl *); > +void ldatapath_index_destroy(struct ldatapath_index *); > + > +const struct ldatapath *ldatapath_lookup_by_key( > + const struct ldatapath_index *, uint32_t dp_key); > + > +/* Logical port index > + * ================== > * > * This data structure holds multiple indexes over logical ports, to > allow for > * efficient searching for logical ports by name or number. > diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn- > controller.c > index 894af6f..5b95a55 100644 > --- a/ovn/controller/ovn-controller.c > +++ b/ovn/controller/ovn-controller.c > @@ -512,8 +512,10 @@ main(int argc, char *argv[]) > const struct ovsrec_bridge *br_int = get_br_int(&ctx); > const char *chassis_id = get_chassis_id(ctx.ovs_idl); > > + struct ldatapath_index ldatapaths; > struct lport_index lports; > struct mcgroup_index mcgroups; > + ldatapath_index_init(&ldatapaths, ctx.ovnsb_idl); > lport_index_init(&lports, ctx.ovnsb_idl); > mcgroup_index_init(&mcgroups, ctx.ovnsb_idl); > > @@ -561,6 +563,8 @@ main(int argc, char *argv[]) > > mcgroup_index_destroy(&mcgroups); > lport_index_destroy(&lports); > + ldatapath_index_destroy(&ldatapaths); > + > sset_destroy(&all_lports); > > struct local_datapath *cur_node, *next_node; > @@ -570,14 +574,6 @@ main(int argc, char *argv[]) > } > hmap_destroy(&local_datapaths); > > - struct patched_datapath *pd_cur_node, *pd_next_node; > - HMAP_FOR_EACH_SAFE (pd_cur_node, pd_next_node, hmap_node, > - &patched_datapaths) { > - hmap_remove(&patched_datapaths, &pd_cur_node->hmap_node); > - free(pd_cur_node); > - } > - hmap_destroy(&patched_datapaths); > - > The deletion above does not belong in this patch. It belongs in patch 6. Mickey > unixctl_server_run(unixctl); > > unixctl_server_wait(unixctl); > -- > 2.10.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Mon, Dec 05, 2016 at 06:30:17PM -0800, Mickey Spiegel wrote: > On Sun, Dec 4, 2016 at 11:17 PM, Ben Pfaff <blp@ovn.org> wrote: > > > This will have its first real user in an upcoming commit. > > > > I am not sure this is really necessary. See my comments on patch 6. OK, I'll read those comments in patch 6. > At the end there is deleted code for destroying patched_datapaths. > That deletion should not be in this patch, it should be in patch 6. > > Acked-by: Mickey Spiegel <mickeys.dev@gmail.com> Thanks for noticing, I've moved that to patch 6.
diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c index 38aca22..906fda2 100644 --- a/ovn/controller/lport.c +++ b/ovn/controller/lport.c @@ -22,6 +22,68 @@ VLOG_DEFINE_THIS_MODULE(lport); +static struct ldatapath *ldatapath_lookup_by_key__( + const struct ldatapath_index *, uint32_t dp_key); + +void +ldatapath_index_init(struct ldatapath_index *ldatapaths, + struct ovsdb_idl *ovnsb_idl) +{ + hmap_init(&ldatapaths->by_key); + + const struct sbrec_port_binding *pb; + SBREC_PORT_BINDING_FOR_EACH (pb, ovnsb_idl) { + if (!pb->datapath) { + continue; + } + uint32_t dp_key = pb->datapath->tunnel_key; + struct ldatapath *ld = ldatapath_lookup_by_key__(ldatapaths, dp_key); + if (!ld) { + ld = xzalloc(sizeof *ld); + hmap_insert(&ldatapaths->by_key, &ld->by_key_node, dp_key); + ld->db = pb->datapath; + } + + if (ld->n_lports >= ld->allocated_lports) { + ld->lports = x2nrealloc(ld->lports, &ld->allocated_lports, + sizeof *ld->lports); + } + ld->lports[ld->n_lports++] = pb; + } +} + +void +ldatapath_index_destroy(struct ldatapath_index *ldatapaths) +{ + if (!ldatapaths) { + return; + } + + struct ldatapath *ld, *ld_next; + HMAP_FOR_EACH_SAFE (ld, ld_next, by_key_node, &ldatapaths->by_key) { + hmap_remove(&ldatapaths->by_key, &ld->by_key_node); + free(ld->lports); + free(ld); + } + hmap_destroy(&ldatapaths->by_key); +} + +static struct ldatapath *ldatapath_lookup_by_key__( + const struct ldatapath_index *ldatapaths, uint32_t dp_key) +{ + struct ldatapath *ld; + HMAP_FOR_EACH_WITH_HASH (ld, by_key_node, dp_key, &ldatapaths->by_key) { + return ld; + } + return NULL; +} + +const struct ldatapath *ldatapath_lookup_by_key( + const struct ldatapath_index *ldatapaths, uint32_t dp_key) +{ + return ldatapath_lookup_by_key__(ldatapaths, dp_key); +} + /* A logical port. */ struct lport { struct hmap_node name_node; /* Index by name. */ diff --git a/ovn/controller/lport.h b/ovn/controller/lport.h index 0cad74a..fe0e430 100644 --- a/ovn/controller/lport.h +++ b/ovn/controller/lport.h @@ -22,8 +22,37 @@ struct ovsdb_idl; struct sbrec_datapath_binding; -/* Logical port and multicast group indexes - * ======================================== +/* Database indexes. + * ================= + * + * If the database IDL were a little smarter, it would allow us to directly + * look up data based on values of its fields. It's not that smart (yet), so + * instead we define our own indexes. + */ + +/* Logical datapath index + * ====================== + */ + +struct ldatapath { + struct hmap_node by_key_node; /* Index by tunnel key. */ + const struct sbrec_datapath_binding *db; + const struct sbrec_port_binding **lports; + size_t n_lports, allocated_lports; +}; + +struct ldatapath_index { + struct hmap by_key; +}; + +void ldatapath_index_init(struct ldatapath_index *, struct ovsdb_idl *); +void ldatapath_index_destroy(struct ldatapath_index *); + +const struct ldatapath *ldatapath_lookup_by_key( + const struct ldatapath_index *, uint32_t dp_key); + +/* Logical port index + * ================== * * This data structure holds multiple indexes over logical ports, to allow for * efficient searching for logical ports by name or number. diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 894af6f..5b95a55 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -512,8 +512,10 @@ main(int argc, char *argv[]) const struct ovsrec_bridge *br_int = get_br_int(&ctx); const char *chassis_id = get_chassis_id(ctx.ovs_idl); + struct ldatapath_index ldatapaths; struct lport_index lports; struct mcgroup_index mcgroups; + ldatapath_index_init(&ldatapaths, ctx.ovnsb_idl); lport_index_init(&lports, ctx.ovnsb_idl); mcgroup_index_init(&mcgroups, ctx.ovnsb_idl); @@ -561,6 +563,8 @@ main(int argc, char *argv[]) mcgroup_index_destroy(&mcgroups); lport_index_destroy(&lports); + ldatapath_index_destroy(&ldatapaths); + sset_destroy(&all_lports); struct local_datapath *cur_node, *next_node; @@ -570,14 +574,6 @@ main(int argc, char *argv[]) } hmap_destroy(&local_datapaths); - struct patched_datapath *pd_cur_node, *pd_next_node; - HMAP_FOR_EACH_SAFE (pd_cur_node, pd_next_node, hmap_node, - &patched_datapaths) { - hmap_remove(&patched_datapaths, &pd_cur_node->hmap_node); - free(pd_cur_node); - } - hmap_destroy(&patched_datapaths); - unixctl_server_run(unixctl); unixctl_server_wait(unixctl);
This will have its first real user in an upcoming commit. Signed-off-by: Ben Pfaff <blp@ovn.org> --- ovn/controller/lport.c | 62 +++++++++++++++++++++++++++++++++++++++++ ovn/controller/lport.h | 33 ++++++++++++++++++++-- ovn/controller/ovn-controller.c | 12 +++----- 3 files changed, 97 insertions(+), 10 deletions(-)