Re: [ovs-dev] [PATCH v2] OVN: Add support for Transport Zones
On Thu, Mar 28, 2019 at 3:34 PM Lucas Alvares Gomes Martins < lmart...@redhat.com> wrote: > On Thu, Mar 28, 2019 at 9:19 AM Numan Siddique > wrote: > > > > > > Thanks Lucas for the patch. The patch looks good to me. > > Just a few comments inline. > > > > Thanks > > Numan > > > > On Tue, Mar 26, 2019 at 11:55 PM Mark Michelson > wrote: > >> > >> Thanks Lucas, looks good to me. > >> > >> Acked-by: Mark Michelson > >> > >> On 3/25/19 2:24 PM, lmart...@redhat.com wrote: > >> > From: Lucas Alvares Gomes > >> > > >> > 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 > >> > Reported-at: > https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048255.html > >> > Signed-off-by: Lucas Alvares Gomes > >> > --- > >> > > >> > 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", ""); > > > > > > I think you can delete this. It's not used any where. At Line 172 below > you are getting > > the value of "ovn-transport-zones" with in the "if (chassis_rec)" scope. > > > > Hmm I was looking at the code and I think this has to stay because > this value (which comes from the Open_vSwitch table) is being compared > with the one from the Chassis table to see if something changed and > before updating it. > Oops. My bad. Please ignor
Re: [ovs-dev] [PATCH v2] OVN: Add support for Transport Zones
On Thu, Mar 28, 2019 at 9:19 AM Numan Siddique wrote: > > > Thanks Lucas for the patch. The patch looks good to me. > Just a few comments inline. > > Thanks > Numan > > On Tue, Mar 26, 2019 at 11:55 PM Mark Michelson wrote: >> >> Thanks Lucas, looks good to me. >> >> Acked-by: Mark Michelson >> >> On 3/25/19 2:24 PM, lmart...@redhat.com wrote: >> > From: Lucas Alvares Gomes >> > >> > 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 >> > Reported-at: >> > https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048255.html >> > Signed-off-by: Lucas Alvares Gomes >> > --- >> > >> > 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", ""); > > > I think you can delete this. It's not used any where. At Line 172 below you > are getting > the value of "ovn-transport-zones" with in the "if (chassis_rec)" scope. > Hmm I was looking at the code and I think this has to stay because this value (which comes from the Open_vSwitch table) is being compared with the one from the Chassis table to see if something changed and before updating it. An alternative would be to always update the Chassis table with the value from the Open_vSwitch table without comparing it before. But that wouldn't conform with two other options (cms_options and bridge_mappings) which are being updated in the same way; and probably we do not want to issue an update if nothing
Re: [ovs-dev] [PATCH v2] OVN: Add support for Transport Zones
Thanks Lucas for the patch. The patch looks good to me. Just a few comments inline. Thanks Numan On Tue, Mar 26, 2019 at 11:55 PM Mark Michelson wrote: > Thanks Lucas, looks good to me. > > Acked-by: Mark Michelson > > On 3/25/19 2:24 PM, lmart...@redhat.com wrote: > > From: Lucas Alvares Gomes > > > > 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 > > Reported-at: > https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048255.html > > Signed-off-by: Lucas Alvares Gomes > > --- > > > > 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", > ""); > I think you can delete this. It's not used any where. At Line 172 below you are getting the value of "ovn-transport-zones" with in the "if (chassis_rec)" scope. > > > > 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
Re: [ovs-dev] [PATCH v2] OVN: Add support for Transport Zones
Thanks Lucas, looks good to me.
Acked-by: Mark Michelson
On 3/25/19 2:24 PM, lmart...@redhat.com wrote:
From: Lucas Alvares Gomes
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
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048255.html
Signed-off-by: Lucas Alvares Gomes
---
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",
[ovs-dev] [PATCH v2] OVN: Add support for Transport Zones
From: Lucas Alvares Gomes
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
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048255.html
Signed-off-by: Lucas Alvares Gomes
---
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);
