[ovs-dev] [PATCH v2] OVN: Add support for Transport Zones
Mark Michelson
mmichels at redhat.com
Tue Mar 26 18:25:11 UTC 2019
Thanks Lucas, looks good to me.
Acked-by: Mark Michelson <mmichels at redhat.com>
On 3/25/19 2:24 PM, lmartins at redhat.com wrote:
> From: Lucas Alvares Gomes <lucasagomes at gmail.com>
>
> This patch is adding support for Transport Zones. Transport zones (a.k.a
> TZs) is way to enable users of OVN to separate Chassis into different
> logical groups that will form tunnels only between members of the same
> group(s).
>
> Each Chassis can belong to one or more Transport Zones. If not set,
> the Chassis will be considered part of a default group; this feature
> is backward compatible and did not require any changes to the database
> schemas.
>
> Configuring Transport Zones is done by creating a key called
> "ovn-transport-zones" in the external_ids of the Open_vSwitch table of the
> local OVS instance. The value is a string with the name of the Transport
> Zone that this instance is part of. Multiple TZs may be specified with
> a comma-separated list. For example:
>
> $ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1
>
> or
>
> $ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1,tz2,tz3
>
> This configuration will be also exposed in the Chassis table of the OVN
> Southbound Database so that external systems can see what TZ(s) each
> Chassis are part of and make decisions based those values.
>
> The use for Transport Zones includes but are not limited to:
>
> * Edge computing: As a way to preventing edge sites from trying to create
> tunnels with every node on every other edge site while still allowing
> these sites to create tunnels with the central node.
>
> * Extra security layer: Where users wants to create "trust zones"
> and prevent computes in a more secure zone to communicate with a less
> secure zone.
>
> Reported-by: Daniel Alvarez Sanchez <dalvarez at redhat.com>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048255.html
> Signed-off-by: Lucas Alvares Gomes <lucasagomes at gmail.com>
> ---
>
> v1 -> v2
> * Rename the function check_chassis_tzones to chassis_tzones_overlap.
> * Fix a memory leak in chassis_tzones_overlap.
> * Pass the transport_zones to encaps_run() as a "const char *"
> instead of "struct sbrec_chassis". With this we can also avoid not
> running the function in case the Chassis entry is not yet created.
>
> NEWS | 3 +
> ovn/controller/chassis.c | 8 ++-
> ovn/controller/encaps.c | 58 +++++++++++++++++-
> ovn/controller/encaps.h | 3 +-
> ovn/controller/ovn-controller.8.xml | 9 +++
> ovn/controller/ovn-controller.c | 14 ++++-
> tests/ovn.at | 93 +++++++++++++++++++++++++++++
> 7 files changed, 183 insertions(+), 5 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 1e4744dbd..4adf49f57 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -24,6 +24,9 @@ Post-v2.11.0
> protocol extension.
> - OVN:
> * Select IPAM mac_prefix in a random manner if not provided by the user
> + * Support for Transport Zones, a way to separate chassis into
> + logical groups which results in tunnels only been formed between
> + members of the same transport zone(s).
> - New QoS type "linux-netem" on Linux.
>
> v2.11.0 - 19 Feb 2019
> diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
> index 3ea908d18..34c260410 100644
> --- a/ovn/controller/chassis.c
> +++ b/ovn/controller/chassis.c
> @@ -139,6 +139,8 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> const char *datapath_type =
> br_int && br_int->datapath_type ? br_int->datapath_type : "";
> const char *cms_options = get_cms_options(&cfg->external_ids);
> + const char *transport_zones = smap_get_def(&cfg->external_ids,
> + "ovn-transport-zones", "");
>
> struct ds iface_types = DS_EMPTY_INITIALIZER;
> ds_put_cstr(&iface_types, "");
> @@ -167,18 +169,22 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> = smap_get_def(&chassis_rec->external_ids, "iface-types", "");
> const char *chassis_cms_options
> = get_cms_options(&chassis_rec->external_ids);
> + const char *chassis_transport_zones = smap_get_def(
> + &chassis_rec->external_ids, "ovn-transport-zones", "");
>
> /* 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) ||
> - strcmp(cms_options, chassis_cms_options)) {
> + strcmp(cms_options, chassis_cms_options) ||
> + strcmp(transport_zones, chassis_transport_zones)) {
> struct smap new_ids;
> smap_clone(&new_ids, &chassis_rec->external_ids);
> 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);
> smap_replace(&new_ids, "ovn-cms-options", cms_options);
> + smap_replace(&new_ids, "ovn-transport-zones", transport_zones);
> sbrec_chassis_verify_external_ids(chassis_rec);
> sbrec_chassis_set_external_ids(chassis_rec, &new_ids);
> smap_destroy(&new_ids);
> diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
> index 610b833de..72c43a9dd 100644
> --- a/ovn/controller/encaps.c
> +++ b/ovn/controller/encaps.c
> @@ -195,13 +195,56 @@ chassis_tunnel_add(const struct sbrec_chassis *chassis_rec, const struct sbrec_s
> return tuncnt;
> }
>
> +/*
> +* Returns true if our_chassis_tzones and chassis_tzones have at least
> +* one common transport zone.
> +*/
> +static bool
> +chassis_tzones_overlap(const char *our_chassis_tzones,
> + const char *chassis_tzones)
> +{
> + if (!strcmp(our_chassis_tzones, "") && !strcmp(chassis_tzones, "")) {
> + return true;
> + }
> +
> + bool found = false;
> + char *our_tzones_orig;
> + char *our_tzones = xstrdup(our_chassis_tzones);
> + char *i = our_tzones_orig = our_tzones;
> +
> + while ((i = strsep(&our_tzones, ","))) {
> +
> + char *tzones_orig;
> + char *tzones = xstrdup(chassis_tzones);
> + char *j = tzones_orig = tzones;
> +
> + while ((j = strsep(&tzones, ","))) {
> +
> + if (!strcmp(i, j)) {
> + found = true;
> + break;
> + }
> + }
> +
> + free(tzones_orig);
> + if (found) {
> + break;
> + }
> +
> + }
> +
> + free(our_tzones_orig);
> + return found;
> +}
> +
> void
> encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
> const struct ovsrec_bridge_table *bridge_table,
> const struct ovsrec_bridge *br_int,
> const struct sbrec_chassis_table *chassis_table,
> const char *chassis_id,
> - const struct sbrec_sb_global *sbg)
> + const struct sbrec_sb_global *sbg,
> + const char *transport_zones)
> {
> if (!ovs_idl_txn || !br_int) {
> return;
> @@ -251,7 +294,18 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
>
> SBREC_CHASSIS_TABLE_FOR_EACH (chassis_rec, chassis_table) {
> if (strcmp(chassis_rec->name, chassis_id)) {
> - /* Create tunnels to the other chassis. */
> + /* Create tunnels to the other Chassis belonging to the
> + * same transport zone */
> + const char *chassis_tzones = smap_get_def(
> + &chassis_rec->external_ids, "ovn-transport-zones", "");
> +
> + if (!chassis_tzones_overlap(transport_zones, chassis_tzones)) {
> + VLOG_DBG("Skipping encap creation for Chassis '%s' because "
> + "it belongs to different transport zones",
> + chassis_rec->name);
> + continue;
> + }
> +
> if (chassis_tunnel_add(chassis_rec, sbg, &tc) == 0) {
> VLOG_INFO("Creating encap for '%s' failed", chassis_rec->name);
> continue;
> diff --git a/ovn/controller/encaps.h b/ovn/controller/encaps.h
> index 3e0e110ef..9eafec948 100644
> --- a/ovn/controller/encaps.h
> +++ b/ovn/controller/encaps.h
> @@ -32,7 +32,8 @@ void encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
> const struct ovsrec_bridge *br_int,
> const struct sbrec_chassis_table *,
> const char *chassis_id,
> - const struct sbrec_sb_global *);
> + const struct sbrec_sb_global *,
> + const char *transport_zones);
>
> bool encaps_cleanup(struct ovsdb_idl_txn *ovs_idl_txn,
> const struct ovsrec_bridge *br_int);
> diff --git a/ovn/controller/ovn-controller.8.xml b/ovn/controller/ovn-controller.8.xml
> index fd2e10a7a..072ec5820 100644
> --- a/ovn/controller/ovn-controller.8.xml
> +++ b/ovn/controller/ovn-controller.8.xml
> @@ -167,6 +167,15 @@
> specific to this particular chassis. An example would be:
> <code>cms_option1,cms_option2:foo</code>.
> </dd>
> +
> + <dt><code>external_ids:ovn-transport-zones</code></dt>
> + <dd>
> + The transport zone(s) that this chassis belongs to. Transport
> + zones is a way to group different chassis so that tunnels are only
> + formed between members of the same group(s). Multiple transport
> + zones may be specified with a comma-separated list. For example:
> + tz1,tz2,tz3.
> + </dd>
> </dl>
>
> <p>
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
> index 882cc195f..9383ec191 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -511,6 +511,14 @@ get_nb_cfg(const struct sbrec_sb_global_table *sb_global_table)
> return sb ? sb->nb_cfg : 0;
> }
>
> +static const char *
> +get_transport_zones(const struct ovsrec_open_vswitch_table *ovs_table)
> +{
> + const struct ovsrec_open_vswitch *cfg
> + = ovsrec_open_vswitch_table_first(ovs_table);
> + return smap_get_def(&cfg->external_ids, "ovn-transport-zones", "");
> +}
> +
> static void
> ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> {
> @@ -686,6 +694,9 @@ main(int argc, char *argv[])
> const char *chassis_id
> = get_chassis_id(ovsrec_open_vswitch_table_get(
> ovs_idl_loop.idl));
> + const char *transport_zones
> + = get_transport_zones(ovsrec_open_vswitch_table_get(
> + ovs_idl_loop.idl));
>
> const struct sbrec_chassis *chassis = NULL;
> if (chassis_id) {
> @@ -697,7 +708,8 @@ main(int argc, char *argv[])
> ovs_idl_txn,
> ovsrec_bridge_table_get(ovs_idl_loop.idl), br_int,
> sbrec_chassis_table_get(ovnsb_idl_loop.idl), chassis_id,
> - sbrec_sb_global_first(ovnsb_idl_loop.idl));
> + sbrec_sb_global_first(ovnsb_idl_loop.idl),
> + transport_zones);
>
> if (ofctrl_is_connected()) {
> /* Calculate the active tunnels only if have an an active
> diff --git a/tests/ovn.at b/tests/ovn.at
> index f2f2bc405..3988474d0 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -12281,3 +12281,96 @@ ovn-nbctl list logical_switch_port
> ovn-nbctl list logical_router_port
>
> AT_CLEANUP
> +
> +AT_SETUP([ovn -- test transport zones])
> +ovn_start
> +
> +net_add n1
> +for i in 1 2 3; do
> + sim_add hv$i
> + as hv$i
> + ovs-vsctl add-br br-phys
> + ovn_attach n1 br-phys 192.168.$i.1
> +done
> +
> +dnl Assert that each Chassis has a tunnel formed to every other Chassis
> +as hv1
> +AT_CHECK([ovs-vsctl --bare --columns=name find interface type="geneve" | awk NF | sort], [0],
> +[[ovn-hv2-0
> +ovn-hv3-0
> +]])
> +
> +as hv2
> +AT_CHECK([ovs-vsctl --bare --columns=name find interface type="geneve" | awk NF | sort], [0],
> +[[ovn-hv1-0
> +ovn-hv3-0
> +]])
> +
> +as hv3
> +AT_CHECK([ovs-vsctl --bare --columns=name find interface type="geneve" | awk NF | sort], [0],
> +[[ovn-hv1-0
> +ovn-hv2-0
> +]])
> +
> +dnl Let's now add some Chassis to different transport zones
> +dnl * hv1: Will be part of two transport zones: tz1 and tz2 so it
> +dnl should have tunnels formed between the other two Chassis (hv2 and hv3)
> +dnl
> +dnl * hv2: Will be part of one transport zone: tz1. It should have a tunnel
> +dnl to hv1 but not to hv3
> +dnl
> +dnl * hv3: Will be part of one transport zone: tz2. It should have a tunnel
> +dnl to hv1 but not to hv2
> +dnl
> +as hv1
> +ovs-vsctl set open . external-ids:ovn-transport-zones=tz1,tz2
> +
> +as hv2
> +ovs-vsctl set open . external-ids:ovn-transport-zones=tz1
> +
> +as hv3
> +ovs-vsctl set open . external-ids:ovn-transport-zones=tz2
> +
> +as hv1
> +AT_CHECK([ovs-vsctl --bare --columns=name find interface type="geneve" | awk NF | sort], [0],
> +[[ovn-hv2-0
> +ovn-hv3-0
> +]])
> +
> +as hv2
> +AT_CHECK([ovs-vsctl --bare --columns=name find interface type="geneve" | awk NF | sort], [0],
> +[[ovn-hv1-0
> +]])
> +
> +as hv3
> +AT_CHECK([ovs-vsctl --bare --columns=name find interface type="geneve" | awk NF | sort], [0],
> +[[ovn-hv1-0
> +]])
> +
> +dnl Removing the transport zones should make all Chassis to create
> +dnl tunnels between every other Chassis again
> +for i in 1 2 3; do
> + as hv$i
> + ovs-vsctl remove open . external-ids ovn-transport-zones
> +done
> +
> +as hv1
> +AT_CHECK([ovs-vsctl --bare --columns=name find interface type="geneve" | awk NF | sort], [0],
> +[[ovn-hv2-0
> +ovn-hv3-0
> +]])
> +
> +as hv2
> +AT_CHECK([ovs-vsctl --bare --columns=name find interface type="geneve" | awk NF | sort], [0],
> +[[ovn-hv1-0
> +ovn-hv3-0
> +]])
> +
> +as hv3
> +AT_CHECK([ovs-vsctl --bare --columns=name find interface type="geneve" | awk NF | sort], [0],
> +[[ovn-hv1-0
> +ovn-hv2-0
> +]])
> +
> +OVN_CLEANUP([hv1], [hv2], [hv3])
> +AT_CLEANUP
>
More information about the dev
mailing list