On Sat, Jul 30, 2016 at 03:32:01PM +0530, Numan Siddique wrote: > This patch reads the 'Bridge.datapath_type' column value of the integration > bridge and 'Open_vSwitch.iface_types' column value and sets these in the > external_ids:datapath-type and external_ids:iface-types 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 <nusid...@redhat.com>
Thanks for the patch. I made a few simplifications to the code in chassis.c, and fixed a memory leak. I clarified a little in the documentation. I also fixed some terminology (chassis:external-ids implies a column named chassis and a key named external-ids; the right terminology is Chassis external-ids for the Chassis table and external-ids column) and adjusted the test to eliminate some races. I folded in those changes, copied below, and applied this to master. Thanks again! --8<--------------------------cut here-------------------------->8-- diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c index 9c1be0b..97fc2bf 100644 --- a/ovn/controller/chassis.c +++ b/ovn/controller/chassis.c @@ -136,47 +136,23 @@ chassis_run(struct controller_ctx *ctx, const char *chassis_id, sbrec_chassis_set_hostname(chassis_rec, hostname); } + /* Determine new values for Chassis external-ids. */ const char *chassis_bridge_mappings = get_bridge_mappings(&chassis_rec->external_ids); const char *chassis_datapath_type - = smap_get(&chassis_rec->external_ids, "datapath-type"); + = smap_get_def(&chassis_rec->external_ids, "datapath-type", ""); const char *chassis_iface_types - = smap_get(&chassis_rec->external_ids, "iface-types"); + = smap_get_def(&chassis_rec->external_ids, "iface-types", ""); - if (!chassis_datapath_type) { - chassis_datapath_type = ""; - } - - if (!chassis_iface_types) { - chassis_iface_types = ""; - } - - if (!strcmp(bridge_mappings, chassis_bridge_mappings)) { - bridge_mappings = NULL; - } - if (!strcmp(datapath_type, chassis_datapath_type)) { - datapath_type = NULL; - } - - if (!strcmp(iface_types_str, chassis_iface_types)) { - iface_types_str = NULL; - } - - if (bridge_mappings || datapath_type || iface_types_str) { + /* If any of the external-ids should change, update them. */ + if (strcmp(bridge_mappings, chassis_bridge_mappings) || + strcmp(datapath_type, chassis_datapath_type) || + strcmp(iface_types_str, chassis_iface_types)) { struct smap new_ids; smap_clone(&new_ids, &chassis_rec->external_ids); - if (bridge_mappings) { - smap_replace(&new_ids, "ovn-bridge-mappings", bridge_mappings); - } - - if (datapath_type) { - smap_replace(&new_ids, "datapath-type", datapath_type); - } - - if (iface_types_str) { - smap_replace(&new_ids, "iface-types", iface_types_str); - } - + smap_replace(&new_ids, "ovn-bridge-mappings", bridge_mappings); + smap_replace(&new_ids, "datapath-type", datapath_type); + smap_replace(&new_ids, "iface-types", iface_types_str); sbrec_chassis_verify_external_ids(chassis_rec); sbrec_chassis_set_external_ids(chassis_rec, &new_ids); smap_destroy(&new_ids); @@ -194,6 +170,7 @@ chassis_run(struct controller_ctx *ctx, const char *chassis_id, if (same) { /* Nothing changed. */ inited = true; + ds_destroy(&iface_types); return chassis_rec; } else if (!inited) { struct ds cur_encaps = DS_EMPTY_INITIALIZER; diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index f7acf2d..795be68 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -215,18 +215,18 @@ <column name="external_ids" key="datapath-type"> <code>ovn-controller</code> populates this key with the datapath type - configured in the <ref table="Bridge" column="datapath_type"/> of the - Open_vSwitch database's <ref table="Bridge" db="Open_vSwitch"/> table. - Other applications should treat this key as read-only. See + configured in the <ref table="Bridge" column="datapath_type"/> column of + the Open_vSwitch database's <ref table="Bridge" db="Open_vSwitch"/> + table. Other applications should treat this key as read-only. See <code>ovn-controller</code>(8) for more information. </column> <column name="external_ids" key="iface-types"> - <code>ovn-controller</code> populates this key with the iface-types - configured in the <ref table="Open_vSwitch" column="iface_types"/> of the - Open_vSwitch database's <ref table="Open_vSwitch" db="Open_vSwitch"/> table. - Other applications should treat this key as read-only. See - <code>ovn-controller</code>(8) for more information. + <code>ovn-controller</code> populates this key with the interface types + configured in the <ref table="Open_vSwitch" column="iface_types"/> column + of the Open_vSwitch database's <ref table="Open_vSwitch" + db="Open_vSwitch"/> table. Other applications should treat this key as + read-only. See <code>ovn-controller</code>(8) for more information. </column> <group title="Common Columns"> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index 91fb2af..af64804 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -152,7 +152,9 @@ OVS_APP_EXIT_AND_WAIT([ovsdb-server]) AT_CLEANUP -AT_SETUP([ovn-controller - chassis:external_ids - datapath-type and iface-types]) +# Checks that ovn-controller populates datapath-type and iface-types +# correctly in the Chassis external-ids column. +AT_SETUP([ovn-controller - Chassis external_ids]) AT_KEYWORDS([ovn]) ovn_init_db ovn-sb @@ -166,32 +168,34 @@ ovs-vsctl \ -- add-br br-eth2 ovn_attach n1 br-phys 192.168.0.1 -# Make sure that the datapath_type set in the bridge table +sysid=$(ovs-vsctl get Open_vSwitch . external_ids:system-id) + +# Make sure that the datapath_type set in the Bridge table # 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') - AT_CHECK([test "${datapath_type}" = "${chassis_datapath_type}"]) + chassis_datapath_type=$(ovn-sbctl get Chassis ${sysid} external_ids:datapath-type | sed -e 's/"//g') #" + test "${datapath_type}" = "${chassis_datapath_type}" } -check_datapath_type "" +OVS_WAIT_UNTIL([check_datapath_type ""]) ovs-vsctl set Bridge br-int datapath-type=foo -check_datapath_type foo +OVS_WAIT_UNTIL([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 Bridge br-int datapath-type=bar -check_datapath_type bar +OVS_WAIT_UNTIL([check_datapath_type bar]) ovs-vsctl set Bridge br-int datapath-type=\"\" -check_datapath_type "" +OVS_WAIT_UNTIL([check_datapath_type ""]) +# The following will need to be updated as OVS starts to support more +# interface types. expected_iface_types="dummy,dummy-pmd,geneve,gre,internal,ipsec_gre,lisp,patch,stt,system,tap,vxlan" -sysid=$(ovs-vsctl get Open_vSwitch . external_ids:system-id) chassis_iface_types=$(ovn-sbctl get Chassis ${sysid} external_ids:iface-types | sed -e 's/\"//g') echo "chassis_iface_types = ${chassis_iface_types}" AT_CHECK([test "${expected_iface_types}" = "${chassis_iface_types}"]) @@ -199,10 +203,11 @@ AT_CHECK([test "${expected_iface_types}" = "${chassis_iface_types}"]) # Change the value of external_ids:iface-types using ovn-sbctl. # ovn-controller should again set it back to proper one. ovn-sbctl set Chassis ${sysid} external_ids:iface-types="foo" -sleep 1 -chassis_iface_types=$(ovn-sbctl get Chassis ${sysid} external_ids:iface-types | sed -e 's/\"//g') -echo "chassis_iface_types = ${chassis_iface_types}" -AT_CHECK([test "${expected_iface_types}" = "${chassis_iface_types}"]) +OVS_WAIT_UNTIL([ + chassis_iface_types=$(ovn-sbctl get Chassis ${sysid} external_ids:iface-types | sed -e 's/\"//g') + echo "chassis_iface_types = ${chassis_iface_types}" + test "${expected_iface_types}" = "${chassis_iface_types}" +]) # Gracefully terminate daemons OVN_CLEANUP_SBOX([hv]) _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev