Re: [ovs-dev] [patch_v5] ovn: Add datapaths of interest filtering.

2016-10-05 Thread Darrell Ball 
I made a correction inline

On Tue, Oct 4, 2016 at 10:04 PM, Darrell Ball  wrote:

> This patch adds datapaths of interest support where only datapaths of
> local interest are monitored by the ovn-controller ovsdb client.  The
> idea is to do a flood fill in ovn-controller of datapath associations
> calculated by northd. A new column is added to the SB database
> datapath_binding table - related_datapaths to facilitate this so all
> datapaths associations are known quickly in ovn-controller.  This
> allows monitoring to adapt quickly with a single new monitor setting
> for all datapaths of interest locally.
>
> To reduce risk and minimize latency on network churn, only logical
> flows are conditionally monitored for now.  This should provide
> the bulk of the benefit.  This will be re-evaluated later to
> possibly add additional conditional monitoring such as for
> logical ports.
>
> Liran Schour contributed enhancements to the conditional
> monitoring granularity in ovs and also submitted patches
> for ovn usage of conditional monitoring which aided this patch
> though sharing of concepts through code review work.
>
> Ben Pfaff suggested that northd could be used to pre-populate
> related datapaths for ovn-controller to use.  That idea is
> used as part of this patch.
>
> CC: Ben Pfaff 
> CC: Liran Schour 
> Signed-off-by: Darrell Ball 
> ---
>
> v4->v5: Correct cleanup of monitors.
> Fix warning.
>
> v3->v4: Refactor after incremental processing backout.
> Limit filtering to logical flows to limit risk.
>
> v2->v3: Line length violation fixups :/
>
> v1->v2: Added logical port removal monitoring handling, factoring
> in recent changes for incremental processing.  Added some
> comments in the code regarding initial conditions for
> database conditional monitoring.
>
>  ovn/controller/binding.c|  11 +++--
>  ovn/controller/ovn-controller.c | 103 ++
> ++
>  ovn/controller/ovn-controller.h |   5 ++
>  ovn/controller/patch.c  |   7 ++-
>  ovn/northd/ovn-northd.c |  76 +
>  ovn/ovn-sb.ovsschema|  11 +++--
>  ovn/ovn-sb.xml  |   9 
>  tests/ovn.at|   8 
>  8 files changed, 221 insertions(+), 9 deletions(-)
>
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index 13c51da..0561b6c 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -84,7 +84,8 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
>
>  static void
>  add_local_datapath(struct hmap *local_datapaths,
> -const struct sbrec_port_binding *binding_rec)
> +const struct sbrec_port_binding *binding_rec,
> +const struct controller_ctx *ctx)
>  {
>  if (get_local_datapath(local_datapaths,
> binding_rec->datapath->tunnel_key)) {
> @@ -96,6 +97,8 @@ add_local_datapath(struct hmap *local_datapaths,
>  memcpy(>uuid, _rec->header_.uuid, sizeof ld->uuid);
>  hmap_insert(local_datapaths, >hmap_node,
>  binding_rec->datapath->tunnel_key);
> +do_datapath_flood_fill(ctx, binding_rec->datapath,
> +   local_datapaths, NULL);
>  }
>
>  static void
> @@ -127,7 +130,7 @@ consider_local_datapath(struct controller_ctx *ctx,
>  /* Add child logical port to the set of all local ports. */
>  sset_add(all_lports, binding_rec->logical_port);
>  }
> -add_local_datapath(local_datapaths, binding_rec);
> +add_local_datapath(local_datapaths, binding_rec, ctx);
>  if (iface_rec && ctx->ovs_idl_txn) {
>  update_qos(iface_rec, binding_rec);
>  }
> @@ -162,7 +165,7 @@ consider_local_datapath(struct controller_ctx *ctx,
>  }
>
>  sset_add(all_lports, binding_rec->logical_port);
> -add_local_datapath(local_datapaths, binding_rec);
> +add_local_datapath(local_datapaths, binding_rec, ctx);
>  if (binding_rec->chassis == chassis_rec) {
>  return;
>  }
> @@ -176,7 +179,7 @@ consider_local_datapath(struct controller_ctx *ctx,
>  const char *chassis = smap_get(_rec->options,
> "l3gateway-chassis");
>  if (!strcmp(chassis, chassis_rec->name) && ctx->ovnsb_idl_txn) {
> -add_local_datapath(local_datapaths, binding_rec);
> +add_local_datapath(local_datapaths, binding_rec, ctx);
>  }
>  } else if (chassis_rec && binding_rec->chassis == chassis_rec) {
>  if (ctx->ovnsb_idl_txn) {
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-
> controller.c
> index 00392ca..02bafa0 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -69,6 +69,14 @@ OVS_NO_RETURN static void usage(void);
>
>  static char *ovs_remote;
>
>

[ovs-dev] [patch_v5] ovn: Add datapaths of interest filtering.

2016-10-04 Thread Darrell Ball 
This patch adds datapaths of interest support where only datapaths of
local interest are monitored by the ovn-controller ovsdb client.  The
idea is to do a flood fill in ovn-controller of datapath associations
calculated by northd. A new column is added to the SB database
datapath_binding table - related_datapaths to facilitate this so all
datapaths associations are known quickly in ovn-controller.  This
allows monitoring to adapt quickly with a single new monitor setting
for all datapaths of interest locally.

To reduce risk and minimize latency on network churn, only logical
flows are conditionally monitored for now.  This should provide
the bulk of the benefit.  This will be re-evaluated later to
possibly add additional conditional monitoring such as for
logical ports.

Liran Schour contributed enhancements to the conditional
monitoring granularity in ovs and also submitted patches
for ovn usage of conditional monitoring which aided this patch
though sharing of concepts through code review work.

Ben Pfaff suggested that northd could be used to pre-populate
related datapaths for ovn-controller to use.  That idea is
used as part of this patch.

CC: Ben Pfaff 
CC: Liran Schour 
Signed-off-by: Darrell Ball 
---

v4->v5: Correct cleanup of monitors.
Fix warning.

v3->v4: Refactor after incremental processing backout.
Limit filtering to logical flows to limit risk.

v2->v3: Line length violation fixups :/

v1->v2: Added logical port removal monitoring handling, factoring
in recent changes for incremental processing.  Added some
comments in the code regarding initial conditions for
database conditional monitoring.

 ovn/controller/binding.c|  11 +++--
 ovn/controller/ovn-controller.c | 103 
 ovn/controller/ovn-controller.h |   5 ++
 ovn/controller/patch.c  |   7 ++-
 ovn/northd/ovn-northd.c |  76 +
 ovn/ovn-sb.ovsschema|  11 +++--
 ovn/ovn-sb.xml  |   9 
 tests/ovn.at|   8 
 8 files changed, 221 insertions(+), 9 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 13c51da..0561b6c 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -84,7 +84,8 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
 
 static void
 add_local_datapath(struct hmap *local_datapaths,
-const struct sbrec_port_binding *binding_rec)
+const struct sbrec_port_binding *binding_rec,
+const struct controller_ctx *ctx)
 {
 if (get_local_datapath(local_datapaths,
binding_rec->datapath->tunnel_key)) {
@@ -96,6 +97,8 @@ add_local_datapath(struct hmap *local_datapaths,
 memcpy(>uuid, _rec->header_.uuid, sizeof ld->uuid);
 hmap_insert(local_datapaths, >hmap_node,
 binding_rec->datapath->tunnel_key);
+do_datapath_flood_fill(ctx, binding_rec->datapath,
+   local_datapaths, NULL);
 }
 
 static void
@@ -127,7 +130,7 @@ consider_local_datapath(struct controller_ctx *ctx,
 /* Add child logical port to the set of all local ports. */
 sset_add(all_lports, binding_rec->logical_port);
 }
-add_local_datapath(local_datapaths, binding_rec);
+add_local_datapath(local_datapaths, binding_rec, ctx);
 if (iface_rec && ctx->ovs_idl_txn) {
 update_qos(iface_rec, binding_rec);
 }
@@ -162,7 +165,7 @@ consider_local_datapath(struct controller_ctx *ctx,
 }
 
 sset_add(all_lports, binding_rec->logical_port);
-add_local_datapath(local_datapaths, binding_rec);
+add_local_datapath(local_datapaths, binding_rec, ctx);
 if (binding_rec->chassis == chassis_rec) {
 return;
 }
@@ -176,7 +179,7 @@ consider_local_datapath(struct controller_ctx *ctx,
 const char *chassis = smap_get(_rec->options,
"l3gateway-chassis");
 if (!strcmp(chassis, chassis_rec->name) && ctx->ovnsb_idl_txn) {
-add_local_datapath(local_datapaths, binding_rec);
+add_local_datapath(local_datapaths, binding_rec, ctx);
 }
 } else if (chassis_rec && binding_rec->chassis == chassis_rec) {
 if (ctx->ovnsb_idl_txn) {
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 00392ca..02bafa0 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -69,6 +69,14 @@ OVS_NO_RETURN static void usage(void);
 
 static char *ovs_remote;
 
+struct dp_of_interest_node {
+struct hmap_node hmap_node;
+const struct sbrec_datapath_binding *sb_db;
+bool stale;
+};
+
+static struct hmap dps_of_interest = HMAP_INITIALIZER(_of_interest);
+
 struct local_datapath *
 get_local_datapath(const struct hmap *local_datapaths, uint32_t tunnel_key)