[ovs-dev] [PATCH v2 16/21] ovn: Rename Binding table to Port_Binding.

Ben Pfaff blp at nicira.com
Thu Jul 30 17:41:45 UTC 2015


Thanks, I fixed those up too.

On Thu, Jul 30, 2015 at 09:32:05AM -0700, Alex Wang wrote:
> I ran this:
> 
> $ git grep "Binding table" | grep -v Port_
> ovn/controller/pipeline.c: * Binding table within the logical datapath. */
> ovn/controller/pipeline.c:/* Iterates through all of the records in the
> Binding table, updating the
> ovn/ovn-architecture.7.xml:        Binding tables, whereas
> <code>ovn-northd</code>(8) populates the LN
> ovn/ovn-sb.xml:    The Binding tables contain the current placement of
> logical components
> 
> We may need to also clarify these?
> 
> Thanks,
> Alex Wang,
> 
> On Tue, Jul 28, 2015 at 8:44 AM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > An upcoming patch will add a Datapath_Binding table, so clarifying the
> > name seems useful.
> >
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> >  ovn/controller/binding.c  |  28 ++++++-------
> >  ovn/controller/physical.c |   4 +-
> >  ovn/controller/pipeline.c |   4 +-
> >  ovn/northd/ovn-northd.c   | 105
> > ++++++++++++++++++++++++----------------------
> >  ovn/ovn-sb.ovsschema      |   2 +-
> >  ovn/ovn-sb.xml            |   8 ++--
> >  6 files changed, 79 insertions(+), 72 deletions(-)
> >
> > diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> > index c83b044..1b137bb 100644
> > --- a/ovn/controller/binding.c
> > +++ b/ovn/controller/binding.c
> > @@ -76,7 +76,7 @@ binding_run(struct controller_ctx *ctx, const struct
> > ovsrec_bridge *br_int,
> >              const char *chassis_id)
> >  {
> >      const struct sbrec_chassis *chassis_rec;
> > -    const struct sbrec_binding *binding_rec;
> > +    const struct sbrec_port_binding *binding_rec;
> >      struct sset lports, all_lports;
> >      const char *name;
> >
> > @@ -96,11 +96,11 @@ binding_run(struct controller_ctx *ctx, const struct
> > ovsrec_bridge *br_int,
> >      }
> >      sset_clone(&all_lports, &lports);
> >
> > -    ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
> > -                              "ovn-controller: updating bindings for
> > '%s'",
> > -                              chassis_id);
> > +    ovsdb_idl_txn_add_comment(
> > +        ctx->ovnsb_idl_txn,"ovn-controller: updating port bindings for
> > '%s'",
> > +        chassis_id);
> >
> > -    SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> > +    SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> >          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))) {
> > @@ -113,14 +113,14 @@ binding_run(struct controller_ctx *ctx, const struct
> > ovsrec_bridge *br_int,
> >                            binding_rec->chassis->name,
> >                            chassis_rec->name);
> >              }
> > -            sbrec_binding_set_chassis(binding_rec, chassis_rec);
> > +            sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
> >          } else if (binding_rec->chassis == chassis_rec) {
> > -            sbrec_binding_set_chassis(binding_rec, NULL);
> > +            sbrec_port_binding_set_chassis(binding_rec, NULL);
> >          }
> >      }
> >
> >      SSET_FOR_EACH (name, &lports) {
> > -        VLOG_DBG("No binding record for lport %s", name);
> > +        VLOG_DBG("No port binding record for lport %s", name);
> >      }
> >      sset_destroy(&lports);
> >      sset_destroy(&all_lports);
> > @@ -144,15 +144,15 @@ binding_cleanup(struct controller_ctx *ctx, const
> > char *chassis_id)
> >          return true;
> >      }
> >
> > -    ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
> > -                              "ovn-controller: removing all bindings for
> > '%s'",
> > -                              chassis_id);
> > +    ovsdb_idl_txn_add_comment(
> > +        ctx->ovnsb_idl_txn,
> > +        "ovn-controller: removing all port bindings for '%s'",
> > chassis_id);
> >
> > -    const struct sbrec_binding *binding_rec;
> > +    const struct sbrec_port_binding *binding_rec;
> >      bool any_changes = false;
> > -    SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> > +    SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> >          if (binding_rec->chassis == chassis_rec) {
> > -            sbrec_binding_set_chassis(binding_rec, NULL);
> > +            sbrec_port_binding_set_chassis(binding_rec, NULL);
> >              any_changes = true;
> >          }
> >      }
> > diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> > index 11f88cb..55d6107 100644
> > --- a/ovn/controller/physical.c
> > +++ b/ovn/controller/physical.c
> > @@ -91,8 +91,8 @@ physical_run(struct controller_ctx *ctx, const struct
> > ovsrec_bridge *br_int,
> >
> >      /* Set up flows in table 0 for physical-to-logical translation and in
> > table
> >       * 64 for logical-to-physical translation. */
> > -    const struct sbrec_binding *binding;
> > -    SBREC_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> > +    const struct sbrec_port_binding *binding;
> > +    SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> >          /* Find the OpenFlow port for the logical port, as 'ofport'.  If
> > it's
> >           * on a remote chassis, this is the OpenFlow port for the tunnel
> > to
> >           * that chassis (and set 'local' to false).  Otherwise, if it's
> > on the
> > diff --git a/ovn/controller/pipeline.c b/ovn/controller/pipeline.c
> > index 4c0ffd3..1927ce4 100644
> > --- a/ovn/controller/pipeline.c
> > +++ b/ovn/controller/pipeline.c
> > @@ -214,8 +214,8 @@ ldp_run(struct controller_ctx *ctx)
> >          simap_clear(&ldp->ports);
> >      }
> >
> > -    const struct sbrec_binding *binding;
> > -    SBREC_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> > +    const struct sbrec_port_binding *binding;
> > +    SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> >          struct logical_datapath *ldp;
> >
> >          ldp = ldp_lookup(&binding->logical_datapath);
> > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> > index 70d868a..2ad727c 100644
> > --- a/ovn/northd/ovn-northd.c
> > +++ b/ovn/northd/ovn-northd.c
> > @@ -406,7 +406,7 @@ build_pipeline(struct northd_context *ctx)
> >  }
> >
> >  static bool
> > -parents_equal(const struct sbrec_binding *binding,
> > +parents_equal(const struct sbrec_port_binding *binding,
> >                const struct nbrec_logical_port *lport)
> >  {
> >      if (!!binding->parent_port != !!lport->parent_name) {
> > @@ -424,7 +424,7 @@ parents_equal(const struct sbrec_binding *binding,
> >  }
> >
> >  static bool
> > -tags_equal(const struct sbrec_binding *binding,
> > +tags_equal(const struct sbrec_port_binding *binding,
> >             const struct nbrec_logical_port *lport)
> >  {
> >      if (binding->n_tag != lport->n_tag) {
> > @@ -434,16 +434,16 @@ tags_equal(const struct sbrec_binding *binding,
> >      return binding->n_tag ? (binding->tag[0] == lport->tag[0]) : true;
> >  }
> >
> > -struct binding_hash_node {
> > +struct port_binding_hash_node {
> >      struct hmap_node lp_node; /* In 'lp_map', by binding->logical_port. */
> >      struct hmap_node tk_node; /* In 'tk_map', by binding->tunnel_key. */
> > -    const struct sbrec_binding *binding;
> > +    const struct sbrec_port_binding *binding;
> >  };
> >
> >  static bool
> >  tunnel_key_in_use(const struct hmap *tk_hmap, uint16_t tunnel_key)
> >  {
> > -    const struct binding_hash_node *hash_node;
> > +    const struct port_binding_hash_node *hash_node;
> >
> >      HMAP_FOR_EACH_IN_BUCKET (hash_node, tk_node, hash_int(tunnel_key, 0),
> >                               tk_hmap) {
> > @@ -475,19 +475,19 @@ choose_tunnel_key(const struct hmap *tk_hmap)
> >
> >  /*
> >   * When a change has occurred in the OVN_Northbound database, we go
> > through and
> > - * make sure that the contents of the Binding table in the OVN_Southbound
> > + * make sure that the contents of the Port_Binding table in the
> > OVN_Southbound
> >   * database are up to date with the logical ports defined in the
> >   * OVN_Northbound database.
> >   */
> >  static void
> > -set_bindings(struct northd_context *ctx)
> > +set_port_bindings(struct northd_context *ctx)
> >  {
> > -    const struct sbrec_binding *binding;
> > +    const struct sbrec_port_binding *binding;
> >
> >      /*
> > -     * We will need to look up a binding for every logical port.  We
> > don't want
> > -     * to have to do an O(n) search for every binding, so start out by
> > hashing
> > -     * them on the logical port.
> > +     * We will need to look up a port binding for every logical port.  We
> > don't
> > +     * want to have to do an O(n) search for every binding, so start out
> > by
> > +     * hashing them on the logical port.
> >       *
> >       * As we go through every logical port, we will update the binding if
> > it
> >       * exists or create one otherwise.  When the update is done, we'll
> > remove
> > @@ -496,14 +496,14 @@ set_bindings(struct northd_context *ctx)
> >       *
> >       * We index the logical_port column because that's the shared key
> > between
> >       * the OVN_NB and OVN_SB databases.  We index the tunnel_key column to
> > -     * allow us to choose a unique tunnel key for any Binding rows we
> > have to
> > -     * add.
> > +     * allow us to choose a unique tunnel key for any Port_Binding rows
> > we have
> > +     * to add.
> >       */
> >      struct hmap lp_hmap = HMAP_INITIALIZER(&lp_hmap);
> >      struct hmap tk_hmap = HMAP_INITIALIZER(&tk_hmap);
> >
> > -    SBREC_BINDING_FOR_EACH(binding, ctx->ovnsb_idl) {
> > -        struct binding_hash_node *hash_node = xzalloc(sizeof *hash_node);
> > +    SBREC_PORT_BINDING_FOR_EACH(binding, ctx->ovnsb_idl) {
> > +        struct port_binding_hash_node *hash_node = xzalloc(sizeof
> > *hash_node);
> >          hash_node->binding = binding;
> >          hmap_insert(&lp_hmap, &hash_node->lp_node,
> >                      hash_string(binding->logical_port, 0));
> > @@ -517,7 +517,7 @@ set_bindings(struct northd_context *ctx)
> >
> >          for (size_t i = 0; i < lswitch->n_ports; i++) {
> >              const struct nbrec_logical_port *lport = lswitch->ports[i];
> > -            struct binding_hash_node *hash_node;
> > +            struct port_binding_hash_node *hash_node;
> >              binding = NULL;
> >              HMAP_FOR_EACH_WITH_HASH(hash_node, lp_node,
> >                                      hash_string(lport->name, 0),
> > &lp_hmap) {
> > @@ -535,19 +535,22 @@ set_bindings(struct northd_context *ctx)
> >
> >                  if (!macs_equal(binding->mac, binding->n_mac,
> >                                  lport->macs, lport->n_macs)) {
> > -                    sbrec_binding_set_mac(binding, (const char **)
> > lport->macs,
> > -                                          lport->n_macs);
> > +                    sbrec_port_binding_set_mac(binding,
> > +                                               (const char **)
> > lport->macs,
> > +                                               lport->n_macs);
> >                  }
> >                  if (!parents_equal(binding, lport)) {
> > -                    sbrec_binding_set_parent_port(binding,
> > lport->parent_name);
> > +                    sbrec_port_binding_set_parent_port(binding,
> > +
> >  lport->parent_name);
> >                  }
> >                  if (!tags_equal(binding, lport)) {
> > -                    sbrec_binding_set_tag(binding, lport->tag,
> > lport->n_tag);
> > +                    sbrec_port_binding_set_tag(binding,
> > +                                               lport->tag, lport->n_tag);
> >                  }
> >                  if (!uuid_equals(&binding->logical_datapath,
> >                                   logical_datapath)) {
> > -                    sbrec_binding_set_logical_datapath(binding,
> > -                                                       *logical_datapath);
> > +                    sbrec_port_binding_set_logical_datapath(binding,
> > +
> > *logical_datapath);
> >                  }
> >              } else {
> >                  /* There is no binding for this logical port, so create
> > one. */
> > @@ -557,23 +560,27 @@ set_bindings(struct northd_context *ctx)
> >                      continue;
> >                  }
> >
> > -                binding = sbrec_binding_insert(ctx->ovnsb_txn);
> > -                sbrec_binding_set_logical_port(binding, lport->name);
> > -                sbrec_binding_set_mac(binding, (const char **)
> > lport->macs,
> > -                                      lport->n_macs);
> > +                binding = sbrec_port_binding_insert(ctx->ovnsb_txn);
> > +                sbrec_port_binding_set_logical_port(binding, lport->name);
> > +                sbrec_port_binding_set_mac(binding,
> > +                                           (const char **) lport->macs,
> > +                                           lport->n_macs);
> >                  if (lport->parent_name && lport->n_tag > 0) {
> > -                    sbrec_binding_set_parent_port(binding,
> > lport->parent_name);
> > -                    sbrec_binding_set_tag(binding, lport->tag,
> > lport->n_tag);
> > +                    sbrec_port_binding_set_parent_port(binding,
> > +
> >  lport->parent_name);
> > +                    sbrec_port_binding_set_tag(binding,
> > +                                               lport->tag, lport->n_tag);
> >                  }
> >
> > -                sbrec_binding_set_tunnel_key(binding, tunnel_key);
> > -                sbrec_binding_set_logical_datapath(binding,
> > *logical_datapath);
> > +                sbrec_port_binding_set_tunnel_key(binding, tunnel_key);
> > +                sbrec_port_binding_set_logical_datapath(binding,
> > +
> > *logical_datapath);
> >
> >                  /* Add the tunnel key to the tk_hmap so that we don't try
> > to
> >                   * use it for another port.  (We don't want it in the
> > lp_hmap
> >                   * because that would just get the Binding record deleted
> >                   * later.) */
> > -                struct binding_hash_node *hash_node
> > +                struct port_binding_hash_node *hash_node
> >                      = xzalloc(sizeof *hash_node);
> >                  hash_node->binding = binding;
> >                  hmap_insert(&tk_hmap, &hash_node->tk_node,
> > @@ -582,14 +589,14 @@ set_bindings(struct northd_context *ctx)
> >          }
> >      }
> >
> > -    struct binding_hash_node *hash_node;
> > +    struct port_binding_hash_node *hash_node;
> >      HMAP_FOR_EACH (hash_node, lp_node, &lp_hmap) {
> >          hmap_remove(&lp_hmap, &hash_node->lp_node);
> > -        sbrec_binding_delete(hash_node->binding);
> > +        sbrec_port_binding_delete(hash_node->binding);
> >      }
> >      hmap_destroy(&lp_hmap);
> >
> > -    struct binding_hash_node *hash_node_next;
> > +    struct port_binding_hash_node *hash_node_next;
> >      HMAP_FOR_EACH_SAFE (hash_node, hash_node_next, tk_node, &tk_hmap) {
> >          hmap_remove(&tk_hmap, &hash_node->tk_node);
> >          free(hash_node);
> > @@ -602,20 +609,20 @@ ovnnb_db_changed(struct northd_context *ctx)
> >  {
> >      VLOG_DBG("ovn-nb db contents have changed.");
> >
> > -    set_bindings(ctx);
> > +    set_port_bindings(ctx);
> >      build_pipeline(ctx);
> >  }
> >
> >  /*
> >   * The only change we get notified about is if the 'chassis' column of the
> > - * 'Binding' table changes.  When this column is not empty, it means we
> > need to
> > - * set the corresponding logical port as 'up' in the northbound DB.
> > + * 'Port_Binding' table changes.  When this column is not empty, it means
> > we
> > + * need to set the corresponding logical port as 'up' in the northbound
> > DB.
> >   */
> >  static void
> >  ovnsb_db_changed(struct northd_context *ctx)
> >  {
> >      struct hmap lports_hmap;
> > -    const struct sbrec_binding *binding;
> > +    const struct sbrec_port_binding *binding;
> >      const struct nbrec_logical_port *lport;
> >
> >      struct lport_hash_node {
> > @@ -634,7 +641,7 @@ ovnsb_db_changed(struct northd_context *ctx)
> >                  hash_string(lport->name, 0));
> >      }
> >
> > -    SBREC_BINDING_FOR_EACH(binding, ctx->ovnsb_idl) {
> > +    SBREC_PORT_BINDING_FOR_EACH(binding, ctx->ovnsb_idl) {
> >          lport = NULL;
> >          HMAP_FOR_EACH_WITH_HASH(hash_node, node,
> >                  hash_string(binding->logical_port, 0), &lports_hmap) {
> > @@ -645,9 +652,9 @@ ovnsb_db_changed(struct northd_context *ctx)
> >          }
> >
> >          if (!lport) {
> > -            /* The logical port doesn't exist for this binding.  This can
> > +            /* The logical port doesn't exist for this port binding.
> > This can
> >               * happen under normal circumstances when ovn-northd hasn't
> > gotten
> > -             * around to pruning the Binding yet. */
> > +             * around to pruning the Port_Binding yet. */
> >              continue;
> >          }
> >
> > @@ -789,14 +796,14 @@ main(int argc, char *argv[])
> >       * has to care about, so we'll enable monitoring those directly. */
> >      ctx.ovnsb_idl = ovnsb_idl = ovsdb_idl_create(ovnsb_db,
> >              &sbrec_idl_class, false, true);
> > -    ovsdb_idl_add_table(ovnsb_idl, &sbrec_table_binding);
> > -    ovsdb_idl_add_column(ovnsb_idl, &sbrec_binding_col_logical_port);
> > -    ovsdb_idl_add_column(ovnsb_idl, &sbrec_binding_col_chassis);
> > -    ovsdb_idl_add_column(ovnsb_idl, &sbrec_binding_col_mac);
> > -    ovsdb_idl_add_column(ovnsb_idl, &sbrec_binding_col_tag);
> > -    ovsdb_idl_add_column(ovnsb_idl, &sbrec_binding_col_parent_port);
> > -    ovsdb_idl_add_column(ovnsb_idl, &sbrec_binding_col_logical_datapath);
> > -    ovsdb_idl_add_column(ovnsb_idl, &sbrec_binding_col_tunnel_key);
> > +    ovsdb_idl_add_table(ovnsb_idl, &sbrec_table_port_binding);
> > +    ovsdb_idl_add_column(ovnsb_idl, &sbrec_port_binding_col_logical_port);
> > +    ovsdb_idl_add_column(ovnsb_idl, &sbrec_port_binding_col_chassis);
> > +    ovsdb_idl_add_column(ovnsb_idl, &sbrec_port_binding_col_mac);
> > +    ovsdb_idl_add_column(ovnsb_idl, &sbrec_port_binding_col_tag);
> > +    ovsdb_idl_add_column(ovnsb_idl, &sbrec_port_binding_col_parent_port);
> > +    ovsdb_idl_add_column(ovnsb_idl,
> > &sbrec_port_binding_col_logical_datapath);
> > +    ovsdb_idl_add_column(ovnsb_idl, &sbrec_port_binding_col_tunnel_key);
> >      ovsdb_idl_add_column(ovnsb_idl, &sbrec_pipeline_col_logical_datapath);
> >      ovsdb_idl_omit_alert(ovnsb_idl, &sbrec_pipeline_col_logical_datapath);
> >      ovsdb_idl_add_column(ovnsb_idl, &sbrec_pipeline_col_table_id);
> > diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema
> > index f255006..4a2df47 100644
> > --- a/ovn/ovn-sb.ovsschema
> > +++ b/ovn/ovn-sb.ovsschema
> > @@ -44,7 +44,7 @@
> >                  "match": {"type": "string"},
> >                  "actions": {"type": "string"}},
> >              "isRoot": true},
> > -        "Binding": {
> > +        "Port_Binding": {
> >              "columns": {
> >                  "logical_datapath": {"type": "uuid"},
> >                  "logical_port": {"type": "string"},
> > diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
> > index 52fe969..13e5145 100644
> > --- a/ovn/ovn-sb.xml
> > +++ b/ovn/ovn-sb.xml
> > @@ -98,7 +98,7 @@
> >    </p>
> >
> >    <p>
> > -    The <ref table="Binding"/> table is currently the only binding data.
> > +    The <ref table="Port_Binding"/> table is currently the only binding
> > data.
> >    </p>
> >
> >    <table name="Chassis" title="Physical Network Hypervisor and Gateway
> > Information">
> > @@ -226,7 +226,7 @@
> >      <column name="logical_datapath">
> >        The logical datapath to which the logical flow belongs.  A logical
> >        datapath implements a logical pipeline among the ports in the <ref
> > -      table="Binding"/> table associated with it.  (No table represents a
> > +      table="Port_Binding"/> table associated with it.  (No table
> > represents a
> >        logical datapath.)  In practice, the pipeline in a given logical
> > datapath
> >        implements either a logical switch or a logical router, and
> >        <code>ovn-northd</code> reuses the UUIDs for those logical entities
> > from
> > @@ -451,7 +451,7 @@
> >          String constants have the same syntax as quoted strings in JSON
> > (thus,
> >          they are Unicode strings).  String constants are used for naming
> >          logical ports.  Thus, the useful values are <ref
> > -        column="logical_port"/> names from the <ref column="Binding"/> and
> > +        column="logical_port"/> names from the <ref
> > column="Port_Binding"/> and
> >          <ref column="Gateway"/> tables in a logical flow's <ref
> >          column="logical_datapath"/>.
> >        </p>
> > @@ -628,7 +628,7 @@
> >      </column>
> >    </table>
> >
> > -  <table name="Binding" title="Physical-Logical Bindings">
> > +  <table name="Port_Binding" title="Physical-Logical Port Bindings">
> >      <p>
> >        Each row in this table identifies the physical location of a logical
> >        port.
> > --
> > 2.1.3
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> >



More information about the dev mailing list