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

Darrell Ball dlu998 at gmail.com
Fri Dec 2 04:07:47 UTC 2016


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.

We monitor both logical flows and logical ports. The mac bindings
will likely go away and multicast groups are relatively few.
To reduce risk and overall latency we only suppress logical flows as
a starting point. Logical ports are not suppressed as a starting
point and but then are monitor suppressed as appropriate from that
point on. The initial suppression has little practical gain and not
doing it allows for us to be flexible in terms of how different port
types are supported.

To validate the gain, we take the approach to verify the l3gateway
router flow suppression from the non-l3gateway HVs POV and also add
a separate test to verify the flow and port handling scale advantages.
The separate test added verifies the logical port and logical flow
suppression advantage. This measures the difference in flows and ports
sent to the HV clients where the far majority of the overall work is.
The less ports and flows that are unnecessarily processed on the HVs,
the more benefit, excluding latency skew. The separate test is
simple for interpretation and shows an order of magnitude advantage;
the test uses only logical switches. The advantage would increase with
logical routers. The test is conservative in estimating the numerical
advantage to increase the probability of the check passing the first
time, since there are some timing conssiderations that affect the numbers.

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 <blp at ovn.org>
CC: Liran Schour <LIRANS at il.ibm.com>
Signed-off-by: Darrell Ball <dlu998 at gmail.com>
---

v8->v9: Reinstate logical port monitoring from patch V3
        now to handle some LP high influence topologies, especially
        in lieu of a lack of incremental processing right now.
        The original plan was to forgo monitoring logical ports
        in the first commit, but without incremental processing,
        port processing gains importance.

        Parse out some functions

        Add a separate test for conditional monitoring to
        guesstimate the reduction in logical port and flow events
        seen at the HVs.

v7->v8: Start wth logical flow conditional monitoring off.
        Remove deprecated code.
        Recover SB DB version number change.
        Change test to be more definitive. 

v6->v7: Rebase

v5->v6: Rebase; fix stale handling.

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        |  24 +++++--
 ovn/controller/lflow.c          |   5 ++
 ovn/controller/ovn-controller.c | 128 +++++++++++++++++++++++++++++++++-
 ovn/controller/ovn-controller.h |   5 ++
 ovn/controller/patch.c          |  34 ++++++++-
 ovn/northd/ovn-northd.c         |  76 ++++++++++++++++++++
 ovn/ovn-sb.ovsschema            |  11 ++-
 ovn/ovn-sb.xml                  |   9 +++
 tests/ovn.at                    | 149 ++++++++++++++++++++++++++++++++++++++++
 9 files changed, 427 insertions(+), 14 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index d7b175e..169b3b6 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -68,7 +68,8 @@ static void
 get_local_iface_ids(const struct ovsrec_bridge *br_int,
                     struct shash *lport_to_iface,
                     struct sset *all_lports,
-                    struct sset *egress_ifaces)
+                    struct sset *egress_ifaces,
+                    const struct controller_ctx *ctx)
 {
     int i;
 
@@ -88,6 +89,9 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
             iface_id = smap_get(&iface_rec->external_ids, "iface-id");
 
             if (iface_id) {
+                sbrec_port_binding_add_clause_logical_port(ctx->ovnsb_idl,
+                                                           OVSDB_F_EQ,
+                                                           iface_id);
                 shash_add(lport_to_iface, iface_id, iface_rec);
                 sset_add(all_lports, iface_id);
             }
@@ -106,7 +110,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)) {
@@ -115,9 +120,11 @@ add_local_datapath(struct hmap *local_datapaths,
 
     struct local_datapath *ld = xzalloc(sizeof *ld);
     ld->logical_port = xstrdup(binding_rec->logical_port);
+    ld->dp = binding_rec->datapath;
     memcpy(&ld->uuid, &binding_rec->header_.uuid, sizeof ld->uuid);
     hmap_insert(local_datapaths, &ld->hmap_node,
                 binding_rec->datapath->tunnel_key);
+    do_datapath_flood_fill(ctx, binding_rec->datapath);
 }
 
 static void
@@ -288,6 +295,9 @@ consider_local_datapath(struct controller_ctx *ctx,
     const struct ovsrec_interface *iface_rec
         = shash_find_data(lport_to_iface, binding_rec->logical_port);
 
+    VLOG_DBG("consider local datapath: lp %s tunnel_key %lu",
+             binding_rec->logical_port, binding_rec->datapath->tunnel_key);
+
     if (iface_rec
         || (binding_rec->parent_port && binding_rec->parent_port[0] &&
             sset_contains(all_lports, binding_rec->parent_port))) {
@@ -295,7 +305,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 && qos_map && ctx->ovs_idl_txn) {
             get_qos_params(binding_rec, qos_map);
         }
@@ -330,7 +340,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;
         }
@@ -344,7 +354,7 @@ consider_local_datapath(struct controller_ctx *ctx,
         const char *chassis = smap_get(&binding_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) {
@@ -353,6 +363,8 @@ consider_local_datapath(struct controller_ctx *ctx,
             for (int i = 0; i < binding_rec->n_mac; i++) {
                 VLOG_INFO("Releasing %s", binding_rec->mac[i]);
             }
+            sbrec_port_binding_remove_clause_logical_port(
+                ctx->ovnsb_idl, OVSDB_F_EQ, binding_rec->logical_port);
             sbrec_port_binding_set_chassis(binding_rec, NULL);
             sset_find_and_delete(all_lports, binding_rec->logical_port);
         }
@@ -383,7 +395,7 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
     hmap_init(&qos_map);
     if (br_int) {
         get_local_iface_ids(br_int, &lport_to_iface, all_lports,
-                            &egress_ifaces);
+                            &egress_ifaces, ctx);
     }
 
     /* Run through each binding record to see if it is resident on this
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 4e67365..765eae4 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -167,6 +167,11 @@ consider_logical_flow(const struct lport_index *lports,
     if (!ldp) {
         return;
     }
+
+    VLOG_DBG("consider logical flow: tunnel_key %lu "
+             "\"match\" \"%s\", \"actions\" \"%s\"",
+             ldp->tunnel_key, lflow->match, lflow->actions);
+
     if (is_switch(ldp)) {
         /* For a logical switch datapath, local_datapaths tells us if there
          * are any local ports for this datapath.  If not, we can skip
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 52eb491..93df876 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -69,6 +69,17 @@ 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;
+};
+
+struct hmap local_datapaths;
+struct hmap patched_datapaths;
+
+static struct hmap dps_of_interest = HMAP_INITIALIZER(&dps_of_interest);
+
 struct local_datapath *
 get_local_datapath(const struct hmap *local_datapaths, uint32_t tunnel_key)
 {
@@ -128,6 +139,96 @@ get_bridge(struct ovsdb_idl *ovs_idl, const char *br_name)
     return NULL;
 }
 
+static bool
+add_datapath_of_interest(const struct controller_ctx *ctx,
+                         const struct sbrec_datapath_binding *dp)
+{
+    struct dp_of_interest_node *node;
+
+    HMAP_FOR_EACH_WITH_HASH (node, hmap_node, uuid_hash(&dp->header_.uuid),
+                             &dps_of_interest) {
+        if (uuid_equals(&node->sb_db->header_.uuid, &dp->header_.uuid)) {
+            node->stale = false;
+            return false;
+        }
+    }
+
+    sbrec_logical_flow_add_clause_logical_datapath(ctx->ovnsb_idl,
+                                                   OVSDB_F_EQ,
+                                                   &dp->header_.uuid);
+    sbrec_port_binding_add_clause_datapath(ctx->ovnsb_idl, OVSDB_F_EQ,
+                                           &dp->header_.uuid);
+
+    node = xzalloc(sizeof *node);
+    hmap_insert(&dps_of_interest, &node->hmap_node,
+                uuid_hash(&dp->header_.uuid));
+    node->sb_db = dp;
+    return true;
+}
+
+void
+do_datapath_flood_fill(const struct controller_ctx *ctx,
+                       const struct sbrec_datapath_binding *dp_start)
+{
+    struct interest_dps_list_elem {
+        struct ovs_list list_node;
+        const struct sbrec_datapath_binding * dp;
+    };
+
+    struct ovs_list interest_datapaths;
+    ovs_list_init(&interest_datapaths);
+
+    struct interest_dps_list_elem *dp_list_elem =
+        xzalloc(sizeof *dp_list_elem);
+    dp_list_elem->dp = dp_start;
+    ovs_list_push_back(&interest_datapaths, &dp_list_elem->list_node);
+
+    while (!ovs_list_is_empty(&interest_datapaths)) {
+
+        struct ovs_list *list_node_ptr =
+            ovs_list_pop_front(&interest_datapaths);
+        dp_list_elem = CONTAINER_OF(list_node_ptr,
+            struct interest_dps_list_elem, list_node);
+
+	    size_t num_related_datapaths =
+            dp_list_elem->dp->n_related_datapaths;
+	    struct sbrec_datapath_binding **related_datapaths =
+            dp_list_elem->dp->related_datapaths;
+        if (!add_datapath_of_interest(ctx, dp_list_elem->dp)) {
+            free(dp_list_elem);
+            continue;
+        }
+        free(dp_list_elem);
+        for (int i = 0; i < num_related_datapaths; i++) {
+            dp_list_elem = xzalloc(sizeof *dp_list_elem);
+            dp_list_elem->dp = related_datapaths[i];
+            ovs_list_push_back(&interest_datapaths, &dp_list_elem->list_node);
+        }
+    }
+}
+
+static void
+remove_datapath_of_interest(struct controller_ctx *ctx,
+                            const struct sbrec_datapath_binding *dp)
+{
+
+    struct dp_of_interest_node *node;
+    HMAP_FOR_EACH_WITH_HASH (node, hmap_node, uuid_hash(&dp->header_.uuid),
+                             &dps_of_interest) {
+        if (uuid_equals(&node->sb_db->header_.uuid, &dp->header_.uuid)) {
+
+            sbrec_port_binding_remove_clause_datapath(ctx->ovnsb_idl,
+                                                      OVSDB_F_EQ,
+                                                      &dp->header_.uuid);
+            sbrec_logical_flow_remove_clause_logical_datapath(
+                ctx->ovnsb_idl, OVSDB_F_EQ, &dp->header_.uuid);
+            hmap_remove(&dps_of_interest, &node->hmap_node);
+            free(node);
+            return;
+        }
+    }
+}
+
 static const struct ovsrec_bridge *
 create_br_int(struct controller_ctx *ctx,
               const struct ovsrec_open_vswitch *cfg,
@@ -465,6 +566,10 @@ main(int argc, char *argv[])
         ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class, true, true));
     ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, &sbrec_chassis_col_nb_cfg);
 
+    /* Start with suppressing all logical flows and then later request those
+     * that are specifically needed. */
+    sbrec_logical_flow_add_clause_false(ovnsb_idl_loop.idl);
+
     /* Track the southbound idl. */
     ovsdb_idl_track_add_all(ovnsb_idl_loop.idl);
 
@@ -504,11 +609,20 @@ main(int argc, char *argv[])
 
         /* Contains "struct local_datapath" nodes whose hash values are the
          * tunnel_key of datapaths with at least one local port binding. */
-        struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths);
+        hmap_init(&local_datapaths);
+        hmap_init(&patched_datapaths);
+
 
-        struct hmap patched_datapaths = HMAP_INITIALIZER(&patched_datapaths);
         struct sset all_lports = SSET_INITIALIZER(&all_lports);
 
+        struct dp_of_interest_node *dp_cur_node, *dp_next_node;
+        HMAP_FOR_EACH_SAFE (dp_cur_node, dp_next_node, hmap_node,
+                            &dps_of_interest) {
+            if (ctx.ovs_idl_txn) {
+                dp_cur_node->stale = true;
+            }
+        }
+
         const struct ovsrec_bridge *br_int = get_br_int(&ctx);
         const char *chassis_id = get_chassis_id(ctx.ovs_idl);
 
@@ -559,6 +673,14 @@ main(int argc, char *argv[])
             }
             mcgroup_index_destroy(&mcgroups);
             lport_index_destroy(&lports);
+
+            HMAP_FOR_EACH_SAFE (dp_cur_node, dp_next_node, hmap_node,
+                                &dps_of_interest) {
+                if(dp_cur_node->stale == true) {
+                    remove_datapath_of_interest(&ctx, dp_cur_node->sb_db);
+                }
+            }
+
         }
 
         sset_destroy(&all_lports);
@@ -573,7 +695,7 @@ main(int argc, char *argv[])
 
         struct patched_datapath *pd_cur_node, *pd_next_node;
         HMAP_FOR_EACH_SAFE (pd_cur_node, pd_next_node, hmap_node,
-                &patched_datapaths) {
+                            &patched_datapaths) {
             hmap_remove(&patched_datapaths, &pd_cur_node->hmap_node);
             free(pd_cur_node);
         }
diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h
index 78c8b08..ec4d4d0 100644
--- a/ovn/controller/ovn-controller.h
+++ b/ovn/controller/ovn-controller.h
@@ -57,6 +57,7 @@ struct local_datapath {
     struct uuid uuid;
     char *logical_port;
     const struct sbrec_port_binding *localnet_port;
+    const struct sbrec_datapath_binding *dp;
 };
 
 struct local_datapath *get_local_datapath(const struct hmap *,
@@ -67,6 +68,7 @@ struct local_datapath *get_local_datapath(const struct hmap *,
 struct patched_datapath {
     struct hmap_node hmap_node;
     struct uuid key;   /* UUID of the corresponding datapath. */
+    const struct sbrec_datapath_binding *dp;
     bool local; /* 'True' if the datapath is for gateway router. */
 };
 
@@ -79,6 +81,9 @@ const struct ovsrec_bridge *get_bridge(struct ovsdb_idl *,
 const struct sbrec_chassis *get_chassis(struct ovsdb_idl *,
                                         const char *chassis_id);
 
+void do_datapath_flood_fill(const struct controller_ctx *,
+                            const struct sbrec_datapath_binding *);
+
 /* Must be a bit-field ordered from most-preferred (higher number) to
  * least-preferred (lower number). */
 enum chassis_tunnel_type {
diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
index 79a7d81..d8f5c65 100644
--- a/ovn/controller/patch.c
+++ b/ovn/controller/patch.c
@@ -252,7 +252,8 @@ add_bridge_mappings(struct controller_ctx *ctx,
 
 static void
 add_patched_datapath(struct hmap *patched_datapaths,
-                     const struct sbrec_port_binding *binding_rec, bool local)
+                     const struct sbrec_port_binding *binding_rec, bool local,
+                     struct controller_ctx *ctx)
 {
     struct patched_datapath *pd = get_patched_datapath(patched_datapaths,
                                        binding_rec->datapath->tunnel_key);
@@ -263,8 +264,10 @@ add_patched_datapath(struct hmap *patched_datapaths,
     pd = xzalloc(sizeof *pd);
     pd->local = local;
     pd->key = binding_rec->datapath->header_.uuid;
+    pd->dp = binding_rec->datapath;
     hmap_insert(patched_datapaths, &pd->hmap_node,
                 binding_rec->datapath->tunnel_key);
+    do_datapath_flood_fill(ctx, binding_rec->datapath);
 }
 
 /* Add one OVS patch port for each OVN logical patch port.
@@ -318,6 +321,10 @@ add_logical_patch_ports(struct controller_ctx *ctx,
         if (!strcmp(binding->type, "patch") || local_port) {
             const char *local = binding->logical_port;
             const char *peer = smap_get(&binding->options, "peer");
+
+            VLOG_DBG("consider patched datapath: lp %s tunnel_key %lu",
+                     binding->logical_port, binding->datapath->tunnel_key);
+
             if (!peer) {
                 continue;
             }
@@ -329,7 +336,9 @@ add_logical_patch_ports(struct controller_ctx *ctx,
                               existing_ports);
             free(dst_name);
             free(src_name);
-            add_patched_datapath(patched_datapaths, binding, local_port);
+            add_patched_datapath(patched_datapaths, binding, local_port, ctx);
+            sbrec_port_binding_add_clause_logical_port(ctx->ovnsb_idl,
+                                                       OVSDB_F_EQ, peer);
             if (local_port) {
                 if (binding->chassis != chassis_rec && ctx->ovnsb_idl_txn) {
                     sbrec_port_binding_set_chassis(binding, chassis_rec);
@@ -339,6 +348,26 @@ add_logical_patch_ports(struct controller_ctx *ctx,
     }
 }
 
+/* Non-logical patch ports should be unaffected. */
+static void
+remove_patch_port_monitors(struct controller_ctx *ctx,
+                            const struct ovsrec_port *port)
+{
+    for (size_t i = 0; i < port->n_interfaces; i++) {
+        struct ovsrec_interface *iface = port->interfaces[i];
+        if (strcmp(iface->type, "patch")) {
+            continue;
+        }
+        const char *iface_id = smap_get(&iface->external_ids,
+                                        "iface-id");
+        if (iface_id) {
+            sbrec_port_binding_remove_clause_logical_port(ctx->ovnsb_idl,
+                                                          OVSDB_F_EQ,
+                                                          iface_id);
+        }
+    }
+}
+
 void
 patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
           const char *chassis_id, struct hmap *local_datapaths,
@@ -374,6 +403,7 @@ patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
     SHASH_FOR_EACH_SAFE (port_node, port_next_node, &existing_ports) {
         struct ovsrec_port *port = port_node->data;
         shash_delete(&existing_ports, port_node);
+        remove_patch_port_monitors(ctx, port);
         remove_port(ctx, port);
     }
     shash_destroy(&existing_ports);
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index f2dc353..ca792aa 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -231,6 +231,42 @@ struct tnlid_node {
     uint32_t tnlid;
 };
 
+struct related_datapath_node {
+    struct hmap_node hmap_node;
+    const struct sbrec_datapath_binding *sb_db;
+};
+
+static void
+add_related_datapath(struct hmap *related_datapaths,
+                     const struct sbrec_datapath_binding *sb,
+                     size_t *n_related_datapaths)
+{
+    struct related_datapath_node *node;
+    HMAP_FOR_EACH_WITH_HASH (node, hmap_node,
+                             uuid_hash(&sb->header_.uuid),
+                             related_datapaths) {
+        if (uuid_equals(&sb->header_.uuid, &node->sb_db->header_.uuid)) {
+            return;
+        }
+    }
+
+    node = xzalloc(sizeof *node);
+    hmap_insert(related_datapaths, &node->hmap_node,
+                uuid_hash(&sb->header_.uuid));
+    node->sb_db = sb;
+    (*n_related_datapaths)++;
+}
+
+static void
+destroy_related_datapaths(struct hmap *related_datapaths)
+{
+    struct related_datapath_node *node;
+    HMAP_FOR_EACH_POP (node, hmap_node, related_datapaths) {
+        free(node);
+    }
+    hmap_destroy(related_datapaths);
+}
+
 static void
 destroy_tnlids(struct hmap *tnlids)
 {
@@ -376,6 +412,8 @@ struct ovn_datapath {
     size_t n_router_ports;
 
     struct hmap port_tnlids;
+    struct hmap related_datapaths;
+    size_t n_related_datapaths;
     uint32_t port_key_hint;
 
     bool has_unknown;
@@ -426,6 +464,7 @@ ovn_datapath_create(struct hmap *datapaths, const struct uuid *key,
     od->nbr = nbr;
     hmap_init(&od->port_tnlids);
     hmap_init(&od->ipam);
+    hmap_init(&od->related_datapaths);
     od->port_key_hint = 0;
     hmap_insert(datapaths, &od->key_node, uuid_hash(&od->key));
     return od;
@@ -441,6 +480,7 @@ ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od)
         hmap_remove(datapaths, &od->key_node);
         destroy_tnlids(&od->port_tnlids);
         destroy_ipam(&od->ipam);
+        destroy_related_datapaths(&od->related_datapaths);
         free(od->router_ports);
         free(od);
     }
@@ -624,6 +664,28 @@ build_datapaths(struct northd_context *ctx, struct hmap *datapaths)
             smap_destroy(&ids);
 
             sbrec_datapath_binding_set_tunnel_key(od->sb, tunnel_key);
+
+            struct sbrec_datapath_binding **sb_related_datapaths
+                = xmalloc(sizeof(*sb_related_datapaths) * od->n_related_datapaths);
+            int rdi = 0;
+            struct related_datapath_node *related_datapath;
+            HMAP_FOR_EACH (related_datapath, hmap_node,
+                           &od->related_datapaths) {
+                if (rdi >= od->n_related_datapaths) {
+                    static struct vlog_rate_limit rl
+                        = VLOG_RATE_LIMIT_INIT(5, 1);
+                    VLOG_ERR_RL(&rl, "related datapaths accounting error"
+                                UUID_FMT, UUID_ARGS(&od->key));
+                    break;
+                }
+                sb_related_datapaths[rdi] = CONST_CAST(
+                    struct sbrec_datapath_binding *, related_datapath->sb_db);
+                rdi++;
+            }
+            sbrec_datapath_binding_set_related_datapaths(od->sb,
+                sb_related_datapaths, od->n_related_datapaths);
+            free(sb_related_datapaths);
+
         }
         destroy_tnlids(&dp_tnlids);
     }
@@ -1359,6 +1421,12 @@ ovn_port_update_sbrec(const struct ovn_port *op,
             sbrec_port_binding_set_type(op->sb, "patch");
         }
 
+        if (op->peer && op->peer->od && op->peer->od->sb) {
+            add_related_datapath(&op->od->related_datapaths,
+                                 op->peer->od->sb,
+                                 &op->od->n_related_datapaths);
+        }
+
         const char *peer = op->peer ? op->peer->key : "<error>";
         struct smap new;
         smap_init(&new);
@@ -1411,6 +1479,12 @@ ovn_port_update_sbrec(const struct ovn_port *op,
                 sbrec_port_binding_set_type(op->sb, "patch");
             }
 
+            if (op->peer && op->peer->od && op->peer->od->sb) {
+                add_related_datapath(&op->od->related_datapaths,
+                                     op->peer->od->sb,
+                                     &op->od->n_related_datapaths);
+            }
+
             const char *router_port = smap_get_def(&op->nbsp->options,
                                                    "router-port", "<error>");
             struct smap new;
@@ -4952,6 +5026,8 @@ main(int argc, char *argv[])
     add_column_noalert(ovnsb_idl_loop.idl,
                        &sbrec_datapath_binding_col_tunnel_key);
     add_column_noalert(ovnsb_idl_loop.idl,
+                       &sbrec_datapath_binding_col_related_datapaths);
+    add_column_noalert(ovnsb_idl_loop.idl,
                        &sbrec_datapath_binding_col_external_ids);
 
     ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_port_binding);
diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema
index 89342fe..51c222d 100644
--- a/ovn/ovn-sb.ovsschema
+++ b/ovn/ovn-sb.ovsschema
@@ -1,7 +1,7 @@
 {
     "name": "OVN_Southbound",
-    "version": "1.9.0",
-    "cksum": "239060528 9012",
+    "version": "1.10.0",
+    "cksum": "1139190282 9320",
     "tables": {
         "SB_Global": {
             "columns": {
@@ -93,7 +93,12 @@
                                       "maxInteger": 16777215}}},
                 "external_ids": {
                     "type": {"key": "string", "value": "string",
-                             "min": 0, "max": "unlimited"}}},
+                             "min": 0, "max": "unlimited"}},
+                "related_datapaths": {"type": {"key": {"type": "uuid",
+                                      "refTable": "Datapath_Binding",
+                                      "refType": "weak"},
+                                      "min": 0,
+                                      "max": "unlimited"}}},
             "indexes": [["tunnel_key"]],
             "isRoot": true},
         "Port_Binding": {
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index 65191ed..c20861f 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1556,6 +1556,15 @@ tcp.flags = RST;
       constructed for each supported encapsulation.
     </column>
 
+    <column name="related_datapaths">
+      The related_datapaths column is used to keep track of which datapaths
+      are connected to each other.  This is leveraged in ovn-controller to
+      do a flood fill from each local logical switch to determine the full
+      set of datapaths needed on a given ovn-controller.   This set of
+      datapaths can be used to determine which logical ports and logical
+      flows an ovn-controller should monitor.
+    </column>
+
     <group title="OVN_Northbound Relationship">
       <p>
         Each row in <ref table="Datapath_Binding"/> is associated with some
diff --git a/tests/ovn.at b/tests/ovn.at
index a226849..bf36808 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1673,6 +1673,147 @@ OVN_CLEANUP([hv1],[hv2])
 
 AT_CLEANUP
 
+# 2 hypervisors, each with one logical switch with 2 logical ports.
+AT_SETUP([ovn -- 2 HVs, 2 LSs; conditional monitoring])
+AT_SKIP_IF([test $HAVE_PYTHON = no])
+ovn_start
+
+# In this test cases we create 2 switches, all connected to same
+# physical network (through br-phys on each HV). Each switch has
+# VIF ports across 1 HV. Each HV has 2 VIF ports. The first digit
+# of VIF port name indicates the hypervisor it is bound to, e.g.
+# lp12 means VIF 2 on hv1.
+
+for i in 1 2; do
+    ls_name=ls$i
+    ovn-nbctl ls-add $ls_name
+done
+
+net_add n1
+for i in 1 2; do
+    sim_add hv$i
+    as hv$i
+    ovs-vsctl add-br br-phys
+    ovn_attach n1 br-phys 192.168.0.$i
+    ovs-appctl -t ovn-controller vlog/set ANY:file:DBG
+
+    for j in 1 2; do
+        ovs-vsctl add-port br-int vif$i$j -- \
+            set Interface vif$i$j external-ids:iface-id=lp$i$j \
+                                  options:tx_pcap=hv$i/vif$i$j-tx.pcap \
+                                  options:rxq_pcap=hv$i/vif$i$j-rx.pcap \
+                                  ofport-request=$i$j
+
+        lsp_name=lp$i$j
+        if test $i -eq 1; then
+            ls_name=ls1
+        else
+            ls_name=ls2
+        fi
+
+        ovn-nbctl lsp-add $ls_name $lsp_name
+        ovn-nbctl lsp-set-addresses $lsp_name f0:00:00:00:00:$i$j
+        ovn-nbctl lsp-set-port-security $lsp_name f0:00:00:00:00:$i$j
+
+        OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up $lsp_name` = xup])
+    done
+done
+
+ovn_populate_arp
+
+# XXX This is now the 3rd copy of these functions in this file ...
+
+# Given the name of a logical port, prints the name of the hypervisor
+# on which it is located.
+vif_to_hv() {
+    echo hv${1%?}
+}
+#
+# test_packet INPORT DST SRC ETHTYPE OUTPORT...
+#
+# This shell function causes a packet to be received on INPORT.  The packet's
+# content has Ethernet destination DST and source SRC (each exactly 12 hex
+# digits) and Ethernet type ETHTYPE (4 hex digits).  The OUTPORTs (zero or
+# more) list the VIFs on which the packet should be received.  INPORT and the
+# OUTPORTs are specified as logical switch port numbers, e.g. 11 for vif11.
+for i in 1 2; do
+    for j in 1 2; do
+        : > $i$j.expected
+    done
+done
+test_packet() {
+    local inport=$1 src=$2 dst=$3 eth=$4; shift; shift; shift; shift
+    local packet=${src}${dst}${eth}
+    hv=`vif_to_hv $inport`
+    vif=vif$inport
+    as $hv ovs-appctl netdev-dummy/receive $vif $packet
+    for outport; do
+        echo $packet >> $outport.expected
+    done
+}
+
+# lp11 and lp12 are on the same network and on
+# same hypervisors
+test_packet 11 f00000000012 f00000000011 1112 12
+test_packet 12 f00000000011 f00000000012 1211 11
+
+# lp21 and lp22 are on the same network and on
+# the same hypervisor
+test_packet 21 f00000000022 f00000000021 2122 22
+test_packet 22 f00000000021 f00000000022 2221 21
+
+# lp11 and lp22 are on different networks so should
+# not be able to communicate.
+test_packet 11 f00000000022 f00000000011 1122
+test_packet 22 f00000000011 f00000000022 2211
+
+echo "------ OVN dump ------"
+ovn-nbctl show
+ovn-sbctl show
+
+echo "------ hv1 dump ------"
+as hv1 ovs-vsctl show
+as hv1 ovs-ofctl -O OpenFlow13 dump-flows br-int
+
+echo "------ hv2 dump ------"
+as hv2 ovs-vsctl show
+as hv2 ovs-ofctl -O OpenFlow13 dump-flows br-int
+
+dbg_instances1=`cat hv1/ovn-controller.log | grep 'lp.*tunnel_key 1' | wc -l`
+echo LS1 LPs on HV1 $dbg_instances1
+dbg_instances2=`cat hv1/ovn-controller.log | grep 'lp.*tunnel_key 2' | wc -l`
+echo LS2 LPs on HV1 $dbg_instances2
+AT_CHECK([test $dbg_instances1 -gt $dbg_instances2], [0], [ignore-nolog])
+
+dbg_instances1=`cat hv2/ovn-controller.log | grep 'lp.*tunnel_key 2' | wc -l`
+echo LS2 LPs on HV2 $dbg_instances1
+dbg_instances2=`cat hv2/ovn-controller.log | grep 'lp.*tunnel_key 1' | wc -l`
+echo LS1 LPs on HV2 $dbg_instances2
+AT_CHECK([test $dbg_instances1 -gt $dbg_instances2], [0], [ignore-nolog])
+
+dbg_instances1=`cat hv1/ovn-controller.log | grep 'tunnel_key 1' | wc -l`
+echo LS1 LPs and LFs on HV1 $dbg_instances1
+dbg_instances2=`cat hv1/ovn-controller.log | grep 'tunnel_key 2' | wc -l`
+echo LS2 LPs and LFs on HV1 $dbg_instances2
+AT_CHECK([test $dbg_instances1 -gt $dbg_instances2], [0], [ignore-nolog])
+
+dbg_instances1=`cat hv2/ovn-controller.log | grep 'tunnel_key 2' | wc -l`
+echo LS2 LPs and LFs on HV2 $dbg_instances1
+dbg_instances2=`cat hv2/ovn-controller.log | grep 'tunnel_key 1' | wc -l`
+echo LS1 LPs and LFs on HV2 $dbg_instances2
+AT_CHECK([test $dbg_instances1 -gt $dbg_instances2], [0], [ignore-nolog])
+
+# Now check the packets actually received against the ones expected.
+for i in 1 2; do
+    for j in 1 2; do
+        OVN_CHECK_PACKETS([hv$i/vif$i$j-tx.pcap], [$i$j.expected])
+    done
+done
+
+OVN_CLEANUP([hv1],[hv2])
+
+AT_CLEANUP
+
 AT_SETUP([ovn -- vtep: 3 HVs, 1 VIFs/HV, 1 GW, 1 LS])
 AT_KEYWORDS([vtep])
 AT_SKIP_IF([test $HAVE_PYTHON = no])
@@ -4109,6 +4250,7 @@ ovs-vsctl -- add-port br-int hv1-vif1 -- \
     options:tx_pcap=hv1/vif1-tx.pcap \
     options:rxq_pcap=hv1/vif1-rx.pcap \
     ofport-request=1
+ovs-appctl -t ovn-controller vlog/set ANY:file:DBG
 
 
 sim_add hv2
@@ -4120,6 +4262,7 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \
     options:tx_pcap=hv2/vif1-tx.pcap \
     options:rxq_pcap=hv2/vif1-rx.pcap \
     ofport-request=1
+ovs-appctl -t ovn-controller vlog/set ANY:file:DBG
 
 # Pre-populate the hypervisors' ARP tables so that we don't lose any
 # packets for ARP resolution (native tunneling doesn't queue packets
@@ -4225,6 +4368,12 @@ as hv2 ovs-ofctl show br-int
 as hv2 ovs-ofctl dump-flows br-int
 echo "----------------------------"
 
+# tunnel key 2 represents the gateway router and the associated
+# logical flows should only be on hv2 not hv1 when conditional
+# monitoring of flows is being used.
+AT_CHECK([cat hv2/ovn-controller.log | grep 'tunnel_key 2'], [0], [ignore-nolog])
+AT_CHECK([cat hv1/ovn-controller.log | grep 'tunnel_key 2'], [1], [ignore-nolog])
+
 echo $expected > expected
 OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected])
 
-- 
1.9.1



More information about the dev mailing list