| Message ID | 20170503154600.27401-2-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, May 3, 2017 at 8:45 AM, Ben Pfaff <blp@ovn.org> wrote: > This makes it possible for southbound utilities to use friendlier names, > which will happen in upcoming commits. > > Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Andy Zhou <azhou@ovn.org>
Relates to previous comments: would it make sense to use a more generic name instead of 'neutron' to avoid coupling to one solution? On May 3, 2017 11:57 PM, "Andy Zhou" <azhou@ovn.org> wrote: > On Wed, May 3, 2017 at 8:45 AM, Ben Pfaff <blp@ovn.org> wrote: > > This makes it possible for southbound utilities to use friendlier names, > > which will happen in upcoming commits. > > > > Signed-off-by: Ben Pfaff <blp@ovn.org> > Acked-by: Andy Zhou <azhou@ovn.org> > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Thu, May 04, 2017 at 10:01:00AM +0200, Miguel Angel Ajo Pelayo wrote: > Relates to previous comments: would it make sense to use a more generic > name instead of 'neutron' to avoid coupling to one solution? Yes. The situation we have now, for Logical_Switch, is that "name" is a unique identifier and "external_ids:neutron:network_name" is a friendly name. As a technical matter, this works fine for any situation and any CMS. From a non-technical point of view, it is surprising in two ways, first that "name" is a unique identifier rather than a friendly name and second that non-OpenStack CMSes would use a key that contains "neutron". The ideal situation is probably to have "name" be a friendly name and to use an external_ids key for the unique name. It may not be necessary to standardize the unique name, but if we want to then we could choose external_ids:id or external_ids:unique_id or whatever. I didn't propose that change here because it requires coordination with networking-ovn, including some care for upgrades, which might include support for both keys for one or more releases. If folks who work on the Neutron side judge it worthwhile, then I'm more than willing to help out. (There is some precedent for this kind of change in OVS itself. In OVS, we first developed bindings to XenServer, then to other systems. If you search through ovs-vswitchd.conf.db(5), you will find various keys with an "xs-" prefix for XenServer, and you will also find similar keys without those prefixes. The "xs-" keys are the counterparts to the "neutron:" keys. We invented them before we realized that we needed to generalize. It might be possible to remove the "xs-" keys now; the main obstacle to that is that XenServer users don't speak up until a couple of releases have gone by because they don't keep up, so we'd have a very delayed reaction to any problems we might introduce.)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 96aad9ae7582..d393e544c308 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -1705,6 +1705,9 @@ ovn_port_update_sbrec(const struct ovn_port *op, sbrec_port_binding_set_parent_port(op->sb, NULL); sbrec_port_binding_set_tag(op->sb, NULL, 0); sbrec_port_binding_set_mac(op->sb, NULL, 0); + + struct smap ids = SMAP_INITIALIZER(&ids); + sbrec_port_binding_set_external_ids(op->sb, &ids); } else { if (strcmp(op->nbsp->type, "router")) { uint32_t queue_id = smap_get_int( @@ -1797,6 +1800,14 @@ ovn_port_update_sbrec(const struct ovn_port *op, sbrec_port_binding_set_tag(op->sb, op->nbsp->tag, op->nbsp->n_tag); sbrec_port_binding_set_mac(op->sb, (const char **) op->nbsp->addresses, op->nbsp->n_addresses); + + struct smap ids = SMAP_INITIALIZER(&ids); + const char *name = smap_get(&op->nbsp->external_ids, + "neutron:port_name"); + if (name && name[0]) { + smap_add(&ids, "name", name); + } + sbrec_port_binding_set_external_ids(op->sb, &ids); } } @@ -5974,6 +5985,8 @@ main(int argc, char *argv[]) add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_nat_addresses); ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_chassis); + add_column_noalert(ovnsb_idl_loop.idl, + &sbrec_port_binding_col_external_ids); ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_mac_binding); add_column_noalert(ovnsb_idl_loop.idl, &sbrec_mac_binding_col_datapath); add_column_noalert(ovnsb_idl_loop.idl, &sbrec_mac_binding_col_ip); diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml index f127d0df8f8f..383b5b76b569 100644 --- a/ovn/ovn-nb.xml +++ b/ovn/ovn-nb.xml @@ -821,6 +821,22 @@ </column> </group> + <group title="Naming"> + <column name="external_ids" key="neutron:port_name"> + <p> + This column gives an optional human-friendly name for the port. This + name has no special meaning or purpose other than to provide + convenience for human interaction with the northbound database. + </p> + + <p> + Neutron copies this from its own port object's name. (Neutron ports + do are not assigned human-friendly names by default, so it will often + be empty.) + </p> + </column> + </group> + <group title="Common Columns"> <column name="external_ids"> See <em>External IDs</em> at the beginning of this document. diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema index 8a39e61051d1..ae248f8a7c4c 100644 --- a/ovn/ovn-sb.ovsschema +++ b/ovn/ovn-sb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Southbound", - "version": "1.11.0", - "cksum": "2783500150 10696", + "version": "1.12.0", + "cksum": "4119465100 10905", "tables": { "SB_Global": { "columns": { @@ -130,7 +130,11 @@ "max": "unlimited"}}, "nat_addresses": {"type": {"key": "string", "min": 0, - "max": "unlimited"}}}, + "max": "unlimited"}}, + "external_ids": {"type": {"key": "string", + "value": "string", + "min": 0, + "max": "unlimited"}}}, "indexes": [["datapath", "tunnel_key"], ["logical_port"]], "isRoot": true}, "MAC_Binding": { diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index b92c2d065608..8bb0f3ddebc9 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -2129,6 +2129,29 @@ tcp.flags = RST; </p> </column> </group> + + <group title="Naming"> + <column name="external_ids" key="name"> + <p> + For a logical switch port, <code>ovn-northd</code> copies this from + <ref table="Logical_Switch_Port" db="OVN_Northbound" + column="external_ids" key="neutron:port_name"/> in the <ref + table="Logical_Switch_Port" db="OVN_Northbound"/> table in the + OVN_Northbound database, if it is a nonempty string. + </p> + + <p> + For a logical switch port, <code>ovn-northd</code> does not currently + set this key. + </p> + </column> + </group> + + <group title="Common Columns"> + <column name="external_ids"> + See <em>External IDs</em> at the beginning of this document. + </column> + </group> </table> <table name="MAC_Binding" title="IP to MAC bindings">
This makes it possible for southbound utilities to use friendlier names, which will happen in upcoming commits. Signed-off-by: Ben Pfaff <blp@ovn.org> --- ovn/northd/ovn-northd.c | 13 +++++++++++++ ovn/ovn-nb.xml | 16 ++++++++++++++++ ovn/ovn-sb.ovsschema | 10 +++++++--- ovn/ovn-sb.xml | 23 +++++++++++++++++++++++ 4 files changed, 59 insertions(+), 3 deletions(-)