[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