Message ID | 1469706510-17617-1-git-send-email-lirans@il.ibm.com |
---|---|
State | Superseded |
Headers | show |
Mickey Spiegel/San Jose/IBM wrote on 31/07/2016 08:59:29 PM: > > Comments inline with <Mickey> > > -----"dev" <dev-bounces@openvswitch.org> wrote: ----- > To: Ben Pfaff <blp@ovn.org> > From: Liran Schour > Sent by: "dev" > Date: 07/28/2016 04:49AM > Cc: dev@openvswitch.org > Subject: [ovs-dev] [PATCH monitor_cond V11] ovn: implementation of > conditional monitoring usage > Conditional monitor of: Port_Binding, Logical_Flow, Multicast_Group > MAC_Binding tables. As a result ovn-controller will be notified only about > records belongs to a datapath that is being served by this hypervisor. > > Performance evaluation: > OVN is the main candidate for conditional monitoring usage. It is clear that > conditional monitoring reduces computation on the ovn-controller (client) side > due to the reduced size of flow tables and update messages. Performance > evaluation shows up to 75% computation reduction on the ovn-controller side. > However, performance evaluation shows also a reduction in > computation on the SB > ovsdb-server side proportional to the degree that each logical network is > spread over physical hosts in the DC. Evaluation shows that in a realistic > scenarios there is a computation reduction also in the server side. > > Evaluation on simulated environment of 50 hosts and 1000 logical ports shows > the following results (cycles #): > > LN spread over # hosts| master | patch | change > ------------------------------------------------------------- > 1 | 24597200127 | 24339235374 | 1.0% > 6 | 23788521572 | 19145229352 | 19.5% > 12 | 23886405758 | 17913143176 | 25.0% > 18 | 25812686279 | 23675094540 | 8.2% > 24 | 28414671499 | 24770202308 | 12.8% > 30 | 31487218890 | 28397543436 | 9.8% > 36 | 36116993930 | 34105388739 | 5.5% > 42 | 37898342465 | 38647139083 | -1.9% > 48 | 41637996229 | 41846616306 | -0.5% > 50 | 41679995357 | 43455565977 | -4.2% > > Signed-off-by: Liran Schour <lirans@il.ibm.com> > --- > ovn/controller/automake.mk | 4 +- > ovn/controller/binding.c | 45 ++++++--- > ovn/controller/binding.h | 4 +- > ovn/controller/filter.c | 207 +++++++++++++++++++++++++++++ > +++++++++++ > ovn/controller/filter.h | 36 +++++++ > ovn/controller/ovn-controller.c | 18 +++- > ovn/controller/ovn-controller.h | 2 + > ovn/controller/patch.c | 30 ++++-- > ovn/controller/patch.h | 4 +- > tests/ovn-controller.at | 3 + > tests/ovn.at | 1 + > 11 files changed, 327 insertions(+), 27 deletions(-) > create mode 100644 ovn/controller/filter.c > create mode 100644 ovn/controller/filter.h > > <Mickey> Thanks to all_lports and possibly more, this no longer > applies cleanly. Rebase required. > > <Mickey> A few nits inline. > > > diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk > index cf57bbd..0318654 100644 > --- a/ovn/controller/automake.mk > +++ b/ovn/controller/automake.mk > @@ -19,7 +19,9 @@ ovn_controller_ovn_controller_SOURCES = \ > ovn/controller/ovn-controller.c \ > ovn/controller/ovn-controller.h \ > ovn/controller/physical.c \ > - ovn/controller/physical.h > + ovn/controller/physical.h \ > + ovn/controller/filter.c \ > + ovn/controller/filter.h > ovn_controller_ovn_controller_LDADD = ovn/lib/libovn.la lib/libopenvswitch.la > man_MANS += ovn/controller/ovn-controller.8 > EXTRA_DIST += ovn/controller/ovn-controller.8.xml > diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c > index e83c1d5..be77619 100644 > --- a/ovn/controller/binding.c > +++ b/ovn/controller/binding.c > @@ -27,6 +27,8 @@ > #include "openvswitch/vlog.h" > #include "ovn/lib/ovn-sb-idl.h" > #include "ovn-controller.h" > +#include "lport.h" > +#include "filter.h" > > VLOG_DEFINE_THIS_MODULE(binding); > > @@ -66,7 +68,9 @@ 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 shash *lport_to_iface, > + struct lport_index *lports_index, > + struct controller_ctx *ctx) > { > int i; > bool changed = false; > @@ -92,6 +96,9 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, > continue; > } > shash_add(lport_to_iface, iface_id, iface_rec); > + if (!lport_lookup_by_name(lports_index, iface_id)) { > + filter_lport(ctx, iface_id); > + } > if (!sset_find_and_delete(&old_local_ids, iface_id)) { > sset_add(&local_ids, iface_id); > changed = true; > @@ -123,8 +130,14 @@ 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 controller_ctx *ctx, struct hmap > *local_datapaths, > + struct hmap *patched_datapaths, > + struct local_datapath *ld) > { > + if (!get_patched_datapath(patched_datapaths, ld->tunnel_key)) { > + unfilter_datapath(ctx, ld->tunnel_key); > + } > + > if (ld->logical_port) { > free(ld->logical_port); > ld->logical_port = NULL; > @@ -136,7 +149,8 @@ remove_local_datapath(struct hmap > *local_datapaths, struct local_datapath *ld) > > 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)) { > @@ -146,11 +160,13 @@ add_local_datapath(struct hmap *local_datapaths, > struct local_datapath *ld = xzalloc(sizeof *ld); > ld->logical_port = xstrdup(binding_rec->logical_port); > memcpy(&ld->uuid, &binding_rec->header_.uuid, sizeof ld->uuid); > + ld->tunnel_key = binding_rec->datapath->tunnel_key; > hmap_insert(local_datapaths, &ld->hmap_node, > binding_rec->datapath->tunnel_key); > lport_index_reset(); > mcgroup_index_reset(); > lflow_reset_processing(); > + filter_datapath(ctx, binding_rec->datapath); > } > > static void > @@ -177,7 +193,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); > } > @@ -216,7 +232,7 @@ consider_local_datapath(struct controller_ctx *ctx, > VLOG_INFO("Claiming l2gateway port %s for this chassis.", > binding_rec->logical_port); > sbrec_port_binding_set_chassis(binding_rec, chassis_rec); > - 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, "gateway")) { > @@ -230,7 +246,8 @@ consider_local_datapath(struct controller_ctx *ctx, > > void > binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, > - const char *chassis_id, struct hmap *local_datapaths) > + const char *chassis_id, struct lport_index *lports_index, > + struct hmap *local_datapaths, struct hmap *patched_datapaths) > { > const struct sbrec_chassis *chassis_rec; > const struct sbrec_port_binding *binding_rec; > @@ -242,7 +259,8 @@ 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)) { > + if (ctx->ovnsb_idl_txn && get_local_iface_ids(br_int, > &lport_to_iface, > + lports_index, ctx)) { > process_full_binding = true; > } > } else { > @@ -258,8 +276,9 @@ binding_run(struct controller_ctx *ctx, const > struct ovsrec_bridge *br_int, > struct hmap keep_local_datapath_by_uuid = > HMAP_INITIALIZER(&keep_local_datapath_by_uuid); > SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) { > - consider_local_datapath(ctx, chassis_rec, binding_rec, > - local_datapaths, &lport_to_iface); > + consider_local_datapath(ctx, chassis_rec, > + binding_rec, local_datapaths, > + &lport_to_iface); > <Mickey> There is no longer any change here, just reformatting that > is not required. Right. > > struct local_datapath *ld = xzalloc(sizeof *ld); > memcpy(&ld->uuid, &binding_rec->header_.uuid, sizeof ld->uuid); > hmap_insert(&keep_local_datapath_by_uuid, &ld->uuid_hmap_node, > @@ -269,7 +288,8 @@ 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(ctx, local_datapaths, > patched_datapaths, > + old_ld); > } > } > hmap_destroy(&keep_local_datapath_by_uuid); > @@ -286,8 +306,9 @@ binding_run(struct controller_ctx *ctx, const > struct ovsrec_bridge *br_int, > poll_immediate_wake(); > } > } else { > - consider_local_datapath(ctx, chassis_rec, binding_rec, > - local_datapaths, &lport_to_iface); > + consider_local_datapath(ctx, chassis_rec, > + binding_rec, local_datapaths, > + &lport_to_iface); > <Mickey> There is no longer any change here, just reformatting that > is not required. > Right. > } > } > } > diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h > index 8753d44..60d99db 100644 > --- a/ovn/controller/binding.h > +++ b/ovn/controller/binding.h > @@ -25,11 +25,13 @@ struct ovsdb_idl; > struct ovsrec_bridge; > struct simap; > struct sset; > +struct lport_index; > > void binding_register_ovs_idl(struct ovsdb_idl *); > void binding_reset_processing(void); > void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int, > - const char *chassis_id, struct hmap *local_datapaths); > + const char *chassis_id, struct lport_index *lports_index, > + struct hmap *local_datapaths, struct hmap > *patched_datapaths); > bool binding_cleanup(struct controller_ctx *, const char *chassis_id); > > #endif /* ovn/binding.h */ > diff --git a/ovn/controller/filter.c b/ovn/controller/filter.c > new file mode 100644 > index 0000000..d90fb79 > --- /dev/null > +++ b/ovn/controller/filter.c > @@ -0,0 +1,207 @@ > +/* Copyright (c) 2015, 2016 Nicira, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#include <config.h> > +#include "filter.h" > + > +#include "openvswitch/vlog.h" > +#include "ovn/lib/ovn-sb-idl.h" > +#include "ovn-controller.h" > +#include "lport.h" > + > +VLOG_DEFINE_THIS_MODULE(filter); > + > +static struct hmap filtered_dps = HMAP_INITIALIZER(&filtered_dps); > +static struct hmap filtered_lps = HMAP_INITIALIZER(&filtered_lps); > + > +struct filtered_dp { > + struct hmap_node hmap_node; > + int64_t tunnel_key; > + const struct sbrec_datapath_binding *datapath; > +}; > + > +struct filtered_lp { > + struct hmap_node hmap_node; > + const char *lport_name; > + bool used; > +}; > + > +void > +filter_init(struct ovsdb_idl *idl) > +{ > + sbrec_port_binding_add_clause_false(idl); > + sbrec_mac_binding_add_clause_false(idl); > + sbrec_logical_flow_add_clause_false(idl); > + sbrec_multicast_group_add_clause_false(idl); > +} > + > +void > +filter_mark_unused(void) > +{ > + struct filtered_lp *lp; > + > + HMAP_FOR_EACH(lp, hmap_node, &filtered_lps) { > + lp->used = false; > + } > +} > + > +void > +filter_clear(struct ovsdb_idl *idl) > +{ > + struct filtered_lp *lp, *next; > + struct filtered_lp *dp, *next_dp; > + > + HMAP_FOR_EACH_SAFE(lp, next, hmap_node, &filtered_lps) { > + hmap_remove(&filtered_lps, &lp->hmap_node); > + free(lp); > + } > + HMAP_FOR_EACH_SAFE(dp, next_dp, hmap_node, &filtered_dps) { > + hmap_remove(&filtered_dps, &dp->hmap_node); > + free(dp); > + } > + > + ovsdb_idl_condition_reset(idl, > + &sbrec_table_port_binding); > + ovsdb_idl_condition_reset(idl, > + &sbrec_table_logical_flow); > + ovsdb_idl_condition_reset(idl, > + &sbrec_table_mac_binding); > + ovsdb_idl_condition_reset(idl, > + &sbrec_table_multicast_group); > +} > + > +static struct filtered_dp * > +lookup_dp_by_key(int64_t tunnel_key) > +{ > + struct filtered_dp *dp; > + > + HMAP_FOR_EACH_WITH_HASH(dp, hmap_node, tunnel_key, > + &filtered_dps) { > + if (dp->tunnel_key == tunnel_key) { > + return dp; > + } > + } > + return NULL; > +} > + > +void > +filter_remove_unused_lports(struct controller_ctx *ctx, > + const struct lport_index *lports_index) > +{ > + struct filtered_lp *lp, *next; > + > + HMAP_FOR_EACH_SAFE(lp, next, hmap_node, &filtered_lps) { > + if (!lp->used) { > + const struct sbrec_port_binding *pb = > + lport_lookup_by_name(lports_index, lp->lport_name); > + if (!pb) { > + continue; > + } > + if (lookup_dp_by_key(pb->datapath->tunnel_key)) { > + VLOG_INFO("Unfilter Port %s", lp->lport_name); > > <Mickey> Wondering if the VLOGs should have a rate limit on them. > I do not see rate limit on VLOGS of similar cases in the code. For example VLOG_INFO("Claiming lport %s for this chassis.",.. Will leave it untouched for now. > + sbrec_port_binding_remove_clause_logical_port(ctx->ovnsb_idl, > + OVSDB_F_EQ, > + > lp->lport_name); > + hmap_remove(&filtered_lps, &lp->hmap_node); > + free(lp); > + } > + } > + } > +} > + > +void > +filter_lport(struct controller_ctx *ctx, const char *lport_name) > +{ > + struct filtered_lp *lp; > + size_t hash = hash_bytes(lport_name, strlen(lport_name), 0); > + > + HMAP_FOR_EACH_WITH_HASH(lp, hmap_node, hash, &filtered_lps) { > + if (!strcmp(lp->lport_name, lport_name)) { > + lp->used = true; > + return; > + } > + } > + > + VLOG_INFO("Filter Port %s", lport_name); > <Mickey> Another VLOG candidate for a rate limit. > > + > + sbrec_port_binding_add_clause_logical_port(ctx->ovnsb_idl, > + OVSDB_F_EQ, > + lport_name); > + > + char *name = xmalloc(strlen(lport_name)); > + lp = xmalloc(sizeof *lp); > + > + strcpy(name, lport_name); > + lp->lport_name = name; > + lp->used = true; > + hmap_insert(&filtered_lps, &lp->hmap_node, > + hash); > +} > + > +void > +filter_datapath(struct controller_ctx *ctx, > + const struct sbrec_datapath_binding *datapath) > +{ > + struct filtered_dp *dp; > + > + dp = lookup_dp_by_key(datapath->tunnel_key); > + if (dp) { > + return; > + } > + > + VLOG_INFO("Filter DP "UUID_FMT, UUID_ARGS(&datapath->header_.uuid)); > <Mickey> Another VLOG candidate for a rate limit. > > + sbrec_port_binding_add_clause_datapath(ctx->ovnsb_idl, > + OVSDB_F_EQ, > + datapath); > + sbrec_mac_binding_add_clause_datapath(ctx->ovnsb_idl, > + OVSDB_F_EQ, > + datapath); > + sbrec_logical_flow_add_clause_logical_datapath(ctx->ovnsb_idl, > + OVSDB_F_EQ, > + datapath); > + sbrec_multicast_group_add_clause_datapath(ctx->ovnsb_idl, > + OVSDB_F_EQ, > + datapath); > + > + dp = xmalloc(sizeof *dp); > + dp->tunnel_key = datapath->tunnel_key; > + dp->datapath = datapath; > + hmap_insert(&filtered_dps, &dp->hmap_node, datapath->tunnel_key); > +} > + > +void > +unfilter_datapath(struct controller_ctx *ctx, int64_t tunnel_key) > +{ > + struct filtered_dp *dp = lookup_dp_by_key(tunnel_key); > + > + if (dp) { > + VLOG_INFO("Unfilter DP "UUID_FMT, > <Mickey> Another VLOG candidate for a rate limit. > > + UUID_ARGS(&dp->datapath->header_.uuid)); > + sbrec_port_binding_remove_clause_datapath(ctx->ovnsb_idl, > + OVSDB_F_EQ, > + dp->datapath); > + sbrec_mac_binding_remove_clause_datapath(ctx->ovnsb_idl, > + OVSDB_F_EQ, > + dp->datapath); > + sbrec_logical_flow_remove_clause_logical_datapath(ctx->ovnsb_idl, > + OVSDB_F_EQ, > + dp->datapath); > + sbrec_multicast_group_remove_clause_datapath(ctx->ovnsb_idl, > + OVSDB_F_EQ, > + dp->datapath); > + hmap_remove(&filtered_dps, &dp->hmap_node); > + free(dp); > + } > +} > diff --git a/ovn/controller/filter.h b/ovn/controller/filter.h > new file mode 100644 > index 0000000..e09c1c8 > --- /dev/null > +++ b/ovn/controller/filter.h > @@ -0,0 +1,36 @@ > +/* Copyright (c) 2015 Nicira, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#ifndef OVN_FILTER_H > +#define OVN_FILTER_H 1 > + > +#include <stdint.h> > + > +struct controller_ctx; > +struct ovsdb_idl; > +struct sbrec_datapath_binding; > +struct lport_index; > + > +void filter_init(struct ovsdb_idl *idl); > +void filter_clear(struct ovsdb_idl *idl); > +void filter_mark_unused(void); > +void filter_remove_unused_lports(struct controller_ctx *ctx, > + const struct lport_index *lports); > +void filter_lport(struct controller_ctx *ctx, const char *lport_name); > +void filter_datapath(struct controller_ctx *ctx, > + const struct sbrec_datapath_binding *datapath); > +void unfilter_datapath(struct controller_ctx *ctx, int64_t tunnel_key); > + > +#endif /* ovn/controller/filter.h */ > diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c > index 5c74186..1782561 100644 > --- a/ovn/controller/ovn-controller.c > +++ b/ovn/controller/ovn-controller.c > @@ -54,6 +54,7 @@ > #include "stream.h" > #include "unixctl.h" > #include "util.h" > +#include "filter.h" > > VLOG_DEFINE_THIS_MODULE(main); > > @@ -387,6 +388,8 @@ 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); > > + filter_init(ovnsb_idl_loop.idl); > + > /* Track the southbound idl. */ > ovsdb_idl_track_add_all(ovnsb_idl_loop.idl); > > @@ -412,6 +415,7 @@ main(int argc, char *argv[]) > binding_reset_processing(); > lport_index_clear(&lports); > mcgroup_index_clear(&mcgroups); > + filter_clear(ovnsb_idl_loop.idl); > } else { > free(new_ovnsb_remote); > } > @@ -431,18 +435,21 @@ main(int argc, char *argv[]) > const char *chassis_id = get_chassis_id(ctx.ovs_idl); > > const struct sbrec_chassis *chassis = NULL; > + > + lport_index_fill(&lports, ctx.ovnsb_idl); > + mcgroup_index_fill(&mcgroups, ctx.ovnsb_idl); > + filter_mark_unused(); > + > if (chassis_id) { > chassis = chassis_run(&ctx, chassis_id); > encaps_run(&ctx, br_int, chassis_id); > - binding_run(&ctx, br_int, chassis_id, &local_datapaths); > + binding_run(&ctx, br_int, chassis_id, &lports, &local_datapaths, > + &patched_datapaths); > } > > if (br_int && chassis_id) { > patch_run(&ctx, br_int, chassis_id, &local_datapaths, > - &patched_datapaths); > - > - lport_index_fill(&lports, ctx.ovnsb_idl); > - mcgroup_index_fill(&mcgroups, ctx.ovnsb_idl); > + &patched_datapaths, &lports); > > enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int); > > @@ -466,6 +473,7 @@ main(int argc, char *argv[]) > } > } > > + filter_remove_unused_lports(&ctx, &lports); > sset_destroy(&all_lports); > > unixctl_server_run(unixctl); > diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h > index 470b1f5..8eb0b23 100644 > --- a/ovn/controller/ovn-controller.h > +++ b/ovn/controller/ovn-controller.h > @@ -40,6 +40,7 @@ struct local_datapath { > struct hmap_node hmap_node; > struct hmap_node uuid_hmap_node; > struct uuid uuid; > + int64_t tunnel_key; > char *logical_port; > const struct sbrec_port_binding *localnet_port; > }; > @@ -55,6 +56,7 @@ struct patched_datapath { > bool local; /* 'True' if the datapath is for gateway router. */ > bool stale; /* 'True' if the datapath is not referenced by any patch > * port. */ > + int64_t tunnel_key; > }; > > struct patched_datapath *get_patched_datapath(const struct hmap *, > diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c > index 707d08b..72e39c8 100644 > --- a/ovn/controller/patch.c > +++ b/ovn/controller/patch.c > @@ -24,6 +24,7 @@ > #include "openvswitch/hmap.h" > #include "openvswitch/vlog.h" > #include "ovn-controller.h" > +#include "filter.h" > > VLOG_DEFINE_THIS_MODULE(patch); > > @@ -258,7 +259,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); > @@ -273,9 +275,11 @@ add_patched_datapath(struct hmap *patched_datapaths, > pd->local = local; > pd->key = xasprintf(UUID_FMT, > UUID_ARGS(&binding_rec->datapath->header_.uuid)); > + pd->tunnel_key = binding_rec->datapath->tunnel_key; > /* stale is set to false. */ > hmap_insert(patched_datapaths, &pd->hmap_node, > binding_rec->datapath->tunnel_key); > + filter_datapath(ctx, binding_rec->datapath); > } > > static void > @@ -292,7 +296,9 @@ 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 hmap *local_datapaths, > + struct controller_ctx *ctx) > { > /* Clean up stale patched datapaths. */ > struct patched_datapath *pd_cur_node, *pd_next_node; > @@ -300,6 +306,9 @@ 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); > + if (!get_local_datapath(local_datapaths, > pd_cur_node->tunnel_key)) { > + unfilter_datapath(ctx, pd_cur_node->tunnel_key); > + } > free(pd_cur_node->key); > free(pd_cur_node); > } > @@ -333,7 +342,9 @@ add_logical_patch_ports(struct controller_ctx *ctx, > const struct ovsrec_bridge *br_int, > const char *local_chassis_id, > struct shash *existing_ports, > - struct hmap *patched_datapaths) > + struct hmap *patched_datapaths, > + struct hmap *local_datapaths, > + struct lport_index *lports_index) > { > const struct sbrec_chassis *chassis_rec; > chassis_rec = get_chassis(ctx->ovnsb_idl, local_chassis_id); > @@ -368,21 +379,26 @@ 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); > } > } > + if (peer) { > <Mickey> There is "if (!peer) { continue; }" a few lines up, so this > "if (peer)" is redundant. > Right. Will remove the if statement. Thanks for the review. - Liran
diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk index cf57bbd..0318654 100644 --- a/ovn/controller/automake.mk +++ b/ovn/controller/automake.mk @@ -19,7 +19,9 @@ ovn_controller_ovn_controller_SOURCES = \ ovn/controller/ovn-controller.c \ ovn/controller/ovn-controller.h \ ovn/controller/physical.c \ - ovn/controller/physical.h + ovn/controller/physical.h \ + ovn/controller/filter.c \ + ovn/controller/filter.h ovn_controller_ovn_controller_LDADD = ovn/lib/libovn.la lib/libopenvswitch.la man_MANS += ovn/controller/ovn-controller.8 EXTRA_DIST += ovn/controller/ovn-controller.8.xml diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c index e83c1d5..be77619 100644 --- a/ovn/controller/binding.c +++ b/ovn/controller/binding.c @@ -27,6 +27,8 @@ #include "openvswitch/vlog.h" #include "ovn/lib/ovn-sb-idl.h" #include "ovn-controller.h" +#include "lport.h" +#include "filter.h" VLOG_DEFINE_THIS_MODULE(binding); @@ -66,7 +68,9 @@ 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 shash *lport_to_iface, + struct lport_index *lports_index, + struct controller_ctx *ctx) { int i; bool changed = false; @@ -92,6 +96,9 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, continue; } shash_add(lport_to_iface, iface_id, iface_rec); + if (!lport_lookup_by_name(lports_index, iface_id)) { + filter_lport(ctx, iface_id); + } if (!sset_find_and_delete(&old_local_ids, iface_id)) { sset_add(&local_ids, iface_id); changed = true; @@ -123,8 +130,14 @@ 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 controller_ctx *ctx, struct hmap *local_datapaths, + struct hmap *patched_datapaths, + struct local_datapath *ld) { + if (!get_patched_datapath(patched_datapaths, ld->tunnel_key)) { + unfilter_datapath(ctx, ld->tunnel_key); + } + if (ld->logical_port) { free(ld->logical_port); ld->logical_port = NULL; @@ -136,7 +149,8 @@ remove_local_datapath(struct hmap *local_datapaths, struct local_datapath *ld) 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)) { @@ -146,11 +160,13 @@ add_local_datapath(struct hmap *local_datapaths, struct local_datapath *ld = xzalloc(sizeof *ld); ld->logical_port = xstrdup(binding_rec->logical_port); memcpy(&ld->uuid, &binding_rec->header_.uuid, sizeof ld->uuid); + ld->tunnel_key = binding_rec->datapath->tunnel_key; hmap_insert(local_datapaths, &ld->hmap_node, binding_rec->datapath->tunnel_key); lport_index_reset(); mcgroup_index_reset(); lflow_reset_processing(); + filter_datapath(ctx, binding_rec->datapath); } static void @@ -177,7 +193,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); } @@ -216,7 +232,7 @@ consider_local_datapath(struct controller_ctx *ctx, VLOG_INFO("Claiming l2gateway port %s for this chassis.", binding_rec->logical_port); sbrec_port_binding_set_chassis(binding_rec, chassis_rec); - 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, "gateway")) { @@ -230,7 +246,8 @@ consider_local_datapath(struct controller_ctx *ctx, void binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, - const char *chassis_id, struct hmap *local_datapaths) + const char *chassis_id, struct lport_index *lports_index, + struct hmap *local_datapaths, struct hmap *patched_datapaths) { const struct sbrec_chassis *chassis_rec; const struct sbrec_port_binding *binding_rec; @@ -242,7 +259,8 @@ 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)) { + if (ctx->ovnsb_idl_txn && get_local_iface_ids(br_int, &lport_to_iface, + lports_index, ctx)) { process_full_binding = true; } } else { @@ -258,8 +276,9 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, struct hmap keep_local_datapath_by_uuid = HMAP_INITIALIZER(&keep_local_datapath_by_uuid); SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) { - consider_local_datapath(ctx, chassis_rec, binding_rec, - local_datapaths, &lport_to_iface); + consider_local_datapath(ctx, chassis_rec, + binding_rec, local_datapaths, + &lport_to_iface); struct local_datapath *ld = xzalloc(sizeof *ld); memcpy(&ld->uuid, &binding_rec->header_.uuid, sizeof ld->uuid); hmap_insert(&keep_local_datapath_by_uuid, &ld->uuid_hmap_node, @@ -269,7 +288,8 @@ 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(ctx, local_datapaths, patched_datapaths, + old_ld); } } hmap_destroy(&keep_local_datapath_by_uuid); @@ -286,8 +306,9 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, poll_immediate_wake(); } } else { - consider_local_datapath(ctx, chassis_rec, binding_rec, - local_datapaths, &lport_to_iface); + consider_local_datapath(ctx, chassis_rec, + binding_rec, local_datapaths, + &lport_to_iface); } } } diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h index 8753d44..60d99db 100644 --- a/ovn/controller/binding.h +++ b/ovn/controller/binding.h @@ -25,11 +25,13 @@ struct ovsdb_idl; struct ovsrec_bridge; struct simap; struct sset; +struct lport_index; void binding_register_ovs_idl(struct ovsdb_idl *); void binding_reset_processing(void); void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int, - const char *chassis_id, struct hmap *local_datapaths); + const char *chassis_id, struct lport_index *lports_index, + struct hmap *local_datapaths, struct hmap *patched_datapaths); bool binding_cleanup(struct controller_ctx *, const char *chassis_id); #endif /* ovn/binding.h */ diff --git a/ovn/controller/filter.c b/ovn/controller/filter.c new file mode 100644 index 0000000..d90fb79 --- /dev/null +++ b/ovn/controller/filter.c @@ -0,0 +1,207 @@ +/* Copyright (c) 2015, 2016 Nicira, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <config.h> +#include "filter.h" + +#include "openvswitch/vlog.h" +#include "ovn/lib/ovn-sb-idl.h" +#include "ovn-controller.h" +#include "lport.h" + +VLOG_DEFINE_THIS_MODULE(filter); + +static struct hmap filtered_dps = HMAP_INITIALIZER(&filtered_dps); +static struct hmap filtered_lps = HMAP_INITIALIZER(&filtered_lps); + +struct filtered_dp { + struct hmap_node hmap_node; + int64_t tunnel_key; + const struct sbrec_datapath_binding *datapath; +}; + +struct filtered_lp { + struct hmap_node hmap_node; + const char *lport_name; + bool used; +}; + +void +filter_init(struct ovsdb_idl *idl) +{ + sbrec_port_binding_add_clause_false(idl); + sbrec_mac_binding_add_clause_false(idl); + sbrec_logical_flow_add_clause_false(idl); + sbrec_multicast_group_add_clause_false(idl); +} + +void +filter_mark_unused(void) +{ + struct filtered_lp *lp; + + HMAP_FOR_EACH(lp, hmap_node, &filtered_lps) { + lp->used = false; + } +} + +void +filter_clear(struct ovsdb_idl *idl) +{ + struct filtered_lp *lp, *next; + struct filtered_lp *dp, *next_dp; + + HMAP_FOR_EACH_SAFE(lp, next, hmap_node, &filtered_lps) { + hmap_remove(&filtered_lps, &lp->hmap_node); + free(lp); + } + HMAP_FOR_EACH_SAFE(dp, next_dp, hmap_node, &filtered_dps) { + hmap_remove(&filtered_dps, &dp->hmap_node); + free(dp); + } + + ovsdb_idl_condition_reset(idl, + &sbrec_table_port_binding); + ovsdb_idl_condition_reset(idl, + &sbrec_table_logical_flow); + ovsdb_idl_condition_reset(idl, + &sbrec_table_mac_binding); + ovsdb_idl_condition_reset(idl, + &sbrec_table_multicast_group); +} + +static struct filtered_dp * +lookup_dp_by_key(int64_t tunnel_key) +{ + struct filtered_dp *dp; + + HMAP_FOR_EACH_WITH_HASH(dp, hmap_node, tunnel_key, + &filtered_dps) { + if (dp->tunnel_key == tunnel_key) { + return dp; + } + } + return NULL; +} + +void +filter_remove_unused_lports(struct controller_ctx *ctx, + const struct lport_index *lports_index) +{ + struct filtered_lp *lp, *next; + + HMAP_FOR_EACH_SAFE(lp, next, hmap_node, &filtered_lps) { + if (!lp->used) { + const struct sbrec_port_binding *pb = + lport_lookup_by_name(lports_index, lp->lport_name); + if (!pb) { + continue; + } + if (lookup_dp_by_key(pb->datapath->tunnel_key)) { + VLOG_INFO("Unfilter Port %s", lp->lport_name); + sbrec_port_binding_remove_clause_logical_port(ctx->ovnsb_idl, + OVSDB_F_EQ, + lp->lport_name); + hmap_remove(&filtered_lps, &lp->hmap_node); + free(lp); + } + } + } +} + +void +filter_lport(struct controller_ctx *ctx, const char *lport_name) +{ + struct filtered_lp *lp; + size_t hash = hash_bytes(lport_name, strlen(lport_name), 0); + + HMAP_FOR_EACH_WITH_HASH(lp, hmap_node, hash, &filtered_lps) { + if (!strcmp(lp->lport_name, lport_name)) { + lp->used = true; + return; + } + } + + VLOG_INFO("Filter Port %s", lport_name); + + sbrec_port_binding_add_clause_logical_port(ctx->ovnsb_idl, + OVSDB_F_EQ, + lport_name); + + char *name = xmalloc(strlen(lport_name)); + lp = xmalloc(sizeof *lp); + + strcpy(name, lport_name); + lp->lport_name = name; + lp->used = true; + hmap_insert(&filtered_lps, &lp->hmap_node, + hash); +} + +void +filter_datapath(struct controller_ctx *ctx, + const struct sbrec_datapath_binding *datapath) +{ + struct filtered_dp *dp; + + dp = lookup_dp_by_key(datapath->tunnel_key); + if (dp) { + return; + } + + VLOG_INFO("Filter DP "UUID_FMT, UUID_ARGS(&datapath->header_.uuid)); + sbrec_port_binding_add_clause_datapath(ctx->ovnsb_idl, + OVSDB_F_EQ, + datapath); + sbrec_mac_binding_add_clause_datapath(ctx->ovnsb_idl, + OVSDB_F_EQ, + datapath); + sbrec_logical_flow_add_clause_logical_datapath(ctx->ovnsb_idl, + OVSDB_F_EQ, + datapath); + sbrec_multicast_group_add_clause_datapath(ctx->ovnsb_idl, + OVSDB_F_EQ, + datapath); + + dp = xmalloc(sizeof *dp); + dp->tunnel_key = datapath->tunnel_key; + dp->datapath = datapath; + hmap_insert(&filtered_dps, &dp->hmap_node, datapath->tunnel_key); +} + +void +unfilter_datapath(struct controller_ctx *ctx, int64_t tunnel_key) +{ + struct filtered_dp *dp = lookup_dp_by_key(tunnel_key); + + if (dp) { + VLOG_INFO("Unfilter DP "UUID_FMT, + UUID_ARGS(&dp->datapath->header_.uuid)); + sbrec_port_binding_remove_clause_datapath(ctx->ovnsb_idl, + OVSDB_F_EQ, + dp->datapath); + sbrec_mac_binding_remove_clause_datapath(ctx->ovnsb_idl, + OVSDB_F_EQ, + dp->datapath); + sbrec_logical_flow_remove_clause_logical_datapath(ctx->ovnsb_idl, + OVSDB_F_EQ, + dp->datapath); + sbrec_multicast_group_remove_clause_datapath(ctx->ovnsb_idl, + OVSDB_F_EQ, + dp->datapath); + hmap_remove(&filtered_dps, &dp->hmap_node); + free(dp); + } +} diff --git a/ovn/controller/filter.h b/ovn/controller/filter.h new file mode 100644 index 0000000..e09c1c8 --- /dev/null +++ b/ovn/controller/filter.h @@ -0,0 +1,36 @@ +/* Copyright (c) 2015 Nicira, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef OVN_FILTER_H +#define OVN_FILTER_H 1 + +#include <stdint.h> + +struct controller_ctx; +struct ovsdb_idl; +struct sbrec_datapath_binding; +struct lport_index; + +void filter_init(struct ovsdb_idl *idl); +void filter_clear(struct ovsdb_idl *idl); +void filter_mark_unused(void); +void filter_remove_unused_lports(struct controller_ctx *ctx, + const struct lport_index *lports); +void filter_lport(struct controller_ctx *ctx, const char *lport_name); +void filter_datapath(struct controller_ctx *ctx, + const struct sbrec_datapath_binding *datapath); +void unfilter_datapath(struct controller_ctx *ctx, int64_t tunnel_key); + +#endif /* ovn/controller/filter.h */ diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 5c74186..1782561 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -54,6 +54,7 @@ #include "stream.h" #include "unixctl.h" #include "util.h" +#include "filter.h" VLOG_DEFINE_THIS_MODULE(main); @@ -387,6 +388,8 @@ 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); + filter_init(ovnsb_idl_loop.idl); + /* Track the southbound idl. */ ovsdb_idl_track_add_all(ovnsb_idl_loop.idl); @@ -412,6 +415,7 @@ main(int argc, char *argv[]) binding_reset_processing(); lport_index_clear(&lports); mcgroup_index_clear(&mcgroups); + filter_clear(ovnsb_idl_loop.idl); } else { free(new_ovnsb_remote); } @@ -431,18 +435,21 @@ main(int argc, char *argv[]) const char *chassis_id = get_chassis_id(ctx.ovs_idl); const struct sbrec_chassis *chassis = NULL; + + lport_index_fill(&lports, ctx.ovnsb_idl); + mcgroup_index_fill(&mcgroups, ctx.ovnsb_idl); + filter_mark_unused(); + if (chassis_id) { chassis = chassis_run(&ctx, chassis_id); encaps_run(&ctx, br_int, chassis_id); - binding_run(&ctx, br_int, chassis_id, &local_datapaths); + binding_run(&ctx, br_int, chassis_id, &lports, &local_datapaths, + &patched_datapaths); } if (br_int && chassis_id) { patch_run(&ctx, br_int, chassis_id, &local_datapaths, - &patched_datapaths); - - lport_index_fill(&lports, ctx.ovnsb_idl); - mcgroup_index_fill(&mcgroups, ctx.ovnsb_idl); + &patched_datapaths, &lports); enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int); @@ -466,6 +473,7 @@ main(int argc, char *argv[]) } } + filter_remove_unused_lports(&ctx, &lports); sset_destroy(&all_lports); unixctl_server_run(unixctl); diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h index 470b1f5..8eb0b23 100644 --- a/ovn/controller/ovn-controller.h +++ b/ovn/controller/ovn-controller.h @@ -40,6 +40,7 @@ struct local_datapath { struct hmap_node hmap_node; struct hmap_node uuid_hmap_node; struct uuid uuid; + int64_t tunnel_key; char *logical_port; const struct sbrec_port_binding *localnet_port; }; @@ -55,6 +56,7 @@ struct patched_datapath { bool local; /* 'True' if the datapath is for gateway router. */ bool stale; /* 'True' if the datapath is not referenced by any patch * port. */ + int64_t tunnel_key; }; struct patched_datapath *get_patched_datapath(const struct hmap *, diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c index 707d08b..72e39c8 100644 --- a/ovn/controller/patch.c +++ b/ovn/controller/patch.c @@ -24,6 +24,7 @@ #include "openvswitch/hmap.h" #include "openvswitch/vlog.h" #include "ovn-controller.h" +#include "filter.h" VLOG_DEFINE_THIS_MODULE(patch); @@ -258,7 +259,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); @@ -273,9 +275,11 @@ add_patched_datapath(struct hmap *patched_datapaths, pd->local = local; pd->key = xasprintf(UUID_FMT, UUID_ARGS(&binding_rec->datapath->header_.uuid)); + pd->tunnel_key = binding_rec->datapath->tunnel_key; /* stale is set to false. */ hmap_insert(patched_datapaths, &pd->hmap_node, binding_rec->datapath->tunnel_key); + filter_datapath(ctx, binding_rec->datapath); } static void @@ -292,7 +296,9 @@ 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 hmap *local_datapaths, + struct controller_ctx *ctx) { /* Clean up stale patched datapaths. */ struct patched_datapath *pd_cur_node, *pd_next_node; @@ -300,6 +306,9 @@ 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); + if (!get_local_datapath(local_datapaths, pd_cur_node->tunnel_key)) { + unfilter_datapath(ctx, pd_cur_node->tunnel_key); + } free(pd_cur_node->key); free(pd_cur_node); } @@ -333,7 +342,9 @@ add_logical_patch_ports(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, const char *local_chassis_id, struct shash *existing_ports, - struct hmap *patched_datapaths) + struct hmap *patched_datapaths, + struct hmap *local_datapaths, + struct lport_index *lports_index) { const struct sbrec_chassis *chassis_rec; chassis_rec = get_chassis(ctx->ovnsb_idl, local_chassis_id); @@ -368,21 +379,26 @@ 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); } } + if (peer) { + if (!lport_lookup_by_name(lports_index, peer)) { + filter_lport(ctx, peer); + } + } } } - add_logical_patch_ports_postprocess(patched_datapaths); + add_logical_patch_ports_postprocess(patched_datapaths, local_datapaths, ctx); } void patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, const char *chassis_id, struct hmap *local_datapaths, - struct hmap *patched_datapaths) + struct hmap *patched_datapaths, struct lport_index *lports_index) { if (!ctx->ovs_idl_txn) { return; @@ -404,7 +420,7 @@ patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, * should be there. */ add_bridge_mappings(ctx, br_int, &existing_ports, local_datapaths, chassis_id); add_logical_patch_ports(ctx, br_int, chassis_id, &existing_ports, - patched_datapaths); + patched_datapaths, local_datapaths, lports_index); /* Now 'existing_ports' only still contains patch ports that exist in the * database but shouldn't. Delete them from the database. */ diff --git a/ovn/controller/patch.h b/ovn/controller/patch.h index 7920a48..6177559 100644 --- a/ovn/controller/patch.h +++ b/ovn/controller/patch.h @@ -25,9 +25,11 @@ struct controller_ctx; struct hmap; struct ovsrec_bridge; +struct lport_index; void patch_run(struct controller_ctx *, const struct ovsrec_bridge *br_int, const char *chassis_id, struct hmap *local_datapaths, - struct hmap *patched_datapaths); + struct hmap *patched_datapaths, + struct lport_index *lports_index); #endif /* ovn/patch.h */ diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index a2349a4..f2962eb 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -92,11 +92,14 @@ AT_CHECK([ovn-sbctl \ -- --id=@dp2 create Datapath_Binding tunnel_key=2 \ -- create Port_Binding datapath=@dp1 logical_port=foo tunnel_key=1 type=patch options:peer=bar \ -- create Port_Binding datapath=@dp2 logical_port=bar tunnel_key=2 type=patch options:peer=foo \ + -- create Port_Binding datapath=@dp1 logical_port=localvif3 tunnel_key=3 \ | ${PERL} $srcdir/uuidfilt.pl], [0], [<0> <1> <2> <3> +<4> ]) +ovs-vsctl add-port br-int localvif3 -- set Interface localvif3 external_ids:iface-id=localvif3 check_patches \ 'br-int patch-br-int-to-localnet2 patch-localnet2-to-br-int' \ 'br-eth0 patch-localnet2-to-br-int patch-br-int-to-localnet2' \ diff --git a/tests/ovn.at b/tests/ovn.at index 86efcf5..0754e7c 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -1356,6 +1356,7 @@ sim_add hv_gw as hv_gw ovs-vsctl add-br br-phys ovn_attach n1 br-phys 192.168.0.3 +ovs-vsctl add-port br-int viflp-gw -- set Interface viflp-gw external-ids:iface-id=lp-gw ovs-vsctl add-br br-phys2 net_attach n2 br-phys2 ovs-vsctl set open . external_ids:ovn-bridge-mappings="physnet1:br-phys2"
Conditional monitor of: Port_Binding, Logical_Flow, Multicast_Group MAC_Binding tables. As a result ovn-controller will be notified only about records belongs to a datapath that is being served by this hypervisor. Performance evaluation: OVN is the main candidate for conditional monitoring usage. It is clear that conditional monitoring reduces computation on the ovn-controller (client) side due to the reduced size of flow tables and update messages. Performance evaluation shows up to 75% computation reduction on the ovn-controller side. However, performance evaluation shows also a reduction in computation on the SB ovsdb-server side proportional to the degree that each logical network is spread over physical hosts in the DC. Evaluation shows that in a realistic scenarios there is a computation reduction also in the server side. Evaluation on simulated environment of 50 hosts and 1000 logical ports shows the following results (cycles #): LN spread over # hosts| master | patch | change ------------------------------------------------------------- 1 | 24597200127 | 24339235374 | 1.0% 6 | 23788521572 | 19145229352 | 19.5% 12 | 23886405758 | 17913143176 | 25.0% 18 | 25812686279 | 23675094540 | 8.2% 24 | 28414671499 | 24770202308 | 12.8% 30 | 31487218890 | 28397543436 | 9.8% 36 | 36116993930 | 34105388739 | 5.5% 42 | 37898342465 | 38647139083 | -1.9% 48 | 41637996229 | 41846616306 | -0.5% 50 | 41679995357 | 43455565977 | -4.2% Signed-off-by: Liran Schour <lirans@il.ibm.com> --- ovn/controller/automake.mk | 4 +- ovn/controller/binding.c | 45 ++++++--- ovn/controller/binding.h | 4 +- ovn/controller/filter.c | 207 ++++++++++++++++++++++++++++++++++++++++ ovn/controller/filter.h | 36 +++++++ ovn/controller/ovn-controller.c | 18 +++- ovn/controller/ovn-controller.h | 2 + ovn/controller/patch.c | 30 ++++-- ovn/controller/patch.h | 4 +- tests/ovn-controller.at | 3 + tests/ovn.at | 1 + 11 files changed, 327 insertions(+), 27 deletions(-) create mode 100644 ovn/controller/filter.c create mode 100644 ovn/controller/filter.h