Re: [ovs-dev] [PATCH ovn v2] Avoid nb_cfg update notification flooding

2020-07-30 Thread Lucas Alvares Gomes Martins
On Wed, Jul 29, 2020 at 8:03 PM Han Zhou  wrote:
>
>
>
> On Wed, Jul 29, 2020 at 2:46 AM Lucas Alvares Gomes  
> wrote:
> >
> > Hi Han,
> >
> > >
> > > Just to follow up. I didn't find any new versions for this patch and it
> > > doesn't seem to be merged. Did you send any new versions or do you still
> > > plan to work on this?
> > >
> >
> > Thanks for this follow up.
> >
> > So I am not currently working on it but the work is still in our
> > backlog. Many of the problems that we had with the mechanism
> > incrementing the "nb_cfg" has been mitigated by some code in the
> > OpenStack side as well as this commit for core OVN [0] that disabled
> > flow recomputation upon updates to the Chassis' external_ids column
> > (we had a few updates to that column every time the nb_cfg was
> > bumped).
> >
> > But, I can try to check if I can get some prioritization and start
> > working again on this patch. Is this something that you need as well ?
> >
> > [0] 
> > https://github.com/ovn-org/ovn/commit/74d90c2223d0a8c123823fb849b4c2de58c296e4
> >
> > Cheers,
> > Lucas
>
> Thanks Lucas for the update.
> Yes, I need it for latency analysis in scale test. I can pick it up. I 
> rebased on master and also fixed the RBAC issue and some minor updates. I 
> will post it later this week.
>

Thank you very much Han!

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2] OVN: Add support for Transport Zones

2019-03-28 Thread Lucas Alvares Gomes Martins
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] OVN: Add support for Transport Zones

2019-03-25 Thread Lucas Alvares Gomes Martins
Hi,

Thanks a lot Mark for the quick review.

On Mon, Mar 25, 2019 at 3:40 PM Mark Michelson  wrote:
>
> Hi Lucas,
>
> Thanks for the patch. It's mostly good but I have a few issues with it.
> See below.
>
> On 3/25/19 10:36 AM, 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 
> > ---
> >   NEWS|  3 +
> >   ovn/controller/chassis.c|  8 ++-
> >   ovn/controller/encaps.c | 54 +++--
> >   ovn/controller/encaps.h |  3 +-
> >   ovn/controller/ovn-controller.8.xml |  9 +++
> >   ovn/controller/ovn-controller.c |  2 +-
> >   tests/ovn.at| 93 +
> >   7 files changed, 164 insertions(+), 8 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);
> >   sm

Re: [ovs-dev] [PATCH v1] OVN: Add external_ids to NAT and Logical_Router_Static_Route tables.

2017-12-06 Thread Lucas Alvares Gomes Martins
Hi,

Thanks a lot Russell and Ben for this email. Having this change in the
2.8 will make certain things *a lot* easier in networking-ovn.

I will go ahead and propose a patch for the 2.8 branch then.

Cheers,
Lucas

On Tue, Dec 5, 2017 at 7:20 PM, Ben Pfaff  wrote:
> I don't object to #2.
>
> On Tue, Dec 05, 2017 at 01:44:25PM -0500, Russell Bryant wrote:
>> Lucas asked me about backporting this one, as OpenStack would start
>> making use of it with an OVS 2.8 update if available.
>>
>> The schema change seems pretty harmless.  The catch is that this also
>> updated the schema version number from "5.8.1" to "5.8.2", while
>> branch-2.8 has "5.8.0".  master includes a change that introduced a
>> new feature, along with the "5.8.1" update, that we would not
>> backport.
>>
>> The main choices seem to be ...
>>
>> 1) Don't backport.
>>
>> 2) Backport, but leave the schema version number unchanged in branch-2.8.
>>
>> Does anyone see a problem with option #2?  It's easy enough to
>> determine if the new columns are present, even without the version
>> number bump.
>>
>> On Mon, Dec 4, 2017 at 2:11 PM, Ben Pfaff  wrote:
>> > Applied, thanks.
>> >
>> > On Mon, Dec 04, 2017 at 03:06:39PM +0100, Daniel Alvarez Sanchez wrote:
>> >> Acked-by: Daniel Alvarez 
>> >>
>> >> From [0] one can expect this column to be present in all tables.
>> >> [0] https://github.com/openvswitch/ovs/blob/v2.8.1/ovn/ovn-nb.xml#L19
>> >>
>> >> On Mon, Dec 4, 2017 at 2:16 PM,  wrote:
>> >>
>> >> > From: Lucas Alvares Gomes 
>> >> >
>> >> > The external_ids column is missing from the NAT and
>> >> > Logical_Router_Static_Route tables.
>> >> >
>> >> > Signed-off-by: Lucas Alvares Gomes 
>> >> > ---
>> >> >  ovn/ovn-nb.ovsschema | 14 ++
>> >> >  ovn/ovn-nb.xml   | 14 ++
>> >> >  2 files changed, 24 insertions(+), 4 deletions(-)
>> >> >
>> >> > diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
>> >> > index fcd878cf2..081ddb54c 100644
>> >> > --- a/ovn/ovn-nb.ovsschema
>> >> > +++ b/ovn/ovn-nb.ovsschema
>> >> > @@ -1,7 +1,7 @@
>> >> >  {
>> >> >  "name": "OVN_Northbound",
>> >> > -"version": "5.8.1",
>> >> > -"cksum": "607160660 16929",
>> >> > +"version": "5.9.0",
>> >> > +"cksum": "1120419033 17249",
>> >> >  "tables": {
>> >> >  "NB_Global": {
>> >> >  "columns": {
>> >> > @@ -238,7 +238,10 @@
>> >> >   
>> >> > "dst-ip"]]},
>> >> >  "min": 0, "max": 1}},
>> >> >  "nexthop": {"type": "string"},
>> >> > -"output_port": {"type": {"key": "string", "min": 0,
>> >> > "max": 1}}},
>> >> > +"output_port": {"type": {"key": "string", "min": 0,
>> >> > "max": 1}},
>> >> > +"external_ids": {
>> >> > +"type": {"key": "string", "value": "string",
>> >> > + "min": 0, "max": "unlimited"}}},
>> >> >  "isRoot": false},
>> >> >  "NAT": {
>> >> >  "columns": {
>> >> > @@ -252,7 +255,10 @@
>> >> > "enum": ["set", ["dnat",
>> >> >   "snat",
>> >> >
>> >> > "dnat_and_snat"
>> >> > -   ]],
>> >> > +   ]]}}},
>> >> > +"external_ids": {
>> >> > +"type": {"key": "string", "value": "string",
>> >> > + "min": 0, "max": "unlimited"}}},
>> >> >  "isRoot": false},
>> >> >  "DHCP_Options": {
>> >> >  "columns": {
>> >> > diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
>> >> > index 1091c05ce..4e3899f28 100644
>> >> > --- a/ovn/ovn-nb.xml
>> >> > +++ b/ovn/ovn-nb.xml
>> >> > @@ -1540,6 +1540,13 @@
>> >> >  address as the one via which the  is
>> >> > reachable.
>> >> >
>> >> >  
>> >> > +
>> >> > +
>> >> > +  
>> >> > +See External IDs at the beginning of this document.
>> >> > +  
>> >> > +
>> >> > +
>> >> >
>> >> >
>> >> >
>> >> > @@ -1618,6 +1625,13 @@
>> >> >  port instance on the redirect-chassis.
>> >> >
>> >> >  
>> >> > +
>> >> > +
>> >> > +  
>> >> > +See External IDs at the beginning of this document.
>> >> > +  
>> >> > +
>> >> > +
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > 2.15.1
>> >> >
>> >> > ___
>> >> > dev mailing list
>> >> > d...@openvswitch.org
>> >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> >> >
>> >> ___
>> >> dev mailing list
>> >> d...@openvswitch.org
>> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> > ___
>> > dev mailing list
>> > d...@openvswitch.org
>> > ht