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.
Signed-off-by: Darrell Ball <dlu...@gmail.com> --- 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 | 23 +++++--- ovn/controller/ovn-controller.c | 120 ++++++++++++++++++++++++++++++++++++++-- ovn/controller/ovn-controller.h | 8 +++ ovn/controller/patch.c | 39 +++++++++++-- ovn/northd/ovn-northd.c | 72 ++++++++++++++++++++++++ ovn/ovn-sb.ovsschema | 11 +++- ovn/ovn-sb.xml | 9 +++ 7 files changed, 263 insertions(+), 19 deletions(-) diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c index 3073727..a757838 100644 --- a/ovn/controller/binding.c +++ b/ovn/controller/binding.c @@ -67,7 +67,8 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl) static bool get_local_iface_ids(const struct ovsrec_bridge *br_int, struct shash *lport_to_iface, - struct sset *all_lports) + struct sset *all_lports, + struct controller_ctx *ctx) { int i; bool changed = false; @@ -93,6 +94,7 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, continue; } shash_add(lport_to_iface, iface_id, iface_rec); + sbrec_port_binding_add_clause_logical_port(ctx->ovnsb_idl, OVSDB_F_EQ, iface_id); if (!sset_find_and_delete(&old_local_ids, iface_id)) { sset_add(&local_ids, iface_id); sset_add(all_lports, iface_id); @@ -108,6 +110,8 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, const char *cur_id; SSET_FOR_EACH(cur_id, &old_local_ids) { + sbrec_port_binding_remove_clause_logical_port(ctx->ovnsb_idl, + OVSDB_F_EQ, cur_id); sset_find_and_delete(&local_ids, cur_id); sset_find_and_delete(all_lports, cur_id); } @@ -131,20 +135,23 @@ local_datapath_lookup_by_uuid(struct hmap *hmap_p, const struct uuid *uuid) } static void -remove_local_datapath(struct hmap *local_datapaths, struct local_datapath *ld) +remove_local_datapath(struct hmap *local_datapaths, struct local_datapath *ld, + struct controller_ctx *ctx) { if (ld->logical_port) { free(ld->logical_port); ld->logical_port = NULL; } hmap_remove(local_datapaths, &ld->hmap_node); + remove_datapath_of_interest(ctx, ld->dp); free(ld); lflow_reset_processing(); } static void add_local_datapath(struct hmap *local_datapaths, - const struct sbrec_port_binding *binding_rec) + const struct sbrec_port_binding *binding_rec, + struct controller_ctx *ctx) { if (get_local_datapath(local_datapaths, binding_rec->datapath->tunnel_key)) { @@ -153,9 +160,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); lport_index_reset(); mcgroup_index_reset(); lflow_reset_processing(); @@ -186,7 +195,7 @@ consider_local_datapath(struct controller_ctx *ctx, if (iface_rec || (binding_rec->parent_port && binding_rec->parent_port[0] && sset_contains(&local_ids, binding_rec->parent_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); } @@ -230,7 +239,7 @@ consider_local_datapath(struct controller_ctx *ctx, binding_rec->logical_port); sbrec_port_binding_set_chassis(binding_rec, chassis_rec); sset_add(all_lports, binding_rec->logical_port); - add_local_datapath(local_datapaths, binding_rec); + add_local_datapath(local_datapaths, binding_rec, ctx); } } else if (chassis_rec && binding_rec->chassis == chassis_rec && strcmp(binding_rec->type, "l3gateway")) { @@ -264,7 +273,7 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, if (br_int) { if (ctx->ovnsb_idl_txn && get_local_iface_ids(br_int, &lport_to_iface, - all_lports)) { + all_lports, ctx)) { process_full_binding = true; } } else { @@ -299,7 +308,7 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, HMAP_FOR_EACH_SAFE (old_ld, next, hmap_node, local_datapaths) { if (!local_datapath_lookup_by_uuid(&keep_local_datapath_by_uuid, &old_ld->uuid)) { - remove_local_datapath(local_datapaths, old_ld); + remove_local_datapath(local_datapaths, old_ld, ctx); } } hmap_destroy(&keep_local_datapath_by_uuid); diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 2ca0295..2ae9982 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -69,6 +69,116 @@ 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; +}; + +/* Contains "struct local_datapath" nodes whose hash values are the + * tunnel_key of datapaths with at least one local port binding. */ +static struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths); +static struct hmap patched_datapaths = HMAP_INITIALIZER(&patched_datapaths); + +static struct hmap dps_of_interest = HMAP_INITIALIZER(&dps_of_interest); + +static bool +add_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)) { + return false; + } + } + + if (!get_local_datapath(&local_datapaths, dp->tunnel_key) && + !get_patched_datapath(&patched_datapaths, dp->tunnel_key)){ + + return false; + } + sbrec_port_binding_add_clause_datapath(ctx->ovnsb_idl, OVSDB_F_EQ, dp); + sbrec_logical_flow_add_clause_logical_datapath(ctx->ovnsb_idl, OVSDB_F_EQ, dp); + sbrec_mac_binding_add_clause_datapath(ctx->ovnsb_idl, OVSDB_F_EQ, dp); + sbrec_multicast_group_add_clause_datapath(ctx->ovnsb_idl, OVSDB_F_EQ, dp); + + node = xzalloc(sizeof *node); + hmap_insert(&dps_of_interest, &node->hmap_node, + uuid_hash(&dp->header_.uuid)); + node->sb_db = dp; + return true; +} + +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)) { + if (!get_local_datapath(&local_datapaths, dp->tunnel_key) && + !get_patched_datapath(&patched_datapaths, dp->tunnel_key)){ + + sbrec_port_binding_remove_clause_datapath(ctx->ovnsb_idl, + OVSDB_F_EQ, dp); + sbrec_logical_flow_remove_clause_logical_datapath(ctx->ovnsb_idl, + OVSDB_F_EQ, dp); + sbrec_mac_binding_remove_clause_datapath(ctx->ovnsb_idl, + OVSDB_F_EQ, dp); + sbrec_multicast_group_remove_clause_datapath(ctx->ovnsb_idl, + OVSDB_F_EQ, dp); + + hmap_remove(&dps_of_interest, &node->hmap_node); + free(node); + return; + } + } + } +} + +void +do_datapath_flood_fill(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); + } + } +} + struct local_datapath * get_local_datapath(const struct hmap *local_datapaths, uint32_t tunnel_key) { @@ -306,11 +416,6 @@ get_nb_cfg(struct ovsdb_idl *idl) return sb ? sb->nb_cfg : 0; } -/* Contains "struct local_datapath" nodes whose hash values are the - * tunnel_key of datapaths with at least one local port binding. */ -static struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths); -static struct hmap patched_datapaths = HMAP_INITIALIZER(&patched_datapaths); - static struct lport_index lports; static struct mcgroup_index mcgroups; @@ -386,6 +491,8 @@ main(int argc, char *argv[]) physical_register_ovs_idl(ovs_idl_loop.idl); ovsdb_idl_get_initial_snapshot(ovs_idl_loop.idl); + /* ovn-controller is starting - nothing special for database monitoring. */ + /* Connect to OVN SB database. */ char *ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl); struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER( @@ -417,6 +524,9 @@ main(int argc, char *argv[]) binding_reset_processing(); lport_index_clear(&lports); mcgroup_index_clear(&mcgroups); + /* ovn-controller is pointed to a new SB database server which + * should be in sync with the previous server ???; hence leave + * database monitoring the same ??? */ } else { free(new_ovnsb_remote); } diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h index 470b1f5..2fe4f0c 100644 --- a/ovn/controller/ovn-controller.h +++ b/ovn/controller/ovn-controller.h @@ -42,6 +42,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 *, @@ -52,6 +53,7 @@ struct local_datapath *get_local_datapath(const struct hmap *, struct patched_datapath { struct hmap_node hmap_node; char *key; /* Holds the uuid of the corresponding datapath. */ + const struct sbrec_datapath_binding *dp; bool local; /* 'True' if the datapath is for gateway router. */ bool stale; /* 'True' if the datapath is not referenced by any patch * port. */ @@ -66,6 +68,12 @@ 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(struct controller_ctx *, + const struct sbrec_datapath_binding *); + +void remove_datapath_of_interest(struct controller_ctx *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 62d68d9..837c262 100644 --- a/ovn/controller/patch.c +++ b/ovn/controller/patch.c @@ -50,6 +50,25 @@ match_patch_port(const struct ovsrec_port *port, const char *peer) return false; } +/* 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); + } + } +} + /* Creates a patch port in bridge 'src' named 'src_name', whose peer is * 'dst_name' in bridge 'dst'. Initializes the patch port's external-ids:'key' * to 'key'. @@ -258,7 +277,9 @@ 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); @@ -273,9 +294,11 @@ add_patched_datapath(struct hmap *patched_datapaths, pd->local = local; pd->key = xasprintf(UUID_FMT, UUID_ARGS(&binding_rec->datapath->header_.uuid)); + pd->dp = binding_rec->datapath; /* stale is set to false. */ hmap_insert(patched_datapaths, &pd->hmap_node, binding_rec->datapath->tunnel_key); + do_datapath_flood_fill(ctx, binding_rec->datapath); } static void @@ -292,7 +315,8 @@ add_logical_patch_ports_preprocess(struct hmap *patched_datapaths) /* This function should cleanup stale patched datapaths and any memory * allocated for fields within a stale patched datapath. */ static void -add_logical_patch_ports_postprocess(struct hmap *patched_datapaths) +add_logical_patch_ports_postprocess(struct hmap *patched_datapaths, + struct controller_ctx *ctx) { /* Clean up stale patched datapaths. */ struct patched_datapath *pd_cur_node, *pd_next_node; @@ -300,6 +324,7 @@ add_logical_patch_ports_postprocess(struct hmap *patched_datapaths) patched_datapaths) { if (pd_cur_node->stale == true) { hmap_remove(patched_datapaths, &pd_cur_node->hmap_node); + remove_datapath_of_interest(ctx, pd_cur_node->dp); free(pd_cur_node->key); free(pd_cur_node); } @@ -368,15 +393,20 @@ 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); if (local_port) { if (binding->chassis != chassis_rec && ctx->ovnsb_idl_txn) { sbrec_port_binding_set_chassis(binding, chassis_rec); } } + sbrec_port_binding_add_clause_logical_port(ctx->ovnsb_idl, + OVSDB_F_EQ, local); + sbrec_port_binding_add_clause_logical_port(ctx->ovnsb_idl, + OVSDB_F_EQ, peer); + } } - add_logical_patch_ports_postprocess(patched_datapaths); + add_logical_patch_ports_postprocess(patched_datapaths, ctx); } void @@ -412,6 +442,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 d6c14cf..f8c3211 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -227,6 +227,41 @@ 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) { @@ -292,6 +327,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; @@ -342,6 +379,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; @@ -357,6 +395,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); } @@ -528,6 +567,25 @@ build_datapaths(struct northd_context *ctx, struct hmap *datapaths) sbrec_datapath_binding_set_external_ids(od->sb, &id); 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] = 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); } @@ -1147,6 +1205,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); @@ -1178,6 +1242,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(&op->nbsp->options, "router-port"); if (!router_port) { @@ -4174,6 +4244,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 b1737f5..35a3a63 100644 --- a/ovn/ovn-sb.ovsschema +++ b/ovn/ovn-sb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Southbound", - "version": "1.7.0", - "cksum": "3677179333 6917", + "version": "1.8.0", + "cksum": "945864814 7260", "tables": { "SB_Global": { "columns": { @@ -88,7 +88,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 13c9526..4689454 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -1404,6 +1404,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 -- 1.9.1 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev