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

Justin Pettit jpettit at nicira.com
Thu Jul 30 23:17:16 UTC 2015


Acked-by: Justin Pettit <jpettit at nicira.com>

--Justin


> On 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