How about adding an inline function like this?~ On Thu, May 21, 2015 at 10:04 AM, Alex Wang <[email protected]> wrote:
> This commit changes the type of 'chassis' column in 'Binding' table > from string to weak reference of 'Chassis' table entry. This will > make accessing the chassis from binding more efficient. > > Signed-off-by: Alex Wang <[email protected]> > Acked-by: Ben Pfaff <[email protected]> > > --- > PATCH->V2: > - Add helper function for getting chassis by name. > --- > ovn/controller/binding.c | 31 ++++++++++++++++++++++--------- > ovn/controller/chassis.c | 13 ++----------- > ovn/controller/ovn-controller.h | 16 ++++++++++++++++ > ovn/controller/physical.c | 2 +- > ovn/northd/ovn-northd.c | 4 ++-- > ovn/ovn-sb.ovsschema | 5 ++++- > ovn/ovn-sb.xml | 3 +-- > 7 files changed, 48 insertions(+), 26 deletions(-) > > diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c > index ab6d9f9..0a4a39e 100644 > --- a/ovn/controller/binding.c > +++ b/ovn/controller/binding.c > @@ -74,12 +74,18 @@ get_local_iface_ids(struct controller_ctx *ctx, struct > sset *lports) > void > binding_run(struct controller_ctx *ctx) > { > + const struct sbrec_chassis *chassis_rec; > const struct sbrec_binding *binding_rec; > struct ovsdb_idl_txn *txn; > struct sset lports, all_lports; > const char *name; > int retval; > > + chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id); > + if (!chassis_rec) { > + return; > + } > + > sset_init(&lports); > sset_init(&all_lports); > get_local_iface_ids(ctx, &lports); > @@ -94,17 +100,18 @@ binding_run(struct controller_ctx *ctx) > if (sset_find_and_delete(&lports, binding_rec->logical_port) || > (binding_rec->parent_port && binding_rec->parent_port[0] > && > sset_contains(&all_lports, binding_rec->parent_port))) { > - if (!strcmp(binding_rec->chassis, ctx->chassis_id)) { > + if (binding_rec->chassis == chassis_rec) { > continue; > } > - if (binding_rec->chassis[0]) { > + if (binding_rec->chassis) { > VLOG_INFO("Changing chassis for lport %s from %s to %s", > - binding_rec->logical_port, binding_rec->chassis, > - ctx->chassis_id); > + binding_rec->logical_port, > + binding_rec->chassis->name, > + chassis_rec->name); > } > - sbrec_binding_set_chassis(binding_rec, ctx->chassis_id); > - } else if (!strcmp(binding_rec->chassis, ctx->chassis_id)) { > - sbrec_binding_set_chassis(binding_rec, ""); > + sbrec_binding_set_chassis(binding_rec, chassis_rec); > + } else if (binding_rec->chassis == chassis_rec) { > + sbrec_binding_set_chassis(binding_rec, NULL); > } > } > > @@ -126,10 +133,16 @@ binding_run(struct controller_ctx *ctx) > void > binding_destroy(struct controller_ctx *ctx) > { > + const struct sbrec_chassis *chassis_rec; > int retval = TXN_TRY_AGAIN; > > ovs_assert(ctx->ovnsb_idl); > > + chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id); > + if (!chassis_rec) { > + return; > + } > + > while (retval != TXN_SUCCESS && retval != TXN_UNCHANGED) { > const struct sbrec_binding *binding_rec; > struct ovsdb_idl_txn *txn; > @@ -140,8 +153,8 @@ binding_destroy(struct controller_ctx *ctx) > ctx->chassis_id); > > SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) { > - if (!strcmp(binding_rec->chassis, ctx->chassis_id)) { > - sbrec_binding_set_chassis(binding_rec, ""); > + if (binding_rec->chassis == chassis_rec) { > + sbrec_binding_set_chassis(binding_rec, NULL); > } > } > > diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c > index 1a2bb32..63e1160 100644 > --- a/ovn/controller/chassis.c > +++ b/ovn/controller/chassis.c > @@ -55,11 +55,7 @@ register_chassis(struct controller_ctx *ctx) > int retval = TXN_TRY_AGAIN; > struct ovsdb_idl_txn *txn; > > - SBREC_CHASSIS_FOR_EACH(chassis_rec, ctx->ovnsb_idl) { > - if (!strcmp(chassis_rec->name, ctx->chassis_id)) { > - break; > - } > - } > + chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id); > > /* xxx Need to support more than one encap. Also need to support > * xxx encap options. */ > @@ -396,12 +392,7 @@ chassis_destroy(struct controller_ctx *ctx) > const struct sbrec_chassis *chassis_rec; > struct ovsdb_idl_txn *txn; > > - SBREC_CHASSIS_FOR_EACH(chassis_rec, ctx->ovnsb_idl) { > - if (!strcmp(chassis_rec->name, ctx->chassis_id)) { > - break; > - } > - } > - > + chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, > ctx->chassis_id); > if (!chassis_rec) { > break; > } > diff --git a/ovn/controller/ovn-controller.h > b/ovn/controller/ovn-controller.h > index a1630f7..6f98658 100644 > --- a/ovn/controller/ovn-controller.h > +++ b/ovn/controller/ovn-controller.h > @@ -17,6 +17,8 @@ > #ifndef OVN_CONTROLLER_H > #define OVN_CONTROLLER_H 1 > > +#include "ovn/lib/ovn-sb-idl.h" > + > struct controller_ctx { > char *chassis_id; /* ID for this chassis. */ > char *br_int_name; /* Name of local integration bridge. > */ > @@ -26,4 +28,18 @@ struct controller_ctx { > const struct ovsrec_bridge *br_int; > }; > > +static inline const struct sbrec_chassis * > +get_chassis_by_name(struct ovsdb_idl *ovnsb_idl, char *chassis_id) > +{ > + const struct sbrec_chassis *chassis_rec; > + > + SBREC_CHASSIS_FOR_EACH(chassis_rec, ovnsb_idl) { > + if (!strcmp(chassis_rec->name, chassis_id)) { > + break; > + } > + } > + > + return chassis_rec; > +} > + > #endif /* ovn/ovn-controller.h */ > diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c > index 0fb43c0..dc2fcee 100644 > --- a/ovn/controller/physical.c > +++ b/ovn/controller/physical.c > @@ -116,7 +116,7 @@ physical_run(struct controller_ctx *ctx) > bool local = ofport != 0; > if (!local) { > ofport = u16_to_ofp(simap_get(&chassis_to_ofport, > - binding->chassis)); > + binding->chassis->name)); > if (!ofport) { > continue; > } > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index cfad6be..f00e43e 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -660,10 +660,10 @@ ovnsb_db_changed(struct northd_context *ctx) > continue; > } > > - if (*binding->chassis && (!lport->up || !*lport->up)) { > + if (binding->chassis && (!lport->up || !*lport->up)) { > bool up = true; > nbrec_logical_port_set_up(lport, &up, 1); > - } else if (!*binding->chassis && (!lport->up || *lport->up)) { > + } else if (!binding->chassis && (!lport->up || *lport->up)) { > bool up = false; > nbrec_logical_port_set_up(lport, &up, 1); > } > diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema > index 699bfc5..a688e76 100644 > --- a/ovn/ovn-sb.ovsschema > +++ b/ovn/ovn-sb.ovsschema > @@ -59,7 +59,10 @@ > "minInteger": 0, > "maxInteger": 4095}, > "min": 0, "max": 1}}, > - "chassis": {"type": "string"}, > + "chassis": {"type": {"key": {"type": "uuid", > + "refTable": "Chassis", > + "refType": "weak"}, > + "min": 0, "max": 1}}, > "mac": {"type": {"key": "string", > "min": 0, > "max": "unlimited"}}}, > diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml > index 334d11a..fdf59f0 100644 > --- a/ovn/ovn-sb.xml > +++ b/ovn/ovn-sb.xml > @@ -711,8 +711,7 @@ > > <column name="chassis"> > The physical location of the logical port. To successfully > identify a > - chassis, this column must match the <ref table="Chassis" > column="name"/> > - column in some row in the <ref table="Chassis"/> table. This is > + chassis, this column must be a <ref table="Chassis"/> record. This > is > populated by <code>ovn-controller</code>. > </column> > > -- > 1.7.9.5 > > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
