[ovs-dev] [PATCH v4] ovn-controller: Add datapath-type and iface-types in chassis:external_ids
Ben Pfaff
blp at ovn.org
Sat Aug 13 16:20:47 UTC 2016
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 <nusiddiq at 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])
More information about the dev
mailing list