Fine with me.
On Thu, May 21, 2015 at 10:02:12AM -0700, Alex Wang wrote:
> How about adding an inline function like this?~
>
> On Thu, May 21, 2015 at 10:04 AM, Alex Wang <al...@nicira.com> 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 <al...@nicira.com>
> > Acked-by: Ben Pfaff <b...@nicira.com>
> >
> > ---
> > 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
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
