| Message ID | 7b34548a-cf80-96bd-acc0-2accf4c41c59@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Thu, Jul 28, 2016 at 08:05:20PM +0530, Numan Siddique wrote: > This patch reads the external_ids:datapath-type value from the > Open_vSwitch table if defined and sets it in the external_ids:datapath-type > of Chassis table. What sets external_ids:datapath-type? It isn't documented anywhere. > This will provide hints to the CMS or clients monitoring OVN SB DB to > determine the datapath type (DPDK or non-DPDK) configured and take some > actions based on it. > > One usecase is, OVN neutron plugin can use this information to set the > vif_type (ovs or vhostuser) during the port binding. Why would neutron need this information? What does it use it for?
On Thu, Jul 28, 2016 at 10:28 PM, Ben Pfaff <blp@ovn.org> wrote: > On Thu, Jul 28, 2016 at 08:05:20PM +0530, Numan Siddique wrote: > > This patch reads the external_ids:datapath-type value from the > > Open_vSwitch table if defined and sets it in the > external_ids:datapath-type > > of Chassis table. > > What sets external_ids:datapath-type? It isn't documented anywhere. > The way "ovn-bridge-mappings" is set, it is expected to set in the same way. I presume either the Administrator will set it or puppet or some other tool would set it. I will update the documentation accordingly. > > This will provide hints to the CMS or clients monitoring OVN SB DB to > > determine the datapath type (DPDK or non-DPDK) configured and take some > > actions based on it. > > > > One usecase is, OVN neutron plugin can use this information to set the > > vif_type (ovs or vhostuser) during the port binding. > > Why would neutron need this information? What does it use it for? > When a user creates a VM in an open stack deployment, open stack nova will select a compute host for the VM. Before booting the VM, nova asks neutron to create a port and also provides the host name. Neutron can read the datapath-type of the host from the OVN SB DB chassis table and tells nova if it is a dpdk host or normal host so that nova can plug the vif to the ovs bridge properly. With this approach, we can have openstack deployments with DPDK and non-DPDK compute hosts. Right now this mixed deployment is not supported. Russel, Ryan - Please correct me if I am wrong here. Please let me know if this approach seems fine in which case I will update the patch with the documentation. Thanks Numan
On Thu, Jul 28, 2016 at 1:40 PM, Numan Siddique <nusiddiq@redhat.com> wrote: > On Thu, Jul 28, 2016 at 10:28 PM, Ben Pfaff <blp@ovn.org> wrote: > > > On Thu, Jul 28, 2016 at 08:05:20PM +0530, Numan Siddique wrote: > > > This patch reads the external_ids:datapath-type value from the > > > Open_vSwitch table if defined and sets it in the > > external_ids:datapath-type > > > of Chassis table. > > > > What sets external_ids:datapath-type? It isn't documented anywhere. > > > > The way "ovn-bridge-mappings" is set, it is expected to set in the same > way. > I presume either the Administrator will set it or puppet or some other tool > would set it. > I will update the documentation accordingly. > > We spoke about this briefly on IRC, but I think it would be better to read the datapath_type column from the row in the Bridge table for br-int. > > > > This will provide hints to the CMS or clients monitoring OVN SB DB to > > > determine the datapath type (DPDK or non-DPDK) configured and take some > > > actions based on it. > > > > > > One usecase is, OVN neutron plugin can use this information to set the > > > vif_type (ovs or vhostuser) during the port binding. > > > > Why would neutron need this information? What does it use it for? > > > > When a user creates a VM in an open stack deployment, open stack nova will > select a compute host for the VM. Before booting the VM, nova asks neutron > to create a port and also provides the host name. Neutron can read the > datapath-type of the host from the OVN SB DB chassis table and tells nova > if it is a dpdk host or normal host so that nova can plug the vif to the > ovs bridge properly. > > With this approach, we can have openstack deployments with DPDK and > non-DPDK compute hosts. Right now this mixed deployment is not supported. > > Russel, Ryan - Please correct me if I am wrong here. Yes, that's the use case. It seems pretty round about, but this is easiest way for us to integrate into OpenStack today. Exposing it as an external_id on the Chassis seemed like a pretty non-intrusive way to expose the info. Hopefully it's not *too* offensive. :-)
On Thu, Jul 28, 2016 at 02:07:35PM -0400, Russell Bryant wrote: > On Thu, Jul 28, 2016 at 1:40 PM, Numan Siddique <nusiddiq@redhat.com> wrote: > > > On Thu, Jul 28, 2016 at 10:28 PM, Ben Pfaff <blp@ovn.org> wrote: > > > > > On Thu, Jul 28, 2016 at 08:05:20PM +0530, Numan Siddique wrote: > > > > This patch reads the external_ids:datapath-type value from the > > > > Open_vSwitch table if defined and sets it in the > > > external_ids:datapath-type > > > > of Chassis table. > > > > > > What sets external_ids:datapath-type? It isn't documented anywhere. > > > > > > > The way "ovn-bridge-mappings" is set, it is expected to set in the same > > way. > > I presume either the Administrator will set it or puppet or some other tool > > would set it. > > I will update the documentation accordingly. > > > > > We spoke about this briefly on IRC, but I think it would be better to read > the datapath_type column from the row in the Bridge table for br-int. > > > > > > > > This will provide hints to the CMS or clients monitoring OVN SB DB to > > > > determine the datapath type (DPDK or non-DPDK) configured and take some > > > > actions based on it. > > > > > > > > One usecase is, OVN neutron plugin can use this information to set the > > > > vif_type (ovs or vhostuser) during the port binding. > > > > > > Why would neutron need this information? What does it use it for? > > > > > > > When a user creates a VM in an open stack deployment, open stack nova will > > select a compute host for the VM. Before booting the VM, nova asks neutron > > to create a port and also provides the host name. Neutron can read the > > datapath-type of the host from the OVN SB DB chassis table and tells nova > > if it is a dpdk host or normal host so that nova can plug the vif to the > > ovs bridge properly. > > > > With this approach, we can have openstack deployments with DPDK and > > non-DPDK compute hosts. Right now this mixed deployment is not supported. > > > > Russel, Ryan - Please correct me if I am wrong here. > > > Yes, that's the use case. It seems pretty round about, but this is easiest > way for us to integrate into OpenStack today. Exposing it as an > external_id on the Chassis seemed like a pretty non-intrusive way to expose > the info. Hopefully it's not *too* offensive. :-) It's really the idea of adding a external-ids:datapath-type to the Open_vSwitch table that bothered me the most. If we can get it from existing data in br-int, that's much better.
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c index a1545ec..5fc661e 100644 --- a/ovn/controller/chassis.c +++ b/ovn/controller/chassis.c @@ -64,6 +64,13 @@ get_bridge_mappings(const struct smap *ext_ids) return bridge_mappings ? bridge_mappings : ""; } +static const char * +get_datapath_type(const struct smap *ext_ids) +{ + const char *datapath_type = smap_get(ext_ids, "datapath-type"); + return datapath_type ? datapath_type : ""; +} + /* Returns this chassis's Chassis record, if it is available and is currently * amenable to a transaction. */ const struct sbrec_chassis * @@ -114,6 +121,7 @@ chassis_run(struct controller_ctx *ctx, const char *chassis_id) } const char *bridge_mappings = get_bridge_mappings(&cfg->external_ids); + const char *datapath_type = get_datapath_type(&cfg->external_ids); const struct sbrec_chassis *chassis_rec = get_chassis(ctx->ovnsb_idl, chassis_id); @@ -125,10 +133,27 @@ chassis_run(struct controller_ctx *ctx, const char *chassis_id) const char *chassis_bridge_mappings = get_bridge_mappings(&chassis_rec->external_ids); - if (strcmp(bridge_mappings, chassis_bridge_mappings)) { + const char *chassis_datapath_type + = get_datapath_type(&chassis_rec->external_ids); + + if (!strcmp(bridge_mappings, chassis_bridge_mappings)) { + bridge_mappings = NULL; + } + if (!strcmp(datapath_type, chassis_datapath_type)) { + datapath_type = NULL; + } + + if (bridge_mappings || datapath_type) { struct smap new_ids; smap_clone(&new_ids, &chassis_rec->external_ids); - smap_replace(&new_ids, "ovn-bridge-mappings", bridge_mappings); + if (bridge_mappings) { + smap_replace(&new_ids, "ovn-bridge-mappings", bridge_mappings); + } + + if (datapath_type) { + smap_replace(&new_ids, "datapath-type", datapath_type); + } + sbrec_chassis_verify_external_ids(chassis_rec); sbrec_chassis_set_external_ids(chassis_rec, &new_ids); smap_destroy(&new_ids); @@ -169,12 +194,15 @@ chassis_run(struct controller_ctx *ctx, const char *chassis_id) chassis_id); if (!chassis_rec) { - struct smap ext_ids = SMAP_CONST1(&ext_ids, "ovn-bridge-mappings", - bridge_mappings); + struct smap ext_ids = SMAP_INITIALIZER(&ext_ids); + smap_add(&ext_ids, "ovn-bridge-mappings", bridge_mappings); + smap_add(&ext_ids, "datapath-type", datapath_type); + chassis_rec = sbrec_chassis_insert(ctx->ovnsb_idl_txn); sbrec_chassis_set_name(chassis_rec, chassis_id); sbrec_chassis_set_hostname(chassis_rec, hostname); sbrec_chassis_set_external_ids(chassis_rec, &ext_ids); + smap_destroy(&ext_ids); } int n_encaps = count_1bits(req_tunnels); diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index a2349a4..e4084ac 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -151,3 +151,51 @@ OVS_APP_EXIT_AND_WAIT_BY_TARGET([$ovs_base/ovn-sb/ovsdb-server-2.ctl], [$ovs_bas OVS_APP_EXIT_AND_WAIT([ovsdb-server]) AT_CLEANUP + +AT_SETUP([ovn-controller - datapath-type]) +AT_KEYWORDS([ovn]) +ovn_init_db ovn-sb + +net_add n1 +sim_add hv +as hv +ovs-vsctl \ + -- add-br br-phys \ + -- add-br br-eth0 \ + -- add-br br-eth1 \ + -- add-br br-eth2 +ovn_attach n1 br-phys 192.168.0.1 + +# Make sure that the configured bridge mappings in the Open_vSwitch db +# is mirrored into the Chassis record in the OVN_Southbound db. +check_datapath_type () { + datapath_type=$1 + sysid=$(ovs-vsctl get Open_vSwitch . external_ids:system-id) + chassis_datapath_type=$(ovn-sbctl get Chassis ${sysid} external_ids:datapath-type | sed -e 's/\"//g') + #echo "datapath_type = " $datapath_type + #echo "expected datapath_type = " $chassis_datapath_type + AT_CHECK([test "${datapath_type}" = "${chassis_datapath_type}"]) +} + +check_datapath_type "" + +ovs-vsctl set Open_vSwitch . external_ids:datapath-type=foo +check_datapath_type foo + +# Change "ovn-bridge-mappings" value. It should not change the "datapath-type". +ovs-vsctl set Open_vSwitch . external_ids:ovn-bridge-mappings=foo-mapping +check_datapath_type foo + +ovs-vsctl set Open_vSwitch . external_ids:datapath-type=bar +check_datapath_type bar + +ovs-vsctl set Open_vSwitch . external_ids:datapath-type=\"\" +check_datapath_type "" + +# Gracefully terminate daemons +OVN_CLEANUP_SBOX([hv]) +OVN_CLEANUP_VSWITCH([main]) +as ovn-sb +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) + +AT_CLEANUP
This patch reads the external_ids:datapath-type value from the Open_vSwitch table if defined and sets it in the external_ids:datapath-type of Chassis table. This will provide hints to the CMS or clients monitoring OVN SB DB to determine the datapath type (DPDK or non-DPDK) configured and take some actions based on it. One usecase is, OVN neutron plugin can use this information to set the vif_type (ovs or vhostuser) during the port binding. Signed-off-by: Numan Siddique <nusiddiq@redhat.com> --- ovn/controller/chassis.c | 36 ++++++++++++++++++++++++++++++++---- tests/ovn-controller.at | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 4 deletions(-)